From: Jeff Layton <jlayton@redhat.com>
To: "Myklebust, Trond" <Trond.Myklebust@netapp.com>
Cc: "J. Bruce Fields" <bfields@fieldses.org>,
"J. Bruce Fields" <bfields@redhat.com>,
<linux-nfs@vger.kernel.org>
Subject: Re: [PATCH] nfs: open-associated setattr shouldn't invalidate own cache
Date: Tue, 1 Nov 2011 20:43:25 -0400 [thread overview]
Message-ID: <20111101204325.522c35b3@corrin.poochiereds.net> (raw)
In-Reply-To: <2E1EB2CF9ED1CB4AA966F0EB76EAB4430BDE7BC5@SACMVEXC2-PRD.hq.netapp.com>
On Tue, 1 Nov 2011 16:07:27 -0700
"Myklebust, Trond" <Trond.Myklebust@netapp.com> wrote:
> > -----Original Message-----
> > From: J. Bruce Fields [mailto:bfields@fieldses.org]
> > Sent: Tuesday, November 01, 2011 4:27 PM
> > To: Myklebust, Trond
> > Cc: J. Bruce Fields; Myklebust, Trond; linux-nfs@vger.kernel.org
> > 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?
> The other micro-optimisation that we might want to consider is to not
> set NFS_INO_INVALID_DATA in nfs_update_inode() if inode->i_data.nrpages
> == 0.
>
This I'm a little less clear on...
If we find that another host has truncated the i_size to 0, don't we
still need to call invalidate_inode_pages2() somehow?
--
Jeff Layton <jlayton@redhat.com>
next prev parent reply other threads:[~2011-11-02 0:43 UTC|newest]
Thread overview: 14+ messages / expand[flat|nested] mbox.gz Atom feed top
2011-07-19 0:09 [PATCH] nfs: open-associated setattr shouldn't invalidate own cache Myklebust, Trond
2011-11-01 20:27 ` J. Bruce Fields
2011-11-01 23:07 ` Myklebust, Trond
2011-11-02 0:43 ` Jeff Layton [this message]
2011-11-02 1:23 ` J. Bruce Fields
2011-11-02 1:36 ` Myklebust, Trond
2011-11-02 11:07 ` Jeff Layton
2011-11-02 14:46 ` J. Bruce Fields
2011-11-02 15:54 ` Jeff Layton
2011-11-03 20:44 ` J. Bruce Fields
2011-11-03 21:03 ` Trond Myklebust
2011-11-03 21:20 ` J. Bruce Fields
2011-11-03 21:39 ` Trond Myklebust
-- strict thread matches above, loose matches on Subject: below --
2011-07-18 22:23 J. Bruce Fields
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20111101204325.522c35b3@corrin.poochiereds.net \
--to=jlayton@redhat.com \
--cc=Trond.Myklebust@netapp.com \
--cc=bfields@fieldses.org \
--cc=bfields@redhat.com \
--cc=linux-nfs@vger.kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).