Re: JFFS2/xattr problems.

[KaiGai Kohei <kaigai@ak.jp.nec.com>]

David Woodhouse wrote:
> On Sat, 2006-06-24 at 14:58 +0900, KaiGai Kohei wrote:
>> [3/5] jffs2-xattr-v6.1-03-redefine-refcnt-as-atomic_t.patch
>>    In jffs2_release_xattr_datum(), it refers xd->refcnt to ensure
>>    whether releasing xd is allowed or not.
>>    But we can't hold xattr_sem since this function is called under
>>    spin_lock(&c->erase_completion_lock). Thus we have to refer it
>>    without any locking.
> 
> Surely c->alloc_sem is sufficient for this purpose? We can never change
> the refcount of anything without writing to the medium, so the alloc_sem
> ought to be enough.

In some cases without holding c->alloc_sem, we have a possibility to change
xd->refcnt. For example, it's changed when delete_xattr_ref() is called
from jffs2_xattr_delete_inode().
This processing pass does not write anything (includes a delete marker),
because orphan xattr_refs are certainly ignored in next mounting.


But I noticed this patch has another problem last night.
In delete_xattr_ref(), we don't hold c->erase_completion_lock before
atomic_dec_and_test(), so there is a possibility to release
a xattr_datum twice.
I think atomic_dec_and_lock() should be used in delete_xattr_datum().
If so, we can release it safely with minimum locking pain.
This behavior may be correct. Please wait a fix.

Thanks,
-- 
Open Source Software Promotion Center, NEC
KaiGai Kohei <kaigai@ak.jp.nec.com>