Hello, sorry about the delay. Just one nitpick. On Thu, Oct 25, 2012 at 02:30:18PM -0400, Aristeu Rozanski wrote: > +static int __simple_xattr_remove(struct simple_xattrs *xattrs, > + const char *name) > +{ > + struct simple_xattr *xattr; > + > + xattr = __find_xattr(xattrs, name); > if (xattr) { > + list_del(&xattr->list); > kfree(xattr->name); > kfree(xattr); > + return 0; > } > - return err; > > + return -ENODATA; > +} > + > +/* > + * xattr REMOVE operation for in-memory/pseudo filesystems > + */ > +int simple_xattr_remove(struct simple_xattrs *xattrs, const char *name) > +{ > + int rc; > + > + spin_lock(&xattrs->lock); > + rc = __simple_xattr_remove(xattrs, name); > + spin_unlock(&xattrs->lock); > + > + return rc; Do we need these two functions? Can't you either collapse __simple_xttar_remove() into simple_xattr_remove() or just call __simple_xattr_remove() directly from simple_xattr_set() with locking handled there? Also, why doesn't simple_xattr_remove() have static? Thanks. -- tejun -- 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