| Message ID | 20240924011709.7037-5-kanchana.p.sridhar@intel.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | mm: ZSWAP swap-out of mTHP folios | expand |
On Mon, Sep 23, 2024 at 6:17 PM Kanchana P Sridhar <kanchana.p.sridhar@intel.com> wrote: > > Added a new procedure zswap_delete_stored_offsets() that can be > called to delete stored offsets in a folio in case zswap_store() > fails or zswap is disabled. > > Refactored the code in zswap_store() that handles these cases, > to call zswap_delete_stored_offsets(). > > Signed-off-by: Kanchana P Sridhar <kanchana.p.sridhar@intel.com> > --- > mm/zswap.c | 33 ++++++++++++++++++++++++++++++--- > 1 file changed, 30 insertions(+), 3 deletions(-) > > diff --git a/mm/zswap.c b/mm/zswap.c > index fd35a81b6e36..9bea948d653e 100644 > --- a/mm/zswap.c > +++ b/mm/zswap.c > @@ -1435,8 +1435,37 @@ static bool zswap_store_entry(struct xarray *tree, > return true; > } > > +/* > + * If the zswap store fails or zswap is disabled, we must invalidate the > + * possibly stale entries which were previously stored at the offsets > + * corresponding to each page of the folio. Otherwise, writeback could > + * overwrite the new data in the swapfile. > + * > + * This is called after the store of an offset in a large folio has failed. "store of a subpage" rather than "stored of an offset"? > + * All zswap entries in the folio must be deleted. This helps make sure > + * that a swapped-out mTHP is either entirely stored in zswap, or entirely > + * not stored in zswap. > + * > + * This is also called if zswap_store() is invoked, but zswap is not enabled. > + * All offsets for the folio are deleted from zswap in this case. > + */ > +static void zswap_delete_stored_offsets(struct xarray *tree, > + pgoff_t offset, > + long nr_pages) > +{ > + struct zswap_entry *entry; > + long i; > + > + for (i = 0; i < nr_pages; ++i) { > + entry = xa_erase(tree, offset + i); > + if (entry) > + zswap_entry_free(entry); > + } > +} > +
On Mon, Sep 23, 2024 at 6:17 PM Kanchana P Sridhar <kanchana.p.sridhar@intel.com> wrote: > > Added a new procedure zswap_delete_stored_offsets() that can be > called to delete stored offsets in a folio in case zswap_store() > fails or zswap is disabled. I don't see the value in this helper. It will get called in one place AFAICT, and it is a bit inconsistent that we have to explicitly loop in zswap_store() to store pages, but the loop to delete pages upon failure is hidden in the helper. I am not against adding a trivial zswap_tree_delete() helper (or similar) that calls xa_erase() and zswap_entry_free() to match zswap_tree_store() if you prefer that. > > Refactored the code in zswap_store() that handles these cases, > to call zswap_delete_stored_offsets(). > > Signed-off-by: Kanchana P Sridhar <kanchana.p.sridhar@intel.com> > --- > mm/zswap.c | 33 ++++++++++++++++++++++++++++++--- > 1 file changed, 30 insertions(+), 3 deletions(-) > > diff --git a/mm/zswap.c b/mm/zswap.c > index fd35a81b6e36..9bea948d653e 100644 > --- a/mm/zswap.c > +++ b/mm/zswap.c > @@ -1435,8 +1435,37 @@ static bool zswap_store_entry(struct xarray *tree, > return true; > } > > +/* > + * If the zswap store fails or zswap is disabled, we must invalidate the > + * possibly stale entries which were previously stored at the offsets > + * corresponding to each page of the folio. Otherwise, writeback could > + * overwrite the new data in the swapfile. > + * > + * This is called after the store of an offset in a large folio has failed. > + * All zswap entries in the folio must be deleted. This helps make sure > + * that a swapped-out mTHP is either entirely stored in zswap, or entirely > + * not stored in zswap. > + * > + * This is also called if zswap_store() is invoked, but zswap is not enabled. > + * All offsets for the folio are deleted from zswap in this case. > + */ > +static void zswap_delete_stored_offsets(struct xarray *tree, > + pgoff_t offset, > + long nr_pages) > +{ > + struct zswap_entry *entry; > + long i; > + > + for (i = 0; i < nr_pages; ++i) { > + entry = xa_erase(tree, offset + i); > + if (entry) > + zswap_entry_free(entry); > + } > +} > + > bool zswap_store(struct folio *folio) > { > + long nr_pages = folio_nr_pages(folio); > swp_entry_t swp = folio->swap; > pgoff_t offset = swp_offset(swp); > struct xarray *tree = swap_zswap_tree(swp); > @@ -1541,9 +1570,7 @@ bool zswap_store(struct folio *folio) > * possibly stale entry which was previously stored at this offset. > * Otherwise, writeback could overwrite the new data in the swapfile. > */ > - entry = xa_erase(tree, offset); > - if (entry) > - zswap_entry_free(entry); > + zswap_delete_stored_offsets(tree, offset, nr_pages); > return false; > } > > -- > 2.27.0 >
diff --git a/mm/zswap.c b/mm/zswap.c index fd35a81b6e36..9bea948d653e 100644 --- a/mm/zswap.c +++ b/mm/zswap.c @@ -1435,8 +1435,37 @@ static bool zswap_store_entry(struct xarray *tree, return true; } +/* + * If the zswap store fails or zswap is disabled, we must invalidate the + * possibly stale entries which were previously stored at the offsets + * corresponding to each page of the folio. Otherwise, writeback could + * overwrite the new data in the swapfile. + * + * This is called after the store of an offset in a large folio has failed. + * All zswap entries in the folio must be deleted. This helps make sure + * that a swapped-out mTHP is either entirely stored in zswap, or entirely + * not stored in zswap. + * + * This is also called if zswap_store() is invoked, but zswap is not enabled. + * All offsets for the folio are deleted from zswap in this case. + */ +static void zswap_delete_stored_offsets(struct xarray *tree, + pgoff_t offset, + long nr_pages) +{ + struct zswap_entry *entry; + long i; + + for (i = 0; i < nr_pages; ++i) { + entry = xa_erase(tree, offset + i); + if (entry) + zswap_entry_free(entry); + } +} + bool zswap_store(struct folio *folio) { + long nr_pages = folio_nr_pages(folio); swp_entry_t swp = folio->swap; pgoff_t offset = swp_offset(swp); struct xarray *tree = swap_zswap_tree(swp); @@ -1541,9 +1570,7 @@ bool zswap_store(struct folio *folio) * possibly stale entry which was previously stored at this offset. * Otherwise, writeback could overwrite the new data in the swapfile. */ - entry = xa_erase(tree, offset); - if (entry) - zswap_entry_free(entry); + zswap_delete_stored_offsets(tree, offset, nr_pages); return false; }
Added a new procedure zswap_delete_stored_offsets() that can be called to delete stored offsets in a folio in case zswap_store() fails or zswap is disabled. Refactored the code in zswap_store() that handles these cases, to call zswap_delete_stored_offsets(). Signed-off-by: Kanchana P Sridhar <kanchana.p.sridhar@intel.com> --- mm/zswap.c | 33 ++++++++++++++++++++++++++++++--- 1 file changed, 30 insertions(+), 3 deletions(-)