On Mon, 12 Jun 2006 14:28:32 +0200 Miklos Szeredi <miklos@xxxxxxxxxx> wrote: > Add a control filesystem to fuse, replacing the attributes currently > exported through sysfs. An empty directory '/sys/fs/fuse/connections' > is still created in sysfs, and mounting the control filesystem here > provides backward compatibility. > > Advantages of the control filesystem over the previous solution: > > - allows the object directory and the attributes to be owned by the > filesystem owner, hence letting unpriviled users abort the > filesystem connection > > - does not suffer from module unload race > Presumably people with currently-working setups will find that whatever they used to have in /sys/fs/fuse/connections won't be there any more, so this is a non-back-compatible change. How do we help them with that? > +static ssize_t fuse_conn_waiting_read(struct file *file, char __user *buf, > + size_t len, loff_t *ppos) > +{ > + char tmp[32]; > + size_t size; > + > + if (!*ppos) { > + struct fuse_conn *fc = fuse_ctl_file_conn_get(file); > + if (!fc) > + return 0; > + > + file->private_data = (void *) atomic_read(&fc->num_waiting); > + fuse_conn_put(fc); > + } > + size = sprintf(tmp, "%i\n", (int) file->private_data); > + return simple_read_from_buffer(buf, len, ppos, tmp, size); > +} What happens if the first read isn't at file offset 0? > + > +static struct dentry *fuse_ctl_add_dentry(struct dentry *parent, > + struct fuse_conn *fc, > + const char *name, > + int mode, int nlink, > + struct inode_operations *iop, > + const struct file_operations *fop) > +{ > + struct dentry *dentry; > + struct inode *inode; > + > + BUG_ON(fc->ctl_ndents >= FUSE_CTL_NUM_DENTRIES); > + dentry = d_alloc_name(parent, name); > + if (!dentry) > + return NULL; > + > + fc->ctl_dentry[fc->ctl_ndents++] = dentry; What locking protects fc->ctl_ndents? > + inode = new_inode(fuse_control_sb); > + if (!inode) > + return NULL; > + > + inode->i_mode = mode; > + inode->i_uid = fc->user_id; > + inode->i_gid = fc->group_id; > + inode->i_atime = inode->i_mtime = inode->i_ctime = CURRENT_TIME; > + if (iop) > + inode->i_op = iop; Is iop ever null? > + inode->i_fop = fop; > + inode->i_nlink = nlink; > + inode->u.generic_ip = fc; > + d_add(dentry, inode); > + return dentry; > +} > + > +int fuse_ctl_add_conn(struct fuse_conn *fc) > +{ > + struct dentry *parent; > + char name[32]; > + > + if (!fuse_control_sb) > + return 0; Can this happen? > + parent = fuse_control_sb->s_root; > + parent->d_inode->i_nlink++; What locking protects i_nlink? > + sprintf(name, "%llu", (unsigned long long) fc->id); > + parent = fuse_ctl_add_dentry(parent, fc, name, S_IFDIR | 0500, 2, > + &simple_dir_inode_operations, > + &simple_dir_operations); > + if (!parent) > + goto err; > + > + if (!fuse_ctl_add_dentry(parent, fc, "waiting", S_IFREG | 0400, 1, > + NULL, &fuse_ctl_waiting_ops) || > + !fuse_ctl_add_dentry(parent, fc, "abort", S_IFREG | 0200, 1, > + NULL, &fuse_ctl_abort_ops)) > + goto err; > + > + return 0; > + > + err: > + fuse_ctl_remove_conn(fc); > + return -ENOMEM; > +} > + > +void fuse_ctl_remove_conn(struct fuse_conn *fc) > +{ > + int i; > + > + if (!fuse_control_sb) > + return; > + > + for (i = fc->ctl_ndents - 1; i >= 0; i--) { > + struct dentry *dentry = fc->ctl_dentry[i]; > + dentry->d_inode->u.generic_ip = NULL; > + d_drop(dentry); > + dput(dentry); > + } > + fuse_control_sb->s_root->d_inode->i_nlink--; Ditto. - To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html