Re: [PATCH 4/4] zcache: add pageframes count once compress zero-filled pages twice

[Wanpeng Li <liwanp@linux.vnet.ibm.com>]

On Thu, Mar 14, 2013 at 09:10:48AM -0700, Dan Magenheimer wrote:
>> From: Wanpeng Li [mailto:liwanp@linux.vnet.ibm.com]
>> Sent: Wednesday, March 13, 2013 6:21 PM
>> To: Dan Magenheimer
>> Cc: Andrew Morton; Greg Kroah-Hartman; Dan Magenheimer; Seth Jennings; Konrad Rzeszutek Wilk; Minchan
>> Kim; linux-mm@kvack.org; linux-kernel@vger.kernel.org
>> Subject: Re: [PATCH 4/4] zcache: add pageframes count once compress zero-filled pages twice
>> 
>> On Wed, Mar 13, 2013 at 09:42:16AM -0700, Dan Magenheimer wrote:
>> >> From: Wanpeng Li [mailto:liwanp@linux.vnet.ibm.com]
>> >> Sent: Wednesday, March 13, 2013 1:05 AM
>> >> To: Andrew Morton
>> >> Cc: Greg Kroah-Hartman; Dan Magenheimer; Seth Jennings; Konrad Rzeszutek Wilk; Minchan Kim; linux-
>> >> mm@kvack.org; linux-kernel@vger.kernel.org; Wanpeng Li
>> >> Subject: [PATCH 4/4] zcache: add pageframes count once compress zero-filled pages twice
>> >
>> >Hi Wanpeng --
>> >
>> >Thanks for taking on this task from the drivers/staging/zcache TODO list!
>> >
>> >> Since zbudpage consist of two zpages, two zero-filled pages compression
>> >> contribute to one [eph|pers]pageframe count accumulated.
>> >
>> 
>> Hi Dan,
>> 
>> >I'm not sure why this is necessary.  The [eph|pers]pageframe count
>> >is supposed to be counting actual pageframes used by zcache.  Since
>> >your patch eliminates the need to store zero pages, no pageframes
>> >are needed at all to store zero pages, so it's not necessary
>> >to increment zcache_[eph|pers]_pageframes when storing zero
>> >pages.
>> >
>> 
>> Great point! It seems that we also don't need to caculate
>> zcache_[eph|pers]_zpages for zero-filled pages. I will fix
>> it in next version. :-)
>
>Hi Wanpeng --
>

Hi Dan,

>I think we DO need to increment/decrement zcache_[eph|pers]_zpages
>for zero-filled pages.
>
>The main point of the counters for zpages and pageframes
>is to be able to calculate density == zpages/pageframes.
>A zero-filled page becomes a zpage that "compresses" to zero bytes
>and, as a result, requires zero pageframes for storage.
>So the zpages counter should be increased but the pageframes
>counter should not.

It is reasonable to me, I will increment/decrement zcache_[eph|pers]_zpages
in next version.

>
>If you are changing the patch anyway, I do like better the use
>of "zero_filled_page" rather than just "zero" or "zero page".
>So it might be good to change:
>
>handle_zero_page -> handle_zero_filled_page
>pages_zero -> zero_filled_pages
>zcache_pages_zero -> zcache_zero_filled_pages
>
>and maybe
>
>page_zero_filled -> page_is_zero_filled

Great rename! :-)

Regards,
Wanpeng Li 

>
>Thanks,
>Dan
>
>--
>To unsubscribe, send a message with 'unsubscribe linux-mm' in
>the body to majordomo@kvack.org.  For more info on Linux MM,
>see: http://www.linux-mm.org/ .
>Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>