[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <YbDXuegc6BtRzs/5@casper.infradead.org>
Date: Wed, 8 Dec 2021 16:05:13 +0000
From: Matthew Wilcox <willy@...radead.org>
To: Michal Hocko <mhocko@...e.com>
Cc: Suren Baghdasaryan <surenb@...gle.com>, akpm@...ux-foundation.org,
rientjes@...gle.com, hannes@...xchg.org, guro@...com,
riel@...riel.com, minchan@...nel.org, kirill@...temov.name,
aarcange@...hat.com, christian@...uner.io, hch@...radead.org,
oleg@...hat.com, david@...hat.com, jannh@...gle.com,
shakeelb@...gle.com, luto@...nel.org, christian.brauner@...ntu.com,
fweimer@...hat.com, jengelh@...i.de, timmurray@...gle.com,
linux-mm@...ck.org, linux-kernel@...r.kernel.org,
kernel-team@...roid.com
Subject: Re: [PATCH v3 1/2] mm: protect free_pgtables with mmap_lock write
lock in exit_mmap
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.
Powered by blists - more mailing lists