On Fri, 5 Apr 2024 09:09:30 +0200
David Hildenbrand <david@xxxxxxxxxx> wrote:
> On 05.04.24 05:29, Matthew Wilcox wrote:
> > On Thu, Apr 04, 2024 at 06:36:39PM +0200, David Hildenbrand wrote:
> >> + /* We might get PTE-mapped large folios; split them first. */
> >> + if (folio_test_large(folio)) {
> >> + rc = -E2BIG;
> >
> > We agree to this point. I just turned this into -EINVAL.
> >
> >>
> >> + if (rc == -E2BIG) {
> >> + /*
> >> + * Splitting might fail with -EBUSY due to unexpected folio
> >> + * references, just like make_folio_secure(). So handle it
> >> + * ahead of time without the PTL being held.
> >> + */
> >> + folio_lock(folio);
> >> + rc = split_folio(folio);
> >> + folio_unlock(folio);
> >> + folio_put(folio);
> >> + }
> >
> > Ummm ... if split_folio() succeeds, aren't we going to return 0 from
> > this function, which will be interpreted as make_folio_secure() having
> > succeeded?
>
> I assume the code would have to handle that, because it must deal with
> possible races that would try to convert the folio page.
>
> But the right thing to do is
>
> if (!rc)
> goto again;
>
> after the put.
yes please
