Received-SPF: pass (google.com: domain of linux-kernel+bounces-3138-linux.lists.archive=gmail.com@vger.kernel.org designates 147.75.80.249 as permitted sender) client-ip=147.75.80.249;
Message-ID: <b3736d08-fccc-4499-8801-4e9b8a7394d6@bytedance.com>
Date: Mon, 18 Dec 2023 16:15:39 +0800
Precedence: bulk
MIME-Version: 1.0
User-Agent: Mozilla Thunderbird
Subject: Re: [PATCH 3/5] mm/zswap: refactor out __zswap_load()
Content-Language: en-US
To: Yosry Ahmed <yosryahmed@google.com>
Cc: Andrew Morton <akpm@linux-foundation.org>, Nhat Pham <nphamcs@gmail.com>,
 Chris Li <chriscli@google.com>, Johannes Weiner <hannes@cmpxchg.org>,
 Seth Jennings <sjenning@redhat.com>, Dan Streetman <ddstreet@ieee.org>,
 Vitaly Wool <vitaly.wool@konsulko.com>, linux-kernel@vger.kernel.org,
 linux-mm@kvack.org
References: <20231213-zswap-dstmem-v1-0-896763369d04@bytedance.com>
 <20231213-zswap-dstmem-v1-3-896763369d04@bytedance.com>
 <CAJD7tkbPPy6Xqy7Xtei24B7CzxdaGwYN7tdbLH_UMVmYkYYJ=g@mail.gmail.com>
From: Chengming Zhou <zhouchengming@bytedance.com>
In-Reply-To: <CAJD7tkbPPy6Xqy7Xtei24B7CzxdaGwYN7tdbLH_UMVmYkYYJ=g@mail.gmail.com>
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

On 2023/12/14 08:52, Yosry Ahmed wrote:
> On Tue, Dec 12, 2023 at 8:18 PM Chengming Zhou
> <zhouchengming@bytedance.com> wrote:
>>
>> The zswap_load() and zswap_writeback_entry() have the same part that
>> decompress the data from zswap_entry to page, so refactor out the
>> common part as __zswap_load(entry, page).
>>
>> Signed-off-by: Chengming Zhou <zhouchengming@bytedance.com>
>> Reviewed-by: Nhat Pham <nphamcs@gmail.com>
> 
> On a second look, there a few nits here.
> 
> First I think it makes more sense to move this refactoring ahead of
> reusing destmem. Right now, we add the destmem reuse to zswap_load()
> only, then we do the refactor and zswap_writeback_entry() gets it
> automatically, so there is a slight change coming to
> zswap_writeback_entry() hidden in the refactoring patch.
> 
> Let's refactor out __zswap_load() first, then reuse destmem in it.

I tried but found that putting the __zswap_load() first would introduce
another failure case in zswap_writeback_entry(), since the temporary
memory allocation may fail.

So instead, I also move the dstmem reusing in zswap_writeback_entry() to
the dstmem reusing patch. Then this patch becomes having only refactoring.

Thanks.