Re: [PATCH] nfs: open-associated setattr shouldn't invalidate own cache

["J. Bruce Fields" <bfields@fieldses.org>]

I'm feeling dense.

On Wed, Nov 02, 2011 at 11:54:53AM -0400, Jeff Layton wrote:
> Yes, I think it is different. When we truncate the file to 0 then we
> don't care if another write races in.

	Client 1		Client 2
	--------		--------

	setattr size to 0
				write to file
	get change attribute

If we decide not to invalidate the cache here, then we miss Client 2's
write.  The same is true if we set the size to something other than 0.

There's a second possible race when Client 2's write comes before the
setattr, and it's true that that race doesn't matter in the size-0 case
and does in the other.

But if we were doing a write instead of a setattr, we'd ignore that
second race.

And I don't understand why something like ftruncate should be treated
differently from write.  In either case we're modifying a file while
holding a write open, and I'd expect us to assume in both cases that
nobody else is doing the same.

> We're invalidating all of the
> pages we have cached for the file at that point anyway. If we try to
> reread the file afterward we're going to have to fetch the data from
> the server regardless.
> 
> I guess my main point is that we won't have any cached data after a
> truncate to 0, so there's no need to worry about ensuring that the
> cache is eventually invalidated after that. The VFS-level truncate on
> the client is going to take care of that for us anyway. That allows us
> to optimize for this special (but common) case.
> 
> A truncate to a non-zero size is different however because we may still
> have lingering cached pages. We will need to invalidate those if the
> server sends pre-op attrs

There are no pre-op attrs: the Linux v4 client's write compound is
putfh, write, getattr, with no getattr before the write, and the
nfs_post_op_update_inode_force_wcc() call in nfs4_write_done_cb() makes
the client pretends there were pre-op attributes the same as whatever it
currently has cached in the inode--if I understand it right.

--b.

> that indicate that something else happened
> prior to the truncate, or if the server doesn't send pre-op attrs at
> all.