On Thu, Mar 21, 2024 at 12:58 PM Yosry Ahmed <yosryahmed@xxxxxxxxxx> wrote: > > On Thu, Mar 21, 2024 at 12:29 PM Johannes Weiner <hannes@xxxxxxxxxxx> wrote: > > > > On Thu, Mar 21, 2024 at 06:44:54PM +0000, Yosry Ahmed wrote: > > > > In some cases where the current objcg is not "correct", the testcases in > > > > test_zswap.c may break? Maybe we can use swap_cgroup info to charge the > > > > stats to the correct memcg? Not sure if this is feasible. > > > > > > For cgroup v1, swap_cgroup will be cleared from > > > mem_cgroup_swapin_uncharge_swap() before the zswap load. > > > > > > I think the current objcg will remain correct as long as swapin happens > > > from the same memcg as swapout (or if swapin happens from the parent > > > memcg and the swapout memcg was offlined). > > > > Swap readahead will pull in physically adjacent entries that may > > belong to somebody unrelated. > > Right. For those as well the current objcg would be correct if they > are readahead from the same memcg as the one they were swapped out > from, but I understand your point that readahead makes that more > likely to not be the case. > > I am slightly nervous about using the current objcg tbh, even though > it only affects the stats. It's just less straightforward this way. I > think I prefer either: > (a) Only supporting zero-filled pages and storing the objcg directly > in the xarray. We can add the zero-filled page as a special case to speed it up. I don't think we should remove non zero values of the same fill pages from zswap though. We can never declare the non zero same filled page is not going to happen. In that case, the current same fill is still better than going through the zsmalloc. Chris > (b) Having a separate two-word struct to store objcg and value for > same-filled pages. > > In both cases, we would need one tag bit to identify same-filled pages > in the xarray. (a) is more tempting for me, but I am not sure everyone > else agrees with dropping support for non-zero same-filled pages.
