On 28/10/2024 19:54, Barry Song wrote: > On Tue, Oct 29, 2024 at 1:20 AM Usama Arif <usamaarif642@xxxxxxxxx> wrote: >> >> >> >> On 28/10/2024 17:08, Yosry Ahmed wrote: >>> On Mon, Oct 28, 2024 at 10:00 AM Usama Arif <usamaarif642@xxxxxxxxx> wrote: >>>> >>>> >>>> >>>> On 28/10/2024 16:33, Nhat Pham wrote: >>>>> On Mon, Oct 28, 2024 at 5:23 AM Usama Arif <usamaarif642@xxxxxxxxx> 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
