Re: [patch 3/3] mm: fault handler to replace nopage and populate

[Benjamin Herrenschmidt <benh@kernel.crashing.org>]

On Mon, 2006-10-09 at 15:52 +0200, Nick Piggin wrote:
> On Mon, Oct 09, 2006 at 03:38:10PM +0200, Thomas Hellstrom wrote:
> > Nick Piggin wrote:
> > >On Mon, Oct 09, 2006 at 10:07:50PM +1000, Benjamin Herrenschmidt wrote:
> > >
> > >Ok I guess that would work. I was kind of thinking that one needs to
> > >hold the mmap_sem for writing when changing the flags, but so long
> > >as everyone *else* does, then I guess you can get exclusion from just
> > >the read lock. And your per-object mutex would prevent concurrent
> > >nopages from modifying it.
> > 
> > Wouldn't that confuse concurrent readers?
> 
> I think it should be safe so long as the entire mapping has been
> unmapped. After that, there is no read path that should care about
> that flag bit. So long as it is well commented (and maybe done via
> a helper in mm/memory.c), I can't yet see a problem with it.

Should be fine then. Migration does

	- take object mutex
	- unmap_mapping_range() -> remove all PTEs for all mappings to
          that object
	- do whatever is needed for actual migration (copy data etc...)
	- release object mutex

And nopage() does

	- take object mutex
	- check object flags consistency, possibly update VMA
          (also possibly updaet VMA pgprot too while at it for cacheable
           vs. non cacheable, though it's not strictly necessary if we
           use the helper)
	- if object is in ram
		- get struct page
		- drop mutex
		- return struct page
	- else
		- get pfn
		- use helper to install PTE
		- drop mutex
		- return NOPAGE_REFAULT

We don't strictly have to return struct page when the object is in ram
but I feel like it's better for accounting.

Now there is still the question of where that RAM comes from, how it
gets accounted, and wether there is any way we can make it swappable
(which complicates things but would be nice as objects can be fairly big
and we may end up using a significant amount of physical memory with the
graphic objects).

> > Could it be an option to make it safe for the fault handler to 
> > temporarily drop the mmap_sem read lock given that some conditions TBD 
> > are met?
> > In that case it can retake the mmap_sem write lock, do the VMA flags 
> > modifications, downgrade and do the pte modifications using a helper, or 
> > even use remap_pfn_range() during the time the write lock is held?
> 
> When you drop and retake the mmap_sem, you need to start again from
> find_vma. At which point you technically probably want to start again
> from the architecture specfic fault code. It sounds difficult but I
> won't say it can't be done.

I can be done with returning NOPAGE_REFAULT but as you said, I don't
think it's necessary.

Cheers,
Ben.


-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/