Re: [PATCH RFC v2 19/27] mm: mprotect: Introduce PAGE_FAULT_ON_ACCESS for mprotect(PROT_MTE)

From: Alexandru Elisei
Date: Thu Nov 30 2023 - 08:32:28 EST


Hi,

On Thu, Nov 30, 2023 at 01:49:34PM +0100, David Hildenbrand wrote:
> > > > +
> > > > +out_retry:
> > > > + put_page(page);
> > > > + if (vmf->flags & FAULT_FLAG_VMA_LOCK)
> > > > + vma_end_read(vma);
> > > > + if (fault_flag_allow_retry_first(vmf->flags)) {
> > > > + err = VM_FAULT_RETRY;
> > > > + } else {
> > > > + /* Replay the fault. */
> > > > + err = 0;
> > >
> > > Hello!
> > >
> > > Unfortunately, if the page continues to be pinned, it seems like fault will continue to occur.
> > > I guess it makes system stability issue. (but I'm not familiar with that, so please let me know if I'm mistaken!)
> > >
> > > How about migrating the page when migration problem repeats.
> >
> > Yes, I had the same though in the previous iteration of the series, the
> > page was migrated out of the VMA if tag storage couldn't be reserved.
> >
> > Only short term pins are allowed on MIGRATE_CMA pages, so I expect that the
> > pin will be released before the fault is replayed. Because of this, and
> > because it makes the code simpler, I chose not to migrate the page if tag
> > storage couldn't be reserved.
>
> There are still some cases that are theoretically problematic: vmsplice()
> can pin pages forever and doesn't use FOLL_LONGTERM yet.
>
> All these things also affect other users that rely on movability (e.g., CMA,
> memory hotunplug).

I wasn't aware of that, thank you for the information. Then to ensure that the
process doesn't hang by replying the loop indefinitely, I'll migrate the page if
tag storage cannot be reserved. Looking over the code again, I think I can reuse
the same function that migrates tag storage pages out of the MTE VMA (added in
patch #21), so no major changes needed.

Thanks,
Alex

>
> --
> Cheers,
>
> David / dhildenb
>
>