Messages in this thread | | | Subject | Re: [PATCH 2/11] FUSE - core | From | Miklos Szeredi <> | Date | Mon, 10 Jan 2005 23:28:59 +0100 |
| |
> > + * > > + * - the private_data field of the device file > > + * - the s_fs_info field of the super block > > + * - unused_list, pending, processing lists in fuse_conn > > + * - the unique request ID counter reqctr in fuse_conn > > + * - the sb (super_block) field in fuse_conn > > + * - the file (device file) field in fuse_conn > > + */ > > These comments seem out of date. There is no unused_lsit, pending or > processing lists in fuse_conn. Nor is there a reqctr or file. >
[...]
> > + return NULL; > > + spin_lock(&fuse_lock); > > + fc->sb = sb; > > + spin_unlock(&fuse_lock); > > The lock here looks unnessary, fc is private to this function at this point.
Yes, well these are caused by the split. Later patches will explain these. I could split up the comment too...
> > +static int fuse_read_super(struct super_block *sb, void *data, int silent) > > Can you rename this to fuse_fill_super so its consistent with what the > VFS calls it?
Yes.
> > + fuse_inode_cachep = kmem_cache_create("fuse_inode", > > + sizeof(struct inode) + sizeof(struct fuse_inode) , > > I'm not convinced this will get the right alignments in the case where > struct inode ever changes size. You're better off using a new struct > that contains both and using the size of it here, as well as using it > for calculating the offset in get_fuse_inode instead of &inode[1].
Good point. I haven't thought of this.
> > +int __init fuse_init(void) > > static?
OK.
> > +{ > > + printk(KERN_DEBUG "fuse exit\n"); > > + > > + fuse_fs_cleanup(); > > +} > > Why not just do the cleanup here? If you still want to keep fuse_exit > seperate from fuse_fs_cleanup, may I suggest marking the former __exit?
Again, later patches explain this.
> > +/** Version number of this interface */ > > +#define FUSE_KERNEL_VERSION 5 > > + > > +/** Minor version number of this interface */ > > +#define FUSE_KERNEL_MINOR_VERSION 1 > > I haven't yet looked at the other patches, but is this VERSION info > negotiated with userspace?
Yes.
Thanks for the comments!
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/
| |