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

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

[Juergen Gross]

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


OpenPGP_0xB0DE9DD628BF132F.asc
Description: OpenPGP public key


OpenPGP_signature
Description: OpenPGP digital signature

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 

Acked-by: Roger Pau Monné 

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

[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 
> ---

Looks good.

Reviewed-by: Chaitanya Kulkarni 

-ck

[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 
---
 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