Re: [PATCH v7 6/8] mm: zswap: Support mTHP swapout in zswap_store().

From: Yosry Ahmed
Date: Thu Sep 26 2024 - 00:53:12 EST


[..]
>
> One thing I realized while reworking the patches for the batched checks is:
> within zswap_store_page(), we set the entry->objcg and entry->pool before
> adding it to the xarray. Given this, wouldn't it be safer to get the objcg
> and pool reference per sub-page, locally in zswap_store_page(), rather than
> obtaining batched references at the end if the store is successful? If we want
> zswap_store_page() to be self-contained and correct as far as the entry
> being created and added to the xarray, it seems like the right thing to do?
> I am a bit apprehensive about the entry being added to the xarray without
> a reference obtained on the objcg and pool, because any page-faults/writeback
> that occur on sub-pages added to the xarray before the entire folio has been
> stored, would run into issues.

We definitely should not obtain references to the pool and objcg after
initializing the entries with them. We can obtain all references in
zswap_store() before zswap_store_page(). IOW, the batching in this
case should be done before the per-page operations, not after.

>
> Just wanted to run this by you. The rest of the batched charging, atomic
> and stat updates should be Ok.
>
> Thanks,
> Kanchana
>
> >
> > Thanks,
> > Kanchana