[PATCH] XATTR support on JFFS2 (version. 5)

David Woodhouse dwmw2 at infradead.org
Sat May 6 08:38:28 EDT 2006


On Sat, 2006-05-06 at 13:07 +0100, David Woodhouse wrote:
> Thank you. These patches look good to me, in general. I do have a couple
> of minor comments to make, but I'll do that with a slightly shorter Cc
> list. 

As I said, it looks good in general. A few minor points though. I
haven't looked _too_ hard yet at the details, but this is what jumped
out at me when I scrolled through it.

1. Please don't use typedefs. Just 'struct jffs2_acl_header' instead of
this:
	typedef struct {
	       jint32_t        a_version;
	} jffs2_acl_header;


2. You're adding locks without documenting them in README.Locking.
Please make sure any new locks (c->xattr_sem) are fully documented.

3. You have very strange locking behaviour in
jffs2_garbage_collect_xattr() -- sometimes you return with the lock
still held, sometimes without. I think it would be best to check
ic->class within the jffs2_garbage_collect_passs() function instead.

4. You add a list_head to struct jffs2_inode_cache. That's two pointers
-- could we get away with only a single pointer instead?

How much has your code been tested on SMP machines?

Please let me know if you'd like an account with direct access to the
jffs2-xattr-2.6.git tree. I can continue to apply patches if you prefer,
but git is quite easy to use.

-- 
dwmw2





More information about the linux-mtd mailing list