Jeff Layton wrote: > Peter Staubach wrote: > > > > If iunique_register() fails, does not this create a memory leak > > because root will need to get iput()'d? > > > > Good point, and now that we have a wrapper for new_inode that handles this > error transparently, both places are easy to fix. This patch should do it: I really need to proofread better before sending. That patch would crash if new_inode returned NULL. This should be correct: Signed-off-by: Jeff Layton <jlayton@xxxxxxxxxx> --- linux-2.6/fs/debugfs/inode.c.super +++ linux-2.6/fs/debugfs/inode.c @@ -107,7 +107,7 @@ static int debug_fill_super(struct super { static struct tree_descr debug_files[] = {{""}}; - return simple_fill_super(sb, DEBUGFS_MAGIC, debug_files); + return registered_fill_super(sb, DEBUGFS_MAGIC, debug_files); } static int debug_get_sb(struct file_system_type *fs_type, --- linux-2.6/fs/fuse/control.c.super +++ linux-2.6/fs/fuse/control.c @@ -163,7 +163,7 @@ static int fuse_ctl_fill_super(struct su struct fuse_conn *fc; int err; - err = simple_fill_super(sb, FUSE_CTL_SUPER_MAGIC, &empty_descr); + err = registered_fill_super(sb, FUSE_CTL_SUPER_MAGIC, &empty_descr); if (err) return err; --- linux-2.6/fs/libfs.c.super +++ linux-2.6/fs/libfs.c @@ -214,7 +214,7 @@ int get_sb_pseudo(struct file_system_typ s->s_magic = magic; s->s_op = ops ? ops : &default_ops; s->s_time_gran = 1; - root = new_inode(s); + root = new_registered_inode(s, 0); if (!root) goto Enomem; root->i_mode = S_IFDIR | S_IRUSR | S_IWUSR; @@ -356,7 +356,8 @@ int simple_commit_write(struct file *fil return 0; } -int simple_fill_super(struct super_block *s, int magic, struct tree_descr *files) +int __simple_fill_super(struct super_block *s, int magic, + struct tree_descr *files, bool registered) { static struct super_operations s_ops = {.statfs = simple_statfs}; struct inode *inode; @@ -380,6 +381,12 @@ int simple_fill_super(struct super_block inode->i_op = &simple_dir_inode_operations; inode->i_fop = &simple_dir_operations; inode->i_nlink = 2; + /* + * set this as high as a 32 bit val as possible to avoid collisions. + * This is also well above the highest value that iunique_register + * will assign to an inode + */ + inode->i_ino = 0xffffffff; root = d_alloc_root(inode); if (!root) { iput(inode); @@ -391,15 +398,21 @@ int simple_fill_super(struct super_block dentry = d_alloc_name(root, files->name); if (!dentry) goto out; - inode = new_inode(s); + if (registered) + inode = new_registered_inode(s, 0); + else + inode = new_inode(s); + if (!inode) goto out; + + if (!registered) + inode->i_ino = i; inode->i_mode = S_IFREG | files->mode; inode->i_uid = inode->i_gid = 0; inode->i_blocks = 0; inode->i_atime = inode->i_mtime = inode->i_ctime = CURRENT_TIME; inode->i_fop = files->ops; - inode->i_ino = i; d_add(dentry, inode); } s->s_root = root; @@ -618,7 +631,7 @@ EXPORT_SYMBOL(simple_dir_inode_operation EXPORT_SYMBOL(simple_dir_operations); EXPORT_SYMBOL(simple_empty); EXPORT_SYMBOL(d_alloc_name); -EXPORT_SYMBOL(simple_fill_super); +EXPORT_SYMBOL(__simple_fill_super); EXPORT_SYMBOL(simple_getattr); EXPORT_SYMBOL(simple_link); EXPORT_SYMBOL(simple_lookup); --- linux-2.6/include/linux/fs.h.super +++ linux-2.6/include/linux/fs.h @@ -1879,10 +1879,35 @@ extern const struct file_operations simp extern struct inode_operations simple_dir_inode_operations; struct tree_descr { char *name; const struct file_operations *ops; int mode; }; struct dentry *d_alloc_name(struct dentry *, const char *); -extern int simple_fill_super(struct super_block *, int, struct tree_descr *); +extern int __simple_fill_super(struct super_block *s, int magic, + struct tree_descr *files, bool registered); extern int simple_pin_fs(struct file_system_type *, struct vfsmount **mount, int *count); extern void simple_release_fs(struct vfsmount **mount, int *count); +/* + * Fill a superblock with a standard set of fields, and add the entries in the + * "files" struct. Assign i_ino values to the files sequentially. This function + * is appropriate for filesystems that need a particular i_ino value assigned + * to a particular "files" entry. + */ +static inline int simple_fill_super(struct super_block *s, int magic, + struct tree_descr *files) +{ + return __simple_fill_super(s, magic, files, false); +} + +/* + * Just like simple_fill_super, but does an iunique_register on the inodes + * created for "files" entries. This function is appropriate when you don't + * need a particular i_ino value assigned to each files entry, and when the + * filesystem will have other registered inodes. + */ +static inline int registered_fill_super(struct super_block *s, int magic, + struct tree_descr *files) +{ + return __simple_fill_super(s, magic, files, true); +} + extern ssize_t simple_read_from_buffer(void __user *, size_t, loff_t *, const void *, size_t); #ifdef CONFIG_MIGRATION --- linux-2.6/security/inode.c.super +++ linux-2.6/security/inode.c @@ -130,7 +130,7 @@ static int fill_super(struct super_block { static struct tree_descr files[] = {{""}}; - return simple_fill_super(sb, SECURITYFS_MAGIC, files); + return registered_fill_super(sb, SECURITYFS_MAGIC, files); } static int get_sb(struct file_system_type *fs_type, - 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