Re: smp race fix between invalidate_inode_pages* and do_no_page

[Andrew Morton <akpm@osdl.org>]

Andrea Arcangeli <andrea@suse.de> wrote:
>
>  On Wed, Jan 11, 2006 at 12:51:34AM -0800, Andrew Morton wrote:
>  > Andrea Arcangeli <andrea@suse.de> wrote:
>  > >
>  > >  On Wed, Jan 11, 2006 at 03:08:31PM +1100, Nick Piggin wrote:
>  > >  > I'd be inclined to think a lock_page is not a big SMP scalability
>  > >  > problem because the struct page's cacheline(s) will be written to
>  > >  > several times in the process of refcounting anyway. Such a workload
>  > >  > would also be running into tree_lock as well.
>  > > 
>  > >  I seem to recall you wanted to make the tree_lock a readonly lock for
>  > >  readers for the exact same scalability reason? do_no_page is quite a
>  > >  fast path for the tree lock too. But I totally agree the unavoidable is
>  > >  the atomic_inc though, good point, so it worth more to remove the
>  > >  tree_lock than to remove the page lock, the tree_lock can be avoided the
>  > >  atomic_inc on page->_count not.
>  > > 
>  > >  The other bonus that makes this attractive is that then we can drop the
>  > >  *whole* vm_truncate_count mess... vm_truncate_count and
>  > >  inode->trunate_count exists for the only single reason that do_no_page
>  > >  must not map into the pte a page that is under truncation.
>  > 
>  > I think you'll find this hard - filemap_nopage() is the first to find the
>  > page but we need lock coverage up in do_no_page().  So the ->nopage
>  > protocol will need to be changed to "must return with the page locked".  Or
>  > we add a new ->nopage_locked and call that if the vm_ops implements it.
> 
>  Can't we avoid to change the protocol and use lock_page in do_no_page
>  instead?

Confused.  do_no_page() doesn't have a page to lock until it has called
->nopage.