Re: [PATCH 2/2] mm: prevent gup_fast from racing with COW during fork

[Jason Gunthorpe <jgg@nvidia.com>]

On Fri, Oct 23, 2020 at 10:31:51PM -0700, John Hubbard wrote:
> On 10/23/20 10:19 PM, John Hubbard wrote:
> > On 10/23/20 5:19 PM, Jason Gunthorpe wrote:
> ...
> > > diff --git a/mm/memory.c b/mm/memory.c
> > > index c48f8df6e50268..e2f959cce8563d 100644
> > > --- a/mm/memory.c
> > > +++ b/mm/memory.c
> > > @@ -1171,6 +1171,17 @@ copy_page_range(struct vm_area_struct
> > > *dst_vma, struct vm_area_struct *src_vma)
> > >           mmu_notifier_range_init(&range, MMU_NOTIFY_PROTECTION_PAGE,
> > >                       0, src_vma, src_mm, addr, end);
> > >           mmu_notifier_invalidate_range_start(&range);
> > > +        /*
> > > +         * This is like a seqcount where the mmap_lock provides
> > > +         * serialization for the write side. However, unlike seqcount
> > > +         * the read side falls back to obtaining the mmap_lock rather
> > > +         * than spinning. For this reason none of the preempt related
> > > +         * machinery in seqcount is desired here.
> 
> ooops...actually, that's a counter-argument to using the raw seqlock API. So
> maybe that's a dead end, after all.

Right, it isn't a "seqcount" because the read side doesn't spin.

If this uses the raw seqcount API then every touch uses the raw API. I
checked and there is no place else in the kernel using the raw
side..

> If so, it would still be good to wrap the "acquire" and "release"
> parts of this into functions, IMHO. So we'd end up with,
> effectively, a lock API anyway.

What it really needs is a smp_load_mb() to match the existing smp_store_mb(),
then this implementation wouldn't look so odd.

It is actually really straightforwad since the write side is under the
exclusive mmap_lock.

Jason