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@xxxxxxxxxx> 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 -- To unsubscribe from this list: send the line "unsubscribe linux-embedded" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
