On Wed, Dec 6, 2023 at 1:46 AM Chengming Zhou
<[email protected]> wrote:
>
> Maybe I missed something, but the dstmem size of 2 * PAGE_SIZE is
> very confusing, since we only need at most one page when compress,
> and the "dlen" is also PAGE_SIZE in acomp_request_set_params().
>
> So change it to one page, and fix the comments.
>
> Signed-off-by: Chengming Zhou <[email protected]>
> ---
> mm/zswap.c | 5 ++---
> 1 file changed, 2 insertions(+), 3 deletions(-)
>
> diff --git a/mm/zswap.c b/mm/zswap.c
> index d93a7b58b5af..999671dcb469 100644
> --- a/mm/zswap.c
> +++ b/mm/zswap.c
> @@ -699,7 +699,7 @@ static int zswap_dstmem_prepare(unsigned int cpu)
> struct mutex *mutex;
> u8 *dst;
>
> - dst = kmalloc_node(PAGE_SIZE * 2, GFP_KERNEL, cpu_to_node(cpu));
> + dst = kmalloc_node(PAGE_SIZE, GFP_KERNEL, cpu_to_node(cpu));
> if (!dst)
> return -ENOMEM;
>
> @@ -1649,8 +1649,7 @@ bool zswap_store(struct folio *folio)
> sg_init_table(&input, 1);
> sg_set_page(&input, page, PAGE_SIZE, 0);
>
> - /* zswap_dstmem is of size (PAGE_SIZE * 2). Reflect same in sg_list */
> - sg_init_one(&output, dst, PAGE_SIZE * 2);
> + sg_init_one(&output, dst, PAGE_SIZE);
Hmm. This is very weird. It looks very intentional though, so perhaps
we should consult the maintainer or the original author of this logic
to double check this?
My best guess is for cases where the compression algorithm fails - i.e
the output (header + payload) is somehow bigger than the original
data. But not sure if this happens at all, and if the size > PAGE_SIZE
we don't wanna store the output in zswap anyway.
> acomp_request_set_params(acomp_ctx->req, &input, &output, PAGE_SIZE, dlen);
> /*
> * it maybe looks a little bit silly that we send an asynchronous request,
>
> --
> b4 0.10.1
On 2023/12/7 01:12, Nhat Pham wrote:
> On Wed, Dec 6, 2023 at 1:46 AM Chengming Zhou
> <[email protected]> wrote:
>>
>> Maybe I missed something, but the dstmem size of 2 * PAGE_SIZE is
>> very confusing, since we only need at most one page when compress,
>> and the "dlen" is also PAGE_SIZE in acomp_request_set_params().
>>
>> So change it to one page, and fix the comments.
>>
>> Signed-off-by: Chengming Zhou <[email protected]>
>> ---
>> mm/zswap.c | 5 ++---
>> 1 file changed, 2 insertions(+), 3 deletions(-)
>>
>> diff --git a/mm/zswap.c b/mm/zswap.c
>> index d93a7b58b5af..999671dcb469 100644
>> --- a/mm/zswap.c
>> +++ b/mm/zswap.c
>> @@ -699,7 +699,7 @@ static int zswap_dstmem_prepare(unsigned int cpu)
>> struct mutex *mutex;
>> u8 *dst;
>>
>> - dst = kmalloc_node(PAGE_SIZE * 2, GFP_KERNEL, cpu_to_node(cpu));
>> + dst = kmalloc_node(PAGE_SIZE, GFP_KERNEL, cpu_to_node(cpu));
>> if (!dst)
>> return -ENOMEM;
>>
>> @@ -1649,8 +1649,7 @@ bool zswap_store(struct folio *folio)
>> sg_init_table(&input, 1);
>> sg_set_page(&input, page, PAGE_SIZE, 0);
>>
>> - /* zswap_dstmem is of size (PAGE_SIZE * 2). Reflect same in sg_list */
>> - sg_init_one(&output, dst, PAGE_SIZE * 2);
>> + sg_init_one(&output, dst, PAGE_SIZE);
>
> Hmm. This is very weird. It looks very intentional though, so perhaps
> we should consult the maintainer or the original author of this logic
> to double check this?
Yes, it's also weird to me. But it seems to be 2 pages when the zswap code
merged in mm 10 years ago. Hope the maintainer or author could shed some
light on it. :)
> My best guess is for cases where the compression algorithm fails - i.e
> the output (header + payload) is somehow bigger than the original
> data. But not sure if this happens at all, and if the size > PAGE_SIZE
> we don't wanna store the output in zswap anyway.
>
Agree, so I think one page is enough.
>> acomp_request_set_params(acomp_ctx->req, &input, &output, PAGE_SIZE, dlen);
And here we just use PAGE_SIZE in the parameter actually.
Thanks!
>> /*
>> * it maybe looks a little bit silly that we send an asynchronous request,
>>
>> --
>> b4 0.10.1