linux-fsdevel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] Add O_ASYNC support to FUSE
@ 2006-02-27 18:37 Jeff Dike
  2006-02-28 10:15 ` Miklos Szeredi
  0 siblings, 1 reply; 3+ messages in thread
From: Jeff Dike @ 2006-02-27 18:37 UTC (permalink / raw)
  To: Miklos Szeredi
  Cc: fuse-devel-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f,
	linux-fsdevel-u79uwXL29TY76Z2rM5mHXA,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA

This adds asynchronous notification to FUSE - a FUSE server can
request O_ASYNC on a /dev/fuse file descriptor and receive SIGIO
when there is input available.

One subtlety - fuse_dev_fasync, which is called when O_ASYNC is
requested, does no locking, unlike the other methods.  I think it's
unnecessary, as the fuse_conn.fasync list is manipulated only by
fasync_helper and kill_fasync, which provide their own locking.  It
would also be wrong to use fuse_lock, as it's a spin lock and
fasync_helper can sleep.  My one concern with this is the fuse_conn
going away underneath fuse_dev_fasync - sys_fcntl takes a reference
on the file struct, so this seems not to be a problem.

One possible bug - there is a wakeup in inode.c:fuse_put_super, but
sticking a kill_fasync there causes BUGs because the /dev/fuse
file struct seems to have gone away.  So, this may be lacking when a
FUSE mount is unmounted from underneath the server (is that
possible, or must the server be killed in order to do the umount?).

Signed-off-by: Jeff Dike <jdike-OPE4K8JWMJJBDgjK7y7TUQ@public.gmane.org>

Index: host-2.6.15-fuse/fs/fuse/dev.c
===================================================================
--- host-2.6.15-fuse.orig/fs/fuse/dev.c	2006-02-18 12:19:07.000000000 -0500
+++ host-2.6.15-fuse/fs/fuse/dev.c	2006-02-27 13:19:10.000000000 -0500
@@ -311,6 +311,7 @@ static void queue_request(struct fuse_co
 	list_add_tail(&req->list, &fc->pending);
 	req->state = FUSE_REQ_PENDING;
 	wake_up(&fc->waitq);
+	kill_fasync(&fc->fasync, SIGIO, POLL_IN);
 }
 
 /*
@@ -894,6 +895,7 @@ void fuse_abort_conn(struct fuse_conn *f
 		end_requests(fc, &fc->pending);
 		end_requests(fc, &fc->processing);
 		wake_up_all(&fc->waitq);
+		kill_fasync(&fc->fasync, SIGIO, POLLIN);
 	}
 	spin_unlock(&fuse_lock);
 }
@@ -913,9 +915,20 @@ static int fuse_dev_release(struct inode
 	if (fc)
 		kobject_put(&fc->kobj);
 
+	fasync_helper(-1, file, 0, &fc->fasync);
+
 	return 0;
 }
 
+static int fuse_dev_fasync(int fd, struct file *file, int on)
+{
+	struct fuse_conn *fc;
+
+	/* No locking - fasync_helper does its own locking */
+	fc = file->private_data;
+	return(fasync_helper(fd, file, on, &fc->fasync));
+}
+
 struct file_operations fuse_dev_operations = {
 	.owner		= THIS_MODULE,
 	.llseek		= no_llseek,
@@ -925,6 +938,7 @@ struct file_operations fuse_dev_operatio
 	.writev		= fuse_dev_writev,
 	.poll		= fuse_dev_poll,
 	.release	= fuse_dev_release,
+	.fasync		= fuse_dev_fasync,
 };
 
 static struct miscdevice fuse_miscdevice = {
Index: host-2.6.15-fuse/fs/fuse/fuse_i.h
===================================================================
--- host-2.6.15-fuse.orig/fs/fuse/fuse_i.h	2006-02-24 17:46:47.000000000 -0500
+++ host-2.6.15-fuse/fs/fuse/fuse_i.h	2006-02-24 18:24:49.000000000 -0500
@@ -318,6 +318,9 @@ struct fuse_conn {
 
 	/** kobject */
 	struct kobject kobj;
+
+	/** O_ASYNC requests */
+	struct fasync_struct *fasync;
 };
 
 static inline struct fuse_conn *get_fuse_conn_super(struct super_block *sb)
Index: host-2.6.15-fuse/fs/fuse/inode.c
===================================================================
--- host-2.6.15-fuse.orig/fs/fuse/inode.c	2006-02-18 12:19:07.000000000 -0500
+++ host-2.6.15-fuse/fs/fuse/inode.c	2006-02-27 13:17:50.000000000 -0500
@@ -17,6 +17,7 @@
 #include <linux/module.h>
 #include <linux/parser.h>
 #include <linux/statfs.h>
+#include <linux/poll.h>
 
 MODULE_AUTHOR("Miklos Szeredi <miklos-sUDqSbJrdHQHWmgEVkV9KA@public.gmane.org>");
 MODULE_DESCRIPTION("Filesystem in Userspace");
@@ -377,6 +378,7 @@ static void fuse_conn_release(struct kob
 		list_del(&req->list);
 		fuse_request_free(req);
 	}
+
 	kfree(fc);
 }
 
@@ -409,6 +411,7 @@ static struct fuse_conn *new_conn(void)
 		fc->bdi.ra_pages = (VM_MAX_READAHEAD * 1024) / PAGE_CACHE_SIZE;
 		fc->bdi.unplug_io_fn = default_unplug_io_fn;
 		fc->reqctr = 0;
+		fc->fasync = NULL;
 	}
 	return fc;
 }


-------------------------------------------------------
This SF.Net email is sponsored by xPML, a groundbreaking scripting language
that extends applications into web and mobile media. Attend the live webcast
and join the prime developer group breaking into this new coding territory!
http://sel.as-us.falkag.net/sel?cmd=lnk&kid=110944&bid=241720&dat=121642

^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [PATCH] Add O_ASYNC support to FUSE
  2006-02-27 18:37 [PATCH] Add O_ASYNC support to FUSE Jeff Dike
@ 2006-02-28 10:15 ` Miklos Szeredi
  2006-03-01  2:28   ` Jeff Dike
  0 siblings, 1 reply; 3+ messages in thread
From: Miklos Szeredi @ 2006-02-28 10:15 UTC (permalink / raw)
  To: jdike; +Cc: fuse-devel, linux-fsdevel, linux-kernel

> This adds asynchronous notification to FUSE - a FUSE server can
> request O_ASYNC on a /dev/fuse file descriptor and receive SIGIO
> when there is input available.
> 
> One subtlety - fuse_dev_fasync, which is called when O_ASYNC is
> requested, does no locking, unlike the other methods.  I think it's
> unnecessary, as the fuse_conn.fasync list is manipulated only by
> fasync_helper and kill_fasync, which provide their own locking.

Yes, but you need to use fuse_get_conn() and check the result (see
fuse_dev_poll()).

> It would also be wrong to use fuse_lock, as it's a spin lock and
> fasync_helper can sleep.  My one concern with this is the fuse_conn
> going away underneath fuse_dev_fasync - sys_fcntl takes a reference
> on the file struct, so this seems not to be a problem.

Yes, file holds a reference to fuse_conn.

> One possible bug - there is a wakeup in inode.c:fuse_put_super, but
> sticking a kill_fasync there causes BUGs because the /dev/fuse
> file struct seems to have gone away.

I don't see how that could happen.  fuse_dev_release() removes the
file from fc->fasync, and fasync_helper() and kill_fasync() are
protected with fasync_lock against racing with each other.

Although it may be possible that kill_fasync() races with the final
fput(), but as I see kill_fasync() only uses the file->f_owner field
which should still be valid before and during file->release().

Which BUG is triggered?

> So, this may be lacking when a FUSE mount is unmounted from
> underneath the server (is that possible, or must the server be
> killed in order to do the umount?).

Yes it's possible, it is how a normal unmount happens:

  - umount() called on the filesystem, filesystem unmounted
  - read on /dev/fuse returns -ENODEV
  - userspace filesystem exits gracefuly

> @@ -894,6 +895,7 @@ void fuse_abort_conn(struct fuse_conn *f
>  		end_requests(fc, &fc->pending);
>  		end_requests(fc, &fc->processing);
>  		wake_up_all(&fc->waitq);
> +		kill_fasync(&fc->fasync, SIGIO, POLLIN);

This should be POLL_IN too, no?

> +static int fuse_dev_fasync(int fd, struct file *file, int on)
> +{
> +	struct fuse_conn *fc;
> +
> +	/* No locking - fasync_helper does its own locking */
> +	fc = file->private_data;
> +	return(fasync_helper(fd, file, on, &fc->fasync));

I like this style better:

	return fasync_helper(fd, file, on, &fc->fasync);

Thanks,
Miklos

^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [PATCH] Add O_ASYNC support to FUSE
  2006-02-28 10:15 ` Miklos Szeredi
