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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



> -----Original Message-----
> From: J. Bruce Fields [mailto:bfields@xxxxxxxxxxxx]
> Sent: Tuesday, November 01, 2011 9:23 PM
> To: Jeff Layton
> Cc: Myklebust, Trond; J. Bruce Fields; linux-nfs@xxxxxxxxxxxxxxx
> Subject: Re: [PATCH] nfs: open-associated setattr shouldn't invalidate
own
> cache
> 
> On Tue, Nov 01, 2011 at 08:43:25PM -0400, Jeff Layton wrote:
> > On Tue, 1 Nov 2011 16:07:27 -0700
> > "Myklebust, Trond" <Trond.Myklebust@xxxxxxxxxx> wrote:
> >
> > > > -----Original Message-----
> > > > From: J. Bruce Fields [mailto:bfields@xxxxxxxxxxxx]
> > > > Sent: Tuesday, November 01, 2011 4:27 PM
> > > > To: Myklebust, Trond
> > > > Cc: J. Bruce Fields; Myklebust, Trond; linux-nfs@xxxxxxxxxxxxxxx
> > > > Subject: Re: [PATCH] nfs: open-associated setattr shouldn't
> > > > invalidate
> > > own
> > > > cache
> > > >
> > > > On Mon, Jul 18, 2011 at 08:09:15PM -0400, Myklebust, Trond
wrote:
> > > > > We should already optimize away the unnecessary setting of the
size.
> > > >
> > > > Do you remember what commit fixed that?  (Was it an nfs change
or
> > > > a
> > > vfs
> > > > change?)
> > >
> > > It predates the git repository. See the comment about
> > > "Optimization:" in nfs_setattr().
> > >
> > > > > The problem is that truncate() still requires you to set the
> > > > > ctime,
> > > whereas
> > > > ftruncate() does not iirc.
> > > >
> > > > Staring at the code....  I think you mean the opposite?  I
notice
> > > > do_sys_ftruncate() calling
> > > >
> > > > 	do_truncate(dentry, length, ATTR_MTIME|ATTR_CTIME,
file);
> > > >
> > > > and do_sys_truncate() calling
> > > >
> > > > 	do_truncate(path.dentry, length, 0, NULL);
> > > >
> > > > where the third argument is getting OR'd with ATTR_FILE to pass
> > > > into notify_change().
> > >
> > > Sorry, yes. ftruncate() is the one that unconditionally sets the
> > > mtime/ctime on success according to the POSIX spec.
> > >
> >
> > Even when it's a noop? Blech.
> >
> > > > Also even when a setattr does get through, I don't understand
why
> > > > it
> > > should
> > > > be invalidating our data cache.  Is there some reason it needs
to,
> > > > or
> > > is this just
> > > > a case that hasn't seemed worth fixing?
> > >
> > > Is the problem perhaps that we should be clearing the
> > > NFS_INO_INVALID_DATA flag in nfs_vmtruncate() when the size gets
set
> > > to zero?
> > >
> >
> > That was my thinking too. Whenever we truncate the i_size to 0, we
can
> > safely assume that the pagecache is now valid, and should be able to
> > clear NFS_INO_INVALID_DATA no matter when it was set, right?
> 
> I don't understand why 0 is a special case: why should my setting the
size
> ever mean that I have to go reread data from the server?

Your test case was:

open(O_RDWR|O_TRUNCATE)
write()
close()
open(O_RDONLY)
read()
...

That truncates _all_ data, so there is nothing left in the data cache
when we get to the write().

The question therefore is not "Why does setattr clear my cache?".

The question is "Why isn't that write() cached?".

That's why I'm exploring reasons why the cache might get cleared after
that write(). The other obvious one would be if the server is failing to
return post-op attributes in the WRITE reply.

Trond
--
To unsubscribe from this list: send the line "unsubscribe linux-nfs" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[Index of Archives]     [Linux Filesystem Development]     [Linux USB Development]     [Linux Media Development]     [Video for Linux]     [Linux NILFS]     [Linux Audio Users]     [Yosemite Info]     [Linux SCSI]

  Powered by Linux