On Wed, Dec 8, 2021 at 8:05 AM Matthew Wilcox <willy@xxxxxxxxxxxxx> wrote: > > On Wed, Dec 08, 2021 at 04:51:58PM +0100, Michal Hocko wrote: > > On Wed 08-12-21 15:01:24, Matthew Wilcox wrote: > > > On Tue, Dec 07, 2021 at 03:08:19PM -0800, Suren Baghdasaryan wrote: > > > > > > /** > > > > > > * @close: Called when the VMA is being removed from the MM. > > > > > > * Context: Caller holds mmap_lock. > > > > > > > > BTW, is the caller always required to hold mmap_lock for write or it > > > > *might* hold it? > > > > > > __do_munmap() might hold it for read, thanks to: > > > > > > if (downgrade) > > > mmap_write_downgrade(mm); > > > > > > Should probably say: > > > > > > * Context: User context. May sleep. Caller holds mmap_lock. > > > > > > I don't think we should burden the implementor of the vm_ops with the > > > knowledge that the VM chooses to not hold the mmap_lock under certain > > > circumstances when it doesn't matter whether it's holding the mmap_lock > > > or not. > > > > If we document it like that some code might depend on that lock to be > > held. I think we only want to document that the holder itself is not > > allowed to take mmap sem or a depending lock. > > The only place where we're not currently holding the mmap_lock is at > task exit, where the mmap_lock is effectively held because nobody else > can modify the task's mm. Besides, Suren is changing that in this patch > series anyway, so it will be always true. Ok, I'll make it a separate patch after the patch that changes exit_mmap and this statement will become always true. Sounds reasonable?
