lkml.org 
[lkml]   [2006]   [Mar]   [31]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
Patch in this message
/
Subject[PATCH 5/10] fuse: simplify locking
From
Date
This is in preparation for removing the global spinlock in favor of a
per-mount one.

The only critical part is the interaction between fuse_dev_release()
and fuse_fill_super(): fuse_dev_release() must see the assignment to
file->private_data, otherwise it will leak the reference to fuse_conn.

This is ensured by the fput() operation, which will synchronize the
assignment with other CPU's that may do a final fput() soon after
this.

Also redundant locking is removed from fuse_fill_super(), where
exclusion is already ensured by the BKL held for this function by the
VFS.

Signed-off-by: Miklos Szeredi <miklos@szeredi.hu>

Index: linux/fs/fuse/dev.c
===================================================================
--- linux.orig/fs/fuse/dev.c 2006-03-31 18:55:31.000000000 +0200
+++ linux/fs/fuse/dev.c 2006-03-31 18:55:31.000000000 +0200
@@ -23,13 +23,11 @@ static kmem_cache_t *fuse_req_cachep;

static struct fuse_conn *fuse_get_conn(struct file *file)
{
- struct fuse_conn *fc;
- spin_lock(&fuse_lock);
- fc = file->private_data;
- if (fc && !fc->connected)
- fc = NULL;
- spin_unlock(&fuse_lock);
- return fc;
+ /*
+ * Lockless access is OK, because file->private data is set
+ * once during mount and is valid until the file is released.
+ */
+ return file->private_data;
}

static void fuse_request_init(struct fuse_req *req)
@@ -607,19 +605,16 @@ static ssize_t fuse_dev_readv(struct fil
unsigned long nr_segs, loff_t *off)
{
int err;
- struct fuse_conn *fc;
struct fuse_req *req;
struct fuse_in *in;
struct fuse_copy_state cs;
unsigned reqsize;
+ struct fuse_conn *fc = fuse_get_conn(file);
+ if (!fc)
+ return -EPERM;

restart:
spin_lock(&fuse_lock);
- fc = file->private_data;
- err = -EPERM;
- if (!fc)
- goto err_unlock;
-
err = -EAGAIN;
if((file->f_flags & O_NONBLOCK) && fc->connected &&
list_empty(&fc->pending))
@@ -915,17 +910,13 @@ void fuse_abort_conn(struct fuse_conn *f

static int fuse_dev_release(struct inode *inode, struct file *file)
{
- struct fuse_conn *fc;
-
- spin_lock(&fuse_lock);
- fc = file->private_data;
+ struct fuse_conn *fc = fuse_get_conn(file);
if (fc) {
+ spin_lock(&fuse_lock);
fc->connected = 0;
end_requests(fc, &fc->pending);
end_requests(fc, &fc->processing);
- }
- spin_unlock(&fuse_lock);
- if (fc) {
+ spin_unlock(&fuse_lock);
fasync_helper(-1, file, 0, &fc->fasync);
kobject_put(&fc->kobj);
}
Index: linux/fs/fuse/inode.c
===================================================================
--- linux.orig/fs/fuse/inode.c 2006-03-31 18:55:31.000000000 +0200
+++ linux/fs/fuse/inode.c 2006-03-31 18:55:31.000000000 +0200
@@ -414,37 +414,6 @@ static struct fuse_conn *new_conn(void)
return fc;
}

-static struct fuse_conn *get_conn(struct file *file, struct super_block *sb)
-{
- struct fuse_conn *fc;
- int err;
-
- err = -EINVAL;
- if (file->f_op != &fuse_dev_operations)
- goto out_err;
-
- err = -ENOMEM;
- fc = new_conn();
- if (!fc)
- goto out_err;
-
- spin_lock(&fuse_lock);
- err = -EINVAL;
- if (file->private_data)
- goto out_unlock;
-
- kobject_get(&fc->kobj);
- file->private_data = fc;
- spin_unlock(&fuse_lock);
- return fc;
-
- out_unlock:
- spin_unlock(&fuse_lock);
- kobject_put(&fc->kobj);
- out_err:
- return ERR_PTR(err);
-}
-
static struct inode *get_root_inode(struct super_block *sb, unsigned mode)
{
struct fuse_attr attr;
@@ -526,12 +495,9 @@ static void fuse_send_init(struct fuse_c

static unsigned long long conn_id(void)
{
+ /* BKL is held for ->get_sb() */
static unsigned long long ctr = 1;
- unsigned long long val;
- spin_lock(&fuse_lock);
- val = ctr++;
- spin_unlock(&fuse_lock);
- return val;
+ return ctr++;
}

static int fuse_fill_super(struct super_block *sb, void *data, int silent)
@@ -556,10 +522,17 @@ static int fuse_fill_super(struct super_
if (!file)
return -EINVAL;

- fc = get_conn(file, sb);
- fput(file);
- if (IS_ERR(fc))
- return PTR_ERR(fc);
+ if (file->f_op != &fuse_dev_operations)
+ return -EINVAL;
+
+ /* Setting file->private_data can't race with other mount()
+ instances, since BKL is held for ->get_sb() */
+ if (file->private_data)
+ return -EINVAL;
+
+ fc = new_conn();
+ if (!fc)
+ return -ENOMEM;

fc->flags = d.flags;
fc->user_id = d.user_id;
@@ -589,10 +562,16 @@ static int fuse_fill_super(struct super_
goto err_put_root;

sb->s_root = root_dentry;
- spin_lock(&fuse_lock);
fc->mounted = 1;
fc->connected = 1;
- spin_unlock(&fuse_lock);
+ kobject_get(&fc->kobj);
+ file->private_data = fc;
+ /*
+ * atomic_dec_and_test() in fput() provides the necessary
+ * memory barrier for file->private_data to be visible on all
+ * CPUs after this
+ */
+ fput(file);

fuse_send_init(fc);

@@ -601,6 +580,7 @@ static int fuse_fill_super(struct super_
err_put_root:
dput(root_dentry);
err:
+ fput(file);
kobject_put(&fc->kobj);
return err;
}
-
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/
\
 
 \ /
  Last update: 2006-03-31 19:57    [W:0.063 / U:1.452 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site