Messages in this thread |  | | | Subject | Re: [PATCH] Add O_ASYNC support to FUSE | | From | Miklos Szeredi <> | | Date | Tue, 28 Feb 2006 11:15:16 +0100 |
| |
> 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 - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
|  |