@ 2006-03-01  2:28   ` Jeff Dike
  0 siblings, 0 replies; 3+ messages in thread
From: Jeff Dike @ 2006-03-01  2:28 UTC (permalink / raw)
  To: Miklos Szeredi; +Cc: fuse-devel, linux-fsdevel, linux-kernel

Updated patch below...

On Tue, Feb 28, 2006 at 11:15:16AM +0100, Miklos Szeredi wrote:
> Yes, but you need to use fuse_get_conn() and check the result (see
> fuse_dev_poll()).

Fixed.

> Although it may be possible that kill_fasync() races with the final
> fput(), but as I see kill_fasync() only uses the file->f_owner field
> which should still be valid before and during file->release().
> 
> Which BUG is triggered?

I cleaned up a couple things, and I can't make it break any more.

> This should be POLL_IN too, no?

Duh, yes.

> I like this style better:

Fixed.

				Jeff

# This adds asynchronous notification to FUSE - a FUSE server can
# request O_ASYNC on a /dev/fuse file descriptor and receive SIGIO
# when there is input available.
#
# One subtlety - fuse_dev_fasync, which is called when O_ASYNC is
# requested, does no locking, unlink the other methods.  I think it's
# unnecessary, as the fuse_conn.fasync list is manipulated only by
# fasync_helper and kill_fasync, which provide their own locking.  It
# would also be wrong to use the fuse_lock, as it's a spin lock and
# fasync_helper can sleep.  My one concern with this is the fuse_conn
# going away underneath fuse_dev_fasync - sys_fcntl takes a reference
# on the file struct, so this seems not to be a problem.
#
# Signed-off-by: Jeff Dike <jdike@addtoit.com>

Index: host-2.6.15-fuse/fs/fuse/dev.c
===================================================================
--- host-2.6.15-fuse.orig/fs/fuse/dev.c	2006-02-18 12:19:07.000000000 -0500
+++ host-2.6.15-fuse/fs/fuse/dev.c	2006-02-28 21:04:40.000000000 -0500
@@ -311,6 +311,7 @@ static void queue_request(struct fuse_co
 	list_add_tail(&req->list, &fc->pending);
 	req->state = FUSE_REQ_PENDING;
 	wake_up(&fc->waitq);
+	kill_fasync(&fc->fasync, SIGIO, POLL_IN);
 }
 
 /*
@@ -894,6 +895,7 @@ void fuse_abort_conn(struct fuse_conn *f
 		end_requests(fc, &fc->pending);
 		end_requests(fc, &fc->processing);
 		wake_up_all(&fc->waitq);
+		kill_fasync(&fc->fasync, SIGIO, POLL_IN);
 	}
 	spin_unlock(&fuse_lock);
 }
@@ -910,12 +912,26 @@ static int fuse_dev_release(struct inode
 		end_requests(fc, &fc->processing);
 	}
 	spin_unlock(&fuse_lock);
-	if (fc)
+	if (fc) {
 		kobject_put(&fc->kobj);
+		fasync_helper(-1, file, 0, &fc->fasync);
+		fc->fasync = NULL;
+	}
 
 	return 0;
 }
 
+static int fuse_dev_fasync(int fd, struct file *file, int on)
+{
+	struct fuse_conn *fc = fuse_get_conn(file);
+
+	if (!fc)
+		return -ENODEV;
+
+	/* No locking - fasync_helper does its own locking */
+	return fasync_helper(fd, file, on, &fc->fasync);
+}
+
 struct file_operations fuse_dev_operations = {
 	.owner		= THIS_MODULE,
 	.llseek		= no_llseek,
@@ -925,6 +941,7 @@ struct file_operations fuse_dev_operatio
 	.writev		= fuse_dev_writev,
 	.poll		= fuse_dev_poll,
 	.release	= fuse_dev_release,
+	.fasync		= fuse_dev_fasync,
 };
 
 static struct miscdevice fuse_miscdevice = {
Index: host-2.6.15-fuse/fs/fuse/fuse_i.h
===================================================================
--- host-2.6.15-fuse.orig/fs/fuse/fuse_i.h	2006-02-24 17:46:47.000000000 -0500
+++ host-2.6.15-fuse/fs/fuse/fuse_i.h	2006-02-24 18:24:49.000000000 -0500
@@ -318,6 +318,9 @@ struct fuse_conn {
 
 	/** kobject */
 	struct kobject kobj;
+
+	/** O_ASYNC requests */
+	struct fasync_struct *fasync;
 };
 
 static inline struct fuse_conn *get_fuse_conn_super(struct super_block *sb)
Index: host-2.6.15-fuse/fs/fuse/inode.c
===================================================================
--- host-2.6.15-fuse.orig/fs/fuse/inode.c	2006-02-18 12:19:07.000000000 -0500
+++ host-2.6.15-fuse/fs/fuse/inode.c	2006-02-28 21:00:55.000000000 -0500
@@ -17,6 +17,7 @@
 #include <linux/module.h>
 #include <linux/parser.h>
 #include <linux/statfs.h>
+#include <linux/poll.h>
 
 MODULE_AUTHOR("Miklos Szeredi <miklos@szeredi.hu>");
 MODULE_DESCRIPTION("Filesystem in Userspace");
@@ -216,6 +217,7 @@ static void fuse_put_super(struct super_
 	spin_unlock(&fuse_lock);
 	up_write(&fc->sbput_sem);
 	/* Flush all readers on this fs */
+	kill_fasync(&fc->fasync, SIGIO, POLL_IN);
 	wake_up_all(&fc->waitq);
 	kobject_del(&fc->kobj);
 	kobject_put(&fc->kobj);
@@ -409,6 +411,7 @@ static struct fuse_conn *new_conn(void)
 		fc->bdi.ra_pages = (VM_MAX_READAHEAD * 1024) / PAGE_CACHE_SIZE;
 		fc->bdi.unplug_io_fn = default_unplug_io_fn;
 		fc->reqctr = 0;
+		fc->fasync = NULL;
 	}
 	return fc;
 }

^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2006-03-01  2:28 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2006-02-27 18:37 [PATCH] Add O_ASYNC support to FUSE Jeff Dike
2006-02-28 10:15 ` Miklos Szeredi
2006-03-01  2:28   ` Jeff Dike

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).