| Message ID | a87df5d5ada87834a1cb9f1ab12ed5113d80eda7.1481117708.git.bcodding@redhat.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Wed, Dec 07, 2016 at 08:37:02AM -0500, Benjamin Coddington wrote: > Optimizations to NFS to choose between READDIR and READDIRPLUS don't expect > concurrent users of nfs_readdir(), and can cause the pagecache to > repeatedly be invalidated unnecessarily for this case. Fix this by > serializing nfs_readdir() on the directory's rwsem. Just implement .iterate instead of .iterate_shared and you'll get the serialization for free. While doing that please add a comment on why it's serialized. -- To unsubscribe from this list: send the line "unsubscribe linux-nfs" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Hi Benjamin,
[auto build test ERROR on nfs/linux-next]
[also build test ERROR on v4.9-rc8 next-20161207]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]
url: https://github.com/0day-ci/linux/commits/Benjamin-Coddington/NFS-Serialize-nfs_readdir/20161208-001039
base: git://git.linux-nfs.org/projects/trondmy/linux-nfs.git linux-next
config: xtensa-common_defconfig (attached as .config)
compiler: xtensa-linux-gcc (GCC) 4.9.0
reproduce:
wget https://git.kernel.org/cgit/linux/kernel/git/wfg/lkp-tests.git/plain/sbin/make.cross -O ~/bin/make.cross
chmod +x ~/bin/make.cross
# save the attached .config to linux build tree
make.cross ARCH=xtensa
All errors (new ones prefixed by >>):
fs/nfs/dir.c: In function 'nfs_readdir':
>> fs/nfs/dir.c:921:18: error: 'struct nfs_inode' has no member named 'rwsem'
down_write(&nfsi->rwsem);
^
fs/nfs/dir.c:963:16: error: 'struct nfs_inode' has no member named 'rwsem'
up_write(&nfsi->rwsem);
^
vim +921 fs/nfs/dir.c
915 * *desc->dir_cookie has the cookie for the next entry. We have
916 * to either find the entry with the appropriate number or
917 * revalidate the cookie.
918 */
919 memset(desc, 0, sizeof(*desc));
920
> 921 down_write(&nfsi->rwsem);
922 desc->file = file;
923 desc->ctx = ctx;
924 desc->dir_cookie = &dir_ctx->dir_cookie;
---
0-DAY kernel test infrastructure Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all Intel Corporation
On 7 Dec 2016, at 11:30, Christoph Hellwig wrote: > On Wed, Dec 07, 2016 at 08:37:02AM -0500, Benjamin Coddington wrote: >> Optimizations to NFS to choose between READDIR and READDIRPLUS don't >> expect >> concurrent users of nfs_readdir(), and can cause the pagecache to >> repeatedly be invalidated unnecessarily for this case. Fix this by >> serializing nfs_readdir() on the directory's rwsem. > > Just implement .iterate instead of .iterate_shared and you'll get the > serialization for free. While doing that please add a comment on why > it's serialized. I had it in my head that Al was trying to get rid of .iterate, otherwise yes, this would be more sensible. I think it'll still benefit from .iterate_shared in the regular case (the case where we're not trying to use READDIRPLUS), so I'll try out Trond's suggestion to fix it up before chasing this down. Ben -- To unsubscribe from this list: send the line "unsubscribe linux-nfs" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/fs/nfs/dir.c b/fs/nfs/dir.c index 53e6d6a4bc9c..8321252a4c8d 100644 --- a/fs/nfs/dir.c +++ b/fs/nfs/dir.c @@ -900,6 +900,7 @@ static int nfs_readdir(struct file *file, struct dir_context *ctx) { struct dentry *dentry = file_dentry(file); struct inode *inode = d_inode(dentry); + struct nfs_inode *nfsi = NFS_I(inode); nfs_readdir_descriptor_t my_desc, *desc = &my_desc; struct nfs_open_dir_context *dir_ctx = file->private_data; @@ -917,6 +918,7 @@ static int nfs_readdir(struct file *file, struct dir_context *ctx) */ memset(desc, 0, sizeof(*desc)); + down_write(&nfsi->rwsem); desc->file = file; desc->ctx = ctx; desc->dir_cookie = &dir_ctx->dir_cookie; @@ -958,6 +960,7 @@ static int nfs_readdir(struct file *file, struct dir_context *ctx) break; } while (!desc->eof); out: + up_write(&nfsi->rwsem); if (res > 0) res = 0; dfprintk(FILE, "NFS: readdir(%pD2) returns %d\n", file, res);
Optimizations to NFS to choose between READDIR and READDIRPLUS don't expect concurrent users of nfs_readdir(), and can cause the pagecache to repeatedly be invalidated unnecessarily for this case. Fix this by serializing nfs_readdir() on the directory's rwsem. Signed-off-by: Benjamin Coddington <bcodding@redhat.com> --- fs/nfs/dir.c | 3 +++ 1 file changed, 3 insertions(+)