Re: Yet another borken page_count() check in invalidate_inode_pages2()....

[Andrew Morton <akpm@osdl.org>]

On Wed, 15 Nov 2006 15:57:45 -0500
Trond Myklebust <Trond.Myklebust@netapp.com> wrote:

> On Wed, 2006-11-15 at 11:24 -0800, Andrew Morton wrote:
> 
> > The protocol is
> > 
> > 	lock_page()
> > 	set_page_writeback()
> > 	->writepage()
> 
> We're not using ->writepage().

I think you know what I mean.

> > and there are various places which assume that nobody will start new
> > writeout of a locked page.  But I forget where they are - things have always
> > been this way.
> 
> Huh? There has never been a requirement to lock the page if all you want
> to do is call set_page_writeback().

The protocol is, and always has been

	lock_page()
	set_page_writeback();
	start-io
	unlock_page();

end_io:
	end_page_writeback()


and there are places in the VM which rely upon some or all of that.  I'd
need to go on a big hunt to remember where they are.  One of them is
invalidate_inode_pages2(), as you've just discovered.

> The only reason why we want to do
> that at all is to allow the VM to track that the page is under I/O. All
> other operations involved in scheduling writes are protected by internal
> NFS locks.

Well the VM uses lock_page() for this synchronisation.  If NFS has gone and
decided not to do that then we'll need to either

a) Make NFS follow the protocol or

b) Put stuff in NFS to allow the VM to work correctly (until we change it) or

c) Put very-clearly-commented NFS exception code into the VM.