[patch 0/5] fuse updates

[patch 0/5] fuse updates

[Miklos Szeredi]

These are fuse fixes for 2.6.26.  Please apply.

Thanks,
Miklos

--

[patch 1/5] fuse: fix max i/o size calculation

[Miklos Szeredi]

[-- Attachment #1: fuse_max_write_fix.patch --]
[-- Type: text/plain, Size: 2109 bytes --]

From: Miklos Szeredi <mszeredi@suse.cz>

Fix a bug that Werner Baumann reported: fuse can send a bigger write
request than the maximum specified.  This only affected direct_io
operation.

In addition set a sane minimum for the max_read and max_write
tunables, so I/O always makes some progress.

Signed-off-by: Miklos Szeredi <mszeredi@suse.cz>
---
 fs/fuse/file.c  |    7 ++++---
 fs/fuse/inode.c |    3 ++-
 2 files changed, 6 insertions(+), 4 deletions(-)

Index: linux/fs/fuse/file.c
===================================================================
--- linux.orig/fs/fuse/file.c	2008-04-25 18:48:08.000000000 +0200
+++ linux/fs/fuse/file.c	2008-04-25 19:38:05.000000000 +0200
@@ -960,14 +960,15 @@ static ssize_t fuse_direct_io(struct fil
 
 	while (count) {
 		size_t nres;
-		size_t nbytes = min(count, nmax);
-		int err = fuse_get_user_pages(req, buf, nbytes, !write);
+		size_t nbytes_limit = min(count, nmax);
+		size_t nbytes;
+		int err = fuse_get_user_pages(req, buf, nbytes_limit, !write);
 		if (err) {
 			res = err;
 			break;
 		}
 		nbytes = (req->num_pages << PAGE_SHIFT) - req->page_offset;
-		nbytes = min(count, nbytes);
+		nbytes = min(nbytes_limit, nbytes);
 		if (write)
 			nres = fuse_send_write(req, file, inode, pos, nbytes,
 					       current->files);
Index: linux/fs/fuse/inode.c
===================================================================
--- linux.orig/fs/fuse/inode.c	2008-04-25 19:07:27.000000000 +0200
+++ linux/fs/fuse/inode.c	2008-04-25 19:38:05.000000000 +0200
@@ -585,6 +585,7 @@ static void process_init_reply(struct fu
 		fc->bdi.ra_pages = min(fc->bdi.ra_pages, ra_pages);
 		fc->minor = arg->minor;
 		fc->max_write = arg->minor < 5 ? 4096 : arg->max_write;
+		fc->max_write = min_t(unsigned, 4096, fc->max_write);
 		fc->conn_init = 1;
 	}
 	fuse_put_request(fc, req);
@@ -659,7 +660,7 @@ static int fuse_fill_super(struct super_
 	fc->flags = d.flags;
 	fc->user_id = d.user_id;
 	fc->group_id = d.group_id;
-	fc->max_read = d.max_read;
+	fc->max_read = min_t(unsigned, 4096, d.max_read);
 
 	/* Used by get_root_inode() */
 	sb->s_fs_info = fc;

--

[patch 2/5] fuse: fix node ID type

[Miklos Szeredi]

[-- Attachment #1: fuse_64bit_nodeid.patch --]
[-- Type: text/plain, Size: 2470 bytes --]

From: Miklos Szeredi <mszeredi@suse.cz>

Node ID is 64bit but it is passed as unsigned long to some functions.
This breakage wasn't noticed, because libfuse uses unsigned long too.

Signed-off-by: Miklos Szeredi <mszeredi@suse.cz>
---
 fs/fuse/fuse_i.h |    4 ++--
 fs/fuse/inode.c  |    8 ++++----
 2 files changed, 6 insertions(+), 6 deletions(-)

Index: linux/fs/fuse/fuse_i.h
===================================================================
--- linux.orig/fs/fuse/fuse_i.h	2008-04-09 13:17:45.000000000 +0200
+++ linux/fs/fuse/fuse_i.h	2008-04-09 13:17:59.000000000 +0200
@@ -461,7 +461,7 @@ extern const struct file_operations fuse
 /**
  * Get a filled in inode
  */
-struct inode *fuse_iget(struct super_block *sb, unsigned long nodeid,
+struct inode *fuse_iget(struct super_block *sb, u64 nodeid,
 			int generation, struct fuse_attr *attr,
 			u64 attr_valid, u64 attr_version);
 
@@ -469,7 +469,7 @@ struct inode *fuse_iget(struct super_blo
  * Send FORGET command
  */
 void fuse_send_forget(struct fuse_conn *fc, struct fuse_req *req,
-		      unsigned long nodeid, u64 nlookup);
+		      u64 nodeid, u64 nlookup);
 
 /**
  * Initialize READ or READDIR request
Index: linux/fs/fuse/inode.c
===================================================================
--- linux.orig/fs/fuse/inode.c	2008-04-09 13:17:45.000000000 +0200
+++ linux/fs/fuse/inode.c	2008-04-09 13:17:59.000000000 +0200
@@ -84,7 +84,7 @@ static void fuse_destroy_inode(struct in
 }
 
 void fuse_send_forget(struct fuse_conn *fc, struct fuse_req *req,
-		      unsigned long nodeid, u64 nlookup)
+		      u64 nodeid, u64 nlookup)
 {
 	struct fuse_forget_in *inarg = &req->misc.forget_in;
 	inarg->nlookup = nlookup;
@@ -207,7 +207,7 @@ static void fuse_init_inode(struct inode
 
 static int fuse_inode_eq(struct inode *inode, void *_nodeidp)
 {
-	unsigned long nodeid = *(unsigned long *) _nodeidp;
+	u64 nodeid = *(u64 *) _nodeidp;
 	if (get_node_id(inode) == nodeid)
 		return 1;
 	else
@@ -216,12 +216,12 @@ static int fuse_inode_eq(struct inode *i
 
 static int fuse_inode_set(struct inode *inode, void *_nodeidp)
 {
-	unsigned long nodeid = *(unsigned long *) _nodeidp;
+	u64 nodeid = *(u64 *) _nodeidp;
 	get_fuse_inode(inode)->nodeid = nodeid;
 	return 0;
 }
 
-struct inode *fuse_iget(struct super_block *sb, unsigned long nodeid,
+struct inode *fuse_iget(struct super_block *sb, u64 nodeid,
 			int generation, struct fuse_attr *attr,
 			u64 attr_valid, u64 attr_version)
 {

--

[patch 3/5] fuse: fix race in llseek

[Miklos Szeredi]

[-- Attachment #1: fuse_llseek.patch --]
[-- Type: text/plain, Size: 1781 bytes --]

From: Miklos Szeredi <mszeredi@suse.cz>

Fuse doesn't use i_mutex to protect setting i_size, and so
generic_file_llseek() can be racy: it doesn't use i_size_read().

So do a fuse specific llseek method, which does use i_size_read().

Signed-off-by: Miklos Szeredi <mszeredi@suse.cz>
---
 fs/fuse/file.c |   29 +++++++++++++++++++++++++++--
 1 file changed, 27 insertions(+), 2 deletions(-)

Index: linux/fs/fuse/file.c
===================================================================
--- linux.orig/fs/fuse/file.c	2008-04-25 19:38:10.000000000 +0200
+++ linux/fs/fuse/file.c	2008-04-25 19:40:11.000000000 +0200
@@ -1425,8 +1425,33 @@ static sector_t fuse_bmap(struct address
 	return err ? 0 : outarg.block;
 }
 
+static loff_t fuse_file_llseek(struct file *file, loff_t offset, int origin)
+{
+	long long retval;
+	struct inode *inode = file->f_path.dentry->d_inode;
+
+	mutex_lock(&inode->i_mutex);
+	switch (origin) {
+	case SEEK_END:
+		offset += i_size_read(inode);
+		break;
+	case SEEK_CUR:
+		offset += file->f_pos;
+	}
+	retval = -EINVAL;
+	if (offset >= 0 && offset <= inode->i_sb->s_maxbytes) {
+		if (offset != file->f_pos) {
+			file->f_pos = offset;
+			file->f_version = 0;
+		}
+		retval = offset;
+	}
+	mutex_unlock(&inode->i_mutex);
+	return retval;
+}
+
 static const struct file_operations fuse_file_operations = {
-	.llseek		= generic_file_llseek,
+	.llseek		= fuse_file_llseek,
 	.read		= do_sync_read,
 	.aio_read	= fuse_file_aio_read,
 	.write		= do_sync_write,
@@ -1442,7 +1467,7 @@ static const struct file_operations fuse
 };
 
 static const struct file_operations fuse_direct_io_file_operations = {
-	.llseek		= generic_file_llseek,
+	.llseek		= fuse_file_llseek,
 	.read		= fuse_direct_read,
 	.write		= fuse_direct_write,
 	.open		= fuse_open,

--

Re: [patch 3/5] fuse: fix race in llseek

[Andrew Morton]

On Fri, 25 Apr 2008 19:55:23 +0200 Miklos Szeredi <miklos@szeredi.hu> wrote:

> +static loff_t fuse_file_llseek(struct file *file, loff_t offset, int origin)
> +{
> +	long long retval;

I switched this to have a type of loff_t.

> +	struct inode *inode = file->f_path.dentry->d_inode;
> +
> +	mutex_lock(&inode->i_mutex);
> +	switch (origin) {
> +	case SEEK_END:
> +		offset += i_size_read(inode);

As we hold i_mutex we could directly read inode->i_size here, save a few
cycles.

> +		break;
> +	case SEEK_CUR:
> +		offset += file->f_pos;
> +	}
> +	retval = -EINVAL;
> +	if (offset >= 0 && offset <= inode->i_sb->s_maxbytes) {
> +		if (offset != file->f_pos) {
> +			file->f_pos = offset;
> +			file->f_version = 0;
> +		}
> +		retval = offset;
> +	}
> +	mutex_unlock(&inode->i_mutex);
> +	return retval;
> +}

Re: [patch 3/5] fuse: fix race in llseek

[Miklos Szeredi]

> > +static loff_t fuse_file_llseek(struct file *file, loff_t offset, int origin)
> > +{
> > +	long long retval;
> 
> I switched this to have a type of loff_t.

Ugh, right.  I wonder where that long long came from.  I probably
copied the generic_file_llseek() from some earlier kernel...

> 
> > +	struct inode *inode = file->f_path.dentry->d_inode;
> > +
> > +	mutex_lock(&inode->i_mutex);
> > +	switch (origin) {
> > +	case SEEK_END:
> > +		offset += i_size_read(inode);
> 
> As we hold i_mutex we could directly read inode->i_size here, save a few
> cycles.

No, that's the whole point of the fuse specific function: fuse doesn't
use i_mutex to protect writing i_size, instead it uses a per
filesystem spinlock.

This is probably worth a comment though, so I'll do that.

Thanks,
Miklos

Re: [patch 3/5] fuse: fix race in llseek

[Jan Blunck]

On Fri, 25 Apr 2008 19:55:23 +0200, Miklos Szeredi wrote:

> From: Miklos Szeredi <mszeredi@suse.cz>
> 
> Fuse doesn't use i_mutex to protect setting i_size, and so
> generic_file_llseek() can be racy: it doesn't use i_size_read().
> 
> So do a fuse specific llseek method, which does use i_size_read().
> 

Is there any specific reason why we don't use i_size_read() in 
generic_file_llseek()? If no, why don't you fix it instead?

Regards,
	Jan

-- 
Jan Blunck <jblunck@suse.de>

Re: [patch 3/5] fuse: fix race in llseek

[Miklos Szeredi]

> > From: Miklos Szeredi <mszeredi@suse.cz>
> > 
> > Fuse doesn't use i_mutex to protect setting i_size, and so
> > generic_file_llseek() can be racy: it doesn't use i_size_read().
> > 
> > So do a fuse specific llseek method, which does use i_size_read().
> > 
> 
> Is there any specific reason why we don't use i_size_read() in 
> generic_file_llseek()?

Just to save some cycles on 32bit archs.  Considering that
lseek(SEEK_END) isn't a very performance sensitive operation...

> If no, why don't you fix it instead?

...we could do that.  Does anybody object?

Miklos
----


Some filesystems (e.g. fuse) don't use i_mutex to protect setting
i_size, and so generic_file_llseek() can be racy.

Since lseek(SEEK_END) isn't a very performance sensitive operation, do
this in generic_file_llseek(), instead of requiring such filesystems
to duplicate this function.

Signed-off-by: Miklos Szeredi <mszeredi@suse.cz>
---
 fs/read_write.c |    6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

Index: linux/fs/read_write.c
===================================================================
--- linux.orig/fs/read_write.c	2008-04-17 04:49:44.000000000 +0200
+++ linux/fs/read_write.c	2008-04-28 17:08:25.000000000 +0200
@@ -39,7 +39,11 @@ loff_t generic_file_llseek(struct file *
 	mutex_lock(&inode->i_mutex);
 	switch (origin) {
 		case SEEK_END:
-			offset += inode->i_size;
+			/*
+			 * Some filesystems use a different lock as i_mutex
+			 * to protect writing i_size
+			 */
+			offset += i_size_read(inode);
 			break;
 		case SEEK_CUR:
 			offset += file->f_pos;

[patch 4/5] fuse: fix truncation on short read

[Miklos Szeredi]

[-- Attachment #1: fuse_truncate_file_on_short_read_fix.patch --]
[-- Type: text/plain, Size: 4925 bytes --]

From: Miklos Szeredi <mszeredi@suse.cz>

fuse-update-file-size-on-short-read.patch introduced a bug, where a
read could truncate off the part recently extended by a parallel
write.

Fix by using the attribute versioning already used by getattr().

Signed-off-by: Miklos Szeredi <mszeredi@suse.cz>
---
 fs/fuse/dir.c    |    2 +-
 fs/fuse/file.c   |   22 ++++++++++++++--------
 fs/fuse/fuse_i.h |    7 ++++++-
 3 files changed, 21 insertions(+), 10 deletions(-)

Index: linux/fs/fuse/dir.c
===================================================================
--- linux.orig/fs/fuse/dir.c	2008-04-25 17:10:02.000000000 +0200
+++ linux/fs/fuse/dir.c	2008-04-25 17:11:02.000000000 +0200
@@ -132,7 +132,7 @@ static void fuse_lookup_init(struct fuse
 	req->out.args[0].value = outarg;
 }
 
-static u64 fuse_get_attr_version(struct fuse_conn *fc)
+u64 fuse_get_attr_version(struct fuse_conn *fc)
 {
 	u64 curr_version;
 
Index: linux/fs/fuse/file.c
===================================================================
--- linux.orig/fs/fuse/file.c	2008-04-25 17:10:47.000000000 +0200
+++ linux/fs/fuse/file.c	2008-04-25 17:11:02.000000000 +0200
@@ -363,7 +363,7 @@ static int fuse_fsync(struct file *file,
 void fuse_read_fill(struct fuse_req *req, struct file *file,
 		    struct inode *inode, loff_t pos, size_t count, int opcode)
 {
-	struct fuse_read_in *inarg = &req->misc.read_in;
+	struct fuse_read_in *inarg = &req->misc.read.in;
 	struct fuse_file *ff = file->private_data;
 
 	inarg->fh = ff->fh;
@@ -389,7 +389,7 @@ static size_t fuse_send_read(struct fuse
 
 	fuse_read_fill(req, file, inode, pos, count, FUSE_READ);
 	if (owner != NULL) {
-		struct fuse_read_in *inarg = &req->misc.read_in;
+		struct fuse_read_in *inarg = &req->misc.read.in;
 
 		inarg->read_flags |= FUSE_READ_LOCKOWNER;
 		inarg->lock_owner = fuse_lock_owner_id(fc, owner);
@@ -398,15 +398,17 @@ static size_t fuse_send_read(struct fuse
 	return req->out.args[0].size;
 }
 
-static void fuse_read_update_size(struct inode *inode, loff_t size)
+static void fuse_read_update_size(struct inode *inode, loff_t size,
+				  u64 attr_ver)
 {
 	struct fuse_conn *fc = get_fuse_conn(inode);
 	struct fuse_inode *fi = get_fuse_inode(inode);
 
 	spin_lock(&fc->lock);
-	fi->attr_version = ++fc->attr_version;
-	if (size < inode->i_size)
+	if (attr_ver == fi->attr_version && size < inode->i_size) {
+		fi->attr_version = ++fc->attr_version;
 		i_size_write(inode, size);
+	}
 	spin_unlock(&fc->lock);
 }
 
@@ -418,6 +420,7 @@ static int fuse_readpage(struct file *fi
 	size_t num_read;
 	loff_t pos = page_offset(page);
 	size_t count = PAGE_CACHE_SIZE;
+	u64 attr_ver;
 	int err;
 
 	err = -EIO;
@@ -436,6 +439,8 @@ static int fuse_readpage(struct file *fi
 	if (IS_ERR(req))
 		goto out;
 
+	attr_ver = fuse_get_attr_version(fc);
+
 	req->out.page_zeroing = 1;
 	req->num_pages = 1;
 	req->pages[0] = page;
@@ -448,7 +453,7 @@ static int fuse_readpage(struct file *fi
 		 * Short read means EOF.  If file size is larger, truncate it
 		 */
 		if (num_read < count)
-			fuse_read_update_size(inode, pos + num_read);
+			fuse_read_update_size(inode, pos + num_read, attr_ver);
 
 		SetPageUptodate(page);
 	}
@@ -462,7 +467,7 @@ static int fuse_readpage(struct file *fi
 static void fuse_readpages_end(struct fuse_conn *fc, struct fuse_req *req)
 {
 	int i;
-	size_t count = req->misc.read_in.size;
+	size_t count = req->misc.read.in.size;
 	size_t num_read = req->out.args[0].size;
 	struct inode *inode = req->pages[0]->mapping->host;
 
@@ -471,7 +476,7 @@ static void fuse_readpages_end(struct fu
 	 */
 	if (!req->out.h.error && num_read < count) {
 		loff_t pos = page_offset(req->pages[0]) + num_read;
-		fuse_read_update_size(inode, pos);
+		fuse_read_update_size(inode, pos, req->misc.read.attr_ver);
 	}
 
 	fuse_invalidate_attr(inode); /* atime changed */
@@ -497,6 +502,7 @@ static void fuse_send_readpages(struct f
 	size_t count = req->num_pages << PAGE_CACHE_SHIFT;
 	req->out.page_zeroing = 1;
 	fuse_read_fill(req, file, inode, pos, count, FUSE_READ);
+	req->misc.read.attr_ver = fuse_get_attr_version(fc);
 	if (fc->async_read) {
 		struct fuse_file *ff = file->private_data;
 		req->ff = fuse_file_get(ff);
Index: linux/fs/fuse/fuse_i.h
===================================================================
--- linux.orig/fs/fuse/fuse_i.h	2008-04-25 17:09:29.000000000 +0200
+++ linux/fs/fuse/fuse_i.h	2008-04-25 17:11:02.000000000 +0200
@@ -239,7 +239,10 @@ struct fuse_req {
 		} release;
 		struct fuse_init_in init_in;
 		struct fuse_init_out init_out;
-		struct fuse_read_in read_in;
+		struct {
+			struct fuse_read_in in;
+			u64 attr_ver;
+		} read;
 		struct {
 			struct fuse_write_in in;
 			struct fuse_write_out out;
@@ -637,3 +640,5 @@ void fuse_flush_writepages(struct inode 
 
 void fuse_set_nowrite(struct inode *inode);
 void fuse_release_nowrite(struct inode *inode);
+
+u64 fuse_get_attr_version(struct fuse_conn *fc);

--

[patch 5/5] fuse: fix sparse warnings

[Miklos Szeredi]

[-- Attachment #1: fuse_sparse_fixes.patch --]
[-- Type: text/plain, Size: 1528 bytes --]

From: Miklos Szeredi <mszeredi@suse.cz>

fs/fuse/dev.c:306:2: warning: context imbalance in 'wait_answer_interruptible' - unexpected unlock
fs/fuse/dev.c:361:2: warning: context imbalance in 'request_wait_answer' - unexpected unlock
fs/fuse/dev.c:1002:4: warning: context imbalance in 'end_io_requests' - unexpected unlock

Signed-off-by: Miklos Szeredi <mszeredi@suse.cz>
---
 fs/fuse/dev.c |    4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

Index: linux/fs/fuse/dev.c
===================================================================
--- linux.orig/fs/fuse/dev.c	2008-04-25 18:48:08.000000000 +0200
+++ linux/fs/fuse/dev.c	2008-04-25 18:48:28.000000000 +0200
@@ -299,6 +299,7 @@ static void request_end(struct fuse_conn
 
 static void wait_answer_interruptible(struct fuse_conn *fc,
 				      struct fuse_req *req)
+	__releases(fc->lock) __acquires(fc->lock)
 {
 	if (signal_pending(current))
 		return;
@@ -315,8 +316,8 @@ static void queue_interrupt(struct fuse_
 	kill_fasync(&fc->fasync, SIGIO, POLL_IN);
 }
 
-/* Called with fc->lock held.  Releases, and then reacquires it. */
 static void request_wait_answer(struct fuse_conn *fc, struct fuse_req *req)
+	__releases(fc->lock) __acquires(fc->lock)
 {
 	if (!fc->no_interrupt) {
 		/* Any signal may interrupt this */
@@ -987,6 +988,7 @@ static void end_requests(struct fuse_con
  * locked).
  */
 static void end_io_requests(struct fuse_conn *fc)
+	__releases(fc->lock) __acquires(fc->lock)
 {
 	while (!list_empty(&fc->io)) {
 		struct fuse_req *req =

--

Re: [patch 0/5] fuse updates

[Andrew Morton]

On Fri, 25 Apr 2008 19:55:20 +0200 Miklos Szeredi <miklos@szeredi.hu> wrote:

> These are fuse fixes for 2.6.26.  Please apply.
> 

Are none of them needed in 2.6.25.x?

Re: [patch 0/5] fuse updates

[Miklos Szeredi]

> > These are fuse fixes for 2.6.26.  Please apply.
> > 
> 
> Are none of them needed in 2.6.25.x?

Not really.  All of them are minor issues, and some of them fix
regressions by the fuse patches already in -mm.

Thanks,
Miklos