linux-fsdevel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: David Woodhouse <dwmw2@infradead.org>
To: Neil Brown <neilb@suse.de>
Cc: Christoph Hellwig <hch@infradead.org>,
	linux-fsdevel@vger.kernel.org, linux-nfs@vger.kernel.org,
	linux-mtd@lists.infradead.org
Subject: [PATCH 2/4] Copy XFS readdir hack into nfsd code, introduce FS_NO_LOOKUP_IN_READDIR flag
Date: Thu, 31 Jul 2008 22:54:56 +0100	[thread overview]
Message-ID: <1217541296.1126.18.camel@shinybook.infradead.org> (raw)
In-Reply-To: <18458.28833.539314.455215@notabene.brown>

Some file systems with their own internal locking have problems with the
way that nfsd calls the ->lookup() method from within a filldir function
called from their ->readdir() method. The recursion back into the file
system code can cause deadlock.

XFS has a fairly hackish solution to this which involves doing the
readdir() into a locally-allocated buffer, then going back through it
calling the filldir function afterwards. It's not ideal, but it works.

It's particularly suboptimal because XFS does this for local file
systems too, where it's completely unnecessary.

Copy this hack into the NFS code where it can be used only for NFS, and
only for file systems which indicate that they need it by setting
FS_NO_LOOKUP_IN_READDIR in their fs_type flags.

Signed-off-by: David Woodhouse <David.Woodhouse@intel.com>
---
 fs/nfsd/vfs.c      |  111 +++++++++++++++++++++++++++++++++++++++++++++++++++-
 include/linux/fs.h |    2 +
 2 files changed, 112 insertions(+), 1 deletions(-)

diff --git a/fs/nfsd/vfs.c b/fs/nfsd/vfs.c
index 3e22634..81c9411 100644
--- a/fs/nfsd/vfs.c
+++ b/fs/nfsd/vfs.c
@@ -1814,6 +1814,111 @@ out:
 	return err;
 }
 
