| Message ID | 20240325235018.2028408-9-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: > When storing zero-filled pages, there is no point of checking the global > zswap limit. These pages do not consume any memory that contributes > toward the limit. Move the limit checking after zero-filled pages are > handled. > > This avoids having zero-filled pages skip zswap and go to disk swap if > the limit is hit. It also avoids queueing the shrink worker, which may > end up being unnecessary if the zswap usage goes down on its own before > another store is attempted. > > Ignoring the memcg limits as well for zero-filled pages is more > controversial. Those limits are more a matter of per-workload policy. > Some workloads disable zswap completely by setting memory.zswap.max = 0, > and those workloads could start observing some zswap activity even after > disabling zswap. Although harmless, this could cause confusion to > userspace. Remain conservative and keep respecting those limits. > > Signed-off-by: Yosry Ahmed <yosryahmed@google.com> Yeah, it looks reasonable to keep the memcg limits check. Reviewed-by: Chengming Zhou <chengming.zhou@linux.dev> Thanks. > --- > mm/zswap.c | 6 +++--- > 1 file changed, 3 insertions(+), 3 deletions(-) > > diff --git a/mm/zswap.c b/mm/zswap.c > index efc323bab2f22..9357328d940af 100644 > --- a/mm/zswap.c > +++ b/mm/zswap.c > @@ -1460,9 +1460,6 @@ bool zswap_store(struct folio *folio) > mem_cgroup_put(memcg); > } > > - if (!zswap_check_limit()) > - goto reject; > - > if (zswap_is_folio_zero_filled(folio)) { > if (zswap_store_zero_filled(tree, offset, objcg)) > goto reject; > @@ -1472,6 +1469,9 @@ bool zswap_store(struct folio *folio) > if (!zswap_non_zero_filled_pages_enabled) > goto reject; > > + if (!zswap_check_limit()) > + goto reject; > + > entry = zswap_entry_cache_alloc(GFP_KERNEL, folio_nid(folio)); > if (!entry) { > zswap_reject_kmemcache_fail++;
diff --git a/mm/zswap.c b/mm/zswap.c index efc323bab2f22..9357328d940af 100644 --- a/mm/zswap.c +++ b/mm/zswap.c @@ -1460,9 +1460,6 @@ bool zswap_store(struct folio *folio) mem_cgroup_put(memcg); } - if (!zswap_check_limit()) - goto reject; - if (zswap_is_folio_zero_filled(folio)) { if (zswap_store_zero_filled(tree, offset, objcg)) goto reject; @@ -1472,6 +1469,9 @@ bool zswap_store(struct folio *folio) if (!zswap_non_zero_filled_pages_enabled) goto reject; + if (!zswap_check_limit()) + goto reject; + entry = zswap_entry_cache_alloc(GFP_KERNEL, folio_nid(folio)); if (!entry) { zswap_reject_kmemcache_fail++;
When storing zero-filled pages, there is no point of checking the global zswap limit. These pages do not consume any memory that contributes toward the limit. Move the limit checking after zero-filled pages are handled. This avoids having zero-filled pages skip zswap and go to disk swap if the limit is hit. It also avoids queueing the shrink worker, which may end up being unnecessary if the zswap usage goes down on its own before another store is attempted. Ignoring the memcg limits as well for zero-filled pages is more controversial. Those limits are more a matter of per-workload policy. Some workloads disable zswap completely by setting memory.zswap.max = 0, and those workloads could start observing some zswap activity even after disabling zswap. Although harmless, this could cause confusion to userspace. Remain conservative and keep respecting those limits. Signed-off-by: Yosry Ahmed <yosryahmed@google.com> --- mm/zswap.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-)