| Message ID | 20240325235018.2028408-4-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 limit and acceptance threshold checking outside of > zswap_store(). This code will be moved around in a following patch, so > it would be cleaner to move a function call around. > > Signed-off-by: Yosry Ahmed <yosryahmed@google.com> > --- > mm/zswap.c | 32 ++++++++++++++++---------------- > 1 file changed, 16 insertions(+), 16 deletions(-) > > diff --git a/mm/zswap.c b/mm/zswap.c > index ff1975afb7e3d..6b890c8590ef7 100644 > --- a/mm/zswap.c > +++ b/mm/zswap.c > @@ -1415,6 +1415,21 @@ static void zswap_fill_page(void *ptr, unsigned long value) > memset_l(page, value, PAGE_SIZE / sizeof(unsigned long)); > } > > +static bool zswap_check_limit(void) > +{ > + unsigned long cur_pages = zswap_total_pages(); > + unsigned long thr = zswap_accept_thr_pages(); > + unsigned long max_pages = zswap_max_pages(); > + > + if (cur_pages >= max_pages) { > + zswap_pool_limit_hit++; > + zswap_pool_reached_full = true; > + } else if (zswap_pool_reached_full && cur_pages <= thr) { > + zswap_pool_reached_full = false; > + } > + return !zswap_pool_reached_full; nit: Then we use "!zswap_check_limit()" below, double negation looks complex, should we change to zswap_should_reject() or something? > +} > + > bool zswap_store(struct folio *folio) > { > swp_entry_t swp = folio->swap; > @@ -1422,7 +1437,6 @@ bool zswap_store(struct folio *folio) > struct xarray *tree = swap_zswap_tree(swp); > 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)); > @@ -1446,22 +1460,8 @@ bool zswap_store(struct folio *folio) > mem_cgroup_put(memcg); > } > > - /* Check global limits */ > - cur_pages = zswap_total_pages(); > - max_pages = zswap_max_pages(); > - > - if (cur_pages >= max_pages) { > - zswap_pool_limit_hit++; > - zswap_pool_reached_full = true; > + if (!zswap_check_limit()) > goto reject; > - } > - > - if (zswap_pool_reached_full) { > - if (cur_pages > zswap_accept_thr_pages()) > - goto reject; > - else > - zswap_pool_reached_full = false; > - } > > /* allocate entry */ > entry = zswap_entry_cache_alloc(GFP_KERNEL, folio_nid(folio));
On Tue, Mar 26, 2024 at 7:42 PM Chengming Zhou <chengming.zhou@linux.dev> wrote: > > On 2024/3/26 07:50, Yosry Ahmed wrote: > > Refactor limit and acceptance threshold checking outside of > > zswap_store(). This code will be moved around in a following patch, so > > it would be cleaner to move a function call around. > > > > Signed-off-by: Yosry Ahmed <yosryahmed@google.com> > > --- > > mm/zswap.c | 32 ++++++++++++++++---------------- > > 1 file changed, 16 insertions(+), 16 deletions(-) > > > > diff --git a/mm/zswap.c b/mm/zswap.c > > index ff1975afb7e3d..6b890c8590ef7 100644 > > --- a/mm/zswap.c > > +++ b/mm/zswap.c > > @@ -1415,6 +1415,21 @@ static void zswap_fill_page(void *ptr, unsigned long value) > > memset_l(page, value, PAGE_SIZE / sizeof(unsigned long)); > > } > > > > +static bool zswap_check_limit(void) > > +{ > > + unsigned long cur_pages = zswap_total_pages(); > > + unsigned long thr = zswap_accept_thr_pages(); > > + unsigned long max_pages = zswap_max_pages(); > > + > > + if (cur_pages >= max_pages) { > > + zswap_pool_limit_hit++; > > + zswap_pool_reached_full = true; > > + } else if (zswap_pool_reached_full && cur_pages <= thr) { > > + zswap_pool_reached_full = false; > > + } > > + return !zswap_pool_reached_full; > > nit: Then we use "!zswap_check_limit()" below, double negation looks complex, > should we change to zswap_should_reject() or something? Good point. Will rename it in the next version. Thanks!
diff --git a/mm/zswap.c b/mm/zswap.c index ff1975afb7e3d..6b890c8590ef7 100644 --- a/mm/zswap.c +++ b/mm/zswap.c @@ -1415,6 +1415,21 @@ static void zswap_fill_page(void *ptr, unsigned long value) memset_l(page, value, PAGE_SIZE / sizeof(unsigned long)); } +static bool zswap_check_limit(void) +{ + unsigned long cur_pages = zswap_total_pages(); + unsigned long thr = zswap_accept_thr_pages(); + unsigned long max_pages = zswap_max_pages(); + + if (cur_pages >= max_pages) { + zswap_pool_limit_hit++; + zswap_pool_reached_full = true; + } else if (zswap_pool_reached_full && cur_pages <= thr) { + zswap_pool_reached_full = false; + } + return !zswap_pool_reached_full; +} + bool zswap_store(struct folio *folio) { swp_entry_t swp = folio->swap; @@ -1422,7 +1437,6 @@ bool zswap_store(struct folio *folio) struct xarray *tree = swap_zswap_tree(swp); 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)); @@ -1446,22 +1460,8 @@ bool zswap_store(struct folio *folio) mem_cgroup_put(memcg); } - /* Check global limits */ - cur_pages = zswap_total_pages(); - max_pages = zswap_max_pages(); - - if (cur_pages >= max_pages) { - zswap_pool_limit_hit++; - zswap_pool_reached_full = true; + if (!zswap_check_limit()) goto reject; - } - - if (zswap_pool_reached_full) { - if (cur_pages > zswap_accept_thr_pages()) - goto reject; - else - zswap_pool_reached_full = false; - } /* allocate entry */ entry = zswap_entry_cache_alloc(GFP_KERNEL, folio_nid(folio));
Refactor limit and acceptance threshold checking outside of zswap_store(). This code will be moved around in a following patch, so it would be cleaner to move a function call around. Signed-off-by: Yosry Ahmed <yosryahmed@google.com> --- mm/zswap.c | 32 ++++++++++++++++---------------- 1 file changed, 16 insertions(+), 16 deletions(-)