> On Dec 2, 2016, at 08:56, Benjamin Coddington <bcodding@xxxxxxxxxx> wrote:
>
> On 1 Dec 2016, at 15:47, Trond Myklebust wrote:
>
>> On Wed, 2016-11-30 at 14:09 -0500, Benjamin Coddington wrote:
>>> .. this one breaks things again:
>>>
>>> On 19 Nov 2016, at 11:54, Trond Myklebust wrote:
>>>
>>>> static
>>>> void nfs_advise_use_readdirplus(struct inode *dir)
>>>> {
>>>> - set_bit(NFS_INO_ADVISE_RDPLUS, &NFS_I(dir)->flags);
>>>> + struct nfs_inode *nfsi = NFS_I(dir);
>>>> +
>>>> + if (nfs_server_capable(dir, NFS_CAP_READDIRPLUS) &&
>>>> + !list_empty(&nfsi->open_files)) {
>>>> + set_bit(NFS_INO_ADVISE_RDPLUS, &nfsi->flags);
>>>> + invalidate_mapping_pages(dir->i_mapping, 0, -1);
>>>> + }
>>>> }
>>>
>>> So every time advise_use_readdirplus it drops the mapping.. but what
>>> about
>>> subsequent calls into nfs_readdir() to get the next batch of
>>> entries?
>>> For
>>> the ls -l case, we want to keep setting NFS_INO_ADVISE_RDPLUS, but we
>>> shouldn't start over filling the mapping from the beginning again.
>>
>> How do I ensure that the readdir isn't being served from cache, if I
>> don't invalidate the mapping?
>
> The way it is now. Isn't advise_use_readdirplus() just a marker to say
> "continue to use readdirplus" after nfs_force_use_readdirplus() has
> invalidated the mapping?
So, yes. I was considering renaming the functions in the v2 patch series, but I couldn’t really find a good name.
In essence:
- nfs_force_use_readdirplus() is hinting to readdir that we had a cache miss, and so we should clear the readir cache and use readdirplus in order to populate the cache.
- nfs_advise_use_readdirplus() is hinting that we had a cache hit, and so we should continue to use readdirplus
>
>> The intention of the patch is to ensure that we only call this on a dcache
>> or inode attribute cache miss.
>
> I think it also needs to be called when we detect if a child of the
> directory is looked up/revalidated in order to set NFS_INO_ADVISE_RDPLUS
> again. Otherwise we lose the optimization in commit 311324ad1713.
>
> Maybe I am just really confused..
No, I think that is correct. It just needs to be done more consistently. Again, see the v2 patch series
Cheers
Trond��.n��������+%������w��{.n�����{��w��� jg��� �����ݢj����G���� ���j:+v���w�m������w�������h�����٥
