linux-kernel - Re: [PATCH v2 6/6] mm/zswap: directly use percpu mutex and buffer in load/store

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <27c8af2f-c931-4d36-9db9-112de79ec3cd@bytedance.com>
Date: Mon, 18 Dec 2023 19:54:54 +0800
From: Chengming Zhou <zhouchengming@...edance.com>
To: Yosry Ahmed <yosryahmed@...gle.com>
Cc: Seth Jennings <sjenning@...hat.com>, Dan Streetman <ddstreet@...e.org>,
 Chris Li <chriscli@...gle.com>, Nhat Pham <nphamcs@...il.com>,
 Vitaly Wool <vitaly.wool@...sulko.com>,
 Andrew Morton <akpm@...ux-foundation.org>,
 Johannes Weiner <hannes@...xchg.org>, linux-kernel@...r.kernel.org,
 linux-mm@...ck.org, Chris Li <chrisl@...nel.org>
Subject: Re: [PATCH v2 6/6] mm/zswap: directly use percpu mutex and buffer in
 load/store

On 2023/12/18 17:37, Yosry Ahmed wrote:
> On Mon, Dec 18, 2023 at 12:22 AM Chengming Zhou
> <zhouchengming@...edance.com> wrote:
>>
>> Since the introduce of reusing the dstmem in the load path, it seems
>> confusing that we are now using acomp_ctx->dstmem and acomp_ctx->mutex
>> now for purposes other than what the naming suggests.
>>
>> Yosry suggested removing these two fields from acomp_ctx, and directly
>> using zswap_dstmem and zswap_mutex in both the load and store paths,
>> rename them, and add proper comments above their definitions that they
>> are for generic percpu buffering on the load and store paths.
>>
>> So this patch remove dstmem and mutex from acomp_ctx, and rename the
>> zswap_dstmem to zswap_buffer, using the percpu mutex and buffer on
>> the load and store paths. And refactor out __zswap_store() to only
>> include the compress & store, since I found zswap_store() is too long.
> 
> I am not sure refactoring out __zswap_store() is useful to be honest,
> but I am not objecting to it, it mirrors __zswap_load() in a sense.
> However, if you want to do so, please do it in a separate patch from
> renaming the percpu buffers and mutex. This will make reviewing easier
> (and make my Suggested-by correctly scoped).
> 

After thinking twice, I dropped the __zswap_store() part. Right, it's
not much useful.

Thanks.