Re: [PATCH] cifs: invalidate any mapped pages before turning cache off

[David Howells <dhowells@redhat.com>]

Suresh Jayaraman <sjayaraman@suse.de> wrote:

> When disabling inode cookie, we were returning the cookie and setting
> cifsi->fscache to NULL but failed to invalidate any previously mapped
> pages. This resulted in "Bad page state" errors and manifested in other
> kind of errors when running fsstress. Fix it by invalidating mapped
> pages when we disable the inode cookie.
> 
> Also make cifs_fscache_disable_inode_cookie() return error if
> invalidate_inode_pages2() fails

I think I see what's going on: you have to 'hand back' all the page-in-use
marks before calling fscache_relinquish_cookie().  One way to do that is to
call invalidate_inode_pages2() or similar; the other is to iterate over all
the pages attached to your inode and call cifs_fscache_invalidate_page() on
every page.  Note that this may sleep to achieve its aim of getting rid of the
mark.

Let me have a go at whipping up a patch to do this.  It looks like it belongs
in fscache really as a helper function.

FS-Cache assumes that *you* know where all your pages are, and so it would be
redundant for it to keep its own list and thus keep extra memory around that
isn't strictly necessary.

Note also that:

	void cifs_fscache_set_inode_cookie(struct inode *inode, struct file *filp)
	{
		if ((filp->f_flags & O_ACCMODE) != O_RDONLY)
			cifs_fscache_disable_inode_cookie(inode);
		else
			cifs_fscache_enable_inode_cookie(inode);
	}

doesn't actually work right (and should be fixed in NFS too).  Imagine you
have a file open for read-only and you open it again for read-write or vice
versa.  This really needs a counter of writers.

David