Re: [PATCH] nfs: fix redundant readdir request after get eof

[Benjamin Coddington <bcodding@xxxxxxxxxx>]

On 13 Jul 2023, at 10:52, Kinglong Mee wrote:

> When a directory contains 18 files (includes . and ..), nfs client sends
> a redundant readdir request after get eof.

This breaks the optimization in
85aa8ddc3818 NFS: Trigger the "ls -l" readdir heuristic sooner

The way to see that breakage happing is to "ls -l" a directory with more
than 16 dentries, and then when you do a 2nd "ls -l" you'll see that the NFS
client does a GETATTR for every single dentry instead of just the first 16
and then user READDIRPLUS for the rest.

I think what's going wrong with Kinglong's case is that when
array->folio_is_eof, we set desc->eof to the negation of desc->eob.  That
does the wrong thing for directories with 18 dentries.

Here's a way around it, but I hate how ugly it is just for this single case:


diff --git a/fs/nfs/dir.c b/fs/nfs/dir.c
index 8f3112e71a6a..ace454da9d4d 100644
--- a/fs/nfs/dir.c
+++ b/fs/nfs/dir.c
@@ -1107,14 +1107,20 @@ static void nfs_do_filldir(struct nfs_readdir_descriptor *desc,
                        desc->ctx->pos = desc->dir_cookie;
                else
                        desc->ctx->pos++;
+
                if (first_emit && i > NFS_READDIR_CACHE_MISS_THRESHOLD + 1) {
                        desc->eob = true;
-                       break;
+                       /* handle the case where there are NFS_READDIR_CACHE_MISS_THRESHOLD + 2
+                        * entries:  we also need to set desc->eof */
+                       if (array->folio_is_eof && i == array->size - 1)
+                               desc->eof = true;
+                       goto done;
                }
        }
        if (array->folio_is_eof)
                desc->eof = !desc->eob;

+done:
        kunmap_local(array);
        dfprintk(DIRCACHE, "NFS: nfs_do_filldir() filling ended @ cookie %llu\n",
                        (unsigned long long)desc->dir_cookie);

Ben