lkml.org 
[lkml]   [2017]   [Oct]   [3]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [PATCH v4 4/5] cramfs: add mmap support
On Tue, 3 Oct 2017, Christoph Hellwig wrote:

> On Mon, Oct 02, 2017 at 07:33:29PM -0400, Nicolas Pitre wrote:
> > On Tue, 3 Oct 2017, Richard Weinberger wrote:
> >
> > > On Mon, Oct 2, 2017 at 12:29 AM, Nicolas Pitre <nicolas.pitre@linaro.org> wrote:
> > > > On Sun, 1 Oct 2017, Christoph Hellwig wrote:
> > > >
> > > >> up_read(&mm->mmap_sem) in the fault path is a still a complete
> > > >> no-go,
> > > >>
> > > >> NAK
> > > >
> > > > Care to elaborate?
> > > >
> > > > What about mm/filemap.c:__lock_page_or_retry() then?
> > >
> > > As soon you up_read() in the page fault path other tasks will race
> > > with you before
> > > you're able to grab the write lock.
> >
> > But I _know_ that.
> >
> > Could you highlight an area in my code where this is not accounted for?
>
> Existing users of lock_page_or_retry return VM_FAULT_RETRY right after
> up()ing mmap_sem, and they must already have a reference to the page
> which is the only thing touched until then.
>
> Your patch instead goes for an exclusive mmap_sem if it can, and
> even if there is nothing that breaks with that scheme right now
> there s nothing documenting that this actually safe, and we are
> way down in the complex page fault path.

It is pretty obvious looking at the existing code that if you want to
safely manipulate a vma you need the write lock. There are many things
in the kernel tree that are not explicitly documented. Did that stop
people from adding new code?

I agree that the fault path is quite complex. I've studied it carefully
before coming up with this scheme. This is not something that came about
just because the sunshine felt good when I woke up one day.

So if you agree that I've done a reasonable job creating a scheme that
currently doesn't break then IMHO this should be good enough,
*especially* for such an isolated and specialized use case with zero
impact on anyone else. And if things break in the future than I will be
the one working out the pieces not you, and _that_ can be written down
somewhere if necessary so nobody has an obligation to bend backward for
not breaking it.

Unless you have a better scheme altogether to suggest of course, given
the existing constraints.


Nicolas

\
 
 \ /
  Last update: 2017-10-03 17:32    [W:0.129 / U:2.960 seconds]
©2003-2017 Jasper Spaans. hosted at Digital OceanAdvertise on this site