public inbox for linux-nfs@vger.kernel.org
 help / color / mirror / Atom feed
From: Trond Myklebust <trond.myklebust@primarydata.com>
To: Brown Neil <neilb@suse.de>
Cc: Jeff Layton <jlayton@poochiereds.net>,
	"Mkrtchyan, Tigran" <tigran.mkrtchyan@desy.de>,
	Jim Rees <rees@umich.edu>, linux-nfs <linux-nfs@vger.kernel.org>
Subject: Re: readdir vs. getattr
Date: Fri, 7 Feb 2014 14:47:23 -0500	[thread overview]
Message-ID: <6F6E3499-4C95-44B4-BBD0-CFFBFC4F48DA@primarydata.com> (raw)
In-Reply-To: <20140207153028.7df0d209@notabene.brown>


On Feb 6, 2014, at 23:30, NeilBrown <neilb@suse.de> wrote:

> On Thu, 06 Feb 2014 17:12:59 -0500 Trond Myklebust
> <trond.myklebust@primarydata.com> wrote:
> 
>> On Thu, 2014-02-06 at 13:51 +1100, NeilBrown wrote:
>>> On Thu, 6 Feb 2014 13:45:39 +1100 NeilBrown <neilb@suse.de> wrote:
>>> 
>>> 
>>>> The change to nfs_update_inode fixes an issue that had me stumped for a
>>>> while.  It was still sending lots of GETATTR requests even after it had
>>>> switched to READDIRPLUS instead of using cached info.  So that might be a
>>>> genuine bug that should be fixed independently of this patch.
>>> 
>>> I managed to post the wrong version of the patch, which didn't have this
>>> change.  Sorry.
>>> 
>>> Here is the real one.
>>> 
>>> NeilBrown
>> 
>> Hi Neil,
>> 
>> Is there any reason why this patch couldn't just be replaced with the
>> following?
> 
> Well.... the following doesn't make any change in my test case. :-(
> 
> In my test case there is a large directory on the server which doesn't change.
> The client sends lots of requests to see if it has changed, but it hasn't.
> (Tested with the labeled-NFS-regression fix applied)
> 
> Your patch only changes behaviour if nfs_force_lookup_revalidate is called,
> and that is only called when NFS_ATTR_FATTR_CHANGE is set, or when an NFSv4
> client modifies the directory.  Neither of those are happening.

Right, but I really do not know how to fix that case.

I read your patch as saying: "If we're revalidating a dentry and we find that the directory has not changed but that the dentry’s inode needs revalidation, then invalidate that directory's attribute, acl and readdir cached data. Then drop the dentry”. I have a hard time seeing how that can be beneficial.

If we were to modify it, to only invalidate the readdir cached data, then that might improve matters, but it is still hard for me to see how a single inode needing revalidation can justify a full re-read of the parent directory in the general case. You may end up rereading thousands of readdir entries from the server just because someone did a path lookup.

_________________________________
Trond Myklebust
Linux NFS client maintainer, PrimaryData
trond.myklebust@primarydata.com


  reply	other threads:[~2014-02-07 19:47 UTC|newest]

Thread overview: 24+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2013-04-04 14:47 readdir vs. getattr Tigran Mkrtchyan
2013-04-04 15:15 ` Jim Rees
2013-04-04 15:32   ` Chuck Lever
2013-04-04 16:01     ` Jim Rees
2013-04-04 16:15       ` Myklebust, Trond
2013-04-04 16:35         ` Jim Rees
2013-04-04 18:28           ` Tigran Mkrtchyan
2013-04-04 15:38   ` Tigran Mkrtchyan
2013-04-04 15:48     ` Myklebust, Trond
2013-04-04 15:52       ` Tigran Mkrtchyan
2014-01-29  7:10       ` NeilBrown
2014-01-29  7:25       ` NeilBrown
2014-01-29  9:21         ` Mkrtchyan, Tigran
2014-01-29 12:18           ` Jeff Layton
2014-02-06  2:45             ` NeilBrown
2014-02-06  2:51               ` NeilBrown
2014-02-06 18:08                 ` Jeff Layton
2014-02-06 19:53                 ` [PATCH] NFS: Do not set NFS_INO_INVALID_LABEL unless server supports labeled NFS Trond Myklebust
2014-02-07  4:21                   ` NeilBrown
2014-02-06 22:12                 ` readdir vs. getattr Trond Myklebust
2014-02-07  4:30                   ` NeilBrown
2014-02-07 19:47                     ` Trond Myklebust [this message]
2014-02-07 22:08                       ` Trond Myklebust
2014-02-10  0:16                         ` NeilBrown

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=6F6E3499-4C95-44B4-BBD0-CFFBFC4F48DA@primarydata.com \
    --to=trond.myklebust@primarydata.com \
    --cc=jlayton@poochiereds.net \
    --cc=linux-nfs@vger.kernel.org \
    --cc=neilb@suse.de \
    --cc=rees@umich.edu \
    --cc=tigran.mkrtchyan@desy.de \
    /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