Re: [PATCH v2 3/5] mm, swap: Remove redundant comment for read_swap_cache_async

[Chris Li <chrisl@kernel.org>]

On Thu, Oct 30, 2025 at 11:54 PM Youngjun Park <youngjun.park@lge.com> wrote:
>
> The function now manages get/put_swap_device() internally, making the
> comment explaining this behavior to callers unnecessary.

This commit message is not precise. The previous comment explaining
this behavior is not just unnecessary. The comment is plain wrong. It
does not match the code. __read_swap_cache_async() does not internally
call get/put_swap_device() any more. So this comment regarding
get/put_swap_device should move to __read_swap_cache_async(), the
caller of __read_swap_cache_async() needs to manage the
get/put_swap_device() or by other means not to race with swap off.

>
> Signed-off-by: Youngjun Park <youngjun.park@lge.com>
>
> diff --git a/mm/swap_state.c b/mm/swap_state.c
> index b13e9c4baa90..d20d238109f9 100644
> --- a/mm/swap_state.c
> +++ b/mm/swap_state.c
> @@ -509,10 +509,6 @@ struct folio *__read_swap_cache_async(swp_entry_t entry, gfp_t gfp_mask,
>   * and reading the disk if it is not already cached.
>   * A failure return means that either the page allocation failed or that
>   * the swap entry is no longer in use.
> - *
> - * get/put_swap_device() aren't needed to call this function, because
> - * __read_swap_cache_async() call them and swap_read_folio() holds the
> - * swap cache folio lock.


I don't think we should simply delete this comment. We can delete from
this function but please add a comment to __read_swap_cache_async()
regarding the caller's responsibility.

Chris