Received-SPF: pass (google.com: domain of linux-nfs-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
From:   "Benjamin Coddington" <bcodding@redhat.com>
To:     "Trond Myklebust" <trondmy@hammerspace.com>
Cc:     dwysocha@redhat.com, linux-nfs@vger.kernel.org
Subject: Re: [PATCH v5 00/22] Readdir enhancements
Date:   Thu, 12 Nov 2020 13:49:16 -0500
Message-ID: <AF0AF7F0-EEA1-475B-987F-168724A83E60@redhat.com>
In-Reply-To: <DC49EB88-4B79-40FC-97C3-47D2A588F96C@redhat.com>
References: <20201110213741.860745-1-trondmy@kernel.org>
 <CALF+zOkdXMDZ3TNGSNJQPtxy-ru_4iCYTz3U2uwkPAo3j55FZg@mail.gmail.com>
 <CALF+zO=-Si+CcEJvgzaYAjd2j8APV=4Xwm=FJibhuJRV+zWE5Q@mail.gmail.com>
 <723ef5d47994e34804f5514b06940e96620e2b70.camel@hammerspace.com>
 <6b07ff95824f5b46237fa07f5f72d8261d764007.camel@hammerspace.com>
 <DC49EB88-4B79-40FC-97C3-47D2A588F96C@redhat.com>
MIME-Version: 1.0
Content-Type: text/plain; format=flowed
Precedence: bulk

On 12 Nov 2020, at 13:39, Benjamin Coddington wrote:

> On 12 Nov 2020, at 13:26, Trond Myklebust wrote:
>
>> On Thu, 2020-11-12 at 16:51 +0000, Trond Myklebust wrote:
>>>
>>> I was going to ask you if perhaps reverting Scott's commit
>>> 07b5ce8ef2d8
>>> ("NFS: Make nfs_readdir revalidate less often") might help here?
>>> My thinking is that will trigger more cache invalidations when the
>>> directory is changing underneath us, and will now trigger uncached
>>> readdir in those situations.
>>>
>>>>
>>
>> IOW, the suggestion would be to apply something like the following on
>> top of the existing readdir patchset:
>
> I'm all for this approach, but - I'm rarely seeing the 
> mapping->nrpages == 0
> since the cache is dropped by a process in nfs_readdir() that 
> immediately
> starts filling the cache again.
>
> It would make a lot more sense to me if we could do something like 
> stash
> desc->page_index << PAGE_SHIFT in f_pos after each nfs_readdir, then 
> the
> hueristic could check f_pos >> PAGE_SHIFT > nrpages.
>
> Yes, f_pos is growing many different meanings in NFS directories, 
> maybe we
> can stash it on the directory context.

Ignore the f_pos suggestion -- iterate_dir sets it for us.. would have 
to use
a private value.

Ben