+struct hack_dirent {
+	u64		ino;
+	loff_t		offset;
+	int		namlen;
+	unsigned int	d_type;
+	char		name[];
+};
+
+struct hack_callback {
+	char		*dirent;
+	size_t		len;
+	size_t		used;
+};
+
+static int nfsd_hack_filldir(void *__buf, const char *name, int namlen,
+			     loff_t offset, u64 ino, unsigned int d_type)
+{
+	struct hack_callback *buf = __buf;
+	struct hack_dirent *de = (void *)(buf->dirent + buf->used);
+	unsigned int reclen;
+
+	reclen = ALIGN(sizeof(struct hack_dirent) + namlen, sizeof(u64));
+	if (buf->used + reclen > buf->len)
+		return -EINVAL;
+
+	de->namlen = namlen;
+	de->offset = offset;
+	de->ino = ino;
+	de->d_type = d_type;
+	memcpy(de->name, name, namlen);
+	buf->used += reclen;
+
+	return 0;
+}
+
+static int nfsd_hack_readdir(struct file *file, filldir_t func, 
+			     struct readdir_cd *cdp, loff_t *offsetp)
+{
+	struct hack_callback buf;
+	struct hack_dirent *de;
+	int host_err;
+	int size;
+	loff_t offset;
+
+	/*
+	 * Try fairly hard to get memory
+	 */
+	buf.len = PAGE_CACHE_SIZE;
+	do {
+		buf.dirent = kmalloc(buf.len, GFP_KERNEL);
+		if (buf.dirent)
+			break;
+		buf.len >>= 1;
+	} while (buf.len >= 1024);
+
+	if (!buf.dirent)
+		return -ENOMEM;
+
+	offset = *offsetp;
+	cdp->err = nfserr_eof; /* will be cleared on successful read */
+
+	while (1) {
+		unsigned int reclen;
+
+		buf.used = 0;
+
+		host_err = vfs_readdir(file, nfsd_hack_filldir, &buf);
+		if (host_err)
+			break;
+
+		size = buf.used;
+
+		if (!size)
+			break;
+
+
+		de = (struct hack_dirent *)buf.dirent;
+		while (size > 0) {
+			offset = de->offset;
+
+			if (func(cdp, de->name, de->namlen, de->offset,
+				 de->ino, de->d_type))
+				goto done;
+
+			if (cdp->err != nfs_ok)
+				goto done;
+
+			reclen = ALIGN(sizeof(struct hack_dirent) + de->namlen,
+				       sizeof(u64));
+			size -= reclen;
+			de = (struct hack_dirent *)((char *)de + reclen);
+		}
+		offset = vfs_llseek(file, 0, 1);
+	}
+
+ done:
+	kfree(buf.dirent);
+
+	if (host_err)
+		return nfserrno(host_err);
+
+	*offsetp = offset;
+	return cdp->err;
+}
+
 static int nfsd_do_readdir(struct file *file, filldir_t func,
 			   struct readdir_cd *cdp, loff_t *offsetp)
 {
@@ -1859,7 +1964,11 @@ nfsd_readdir(struct svc_rqst *rqstp, struct svc_fh *fhp, loff_t *offsetp,
 		goto out_close;
 	}
 
-	err = nfsd_do_readdir(file, func, cdp, offsetp);
+	if ((file->f_path.dentry->d_inode->i_sb->s_type->fs_flags &
+	     FS_NO_LOOKUP_IN_READDIR))
+		err = nfsd_hack_readdir(file, func, cdp, offsetp);
+	else
+		err = nfsd_do_readdir(file, func, cdp, offsetp);
 
 	if (err == nfserr_eof || err == nfserr_toosmall)
 		err = nfs_ok; /* can still be found in ->err */
diff --git a/include/linux/fs.h b/include/linux/fs.h
index 580b513..80ca410 100644
--- a/include/linux/fs.h
+++ b/include/linux/fs.h
@@ -100,6 +100,8 @@ extern int dir_notify_enable;
 #define FS_RENAME_DOES_D_MOVE	32768	/* FS will handle d_move()
 					 * during rename() internally.
 					 */
+#define FS_NO_LOOKUP_IN_READDIR	65536	/* FS will deadlock if you call its
+					   lookup() method from filldir */
 
 /*
  * These are the fs-independent mount-flags: up to 32 flags are supported
-- 
1.5.5.1


-- 
David Woodhouse                            Open Source Technology Centre
David.Woodhouse@intel.com                              Intel Corporation


  parent reply	other threads:[~2008-07-31 21:55 UTC|newest]

Thread overview: 55+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2008-05-01 19:42 [RFC] Reinstate NFS exportability for JFFS2 David Woodhouse
2008-05-01 20:48 ` Christoph Hellwig
2008-05-01 22:44   ` David Woodhouse
2008-05-02  1:38     ` Neil Brown
2008-05-02 11:37       ` David Woodhouse
     [not found]         ` <1209728238.25560.686.camel-ZP4jZrcIevRpWr+L1FloEB2eb7JE58TQ@public.gmane.org>
2008-05-02 14:08           ` J. Bruce Fields
2008-07-31 21:54       ` David Woodhouse
2008-08-01  0:16         ` Neil Brown
     [not found]           ` <18578.21997.529551.676627-wvvUuzkyo1EYVZTmpyfIwg@public.gmane.org>
2008-08-01  0:40             ` David Woodhouse
     [not found]               ` <1217551230.3719.15.camel-Fexsq3y4057IgHVZqg5X0TlWvGAXklZc@public.gmane.org>
2008-08-01  0:52                 ` David Woodhouse
2008-08-01  0:53               ` Chuck Lever
     [not found]                 ` <76bd70e30807311753m2785c6d3kd82edd1fe8b5f8b7-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
2008-08-01  1:00                   ` David Woodhouse
     [not found]                     ` <1217552437.3719.30.camel-Fexsq3y4057IgHVZqg5X0TlWvGAXklZc@public.gmane.org>
2008-08-01  1:31                       ` Chuck Lever
2008-08-01  8:13                         ` David Woodhouse
2008-08-01 13:35                         ` David Woodhouse
     [not found]                           ` <1217597759.3454.356.camel-ZP4jZrcIevRpWr+L1FloEB2eb7JE58TQ@public.gmane.org>
2008-08-01 13:56                             ` David Woodhouse
2008-08-01 16:05                               ` Chuck Lever
2008-08-01 16:19                                 ` David Woodhouse
2008-08-01 17:47                                   ` Chuck Lever
2008-08-02 18:26                                     ` J. Bruce Fields
     [not found]                                       ` <20080802182644.GE30454-uC3wQj2KruNg9hUCZPvPmw@public.gmane.org>
2008-08-02 20:42                                         ` David Woodhouse
2008-08-02 21:33                                           ` J. Bruce Fields
     [not found]                                             ` <20080802213337.GA2833-uC3wQj2KruNg9hUCZPvPmw@public.gmane.org>
2008-08-03  8:39                                               ` David Woodhouse
2008-08-03 11:56                                       ` Neil Brown
2008-08-03 17:15                                         ` Chuck Lever
2008-08-04  1:03                                           ` Neil Brown
     [not found]                                             ` <18582.21855.2092.903688-wvvUuzkyo1EYVZTmpyfIwg@public.gmane.org>
2008-08-04  6:19                                               ` Chuck Lever
2008-08-05  8:51                                                 ` Dave Chinner
2008-08-05  8:59                                                   ` David Woodhouse
2008-08-05  9:47                                                     ` Dave Chinner
2008-08-05 23:06                                                   ` Neil Brown
2008-08-06  0:08                                                     ` Dave Chinner
2008-08-06 19:56                                                       ` J. Bruce Fields
2008-08-06 20:10                                                         ` David Woodhouse
     [not found]                                                           ` <1218053443.5111.148.camel-ZP4jZrcIevRpWr+L1FloEB2eb7JE58TQ@public.gmane.org>
2008-08-09 16:47                                                             ` David Woodhouse
2008-08-09 19:55                                                               ` David Woodhouse
     [not found]                                                                 ` <1218311710.26926.125.camel-ZP4jZrcIevRpWr+L1FloEB2eb7JE58TQ@public.gmane.org>
2008-08-09 20:01                                                                   ` [PATCH 1/4] Factor out nfsd_do_readdir() into its own function David Woodhouse
     [not found]                                                                     ` <1218312114.5063.5.camel-ZP4jZrcIevRpWr+L1FloEB2eb7JE58TQ@public.gmane.org>
2008-08-09 20:07                                                                       ` Christoph Hellwig
2008-08-09 20:02                                                                   ` [PATCH 2/4] Copy XFS readdir hack into nfsd code David Woodhouse
2008-08-09 20:08                                                                     ` Christoph Hellwig
2008-08-09 20:03                                                                   ` [PATCH 3/4] Remove XFS buffered readdir hack David Woodhouse
     [not found]                                                                     ` <1218312191.5063.8.camel-ZP4jZrcIevRpWr+L1FloEB2eb7JE58TQ@public.gmane.org>
2008-08-09 20:09                                                                       ` Christoph Hellwig
2008-08-09 20:03                                                                   ` [PATCH 4/4] Reinstate NFS exportability David Woodhouse
     [not found]                                                                     ` <1218312213.5063.9.camel-ZP4jZrcIevRpWr+L1FloEB2eb7JE58TQ@public.gmane.org>
2008-08-09 20:10                                                                       ` Christoph Hellwig
2008-08-17 18:22                                               ` [RFC] Reinstate NFS exportability for JFFS2 Andreas Dilger
2008-08-04 18:41                                             ` J. Bruce Fields
2008-08-04 22:37                                               ` Neil Brown
2008-08-01  2:14               ` Neil Brown
     [not found]                 ` <18578.29049.38904.746701-wvvUuzkyo1EYVZTmpyfIwg@public.gmane.org>
2008-08-01  8:50                   ` David Woodhouse
2008-08-01 10:03                   ` Al Viro
2008-08-01 23:11                     ` Neil Brown
2008-07-31 21:54       ` [PATCH 1/4] Factor out nfsd_do_readdir() into its own function David Woodhouse
2008-07-31 21:54       ` David Woodhouse [this message]
2008-07-31 21:55       ` [PATCH 3/4] Switch XFS to using FS_NO_LOOKUP_IN_READDIR, remove local readdir hack David Woodhouse
     [not found]       ` <18458.28833.539314.455215-wvvUuzkyo1EYVZTmpyfIwg@public.gmane.org>
2008-07-31 21:55         ` [PATCH 4/4] [JFFS2] Reinstate NFS exportability David Woodhouse

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=1217541296.1126.18.camel@shinybook.infradead.org \
    --to=dwmw2@infradead.org \
    --cc=hch@infradead.org \
    --cc=linux-fsdevel@vger.kernel.org \
    --cc=linux-mtd@lists.infradead.org \
    --cc=linux-nfs@vger.kernel.org \
    --cc=neilb@suse.de \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).