Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1710707 > unrolled thread

Re: [PATCH -next] zram: fix compr_data_size stat

Started byMinchan Kim <minchan@kernel.org>
First post2017-08-14 10:10 +0200
Last post2017-08-14 13:00 +0200
Articles 4 — 2 participants

Back to article view | Back to linux.kernel

This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by below is the oldest one visible, not the original post.

Contents

  Re: [PATCH -next] zram: fix compr_data_size stat Minchan Kim <minchan@kernel.org> - 2017-08-14 10:10 +0200
    Re: [PATCH -next] zram: fix compr_data_size stat Yisheng Xie <xieyisheng1@huawei.com> - 2017-08-14 11:30 +0200
      Re: [PATCH -next] zram: fix compr_data_size stat Minchan Kim <minchan@kernel.org> - 2017-08-14 11:40 +0200
        Re: [PATCH -next] zram: fix compr_data_size stat Yisheng Xie <xieyisheng1@huawei.com> - 2017-08-14 13:00 +0200

#1710707 — Re: [PATCH -next] zram: fix compr_data_size stat

FromMinchan Kim <minchan@kernel.org>
Date2017-08-14 10:10 +0200
SubjectRe: [PATCH -next] zram: fix compr_data_size stat
Message-ID<ueiBH-1HF-5@gated-at.bofh.it>
Hi Yisheng,

Thanks for the all fixup!
A minor nitpick below:

