Re: [PATCH v3 01/12] mm, swap: simplify swap cache allocation helper

[Kairui Song]

On Wed, May 6, 2026 at 9:51 PM Chris Li <chrisl@xxxxxxxxxx> wrote:
>
> On Tue, Apr 21, 2026 at 8:16 AM Kairui Song via B4 Relay
> <devnull+kasong.tencent.com@xxxxxxxxxx> wrote:
> >
> > From: Kairui Song <kasong@xxxxxxxxxxx>
> >
> > Instead of trying to return the existing folio if the entry is already
> > cached, simply return an error code if the allocation fails and drop the
>
> Nitpick: Spell out which function changes the return type here. It is
> __swap_cache_prepare_and_add()

Good idea.

>
> > output argument. And introduce proper wrappers that handle the
>
> Nitpick: Spell out the helper function. It is swap_cache_read_folio().
> > allocation failure in different ways.
>
> >
> > For async swapin and readahead, the caller only wants to ensure that a
> > swap-in read is issued when the allocation succeeded. And for zswap swap
> > out, the caller will abort if the allocation failed because the entry is
> > gone or cached already.
>
> Should you add no functional change expected?

Yes indeed, there is no functional change.

>
> >
> > Signed-off-by: Kairui Song <kasong@xxxxxxxxxxx>
>
> Very nice clean ups. I like it. Here are some nitpicks; feel free to
> ignore them.
>
> Acked-by: Chris Li <chrisl@xxxxxxxxx>

Thanks.

>
> Nitpick: IS_ERR() only checks that the pointer is in the error code
> range. If the pointer is -EEXIST, it will always be in the error code
> range. I think the "IS_ERR(folio)" test can be dropped.

Agreed. Actually, I didn't add IS_ERR in V1 then Sashiko complained
that it should be added. I just checked the API documentation again
and existing patterns, there is indeed no rule to prohibit the direct
check. Let me drop it, I also like it better that way, and maybe just
ignore Sashiko next time.