[PATCH 1/1] xen/blkfront: fix comment for need_copy

[PATCH 1/1] xen/blkfront: fix comment for need_copy

[Dongli Zhang]

The 'need_copy' is set when rq_data_dir(req) returns WRITE, in order to
copy the written data to persistent page.

".need_copy = rq_data_dir(req) && info->feature_persistent,"

Signed-off-by: Dongli Zhang <dongli.zhang@oracle.com>
---
 drivers/block/xen-blkfront.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/block/xen-blkfront.c b/drivers/block/xen-blkfront.c
index 03b5fb341e58..dbc32d0a4b1a 100644
--- a/drivers/block/xen-blkfront.c
+++ b/drivers/block/xen-blkfront.c
@@ -576,7 +576,7 @@ struct setup_rw_req {
 	struct blkif_request *ring_req;
 	grant_ref_t gref_head;
 	unsigned int id;
-	/* Only used when persistent grant is used and it's a read request */
+	/* Only used when persistent grant is used and it's a write request */
 	bool need_copy;
 	unsigned int bvec_off;
 	char *bvec_data;
-- 
2.17.1

Re: [PATCH 1/1] xen/blkfront: fix comment for need_copy

[Chaitanya Kulkarni]

On 3/17/22 3:09 PM, Dongli Zhang wrote:
> The 'need_copy' is set when rq_data_dir(req) returns WRITE, in order to
> copy the written data to persistent page.
> 
> ".need_copy = rq_data_dir(req) && info->feature_persistent,"
> 
> Signed-off-by: Dongli Zhang <dongli.zhang@oracle.com>
> ---

Looks good.

Reviewed-by: Chaitanya Kulkarni <kch@nvidia.com>

-ck

Re: [PATCH 1/1] xen/blkfront: fix comment for need_copy

[Roger Pau Monné]

On Thu, Mar 17, 2022 at 03:09:30PM -0700, Dongli Zhang wrote:
> The 'need_copy' is set when rq_data_dir(req) returns WRITE, in order to
> copy the written data to persistent page.
> 
> ".need_copy = rq_data_dir(req) && info->feature_persistent,"

I would also add:

Fixes: c004a6fe0c40 ('block/xen-blkfront: Make it running on 64KB page granularity')

> Signed-off-by: Dongli Zhang <dongli.zhang@oracle.com>

Acked-by: Roger Pau Monné <roger.pau@citrix.com>

Albeit I have one nit since you are already changing the line.

> ---
>  drivers/block/xen-blkfront.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/block/xen-blkfront.c b/drivers/block/xen-blkfront.c
> index 03b5fb341e58..dbc32d0a4b1a 100644
> --- a/drivers/block/xen-blkfront.c
> +++ b/drivers/block/xen-blkfront.c
> @@ -576,7 +576,7 @@ struct setup_rw_req {
>  	struct blkif_request *ring_req;
>  	grant_ref_t gref_head;
>  	unsigned int id;
> -	/* Only used when persistent grant is used and it's a read request */
> +	/* Only used when persistent grant is used and it's a write request */

While there you might want to adjust the comment to:

"... persistent grants are used ..."

Thanks, Roger.

Re: [PATCH 1/1] xen/blkfront: fix comment for need_copy

[Juergen Gross]

[-- Attachment #1.1.1: Type: text/plain, Size: 852 bytes --]

On 21.03.22 11:05, Roger Pau Monné wrote:
> On Thu, Mar 17, 2022 at 03:09:30PM -0700, Dongli Zhang wrote:
>> The 'need_copy' is set when rq_data_dir(req) returns WRITE, in order to
>> copy the written data to persistent page.
>>
>> ".need_copy = rq_data_dir(req) && info->feature_persistent,"
> 
> I would also add:
> 
> Fixes: c004a6fe0c40 ('block/xen-blkfront: Make it running on 64KB page granularity')

Hmm, a "Fixes:" tag for a change in a comment?

This might generate additional work e.g. for downstreams (we at SUSE have
scripts checking "Fixes:" tags and require such changes to be applied to
kernels having the fixed patch applied).

That said I'd prefer not having a "Fixes:" tag for such changes, but maybe
this is just due to the fact that it would be me having to apply this
patch to the SUSE kernels...


Juergen

[-- Attachment #1.1.2: OpenPGP public key --]
[-- Type: application/pgp-keys, Size: 3149 bytes --]

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 495 bytes --]

Re: [PATCH 1/1] xen/blkfront: fix comment for need_copy

[Jens Axboe]

On Thu, 17 Mar 2022 15:09:30 -0700, Dongli Zhang wrote:
> The 'need_copy' is set when rq_data_dir(req) returns WRITE, in order to
> copy the written data to persistent page.
> 
> ".need_copy = rq_data_dir(req) && info->feature_persistent,"
> 
> 

Applied, thanks!

[1/1] xen/blkfront: fix comment for need_copy
      commit: 08719dd9176b4c55f547bd11812fd6cc35907d37

Best regards,
-- 
Jens Axboe