| Message ID | 20200228150518.10496-5-guoqing.jiang@cloud.ionos.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Some cleanups for blk-core.c and blk-flush.c | expand |
On 2020-02-28 07:05, Guoqing Jiang wrote: > Both cmd and sense had been moved to scsi_request, so remove > the related comments to avoid confusion. > > Signed-off-by: Guoqing Jiang <guoqing.jiang@cloud.ionos.com> > --- > block/blk-core.c | 3 --- > 1 file changed, 3 deletions(-) > > diff --git a/block/blk-core.c b/block/blk-core.c > index 883ffda216e4..9094fd7d1b01 100644 > --- a/block/blk-core.c > +++ b/block/blk-core.c > @@ -1583,7 +1583,6 @@ EXPORT_SYMBOL_GPL(blk_rq_unprep_clone); > > /* > * Copy attributes of the original request to the clone request. > - * The actual data parts (e.g. ->cmd, ->sense) are not copied. > */ > static void __blk_rq_prep_clone(struct request *dst, struct request *src) > { Although the removed comment is outdated, the new comment is not useful. How about inlining __blk_rq_prep_clone() into its only caller and removing the comment above that function entirely? It's not clear to me why the code inside __blk_rq_prep_clone() ever was put into a separate function. > * > * Description: > * Clones bios in @rq_src to @rq, and copies attributes of @rq_src to @rq. > - * The actual data parts of @rq_src (e.g. ->cmd, ->sense) > - * are not copied, and copying such parts is the caller's responsibility. > * Also, pages which the original bios are pointing to are not copied > * and the cloned bios just point same pages. > * So cloned bios must be completed before original bios, which means Adding a comment that explains that some but not all struct request members are copied and also why would be welcome. Thanks, Bart.
Hi Bart, Thanks for your review. On 2/29/20 3:34 AM, Bart Van Assche wrote: > On 2020-02-28 07:05, Guoqing Jiang wrote: >> Both cmd and sense had been moved to scsi_request, so remove >> the related comments to avoid confusion. >> >> Signed-off-by: Guoqing Jiang <guoqing.jiang@cloud.ionos.com> >> --- >> block/blk-core.c | 3 --- >> 1 file changed, 3 deletions(-) >> >> diff --git a/block/blk-core.c b/block/blk-core.c >> index 883ffda216e4..9094fd7d1b01 100644 >> --- a/block/blk-core.c >> +++ b/block/blk-core.c >> @@ -1583,7 +1583,6 @@ EXPORT_SYMBOL_GPL(blk_rq_unprep_clone); >> >> /* >> * Copy attributes of the original request to the clone request. >> - * The actual data parts (e.g. ->cmd, ->sense) are not copied. >> */ >> static void __blk_rq_prep_clone(struct request *dst, struct request *src) >> { > > Although the removed comment is outdated, the new comment is not useful. > > How about inlining __blk_rq_prep_clone() into its only caller and > removing the comment above that function entirely?It's not clear to me > why the code inside __blk_rq_prep_clone() ever was put into a separate > function. Not sure about it, the original code was introduced in commit b0fd271d5fba0 ("block: add request clone interface (v2)"), maybe author preferred to use a function to copy attributes from src request to dst request. I will make the change based on your suggestion if no one objects it. > >> * >> * Description: >> * Clones bios in @rq_src to @rq, and copies attributes of @rq_src to @rq. >> - * The actual data parts of @rq_src (e.g. ->cmd, ->sense) >> - * are not copied, and copying such parts is the caller's responsibility. >> * Also, pages which the original bios are pointing to are not copied >> * and the cloned bios just point same pages. >> * So cloned bios must be completed before original bios, which means > > Adding a comment that explains that some but not all struct request > members are copied and also why would be welcome. I think we need care about the actual data parts of request, I guess all the actual data parts had been moved into scsi_request, but my understanding could probably be wrong. Thanks, Guoqing
diff --git a/block/blk-core.c b/block/blk-core.c index 883ffda216e4..9094fd7d1b01 100644 --- a/block/blk-core.c +++ b/block/blk-core.c @@ -1583,7 +1583,6 @@ EXPORT_SYMBOL_GPL(blk_rq_unprep_clone); /* * Copy attributes of the original request to the clone request. - * The actual data parts (e.g. ->cmd, ->sense) are not copied. */ static void __blk_rq_prep_clone(struct request *dst, struct request *src) { @@ -1610,8 +1609,6 @@ static void __blk_rq_prep_clone(struct request *dst, struct request *src) * * Description: * Clones bios in @rq_src to @rq, and copies attributes of @rq_src to @rq. - * The actual data parts of @rq_src (e.g. ->cmd, ->sense) - * are not copied, and copying such parts is the caller's responsibility. * Also, pages which the original bios are pointing to are not copied * and the cloned bios just point same pages. * So cloned bios must be completed before original bios, which means
Both cmd and sense had been moved to scsi_request, so remove the related comments to avoid confusion. Signed-off-by: Guoqing Jiang <guoqing.jiang@cloud.ionos.com> --- block/blk-core.c | 3 --- 1 file changed, 3 deletions(-)