[PATCH v3 0/2] FUSE: Adaptive NFS-like readdirplus support

[PATCH v3 0/2] FUSE: Adaptive NFS-like readdirplus support

[Feng Shuo]

The original idea belows to Anand V. Avati:

http://sourceforge.net/mailarchive/message.php?msg_id=29562240

I did some benchmarking and add the adaptive mechanism as in NFS. For 
the detail of the testing, please refer to:

http://sourceforge.net/mailarchive/message.php?msg_id=30305348

In my setup, this patch set can give about 50% performance up with "ls -l"
and without much penalty with pure "ls" because of the second patch. 

Rebased to 3.8-rc3.

Feng Shuo (1):
  FUSE: Adapt readdirplus to application usage patterns

Miklos Szeredi (1):
  fuse: implement NFS-like readdirplus support

 fs/fuse/dev.c             |  19 +++++
 fs/fuse/dir.c             | 206 ++++++++++++++++++++++++++++++++++++++++++++--
 fs/fuse/fuse_i.h          |  16 ++++
 fs/fuse/inode.c           |   8 +-
 include/uapi/linux/fuse.h |  12 +++
 5 files changed, 254 insertions(+), 7 deletions(-)

-- 
1.7.12.4

[PATCH v3 1/2] fuse: implement NFS-like readdirplus support

[Feng Shuo]

From: Miklos Szeredi <miklos@szeredi.hu>

Anand Avati <avati@redhat.com> writes:

> This version of the patch
>
> - incorporates review comments from the previous submission
> - fixes nlookup leak on entries in the first readdirplus
> - cleaner fuse_direntp structure

Anand,

Here's an updated version of the patch.  It fixes some issues (real and
cosmetic) with your latest patch.  Can you please look it over and give
it a test.

Thanks,
Miklos

Signed-off-by: Feng Shuo <steve.shuo.feng@gmail.com>
---
 fs/fuse/dev.c             |  19 ++++++
 fs/fuse/dir.c             | 160 ++++++++++++++++++++++++++++++++++++++++++++--
 fs/fuse/fuse_i.h          |   6 ++
 fs/fuse/inode.c           |   5 +-
 include/uapi/linux/fuse.h |  12 ++++
 5 files changed, 197 insertions(+), 5 deletions(-)

diff --git a/fs/fuse/dev.c b/fs/fuse/dev.c
index c163353..8e8b216 100644
--- a/fs/fuse/dev.c
+++ b/fs/fuse/dev.c
@@ -491,6 +491,25 @@ void fuse_request_send_background_locked(struct fuse_conn *fc,
 	fuse_request_send_nowait_locked(fc, req);
 }
 
