> -----Original Message----- > From: Yosry Ahmed <yosryahmed@xxxxxxxxxx> > Sent: Tuesday, October 1, 2024 10:01 AM > To: Sridhar, Kanchana P <kanchana.p.sridhar@xxxxxxxxx> > Cc: linux-kernel@xxxxxxxxxxxxxxx; linux-mm@xxxxxxxxx; > hannes@xxxxxxxxxxx; nphamcs@xxxxxxxxx; chengming.zhou@xxxxxxxxx; > usamaarif642@xxxxxxxxx; shakeel.butt@xxxxxxxxx; ryan.roberts@xxxxxxx; > Huang, Ying <ying.huang@xxxxxxxxx>; 21cnbao@xxxxxxxxx; akpm@linux- > foundation.org; willy@xxxxxxxxxxxxx; Zou, Nanhai <nanhai.zou@xxxxxxxxx>; > Feghali, Wajdi K <wajdi.k.feghali@xxxxxxxxx>; Gopal, Vinodh > <vinodh.gopal@xxxxxxxxx> > Subject: Re: [PATCH v9 6/7] mm: zswap: Support large folios in zswap_store(). > > On Tue, Oct 1, 2024 at 9:58 AM Sridhar, Kanchana P > <kanchana.p.sridhar@xxxxxxxxx> wrote: > > > > > -----Original Message----- > > > From: Yosry Ahmed <yosryahmed@xxxxxxxxxx> > > > Sent: Monday, September 30, 2024 11:00 PM > > > To: Sridhar, Kanchana P <kanchana.p.sridhar@xxxxxxxxx> > > > Cc: linux-kernel@xxxxxxxxxxxxxxx; linux-mm@xxxxxxxxx; > > > hannes@xxxxxxxxxxx; nphamcs@xxxxxxxxx; > chengming.zhou@xxxxxxxxx; > > > usamaarif642@xxxxxxxxx; shakeel.butt@xxxxxxxxx; > ryan.roberts@xxxxxxx; > > > Huang, Ying <ying.huang@xxxxxxxxx>; 21cnbao@xxxxxxxxx; akpm@linux- > > > foundation.org; willy@xxxxxxxxxxxxx; Zou, Nanhai > <nanhai.zou@xxxxxxxxx>; > > > Feghali, Wajdi K <wajdi.k.feghali@xxxxxxxxx>; Gopal, Vinodh > > > <vinodh.gopal@xxxxxxxxx> > > > Subject: Re: [PATCH v9 6/7] mm: zswap: Support large folios in > zswap_store(). > > > > > > [..] > > > > > > store_failed: > > > > > > zpool_free(entry->pool->zpool, entry->handle); > > > > > > -put_pool: > > > > > > - zswap_pool_put(entry->pool); > > > > > > -freepage: > > > > > > +put_pool_objcg: > > > > > > + zswap_pool_put(pool); > > > > > > + obj_cgroup_put(objcg); > > > > > > > > > > I think if we reorder the function we can drop these calls, make the > > > > > comments positioned a bit better, and centralize the entry > > > > > initializations. I am also not a fan of passing a semi-initialized > > > > > entry to zswap_compress() to get the pool pointer. > > > > > > > > > > Does the following diff improve things or did I miss something? > > > > > > > > We shouldn’t be adding the entry to the xarray before initializing its pool > > > > and objcg, right? Please let me know if I am misunderstanding what > you're > > > > proposing in the diff. > > > > > > It should be safe. We already initialize entry->lru after we insert > > > the entry in the tree. See the comment above the call to > > > zswap_lru_add(). Basically we are protected against concurrent > > > stores/loads through the folio lock, and are protected against > > > writeback because the entry is not on the LRU yet. > > > > Thanks for the clarification, Yosry. Since this is a change in the entry > > initialization wrt the mainline, is it Ok if this is done in a follow-up patch? > > Sure. We can discuss it separately. Do you want me to send a patch or > do you intend to? Thanks Yosry! I will send the patch separately.
