| Message ID | 20231228061802.25280-1-v-songbaohua@oppo.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | mm: zsmalloc: return -ENOSPC rather than -EINVAL in zs_malloc while size is too large | expand |
On 2023/12/28 14:18, Barry Song wrote: > This is the case the "compressed" data is larger than the original data, > it is better to return -ENOSPC which can help zswap record a poor compr > rather than an invalid request. Then we get more friendly counting for > reject_compress_poor in debugfs. > > bool zswap_store(struct folio *folio) > { > ... > ret = zpool_malloc(zpool, dlen, gfp, &handle); > if (ret == -ENOSPC) { > zswap_reject_compress_poor++; > goto put_dstmem; > } > if (ret) { > zswap_reject_alloc_fail++; > goto put_dstmem; > } > ... > } > > Also, zbud_alloc() and z3fold_alloc() are returning ENOSPC in the same > case, eg > static int z3fold_alloc(struct z3fold_pool *pool, size_t size, gfp_t gfp, > unsigned long *handle) > { > ... > if (!size || (gfp & __GFP_HIGHMEM)) > return -EINVAL; > > if (size > PAGE_SIZE) > return -ENOSPC; > ... > } > > Signed-off-by: Barry Song <v-songbaohua@oppo.com> Looks good to me. Reviewed-by: Chengming Zhou <zhouchengming@bytedance.com> Thanks. > --- > mm/zsmalloc.c | 5 ++++- > 1 file changed, 4 insertions(+), 1 deletion(-) > > diff --git a/mm/zsmalloc.c b/mm/zsmalloc.c > index b1c0dad7f4cf..c937635e0ad1 100644 > --- a/mm/zsmalloc.c > +++ b/mm/zsmalloc.c > @@ -1364,9 +1364,12 @@ unsigned long zs_malloc(struct zs_pool *pool, size_t size, gfp_t gfp) > int newfg; > struct zspage *zspage; > > - if (unlikely(!size || size > ZS_MAX_ALLOC_SIZE)) > + if (unlikely(!size)) > return (unsigned long)ERR_PTR(-EINVAL); > > + if (unlikely(size > ZS_MAX_ALLOC_SIZE)) > + return (unsigned long)ERR_PTR(-ENOSPC); > + > handle = cache_alloc_handle(pool, gfp); > if (!handle) > return (unsigned long)ERR_PTR(-ENOMEM);
On Wed, Dec 27, 2023 at 10:18 PM Barry Song <21cnbao@gmail.com> wrote: > > This is the case the "compressed" data is larger than the original data, > it is better to return -ENOSPC which can help zswap record a poor compr > rather than an invalid request. Then we get more friendly counting for > reject_compress_poor in debugfs. > > bool zswap_store(struct folio *folio) > { > ... > ret = zpool_malloc(zpool, dlen, gfp, &handle); > if (ret == -ENOSPC) { > zswap_reject_compress_poor++; > goto put_dstmem; > } > if (ret) { > zswap_reject_alloc_fail++; > goto put_dstmem; > } > ... > } > > Also, zbud_alloc() and z3fold_alloc() are returning ENOSPC in the same > case, eg > static int z3fold_alloc(struct z3fold_pool *pool, size_t size, gfp_t gfp, > unsigned long *handle) > { > ... > if (!size || (gfp & __GFP_HIGHMEM)) > return -EINVAL; > > if (size > PAGE_SIZE) > return -ENOSPC; > ... > } > > Signed-off-by: Barry Song <v-songbaohua@oppo.com> > --- > mm/zsmalloc.c | 5 ++++- > 1 file changed, 4 insertions(+), 1 deletion(-) > > diff --git a/mm/zsmalloc.c b/mm/zsmalloc.c > index b1c0dad7f4cf..c937635e0ad1 100644 > --- a/mm/zsmalloc.c > +++ b/mm/zsmalloc.c > @@ -1364,9 +1364,12 @@ unsigned long zs_malloc(struct zs_pool *pool, size_t size, gfp_t gfp) > int newfg; > struct zspage *zspage; > > - if (unlikely(!size || size > ZS_MAX_ALLOC_SIZE)) > + if (unlikely(!size)) > return (unsigned long)ERR_PTR(-EINVAL); > > + if (unlikely(size > ZS_MAX_ALLOC_SIZE)) > + return (unsigned long)ERR_PTR(-ENOSPC); > + > handle = cache_alloc_handle(pool, gfp); > if (!handle) > return (unsigned long)ERR_PTR(-ENOMEM); > -- > 2.34.1 > LGTM. Reviewed-by: Nhat Pham <nphamcs@gmail.com>
diff --git a/mm/zsmalloc.c b/mm/zsmalloc.c index b1c0dad7f4cf..c937635e0ad1 100644 --- a/mm/zsmalloc.c +++ b/mm/zsmalloc.c @@ -1364,9 +1364,12 @@ unsigned long zs_malloc(struct zs_pool *pool, size_t size, gfp_t gfp) int newfg; struct zspage *zspage; - if (unlikely(!size || size > ZS_MAX_ALLOC_SIZE)) + if (unlikely(!size)) return (unsigned long)ERR_PTR(-EINVAL); + if (unlikely(size > ZS_MAX_ALLOC_SIZE)) + return (unsigned long)ERR_PTR(-ENOSPC); + handle = cache_alloc_handle(pool, gfp); if (!handle) return (unsigned long)ERR_PTR(-ENOMEM);
This is the case the "compressed" data is larger than the original data, it is better to return -ENOSPC which can help zswap record a poor compr rather than an invalid request. Then we get more friendly counting for reject_compress_poor in debugfs. bool zswap_store(struct folio *folio) { ... ret = zpool_malloc(zpool, dlen, gfp, &handle); if (ret == -ENOSPC) { zswap_reject_compress_poor++; goto put_dstmem; } if (ret) { zswap_reject_alloc_fail++; goto put_dstmem; } ... } Also, zbud_alloc() and z3fold_alloc() are returning ENOSPC in the same case, eg static int z3fold_alloc(struct z3fold_pool *pool, size_t size, gfp_t gfp, unsigned long *handle) { ... if (!size || (gfp & __GFP_HIGHMEM)) return -EINVAL; if (size > PAGE_SIZE) return -ENOSPC; ... } Signed-off-by: Barry Song <v-songbaohua@oppo.com> --- mm/zsmalloc.c | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-)