RE: [PATCH v9 6/7] mm: zswap: Support large folios in zswap_store().

["Sridhar, Kanchana P" <kanchana.p.sridhar@intel.com>]

> -----Original Message-----
> From: Yosry Ahmed <yosryahmed@google.com>
> Sent: Tuesday, October 1, 2024 10:01 AM
> To: Sridhar, Kanchana P <kanchana.p.sridhar@intel.com>
> Cc: linux-kernel@vger.kernel.org; linux-mm@kvack.org;
> hannes@cmpxchg.org; nphamcs@gmail.com; chengming.zhou@linux.dev;
> usamaarif642@gmail.com; shakeel.butt@linux.dev; ryan.roberts@arm.com;
> Huang, Ying <ying.huang@intel.com>; 21cnbao@gmail.com; akpm@linux-
> foundation.org; willy@infradead.org; Zou, Nanhai <nanhai.zou@intel.com>;
> Feghali, Wajdi K <wajdi.k.feghali@intel.com>; Gopal, Vinodh
> <vinodh.gopal@intel.com>
> 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@intel.com> wrote:
> >
> > > -----Original Message-----
> > > From: Yosry Ahmed <yosryahmed@google.com>
> > > Sent: Monday, September 30, 2024 11:00 PM
> > > To: Sridhar, Kanchana P <kanchana.p.sridhar@intel.com>
> > > Cc: linux-kernel@vger.kernel.org; linux-mm@kvack.org;
> > > hannes@cmpxchg.org; nphamcs@gmail.com;
> chengming.zhou@linux.dev;
> > > usamaarif642@gmail.com; shakeel.butt@linux.dev;
> ryan.roberts@arm.com;
> > > Huang, Ying <ying.huang@intel.com>; 21cnbao@gmail.com; akpm@linux-
> > > foundation.org; willy@infradead.org; Zou, Nanhai
> <nanhai.zou@intel.com>;
> > > Feghali, Wajdi K <wajdi.k.feghali@intel.com>; Gopal, Vinodh
> > > <vinodh.gopal@intel.com>
> > > 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.