linux-kernel - Re: [PATCH 16/24] mm/swap: reduce scope of get_swap

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]

Date:   Tue, 21 Nov 2023 09:25:20 -0800
From:   Chris Li <chrisl@...nel.org>
To:     Kairui Song <kasong@...cent.com>
Cc:     linux-mm@...ck.org, Andrew Morton <akpm@...ux-foundation.org>,
        "Huang, Ying" <ying.huang@...el.com>,
        David Hildenbrand <david@...hat.com>,
        Hugh Dickins <hughd@...gle.com>,
        Johannes Weiner <hannes@...xchg.org>,
        Matthew Wilcox <willy@...radead.org>,
        Michal Hocko <mhocko@...e.com>, linux-kernel@...r.kernel.org
Subject: Re: [PATCH 16/24] mm/swap: reduce scope of get_swap_device in swapin path

On Sun, Nov 19, 2023 at 11:48 AM Kairui Song <ryncsn@...il.com> wrote:
>
> From: Kairui Song <kasong@...cent.com>
>
> Move get_swap_device into swapin_readahead, simplify the code
> and prepare for follow up commits.
>
> For the later part in do_swap_page, using swp_swap_info directly is fine
> since in that context, the swap device is pinned by swapcache reference.
>
> Signed-off-by: Kairui Song <kasong@...cent.com>
> ---
>  mm/memory.c     | 16 ++++------------
>  mm/swap_state.c |  8 ++++++--
>  mm/swapfile.c   |  4 +++-
>  3 files changed, 13 insertions(+), 15 deletions(-)
>
> diff --git a/mm/memory.c b/mm/memory.c
> index 22af9f3e8c75..e399b37ef395 100644
> --- a/mm/memory.c
> +++ b/mm/memory.c
> @@ -3789,7 +3789,6 @@ vm_fault_t do_swap_page(struct vm_fault *vmf)
>         struct folio *swapcache = NULL, *folio = NULL;
>         enum swap_cache_result cache_result;
>         struct page *page;
> -       struct swap_info_struct *si = NULL;
>         rmap_t rmap_flags = RMAP_NONE;
>         bool exclusive = false;
>         swp_entry_t entry;
> @@ -3845,14 +3844,11 @@ vm_fault_t do_swap_page(struct vm_fault *vmf)
>                 goto out;
>         }
>
> -       /* Prevent swapoff from happening to us. */
> -       si = get_swap_device(entry);
> -       if (unlikely(!si))
> -               goto out;
> -
>         page = swapin_readahead(entry, GFP_HIGHUSER_MOVABLE,
>                                 vmf, &cache_result);
> -       if (page) {
> +       if (PTR_ERR(page) == -EBUSY) {
> +               goto out;
> +       } else if (page) {

As Matthew suggested, using ERR_PTR(-EBUSY).
you also don't need the else here. The goto already terminates the flow.

if (page == ERR_PTR(-EBUSY))
   goto out;

if (page) {

Chris