+void fuse_force_forget(struct file *file, u64 nodeid)
+{
+	struct inode *inode = file->f_path.dentry->d_inode;
+	struct fuse_conn *fc = get_fuse_conn(inode);
+	struct fuse_req *req;
+	struct fuse_forget_in inarg;
+
+	memset(&inarg, 0, sizeof(inarg));
+	inarg.nlookup = 1;
+	req = fuse_get_req_nofail(fc, file);
+	req->in.h.opcode = FUSE_FORGET;
+	req->in.h.nodeid = nodeid;
+	req->in.numargs = 1;
+	req->in.args[0].size = sizeof(inarg);
+	req->in.args[0].value = &inarg;
+	req->isreply = 0;
+	fuse_request_send_nowait(fc, req);
+}
+
 /*
  * Lock the request.  Up to the next unlock_request() there mustn't be
  * anything that could cause a page-fault.  If the request was already
diff --git a/fs/fuse/dir.c b/fs/fuse/dir.c
index b7c09f9..dcc1e52 100644
--- a/fs/fuse/dir.c
+++ b/fs/fuse/dir.c
@@ -1155,6 +1155,143 @@ static int parse_dirfile(char *buf, size_t nbytes, struct file *file,
 	return 0;
 }
 
+static int fuse_direntplus_link(struct file *file,
+				struct fuse_direntplus *direntplus,
+				u64 attr_version)
+{
+	int err;
+	struct fuse_entry_out *o = &direntplus->entry_out;
+	struct fuse_dirent *dirent = &direntplus->dirent;
+	struct dentry *parent = file->f_path.dentry;
+	struct qstr name = QSTR_INIT(dirent->name, dirent->namelen);
+	struct dentry *dentry;
+	struct dentry *alias;
+	struct inode *dir = parent->d_inode;
+	struct fuse_conn *fc;
+	struct inode *inode;
+
+	if (!o->nodeid) {
+		/*
+		 * Unlike in the case of fuse_lookup, zero nodeid does not mean
+		 * ENOENT. Instead, it only means the userspace filesystem did
+		 * not want to return attributes/handle for this entry.
+		 *
+		 * So do nothing.
+		 */
+		return 0;
+	}
+
+	if (name.name[0] == '.') {
+		/*
+		 * We could potentially refresh the attributes of the directory
+		 * and its parent?
+		 */
+		if (name.len == 1)
+			return 0;
+		if (name.name[1] == '.' && name.len == 2)
+			return 0;
+	}
+	fc = get_fuse_conn(dir);
+
+	name.hash = full_name_hash(name.name, name.len);
+	dentry = d_lookup(parent, &name);
+	if (dentry && dentry->d_inode) {
+		inode = dentry->d_inode;
+		if (get_node_id(inode) == o->nodeid) {
+			struct fuse_inode *fi;
+			fi = get_fuse_inode(inode);
+			spin_lock(&fc->lock);
+			fi->nlookup++;
+			spin_unlock(&fc->lock);
+
+			/*
+			 * The other branch to 'found' comes via fuse_iget()
+			 * which bumps nlookup inside
+			 */
+			goto found;
+		}
+		err = d_invalidate(dentry);
+		if (err)
+			goto out;
+		dput(dentry);
+		dentry = NULL;
+	}
+
+	dentry = d_alloc(parent, &name);
+	err = -ENOMEM;
+	if (!dentry)
+		goto out;
+
+	inode = fuse_iget(dir->i_sb, o->nodeid, o->generation,
+			  &o->attr, entry_attr_timeout(o), attr_version);
+	if (!inode)
+		goto out;
+
+	alias = d_materialise_unique(dentry, inode);
+	err = PTR_ERR(alias);
+	if (IS_ERR(alias))
+		goto out;
+	if (alias) {
+		dput(dentry);
+		dentry = alias;
+	}
+
+found:
+	fuse_change_attributes(inode, &o->attr, entry_attr_timeout(o),
+			       attr_version);
+
+	fuse_change_entry_timeout(dentry, o);
+
+	err = 0;
+out:
+	if (dentry)
+		dput(dentry);
+	return err;
+}
+
+static int parse_dirplusfile(char *buf, size_t nbytes, struct file *file,
+			     void *dstbuf, filldir_t filldir, u64 attr_version)
+{
+	struct fuse_direntplus *direntplus;
+	struct fuse_dirent *dirent;
+	size_t reclen;
+	int over = 0;
+	int ret;
+
+	while (nbytes >= FUSE_NAME_OFFSET_DIRENTPLUS) {
+		direntplus = (struct fuse_direntplus *) buf;
+		dirent = &direntplus->dirent;
+		reclen = FUSE_DIRENTPLUS_SIZE(direntplus);
+
+		if (!dirent->namelen || dirent->namelen > FUSE_NAME_MAX)
+			return -EIO;
+		if (reclen > nbytes)
+			break;
+
+		if (!over) {
+			/* We fill entries into dstbuf only as much as
+			   it can hold. But we still continue iterating
+			   over remaining entries to link them. If not,
+			   we need to send a FORGET for each of those
+			   which we did not link.
+			*/
+			over = filldir(dstbuf, dirent->name, dirent->namelen,
+				       file->f_pos, dirent->ino,
+				       dirent->type);
+			file->f_pos = dirent->off;
+		}
+
+		buf += reclen;
+		nbytes -= reclen;
+
+		ret = fuse_direntplus_link(file, direntplus, attr_version);
+		if (ret)
+			fuse_force_forget(file, direntplus->entry_out.nodeid);
+	}
+
+	return 0;
+}
+
 static int fuse_readdir(struct file *file, void *dstbuf, filldir_t filldir)
 {
 	int err;
@@ -1163,6 +1300,7 @@ static int fuse_readdir(struct file *file, void *dstbuf, filldir_t filldir)
 	struct inode *inode = file->f_path.dentry->d_inode;
 	struct fuse_conn *fc = get_fuse_conn(inode);
 	struct fuse_req *req;
+	u64 attr_version = 0;
 
 	if (is_bad_inode(inode))
 		return -EIO;
@@ -1179,14 +1317,28 @@ static int fuse_readdir(struct file *file, void *dstbuf, filldir_t filldir)
 	req->out.argpages = 1;
 	req->num_pages = 1;
 	req->pages[0] = page;
-	fuse_read_fill(req, file, file->f_pos, PAGE_SIZE, FUSE_READDIR);
+	if (fc->do_readdirplus) {
+		attr_version = fuse_get_attr_version(fc);
+		fuse_read_fill(req, file, file->f_pos, PAGE_SIZE,
+			       FUSE_READDIRPLUS);
+	} else {
+		fuse_read_fill(req, file, file->f_pos, PAGE_SIZE,
+			       FUSE_READDIR);
+	}
 	fuse_request_send(fc, req);
 	nbytes = req->out.args[0].size;
 	err = req->out.h.error;
 	fuse_put_request(fc, req);
-	if (!err)
-		err = parse_dirfile(page_address(page), nbytes, file, dstbuf,
-				    filldir);
+	if (!err) {
+		if (fc->do_readdirplus) {
+			err = parse_dirplusfile(page_address(page), nbytes,
+						file, dstbuf, filldir,
+						attr_version);
+		} else {
+			err = parse_dirfile(page_address(page), nbytes, file,
+					    dstbuf, filldir);
+		}
+	}
 
 	__free_page(page);
 	fuse_invalidate_attr(inode); /* atime changed */
diff --git a/fs/fuse/fuse_i.h b/fs/fuse/fuse_i.h
index e105a53..5c50553 100644
--- a/fs/fuse/fuse_i.h
+++ b/fs/fuse/fuse_i.h
@@ -487,6 +487,9 @@ struct fuse_conn {
 	/** Use enhanced/automatic page cache invalidation. */
 	unsigned auto_inval_data:1;
 
+	/** Does the filesystem support readdir-plus? */
+	unsigned do_readdirplus:1;
+
 	/** The number of requests waiting for completion */
 	atomic_t num_waiting;
 
@@ -578,6 +581,9 @@ void fuse_queue_forget(struct fuse_conn *fc, struct fuse_forget_link *forget,
 
 struct fuse_forget_link *fuse_alloc_forget(void);
 
+/* Used by READDIRPLUS */
+void fuse_force_forget(struct file *file, u64 nodeid);
+
 /**
  * Initialize READ or READDIR request
  */
diff --git a/fs/fuse/inode.c b/fs/fuse/inode.c
index 73ca6b7..6f7d574 100644
--- a/fs/fuse/inode.c
+++ b/fs/fuse/inode.c
@@ -863,6 +863,8 @@ static void process_init_reply(struct fuse_conn *fc, struct fuse_req *req)
 				fc->dont_mask = 1;
 			if (arg->flags & FUSE_AUTO_INVAL_DATA)
 				fc->auto_inval_data = 1;
+			if (arg->flags & FUSE_DO_READDIRPLUS)
+				fc->do_readdirplus = 1;
 		} else {
 			ra_pages = fc->max_read / PAGE_CACHE_SIZE;
 			fc->no_lock = 1;
@@ -889,7 +891,8 @@ static void fuse_send_init(struct fuse_conn *fc, struct fuse_req *req)
 	arg->flags |= FUSE_ASYNC_READ | FUSE_POSIX_LOCKS | FUSE_ATOMIC_O_TRUNC |
 		FUSE_EXPORT_SUPPORT | FUSE_BIG_WRITES | FUSE_DONT_MASK |
 		FUSE_SPLICE_WRITE | FUSE_SPLICE_MOVE | FUSE_SPLICE_READ |
-		FUSE_FLOCK_LOCKS | FUSE_IOCTL_DIR | FUSE_AUTO_INVAL_DATA;
+		FUSE_FLOCK_LOCKS | FUSE_IOCTL_DIR | FUSE_AUTO_INVAL_DATA |
+		FUSE_DO_READDIRPLUS;
 	req->in.h.opcode = FUSE_INIT;
 	req->in.numargs = 1;
 	req->in.args[0].size = sizeof(*arg);
diff --git a/include/uapi/linux/fuse.h b/include/uapi/linux/fuse.h
index d8c713e..5dc1fea 100644
--- a/include/uapi/linux/fuse.h
+++ b/include/uapi/linux/fuse.h
@@ -193,6 +193,7 @@ struct fuse_file_lock {
 #define FUSE_FLOCK_LOCKS	(1 << 10)
 #define FUSE_HAS_IOCTL_DIR	(1 << 11)
 #define FUSE_AUTO_INVAL_DATA	(1 << 12)
+#define FUSE_DO_READDIRPLUS	(1 << 13)
 
 /**
  * CUSE INIT request/reply flags
@@ -299,6 +300,7 @@ enum fuse_opcode {
 	FUSE_NOTIFY_REPLY  = 41,
 	FUSE_BATCH_FORGET  = 42,
 	FUSE_FALLOCATE     = 43,
+	FUSE_READDIRPLUS   = 44,
 
 	/* CUSE specific operations */
 	CUSE_INIT          = 4096,
@@ -630,6 +632,16 @@ struct fuse_dirent {
 #define FUSE_DIRENT_SIZE(d) \
 	FUSE_DIRENT_ALIGN(FUSE_NAME_OFFSET + (d)->namelen)
 
+struct fuse_direntplus {
+	struct fuse_entry_out entry_out;
+	struct fuse_dirent dirent;
+};
+
+#define FUSE_NAME_OFFSET_DIRENTPLUS \
+	offsetof(struct fuse_direntplus, dirent.name)
+#define FUSE_DIRENTPLUS_SIZE(d) \
+	FUSE_DIRENT_ALIGN(FUSE_NAME_OFFSET_DIRENTPLUS + (d)->dirent.namelen)
+
 struct fuse_notify_inval_inode_out {
 	__u64	ino;
 	__s64	off;
-- 
1.7.12.4

[PATCH v3 2/2] FUSE: Adapt readdirplus to application usage patterns

[Feng Shuo]

Use the same adaptive readdirplus mechanism as NFS:

http://permalink.gmane.org/gmane.linux.nfs/49299

If the user space implementation wants to disable readdirplus
temporarily, it could just return ENOTSUPP. Then kernel will
recall it with readdir.

Signed-off-by: Feng Shuo <steve.shuo.feng@gmail.com>
---
 fs/fuse/dir.c    | 50 ++++++++++++++++++++++++++++++++++++++++++++++----
 fs/fuse/fuse_i.h | 10 ++++++++++
 fs/fuse/inode.c  |  3 +++
 3 files changed, 59 insertions(+), 4 deletions(-)

diff --git a/fs/fuse/dir.c b/fs/fuse/dir.c
index dcc1e52..b9975df 100644
--- a/fs/fuse/dir.c
+++ b/fs/fuse/dir.c
@@ -14,6 +14,34 @@
 #include <linux/namei.h>
 #include <linux/slab.h>
 
+static bool fuse_use_readdirplus(struct inode *dir, struct file *filp)
+{
+	struct fuse_conn *fc = get_fuse_conn(dir);
+	struct fuse_inode *fi = get_fuse_inode(dir);
+
+	if (!fc->do_readdirplus)
+		return false;
+	if (test_and_clear_bit(FUSE_I_ADVISE_RDPLUS, &fi->state))
+		return true;
+	if (filp->f_pos == 0)
+		return true;
+	return false;
+}
+
+static void fuse_advise_use_readdirplus(struct inode *dir)
+{
+	struct fuse_inode *fi = get_fuse_inode(dir);
+
+	set_bit(FUSE_I_ADVISE_RDPLUS, &fi->state);
+}
+
+static void fuse_stop_use_readdirplus(struct inode *dir)
+{
+	struct fuse_inode *fi = get_fuse_inode(dir);
+
+	clear_bit(FUSE_I_ADVISE_RDPLUS, &fi->state);
+}
+
 #if BITS_PER_LONG >= 64
 static inline void fuse_dentry_settime(struct dentry *entry, u64 time)
 {
@@ -219,6 +247,7 @@ static int fuse_dentry_revalidate(struct dentry *entry, unsigned int flags)
 				       attr_version);
 		fuse_change_entry_timeout(entry, &outarg);
 	}
+	fuse_advise_use_readdirplus(inode);
 	return 1;
 }
 
@@ -355,6 +384,7 @@ static struct dentry *fuse_lookup(struct inode *dir, struct dentry *entry,
 	else
 		fuse_invalidate_entry_cache(entry);
 
+	fuse_advise_use_readdirplus(dir);
 	return newent;
 
  out_iput:
@@ -1294,7 +1324,7 @@ static int parse_dirplusfile(char *buf, size_t nbytes, struct file *file,
 
 static int fuse_readdir(struct file *file, void *dstbuf, filldir_t filldir)
 {
-	int err;
+	int plus, err;
 	size_t nbytes;
 	struct page *page;
 	struct inode *inode = file->f_path.dentry->d_inode;
@@ -1314,10 +1344,13 @@ static int fuse_readdir(struct file *file, void *dstbuf, filldir_t filldir)
 		fuse_put_request(fc, req);
 		return -ENOMEM;
 	}
+
+	plus = fuse_use_readdirplus(inode, file);
+read_again:
 	req->out.argpages = 1;
 	req->num_pages = 1;
 	req->pages[0] = page;
-	if (fc->do_readdirplus) {
+	if (plus) {
 		attr_version = fuse_get_attr_version(fc);
 		fuse_read_fill(req, file, file->f_pos, PAGE_SIZE,
 			       FUSE_READDIRPLUS);
@@ -1329,8 +1362,17 @@ static int fuse_readdir(struct file *file, void *dstbuf, filldir_t filldir)
 	nbytes = req->out.args[0].size;
 	err = req->out.h.error;
 	fuse_put_request(fc, req);
+	if ((err == -ENOTSUPP) && plus) {
+		fuse_stop_use_readdirplus(inode);
+		plus = 0;
+		req = fuse_get_req(fc);
+		if (!IS_ERR(req))
+			goto read_again;
+		err = PTR_ERR(req);
+		goto out;
+	}
 	if (!err) {
-		if (fc->do_readdirplus) {
+		if (plus) {
 			err = parse_dirplusfile(page_address(page), nbytes,
 						file, dstbuf, filldir,
 						attr_version);
@@ -1339,7 +1381,7 @@ static int fuse_readdir(struct file *file, void *dstbuf, filldir_t filldir)
 					    dstbuf, filldir);
 		}
 	}
-
+out:
 	__free_page(page);
 	fuse_invalidate_attr(inode); /* atime changed */
 	return err;
diff --git a/fs/fuse/fuse_i.h b/fs/fuse/fuse_i.h
index 5c50553..e582603 100644
--- a/fs/fuse/fuse_i.h
+++ b/fs/fuse/fuse_i.h
@@ -103,6 +103,16 @@ struct fuse_inode {
 
 	/** List of writepage requestst (pending or sent) */
 	struct list_head writepages;
+
+	/** Miscellaneous bits describing inode state */
+	unsigned long state;
+};
+
+/** FUSE inode state bits */
+enum {
+	/** Advise readdirplus  */
+	FUSE_I_ADVISE_RDPLUS,
+	/* FUSE_I_MTIME_UPDATED, */
 };
 
 struct fuse_conn;
diff --git a/fs/fuse/inode.c b/fs/fuse/inode.c
index 6f7d574..4ba1cf5 100644
--- a/fs/fuse/inode.c
+++ b/fs/fuse/inode.c
@@ -255,6 +255,9 @@ static void fuse_init_inode(struct inode *inode, struct fuse_attr *attr)
 				   new_decode_dev(attr->rdev));
 	} else
 		BUG();
+
+	/* The readdir_plus is disabled initially. */
+	get_fuse_inode(inode)->state = 0;
 }
 
 int fuse_inode_eq(struct inode *inode, void *_nodeidp)
-- 
1.7.12.4

Re: [PATCH v3 2/2] FUSE: Adapt readdirplus to application usage patterns

[Miklos Szeredi]

On Tue, Jan 15, 2013 at 4:23 AM, Feng Shuo <steve.shuo.feng-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote:
> Use the same adaptive readdirplus mechanism as NFS:
>
> http://permalink.gmane.org/gmane.linux.nfs/49299

OK, applied with minor updates.

> If the user space implementation wants to disable readdirplus
> temporarily, it could just return ENOTSUPP. Then kernel will
> recall it with readdir.

I removed this part.  It may make sense, but it's not part of the
adaptive readdirplus algorithm, so I'd like to see this as a separate
patch.

Thanks,
Miklos



> Signed-off-by: Feng Shuo <steve.shuo.feng-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
> ---
>  fs/fuse/dir.c    | 50 ++++++++++++++++++++++++++++++++++++++++++++++----
>  fs/fuse/fuse_i.h | 10 ++++++++++
>  fs/fuse/inode.c  |  3 +++
>  3 files changed, 59 insertions(+), 4 deletions(-)
>
> diff --git a/fs/fuse/dir.c b/fs/fuse/dir.c
> index dcc1e52..b9975df 100644
> --- a/fs/fuse/dir.c
> +++ b/fs/fuse/dir.c
> @@ -14,6 +14,34 @@
>  #include <linux/namei.h>
>  #include <linux/slab.h>
>
> +static bool fuse_use_readdirplus(struct inode *dir, struct file *filp)
> +{
> +       struct fuse_conn *fc = get_fuse_conn(dir);
> +       struct fuse_inode *fi = get_fuse_inode(dir);
> +
> +       if (!fc->do_readdirplus)
> +               return false;
> +       if (test_and_clear_bit(FUSE_I_ADVISE_RDPLUS, &fi->state))
> +               return true;
> +       if (filp->f_pos == 0)
> +               return true;
> +       return false;
> +}
> +
> +static void fuse_advise_use_readdirplus(struct inode *dir)
> +{
> +       struct fuse_inode *fi = get_fuse_inode(dir);
> +
> +       set_bit(FUSE_I_ADVISE_RDPLUS, &fi->state);
> +}
> +
> +static void fuse_stop_use_readdirplus(struct inode *dir)
> +{
> +       struct fuse_inode *fi = get_fuse_inode(dir);
> +
> +       clear_bit(FUSE_I_ADVISE_RDPLUS, &fi->state);
> +}
> +
>  #if BITS_PER_LONG >= 64
>  static inline void fuse_dentry_settime(struct dentry *entry, u64 time)
>  {
> @@ -219,6 +247,7 @@ static int fuse_dentry_revalidate(struct dentry *entry, unsigned int flags)
>                                        attr_version);
>                 fuse_change_entry_timeout(entry, &outarg);
>         }
> +       fuse_advise_use_readdirplus(inode);
>         return 1;
>  }
>
> @@ -355,6 +384,7 @@ static struct dentry *fuse_lookup(struct inode *dir, struct dentry *entry,
>         else
>                 fuse_invalidate_entry_cache(entry);
>
> +       fuse_advise_use_readdirplus(dir);
>         return newent;
>
>   out_iput:
> @@ -1294,7 +1324,7 @@ static int parse_dirplusfile(char *buf, size_t nbytes, struct file *file,
>
>  static int fuse_readdir(struct file *file, void *dstbuf, filldir_t filldir)
>  {
> -       int err;
> +       int plus, err;
>         size_t nbytes;
>         struct page *page;
>         struct inode *inode = file->f_path.dentry->d_inode;
> @@ -1314,10 +1344,13 @@ static int fuse_readdir(struct file *file, void *dstbuf, filldir_t filldir)
>                 fuse_put_request(fc, req);
>                 return -ENOMEM;
>         }
> +
> +       plus = fuse_use_readdirplus(inode, file);
> +read_again:
>         req->out.argpages = 1;
>         req->num_pages = 1;
>         req->pages[0] = page;
> -       if (fc->do_readdirplus) {
> +       if (plus) {
>                 attr_version = fuse_get_attr_version(fc);
>                 fuse_read_fill(req, file, file->f_pos, PAGE_SIZE,
>                                FUSE_READDIRPLUS);
> @@ -1329,8 +1362,17 @@ static int fuse_readdir(struct file *file, void *dstbuf, filldir_t filldir)
>         nbytes = req->out.args[0].size;
>         err = req->out.h.error;
>         fuse_put_request(fc, req);
> +       if ((err == -ENOTSUPP) && plus) {
> +               fuse_stop_use_readdirplus(inode);
> +               plus = 0;
> +               req = fuse_get_req(fc);
> +               if (!IS_ERR(req))
> +                       goto read_again;
> +               err = PTR_ERR(req);
> +               goto out;
> +       }
>         if (!err) {
> -               if (fc->do_readdirplus) {
> +               if (plus) {
>                         err = parse_dirplusfile(page_address(page), nbytes,
>                                                 file, dstbuf, filldir,
>                                                 attr_version);
> @@ -1339,7 +1381,7 @@ static int fuse_readdir(struct file *file, void *dstbuf, filldir_t filldir)
>                                             dstbuf, filldir);
>                 }
>         }
> -
> +out:
>         __free_page(page);
>         fuse_invalidate_attr(inode); /* atime changed */
>         return err;
> diff --git a/fs/fuse/fuse_i.h b/fs/fuse/fuse_i.h
> index 5c50553..e582603 100644
> --- a/fs/fuse/fuse_i.h
> +++ b/fs/fuse/fuse_i.h
> @@ -103,6 +103,16 @@ struct fuse_inode {
>
>         /** List of writepage requestst (pending or sent) */
>         struct list_head writepages;
> +
> +       /** Miscellaneous bits describing inode state */
> +       unsigned long state;
> +};
> +
> +/** FUSE inode state bits */
> +enum {
> +       /** Advise readdirplus  */
> +       FUSE_I_ADVISE_RDPLUS,
> +       /* FUSE_I_MTIME_UPDATED, */
>  };
>
>  struct fuse_conn;
> diff --git a/fs/fuse/inode.c b/fs/fuse/inode.c
> index 6f7d574..4ba1cf5 100644
> --- a/fs/fuse/inode.c
> +++ b/fs/fuse/inode.c
> @@ -255,6 +255,9 @@ static void fuse_init_inode(struct inode *inode, struct fuse_attr *attr)
>                                    new_decode_dev(attr->rdev));
>         } else
>                 BUG();
> +
> +       /* The readdir_plus is disabled initially. */
> +       get_fuse_inode(inode)->state = 0;
>  }
>
>  int fuse_inode_eq(struct inode *inode, void *_nodeidp)
> --
> 1.7.12.4
>
>
> ------------------------------------------------------------------------------
> Master SQL Server Development, Administration, T-SQL, SSAS, SSIS, SSRS
> and more. Get SQL Server skills now (including 2012) with LearnDevNow -
> 200+ hours of step-by-step video tutorials by Microsoft MVPs and experts.
> SALE $99.99 this month only - learn more at:
> http://p.sf.net/sfu/learnmore_122512
> _______________________________________________
> fuse-devel mailing list
> fuse-devel-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f@public.gmane.org
> https://lists.sourceforge.net/lists/listinfo/fuse-devel

------------------------------------------------------------------------------
Master Visual Studio, SharePoint, SQL, ASP.NET, C# 2012, HTML5, CSS,
MVC, Windows 8 Apps, JavaScript and much more. Keep your skills current
with LearnDevNow - 3,200 step-by-step video tutorials by Microsoft
MVPs and experts. ON SALE this month only -- learn more at:
http://p.sf.net/sfu/learnnow-d2d

Re: [PATCH v3 2/2] FUSE: Adapt readdirplus to application usage patterns

[Eric Wong]

Feng Shuo <steve.shuo.feng@gmail.com> wrote:
> Use the same adaptive readdirplus mechanism as NFS:
> 
> http://permalink.gmane.org/gmane.linux.nfs/49299
> 
> If the user space implementation wants to disable readdirplus
> temporarily, it could just return ENOTSUPP. Then kernel will
> recall it with readdir.

The version of this patch in "for-next" in
git://git.kernel.org/pub/scm/linux/kernel/git/mszeredi/fuse.git
(commit 4582a4ab2a0e7218449fb2e895d0aae9ea753c94)
seems to be causing problems for me with "find -ls"
(I trivially backported all the for-next patches to 3.7.5)

I see getdents() returning a single d_name="" entry at the end of the
list, leading to infinite looping when combined with the userspace
patches I posted in
http://mid.gmane.org/20130202043316.GA19751@dcvr.yhbt.net

Normally getdents() returns 0 to indicate EOF on the directory;
but with this patch, I no longer get that...

This only seems to happen on large directories which require multiple
getdents() calls.  Sometimes "ls -l" will succeed and "find -ls" will
fail, and sometimes strace seems to make the problem go away...

It could be a bug in my end in userspace, too.  However, reverting the
version of this patch that landed in for-next seems to avoid the issue
for me.  I've also verified none of my userspace code is sending
dentries with an empty name.

Re: [PATCH v3 2/2] FUSE: Adapt readdirplus to application usage patterns

[Eric Wong]

Eric Wong <normalperson@yhbt.net> wrote:
> Feng Shuo <steve.shuo.feng@gmail.com> wrote:
> > Use the same adaptive readdirplus mechanism as NFS:
> > 
> > http://permalink.gmane.org/gmane.linux.nfs/49299
> 
> The version of this patch in "for-next" in
> git://git.kernel.org/pub/scm/linux/kernel/git/mszeredi/fuse.git
> (commit 4582a4ab2a0e7218449fb2e895d0aae9ea753c94)

I can confirm this problem exists in Miklos's unmodified for-next,
too, and and reverting 4582a4ab2a0e7218449fb2e895d0aae9ea753c94
solves the problem for me.

Re: [PATCH v3 2/2] FUSE: Adapt readdirplus to application usage patterns

[Feng Shuo]

Thank you. I will check this soon. BTW. Do you think this is related
to the stack issue you mentioned in another thread?

2013/2/3, Eric Wong <normalperson@yhbt.net>:
> Eric Wong <normalperson@yhbt.net> wrote:
>> Feng Shuo <steve.shuo.feng@gmail.com> wrote:
>> > Use the same adaptive readdirplus mechanism as NFS:
>> >
>> > http://permalink.gmane.org/gmane.linux.nfs/49299
>>
>> The version of this patch in "for-next" in
>> git://git.kernel.org/pub/scm/linux/kernel/git/mszeredi/fuse.git
>> (commit 4582a4ab2a0e7218449fb2e895d0aae9ea753c94)
>
> I can confirm this problem exists in Miklos's unmodified for-next,
> too, and and reverting 4582a4ab2a0e7218449fb2e895d0aae9ea753c94
> solves the problem for me.
>

-- 
从我的移动设备发送

Feng Shuo
Tel: (86)10-59851155-2116
Fax: (86)10-59851155-2008
Tianjin Zhongke Blue Whale Information Technologies Co., Ltd
10th Floor, Tower A, The GATE building, No. 19 Zhong-guan-cun Avenue
Haidian District, Beijing, China
Postcode 100080
--
To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v3 2/2] FUSE: Adapt readdirplus to application usage patterns

[Eric Wong]

Feng Shuo <steve.shuo.feng@gmail.com> wrote:
> Thank you. I will check this soon. BTW. Do you think this is related
> to the stack issue you mentioned in another thread?

I initially thought it was related, but no, I don't think I trigger
fuse_force_forget()...  I need to take a closer look later.

Re: [fuse-devel] [PATCH v3 2/2] FUSE: Adapt readdirplus to application usage patterns

[Miklos Szeredi]

I think the problem is that directory offsets are not consistent
between readdir and readdirplus.  The solution is that userspace
should always use the same offsets.  This makes the readdirplus
implementation more complex but that's really the only sane thing to
do.

Thanks,
Miklos

Re: [PATCH v3 2/2] FUSE: Adapt readdirplus to application usage patterns

[Eric Wong]

Miklos Szeredi <miklos-sUDqSbJrdHQHWmgEVkV9KA@public.gmane.org> wrote:
> I think the problem is that directory offsets are not consistent
> between readdir and readdirplus.  The solution is that userspace
> should always use the same offsets.  This makes the readdirplus
> implementation more complex but that's really the only sane thing to
> do.

I figured as much.  Will update the userspace lib tomorrow (or wait
for Feng Shuo to do it :>)

------------------------------------------------------------------------------
Everyone hates slow websites. So do we.
Make your web apps faster with AppDynamics
Download AppDynamics Lite for free today:
http://p.sf.net/sfu/appdyn_d2d_jan

[RFC] fuse: consistently use readdirplus offsets

[Eric Wong]

Miklos Szeredi <miklos@szeredi.hu> wrote:
> I think the problem is that directory offsets are not consistent
> between readdir and readdirplus.  The solution is that userspace
> should always use the same offsets.  This makes the readdirplus
> implementation more complex but that's really the only sane thing to
> do.

I think something like the following is necessary in the kernel, too.
(Userspace patch coming)

------------------------------- 8< --------------------------------
>From ed52619fda61c8f2b723ef72666e7acdd8885a1c Mon Sep 17 00:00:00 2001
From: Eric Wong <normalperson@yhbt.net>
Date: Tue, 5 Feb 2013 02:54:28 +0000
Subject: [PATCH] fuse: consistently use readdirplus offsets

With adaptive readdirplus, filesystems may switch between normal
readdir or readdirplus requests.  However, directory offsets
must be maintained consistently to prevent the kernel from
misinterpreting the offsets between different requests which
may use the same buffer.

Signed-off-by: Eric Wong <normalperson@yhbt.net>
---
 fs/fuse/dir.c |    7 +++----
 1 file changed, 3 insertions(+), 4 deletions(-)

diff --git a/fs/fuse/dir.c b/fs/fuse/dir.c
index dc5e648..a8e4abb 100644
--- a/fs/fuse/dir.c
+++ b/fs/fuse/dir.c
@@ -1313,7 +1313,7 @@ static int parse_dirplusfile(char *buf, size_t nbytes, struct file *file,
 
 static int fuse_readdir(struct file *file, void *dstbuf, filldir_t filldir)
 {
-	int plus, err;
+	int err;
 	size_t nbytes;
 	struct page *page;
 	struct inode *inode = file->f_path.dentry->d_inode;
@@ -1334,12 +1334,11 @@ static int fuse_readdir(struct file *file, void *dstbuf, filldir_t filldir)
 		return -ENOMEM;
 	}
 
-	plus = fuse_use_readdirplus(inode, file);
 	req->out.argpages = 1;
 	req->num_pages = 1;
 	req->pages[0] = page;
 	req->page_descs[0].length = PAGE_SIZE;
-	if (plus) {
+	if (fuse_use_readdirplus(inode, file)) {
 		attr_version = fuse_get_attr_version(fc);
 		fuse_read_fill(req, file, file->f_pos, PAGE_SIZE,
 			       FUSE_READDIRPLUS);
@@ -1352,7 +1351,7 @@ static int fuse_readdir(struct file *file, void *dstbuf, filldir_t filldir)
 	err = req->out.h.error;
 	fuse_put_request(fc, req);
 	if (!err) {
-		if (plus) {
+		if (fc->do_readdirplus) {
 			err = parse_dirplusfile(page_address(page), nbytes,
 						file, dstbuf, filldir,
 						attr_version);
-- 
Eric Wong

Re: [RFC] fuse: consistently use readdirplus offsets

[Miklos Szeredi]

On Tue, Feb 5, 2013 at 4:12 AM, Eric Wong <normalperson-rMlxZR9MS24@public.gmane.org> wrote:
> Miklos Szeredi <miklos-sUDqSbJrdHQHWmgEVkV9KA@public.gmane.org> wrote:
>> I think the problem is that directory offsets are not consistent
>> between readdir and readdirplus.  The solution is that userspace
>> should always use the same offsets.  This makes the readdirplus
>> implementation more complex but that's really the only sane thing to
>> do.
>
> I think something like the following is necessary in the kernel, too.
> (Userspace patch coming)

Can you please explain what this patch is trying to do?  Because I
think it's not doing what it wants ;)

Thanks,
Miklos

>
> ------------------------------- 8< --------------------------------
> From ed52619fda61c8f2b723ef72666e7acdd8885a1c Mon Sep 17 00:00:00 2001
> From: Eric Wong <normalperson-rMlxZR9MS24@public.gmane.org>
> Date: Tue, 5 Feb 2013 02:54:28 +0000
> Subject: [PATCH] fuse: consistently use readdirplus offsets
>
> With adaptive readdirplus, filesystems may switch between normal
> readdir or readdirplus requests.  However, directory offsets
> must be maintained consistently to prevent the kernel from
> misinterpreting the offsets between different requests which
> may use the same buffer.
>
> Signed-off-by: Eric Wong <normalperson-rMlxZR9MS24@public.gmane.org>
> ---
>  fs/fuse/dir.c |    7 +++----
>  1 file changed, 3 insertions(+), 4 deletions(-)
>
> diff --git a/fs/fuse/dir.c b/fs/fuse/dir.c
> index dc5e648..a8e4abb 100644
> --- a/fs/fuse/dir.c
> +++ b/fs/fuse/dir.c
> @@ -1313,7 +1313,7 @@ static int parse_dirplusfile(char *buf, size_t nbytes, struct file *file,
>
>  static int fuse_readdir(struct file *file, void *dstbuf, filldir_t filldir)
>  {
> -       int plus, err;
> +       int err;
>         size_t nbytes;
>         struct page *page;
>         struct inode *inode = file->f_path.dentry->d_inode;
> @@ -1334,12 +1334,11 @@ static int fuse_readdir(struct file *file, void *dstbuf, filldir_t filldir)
>                 return -ENOMEM;
>         }
>
> -       plus = fuse_use_readdirplus(inode, file);
>         req->out.argpages = 1;
>         req->num_pages = 1;
>         req->pages[0] = page;
>         req->page_descs[0].length = PAGE_SIZE;
> -       if (plus) {
> +       if (fuse_use_readdirplus(inode, file)) {
>                 attr_version = fuse_get_attr_version(fc);
>                 fuse_read_fill(req, file, file->f_pos, PAGE_SIZE,
>                                FUSE_READDIRPLUS);
> @@ -1352,7 +1351,7 @@ static int fuse_readdir(struct file *file, void *dstbuf, filldir_t filldir)
>         err = req->out.h.error;
>         fuse_put_request(fc, req);
>         if (!err) {
> -               if (plus) {
> +               if (fc->do_readdirplus) {
>                         err = parse_dirplusfile(page_address(page), nbytes,
>                                                 file, dstbuf, filldir,
>                                                 attr_version);
> --
> Eric Wong

------------------------------------------------------------------------------
Free Next-Gen Firewall Hardware Offer
Buy your Sophos next-gen firewall before the end March 2013 
and get the hardware for free! Learn more.
http://p.sf.net/sfu/sophos-d2d-feb

Re: [RFC] fuse: consistently use readdirplus offsets

[Eric Wong]

Miklos Szeredi <miklos@szeredi.hu> wrote:
> On Tue, Feb 5, 2013 at 4:12 AM, Eric Wong <normalperson@yhbt.net> wrote:
> > Miklos Szeredi <miklos@szeredi.hu> wrote:
> >> I think the problem is that directory offsets are not consistent
> >> between readdir and readdirplus.  The solution is that userspace
> >> should always use the same offsets.  This makes the readdirplus
> >> implementation more complex but that's really the only sane thing to
> >> do.
> >
> > I think something like the following is necessary in the kernel, too.
> > (Userspace patch coming)
> 
> Can you please explain what this patch is trying to do?  Because I
> think it's not doing what it wants ;)

I admit I'm not sure why your original email only said:

	userspace should always use the same offsets

instead of:

	kernel and userspace should always use the same offsets

This patch makes the kernel always use the same offsets, as does
v2 of my high-level userspace patch.

Anyways, I've tested this patch together with my v2 userspace patch and
got the results I expected with adaptive readdirplus.

Perhaps I have two bugs nullifying each other?