Re: [PATCH RFC] mm: count zeromap read and set for swapout and swapin

[Usama Arif <usamaarif642@gmail.com>]

On 28/10/2024 19:54, Barry Song wrote:
> On Tue, Oct 29, 2024 at 1:20 AM Usama Arif <usamaarif642@gmail.com> wrote:
>>
>>
>>
>> On 28/10/2024 17:08, Yosry Ahmed wrote:
>>> On Mon, Oct 28, 2024 at 10:00 AM Usama Arif <usamaarif642@gmail.com> wrote:
>>>>
>>>>
>>>>
>>>> On 28/10/2024 16:33, Nhat Pham wrote:
>>>>> On Mon, Oct 28, 2024 at 5:23 AM Usama Arif <usamaarif642@gmail.com> wrote:
>>>>>>
>>>>>> I wonder if instead of having counters, it might be better to keep track
>>>>>> of the number of zeropages currently stored in zeromap, similar to how
>>>>>> zswap_same_filled_pages did it. It will be more complicated then this
>>>>>> patch, but would give more insight of the current state of the system.
>>>>>>
>>>>>> Joshua (in CC) was going to have a look at that.
>>>>>
>>>>> I don't think one can substitute for the other.
>>>>
>>>> Yes agreed, they have separate uses and provide different information, but
>>>> maybe wasteful to have both types of counters? They are counters so maybe
>>>> dont consume too much resources but I think we should still think about
>>>> it..
>>>
>>> Not for or against here, but I would say that statement is debatable
>>> at best for memcg stats :)
>>>
>>> Each new counter consumes 2 longs per-memcg per-CPU (see
>>> memcg_vmstats_percpu), about 16 bytes, which is not a lot but it can
>>> quickly add up with a large number of CPUs/memcgs/stats.
>>>
>>> Also, when flushing the stats we iterate all of them to propagate
>>> updates from per-CPU counters. This is already a slowpath so adding
>>> one stat is not a big deal, but again because we iterate all stats on
>>> multiple CPUs (and sometimes on each node as well), the overall flush
>>> latency becomes a concern sometimes.
>>>
>>> All of that is not to say we shouldn't add more memcg stats, but we
>>> have to be mindful of the resources.
>>
>> Yes agreed! Plus the cost of incrementing similar counters (which ofcourse is
>> also not much).
>>
>> Not trying to block this patch in anyway. Just think its a good point
>> to discuss here if we are ok with both types of counters. If its too wasteful
>> then which one we should have.
> 
> Hi Usama,
> my point is that with all the below three counters:
> 1. PSWPIN/PSWPOUT
> 2. ZSWPIN/ZSWPOUT
> 3. SWAPIN_SKIP/SWAPOUT_SKIP or (ZEROSWPIN, ZEROSWPOUT what ever)
> 
> Shouldn't we have been able to determine the portion of zeromap
> swap indirectly?
> 

Hmm, I might be wrong, but I would have thought no?

What if you swapout a zero folio, but then discard it?
zeromap_swpout would be incremented, but zeromap_swapin would not.

> Thanks
> Barry