Re: [PATCH v4 2/2] mm: remove code to handle same filled pages

[Usama Arif <usamaarif642@gmail.com>]

On 12/06/2024 16:09, Nhat Pham wrote:
> On Wed, Jun 12, 2024 at 5:47 AM Usama Arif <usamaarif642@gmail.com> wrote:
>> With an earlier commit to handle zero-filled pages in swap directly,
>> and with only 1% of the same-filled pages being non-zero, zswap no
>> longer needs to handle same-filled pages and can just work on compressed
>> pages.
>>
>> Signed-off-by: Usama Arif <usamaarif642@gmail.com>
>> ---
>>   mm/zswap.c | 86 +++++-------------------------------------------------
>>   1 file changed, 8 insertions(+), 78 deletions(-)
>>
>> diff --git a/mm/zswap.c b/mm/zswap.c
>> index b9b35ef86d9b..ca8df9c99abf 100644
>> --- a/mm/zswap.c
>> +++ b/mm/zswap.c
>> @@ -44,8 +44,6 @@
>>   **********************************/
>>   /* The number of compressed pages currently stored in zswap */
>>   atomic_t zswap_stored_pages = ATOMIC_INIT(0);
>> -/* The number of same-value filled pages currently stored in zswap */
>> -static atomic_t zswap_same_filled_pages = ATOMIC_INIT(0);
> Can we re-introduce this counter somewhere? I have found this counter
> to be valuable in the past, and would love to keep it. For instance,
> in a system where a lot of zero-filled pages are reclaimed, we might
> see an increase in swap usage. Having this counter at hands will allow
> us to trace the source of the swap usage more easily. I *suppose* you
> can do elimination work (check zswap usage, check disk swap usage
> somehow - I'm not sure, etc., etc.), but this would be much more
> direct and user-friendly.
>
> Not *entirely* sure where to expose this though. Seems a bit specific
> for vmstat. Maybe as a new debugfs directory?
>
> This doesn't have to be done in this patch (or this series even) - but
> please consider this for follow-up work at least :)

Yes, would be good to have this . There are 2 things to consider: where 
to put it in debugfs as you pointed out, and where to decrement it. 
Decrementing it will require to test and clear instead of clear zeromap 
at different places, so might be a bit more complicated compared to how 
zswap_same_filled_pages is tracked. I think best to keep this separate 
from this series.