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