Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Date:   Sun, 23 Apr 2023 19:17:12 -0700 (PDT)
From:   Hugh Dickins <hughd@google.com>
To:     David Stevens <stevensd@chromium.org>
cc:     Hugh Dickins <hughd@google.com>,
        Andrew Morton <akpm@linux-foundation.org>,
        Ivan Orlov <ivan.orlov0322@gmail.com>,
        Jiaqi Yan <jiaqiyan@google.com>, linux-kernel@vger.kernel.org,
        linux-mm@kvack.org
Subject: Re: [PATCH next] mm/khugepaged: fix conflicting mods to
 collapse_file()
In-Reply-To: <CAD=HUj5NSXX8a5shf-GVYqkMFC-o26vuyo26hMPpPCnuSD683A@mail.gmail.com>
Message-ID: <cf24ea0-92da-cd46-a7cd-87f5449016e8@google.com>
References: <b6c881-c352-bb91-85a8-febeb09dfd71@google.com> <CAD=HUj5NSXX8a5shf-GVYqkMFC-o26vuyo26hMPpPCnuSD683A@mail.gmail.com>
MIME-Version: 1.0
Content-Type: multipart/mixed; boundary="-1463760895-436720276-1682302640=:4090"
Precedence: bulk

  This message is in MIME format.  The first part should be readable text,
  while the remaining parts are likely unreadable without MIME-aware tools.

---1463760895-436720276-1682302640=:4090
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: QUOTED-PRINTABLE

On Mon, 24 Apr 2023, David Stevens wrote:
> On Sun, Apr 23, 2023 at 1:47=E2=80=AFPM Hugh Dickins <hughd@google.com> w=
rote:
> >
> > Inserting Ivan Orlov's syzbot fix commit 2ce0bdfebc74
> > ("mm: khugepaged: fix kernel BUG in hpage_collapse_scan_file()")
> > ahead of Jiaqi Yan's and David Stevens's commits
> > 12904d953364 ("mm/khugepaged: recover from poisoned file-backed memory"=
)
> > cae106dd67b9 ("mm/khugepaged: refactor collapse_file control flow")
> > ac492b9c70ca ("mm/khugepaged: skip shmem with userfaultfd")
> > (all of which restructure collapse_file()) did not work out well.
> >
> > xfstests generic/086 on huge tmpfs (with accelerated khugepaged) freeze=
s
> > (if not on the first attempt, then the 2nd or 3rd) in find_lock_entries=
()
> > while doing drop_caches: the file's xarray seems to have been corrupted=
,
> > with find_get_entry() returning nonsense which makes no progress.
> >
> > Bisection led to ac492b9c70ca; and diff against earlier working linux-n=
ext
> > suggested that it's probably down to an errant xas_store(), which does =
not
> > belong with the later changes (and nor does the positioning of warnings=
).
> > The later changes look as if they fix the syzbot issue independently.
> >
> > Remove most of what's left of 2ce0bdfebc74: just leave one WARN_ON_ONCE
> > (xas_error) after the final xas_store() of the multi-index entry.
> >
> > Signed-off-by: Hugh Dickins <hughd@google.com>
> > ---
> >
> >  mm/khugepaged.c | 23 +----------------------
> >  1 file changed, 1 insertion(+), 22 deletions(-)
> >
> > --- a/mm/khugepaged.c
> > +++ b/mm/khugepaged.c
> > @@ -1941,16 +1941,6 @@ static int collapse_file(struct mm_struct *mm, u=
nsigned long addr,
> >                                         result =3D SCAN_FAIL;
> >                                         goto xa_locked;
> >                                 }
> > -                               xas_store(&xas, hpage);
> > -                               if (xas_error(&xas)) {
> > -                                       /* revert shmem_charge performe=
d
> > -                                        * in the previous condition
> > -                                        */
> > -                                       mapping->nrpages--;
> > -                                       shmem_uncharge(mapping->host, 1=
);
> > -                                       result =3D SCAN_STORE_FAILED;
>=20
> With this being removed, SCAN_STORE_FAILED should also be removed from
> the scan_result enum and trace event definitions.

Only if we also remove your use of SCAN_STORE_FAILED in ac492b9c70ca:
what would you want that to say instead?

I don't care myself for any of those "SCAN" result codes, nor whether they
are few or many: I'd rather have __LINE__ numbers for my own debugging.

But if people want to remove SCAN_STORE_FAILED now, sure, send a patch;
my intent was to unbreak the breakage.

Hugh
---1463760895-436720276-1682302640=:4090--