On Fri, Aug 11, 2017 at 09:10:32AM +0800, Yisheng Xie wrote:
> The compr_data_size is a stat for compressed size of pages stored, which
> should add comp_len when we compresse a page.
> 
> Meanwhile fix typos in comment:
> * ZRAM_SAME means page consists the same element not the entirely zero page
> * read_from_bdev_async() return 1 to avoid call page_endio() in zram_rw_page()
> 
> Signed-off-by: Yisheng Xie <xieyisheng1@huawei.com>
> ---
>  drivers/block/zram/zram_drv.c | 3 ++-
>  drivers/block/zram/zram_drv.h | 2 +-
>  2 files changed, 3 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/block/zram/zram_drv.c b/drivers/block/zram/zram_drv.c
> index e27daca..b2b75b1 100644
> --- a/drivers/block/zram/zram_drv.c
> +++ b/drivers/block/zram/zram_drv.c
> @@ -455,7 +455,7 @@ void zram_page_end_io(struct bio *bio)
>  }
>  
>  /*
> - * Returns 0 if the submission is successful.
> + * Returns 1 if the submission is successful.
>   */
>  static int read_from_bdev_async(struct zram *zram, struct bio_vec *bvec,
>  			unsigned long entry, struct bio *parent)
> @@ -1073,6 +1073,7 @@ static int __zram_bvec_write(struct zram *zram, struct bio_vec *bvec,
>  	}  else {
>  		zram_set_handle(zram, index, handle);
>  		zram_set_obj_size(zram, index, comp_len);
> +		atomic64_add(comp_len, &zram->stats.compr_data_size);

I want to add the accounting line to zs_unmap_object.
Otherwise, looks good to me.
Thanks!

[toc] | [next] | [standalone]

#1710745

FromYisheng Xie <xieyisheng1@huawei.com>
Date2017-08-14 11:30 +0200
Message-ID<uejR8-2n2-5@gated-at.bofh.it>
In reply to#1710707
Hi, Minchan

Thanks for your comment!
On 2017/8/14 16:08, Minchan Kim wrote:
> Hi Yisheng,
> 
> Thanks for the all fixup!
> A minor nitpick below:
> 
> On Fri, Aug 11, 2017 at 09:10:32AM +0800, Yisheng Xie wrote:
>> The compr_data_size is a stat for compressed size of pages stored, which
>> should add comp_len when we compresse a page.
>>
>> Meanwhile fix typos in comment:
>> * ZRAM_SAME means page consists the same element not the entirely zero page
>> * read_from_bdev_async() return 1 to avoid call page_endio() in zram_rw_page()
>>
>> Signed-off-by: Yisheng Xie <xieyisheng1@huawei.com>
>> ---
>>  drivers/block/zram/zram_drv.c | 3 ++-
>>  drivers/block/zram/zram_drv.h | 2 +-
>>  2 files changed, 3 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/block/zram/zram_drv.c b/drivers/block/zram/zram_drv.c
>> index e27daca..b2b75b1 100644
>> --- a/drivers/block/zram/zram_drv.c
>> +++ b/drivers/block/zram/zram_drv.c
>> @@ -455,7 +455,7 @@ void zram_page_end_io(struct bio *bio)
>>  }
>>  
>>  /*
>> - * Returns 0 if the submission is successful.
>> + * Returns 1 if the submission is successful.
>>   */
>>  static int read_from_bdev_async(struct zram *zram, struct bio_vec *bvec,
>>  			unsigned long entry, struct bio *parent)
>> @@ -1073,6 +1073,7 @@ static int __zram_bvec_write(struct zram *zram, struct bio_vec *bvec,
>>  	}  else {
>>  		zram_set_handle(zram, index, handle);
>>  		zram_set_obj_size(zram, index, comp_len);
>> +		atomic64_add(comp_len, &zram->stats.compr_data_size);
> 
> I want to add the accounting line to zs_unmap_object.

Right, this accounting can be done without slot lock!

You will fold this patch? or should I send another version?

Thanks
Yisheng

[toc] | [prev] | [next] | [standalone]

#1710751

FromMinchan Kim <minchan@kernel.org>
Date2017-08-14 11:40 +0200
Message-ID<uek0N-2qo-3@gated-at.bofh.it>
In reply to#1710745
On Mon, Aug 14, 2017 at 05:24:16PM +0800, Yisheng Xie wrote:
> Hi, Minchan
> 
> Thanks for your comment!
> On 2017/8/14 16:08, Minchan Kim wrote:
> > Hi Yisheng,
> > 
> > Thanks for the all fixup!
> > A minor nitpick below:
> > 
> > On Fri, Aug 11, 2017 at 09:10:32AM +0800, Yisheng Xie wrote:
> >> The compr_data_size is a stat for compressed size of pages stored, which
> >> should add comp_len when we compresse a page.
> >>
> >> Meanwhile fix typos in comment:
> >> * ZRAM_SAME means page consists the same element not the entirely zero page
> >> * read_from_bdev_async() return 1 to avoid call page_endio() in zram_rw_page()
> >>
> >> Signed-off-by: Yisheng Xie <xieyisheng1@huawei.com>
> >> ---
> >>  drivers/block/zram/zram_drv.c | 3 ++-
> >>  drivers/block/zram/zram_drv.h | 2 +-
> >>  2 files changed, 3 insertions(+), 2 deletions(-)
> >>
> >> diff --git a/drivers/block/zram/zram_drv.c b/drivers/block/zram/zram_drv.c
> >> index e27daca..b2b75b1 100644
> >> --- a/drivers/block/zram/zram_drv.c
> >> +++ b/drivers/block/zram/zram_drv.c
> >> @@ -455,7 +455,7 @@ void zram_page_end_io(struct bio *bio)
> >>  }
> >>  
> >>  /*
> >> - * Returns 0 if the submission is successful.
> >> + * Returns 1 if the submission is successful.
> >>   */
> >>  static int read_from_bdev_async(struct zram *zram, struct bio_vec *bvec,
> >>  			unsigned long entry, struct bio *parent)
> >> @@ -1073,6 +1073,7 @@ static int __zram_bvec_write(struct zram *zram, struct bio_vec *bvec,
> >>  	}  else {
> >>  		zram_set_handle(zram, index, handle);
> >>  		zram_set_obj_size(zram, index, comp_len);
> >> +		atomic64_add(comp_len, &zram->stats.compr_data_size);
> > 
> > I want to add the accounting line to zs_unmap_object.
> 
> Right, this accounting can be done without slot lock!
> 
> You will fold this patch? or should I send another version?

Could you separate same element comment part and others?
With that, we can apply same element comment correctpart
and fold other parts which fixes writeback feature bugs
to original patches.

Thanks!

[toc] | [prev] | [next] | [standalone]

#1710814

FromYisheng Xie <xieyisheng1@huawei.com>
Date2017-08-14 13:00 +0200
Message-ID<uelgd-389-9@gated-at.bofh.it>
In reply to#1710751

On 2017/8/14 17:34, Minchan Kim wrote:
> On Mon, Aug 14, 2017 at 05:24:16PM +0800, Yisheng Xie wrote:
>> Hi, Minchan
>>
>> Thanks for your comment!
>> On 2017/8/14 16:08, Minchan Kim wrote:
>>> Hi Yisheng,
>>>
>>> Thanks for the all fixup!
>>> A minor nitpick below:
>>>
>>> On Fri, Aug 11, 2017 at 09:10:32AM +0800, Yisheng Xie wrote:
>>>> The compr_data_size is a stat for compressed size of pages stored, which
>>>> should add comp_len when we compresse a page.
>>>>
>>>> Meanwhile fix typos in comment:
>>>> * ZRAM_SAME means page consists the same element not the entirely zero page
>>>> * read_from_bdev_async() return 1 to avoid call page_endio() in zram_rw_page()
>>>>
>>>> Signed-off-by: Yisheng Xie <xieyisheng1@huawei.com>
>>>> ---
>>>>  drivers/block/zram/zram_drv.c | 3 ++-
>>>>  drivers/block/zram/zram_drv.h | 2 +-
>>>>  2 files changed, 3 insertions(+), 2 deletions(-)
>>>>
>>>> diff --git a/drivers/block/zram/zram_drv.c b/drivers/block/zram/zram_drv.c
>>>> index e27daca..b2b75b1 100644
>>>> --- a/drivers/block/zram/zram_drv.c
>>>> +++ b/drivers/block/zram/zram_drv.c
>>>> @@ -455,7 +455,7 @@ void zram_page_end_io(struct bio *bio)
>>>>  }
>>>>  
>>>>  /*
>>>> - * Returns 0 if the submission is successful.
>>>> + * Returns 1 if the submission is successful.
>>>>   */
>>>>  static int read_from_bdev_async(struct zram *zram, struct bio_vec *bvec,
>>>>  			unsigned long entry, struct bio *parent)
>>>> @@ -1073,6 +1073,7 @@ static int __zram_bvec_write(struct zram *zram, struct bio_vec *bvec,
>>>>  	}  else {
>>>>  		zram_set_handle(zram, index, handle);
>>>>  		zram_set_obj_size(zram, index, comp_len);
>>>> +		atomic64_add(comp_len, &zram->stats.compr_data_size);
>>>
>>> I want to add the accounting line to zs_unmap_object.
>>
>> Right, this accounting can be done without slot lock!
>>
>> You will fold this patch? or should I send another version?
> 
> Could you separate same element comment part and others?

Of course.

> With that, we can apply same element comment correctpart
> and fold other parts which fixes writeback feature bugs
> to original patches.
> 
> Thanks!
> 
> .
>

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web