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:   Gonzalo Siero <gsierohu@redhat.com>
To:     trond.myklebust@hammerspace.com
Cc:     anna.schumaker@netapp.com, linux-nfs@vger.kernel.org,
        Gonzalo Siero <gsierohu@redhat.com>
Subject: [PATCH] NFS: limit block size reported for directories
Date:   Fri, 14 Jan 2022 11:54:39 +0100
Message-Id: <20220114105439.19347-1-gsierohu@redhat.com>
MIME-Version: 1.0
Content-Transfer-Encoding: 8bit
Precedence: bulk

With the inclusion of this commit:

1a34c8c9a49e NFS: Support larger readdir buffers

We have observed that if we run 'ls -l' to get data from server, wait
few secs, and run it again on a directory of 80,000 files, second run
takes 53 secs with this commit vs 11 secs without it.

dtsize is set to rsize, but now is no longer limited to 8 pages.
Since we use it to report block size for directories, buffer being
allocated and passed to getdents could be up to 1Mb.

When running 'ls -l', if we have no data in memory for our directory,
we get the attributes in READDIR operations. If we run it again, and
the attributes have expired, the call to nfs_getattr() looking at first
cached entry will timeout and we will end up calling
nfs_force_use_readdirplus() to invalidate parent directory pagecache
pages. Subsequent nfs_getattr() calls on each entry returned in the
*first*  getdents64 call, ends up getting attributes from server. In
cases where we use large readdir buf, the number of entries returned
will be a lot more than before, and consecuently the number of GETATTR
calls.

Fix this by limiting to 32kb the reported block size for directories.
Note that even if we do more getdents syscalls, it still use large
readdir buffer, taking advantage of it.

Signed-off-by: Gonzalo Siero <gsierohu@redhat.com>
---
 fs/nfs/inode.c    | 6 +++++-
 fs/nfs/internal.h | 2 ++
 2 files changed, 7 insertions(+), 1 deletion(-)

diff --git a/fs/nfs/inode.c b/fs/nfs/inode.c
index fda530d5e764..f21530a5092d 100644
--- a/fs/nfs/inode.c
+++ b/fs/nfs/inode.c
@@ -917,8 +917,12 @@ int nfs_getattr(struct user_namespace *mnt_userns, const struct path *path,
 
 	generic_fillattr(&init_user_ns, inode, stat);
 	stat->ino = nfs_compat_user_ino64(NFS_FILEID(inode));
-	if (S_ISDIR(inode->i_mode))
+	if (S_ISDIR(inode->i_mode)) {
 		stat->blksize = NFS_SERVER(inode)->dtsize;
+		/* Limit userland buf size for getdents */
+		if (stat->blksize > NFS_MAX_READDIR_BLOCK)
+			stat->blksize = NFS_MAX_READDIR_BLOCK;
+	}
 out:
 	trace_nfs_getattr_exit(inode, err);
 	return err;
diff --git a/fs/nfs/internal.h b/fs/nfs/internal.h
index 12f6acb483bb..a75fe07bec29 100644
--- a/fs/nfs/internal.h
+++ b/fs/nfs/internal.h
@@ -56,6 +56,8 @@ static inline bool nfs_lookup_is_soft_revalidate(const struct dentry *dentry)
 #define NFS_UNSPEC_RETRANS	(UINT_MAX)
 #define NFS_UNSPEC_TIMEO	(UINT_MAX)
 
+#define NFS_MAX_READDIR_BLOCK	32768
+
 struct nfs_client_initdata {
 	unsigned long init_flags;
 	const char *hostname;			/* Hostname of the server */
-- 
2.21.3