Re: [PATCH 2/3] mm, swap: distinguish a malformed swap entry from a dying device

[David Hildenbrand (Arm)]

On 8/10/26 18:26, Breno Leitao wrote:
> get_swap_device() returns NULL both for an entry that can never name a

"an entry that can never name a slot on any device" is confusing. Can you elaborate?

> slot on any device and for a device that swapoff is taking away. The
> first never becomes valid, the second does, and callers cannot tell
> them apart.
> 
> Return ERR_PTR(-EINVAL) for the two malformed cases and keep NULL for
> swapoff. Callers bail out on failure either way, so switch them to
> IS_ERR_OR_NULL() and clear si where the cleanup path would otherwise
> put an ERR_PTR. No functional change.
> 

[...]

>  	ci = __swap_offset_to_cluster(si, offset);
> @@ -1859,7 +1859,9 @@ void folio_put_swap(struct folio *folio, struct page *page)
>   * Check whether swap entry is valid in the swap device.  If so,
>   * return pointer to swap_info_struct, and keep the swap entry valid
>   * via preventing the swap device from being swapoff, until
> - * put_swap_device() is called.  Otherwise return NULL.
> + * put_swap_device() is called.  Return NULL for an empty entry or a
> + * device that is going away, and ERR_PTR(-EINVAL) if the entry itself
> + * is malformed and can never name a slot on any device.
>   *
>   * Notice that swapoff or swapoff+swapon can still happen before the
>   * percpu_ref_tryget_live() in get_swap_device() or after the
> @@ -1900,12 +1902,13 @@ struct swap_info_struct *get_swap_device(swp_entry_t entry)
>  	return si;
>  bad_nofile:
>  	pr_err_ratelimited("%s: %s%08lx\n", __func__, Bad_file, entry.val);
> +	return ERR_PTR(-EINVAL);
>  out:
>  	return NULL;
>  put_out:
>  	pr_err_ratelimited("%s: %s%08lx\n", __func__, Bad_offset, entry.val);
>  	percpu_ref_put(&si->users);
> -	return NULL;
> +	return ERR_PTR(-EINVAL);
>  }
Is -EINVAL really the logical right return value? Running into bad swap entries
sounds more serious to me than "-EINVAL" :)

-- 
Cheers,

David