On Thu, Jan 25, 2024 at 1:19 AM Mike Rapoport <rppt@xxxxxxxxxx> wrote: > > Hi, > > On Thu, Jan 11, 2024 at 03:32:20PM -0800, Lokesh Gidra wrote: > > Hi, > > > > We have been seeing mmap_lock contention issues while using > > userfaultfd for GC in Android. But now that per-vma locks are being > > used in the kernel, we were hoping to use it in userfaultfd code to > > pin the VMA in COPY/MOVE/ZEROPAGE etc. operations. But while going > > through the code, I noticed that mmap_changing is implicitly protected > > by mmap_lock: > > > > 1) All increments to it (except for userfault_remove) are done with > > mmap_lock in write-mode > > 2) All reads (in copy/move/zeropage etc) are done with mmap_lock in read-mode > > > > I wanted to understand if that's just out of convenience, and > > therefore would it be ok to introduce a read-write semaphore in > > userfaultfd_ctx to achieve the same synchronization: > > > > 1) All increments are done with this semaphore in write-mode > > 2) All operations (copy/move/zeropage etc) are done within the > > critical section of this semaphore in read-mode and checking that > > mmap_changing is 0. > > mmap_changing was added to the existing critical sections that were already > protected with mmap_lock, so I didn't see a reason for additional lock to > protect mmap_changing. > > With per-vma locks, your proposal makes perfect sense to me. Thanks so much for confirming. I'll send the patches for review very soon. > > > If this is wrong, then kindly explain why mmap_changing needs to be > > protected with mmap_lock. > > > > > > Thanks, > > Lokesh > > > > -- > Sincerely yours, > Mike.
