| Message ID | 20240325235018.2028408-3-yosryahmed@google.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | zswap: store zero-filled pages more efficiently | expand |
On 2024/3/26 07:50, Yosry Ahmed wrote: > Refactor the code that attempts storing to the xarray, handling erros, > and freeing stale entries into a helper. This will be reused in a > following patch to free other types of tree elements as well. > > Signed-off-by: Yosry Ahmed <yosryahmed@google.com> > --- > mm/zswap.c | 42 ++++++++++++++++++++++++++---------------- > 1 file changed, 26 insertions(+), 16 deletions(-) > > diff --git a/mm/zswap.c b/mm/zswap.c > index 1cf3ab4b22e64..ff1975afb7e3d 100644 > --- a/mm/zswap.c > +++ b/mm/zswap.c > @@ -827,6 +827,30 @@ static void zswap_entry_free(struct zswap_entry *entry) > atomic_dec(&zswap_stored_pages); > } > > +/********************************* > +* zswap tree functions > +**********************************/ > +static int zswap_tree_store(struct xarray *tree, pgoff_t offset, void *new) > +{ > + void *old; > + int err; > + > + old = xa_store(tree, offset, new, GFP_KERNEL); > + err = xa_is_err(old); Seems to use xa_err() to return errno, xa_is_err() just return a bool. > + if (err) { > + WARN_ONCE(err != -ENOMEM, "unexpected xarray error: %d\n", err); > + zswap_reject_alloc_fail++; > + } else if (old) { > + /* > + * We may have had an existing entry that became stale when > + * the folio was redirtied and now the new version is being > + * swapped out. Get rid of the old. > + */ > + zswap_entry_free(old); > + } > + return err; > +} > + > /********************************* > * compressed storage functions > **********************************/ > @@ -1396,10 +1420,10 @@ bool zswap_store(struct folio *folio) > swp_entry_t swp = folio->swap; > pgoff_t offset = swp_offset(swp); > struct xarray *tree = swap_zswap_tree(swp); > - struct zswap_entry *entry, *old; > struct obj_cgroup *objcg = NULL; > struct mem_cgroup *memcg = NULL; > unsigned long max_pages, cur_pages; > + struct zswap_entry *entry; > > VM_WARN_ON_ONCE(!folio_test_locked(folio)); > VM_WARN_ON_ONCE(!folio_test_swapcache(folio)); > @@ -1485,22 +1509,8 @@ bool zswap_store(struct folio *folio) > entry->swpentry = swp; > entry->objcg = objcg; > > - old = xa_store(tree, offset, entry, GFP_KERNEL); > - if (xa_is_err(old)) { > - int err = xa_err(old); > - > - WARN_ONCE(err != -ENOMEM, "unexpected xarray error: %d\n", err); > - zswap_reject_alloc_fail++; > + if (zswap_tree_store(tree, offset, entry)) > goto store_failed; > - } > - > - /* > - * We may have had an existing entry that became stale when > - * the folio was redirtied and now the new version is being > - * swapped out. Get rid of the old. > - */ > - if (old) > - zswap_entry_free(old); > > if (objcg) { > obj_cgroup_charge_zswap(objcg, entry->length);
On Tue, Mar 26, 2024 at 7:25 PM Chengming Zhou <chengming.zhou@linux.dev> wrote: > > On 2024/3/26 07:50, Yosry Ahmed wrote: > > Refactor the code that attempts storing to the xarray, handling erros, > > and freeing stale entries into a helper. This will be reused in a > > following patch to free other types of tree elements as well. > > > > Signed-off-by: Yosry Ahmed <yosryahmed@google.com> > > --- > > mm/zswap.c | 42 ++++++++++++++++++++++++++---------------- > > 1 file changed, 26 insertions(+), 16 deletions(-) > > > > diff --git a/mm/zswap.c b/mm/zswap.c > > index 1cf3ab4b22e64..ff1975afb7e3d 100644 > > --- a/mm/zswap.c > > +++ b/mm/zswap.c > > @@ -827,6 +827,30 @@ static void zswap_entry_free(struct zswap_entry *entry) > > atomic_dec(&zswap_stored_pages); > > } > > > > +/********************************* > > +* zswap tree functions > > +**********************************/ > > +static int zswap_tree_store(struct xarray *tree, pgoff_t offset, void *new) > > +{ > > + void *old; > > + int err; > > + > > + old = xa_store(tree, offset, new, GFP_KERNEL); > > + err = xa_is_err(old); > > Seems to use xa_err() to return errno, xa_is_err() just return a bool. Good catch. It happens to work out because returning 1 would have the same effect as returning the errno. Will fix it in the next version. Thanks!
diff --git a/mm/zswap.c b/mm/zswap.c index 1cf3ab4b22e64..ff1975afb7e3d 100644 --- a/mm/zswap.c +++ b/mm/zswap.c @@ -827,6 +827,30 @@ static void zswap_entry_free(struct zswap_entry *entry) atomic_dec(&zswap_stored_pages); } +/********************************* +* zswap tree functions +**********************************/ +static int zswap_tree_store(struct xarray *tree, pgoff_t offset, void *new) +{ + void *old; + int err; + + old = xa_store(tree, offset, new, GFP_KERNEL); + err = xa_is_err(old); + if (err) { + WARN_ONCE(err != -ENOMEM, "unexpected xarray error: %d\n", err); + zswap_reject_alloc_fail++; + } else if (old) { + /* + * We may have had an existing entry that became stale when + * the folio was redirtied and now the new version is being + * swapped out. Get rid of the old. + */ + zswap_entry_free(old); + } + return err; +} + /********************************* * compressed storage functions **********************************/ @@ -1396,10 +1420,10 @@ bool zswap_store(struct folio *folio) swp_entry_t swp = folio->swap; pgoff_t offset = swp_offset(swp); struct xarray *tree = swap_zswap_tree(swp); - struct zswap_entry *entry, *old; struct obj_cgroup *objcg = NULL; struct mem_cgroup *memcg = NULL; unsigned long max_pages, cur_pages; + struct zswap_entry *entry; VM_WARN_ON_ONCE(!folio_test_locked(folio)); VM_WARN_ON_ONCE(!folio_test_swapcache(folio)); @@ -1485,22 +1509,8 @@ bool zswap_store(struct folio *folio) entry->swpentry = swp; entry->objcg = objcg; - old = xa_store(tree, offset, entry, GFP_KERNEL); - if (xa_is_err(old)) { - int err = xa_err(old); - - WARN_ONCE(err != -ENOMEM, "unexpected xarray error: %d\n", err); - zswap_reject_alloc_fail++; + if (zswap_tree_store(tree, offset, entry)) goto store_failed; - } - - /* - * We may have had an existing entry that became stale when - * the folio was redirtied and now the new version is being - * swapped out. Get rid of the old. - */ - if (old) - zswap_entry_free(old); if (objcg) { obj_cgroup_charge_zswap(objcg, entry->length);
Refactor the code that attempts storing to the xarray, handling erros, and freeing stale entries into a helper. This will be reused in a following patch to free other types of tree elements as well. Signed-off-by: Yosry Ahmed <yosryahmed@google.com> --- mm/zswap.c | 42 ++++++++++++++++++++++++++---------------- 1 file changed, 26 insertions(+), 16 deletions(-)