JFFS2/xattr problems.

JFFS2/xattr problems.

[David Woodhouse]

I created a user xattr on JFFS2, then attempted to remove it. On NAND
flash I get the following BUG():

jffs2_flash_writev(): Non-contiguous write to 004ca000
wbuf was previously 004ca000-004ca040
------------[ cut here ]------------
kernel BUG at /root/jffs2/wbuf.c:672!

Call Trace:
 [<c8222aad>] jffs2_flash_writev+0x616/0x6ac [jffs2]
 [<c823eadb>] nand_read_ecc+0x29/0x2f [nand]     [<c823eadb>] nand_read_ecc+0x29/0x2f [nand]
 [<c8221a6a>] jffs2_flash_read+0x8a/0x22b [jffs2]     [<c8222b8b>] jffs2_flash_write+0x48/0x51 [jffs2]
 [<c822364d>] delete_xattr_datum_node+0xcc/0x144 [jffs2]     [<c8223dea>] delete_xattr_datum+0x2c/0x3c [jffs2]
 [<c8223ed2>] delete_xattr_ref+0x32/0x3b [jffs2]     [<c8224e37>] do_jffs2_setxattr+0x1a9/0x5aa [jffs2]
 [<c01c1462>] _atomic_dec_and_lock+0x22/0x2c     [<c82254fc>] jffs2_user_setxattr+0x3c/0x47 [jffs2]
 [<c016e9e8>] generic_removexattr+0x37/0x3d     [<c016ef78>] vfs_removexattr+0x78/0xc8

What is delete_xattr_datum_node() trying to do? It seems to be writing
for a second time to an area of flash which has _already_ been written.
You can't do that.

Also, just 'cp -av /lib/libc.so.6 /mnt/jffs2' is failing to set the
POSIX ACL, with -EINVAL. This happens because posix_acl_equiv_mode()
returns zero, because the ACL is entirely equivalent to normal modes. So
the 'value' passed to do_jffs2_setxattr() is NULL, which should _delete_
the corresponding xattr.

But because the xattr is not found, the code returns -EINVAL. Shouldn't
that error be -ENOATTR? And shouldn't the ACL code then convert it to a
more appropriate return value?

-- 
dwmw2

Re: JFFS2/xattr problems.

[David Woodhouse]

Upon removing a file which presumably has an xattr:

jffs2_flush_wbuf() called with alloc_sem not locked!
------------[ cut here ]------------
kernel BUG at /root/jffs2/wbuf.c:424!
invalid opcode: 0000 [#1]
last sysfs file: /class/mtd/mtd0ro/dev
Modules linked in: jffs2(U) mtdchar zlib_deflate cs553x_nand(U) nand(U) mtdpart nand_ids mtdcore nand_ecc msr ipv6 ppdev autofs4 hidp rfcomm l2cap bluetooth sunrpc dm_mirror dm_mod video button battery ac lp parport_pc parport nvram sg usbatm atm rtl8150 snd_ac97_codec snd_seq_dummy snd_seq_oss snd_seq_midi_event snd_seq snd_seq_device snd_pcm_oss snd_mixer_oss snd_pcm snd_timer snd soundcore snd_ac97_bus snd_page_alloc ext3 jbd usb_storage sd_mod scsi_mod ohci_hcd ehci_hcd
CPU:    0
EIP:    0060:[<c822dc4e>]    Not tainted VLI
EFLAGS: 00010282   (2.6.16-1.2111_FC5 #1)
EIP is at __jffs2_flush_wbuf+0x3a/0x7b9 [jffs2]
eax: 00000038   ebx: 00020000   ecx: c62bcdc0   edx: c82364f1
esi: 00000001   edi: 004ca028   ebp: c08bc400   esp: c62bcdbc
ds: 007b   es: 007b   ss: 0068
Process rm (pid: 4149, threadinfo=c62bc000 task=c09da550)
Stack: <0>c82364f1 00000994 00000800 00000100 00000000 00000000 0000000c 00000000
       00000000 c0c5d000 c0c5d800 00000002 c824f628 0000003f 00000003 00000001
       00020000 00000018 004ca028 c08bc400 c822e545 ffffffff ffffffff ffffffff
Call Trace:
 [<c822e545>] jffs2_flash_writev+0x178/0x6ac [jffs2]     [<c824aadb>] nand_read_ecc+0x29/0x2f [nand]
 [<c822d9aa>] jffs2_flash_read+0x8a/0x22b [jffs2]     [<c822eac1>] jffs2_flash_write+0x48/0x51 [jffs2]
 [<c822f441>] delete_xattr_ref_node+0x9d/0x115 [jffs2]     [<c822fe00>] delete_xattr_ref+0x1d/0x3b [jffs2]
 [<c8230560>] jffs2_xattr_delete_inode+0x4d/0x6f [jffs2]     [<c822c5a4>] jffs2_clear_inode+0x1b/0x26 [jffs2]
 [<c016859d>] clear_inode+0xd5/0x102     [<c0168689>] generic_delete_inode+0xbf/0x113
 [<c01680d0>] iput+0x64/0x66     [<c0161018>] do_unlinkat+0xa7/0x10e
 [<c02de11a>] do_page_fault+0x189/0x51d     [<c0102be9>] syscall_call+0x7/0xb
Code: 00 00 00 0f 84 8d 07 00 00 ff 48 4c 0f 88 b0 13 00 00 31 c0 85 c0 75 1c ff 45 4c 0f 8e ae 13 00 00 68 f1 64 23 c8 e8 57 e0 ee f7 <0f> 0b a8 01 de 64 23 c8 59 8b 85 90 01 00 00 85 c0 0f 84 54 07

-- 
dwmw2

Re: JFFS2/xattr problems.

[KaiGai Kohei]

David Woodhouse wrote:
> Upon removing a file which presumably has an xattr:
> 
> jffs2_flush_wbuf() called with alloc_sem not locked!

I confirmed it.
If the previous fix is enabled, this problem will be resolved at same time,
because jffs2_reserve_space() is always called to write a delete marker
when xdatum/xref is removed.

Would you wait for a while?

Thanks,

> ------------[ cut here ]------------
> kernel BUG at /root/jffs2/wbuf.c:424!
> invalid opcode: 0000 [#1]
> last sysfs file: /class/mtd/mtd0ro/dev
> Modules linked in: jffs2(U) mtdchar zlib_deflate cs553x_nand(U) nand(U) mtdpart nand_ids mtdcore nand_ecc msr ipv6 ppdev autofs4 hidp rfcomm l2cap bluetooth sunrpc dm_mirror dm_mod video button battery ac lp parport_pc parport nvram sg usbatm atm rtl8150 snd_ac97_codec snd_seq_dummy snd_seq_oss snd_seq_midi_event snd_seq snd_seq_device snd_pcm_oss snd_mixer_oss snd_pcm snd_timer snd soundcore snd_ac97_bus snd_page_alloc ext3 jbd usb_storage sd_mod scsi_mod ohci_hcd ehci_hcd
> CPU:    0
> EIP:    0060:[<c822dc4e>]    Not tainted VLI
> EFLAGS: 00010282   (2.6.16-1.2111_FC5 #1)
> EIP is at __jffs2_flush_wbuf+0x3a/0x7b9 [jffs2]
> eax: 00000038   ebx: 00020000   ecx: c62bcdc0   edx: c82364f1
> esi: 00000001   edi: 004ca028   ebp: c08bc400   esp: c62bcdbc
> ds: 007b   es: 007b   ss: 0068
> Process rm (pid: 4149, threadinfo=c62bc000 task=c09da550)
> Stack: <0>c82364f1 00000994 00000800 00000100 00000000 00000000 0000000c 00000000
>        00000000 c0c5d000 c0c5d800 00000002 c824f628 0000003f 00000003 00000001
>        00020000 00000018 004ca028 c08bc400 c822e545 ffffffff ffffffff ffffffff
> Call Trace:
>  [<c822e545>] jffs2_flash_writev+0x178/0x6ac [jffs2]     [<c824aadb>] nand_read_ecc+0x29/0x2f [nand]
>  [<c822d9aa>] jffs2_flash_read+0x8a/0x22b [jffs2]     [<c822eac1>] jffs2_flash_write+0x48/0x51 [jffs2]
>  [<c822f441>] delete_xattr_ref_node+0x9d/0x115 [jffs2]     [<c822fe00>] delete_xattr_ref+0x1d/0x3b [jffs2]
>  [<c8230560>] jffs2_xattr_delete_inode+0x4d/0x6f [jffs2]     [<c822c5a4>] jffs2_clear_inode+0x1b/0x26 [jffs2]
>  [<c016859d>] clear_inode+0xd5/0x102     [<c0168689>] generic_delete_inode+0xbf/0x113
>  [<c01680d0>] iput+0x64/0x66     [<c0161018>] do_unlinkat+0xa7/0x10e
>  [<c02de11a>] do_page_fault+0x189/0x51d     [<c0102be9>] syscall_call+0x7/0xb
> Code: 00 00 00 0f 84 8d 07 00 00 ff 48 4c 0f 88 b0 13 00 00 31 c0 85 c0 75 1c ff 45 4c 0f 8e ae 13 00 00 68 f1 64 23 c8 e8 57 e0 ee f7 <0f> 0b a8 01 de 64 23 c8 59 8b 85 90 01 00 00 85 c0 0f 84 54 07
> 
-- 
KaiGai Kohei <kaigai@kaigai.gr.jp>

Re: JFFS2/xattr problems.

[KaiGai Kohei]

Hello, David.

David Woodhouse wrote:
> I created a user xattr on JFFS2, then attempted to remove it. On NAND
> flash I get the following BUG():
> 
> jffs2_flash_writev(): Non-contiguous write to 004ca000
> wbuf was previously 004ca000-004ca040
> ------------[ cut here ]------------
> kernel BUG at /root/jffs2/wbuf.c:672!
> 
> Call Trace:
>  [<c8222aad>] jffs2_flash_writev+0x616/0x6ac [jffs2]
>  [<c823eadb>] nand_read_ecc+0x29/0x2f [nand]     [<c823eadb>] nand_read_ecc+0x29/0x2f [nand]
>  [<c8221a6a>] jffs2_flash_read+0x8a/0x22b [jffs2]     [<c8222b8b>] jffs2_flash_write+0x48/0x51 [jffs2]
>  [<c822364d>] delete_xattr_datum_node+0xcc/0x144 [jffs2]     [<c8223dea>] delete_xattr_datum+0x2c/0x3c [jffs2]
>  [<c8223ed2>] delete_xattr_ref+0x32/0x3b [jffs2]     [<c8224e37>] do_jffs2_setxattr+0x1a9/0x5aa [jffs2]
>  [<c01c1462>] _atomic_dec_and_lock+0x22/0x2c     [<c82254fc>] jffs2_user_setxattr+0x3c/0x47 [jffs2]
>  [<c016e9e8>] generic_removexattr+0x37/0x3d     [<c016ef78>] vfs_removexattr+0x78/0xc8
> 
> What is delete_xattr_datum_node() trying to do? It seems to be writing
> for a second time to an area of flash which has _already_ been written.
> You can't do that.

delete_xattr_datum_node() try to write invalid data to ensure the target
node was already deleted. I was worried about that a xdatum already removed
may be reused as a effective xdatum on next mounting.
But it's purely design problem that should be changed.

Alternatively, I plan to write a delete marker on new node allocated by
jffs2_reserve_space() when xdatum is removed.
I intend to be dealt with a xdatum with 0xffffffff of version number
as a delete marker for xdatum. This node will be ignored on next mounting.

There is a same issue on deletion of xattr_ref, I also plan to implement
with same method.

Some days will be necessary to change it.
Would you wait for a while?

> Also, just 'cp -av /lib/libc.so.6 /mnt/jffs2' is failing to set the
> POSIX ACL, with -EINVAL. This happens because posix_acl_equiv_mode()
> returns zero, because the ACL is entirely equivalent to normal modes. So
> the 'value' passed to do_jffs2_setxattr() is NULL, which should _delete_
> the corresponding xattr.
> 
> But because the xattr is not found, the code returns -EINVAL. Shouldn't
> that error be -ENOATTR? And shouldn't the ACL code then convert it to a
> more appropriate return value?

I agree, I'll fix it.

Thanks,
-- 
KaiGai Kohei <kaigai@kaigai.gr.jp>

Re: JFFS2/xattr problems.

[David Woodhouse]

On Sun, 2006-05-21 at 20:19 +0900, KaiGai Kohei wrote:
> delete_xattr_datum_node() try to write invalid data to ensure the target
> node was already deleted. I was worried about that a xdatum already removed
> may be reused as a effective xdatum on next mounting.
> But it's purely design problem that should be changed.

Yes. You cannot remove it from the flash -- you have to know that it is
there and make sure your scan code copes with it appropriately.

> Alternatively, I plan to write a delete marker on new node allocated by
> jffs2_reserve_space() when xdatum is removed.
> I intend to be dealt with a xdatum with 0xffffffff of version number
> as a delete marker for xdatum. This node will be ignored on next mounting.

OK, that can work. It'll be a little like the 'deletion dirent' with
child inode #0. The same issues apply here -- the 'deletion' node must
remain on the flash for as long as any copies of the _original_ xattr
do.

Also, you must not reuse that xid until all nodes are completely erased.
For inodes, we do this by keeping the jffs2_inode_cache around even when
it's dead, as long as it has nodes. You could possibly use the same
mechanism in erase.c

> Some days will be necessary to change it.
> Would you wait for a while? 

Of course.

If it's convenient (i.e. if I haven't broken it entirely), please work
from git://git.infradead.org/jffs2-devel-2.6.git from now on. That tree
contains a few other JFFS2 improvements, including the elimination of
'__totlen' from the jffs2_raw_node_ref, and some speedup of the code
which reads a summary node from the flash during mount.

-- 
dwmw2

Re: JFFS2/xattr problems.

[KaiGai Kohei]

[-- Attachment #1: Type: text/plain, Size: 3719 bytes --]

Hi, David.

I fixed the problem when xdatum/xref was removed, and so on.

[1/2] jffs2-xattr-v6-01-delete_marker.patch
[2/2] jffs2-xattr-v6-02-fix_posixacl_bug.patch

This correction uses 'delete marker' instead of filling an
existing node by 0xff. It is written when we remvoe xdatum/xref.

Any nodes indicates xdatum/xref includes 'delete marker' are
chained with xd->node or ref->node.
# In the previous implementation, xd->node and ref->node have
# only one node.
Then the garbage collector tries to remove those nodes except
the latest one.

If the latest one is delete marker and any other nodes are not chained
this list, the garbage collector remove the delete marker and in-memory
object (xattr_datum, xattr_ref).
# In previous implementation, deletion of nodes and in-memory objects
# are done immediately

- A xdatum with 'version' equals 0xffffffff means delete marker.
- A xref with odd-numbered 'xseqno' means delete marker.
  It increases 4-byte additional consumption per xref. :(

And the second patch fixes posix-acl bug when we try to replace
null-xattr by null-acl.
Those have been commited into jffs2-xattr-2.6.git.

Thanks,

KaiGai Kohei wrote:
> Hello, David.
> 
> David Woodhouse wrote:
>> I created a user xattr on JFFS2, then attempted to remove it. On NAND
>> flash I get the following BUG():
>>
>> jffs2_flash_writev(): Non-contiguous write to 004ca000
>> wbuf was previously 004ca000-004ca040
>> ------------[ cut here ]------------
>> kernel BUG at /root/jffs2/wbuf.c:672!
>>
>> Call Trace:
>>  [<c8222aad>] jffs2_flash_writev+0x616/0x6ac [jffs2]
>>  [<c823eadb>] nand_read_ecc+0x29/0x2f [nand]     [<c823eadb>] nand_read_ecc+0x29/0x2f [nand]
>>  [<c8221a6a>] jffs2_flash_read+0x8a/0x22b [jffs2]     [<c8222b8b>] jffs2_flash_write+0x48/0x51 [jffs2]
>>  [<c822364d>] delete_xattr_datum_node+0xcc/0x144 [jffs2]     [<c8223dea>] delete_xattr_datum+0x2c/0x3c [jffs2]
>>  [<c8223ed2>] delete_xattr_ref+0x32/0x3b [jffs2]     [<c8224e37>] do_jffs2_setxattr+0x1a9/0x5aa [jffs2]
>>  [<c01c1462>] _atomic_dec_and_lock+0x22/0x2c     [<c82254fc>] jffs2_user_setxattr+0x3c/0x47 [jffs2]
>>  [<c016e9e8>] generic_removexattr+0x37/0x3d     [<c016ef78>] vfs_removexattr+0x78/0xc8
>>
>> What is delete_xattr_datum_node() trying to do? It seems to be writing
>> for a second time to an area of flash which has _already_ been written.
>> You can't do that.
> 
> delete_xattr_datum_node() try to write invalid data to ensure the target
> node was already deleted. I was worried about that a xdatum already removed
> may be reused as a effective xdatum on next mounting.
> But it's purely design problem that should be changed.
> 
> Alternatively, I plan to write a delete marker on new node allocated by
> jffs2_reserve_space() when xdatum is removed.
> I intend to be dealt with a xdatum with 0xffffffff of version number
> as a delete marker for xdatum. This node will be ignored on next mounting.
> 
> There is a same issue on deletion of xattr_ref, I also plan to implement
> with same method.
> 
> Some days will be necessary to change it.
> Would you wait for a while?
> 
>> Also, just 'cp -av /lib/libc.so.6 /mnt/jffs2' is failing to set the
>> POSIX ACL, with -EINVAL. This happens because posix_acl_equiv_mode()
>> returns zero, because the ACL is entirely equivalent to normal modes. So
>> the 'value' passed to do_jffs2_setxattr() is NULL, which should _delete_
>> the corresponding xattr.
>>
>> But because the xattr is not found, the code returns -EINVAL. Shouldn't
>> that error be -ENOATTR? And shouldn't the ACL code then convert it to a
>> more appropriate return value?
> 
> I agree, I'll fix it.
> 
> Thanks,
-- 
Open Source Software Promotion Center, NEC
KaiGai Kohei <kaigai@ak.jp.nec.com>

[-- Attachment #2: jffs2-xattr-v6-01-delete_marker.patch --]
[-- Type: text/plain, Size: 53138 bytes --]

diff --git a/include/linux/jffs2.h b/include/linux/jffs2.h
index c6f7066..c9c7607 100644
--- a/include/linux/jffs2.h
+++ b/include/linux/jffs2.h
@@ -186,6 +186,7 @@ struct jffs2_raw_xref
 	jint32_t hdr_crc;
 	jint32_t ino;		/* inode number */
 	jint32_t xid;		/* XATTR identifier number */
+	jint32_t xseqno;	/* xref sequencial number */
 	jint32_t node_crc;
 } __attribute__((packed));
 
diff --git a/fs/jffs2/acl.c b/fs/jffs2/acl.c
diff --git a/fs/jffs2/erase.c b/fs/jffs2/erase.c
index 1862e8b..1644e34 100644
--- a/fs/jffs2/erase.c
+++ b/fs/jffs2/erase.c
@@ -230,7 +230,6 @@ static inline void jffs2_remove_node_ref
 			   at the end of the linked list. Stash it and continue
 			   from the beginning of the list */
 			ic = (struct jffs2_inode_cache *)(*prev);
-			BUG_ON(ic->class != RAWNODE_CLASS_INODE_CACHE);
 			prev = &ic->nodes;
 			continue;
 		}
@@ -254,7 +253,8 @@ static inline void jffs2_remove_node_ref
 
 	/* PARANOIA */
 	if (!ic) {
-		printk(KERN_WARNING "inode_cache not found in remove_node_refs()!!\n");
+		JFFS2_WARNING("inode_cache/xattr_datum/xattr_ref"
+			      " not found in remove_node_refs()!!\n");
 		return;
 	}
 
@@ -279,8 +279,19 @@ static inline void jffs2_remove_node_ref
 		printk("\n");
 	});
 
-	if (ic->nodes == (void *)ic && ic->nlink == 0)
-		jffs2_del_ino_cache(c, ic);
+	switch (ic->class) {
+#ifdef CONFIG_JFFS2_FS_XATTR
+		case RAWNODE_CLASS_XATTR_DATUM:
+			jffs2_release_xattr_datum(c, (struct jffs2_xattr_datum *)ic);
+			break;
+		case RAWNODE_CLASS_XATTR_REF:
+			jffs2_release_xattr_ref(c, (struct jffs2_xattr_ref *)ic);
+			break;
+#endif
+		default:
+			if (ic->nodes == (void *)ic && ic->nlink == 0)
+				jffs2_del_ino_cache(c, ic);
+	}
 }
 
 void jffs2_free_jeb_node_refs(struct jffs2_sb_info *c, struct jffs2_eraseblock *jeb)
diff --git a/fs/jffs2/gc.c b/fs/jffs2/gc.c
index 477c526..f59b147 100644
--- a/fs/jffs2/gc.c
+++ b/fs/jffs2/gc.c
@@ -275,13 +275,12 @@ #ifdef CONFIG_JFFS2_FS_XATTR
 	 * We can decide whether this node is inode or xattr by ic->class.     */
 	if (ic->class == RAWNODE_CLASS_XATTR_DATUM
 	    || ic->class == RAWNODE_CLASS_XATTR_REF) {
-		BUG_ON(raw->next_in_ino != (void *)ic);
 		spin_unlock(&c->erase_completion_lock);
 
 		if (ic->class == RAWNODE_CLASS_XATTR_DATUM) {
-			ret = jffs2_garbage_collect_xattr_datum(c, (struct jffs2_xattr_datum *)ic);
+			ret = jffs2_garbage_collect_xattr_datum(c, (struct jffs2_xattr_datum *)ic, raw);
 		} else {
-			ret = jffs2_garbage_collect_xattr_ref(c, (struct jffs2_xattr_ref *)ic);
+			ret = jffs2_garbage_collect_xattr_ref(c, (struct jffs2_xattr_ref *)ic, raw);
 		}
 		goto release_sem;
 	}
diff --git a/fs/jffs2/jffs2_fs_sb.h b/fs/jffs2/jffs2_fs_sb.h
index 935fec1..b985949 100644
--- a/fs/jffs2/jffs2_fs_sb.h
+++ b/fs/jffs2/jffs2_fs_sb.h
@@ -119,8 +119,11 @@ #endif
 #ifdef CONFIG_JFFS2_FS_XATTR
 #define XATTRINDEX_HASHSIZE	(57)
 	uint32_t highest_xid;
+	uint32_t highest_xseqno;
 	struct list_head xattrindex[XATTRINDEX_HASHSIZE];
 	struct list_head xattr_unchecked;
+	struct list_head xattr_dead_list;
+	struct jffs2_xattr_ref *xref_dead_list;
 	struct jffs2_xattr_ref *xref_temp;
 	struct rw_semaphore xattr_sem;
 	uint32_t xdatum_mem_usage;
diff --git a/fs/jffs2/malloc.c b/fs/jffs2/malloc.c
index 4889d07..8310c95 100644
--- a/fs/jffs2/malloc.c
+++ b/fs/jffs2/malloc.c
@@ -291,6 +291,7 @@ struct jffs2_xattr_datum *jffs2_alloc_xa
 
 	memset(xd, 0, sizeof(struct jffs2_xattr_datum));
 	xd->class = RAWNODE_CLASS_XATTR_DATUM;
+	xd->node = (void *)xd;
 	INIT_LIST_HEAD(&xd->xindex);
 	return xd;
 }
@@ -309,6 +310,7 @@ struct jffs2_xattr_ref *jffs2_alloc_xatt
 
 	memset(ref, 0, sizeof(struct jffs2_xattr_ref));
 	ref->class = RAWNODE_CLASS_XATTR_REF;
+	ref->node = (void *)ref;
 	return ref;
 }
 
diff --git a/fs/jffs2/nodemgmt.c b/fs/jffs2/nodemgmt.c
index 71d1630..d822bea 100644
--- a/fs/jffs2/nodemgmt.c
+++ b/fs/jffs2/nodemgmt.c
@@ -666,19 +666,26 @@ void jffs2_mark_node_obsolete(struct jff
 		spin_lock(&c->erase_completion_lock);
 
 		ic = jffs2_raw_ref_to_ic(ref);
-		/* It seems we should never call jffs2_mark_node_obsolete() for
-		   XATTR nodes.... yet. Make sure we notice if/when we change
-		   that :) */
-		BUG_ON(ic->class != RAWNODE_CLASS_INODE_CACHE);
 		for (p = &ic->nodes; (*p) != ref; p = &((*p)->next_in_ino))
 			;
 
 		*p = ref->next_in_ino;
 		ref->next_in_ino = NULL;
 
-		if (ic->nodes == (void *)ic && ic->nlink == 0)
-			jffs2_del_ino_cache(c, ic);
-
+		switch (ic->class) {
+#ifdef CONFIG_JFFS2_FS_XATTR
+			case RAWNODE_CLASS_XATTR_DATUM:
+				jffs2_release_xattr_datum(c, (struct jffs2_xattr_datum *)ic);
+				break;
+			case RAWNODE_CLASS_XATTR_REF:
+				jffs2_release_xattr_ref(c, (struct jffs2_xattr_ref *)ic);
+				break;
+#endif
+			default:
+				if (ic->nodes == (void *)ic && ic->nlink == 0)
+					jffs2_del_ino_cache(c, ic);
+				break;
+		}
 		spin_unlock(&c->erase_completion_lock);
 	}
 
diff --git a/fs/jffs2/scan.c b/fs/jffs2/scan.c
index 6161808..79638f5 100644
--- a/fs/jffs2/scan.c
+++ b/fs/jffs2/scan.c
@@ -317,20 +317,25 @@ static int jffs2_scan_xattr_node(struct 
 				 struct jffs2_summary *s)
 {
 	struct jffs2_xattr_datum *xd;
-	uint32_t totlen, crc;
+	uint32_t xid, version, totlen, crc;
 	int err;
 
 	crc = crc32(0, rx, sizeof(struct jffs2_raw_xattr) - 4);
 	if (crc != je32_to_cpu(rx->node_crc)) {
-		if (je32_to_cpu(rx->node_crc) != 0xffffffff)
-			JFFS2_WARNING("node CRC failed at %#08x, read=%#08x, calc=%#08x\n",
-				      ofs, je32_to_cpu(rx->node_crc), crc);
+		JFFS2_WARNING("node CRC failed at %#08x, read=%#08x, calc=%#08x\n",
+			      ofs, je32_to_cpu(rx->node_crc), crc);
 		if ((err = jffs2_scan_dirty_space(c, jeb, je32_to_cpu(rx->totlen))))
 			return err;
 		return 0;
 	}
 
-	totlen = PAD(sizeof(*rx) + rx->name_len + 1 + je16_to_cpu(rx->value_len));
+	xid = je32_to_cpu(rx->xid);
+	version = je32_to_cpu(rx->version);
+
+	totlen = sizeof(struct jffs2_raw_xattr);
+	if (version != XDATUM_DELETE_MARKER)
+		totlen += rx->name_len + 1 + je16_to_cpu(rx->value_len);
+	totlen = PAD(totlen);
 	if (totlen != je32_to_cpu(rx->totlen)) {
 		JFFS2_WARNING("node length mismatch at %#08x, read=%u, calc=%u\n",
 			      ofs, je32_to_cpu(rx->totlen), totlen);
@@ -339,22 +344,24 @@ static int jffs2_scan_xattr_node(struct 
 		return 0;
 	}
 
-	xd = jffs2_setup_xattr_datum(c, je32_to_cpu(rx->xid), je32_to_cpu(rx->version));
-	if (IS_ERR(xd)) {
-		if (PTR_ERR(xd) == -EEXIST) {
-			if ((err = jffs2_scan_dirty_space(c, jeb, PAD(je32_to_cpu(rx->totlen)))))
-				return err;
-			return 0;
-		}
+	xd = jffs2_setup_xattr_datum(c, xid, version);
+	if (IS_ERR(xd))
 		return PTR_ERR(xd);
-	}
-	xd->xprefix = rx->xprefix;
-	xd->name_len = rx->name_len;
-	xd->value_len = je16_to_cpu(rx->value_len);
-	xd->data_crc = je32_to_cpu(rx->data_crc);
 
-	xd->node = jffs2_link_node_ref(c, jeb, ofs | REF_PRISTINE, totlen, NULL);
-	/* FIXME */ xd->node->next_in_ino = (void *)xd;
+	if (xd->version > version) {
+		struct jffs2_raw_node_ref *raw
+			= jffs2_link_node_ref(c, jeb, ofs | REF_PRISTINE, totlen, NULL);
+		raw->next_in_ino = xd->node->next_in_ino;
+		xd->node->next_in_ino = raw;
+	} else {
+		xd->version = version;
+		xd->xprefix = rx->xprefix;
+		xd->name_len = rx->name_len;
+		xd->value_len = je16_to_cpu(rx->value_len);
+		xd->data_crc = je32_to_cpu(rx->data_crc);
+
+		jffs2_link_node_ref(c, jeb, ofs | REF_PRISTINE, totlen, (void *)xd);
+	}
 
 	if (jffs2_sum_active())
 		jffs2_sum_add_xattr_mem(s, rx, ofs - jeb->offset);
@@ -373,9 +380,8 @@ static int jffs2_scan_xref_node(struct j
 
 	crc = crc32(0, rr, sizeof(*rr) - 4);
 	if (crc != je32_to_cpu(rr->node_crc)) {
-		if (je32_to_cpu(rr->node_crc) != 0xffffffff)
-			JFFS2_WARNING("node CRC failed at %#08x, read=%#08x, calc=%#08x\n",
-				      ofs, je32_to_cpu(rr->node_crc), crc);
+		JFFS2_WARNING("node CRC failed at %#08x, read=%#08x, calc=%#08x\n",
+			      ofs, je32_to_cpu(rr->node_crc), crc);
 		if ((err = jffs2_scan_dirty_space(c, jeb, PAD(je32_to_cpu(rr->totlen)))))
 			return err;
 		return 0;
@@ -395,6 +401,7 @@ static int jffs2_scan_xref_node(struct j
 		return -ENOMEM;
 
 	/* BEFORE jffs2_build_xattr_subsystem() called, 
+	 * and AFTER xattr_ref is marked as a dead xref,
 	 * ref->xid is used to store 32bit xid, xd is not used
 	 * ref->ino is used to store 32bit inode-number, ic is not used
 	 * Thoes variables are declared as union, thus using those
@@ -404,11 +411,13 @@ static int jffs2_scan_xref_node(struct j
 	 */
 	ref->ino = je32_to_cpu(rr->ino);
 	ref->xid = je32_to_cpu(rr->xid);
+	ref->xseqno = je32_to_cpu(rr->xseqno);
+	if (ref->xseqno > c->highest_xseqno)
+		c->highest_xseqno = (ref->xseqno & ~XREF_DELETE_MARKER);
 	ref->next = c->xref_temp;
 	c->xref_temp = ref;
 
-	ref->node = jffs2_link_node_ref(c, jeb, ofs | REF_PRISTINE, PAD(je32_to_cpu(rr->totlen)), NULL);
-	/* FIXME */ ref->node->next_in_ino = (void *)ref;
+	jffs2_link_node_ref(c, jeb, ofs | REF_PRISTINE, PAD(je32_to_cpu(rr->totlen)), (void *)ref);
 
 	if (jffs2_sum_active())
 		jffs2_sum_add_xref_mem(s, rr, ofs - jeb->offset);
diff --git a/fs/jffs2/summary.c b/fs/jffs2/summary.c
index 51bf165..d60784a 100644
--- a/fs/jffs2/summary.c
+++ b/fs/jffs2/summary.c
@@ -310,8 +310,6 @@ int jffs2_sum_add_kvec(struct jffs2_sb_i
 #ifdef CONFIG_JFFS2_FS_XATTR
 		case JFFS2_NODETYPE_XATTR: {
 			struct jffs2_sum_xattr_mem *temp;
-			if (je32_to_cpu(node->x.version) == 0xffffffff)
-				return 0;
 			temp = kmalloc(sizeof(struct jffs2_sum_xattr_mem), GFP_KERNEL);
 			if (!temp)
 				goto no_mem;
@@ -327,10 +325,6 @@ #ifdef CONFIG_JFFS2_FS_XATTR
 		}
 		case JFFS2_NODETYPE_XREF: {
 			struct jffs2_sum_xref_mem *temp;
-
-			if (je32_to_cpu(node->r.ino) == 0xffffffff
-			    && je32_to_cpu(node->r.xid) == 0xffffffff)
-				return 0;
 			temp = kmalloc(sizeof(struct jffs2_sum_xref_mem), GFP_KERNEL);
 			if (!temp)
 				goto no_mem;
@@ -483,22 +477,20 @@ #ifdef CONFIG_JFFS2_FS_XATTR
 
 				xd = jffs2_setup_xattr_datum(c, je32_to_cpu(spx->xid),
 								je32_to_cpu(spx->version));
-				if (IS_ERR(xd)) {
-					if (PTR_ERR(xd) == -EEXIST) {
-						/* a newer version of xd exists */
-						if ((err = jffs2_scan_dirty_space(c, jeb, je32_to_cpu(spx->totlen))))
-							return err;
-						sp += JFFS2_SUMMARY_XATTR_SIZE;
-						break;
-					}
-					JFFS2_NOTICE("allocation of xattr_datum failed\n");
+				if (IS_ERR(xd))
 					return PTR_ERR(xd);
+				if (xd->version > je32_to_cpu(spx->version)) {
+					/* node is not the newest one */
+					struct jffs2_raw_node_ref *raw
+						= sum_link_node_ref(c, jeb, je32_to_cpu(spx->offset) | REF_UNCHECKED,
+								    PAD(je32_to_cpu(spx->totlen)), NULL);
+					raw->next_in_ino = xd->node->next_in_ino;
+					xd->node->next_in_ino = raw;
+				} else {
+					xd->version = je32_to_cpu(spx->version);
+					sum_link_node_ref(c, jeb, je32_to_cpu(spx->offset) | REF_UNCHECKED,
+							  PAD(je32_to_cpu(spx->totlen)), (void *)xd);
 				}
-
-				xd->node = sum_link_node_ref(c, jeb, je32_to_cpu(spx->offset) | REF_UNCHECKED,
-							     PAD(je32_to_cpu(spx->totlen)), NULL);
-				/* FIXME */ xd->node->next_in_ino = (void *)xd;
-
 				*pseudo_random += je32_to_cpu(spx->xid);
 				sp += JFFS2_SUMMARY_XATTR_SIZE;
 
@@ -519,14 +511,11 @@ #ifdef CONFIG_JFFS2_FS_XATTR
 					JFFS2_NOTICE("allocation of xattr_datum failed\n");
 					return -ENOMEM;
 				}
-				ref->ino = 0xfffffffe;
-				ref->xid = 0xfffffffd;
 				ref->next = c->xref_temp;
 				c->xref_temp = ref;
 
-				ref->node = sum_link_node_ref(c, jeb, je32_to_cpu(spr->offset) | REF_UNCHECKED,
-							      PAD(sizeof(struct jffs2_raw_xref)), NULL);
-				/* FIXME */ ref->node->next_in_ino = (void *)ref;
+				sum_link_node_ref(c, jeb, je32_to_cpu(spr->offset) | REF_UNCHECKED,
+						  PAD(sizeof(struct jffs2_raw_xref)), (void *)ref);
 
 				*pseudo_random += ref->node->flash_offset;
 				sp += JFFS2_SUMMARY_XREF_SIZE;
diff --git a/fs/jffs2/xattr.c b/fs/jffs2/xattr.c
index 2d82e25..03871ab 100644
--- a/fs/jffs2/xattr.c
+++ b/fs/jffs2/xattr.c
@@ -23,18 +23,15 @@ #include "nodelist.h"
  * xattr_datum_hashkey(xprefix, xname, xvalue, xsize)
  *   is used to calcurate xdatum hashkey. The reminder of hashkey into XATTRINDEX_HASHSIZE is
  *   the index of the xattr name/value pair cache (c->xattrindex).
+ * is_xattr_datum_unchecked(c, xd)
+ *   returns 1, if xdatum contains any unchecked raw nodes. if all raw nodes are not
+ *   unchecked, it returns 0.
  * unload_xattr_datum(c, xd)
  *   is used to release xattr name/value pair and detach from c->xattrindex.
  * reclaim_xattr_datum(c)
  *   is used to reclaim xattr name/value pairs on the xattr name/value pair cache when
  *   memory usage by cache is over c->xdatum_mem_threshold. Currentry, this threshold 
  *   is hard coded as 32KiB.
- * delete_xattr_datum_node(c, xd)
- *   is used to delete a jffs2 node is dominated by xdatum. When EBS(Erase Block Summary) is
- *   enabled, it overwrites the obsolete node by myself.
- * delete_xattr_datum(c, xd)
- *   is used to delete jffs2_xattr_datum object. It must be called with 0-value of reference
- *   counter. (It means how many jffs2_xattr_ref object refers this xdatum.)
  * do_verify_xattr_datum(c, xd)
  *   is used to load the xdatum informations without name/value pair from the medium.
  *   It's necessary once, because those informations are not collected during mounting
@@ -53,8 +50,13 @@ #include "nodelist.h"
  *   is used to write xdatum to medium. xd->version will be incremented.
  * create_xattr_datum(c, xprefix, xname, xvalue, xsize)
  *   is used to create new xdatum and write to medium.
+ * delete_xattr_datum_delay(c, xd)
+ *   is used to delete a xdatum without 'delete marker'. It has a possibility to detect
+ *   orphan xdatum on next mounting.
+ * delete_xattr_datum(c, xd)
+ *   is used to delete a xdatum with 'delete marker'. Calling jffs2_reserve_space() is
+ *   necessary before this function.
  * -------------------------------------------------- */
-
 static uint32_t xattr_datum_hashkey(int xprefix, const char *xname, const char *xvalue, int xsize)
 {
 	int name_len = strlen(xname);
@@ -62,6 +64,22 @@ static uint32_t xattr_datum_hashkey(int 
 	return crc32(xprefix, xname, name_len) ^ crc32(xprefix, xvalue, xsize);
 }
 
+static int is_xattr_datum_unchecked(struct jffs2_sb_info *c, struct jffs2_xattr_datum *xd)
+{
+	struct jffs2_raw_node_ref *raw;
+	int rc = 0;
+
+	spin_lock(&c->erase_completion_lock);
+	for (raw=xd->node; raw != (void *)xd; raw=raw->next_in_ino) {
+		if (ref_flags(raw) == REF_UNCHECKED) {
+			rc = 1;
+			break;
+		}
+	}
+	spin_unlock(&c->erase_completion_lock);
+	return rc;
+}
+
 static void unload_xattr_datum(struct jffs2_sb_info *c, struct jffs2_xattr_datum *xd)
 {
 	/* must be called under down_write(xattr_sem) */
@@ -107,80 +125,39 @@ static void reclaim_xattr_datum(struct j
 		     before, c->xdatum_mem_usage, before - c->xdatum_mem_usage);
 }
 
-static void delete_xattr_datum_node(struct jffs2_sb_info *c, struct jffs2_xattr_datum *xd)
-{
-	/* must be called under down_write(xattr_sem) */
-	struct jffs2_raw_xattr rx;
-	size_t length;
-	int rc;
-
-	if (!xd->node) {
-		JFFS2_WARNING("xdatum (xid=%u) is removed twice.\n", xd->xid);
-		return;
-	}
-	if (jffs2_sum_active()) {
-		memset(&rx, 0xff, sizeof(struct jffs2_raw_xattr));
-		rc = jffs2_flash_read(c, ref_offset(xd->node),
-				      sizeof(struct jffs2_unknown_node),
-				      &length, (char *)&rx);
-		if (rc || length != sizeof(struct jffs2_unknown_node)) {
-			JFFS2_ERROR("jffs2_flash_read()=%d, req=%zu, read=%zu at %#08x\n",
-				    rc, sizeof(struct jffs2_unknown_node),
-				    length, ref_offset(xd->node));
-		}
-		rc = jffs2_flash_write(c, ref_offset(xd->node), sizeof(rx),
-				       &length, (char *)&rx);
-		if (rc || length != sizeof(struct jffs2_raw_xattr)) {
-			JFFS2_ERROR("jffs2_flash_write()=%d, req=%zu, wrote=%zu ar %#08x\n",
-				    rc, sizeof(rx), length, ref_offset(xd->node));
-		}
-	}
-	spin_lock(&c->erase_completion_lock);
-	xd->node->next_in_ino = NULL;
-	spin_unlock(&c->erase_completion_lock);
-	jffs2_mark_node_obsolete(c, xd->node);
-	xd->node = NULL;
-}
-
-static void delete_xattr_datum(struct jffs2_sb_info *c, struct jffs2_xattr_datum *xd)
-{
-	/* must be called under down_write(xattr_sem) */
-	BUG_ON(xd->refcnt);
-
-	unload_xattr_datum(c, xd);
-	if (xd->node) {
-		delete_xattr_datum_node(c, xd);
-		xd->node = NULL;
-	}
-	jffs2_free_xattr_datum(xd);
-}
-
 static int do_verify_xattr_datum(struct jffs2_sb_info *c, struct jffs2_xattr_datum *xd)
 {
 	/* must be called under down_write(xattr_sem) */
 	struct jffs2_eraseblock *jeb;
+	struct jffs2_raw_node_ref *raw;
 	struct jffs2_raw_xattr rx;
 	size_t readlen;
-	uint32_t crc, totlen;
+	uint32_t crc, offset, totlen;
 	int rc;
 
-	BUG_ON(!xd->node);
-	BUG_ON(ref_flags(xd->node) != REF_UNCHECKED);
+	spin_lock(&c->erase_completion_lock);
+	offset = ref_offset(xd->node);
+	if (ref_flags(xd->node) == REF_PRISTINE)
+		goto complete;
+	spin_unlock(&c->erase_completion_lock);
 
-	rc = jffs2_flash_read(c, ref_offset(xd->node), sizeof(rx), &readlen, (char *)&rx);
+	rc = jffs2_flash_read(c, offset, sizeof(rx), &readlen, (char *)&rx);
 	if (rc || readlen != sizeof(rx)) {
 		JFFS2_WARNING("jffs2_flash_read()=%d, req=%zu, read=%zu at %#08x\n",
-			      rc, sizeof(rx), readlen, ref_offset(xd->node));
+			      rc, sizeof(rx), readlen, offset);
 		return rc ? rc : -EIO;
 	}
 	crc = crc32(0, &rx, sizeof(rx) - 4);
 	if (crc != je32_to_cpu(rx.node_crc)) {
-		if (je32_to_cpu(rx.node_crc) != 0xffffffff)
-			JFFS2_ERROR("node CRC failed at %#08x, read=%#08x, calc=%#08x\n",
-				    ref_offset(xd->node), je32_to_cpu(rx.hdr_crc), crc);
+		JFFS2_ERROR("node CRC failed at %#08x, read=%#08x, calc=%#08x\n",
+			    offset, je32_to_cpu(rx.hdr_crc), crc);
+		xd->flags |= JFFS2_XFLAGS_INVALID;
 		return EIO;
 	}
-	totlen = PAD(sizeof(rx) + rx.name_len + 1 + je16_to_cpu(rx.value_len));
+	totlen = sizeof(rx);
+	if (xd->version != XDATUM_DELETE_MARKER)
+		totlen += rx.name_len + 1 + je16_to_cpu(rx.value_len);
+	totlen = PAD(totlen);
 	if (je16_to_cpu(rx.magic) != JFFS2_MAGIC_BITMASK
 	    || je16_to_cpu(rx.nodetype) != JFFS2_NODETYPE_XATTR
 	    || je32_to_cpu(rx.totlen) != totlen
@@ -188,11 +165,12 @@ static int do_verify_xattr_datum(struct 
 	    || je32_to_cpu(rx.version) != xd->version) {
 		JFFS2_ERROR("inconsistent xdatum at %#08x, magic=%#04x/%#04x, "
 			    "nodetype=%#04x/%#04x, totlen=%u/%u, xid=%u/%u, version=%u/%u\n",
-			    ref_offset(xd->node), je16_to_cpu(rx.magic), JFFS2_MAGIC_BITMASK,
+			    offset, je16_to_cpu(rx.magic), JFFS2_MAGIC_BITMASK,
 			    je16_to_cpu(rx.nodetype), JFFS2_NODETYPE_XATTR,
 			    je32_to_cpu(rx.totlen), totlen,
 			    je32_to_cpu(rx.xid), xd->xid,
 			    je32_to_cpu(rx.version), xd->version);
+		xd->flags |= JFFS2_XFLAGS_INVALID;
 		return EIO;
 	}
 	xd->xprefix = rx.xprefix;
@@ -200,14 +178,17 @@ static int do_verify_xattr_datum(struct 
 	xd->value_len = je16_to_cpu(rx.value_len);
 	xd->data_crc = je32_to_cpu(rx.data_crc);
 
-	/* This JFFS2_NODETYPE_XATTR node is checked */
-	jeb = &c->blocks[ref_offset(xd->node) / c->sector_size];
-	totlen = PAD(je32_to_cpu(rx.totlen));
-
 	spin_lock(&c->erase_completion_lock);
-	c->unchecked_size -= totlen; c->used_size += totlen;
-	jeb->unchecked_size -= totlen; jeb->used_size += totlen;
-	xd->node->flash_offset = ref_offset(xd->node) | REF_PRISTINE;
+ complete:
+	for (raw=xd->node; raw != (void *)xd; raw=raw->next_in_ino) {
+		jeb = &c->blocks[ref_offset(raw) / c->sector_size];
+		totlen = PAD(ref_totlen(c, jeb, raw));
+		if (ref_flags(raw) == REF_UNCHECKED) {
+			c->unchecked_size -= totlen; c->used_size += totlen;
+			jeb->unchecked_size -= totlen; jeb->used_size += totlen;
+		}
+		raw->flash_offset = ref_offset(raw) | ((xd->node==raw) ? REF_PRISTINE : REF_NORMAL);
+	}
 	spin_unlock(&c->erase_completion_lock);
 
 	/* unchecked xdatum is chained with c->xattr_unchecked */
@@ -227,7 +208,6 @@ static int do_load_xattr_datum(struct jf
 	uint32_t crc, length;
 	int i, ret, retry = 0;
 
-	BUG_ON(!xd->node);
 	BUG_ON(ref_flags(xd->node) != REF_PRISTINE);
 	BUG_ON(!list_empty(&xd->xindex));
  retry:
@@ -253,6 +233,7 @@ static int do_load_xattr_datum(struct jf
 			      " at %#08x, read: 0x%08x calculated: 0x%08x\n",
 			      ref_offset(xd->node), xd->data_crc, crc);
 		kfree(data);
+		xd->flags |= JFFS2_XFLAGS_INVALID;
 		return EIO;
 	}
 
@@ -286,16 +267,13 @@ static int load_xattr_datum(struct jffs2
 	 * rc > 0 : Unrecoverable error, this node should be deleted.
 	 */
 	int rc = 0;
-	BUG_ON(xd->xname);
-	if (!xd->node)
+
+	if (xd->xname)
+		return 0;
+	if (xd->flags & JFFS2_XFLAGS_INVALID)
 		return EIO;
-	if (unlikely(ref_flags(xd->node) != REF_PRISTINE)) {
+	if (unlikely(is_xattr_datum_unchecked(c, xd)))
 		rc = do_verify_xattr_datum(c, xd);
-		if (rc > 0) {
-			list_del_init(&xd->xindex);
-			delete_xattr_datum_node(c, xd);
-		}
-	}
 	if (!rc)
 		rc = do_load_xattr_datum(c, xd);
 	return rc;
@@ -304,36 +282,43 @@ static int load_xattr_datum(struct jffs2
 static int save_xattr_datum(struct jffs2_sb_info *c, struct jffs2_xattr_datum *xd)
 {
 	/* must be called under down_write(xattr_sem) */
-	struct jffs2_raw_node_ref *raw;
 	struct jffs2_raw_xattr rx;
 	struct kvec vecs[2];
 	size_t length;
-	int rc, totlen;
+	int rc, totlen, nvecs = 1;
 	uint32_t phys_ofs = write_ofs(c);
 
-	BUG_ON(!xd->xname);
+	BUG_ON(is_xattr_datum_dead(xd) || (xd->flags & JFFS2_XFLAGS_INVALID)
+	       ? !!xd->xname : !xd->xname);
 
 	vecs[0].iov_base = &rx;
-	vecs[0].iov_len = PAD(sizeof(rx));
-	vecs[1].iov_base = xd->xname;
-	vecs[1].iov_len = xd->name_len + 1 + xd->value_len;
-	totlen = vecs[0].iov_len + vecs[1].iov_len;
-
+	vecs[0].iov_len = totlen = sizeof(rx);
+	if (!is_xattr_datum_dead(xd) && !(xd->flags & JFFS2_XFLAGS_INVALID)) {
+		nvecs++;
+		vecs[1].iov_base = xd->xname;
+		vecs[1].iov_len = xd->name_len + 1 + xd->value_len;
+		totlen += vecs[1].iov_len;
+	}
 	/* Setup raw-xattr */
+	memset(&rx, 0, sizeof(rx));
 	rx.magic = cpu_to_je16(JFFS2_MAGIC_BITMASK);
 	rx.nodetype = cpu_to_je16(JFFS2_NODETYPE_XATTR);
 	rx.totlen = cpu_to_je32(PAD(totlen));
 	rx.hdr_crc = cpu_to_je32(crc32(0, &rx, sizeof(struct jffs2_unknown_node) - 4));
 
 	rx.xid = cpu_to_je32(xd->xid);
-	rx.version = cpu_to_je32(++xd->version);
-	rx.xprefix = xd->xprefix;
-	rx.name_len = xd->name_len;
-	rx.value_len = cpu_to_je16(xd->value_len);
-	rx.data_crc = cpu_to_je32(crc32(0, vecs[1].iov_base, vecs[1].iov_len));
+	if (!is_xattr_datum_dead(xd) && !(xd->flags & JFFS2_XFLAGS_INVALID)) {
+		rx.version = cpu_to_je32(++xd->version);
+		rx.xprefix = xd->xprefix;
+		rx.name_len = xd->name_len;
+		rx.value_len = cpu_to_je16(xd->value_len);
+		rx.data_crc = cpu_to_je32(crc32(0, vecs[1].iov_base, vecs[1].iov_len));
+	} else {
+		rx.version = cpu_to_je32(XDATUM_DELETE_MARKER);
+	}
 	rx.node_crc = cpu_to_je32(crc32(0, &rx, sizeof(struct jffs2_raw_xattr) - 4));
 
-	rc = jffs2_flash_writev(c, vecs, 2, phys_ofs, &length, 0);
+	rc = jffs2_flash_writev(c, vecs, nvecs, phys_ofs, &length, 0);
 	if (rc || totlen != length) {
 		JFFS2_WARNING("jffs2_flash_writev()=%d, req=%u, wrote=%zu, at %#08x\n",
 			      rc, totlen, length, phys_ofs);
@@ -343,14 +328,8 @@ static int save_xattr_datum(struct jffs2
 
 		return rc;
 	}
-
 	/* success */
-	raw = jffs2_add_physical_node_ref(c, phys_ofs | REF_PRISTINE, PAD(totlen), NULL);
-	/* FIXME */ raw->next_in_ino = (void *)xd;
-
-	if (xd->node)
-		delete_xattr_datum_node(c, xd);
-	xd->node = raw;
+	jffs2_add_physical_node_ref(c, phys_ofs | REF_PRISTINE, PAD(totlen), (void *)xd);
 
 	dbg_xattr("success on saving xdatum (xid=%u, version=%u, xprefix=%u, xname='%s')\n",
 		  xd->xid, xd->version, xd->xprefix, xd->xname);
@@ -426,6 +405,39 @@ static struct jffs2_xattr_datum *create_
 	return xd;
 }
 
+static void delete_xattr_datum_delay(struct jffs2_sb_info *c, struct jffs2_xattr_datum *xd)
+{
+	/* must be called under down_write(xattr_sem) */
+	BUG_ON(xd->refcnt);
+
+	unload_xattr_datum(c, xd);
+	set_xattr_datum_dead(xd);
+	spin_lock(&c->erase_completion_lock);
+	list_add(&xd->xindex, &c->xattr_dead_list);
+	spin_unlock(&c->erase_completion_lock);
+	JFFS2_NOTICE("xdatum(xid=%u) was removed without delete marker. "
+		     "An orphan xdatum may be detected on next mounting.\n", xd->xid);
+}
+
+static void delete_xattr_datum(struct jffs2_sb_info *c, struct jffs2_xattr_datum *xd)
+{
+	/* must be called under jffs2_reserve_space() and down_write(xattr_sem) */
+	int rc;
+	BUG_ON(xd->refcnt);
+
+	unload_xattr_datum(c, xd);
+	set_xattr_datum_dead(xd);	
+	rc = save_xattr_datum(c, xd);
+	if (rc) {
+		JFFS2_NOTICE("xdatum(xid=%u) was removed without delete marker. "
+			     "An orphan xdatum may be detected on next mounting.\n",
+			     xd->xid);
+	}
+	spin_lock(&c->erase_completion_lock);
+	list_add(&xd->xindex, &c->xattr_dead_list);
+	spin_unlock(&c->erase_completion_lock);
+}
+
 /* -------- xref related functions ------------------
  * verify_xattr_ref(c, ref)
  *   is used to load xref information from medium. Because summary data does not
@@ -450,25 +462,29 @@ static struct jffs2_xattr_datum *create_
 static int verify_xattr_ref(struct jffs2_sb_info *c, struct jffs2_xattr_ref *ref)
 {
 	struct jffs2_eraseblock *jeb;
+	struct jffs2_raw_node_ref *raw;
 	struct jffs2_raw_xref rr;
 	size_t readlen;
-	uint32_t crc, totlen;
+	uint32_t crc, offset, totlen;
 	int rc;
 
-	BUG_ON(ref_flags(ref->node) != REF_UNCHECKED);
+	spin_lock(&c->erase_completion_lock);
+	if (ref_flags(ref->node) != REF_UNCHECKED)
+		goto complete;
+	offset = ref_offset(ref->node);
+	spin_unlock(&c->erase_completion_lock);
 
-	rc = jffs2_flash_read(c, ref_offset(ref->node), sizeof(rr), &readlen, (char *)&rr);
+	rc = jffs2_flash_read(c, offset, sizeof(rr), &readlen, (char *)&rr);
 	if (rc || sizeof(rr) != readlen) {
 		JFFS2_WARNING("jffs2_flash_read()=%d, req=%zu, read=%zu, at %#08x\n",
-			      rc, sizeof(rr), readlen, ref_offset(ref->node));
+			      rc, sizeof(rr), readlen, offset);
 		return rc ? rc : -EIO;
 	}
 	/* obsolete node */
 	crc = crc32(0, &rr, sizeof(rr) - 4);
 	if (crc != je32_to_cpu(rr.node_crc)) {
-		if (je32_to_cpu(rr.node_crc) != 0xffffffff)
-			JFFS2_ERROR("node CRC failed at %#08x, read=%#08x, calc=%#08x\n",
-				    ref_offset(ref->node), je32_to_cpu(rr.node_crc), crc);
+		JFFS2_ERROR("node CRC failed at %#08x, read=%#08x, calc=%#08x\n",
+			    offset, je32_to_cpu(rr.node_crc), crc);
 		return EIO;
 	}
 	if (je16_to_cpu(rr.magic) != JFFS2_MAGIC_BITMASK
@@ -476,22 +492,28 @@ static int verify_xattr_ref(struct jffs2
 	    || je32_to_cpu(rr.totlen) != PAD(sizeof(rr))) {
 		JFFS2_ERROR("inconsistent xref at %#08x, magic=%#04x/%#04x, "
 			    "nodetype=%#04x/%#04x, totlen=%u/%zu\n",
-			    ref_offset(ref->node), je16_to_cpu(rr.magic), JFFS2_MAGIC_BITMASK,
+			    offset, je16_to_cpu(rr.magic), JFFS2_MAGIC_BITMASK,
 			    je16_to_cpu(rr.nodetype), JFFS2_NODETYPE_XREF,
 			    je32_to_cpu(rr.totlen), PAD(sizeof(rr)));
 		return EIO;
 	}
 	ref->ino = je32_to_cpu(rr.ino);
 	ref->xid = je32_to_cpu(rr.xid);
-
-	/* fixup superblock/eraseblock info */
-	jeb = &c->blocks[ref_offset(ref->node) / c->sector_size];
-	totlen = PAD(sizeof(rr));
+	ref->xseqno = je32_to_cpu(rr.xseqno);
+	if (ref->xseqno > c->highest_xseqno)
+		c->highest_xseqno = (ref->xseqno & ~XREF_DELETE_MARKER);
 
 	spin_lock(&c->erase_completion_lock);
-	c->unchecked_size -= totlen; c->used_size += totlen;
-	jeb->unchecked_size -= totlen; jeb->used_size += totlen;
-	ref->node->flash_offset = ref_offset(ref->node) | REF_PRISTINE;
+ complete:
+	for (raw=ref->node; raw != (void *)ref; raw=raw->next_in_ino) {
+		jeb = &c->blocks[ref_offset(raw) / c->sector_size];
+		totlen = PAD(ref_totlen(c, jeb, raw));
+		if (ref_flags(raw) == REF_UNCHECKED) {
+			c->unchecked_size -= totlen; c->used_size += totlen;
+			jeb->unchecked_size -= totlen; jeb->used_size += totlen;
+		}
+		raw->flash_offset = ref_offset(raw) | ((ref->node==raw) ? REF_PRISTINE : REF_NORMAL);
+	}
 	spin_unlock(&c->erase_completion_lock);
 
 	dbg_xattr("success on verifying xref (ino=%u, xid=%u) at %#08x\n",
@@ -499,58 +521,12 @@ static int verify_xattr_ref(struct jffs2
 	return 0;
 }
 
-static void delete_xattr_ref_node(struct jffs2_sb_info *c, struct jffs2_xattr_ref *ref)
-{
-	struct jffs2_raw_xref rr;
-	size_t length;
-	int rc;
-
-	if (jffs2_sum_active()) {
-		memset(&rr, 0xff, sizeof(rr));
-		rc = jffs2_flash_read(c, ref_offset(ref->node),
-				      sizeof(struct jffs2_unknown_node),
-				      &length, (char *)&rr);
-		if (rc || length != sizeof(struct jffs2_unknown_node)) {
-			JFFS2_ERROR("jffs2_flash_read()=%d, req=%zu, read=%zu at %#08x\n",
-				    rc, sizeof(struct jffs2_unknown_node),
-				    length, ref_offset(ref->node));
-		}
-		rc = jffs2_flash_write(c, ref_offset(ref->node), sizeof(rr),
-				       &length, (char *)&rr);
-		if (rc || length != sizeof(struct jffs2_raw_xref)) {
-			JFFS2_ERROR("jffs2_flash_write()=%d, req=%zu, wrote=%zu at %#08x\n",
-				    rc, sizeof(rr), length, ref_offset(ref->node));
-		}
-	}
-	spin_lock(&c->erase_completion_lock);
-	ref->node->next_in_ino = NULL;
-	spin_unlock(&c->erase_completion_lock);
-	jffs2_mark_node_obsolete(c, ref->node);
-	ref->node = NULL;
-}
-
-static void delete_xattr_ref(struct jffs2_sb_info *c, struct jffs2_xattr_ref *ref)
-{
-	/* must be called under down_write(xattr_sem) */
-	struct jffs2_xattr_datum *xd;
-
-	BUG_ON(!ref->node);
-	delete_xattr_ref_node(c, ref);
-
-	xd = ref->xd;
-	xd->refcnt--;
-	if (!xd->refcnt)
-		delete_xattr_datum(c, xd);
-	jffs2_free_xattr_ref(ref);
-}
-
 static int save_xattr_ref(struct jffs2_sb_info *c, struct jffs2_xattr_ref *ref)
 {
 	/* must be called under down_write(xattr_sem) */
-	struct jffs2_raw_node_ref *raw;
 	struct jffs2_raw_xref rr;
 	size_t length;
-	uint32_t phys_ofs = write_ofs(c);
+	uint32_t xseqno, phys_ofs = write_ofs(c);
 	int ret;
 
 	rr.magic = cpu_to_je16(JFFS2_MAGIC_BITMASK);
@@ -558,8 +534,16 @@ static int save_xattr_ref(struct jffs2_s
 	rr.totlen = cpu_to_je32(PAD(sizeof(rr)));
 	rr.hdr_crc = cpu_to_je32(crc32(0, &rr, sizeof(struct jffs2_unknown_node) - 4));
 
-	rr.ino = cpu_to_je32(ref->ic->ino);
-	rr.xid = cpu_to_je32(ref->xd->xid);
+	xseqno = (c->highest_xseqno += 2);
+	if (is_xattr_ref_dead(ref)) {
+		xseqno |= XREF_DELETE_MARKER;
+		rr.ino = cpu_to_je32(ref->ino);
+		rr.xid = cpu_to_je32(ref->xid);
+	} else {
+		rr.ino = cpu_to_je32(ref->ic->ino);
+		rr.xid = cpu_to_je32(ref->xd->xid);
+	}
+	rr.xseqno = cpu_to_je32(xseqno);
 	rr.node_crc = cpu_to_je32(crc32(0, &rr, sizeof(rr) - 4));
 
 	ret = jffs2_flash_write(c, phys_ofs, sizeof(rr), &length, (char *)&rr);
@@ -572,12 +556,9 @@ static int save_xattr_ref(struct jffs2_s
 
 		return ret;
 	}
-
-	raw = jffs2_add_physical_node_ref(c, phys_ofs | REF_PRISTINE, PAD(sizeof(rr)), NULL);
-	/* FIXME */ raw->next_in_ino = (void *)ref;
-	if (ref->node)
-		delete_xattr_ref_node(c, ref);
-	ref->node = raw;
+	/* success */
+	ref->xseqno = xseqno;
+	jffs2_add_physical_node_ref(c, phys_ofs | REF_PRISTINE, PAD(sizeof(rr)), (void *)ref);
 
 	dbg_xattr("success on saving xref (ino=%u, xid=%u)\n", ref->ic->ino, ref->xd->xid);
 
@@ -610,22 +591,116 @@ static struct jffs2_xattr_ref *create_xa
 	return ref; /* success */
 }
 
+static void delete_xattr_ref_delay(struct jffs2_sb_info *c, struct jffs2_xattr_ref *ref)
+{
+	/* must be called under down_write(xattr_sem) */
+	struct jffs2_xattr_datum *xd;
+
+	set_xattr_ref_dead(ref);
+	xd = ref->xd;
+	ref->ino = ref->ic->ino;
+	ref->xid = ref->xd->xid;
+	spin_lock(&c->erase_completion_lock);
+	ref->next = c->xref_dead_list;
+	c->xref_dead_list = ref;
+	spin_unlock(&c->erase_completion_lock);
+
+	JFFS2_NOTICE("xref(ino=%u, xid=%u) was removed without delete marker. "
+		     "An orphan xref may be detected on next mounting.\n",
+		     ref->ino, ref->xid);
+
+	if (!--xd->refcnt)
+		delete_xattr_datum_delay(c, xd);
+}
+
+static int delete_xattr_ref(struct jffs2_sb_info *c, struct jffs2_xattr_ref *ref, int enforce)
+{
+	/* must be called under jffs2_reserve_space() and down_write(xattr_sem) */
+	struct jffs2_inode_cache *ic;
+	struct jffs2_xattr_datum *xd;
+	uint32_t length;
+	int rc;
+
+	set_xattr_ref_dead(ref);
+	ic = ref->ic;
+	xd = ref->xd;
+	ref->ino = ic->ino;
+	ref->xid = xd->xid;
+	rc = save_xattr_ref(c, ref);
+	if (rc) {
+		if (!enforce) {
+			clr_xattr_ref_dead(ref);
+			ref->ic = ic;
+			ref->xd = xd;
+			return rc;
+		}
+		JFFS2_WARNING("could not write delete marker of xref(ino=%u, xid=%u). "
+			      "An orphan xref may be detected on next mounting.\n",
+			      ref->ic->ino, ref->xd->xid);
+	}
+	spin_lock(&c->erase_completion_lock);
+	ref->next = c->xref_dead_list;
+	c->xref_dead_list = ref;
+	spin_unlock(&c->erase_completion_lock);
+
+	xd->refcnt--;
+	if (xd->refcnt)
+		return 0;
+
+	/* delete xdatum */
+	unload_xattr_datum(c, xd);
+	up_write(&c->xattr_sem);
+	jffs2_complete_reservation(c);
+
+	rc = jffs2_reserve_space(c, PAD(sizeof(struct jffs2_raw_xattr)), &length,
+				 ALLOC_DELETION, JFFS2_SUMMARY_XATTR_SIZE);
+	if (rc) {
+		down(&c->alloc_sem);
+		down_write(&c->xattr_sem);
+		delete_xattr_datum_delay(c, xd);
+	} else {
+		down_write(&c->xattr_sem);
+		delete_xattr_datum(c, xd);
+	}
+	return 0;
+}
+
 void jffs2_xattr_delete_inode(struct jffs2_sb_info *c, struct jffs2_inode_cache *ic)
 {
 	/* It's called from jffs2_clear_inode() on inode removing.
 	   When an inode with XATTR is removed, those XATTRs must be removed. */
-	struct jffs2_xattr_ref *ref, *_ref;
+	struct jffs2_xattr_ref *ref;
+	uint32_t length;
+	int rc, retry;
 
 	if (!ic || ic->nlink > 0)
 		return;
 
+	down_read(&c->xattr_sem);
+	if (!ic->xref) {
+		up_read(&c->xattr_sem);
+		return;
+	}
+	up_read(&c->xattr_sem);
+ retry:
+	rc = jffs2_reserve_space(c, PAD(sizeof(struct jffs2_raw_xref)), &length,
+				 ALLOC_DELETION, JFFS2_SUMMARY_XREF_SIZE);
 	down_write(&c->xattr_sem);
-	for (ref = ic->xref; ref; ref = _ref) {
-		_ref = ref->next;
-		delete_xattr_ref(c, ref);
+	if (ic->xref) {
+		ref = ic->xref;
+		ic->xref = ref->next;
+		if (rc) {
+			delete_xattr_ref_delay(c, ref);
+		} else {
+			delete_xattr_ref(c, ref, 1);
+		}
 	}
-	ic->xref = NULL;
+	retry = ic->xref ? 1 : 0;
 	up_write(&c->xattr_sem);
+	if (!rc)
+		jffs2_complete_reservation(c);
+	if (retry)
+		goto retry;
 }
 
 void jffs2_xattr_free_inode(struct jffs2_sb_info *c, struct jffs2_inode_cache *ic)
@@ -655,7 +730,7 @@ static int check_xattr_ref_inode(struct 
 	 * duplicate name/value pairs. If duplicate name/value pair would be found,
 	 * one will be removed.
 	 */
-	struct jffs2_xattr_ref *ref, *cmp, **pref;
+	struct jffs2_xattr_ref *ref, *cmp, **pref, **pcmp;
 	int rc = 0;
 
 	if (likely(ic->flags & INO_FLAGS_XATTR_CHECKED))
@@ -668,27 +743,32 @@ static int check_xattr_ref_inode(struct 
 			rc = load_xattr_datum(c, ref->xd);
 			if (unlikely(rc > 0)) {
 				*pref = ref->next;
-				delete_xattr_ref(c, ref);
+				delete_xattr_ref_delay(c, ref);
 				goto retry;
 			} else if (unlikely(rc < 0))
 				goto out;
 		}
-		for (cmp=ref->next, pref=&ref->next; cmp; pref=&cmp->next, cmp=cmp->next) {
+		for (cmp=ref->next, pcmp=&ref->next; cmp; pcmp=&cmp->next, cmp=cmp->next) {
 			if (!cmp->xd->xname) {
 				ref->xd->flags |= JFFS2_XFLAGS_BIND;
 				rc = load_xattr_datum(c, cmp->xd);
 				ref->xd->flags &= ~JFFS2_XFLAGS_BIND;
 				if (unlikely(rc > 0)) {
-					*pref = cmp->next;
-					delete_xattr_ref(c, cmp);
+					*pcmp = cmp->next;
+					delete_xattr_ref_delay(c, cmp);
 					goto retry;
 				} else if (unlikely(rc < 0))
 					goto out;
 			}
 			if (ref->xd->xprefix == cmp->xd->xprefix
 			    && !strcmp(ref->xd->xname, cmp->xd->xname)) {
-				*pref = cmp->next;
-				delete_xattr_ref(c, cmp);
+				if (ref->xseqno > cmp->xseqno) {
+					*pcmp = cmp->next;
+					delete_xattr_ref_delay(c, cmp);
+				} else {
+					*pref = ref->next;
+					delete_xattr_ref_delay(c, ref);
+				}
 				goto retry;
 			}
 		}
@@ -719,9 +799,13 @@ void jffs2_init_xattr_subsystem(struct j
 	for (i=0; i < XATTRINDEX_HASHSIZE; i++)
 		INIT_LIST_HEAD(&c->xattrindex[i]);
 	INIT_LIST_HEAD(&c->xattr_unchecked);
+	INIT_LIST_HEAD(&c->xattr_dead_list);
+	c->xref_dead_list = NULL;
 	c->xref_temp = NULL;
 
 	init_rwsem(&c->xattr_sem);
+	c->highest_xid = 0;
+	c->highest_xseqno = 0;
 	c->xdatum_mem_usage = 0;
 	c->xdatum_mem_threshold = 32 * 1024;	/* Default 32KB */
 }
@@ -751,7 +835,11 @@ void jffs2_clear_xattr_subsystem(struct 
 		_ref = ref->next;
 		jffs2_free_xattr_ref(ref);
 	}
-	c->xref_temp = NULL;
+
+	for (ref=c->xref_dead_list; ref; ref = _ref) {
+		_ref = ref->next;
+		jffs2_free_xattr_ref(ref);
+	}
 
 	for (i=0; i < XATTRINDEX_HASHSIZE; i++) {
 		list_for_each_entry_safe(xd, _xd, &c->xattrindex[i], xindex) {
@@ -761,64 +849,120 @@ void jffs2_clear_xattr_subsystem(struct 
 			jffs2_free_xattr_datum(xd);
 		}
 	}
+
+	list_for_each_entry_safe(xd, _xd, &c->xattr_dead_list, xindex) {
+		list_del(&xd->xindex);
+		jffs2_free_xattr_datum(xd);
+	}
 }
 
+#define XREF_TMPHASH_SIZE	(128)
 void jffs2_build_xattr_subsystem(struct jffs2_sb_info *c)
 {
 	struct jffs2_xattr_ref *ref, *_ref;
+	struct jffs2_xattr_ref *xref_tmphash[XREF_TMPHASH_SIZE];
 	struct jffs2_xattr_datum *xd, *_xd;
 	struct jffs2_inode_cache *ic;
-	int i, xdatum_count =0, xdatum_unchecked_count = 0, xref_count = 0;
+	struct jffs2_raw_node_ref *raw;
+	int i, xdatum_count = 0, xdatum_unchecked_count = 0, xref_count = 0;
 
 	BUG_ON(!(c->flags & JFFS2_SB_FLAG_BUILDING));
+	/* Phase.1 : Drop dead xdatum */
+	for (i=0; i < XATTRINDEX_HASHSIZE; i++) {
+		list_for_each_entry_safe(xd, _xd, &c->xattrindex[i], xindex) {
+			BUG_ON(xd->node == (void *)xd);
+			if (is_xattr_datum_dead(xd)) {
+				list_del_init(&xd->xindex);
+				list_add(&xd->xindex, &c->xattr_unchecked);
+			}
+		}
+	}
 
-	/* Phase.1 */
+	/* Phase.2 : Merge same xref */
+	for (i=0; i < XREF_TMPHASH_SIZE; i++)
+		xref_tmphash[i] = NULL;
 	for (ref=c->xref_temp; ref; ref=_ref) {
+		struct jffs2_xattr_ref *tmp;
+
 		_ref = ref->next;
-		/* checking REF_UNCHECKED nodes */
 		if (ref_flags(ref->node) != REF_PRISTINE) {
 			if (verify_xattr_ref(c, ref)) {
-				delete_xattr_ref_node(c, ref);
+				BUG_ON(ref->node->next_in_ino != (void *)ref);
+				ref->node->next_in_ino = NULL;
+				jffs2_mark_node_obsolete(c, ref->node);
 				jffs2_free_xattr_ref(ref);
 				continue;
 			}
 		}
-		/* At this point, ref->xid and ref->ino contain XID and inode number.
-		   ref->xd and ref->ic are not valid yet. */
-		xd = jffs2_find_xattr_datum(c, ref->xid);
-		ic = jffs2_get_ino_cache(c, ref->ino);
-		if (!xd || !ic) {
-			if (ref_flags(ref->node) != REF_UNCHECKED)
-				JFFS2_WARNING("xref(ino=%u, xid=%u) is orphan. \n",
-					      ref->ino, ref->xid);
-			delete_xattr_ref_node(c, ref);
+
+		i = (ref->ino ^ ref->xid) % XREF_TMPHASH_SIZE;
+		for (tmp=xref_tmphash[i]; tmp; tmp=tmp->next) {
+			if (tmp->ino == ref->ino && tmp->xid == ref->xid)
+				break;
+		}
+		if (tmp) {
+			raw = ref->node;
+			if (ref->xseqno > tmp->xseqno) {
+				tmp->xseqno = ref->xseqno;
+				raw->next_in_ino = tmp->node;
+				tmp->node = raw;
+			} else {
+				raw->next_in_ino = tmp->node->next_in_ino;
+				tmp->node->next_in_ino = raw;
+			}
 			jffs2_free_xattr_ref(ref);
 			continue;
+		} else {
+			ref->next = xref_tmphash[i];
+			xref_tmphash[i] = ref;
 		}
-		ref->xd = xd;
-		ref->ic = ic;
-		xd->refcnt++;
-		ref->next = ic->xref;
-		ic->xref = ref;
-		xref_count++;
 	}
 	c->xref_temp = NULL;
-	/* After this, ref->xid/ino are NEVER used. */
 
-	/* Phase.2 */
+	/* Phase.3 : Bind xref with inode_cache and xattr_datum */
+	for (i=0; i < XREF_TMPHASH_SIZE; i++) {
+		for (ref=xref_tmphash[i]; ref; ref=_ref) {
+			_ref = ref->next;
+			if (is_xattr_ref_dead(ref)) {
+				ref->next = c->xref_dead_list;
+				c->xref_dead_list = ref;
+				continue;
+			}
+			/* At this point, ref->xid and ref->ino contain XID and inode number.
+			   ref->xd and ref->ic are not valid yet. */
+			xd = jffs2_find_xattr_datum(c, ref->xid);
+			ic = jffs2_get_ino_cache(c, ref->ino);
+			if (!xd || !ic) {
+				JFFS2_WARNING("xref(ino=%u, xid=%u, xseqno=%u) is orphan. \n",
+					      ref->ino, ref->xid, ref->xseqno);
+				set_xattr_ref_dead(ref);
+				ref->next = c->xref_dead_list;
+				c->xref_dead_list = ref;
+				continue;
+			}
+			ref->xd = xd;
+			ref->ic = ic;
+			xd->refcnt++;
+			ref->next = ic->xref;
+			ic->xref = ref;
+			xref_count++;
+		}
+	}
+
+	/* Phase.4 : Link unchecked xdatum to xattr_unchecked list */
 	for (i=0; i < XATTRINDEX_HASHSIZE; i++) {
 		list_for_each_entry_safe(xd, _xd, &c->xattrindex[i], xindex) {
 			list_del_init(&xd->xindex);
 			if (!xd->refcnt) {
-				if (ref_flags(xd->node) != REF_UNCHECKED)
-					JFFS2_WARNING("orphan xdatum(xid=%u, version=%u) at %#08x\n",
-						      xd->xid, xd->version, ref_offset(xd->node));
-				delete_xattr_datum(c, xd);
+				JFFS2_WARNING("orphan xdatum(xid=%u, version=%u)\n",
+					      xd->xid, xd->version);
+				set_xattr_datum_dead(xd);
+				list_add(&xd->xindex, &c->xattr_unchecked);
 				continue;
 			}
-			if (ref_flags(xd->node) != REF_PRISTINE) {
-				dbg_xattr("unchecked xdatum(xid=%u) at %#08x\n",
-					  xd->xid, ref_offset(xd->node));
+			if (is_xattr_datum_unchecked(c, xd)) {
+				dbg_xattr("unchecked xdatum(xid=%u, version=%u)\n",
+					  xd->xid, xd->version);
 				list_add(&xd->xindex, &c->xattr_unchecked);
 				xdatum_unchecked_count++;
 			}
@@ -833,28 +977,18 @@ void jffs2_build_xattr_subsystem(struct 
 struct jffs2_xattr_datum *jffs2_setup_xattr_datum(struct jffs2_sb_info *c,
 						  uint32_t xid, uint32_t version)
 {
-	struct jffs2_xattr_datum *xd, *_xd;
+	struct jffs2_xattr_datum *xd;
 
-	_xd = jffs2_find_xattr_datum(c, xid);
-	if (_xd) {
-		dbg_xattr("duplicate xdatum (xid=%u, version=%u/%u) at %#08x\n",
-			  xid, version, _xd->version, ref_offset(_xd->node));
-		if (version < _xd->version)
-			return ERR_PTR(-EEXIST);
-	}
-	xd = jffs2_alloc_xattr_datum();
-	if (!xd)
-		return ERR_PTR(-ENOMEM);
-	xd->xid = xid;
-	xd->version = version;
-	if (xd->xid > c->highest_xid)
-		c->highest_xid = xd->xid;
-	list_add_tail(&xd->xindex, &c->xattrindex[xid % XATTRINDEX_HASHSIZE]);
-
-	if (_xd) {
-		list_del_init(&_xd->xindex);
-		delete_xattr_datum_node(c, _xd);
-		jffs2_free_xattr_datum(_xd);
+	xd = jffs2_find_xattr_datum(c, xid);
+	if (!xd) {
+		xd = jffs2_alloc_xattr_datum();
+		if (!xd)
+			return ERR_PTR(-ENOMEM);
+		xd->xid = xid;
+		xd->version = version;
+		if (xd->xid > c->highest_xid)
+			c->highest_xid = xd->xid;
+		list_add_tail(&xd->xindex, &c->xattrindex[xid % XATTRINDEX_HASHSIZE]);
 	}
 	return xd;
 }
@@ -945,7 +1079,7 @@ ssize_t jffs2_listxattr(struct dentry *d
 				rc = load_xattr_datum(c, xd);
 				if (unlikely(rc > 0)) {
 					*pref = ref->next;
-					delete_xattr_ref(c, ref);
+					delete_xattr_ref_delay(c, ref);
 					goto retry;
 				} else if (unlikely(rc < 0))
 					goto out;
@@ -1006,7 +1140,7 @@ int do_jffs2_getxattr(struct inode *inod
 				rc = load_xattr_datum(c, xd);
 				if (unlikely(rc > 0)) {
 					*pref = ref->next;
-					delete_xattr_ref(c, ref);
+					delete_xattr_ref_delay(c, ref);
 					goto retry;
 				} else if (unlikely(rc < 0)) {
 					goto out;
@@ -1069,7 +1203,7 @@ int do_jffs2_setxattr(struct inode *inod
 			rc = load_xattr_datum(c, xd);
 			if (unlikely(rc > 0)) {
 				*pref = ref->next;
-				delete_xattr_ref(c, ref);
+				delete_xattr_ref_delay(c, ref);
 				goto retry;
 			} else if (unlikely(rc < 0))
 				goto out;
@@ -1081,8 +1215,7 @@ int do_jffs2_setxattr(struct inode *inod
 			}
 			if (!buffer) {
 				*pref = ref->next;
-				delete_xattr_ref(c, ref);
-				rc = 0;
+				rc = delete_xattr_ref(c, ref, 0);
 				goto out;
 			}
 			goto found;
@@ -1094,7 +1227,7 @@ int do_jffs2_setxattr(struct inode *inod
 		goto out;
 	}
 	if (!buffer) {
-		rc = -EINVAL;
+		rc = -ENODATA;
 		goto out;
 	}
  found:
@@ -1110,16 +1243,15 @@ int do_jffs2_setxattr(struct inode *inod
 	request = PAD(sizeof(struct jffs2_raw_xref));
 	rc = jffs2_reserve_space(c, request, &length,
 				 ALLOC_NORMAL, JFFS2_SUMMARY_XREF_SIZE);
+	down_write(&c->xattr_sem);
 	if (rc) {
 		JFFS2_WARNING("jffs2_reserve_space()=%d, request=%u\n", rc, request);
-		down_write(&c->xattr_sem);
 		xd->refcnt--;
 		if (!xd->refcnt)
-			delete_xattr_datum(c, xd);
+			delete_xattr_datum_delay(c, xd);
 		up_write(&c->xattr_sem);
 		return rc;
 	}
-	down_write(&c->xattr_sem);
 	if (ref)
 		*pref = ref->next;
 	newref = create_xattr_ref(c, ic, xd);
@@ -1131,9 +1263,21 @@ int do_jffs2_setxattr(struct inode *inod
 		rc = PTR_ERR(newref);
 		xd->refcnt--;
 		if (!xd->refcnt)
-			delete_xattr_datum(c, xd);
+			delete_xattr_datum_delay(c, xd);
 	} else if (ref) {
-		delete_xattr_ref(c, ref);
+		up_write(&c->xattr_sem);
+		jffs2_complete_reservation(c);
+
+		rc = jffs2_reserve_space(c, request, &length,
+					 ALLOC_DELETION, JFFS2_SUMMARY_XREF_SIZE);
+		down_write(&c->xattr_sem);
+		if (rc) {
+			JFFS2_WARNING("jffs2_reserve_space()=%d, request=%u\n", rc, request);
+			delete_xattr_ref_delay(c, ref);
+			up_write(&c->xattr_sem);
+			return 0;
+		}
+		delete_xattr_ref(c, ref, 1);
 	}
  out:
 	up_write(&c->xattr_sem);
@@ -1142,38 +1286,37 @@ int do_jffs2_setxattr(struct inode *inod
 }
 
 /* -------- garbage collector functions -------------
- * jffs2_garbage_collect_xattr_datum(c, xd)
+ * jffs2_garbage_collect_xattr_datum(c, xd, raw)
  *   is used to move xdatum into new node.
- * jffs2_garbage_collect_xattr_ref(c, ref)
+ * jffs2_garbage_collect_xattr_ref(c, ref, raw)
  *   is used to move xref into new node.
  * jffs2_verify_xattr(c)
  *   is used to call do_verify_xattr_datum() before garbage collecting.
  * -------------------------------------------------- */
-int jffs2_garbage_collect_xattr_datum(struct jffs2_sb_info *c, struct jffs2_xattr_datum *xd)
+int jffs2_garbage_collect_xattr_datum(struct jffs2_sb_info *c, struct jffs2_xattr_datum *xd,
+				      struct jffs2_raw_node_ref *raw)
 {
 	uint32_t totlen, length, old_ofs;
-	int rc = -EINVAL;
+	int rc = 0;
 
 	down_write(&c->xattr_sem);
-	BUG_ON(!xd->node);
+	if (xd->node != raw)
+		goto out;
+	if (is_xattr_datum_dead(xd) && (raw->next_in_ino == (void *)xd))
+		goto out;
 
 	old_ofs = ref_offset(xd->node);
 	totlen = ref_totlen(c, c->gcblock, xd->node);
-	if (totlen < sizeof(struct jffs2_raw_xattr))
-		goto out;
 
-	if (!xd->xname) {
+	if (!is_xattr_datum_dead(xd)) {
 		rc = load_xattr_datum(c, xd);
-		if (unlikely(rc > 0)) {
-			delete_xattr_datum_node(c, xd);
-			rc = 0;
-			goto out;
-		} else if (unlikely(rc < 0))
+		if (unlikely(rc < 0))
 			goto out;
 	}
+
 	rc = jffs2_reserve_space_gc(c, totlen, &length, JFFS2_SUMMARY_XATTR_SIZE);
-	if (rc || length < totlen) {
-		JFFS2_WARNING("jffs2_reserve_space()=%d, request=%u\n", rc, totlen);
+	if (rc) {
+		JFFS2_WARNING("jffs2_reserve_space_gc()=%d, request=%u\n", rc, totlen);
 		rc = rc ? rc : -EBADFD;
 		goto out;
 	}
@@ -1182,27 +1325,33 @@ int jffs2_garbage_collect_xattr_datum(st
 		dbg_xattr("xdatum (xid=%u, version=%u) GC'ed from %#08x to %08x\n",
 			  xd->xid, xd->version, old_ofs, ref_offset(xd->node));
  out:
+	if (!rc)
+		jffs2_mark_node_obsolete(c, raw);
 	up_write(&c->xattr_sem);
 	return rc;
 }
 
 
-int jffs2_garbage_collect_xattr_ref(struct jffs2_sb_info *c, struct jffs2_xattr_ref *ref)
+int jffs2_garbage_collect_xattr_ref(struct jffs2_sb_info *c, struct jffs2_xattr_ref *ref,
+				    struct jffs2_raw_node_ref *raw)
 {
 	uint32_t totlen, length, old_ofs;
-	int rc = -EINVAL;
+	int rc = 0;
 
 	down_write(&c->xattr_sem);
 	BUG_ON(!ref->node);
 
+	if (ref->node != raw)
+		goto out;
+	if (is_xattr_ref_dead(ref) && (raw->next_in_ino == (void *)ref))
+		goto out;
+
 	old_ofs = ref_offset(ref->node);
 	totlen = ref_totlen(c, c->gcblock, ref->node);
-	if (totlen != sizeof(struct jffs2_raw_xref))
-		goto out;
 
 	rc = jffs2_reserve_space_gc(c, totlen, &length, JFFS2_SUMMARY_XREF_SIZE);
-	if (rc || length < totlen) {
-		JFFS2_WARNING("%s: jffs2_reserve_space() = %d, request = %u\n",
+	if (rc) {
+		JFFS2_WARNING("%s: jffs2_reserve_space_gc() = %d, request = %u\n",
 			      __FUNCTION__, rc, totlen);
 		rc = rc ? rc : -EBADFD;
 		goto out;
@@ -1212,6 +1361,8 @@ int jffs2_garbage_collect_xattr_ref(stru
 		dbg_xattr("xref (ino=%u, xid=%u) GC'ed from %#08x to %08x\n",
 			  ref->ic->ino, ref->xd->xid, old_ofs, ref_offset(ref->node));
  out:
+	if (!rc)
+		jffs2_mark_node_obsolete(c, raw);
 	up_write(&c->xattr_sem);
 	return rc;
 }
@@ -1219,20 +1370,59 @@ int jffs2_garbage_collect_xattr_ref(stru
 int jffs2_verify_xattr(struct jffs2_sb_info *c)
 {
 	struct jffs2_xattr_datum *xd, *_xd;
+	struct jffs2_eraseblock *jeb;
+	struct jffs2_raw_node_ref *raw;
+	uint32_t totlen;
 	int rc;
 
 	down_write(&c->xattr_sem);
 	list_for_each_entry_safe(xd, _xd, &c->xattr_unchecked, xindex) {
 		rc = do_verify_xattr_datum(c, xd);
-		if (rc == 0) {
-			list_del_init(&xd->xindex);
-			break;
-		} else if (rc > 0) {
-			list_del_init(&xd->xindex);
-			delete_xattr_datum_node(c, xd);
+		if (rc < 0)
+			continue;
+		list_del_init(&xd->xindex);
+		spin_lock(&c->erase_completion_lock);
+		for (raw=xd->node; raw != (void *)xd; raw=raw->next_in_ino) {
+			if (ref_flags(raw) != REF_UNCHECKED)
+				continue;
+			jeb = &c->blocks[ref_offset(raw) / c->sector_size];
+			totlen = PAD(ref_totlen(c, jeb, raw));
+			c->unchecked_size -= totlen; c->used_size += totlen;
+			jeb->unchecked_size -= totlen; jeb->used_size += totlen;
+			raw->flash_offset = ref_offset(raw)
+				| ((xd->node == (void *)raw) ? REF_PRISTINE : REF_NORMAL);
 		}
+		if (is_xattr_datum_dead(xd))
+			list_add(&xd->xindex, &c->xattr_dead_list);
+		spin_unlock(&c->erase_completion_lock);
 	}
 	up_write(&c->xattr_sem);
-
 	return list_empty(&c->xattr_unchecked) ? 1 : 0;
 }
+
+void jffs2_release_xattr_datum(struct jffs2_sb_info *c, struct jffs2_xattr_datum *xd)
+{
+	/* must be called under spin_lock(&c->erase_completion_lock) */
+	if (xd->node != (void *)xd)
+		return;
+
+	list_del(&xd->xindex);
+	jffs2_free_xattr_datum(xd);
+}
+
+void jffs2_release_xattr_ref(struct jffs2_sb_info *c, struct jffs2_xattr_ref *ref)
+{
+	/* must be called under spin_lock(&c->erase_completion_lock) */
+	struct jffs2_xattr_ref *tmp, **ptmp;
+
+	if (ref->node != (void *)ref)
+		return;
+
+	for (tmp=c->xref_dead_list, ptmp=&c->xref_dead_list; tmp; ptmp=&tmp->next, tmp=tmp->next) {
+		if (ref == tmp) {
+			*ptmp = tmp->next;
+			jffs2_free_xattr_ref(ref);
+			break;
+		}
+	}
+}
diff --git a/fs/jffs2/xattr.h b/fs/jffs2/xattr.h
index 2c19985..06ab7b8 100644
--- a/fs/jffs2/xattr.h
+++ b/fs/jffs2/xattr.h
@@ -16,6 +16,7 @@ #include <linux/list.h>
 
 #define JFFS2_XFLAGS_HOT	(0x01)	/* This datum is HOT */
 #define JFFS2_XFLAGS_BIND	(0x02)	/* This datum is not reclaimed */
+#define JFFS2_XFLAGS_INVALID	(0x80)	/* This datum contains crc error */
 
 struct jffs2_xattr_datum
 {
@@ -23,7 +24,7 @@ struct jffs2_xattr_datum
 	struct jffs2_raw_node_ref *node;
 	uint8_t class;
 	uint8_t flags;
-	uint16_t xprefix;			/* see JFFS2_XATTR_PREFIX_* */
+	uint16_t xprefix;		/* see JFFS2_XATTR_PREFIX_* */
 
 	struct list_head xindex;	/* chained from c->xattrindex[n] */
 	uint32_t refcnt;		/* # of xattr_ref refers this */
@@ -47,6 +48,7 @@ struct jffs2_xattr_ref
 	uint8_t flags;		/* Currently unused */
 	u16 unused;
 
+	uint32_t xseqno;
 	union {
 		struct jffs2_inode_cache *ic;	/* reference to jffs2_inode_cache */
 		uint32_t ino;			/* only used in scanning/building  */
@@ -58,6 +60,34 @@ struct jffs2_xattr_ref
 	struct jffs2_xattr_ref *next;		/* chained from ic->xref_list */
 };
 
+#define XDATUM_DELETE_MARKER	(0xffffffff)
+#define XREF_DELETE_MARKER	(0x00000001)
+static inline int is_xattr_datum_dead(struct jffs2_xattr_datum *xd)
+{
+	return (xd->version == XDATUM_DELETE_MARKER);
+}
+
+static inline void set_xattr_datum_dead(struct jffs2_xattr_datum *xd)
+{
+	xd->version = XDATUM_DELETE_MARKER;
+}
+
+static inline int is_xattr_ref_dead(struct jffs2_xattr_ref *ref)
+{
+	return ((ref->xseqno & XREF_DELETE_MARKER) != 0);
+}
+
+static inline void set_xattr_ref_dead(struct jffs2_xattr_ref *ref)
+{
+	ref->xseqno |= XREF_DELETE_MARKER;
+}
+
+static inline void clr_xattr_ref_dead(struct jffs2_xattr_ref *ref)
+{
+	ref->xseqno &= ~XREF_DELETE_MARKER;
+}
+
+
 #ifdef CONFIG_JFFS2_FS_XATTR
 
 extern void jffs2_init_xattr_subsystem(struct jffs2_sb_info *c);
@@ -70,9 +100,13 @@ extern struct jffs2_xattr_datum *jffs2_s
 extern void jffs2_xattr_delete_inode(struct jffs2_sb_info *c, struct jffs2_inode_cache *ic);
 extern void jffs2_xattr_free_inode(struct jffs2_sb_info *c, struct jffs2_inode_cache *ic);
 
-extern int jffs2_garbage_collect_xattr_datum(struct jffs2_sb_info *c, struct jffs2_xattr_datum *xd);
-extern int jffs2_garbage_collect_xattr_ref(struct jffs2_sb_info *c, struct jffs2_xattr_ref *ref);
+extern int jffs2_garbage_collect_xattr_datum(struct jffs2_sb_info *c, struct jffs2_xattr_datum *xd,
+					     struct jffs2_raw_node_ref *raw);
+extern int jffs2_garbage_collect_xattr_ref(struct jffs2_sb_info *c, struct jffs2_xattr_ref *ref,
+					   struct jffs2_raw_node_ref *raw);
 extern int jffs2_verify_xattr(struct jffs2_sb_info *c);
+extern void jffs2_release_xattr_datum(struct jffs2_sb_info *c, struct jffs2_xattr_datum *xd);
+extern void jffs2_release_xattr_ref(struct jffs2_sb_info *c, struct jffs2_xattr_ref *ref);
 
 extern int do_jffs2_getxattr(struct inode *inode, int xprefix, const char *xname,
 			     char *buffer, size_t size);

[-- Attachment #3: jffs2-xattr-v6-02-fix_posixacl_bug.patch --]
[-- Type: text/plain, Size: 332 bytes --]

diff --git a/fs/jffs2/acl.c b/fs/jffs2/acl.c
index 320dd48..9c2077e 100644
--- a/fs/jffs2/acl.c
+++ b/fs/jffs2/acl.c
@@ -267,6 +267,8 @@ static int jffs2_set_acl(struct inode *i
 	}
 
 	rc = do_jffs2_setxattr(inode, xprefix, "", value, size, 0);
+	if (!value && rc == -ENODATA)
+		rc = 0;
 	if (value)
 		kfree(value);
 	if (!rc) {

Re: JFFS2/xattr problems.

[David Woodhouse]

On Mon, 2006-06-12 at 11:17 +0900, KaiGai Kohei wrote:
> I fixed the problem when xdatum/xref was removed, and so on.
> 
> [1/2] jffs2-xattr-v6-01-delete_marker.patch
> [2/2] jffs2-xattr-v6-02-fix_posixacl_bug.patch

Thank you. I will look at these this morning.

I have an unrelated question.... what is the maximum size of an XATTR?
Am I right in thinking that it's 64KiB? That's rather large; we won't be
able to write it in a single node on some flash chips, where the
eraseblock size is smaller than that. Don't we need to allow multiple
nodes, to cover the range of an xdatum?

-- 
dwmw2

Re: JFFS2/xattr problems.

[KaiGai Kohei]

Hi,

> I have an unrelated question.... what is the maximum size of an XATTR?
> Am I right in thinking that it's 64KiB? That's rather large; we won't be
> able to write it in a single node on some flash chips, where the
> eraseblock size is smaller than that. Don't we need to allow multiple
> nodes, to cover the range of an xdatum?

Indeed, the maximum size of an XATTR is limited by eraseblock size.
As it may be smaller than XATTR_SIZE_MAX (=65536), we can't handle
very large xattr in the current implementation.
It was blind spot for me.

But is it actually needed ?
For example, the length of xattr in EXT2 implementation is limited
by block size which is 4096 at most. But nobody (probably) has claimed
yet.

In my opinion, a xattr across multiple nodes isn't neccesary.
What do you think?

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

Re: JFFS2/xattr problems.

[David Woodhouse]

On Mon, 2006-06-12 at 18:43 +0900, KaiGai Kohei wrote:
> In my opinion, a xattr across multiple nodes isn't neccesary.
> What do you think?

I'd be happier if it worked -- some of us have plans to abuse XATTR
support by building a simple database on it... :)

But as long as the failure mode is _graceful_, I suppose we can live
without it for now, if ext3 is also limited to 4KiB.

I'm looking through your handling of deletion now... I'm not sure that
we _need_ the physical deletion node for xdata, do we? Those can go away
just because there are no xrefs which link to them, just like our inodes
do?

Also, the physical deletion node for xrefs is only needed when the xattr
is deleted _without_ the inode being deleted. When the inode goes away,
again the xref becomes obsolete all by itself, right?

Finally, we need to be careful about garbage collection of deletion
nodes. Remember that the deletion node might be garbage-collected
_before_ the older node which it is deleting. So you must decide whether
to write the deletion node out again, when you are garbage collecting
it.

See the horrid mess we make of 'deletion dirents'.

-- 
dwmw2

Re: JFFS2/xattr problems.

[Jörn Engel]

On Mon, 12 June 2006 10:53:15 +0100, David Woodhouse wrote:
> On Mon, 2006-06-12 at 18:43 +0900, KaiGai Kohei wrote:
> > In my opinion, a xattr across multiple nodes isn't neccesary.
> > What do you think?
> 
> I'd be happier if it worked -- some of us have plans to abuse XATTR
> support by building a simple database on it... :)
> 
> But as long as the failure mode is _graceful_, I suppose we can live
> without it for now, if ext3 is also limited to 4KiB.

Seems you missed Ted's presentation at LCA this year.  Among the
interesting bits:
o Pretty much anything on Linux is limited to 64KiB or less.
o Ext[23] is limited to 4KiB total for all attributes, including all
  keys and all values.
o The biggest user of Alternate Streams (less-limited versions of
  xattr on Windows, Solaris, etc.) arguably is root kits.  Alternate
  Streams have the advantage that tripwire etc. don't understand them
  and won't look for malware there.
o Some system administrators have no plans to upgrade to Solaris 9
  ever, because it supports Alternate Streams.  The trouble of hidden
  malware is not worth the gains.

Notable was also, that Ted repeated the last two points in several
variations.  Not sure if I would follow his line of thought 100%, but
he does have a point.

Jörn

-- 
The competent programmer is fully aware of the strictly limited size of
his own skull; therefore he approaches the programming task in full
humility, and among other things he avoids clever tricks like the plague. 
-- Edsger W. Dijkstra

Re: JFFS2/xattr problems.

[KaiGai Kohei]

Hi, Jörn

> Seems you missed Ted's presentation at LCA this year.  Among the
> interesting bits:

If this presentation is public, could you tell me the URL?
This indication is highly suggestive for me.
Especially, I have not imagine yet the possibility that
malware uses xattr to hide itself.

Thanks,

> o Pretty much anything on Linux is limited to 64KiB or less.
> o Ext[23] is limited to 4KiB total for all attributes, including all
>   keys and all values.
> o The biggest user of Alternate Streams (less-limited versions of
>   xattr on Windows, Solaris, etc.) arguably is root kits.  Alternate
>   Streams have the advantage that tripwire etc. don't understand them
>   and won't look for malware there.
> o Some system administrators have no plans to upgrade to Solaris 9
>   ever, because it supports Alternate Streams.  The trouble of hidden
>   malware is not worth the gains.
> 
> Notable was also, that Ted repeated the last two points in several
> variations.  Not sure if I would follow his line of thought 100%, but
> he does have a point.
> 
> Jörn
-- 
KaiGai Kohei <kaigai@kaigai.gr.jp>

Re: JFFS2/xattr problems.

[Jörn Engel]

On Tue, 13 June 2006 22:36:59 +0900, KaiGai Kohei wrote:
> 
> >Seems you missed Ted's presentation at LCA this year.  Among the
> >interesting bits:
> 
> If this presentation is public, could you tell me the URL?
> This indication is highly suggestive for me.
> Especially, I have not imagine yet the possibility that
> malware uses xattr to hide itself.

I can only find the abstract:
http://lca2006.linux.org.au/abstract.php?id=384

[ adding Ted to Cc: ]

Ted, do still have your foils and can make them available?  Kaigai-san
is working on an xattr implementation for jffs2.

> >o Pretty much anything on Linux is limited to 64KiB or less.
> >o Ext[23] is limited to 4KiB total for all attributes, including all
> >  keys and all values.
> >o The biggest user of Alternate Streams (less-limited versions of
> >  xattr on Windows, Solaris, etc.) arguably is root kits.  Alternate
> >  Streams have the advantage that tripwire etc. don't understand them
> >  and won't look for malware there.
> >o Some system administrators have no plans to upgrade to Solaris 9
> >  ever, because it supports Alternate Streams.  The trouble of hidden
> >  malware is not worth the gains.
> >
> >Notable was also, that Ted repeated the last two points in several
> >variations.  Not sure if I would follow his line of thought 100%, but
> >he does have a point.

Jörn

-- 
Why do musicians compose symphonies and poets write poems?
They do it because life wouldn't have any meaning for them if they didn't.
That's why I draw cartoons.  It's my life.
-- Charles Shultz

Re: JFFS2/xattr problems.

[Theodore Tso]

On Tue, Jun 13, 2006 at 04:13:17PM +0200, Jörn Engel wrote:
> On Tue, 13 June 2006 22:36:59 +0900, KaiGai Kohei wrote:
> > 
> > >Seems you missed Ted's presentation at LCA this year.  Among the
> > >interesting bits:
> > 
> > If this presentation is public, could you tell me the URL?
> > This indication is highly suggestive for me.
> > Especially, I have not imagine yet the possibility that
> > malware uses xattr to hide itself.
> 
> I can only find the abstract:
> http://lca2006.linux.org.au/abstract.php?id=384
> 
> [ adding Ted to Cc: ]
> 
> Ted, do still have your foils and can make them available?  Kaigai-san
> is working on an xattr implementation for jffs2.

Sure, here you go (see attached)

> > >o The biggest user of Alternate Streams (less-limited versions of
> > >  xattr on Windows, Solaris, etc.) arguably is root kits.  Alternate
> > >  Streams have the advantage that tripwire etc. don't understand them
> > >  and won't look for malware there.
> > >o Some system administrators have no plans to upgrade to Solaris 9
> > >  ever, because it supports Alternate Streams.  The trouble of hidden
> > >  malware is not worth the gains.
> > >
> > >Notable was also, that Ted repeated the last two points in several
> > >variations.  Not sure if I would follow his line of thought 100%, but
> > >he does have a point.

See the article referenced in the slide, "Alternate Data Streams:
Threat or Menace?"

	http://www.awprofessional.com/articles/article.asp?p=413685

(I love the title.  "Threat or Menace?"  "Menace or Threat?"  Or, to
take a line from an old Bugs Bunny/Daffy Duck cartoon, "You got me
dead to rights, Doc.  Would you like to shoot him now or shoot him
later?"  :-)

						- Ted

Re: JFFS2/xattr problems.

[Jörn Engel]

On Wed, 14 June 2006 17:58:35 -0400, Theodore Tso wrote:
> On Tue, Jun 13, 2006 at 04:13:17PM +0200, Jörn Engel wrote:
> > 
> > Ted, do still have your foils and can make them available?  Kaigai-san
> > is working on an xattr implementation for jffs2.
> 
> Sure, here you go (see attached)

The attachment gnomes must have stolen it right from your fingers.
Pesky little bastards.

> (I love the title.  "Threat or Menace?"  "Menace or Threat?"  Or, to
> take a line from an old Bugs Bunny/Daffy Duck cartoon, "You got me
> dead to rights, Doc.  Would you like to shoot him now or shoot him
> later?"  :-)

:)

Reading the contents was rather amusing as well.  Telling to user to
look for Alternate Streams > 256 Bytes - but you can ignore images, as
they only contain thumbnails.  Well, most of them at least.

Jörn

-- 
All art is but imitation of nature.
-- Lucius Annaeus Seneca

Re: JFFS2/xattr problems.

[Theodore Tso]

[-- Attachment #1: Type: text/plain, Size: 515 bytes --]

On Thu, Jun 15, 2006 at 01:47:51PM +0200, Jörn Engel wrote:
> On Wed, 14 June 2006 17:58:35 -0400, Theodore Tso wrote:
> > On Tue, Jun 13, 2006 at 04:13:17PM +0200, Jörn Engel wrote:
> > > 
> > > Ted, do still have your foils and can make them available?  Kaigai-san
> > > is working on an xattr implementation for jffs2.
> > 
> > Sure, here you go (see attached)
> 
> The attachment gnomes must have stolen it right from your fingers.
> Pesky little bastards.

Oops, sorry about that.  Here you go.

							- Ted


[-- Attachment #2: forkdepot.odp --]
[-- Type: application/vnd.oasis.opendocument.presentation, Size: 591057 bytes --]

Re: JFFS2/xattr problems.

[KaiGai Kohei]

Hi,

>>In my opinion, a xattr across multiple nodes isn't neccesary.
>>What do you think?
> 
> 
> I'd be happier if it worked -- some of us have plans to abuse XATTR
> support by building a simple database on it... :)
> 
> But as long as the failure mode is _graceful_, I suppose we can live
> without it for now, if ext3 is also limited to 4KiB.

I want to suspend this requirement if it's not imperative.
Probably, this design changing will involve so many fixes...

> I'm looking through your handling of deletion now... I'm not sure that
> we _need_ the physical deletion node for xdata, do we? Those can go away
> just because there are no xrefs which link to them, just like our inodes
> do?
> 
> Also, the physical deletion node for xrefs is only needed when the xattr
> is deleted _without_ the inode being deleted. When the inode goes away,
> again the xref becomes obsolete all by itself, right?

Indeed, you are right.
In mounting process, we can certainly detect xdatum no xref linked and
xref no inode linked. I'll fix its design, so please wait for a while.

> Finally, we need to be careful about garbage collection of deletion
> nodes. Remember that the deletion node might be garbage-collected
> _before_ the older node which it is deleting. So you must decide whether
> to write the deletion node out again, when you are garbage collecting
> it.

Don't worry.
When any other nodes are linked with xdatum or xref, the deletion nodes
are not removed in this garbage collect cycle.

Thanks,
-- 
KaiGai Kohei <kaigai@kaigai.gr.jp>

Re: JFFS2/xattr problems.

[KaiGai Kohei]

[-- Attachment #1: Type: text/plain, Size: 1977 bytes --]

>>I'm looking through your handling of deletion now... I'm not sure that
>>we _need_ the physical deletion node for xdata, do we? Those can go away
>>just because there are no xrefs which link to them, just like our inodes
>>do?
>>
>>Also, the physical deletion node for xrefs is only needed when the xattr
>>is deleted _without_ the inode being deleted. When the inode goes away,
>>again the xref becomes obsolete all by itself, right?
> 
> 
> Indeed, you are right.
> In mounting process, we can certainly detect xdatum no xref linked and
> xref no inode linked. I'll fix its design, so please wait for a while.

I'm sorry to have kept you waiting so long.
The following patches 01-03 modifies its design, and 04 fixes trivial
wrong copyright, 05 enables to use xattr with write buffering support.

Those have been pushed jffs2-xattr-2.6.git yet.

[1/5] jffs2-xattr-v6.1-01-rid-unnecessary-writing-of-delete_marker.patch
   This patch rids writing physical delete marker when we delete xdatum
   and xref with no inode linked. We can certainlly detect those obsolete
   nodes, so those writings are not necessary.

[2/5] jffs2-xattr-v6.1-02-fix-memory-leak-jffs2_xattr_ref.patch
   This patch fixes a problem with memory leak in jffs2_xattr_ref.
   This problem became obvious by the above patch, but it has
   a possibility to cause.

[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.

[4/5] jffs2-xattr-v6.1-04-fix-copyright.patch
   I modified summary.c at 2006, not 2005.

[5/5] jffs2-xattr-v6.1-05-coexistence-between-xattr-and-write-buffering.patch
   drops '&& !JFFS2_FS_WRITEBUFFER' from fs/Kconfig.

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

[-- Attachment #2: jffs2-xattr-v6.1-01-rid-unnecessary-writing-of-delete_marker.patch --]
[-- Type: text/plain, Size: 22302 bytes --]

diff-tree 00c6cf2b8998389c8c04a5585b54902227ceb528 (from 0f42cbcf9f12cf3d9379a4c744da583e8809f03a)
Author: KaiGai Kohei <kaigai@ak.jp.nec.com>
Date:   Fri Jun 23 22:34:51 2006 +0900

    [JFFS2][XATTR] rid unnecessary writing of delete marker.
    
    In the followinf situation, an explicit delete marker is not
    necessary, because we can certainlly detect those obsolete
    xattr_datum or xattr_ref on next mounting.
    
    - When to delete xattr_datum node.
    - When to delete xattr_ref node on removing inode.
    - When to delete xattr_ref node on updating xattr.
    
    This patch rids writing delete marker in those situations.
    
    Signed-off-by: KaiGai Kohei <kaigai@ak.jp.nec.com>

diff --git a/fs/jffs2/scan.c b/fs/jffs2/scan.c
index 79638f5..2bfdc33 100644
--- a/fs/jffs2/scan.c
+++ b/fs/jffs2/scan.c
@@ -332,10 +332,8 @@ static int jffs2_scan_xattr_node(struct 
 	xid = je32_to_cpu(rx->xid);
 	version = je32_to_cpu(rx->version);
 
-	totlen = sizeof(struct jffs2_raw_xattr);
-	if (version != XDATUM_DELETE_MARKER)
-		totlen += rx->name_len + 1 + je16_to_cpu(rx->value_len);
-	totlen = PAD(totlen);
+	totlen = PAD(sizeof(struct jffs2_raw_xattr)
+			+ rx->name_len + 1 + je16_to_cpu(rx->value_len));
 	if (totlen != je32_to_cpu(rx->totlen)) {
 		JFFS2_WARNING("node length mismatch at %#08x, read=%u, calc=%u\n",
 			      ofs, je32_to_cpu(rx->totlen), totlen);
diff --git a/fs/jffs2/xattr.c b/fs/jffs2/xattr.c
index 03871ab..7622f79 100644
--- a/fs/jffs2/xattr.c
+++ b/fs/jffs2/xattr.c
@@ -50,12 +50,9 @@ #include "nodelist.h"
  *   is used to write xdatum to medium. xd->version will be incremented.
  * create_xattr_datum(c, xprefix, xname, xvalue, xsize)
  *   is used to create new xdatum and write to medium.
- * delete_xattr_datum_delay(c, xd)
- *   is used to delete a xdatum without 'delete marker'. It has a possibility to detect
- *   orphan xdatum on next mounting.
  * delete_xattr_datum(c, xd)
- *   is used to delete a xdatum with 'delete marker'. Calling jffs2_reserve_space() is
- *   necessary before this function.
+ *   is used to delete a xdatum. It marks xd JFFS2_XFLAGS_DEAD, and allows
+ *   GC to reclaim those physical nodes.
  * -------------------------------------------------- */
 static uint32_t xattr_datum_hashkey(int xprefix, const char *xname, const char *xvalue, int xsize)
 {
@@ -154,10 +151,7 @@ static int do_verify_xattr_datum(struct 
 		xd->flags |= JFFS2_XFLAGS_INVALID;
 		return EIO;
 	}
-	totlen = sizeof(rx);
-	if (xd->version != XDATUM_DELETE_MARKER)
-		totlen += rx.name_len + 1 + je16_to_cpu(rx.value_len);
-	totlen = PAD(totlen);
+	totlen = PAD(sizeof(rx) + rx.name_len + 1 + je16_to_cpu(rx.value_len));
 	if (je16_to_cpu(rx.magic) != JFFS2_MAGIC_BITMASK
 	    || je16_to_cpu(rx.nodetype) != JFFS2_NODETYPE_XATTR
 	    || je32_to_cpu(rx.totlen) != totlen
@@ -268,6 +262,7 @@ static int load_xattr_datum(struct jffs2
 	 */
 	int rc = 0;
 
+	BUG_ON(xd->flags & JFFS2_XFLAGS_DEAD);
 	if (xd->xname)
 		return 0;
 	if (xd->flags & JFFS2_XFLAGS_INVALID)
@@ -285,20 +280,18 @@ static int save_xattr_datum(struct jffs2
 	struct jffs2_raw_xattr rx;
 	struct kvec vecs[2];
 	size_t length;
-	int rc, totlen, nvecs = 1;
+	int rc, totlen;
 	uint32_t phys_ofs = write_ofs(c);
 
-	BUG_ON(is_xattr_datum_dead(xd) || (xd->flags & JFFS2_XFLAGS_INVALID)
-	       ? !!xd->xname : !xd->xname);
+	BUG_ON(!xd->xname);
+	BUG_ON(xd->flags & (JFFS2_XFLAGS_DEAD|JFFS2_XFLAGS_INVALID));
 
 	vecs[0].iov_base = &rx;
-	vecs[0].iov_len = totlen = sizeof(rx);
-	if (!is_xattr_datum_dead(xd) && !(xd->flags & JFFS2_XFLAGS_INVALID)) {
-		nvecs++;
-		vecs[1].iov_base = xd->xname;
-		vecs[1].iov_len = xd->name_len + 1 + xd->value_len;
-		totlen += vecs[1].iov_len;
-	}
+	vecs[0].iov_len = sizeof(rx);
+	vecs[1].iov_base = xd->xname;
+	vecs[1].iov_len = xd->name_len + 1 + xd->value_len;
+	totlen = vecs[0].iov_len + vecs[1].iov_len;
+
 	/* Setup raw-xattr */
 	memset(&rx, 0, sizeof(rx));
 	rx.magic = cpu_to_je16(JFFS2_MAGIC_BITMASK);
@@ -307,18 +300,14 @@ static int save_xattr_datum(struct jffs2
 	rx.hdr_crc = cpu_to_je32(crc32(0, &rx, sizeof(struct jffs2_unknown_node) - 4));
 
 	rx.xid = cpu_to_je32(xd->xid);
-	if (!is_xattr_datum_dead(xd) && !(xd->flags & JFFS2_XFLAGS_INVALID)) {
-		rx.version = cpu_to_je32(++xd->version);
-		rx.xprefix = xd->xprefix;
-		rx.name_len = xd->name_len;
-		rx.value_len = cpu_to_je16(xd->value_len);
-		rx.data_crc = cpu_to_je32(crc32(0, vecs[1].iov_base, vecs[1].iov_len));
-	} else {
-		rx.version = cpu_to_je32(XDATUM_DELETE_MARKER);
-	}
+	rx.version = cpu_to_je32(++xd->version);
+	rx.xprefix = xd->xprefix;
+	rx.name_len = xd->name_len;
+	rx.value_len = cpu_to_je16(xd->value_len);
+	rx.data_crc = cpu_to_je32(crc32(0, vecs[1].iov_base, vecs[1].iov_len));
 	rx.node_crc = cpu_to_je32(crc32(0, &rx, sizeof(struct jffs2_raw_xattr) - 4));
 
-	rc = jffs2_flash_writev(c, vecs, nvecs, phys_ofs, &length, 0);
+	rc = jffs2_flash_writev(c, vecs, 2, phys_ofs, &length, 0);
 	if (rc || totlen != length) {
 		JFFS2_WARNING("jffs2_flash_writev()=%d, req=%u, wrote=%zu, at %#08x\n",
 			      rc, totlen, length, phys_ofs);
@@ -405,53 +394,36 @@ static struct jffs2_xattr_datum *create_
 	return xd;
 }
 
-static void delete_xattr_datum_delay(struct jffs2_sb_info *c, struct jffs2_xattr_datum *xd)
+static void delete_xattr_datum(struct jffs2_sb_info *c, struct jffs2_xattr_datum *xd)
 {
 	/* must be called under down_write(xattr_sem) */
 	BUG_ON(xd->refcnt);
 
 	unload_xattr_datum(c, xd);
-	set_xattr_datum_dead(xd);
+	xd->flags |= JFFS2_XFLAGS_DEAD;
 	spin_lock(&c->erase_completion_lock);
-	list_add(&xd->xindex, &c->xattr_dead_list);
-	spin_unlock(&c->erase_completion_lock);
-	JFFS2_NOTICE("xdatum(xid=%u) was removed without delete marker. "
-		     "An orphan xdatum may be detected on next mounting.\n", xd->xid);
-}
-
-static void delete_xattr_datum(struct jffs2_sb_info *c, struct jffs2_xattr_datum *xd)
-{
-	/* must be called under jffs2_reserve_space() and down_write(xattr_sem) */
-	int rc;
-	BUG_ON(xd->refcnt);
-
-	unload_xattr_datum(c, xd);
-	set_xattr_datum_dead(xd);	
-	rc = save_xattr_datum(c, xd);
-	if (rc) {
-		JFFS2_NOTICE("xdatum(xid=%u) was removed without delete marker. "
-			     "An orphan xdatum may be detected on next mounting.\n",
-			     xd->xid);
+	if (xd->node == (void *)xd) {
+		BUG_ON(!(xd->flags & JFFS2_XFLAGS_INVALID));
+		jffs2_free_xattr_datum(xd);
+	} else {
+		list_add(&xd->xindex, &c->xattr_dead_list);
 	}
-	spin_lock(&c->erase_completion_lock);
-	list_add(&xd->xindex, &c->xattr_dead_list);
 	spin_unlock(&c->erase_completion_lock);
+	dbg_xattr("xdatum(xid=%u, version=%u) was removed.\n", xd->xid, xd->version);
 }
 
 /* -------- xref related functions ------------------
  * verify_xattr_ref(c, ref)
  *   is used to load xref information from medium. Because summary data does not
  *   contain xid/ino, it's necessary to verify once while mounting process.
- * delete_xattr_ref_node(c, ref)
- *   is used to delete a jffs2 node is dominated by xref. When EBS is enabled,
- *   it overwrites the obsolete node by myself. 
- * delete_xattr_ref(c, ref)
- *   is used to delete jffs2_xattr_ref object. If the reference counter of xdatum
- *   is refered by this xref become 0, delete_xattr_datum() is called later.
  * save_xattr_ref(c, ref)
- *   is used to write xref to medium.
+ *   is used to write xref to medium. If delete marker is marked, it write
+ *   a delete marker of xref into medium.
  * create_xattr_ref(c, ic, xd)
  *   is used to create a new xref and write to medium.
+ * delete_xattr_ref(c, ref)
+ *   is used to delete jffs2_xattr_ref. It marks xref XREF_DELETE_MARKER,
+ *   and allows GC to reclaim those physical nodes.
  * jffs2_xattr_delete_inode(c, ic)
  *   is called to remove xrefs related to obsolete inode when inode is unlinked.
  * jffs2_xattr_free_inode(c, ic)
@@ -591,13 +563,13 @@ static struct jffs2_xattr_ref *create_xa
 	return ref; /* success */
 }
 
-static void delete_xattr_ref_delay(struct jffs2_sb_info *c, struct jffs2_xattr_ref *ref)
+static void delete_xattr_ref(struct jffs2_sb_info *c, struct jffs2_xattr_ref *ref)
 {
 	/* must be called under down_write(xattr_sem) */
 	struct jffs2_xattr_datum *xd;
 
-	set_xattr_ref_dead(ref);
 	xd = ref->xd;
+	ref->xseqno |= XREF_DELETE_MARKER;
 	ref->ino = ref->ic->ino;
 	ref->xid = ref->xd->xid;
 	spin_lock(&c->erase_completion_lock);
@@ -605,102 +577,29 @@ static void delete_xattr_ref_delay(struc
 	c->xref_dead_list = ref;
 	spin_unlock(&c->erase_completion_lock);
 
-	JFFS2_NOTICE("xref(ino=%u, xid=%u) was removed without delete marker. "
-		     "An orphan xref may be detected on next mounting.\n",
-		     ref->ino, ref->xid);
+	dbg_xattr("xref(ino=%u, xid=%u, xseqno=%u) was removed.\n",
+		  ref->ino, ref->xid, ref->xseqno);
 
 	if (!--xd->refcnt)
-		delete_xattr_datum_delay(c, xd);
-}
-
-static int delete_xattr_ref(struct jffs2_sb_info *c, struct jffs2_xattr_ref *ref, int enforce)
-{
-	/* must be called under jffs2_reserve_space() and down_write(xattr_sem) */
-	struct jffs2_inode_cache *ic;
-	struct jffs2_xattr_datum *xd;
-	uint32_t length;
-	int rc;
-
-	set_xattr_ref_dead(ref);
-	ic = ref->ic;
-	xd = ref->xd;
-	ref->ino = ic->ino;
-	ref->xid = xd->xid;
-	rc = save_xattr_ref(c, ref);
-	if (rc) {
-		if (!enforce) {
-			clr_xattr_ref_dead(ref);
-			ref->ic = ic;
-			ref->xd = xd;
-			return rc;
-		}
-		JFFS2_WARNING("could not write delete marker of xref(ino=%u, xid=%u). "
-			      "An orphan xref may be detected on next mounting.\n",
-			      ref->ic->ino, ref->xd->xid);
-	}
-	spin_lock(&c->erase_completion_lock);
-	ref->next = c->xref_dead_list;
-	c->xref_dead_list = ref;
-	spin_unlock(&c->erase_completion_lock);
-
-	xd->refcnt--;
-	if (xd->refcnt)
-		return 0;
-
-	/* delete xdatum */
-	unload_xattr_datum(c, xd);
-	up_write(&c->xattr_sem);
-	jffs2_complete_reservation(c);
-
-	rc = jffs2_reserve_space(c, PAD(sizeof(struct jffs2_raw_xattr)), &length,
-				 ALLOC_DELETION, JFFS2_SUMMARY_XATTR_SIZE);
-	if (rc) {
-		down(&c->alloc_sem);
-		down_write(&c->xattr_sem);
-		delete_xattr_datum_delay(c, xd);
-	} else {
-		down_write(&c->xattr_sem);
 		delete_xattr_datum(c, xd);
-	}
-	return 0;
 }
 
 void jffs2_xattr_delete_inode(struct jffs2_sb_info *c, struct jffs2_inode_cache *ic)
 {
 	/* It's called from jffs2_clear_inode() on inode removing.
 	   When an inode with XATTR is removed, those XATTRs must be removed. */
-	struct jffs2_xattr_ref *ref;
-	uint32_t length;
-	int rc, retry;
+	struct jffs2_xattr_ref *ref, *_ref;
 
 	if (!ic || ic->nlink > 0)
 		return;
 
-	down_read(&c->xattr_sem);
-	if (!ic->xref) {
-		up_read(&c->xattr_sem);
-		return;
-	}
-	up_read(&c->xattr_sem);
- retry:
-	rc = jffs2_reserve_space(c, PAD(sizeof(struct jffs2_raw_xref)), &length,
-				 ALLOC_DELETION, JFFS2_SUMMARY_XREF_SIZE);
 	down_write(&c->xattr_sem);
-	if (ic->xref) {
-		ref = ic->xref;
-		ic->xref = ref->next;
-		if (rc) {
-			delete_xattr_ref_delay(c, ref);
-		} else {
-			delete_xattr_ref(c, ref, 1);
-		}
+	for (ref = ic->xref; ref; ref = _ref) {
+		_ref = ref->next;
+		delete_xattr_ref(c, ref);
 	}
-	retry = ic->xref ? 1 : 0;
+	ic->xref = NULL;
 	up_write(&c->xattr_sem);
-	if (!rc)
-		jffs2_complete_reservation(c);
-	if (retry)
-		goto retry;
 }
 
 void jffs2_xattr_free_inode(struct jffs2_sb_info *c, struct jffs2_inode_cache *ic)
@@ -743,7 +642,7 @@ static int check_xattr_ref_inode(struct 
 			rc = load_xattr_datum(c, ref->xd);
 			if (unlikely(rc > 0)) {
 				*pref = ref->next;
-				delete_xattr_ref_delay(c, ref);
+				delete_xattr_ref(c, ref);
 				goto retry;
 			} else if (unlikely(rc < 0))
 				goto out;
@@ -755,7 +654,7 @@ static int check_xattr_ref_inode(struct 
 				ref->xd->flags &= ~JFFS2_XFLAGS_BIND;
 				if (unlikely(rc > 0)) {
 					*pcmp = cmp->next;
-					delete_xattr_ref_delay(c, cmp);
+					delete_xattr_ref(c, cmp);
 					goto retry;
 				} else if (unlikely(rc < 0))
 					goto out;
@@ -764,10 +663,10 @@ static int check_xattr_ref_inode(struct 
 			    && !strcmp(ref->xd->xname, cmp->xd->xname)) {
 				if (ref->xseqno > cmp->xseqno) {
 					*pcmp = cmp->next;
-					delete_xattr_ref_delay(c, cmp);
+					delete_xattr_ref(c, cmp);
 				} else {
 					*pref = ref->next;
-					delete_xattr_ref_delay(c, ref);
+					delete_xattr_ref(c, ref);
 				}
 				goto retry;
 			}
@@ -865,20 +764,11 @@ void jffs2_build_xattr_subsystem(struct 
 	struct jffs2_inode_cache *ic;
 	struct jffs2_raw_node_ref *raw;
 	int i, xdatum_count = 0, xdatum_unchecked_count = 0, xref_count = 0;
+	int xdatum_orphan_count = 0, xref_orphan_count = 0, xref_dead_count = 0;
 
 	BUG_ON(!(c->flags & JFFS2_SB_FLAG_BUILDING));
-	/* Phase.1 : Drop dead xdatum */
-	for (i=0; i < XATTRINDEX_HASHSIZE; i++) {
-		list_for_each_entry_safe(xd, _xd, &c->xattrindex[i], xindex) {
-			BUG_ON(xd->node == (void *)xd);
-			if (is_xattr_datum_dead(xd)) {
-				list_del_init(&xd->xindex);
-				list_add(&xd->xindex, &c->xattr_unchecked);
-			}
-		}
-	}
 
-	/* Phase.2 : Merge same xref */
+	/* Phase.1 : Merge same xref */
 	for (i=0; i < XREF_TMPHASH_SIZE; i++)
 		xref_tmphash[i] = NULL;
 	for (ref=c->xref_temp; ref; ref=_ref) {
@@ -919,13 +809,15 @@ void jffs2_build_xattr_subsystem(struct 
 	}
 	c->xref_temp = NULL;
 
-	/* Phase.3 : Bind xref with inode_cache and xattr_datum */
+	/* Phase.2 : Bind xref with inode_cache and xattr_datum */
 	for (i=0; i < XREF_TMPHASH_SIZE; i++) {
 		for (ref=xref_tmphash[i]; ref; ref=_ref) {
+			xref_count++;
 			_ref = ref->next;
 			if (is_xattr_ref_dead(ref)) {
 				ref->next = c->xref_dead_list;
 				c->xref_dead_list = ref;
+				xref_dead_count++;
 				continue;
 			}
 			/* At this point, ref->xid and ref->ino contain XID and inode number.
@@ -933,11 +825,12 @@ void jffs2_build_xattr_subsystem(struct 
 			xd = jffs2_find_xattr_datum(c, ref->xid);
 			ic = jffs2_get_ino_cache(c, ref->ino);
 			if (!xd || !ic) {
-				JFFS2_WARNING("xref(ino=%u, xid=%u, xseqno=%u) is orphan. \n",
-					      ref->ino, ref->xid, ref->xseqno);
-				set_xattr_ref_dead(ref);
+				dbg_xattr("xref(ino=%u, xid=%u, xseqno=%u) is orphan.\n",
+					  ref->ino, ref->xid, ref->xseqno);
+				ref->xseqno |= XREF_DELETE_MARKER;
 				ref->next = c->xref_dead_list;
 				c->xref_dead_list = ref;
+				xref_orphan_count++;
 				continue;
 			}
 			ref->xd = xd;
@@ -945,19 +838,20 @@ void jffs2_build_xattr_subsystem(struct 
 			xd->refcnt++;
 			ref->next = ic->xref;
 			ic->xref = ref;
-			xref_count++;
 		}
 	}
 
-	/* Phase.4 : Link unchecked xdatum to xattr_unchecked list */
+	/* Phase.3 : Link unchecked xdatum to xattr_unchecked list */
 	for (i=0; i < XATTRINDEX_HASHSIZE; i++) {
 		list_for_each_entry_safe(xd, _xd, &c->xattrindex[i], xindex) {
+			xdatum_count++;
 			list_del_init(&xd->xindex);
 			if (!xd->refcnt) {
-				JFFS2_WARNING("orphan xdatum(xid=%u, version=%u)\n",
-					      xd->xid, xd->version);
-				set_xattr_datum_dead(xd);
+				dbg_xattr("xdatum(xid=%u, version=%u) is orphan.\n",
+					  xd->xid, xd->version);
+				xd->flags |= JFFS2_XFLAGS_DEAD;
 				list_add(&xd->xindex, &c->xattr_unchecked);
+				xdatum_orphan_count++;
 				continue;
 			}
 			if (is_xattr_datum_unchecked(c, xd)) {
@@ -966,12 +860,14 @@ void jffs2_build_xattr_subsystem(struct 
 				list_add(&xd->xindex, &c->xattr_unchecked);
 				xdatum_unchecked_count++;
 			}
-			xdatum_count++;
 		}
 	}
 	/* build complete */
-	JFFS2_NOTICE("complete building xattr subsystem, %u of xdatum (%u unchecked) and "
-		     "%u of xref found.\n", xdatum_count, xdatum_unchecked_count, xref_count);
+	JFFS2_NOTICE("complete building xattr subsystem, %u of xdatum"
+		     " (%u unchecked, %u orphan) and "
+		     "%u of xref (%u dead, %u orphan) found.\n",
+		     xdatum_count, xdatum_unchecked_count, xdatum_orphan_count,
+		     xref_count, xref_dead_count, xref_orphan_count);
 }
 
 struct jffs2_xattr_datum *jffs2_setup_xattr_datum(struct jffs2_sb_info *c,
@@ -1079,7 +975,7 @@ ssize_t jffs2_listxattr(struct dentry *d
 				rc = load_xattr_datum(c, xd);
 				if (unlikely(rc > 0)) {
 					*pref = ref->next;
-					delete_xattr_ref_delay(c, ref);
+					delete_xattr_ref(c, ref);
 					goto retry;
 				} else if (unlikely(rc < 0))
 					goto out;
@@ -1140,7 +1036,7 @@ int do_jffs2_getxattr(struct inode *inod
 				rc = load_xattr_datum(c, xd);
 				if (unlikely(rc > 0)) {
 					*pref = ref->next;
-					delete_xattr_ref_delay(c, ref);
+					delete_xattr_ref(c, ref);
 					goto retry;
 				} else if (unlikely(rc < 0)) {
 					goto out;
@@ -1203,7 +1099,7 @@ int do_jffs2_setxattr(struct inode *inod
 			rc = load_xattr_datum(c, xd);
 			if (unlikely(rc > 0)) {
 				*pref = ref->next;
-				delete_xattr_ref_delay(c, ref);
+				delete_xattr_ref(c, ref);
 				goto retry;
 			} else if (unlikely(rc < 0))
 				goto out;
@@ -1214,8 +1110,23 @@ int do_jffs2_setxattr(struct inode *inod
 				goto out;
 			}
 			if (!buffer) {
-				*pref = ref->next;
-				rc = delete_xattr_ref(c, ref, 0);
+				ref->ino = ic->ino;
+				ref->xid = xd->xid;
+				ref->xseqno |= XREF_DELETE_MARKER;
+				rc = save_xattr_ref(c, ref);
+				if (!rc) {
+					*pref = ref->next;
+					spin_lock(&c->erase_completion_lock);
+					ref->next = c->xref_dead_list;
+					c->xref_dead_list = ref;
+					spin_unlock(&c->erase_completion_lock);
+					if (!--xd->refcnt)
+						delete_xattr_datum(c, xd);
+				} else {
+					ref->ic = ic;
+					ref->xd = xd;
+					ref->xseqno &= ~XREF_DELETE_MARKER;
+				}
 				goto out;
 			}
 			goto found;
@@ -1248,7 +1159,7 @@ int do_jffs2_setxattr(struct inode *inod
 		JFFS2_WARNING("jffs2_reserve_space()=%d, request=%u\n", rc, request);
 		xd->refcnt--;
 		if (!xd->refcnt)
-			delete_xattr_datum_delay(c, xd);
+			delete_xattr_datum(c, xd);
 		up_write(&c->xattr_sem);
 		return rc;
 	}
@@ -1263,21 +1174,9 @@ int do_jffs2_setxattr(struct inode *inod
 		rc = PTR_ERR(newref);
 		xd->refcnt--;
 		if (!xd->refcnt)
-			delete_xattr_datum_delay(c, xd);
+			delete_xattr_datum(c, xd);
 	} else if (ref) {
-		up_write(&c->xattr_sem);
-		jffs2_complete_reservation(c);
-
-		rc = jffs2_reserve_space(c, request, &length,
-					 ALLOC_DELETION, JFFS2_SUMMARY_XREF_SIZE);
-		down_write(&c->xattr_sem);
-		if (rc) {
-			JFFS2_WARNING("jffs2_reserve_space()=%d, request=%u\n", rc, request);
-			delete_xattr_ref_delay(c, ref);
-			up_write(&c->xattr_sem);
-			return 0;
-		}
-		delete_xattr_ref(c, ref, 1);
+		delete_xattr_ref(c, ref);
 	}
  out:
 	up_write(&c->xattr_sem);
@@ -1292,6 +1191,10 @@ int do_jffs2_setxattr(struct inode *inod
  *   is used to move xref into new node.
  * jffs2_verify_xattr(c)
  *   is used to call do_verify_xattr_datum() before garbage collecting.
+ * jffs2_release_xattr_datum(c, xd)
+ *   is used to release an in-memory object of xdatum.
+ * jffs2_release_xattr_ref(c, ref)
+ *   is used to release an in-memory object of xref.
  * -------------------------------------------------- */
 int jffs2_garbage_collect_xattr_datum(struct jffs2_sb_info *c, struct jffs2_xattr_datum *xd,
 				      struct jffs2_raw_node_ref *raw)
@@ -1302,18 +1205,17 @@ int jffs2_garbage_collect_xattr_datum(st
 	down_write(&c->xattr_sem);
 	if (xd->node != raw)
 		goto out;
-	if (is_xattr_datum_dead(xd) && (raw->next_in_ino == (void *)xd))
+	if (xd->flags & (JFFS2_XFLAGS_DEAD|JFFS2_XFLAGS_INVALID))
 		goto out;
 
-	old_ofs = ref_offset(xd->node);
-	totlen = ref_totlen(c, c->gcblock, xd->node);
-
-	if (!is_xattr_datum_dead(xd)) {
-		rc = load_xattr_datum(c, xd);
-		if (unlikely(rc < 0))
-			goto out;
+	rc = load_xattr_datum(c, xd);
+	if (unlikely(rc)) {
+		rc = (rc > 0) ? 0 : rc;
+		goto out;
 	}
-
+	old_ofs = ref_offset(xd->node);
+	totlen = PAD(sizeof(struct jffs2_raw_xattr)
+			+ xd->name_len + 1 + xd->value_len);
 	rc = jffs2_reserve_space_gc(c, totlen, &length, JFFS2_SUMMARY_XATTR_SIZE);
 	if (rc) {
 		JFFS2_WARNING("jffs2_reserve_space_gc()=%d, request=%u\n", rc, totlen);
@@ -1331,7 +1233,6 @@ int jffs2_garbage_collect_xattr_datum(st
 	return rc;
 }
 
-
 int jffs2_garbage_collect_xattr_ref(struct jffs2_sb_info *c, struct jffs2_xattr_ref *ref,
 				    struct jffs2_raw_node_ref *raw)
 {
@@ -1392,7 +1293,7 @@ int jffs2_verify_xattr(struct jffs2_sb_i
 			raw->flash_offset = ref_offset(raw)
 				| ((xd->node == (void *)raw) ? REF_PRISTINE : REF_NORMAL);
 		}
-		if (is_xattr_datum_dead(xd))
+		if (xd->flags & JFFS2_XFLAGS_DEAD)
 			list_add(&xd->xindex, &c->xattr_dead_list);
 		spin_unlock(&c->erase_completion_lock);
 	}
@@ -1403,7 +1304,7 @@ int jffs2_verify_xattr(struct jffs2_sb_i
 void jffs2_release_xattr_datum(struct jffs2_sb_info *c, struct jffs2_xattr_datum *xd)
 {
 	/* must be called under spin_lock(&c->erase_completion_lock) */
-	if (xd->node != (void *)xd)
+	if (xd->refcnt > 0 || xd->node != (void *)xd)
 		return;
 
 	list_del(&xd->xindex);
@@ -1421,8 +1322,8 @@ void jffs2_release_xattr_ref(struct jffs
 	for (tmp=c->xref_dead_list, ptmp=&c->xref_dead_list; tmp; ptmp=&tmp->next, tmp=tmp->next) {
 		if (ref == tmp) {
 			*ptmp = tmp->next;
-			jffs2_free_xattr_ref(ref);
 			break;
 		}
 	}
+	jffs2_free_xattr_ref(ref);
 }
diff --git a/fs/jffs2/xattr.h b/fs/jffs2/xattr.h
index 06ab7b8..4a10abc 100644
--- a/fs/jffs2/xattr.h
+++ b/fs/jffs2/xattr.h
@@ -16,6 +16,7 @@ #include <linux/list.h>
 
 #define JFFS2_XFLAGS_HOT	(0x01)	/* This datum is HOT */
 #define JFFS2_XFLAGS_BIND	(0x02)	/* This datum is not reclaimed */
+#define JFFS2_XFLAGS_DEAD	(0x40)	/* This datum is already dead */
 #define JFFS2_XFLAGS_INVALID	(0x80)	/* This datum contains crc error */
 
 struct jffs2_xattr_datum
@@ -60,34 +61,12 @@ struct jffs2_xattr_ref
 	struct jffs2_xattr_ref *next;		/* chained from ic->xref_list */
 };
 
-#define XDATUM_DELETE_MARKER	(0xffffffff)
 #define XREF_DELETE_MARKER	(0x00000001)
-static inline int is_xattr_datum_dead(struct jffs2_xattr_datum *xd)
-{
-	return (xd->version == XDATUM_DELETE_MARKER);
-}
-
-static inline void set_xattr_datum_dead(struct jffs2_xattr_datum *xd)
-{
-	xd->version = XDATUM_DELETE_MARKER;
-}
-
 static inline int is_xattr_ref_dead(struct jffs2_xattr_ref *ref)
 {
 	return ((ref->xseqno & XREF_DELETE_MARKER) != 0);
 }
 
-static inline void set_xattr_ref_dead(struct jffs2_xattr_ref *ref)
-{
-	ref->xseqno |= XREF_DELETE_MARKER;
-}
-
-static inline void clr_xattr_ref_dead(struct jffs2_xattr_ref *ref)
-{
-	ref->xseqno &= ~XREF_DELETE_MARKER;
-}
-
-
 #ifdef CONFIG_JFFS2_FS_XATTR
 
 extern void jffs2_init_xattr_subsystem(struct jffs2_sb_info *c);

[-- Attachment #3: jffs2-xattr-v6.1-02-fix-memory-leak-jffs2_xattr_ref.patch --]
[-- Type: text/plain, Size: 2114 bytes --]

diff-tree d4b20cb8ea783807606c403afbf1d6e9614f3b9e (from 00c6cf2b8998389c8c04a5585b54902227ceb528)
Author: KaiGai Kohei <kaigai@ak.jp.nec.com>
Date:   Fri Jun 23 22:53:27 2006 +0900

    [JFFS2][XATTR] Fix memory leak with jffs2_xattr_ref
    
    If xattr_ref is associated with an orphan inode_cache
    on filesystem mounting, those xattr_refs are not
    released even if this inode_cache is released.
    
    This patch enables to call jffs2_xattr_delete_inode()
    for such a irregular inode_cachde too.
    
    Signed-off-by: KaiGai Kohei <kaigai@ak.jp.nec.com>

diff --git a/fs/jffs2/fs.c b/fs/jffs2/fs.c
index 7b6c24b..e8e52db 100644
--- a/fs/jffs2/fs.c
+++ b/fs/jffs2/fs.c
@@ -227,8 +227,6 @@ void jffs2_clear_inode (struct inode *in
 	struct jffs2_inode_info *f = JFFS2_INODE_INFO(inode);
 
 	D1(printk(KERN_DEBUG "jffs2_clear_inode(): ino #%lu mode %o\n", inode->i_ino, inode->i_mode));
-
-	jffs2_xattr_delete_inode(c, f->inocache);
 	jffs2_do_clear_inode(c, f);
 }
 
diff --git a/fs/jffs2/gc.c b/fs/jffs2/gc.c
index f59b147..daff334 100644
--- a/fs/jffs2/gc.c
+++ b/fs/jffs2/gc.c
@@ -165,6 +165,7 @@ int jffs2_garbage_collect_pass(struct jf
 			D1(printk(KERN_DEBUG "Skipping check of ino #%d with nlink zero\n",
 				  ic->ino));
 			spin_unlock(&c->inocache_lock);
+			jffs2_xattr_delete_inode(c, ic);
 			continue;
 		}
 		switch(ic->state) {
diff --git a/fs/jffs2/nodelist.c b/fs/jffs2/nodelist.c
index 927dfe4..7675b33 100644
--- a/fs/jffs2/nodelist.c
+++ b/fs/jffs2/nodelist.c
@@ -906,6 +906,9 @@ void jffs2_del_ino_cache(struct jffs2_sb
 {
 	struct jffs2_inode_cache **prev;
 
+#ifdef CONFIG_JFFS2_FS_XATTR
+	BUG_ON(old->xref);
+#endif
 	dbg_inocache("del %p (ino #%u)\n", old, old->ino);
 	spin_lock(&c->inocache_lock);
 
diff --git a/fs/jffs2/readinode.c b/fs/jffs2/readinode.c
index e1acce8..7f64578 100644
--- a/fs/jffs2/readinode.c
+++ b/fs/jffs2/readinode.c
@@ -939,6 +939,7 @@ void jffs2_do_clear_inode(struct jffs2_s
 	struct jffs2_full_dirent *fd, *fds;
 	int deleted;
 
+	jffs2_xattr_delete_inode(c, f->inocache);
 	down(&f->sem);
 	deleted = f->inocache && !f->inocache->nlink;
 

[-- Attachment #4: jffs2-xattr-v6.1-03-redefine-refcnt-as-atomic_t.patch --]
[-- Type: text/plain, Size: 4468 bytes --]

diff-tree 09ed2f92a606bf4db09e293c8112c2a0628656b1 (from d4b20cb8ea783807606c403afbf1d6e9614f3b9e)
Author: KaiGai Kohei <kaigai@ak.jp.nec.com>
Date:   Fri Jun 23 22:58:43 2006 +0900

    [JFFS2][XATTR] Re-define xd->refcnt as atomic_t
    
    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.
    
    This patch redefine xd->refcnt as atomic_t. It enables to refer
    xd->refcnt without any locking.
    
    Signed-off-by: KaiGai Kohei <kaigai@ak.jp.nec.com>

diff --git a/fs/jffs2/xattr.c b/fs/jffs2/xattr.c
index 7622f79..18e66db 100644
--- a/fs/jffs2/xattr.c
+++ b/fs/jffs2/xattr.c
@@ -345,7 +345,7 @@ static struct jffs2_xattr_datum *create_
 		    && xd->value_len==xsize
 		    && !strcmp(xd->xname, xname)
 		    && !memcmp(xd->xvalue, xvalue, xsize)) {
-			xd->refcnt++;
+			atomic_inc(&xd->refcnt);
 			return xd;
 		}
 	}
@@ -365,7 +365,7 @@ static struct jffs2_xattr_datum *create_
 	strcpy(data, xname);
 	memcpy(data + name_len + 1, xvalue, xsize);
 
-	xd->refcnt = 1;
+	atomic_set(&xd->refcnt, 1);
 	xd->xid = ++c->highest_xid;
 	xd->flags |= JFFS2_XFLAGS_HOT;
 	xd->xprefix = xprefix;
@@ -397,7 +397,7 @@ static struct jffs2_xattr_datum *create_
 static void delete_xattr_datum(struct jffs2_sb_info *c, struct jffs2_xattr_datum *xd)
 {
 	/* must be called under down_write(xattr_sem) */
-	BUG_ON(xd->refcnt);
+	BUG_ON(atomic_read(&xd->refcnt));
 
 	unload_xattr_datum(c, xd);
 	xd->flags |= JFFS2_XFLAGS_DEAD;
@@ -580,7 +580,7 @@ static void delete_xattr_ref(struct jffs
 	dbg_xattr("xref(ino=%u, xid=%u, xseqno=%u) was removed.\n",
 		  ref->ino, ref->xid, ref->xseqno);
 
-	if (!--xd->refcnt)
+	if (atomic_dec_and_test(&xd->refcnt))
 		delete_xattr_datum(c, xd);
 }
 
@@ -612,8 +612,7 @@ void jffs2_xattr_free_inode(struct jffs2
 	for (ref = ic->xref; ref; ref = _ref) {
 		_ref = ref->next;
 		xd = ref->xd;
-		xd->refcnt--;
-		if (!xd->refcnt) {
+		if (atomic_dec_and_test(&xd->refcnt)) {
 			unload_xattr_datum(c, xd);
 			jffs2_free_xattr_datum(xd);
 		}
@@ -835,7 +834,7 @@ void jffs2_build_xattr_subsystem(struct 
 			}
 			ref->xd = xd;
 			ref->ic = ic;
-			xd->refcnt++;
+			atomic_inc(&xd->refcnt);
 			ref->next = ic->xref;
 			ic->xref = ref;
 		}
@@ -846,7 +845,7 @@ void jffs2_build_xattr_subsystem(struct 
 		list_for_each_entry_safe(xd, _xd, &c->xattrindex[i], xindex) {
 			xdatum_count++;
 			list_del_init(&xd->xindex);
-			if (!xd->refcnt) {
+			if (!atomic_read(&xd->refcnt)) {
 				dbg_xattr("xdatum(xid=%u, version=%u) is orphan.\n",
 					  xd->xid, xd->version);
 				xd->flags |= JFFS2_XFLAGS_DEAD;
@@ -1120,7 +1119,7 @@ int do_jffs2_setxattr(struct inode *inod
 					ref->next = c->xref_dead_list;
 					c->xref_dead_list = ref;
 					spin_unlock(&c->erase_completion_lock);
-					if (!--xd->refcnt)
+					if (atomic_dec_and_test(&xd->refcnt))
 						delete_xattr_datum(c, xd);
 				} else {
 					ref->ic = ic;
@@ -1157,8 +1156,7 @@ int do_jffs2_setxattr(struct inode *inod
 	down_write(&c->xattr_sem);
 	if (rc) {
 		JFFS2_WARNING("jffs2_reserve_space()=%d, request=%u\n", rc, request);
-		xd->refcnt--;
-		if (!xd->refcnt)
+		if (atomic_dec_and_test(&xd->refcnt))
 			delete_xattr_datum(c, xd);
 		up_write(&c->xattr_sem);
 		return rc;
@@ -1172,8 +1170,7 @@ int do_jffs2_setxattr(struct inode *inod
 			ic->xref = ref;
 		}
 		rc = PTR_ERR(newref);
-		xd->refcnt--;
-		if (!xd->refcnt)
+		if (atomic_dec_and_test(&xd->refcnt))
 			delete_xattr_datum(c, xd);
 	} else if (ref) {
 		delete_xattr_ref(c, ref);
@@ -1304,7 +1301,7 @@ int jffs2_verify_xattr(struct jffs2_sb_i
 void jffs2_release_xattr_datum(struct jffs2_sb_info *c, struct jffs2_xattr_datum *xd)
 {
 	/* must be called under spin_lock(&c->erase_completion_lock) */
-	if (xd->refcnt > 0 || xd->node != (void *)xd)
+	if (atomic_read(&xd->refcnt) || xd->node != (void *)xd)
 		return;
 
 	list_del(&xd->xindex);
diff --git a/fs/jffs2/xattr.h b/fs/jffs2/xattr.h
index 4a10abc..06a5c69 100644
--- a/fs/jffs2/xattr.h
+++ b/fs/jffs2/xattr.h
@@ -28,7 +28,7 @@ struct jffs2_xattr_datum
 	uint16_t xprefix;		/* see JFFS2_XATTR_PREFIX_* */
 
 	struct list_head xindex;	/* chained from c->xattrindex[n] */
-	uint32_t refcnt;		/* # of xattr_ref refers this */
+	atomic_t refcnt;		/* # of xattr_ref refers this */
 	uint32_t xid;
 	uint32_t version;
 

[-- Attachment #5: jffs2-xattr-v6.1-04-fix-copyright.patch --]
[-- Type: text/plain, Size: 853 bytes --]

diff-tree d0fac87b3d2a9fd5c265f6077fc2f6b3b4672f82 (from 09ed2f92a606bf4db09e293c8112c2a0628656b1)
Author: KaiGai Kohei <kaigai@ak.jp.nec.com>
Date:   Fri Jun 23 23:00:19 2006 +0900

    [JFFS2][XATTR] Fix wrong copyrigh.
    
    summary.c was modified at 2006.
    
    Signed-off-by: KaiGai Kohei <kaigai@ak.jp.nec.com>

diff --git a/fs/jffs2/summary.c b/fs/jffs2/summary.c
index d60784a..4c8f381 100644
--- a/fs/jffs2/summary.c
+++ b/fs/jffs2/summary.c
@@ -5,7 +5,7 @@
  *                     Zoltan Sogor <weth@inf.u-szeged.hu>,
  *                     Patrik Kluba <pajko@halom.u-szeged.hu>,
  *                     University of Szeged, Hungary
- *               2005  KaiGai Kohei <kaigai@ak.jp.nec.com>
+ *               2006  KaiGai Kohei <kaigai@ak.jp.nec.com>
  *
  * For licensing information, see the file 'LICENCE' in this directory.
  *

[-- Attachment #6: jffs2-xattr-v6.1-05-coexistence-between-xattr-and-write-buffering.patch --]
[-- Type: text/plain, Size: 868 bytes --]

diff-tree c3c4bdbb068b0cf93951ca71261bf4aa8e3fa830 (from 486b70a7e4298b53aa3046b8574a1e431e4143ba)
Author: KaiGai Kohei <kaigai@ak.jp.nec.com>
Date:   Fri Jun 23 23:10:44 2006 +0900

    [JFFS2][XATTR] coexistence between xattr and write buffering support.
    
    Drop '&& !JFFS2_FS_WRITEBUFFER' from fs/Kconfig.
    The series of previous patches enables to use those
    functionality at same time.
    
    Signed-off-by: KaiGai Kohei <kaigai@ak.jp.nec.com>

diff --git a/fs/Kconfig b/fs/Kconfig
index 20f9b55..e2643ee 100644
--- a/fs/Kconfig
+++ b/fs/Kconfig
@@ -1115,7 +1115,7 @@ config JFFS2_SUMMARY
 
 config JFFS2_FS_XATTR
 	bool "JFFS2 XATTR support (EXPERIMENTAL)"
-	depends on JFFS2_FS && EXPERIMENTAL && !JFFS2_FS_WRITEBUFFER
+	depends on JFFS2_FS && EXPERIMENTAL
 	default n
 	help
 	  Extended attributes are name:value pairs associated with inodes by

Re: JFFS2/xattr problems.

[David Woodhouse]

On Sat, 2006-06-24 at 14:58 +0900, KaiGai Kohei wrote:
> I'm sorry to have kept you waiting so long.

No problem. I don't expect an _instant_ reaction :)

> The following patches 01-03 modifies its design, and 04 fixes trivial
> wrong copyright, 05 enables to use xattr with write buffering
> support. 

Thanks. I'm away for the weekend, but will review these on Monday.

-- 
dwmw2

Re: JFFS2/xattr problems.

[David Woodhouse]

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.

-- 
dwmw2

Re: JFFS2/xattr problems.

[KaiGai Kohei]

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>

Re: JFFS2/xattr problems.

[KaiGai Kohei]

[-- Attachment #1: Type: text/plain, Size: 1345 bytes --]

> 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.

* jffs2-xattr-v6.2-01-fix-xd_refcnt-race-condition.patch

   Signed-off-by: KaiGai Kohei <kaigai@ak.jp.nec.com>

The attached patch resolves this problem.
When xd->refcnt is checked whether this xdatum should be released
or not, atomic_dec_and_lock() is used to ensure holding the
c->erase_completion_lock.

This fix change a specification of delete_xattr_datum().
Previously, it's only called when xd->refcnt equals zero.
(calling it with positive xd->refcnt cause a BUG())
If you applied this patch, the function checks whether
xd->refcnt is zero or not under the spinlock if necessary.
Then, it marks xd DEAD flahs and links with xattr_dead_list
or releases it immediately when xd->refcnt become zero.

I think it's not appropriate to call this behavior 'delete'.
Thus, I changed the function name 'unrefer_xattr_datum'.
Isn't it superfluous modify?

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

[-- Attachment #2: jffs2-xattr-v6.2-01-fix-xd_refcnt-race-condition.patch --]
[-- Type: text/plain, Size: 3503 bytes --]

diff --git a/fs/jffs2/xattr.c b/fs/jffs2/xattr.c
index 18e66db..25bc1ae 100644
--- a/fs/jffs2/xattr.c
+++ b/fs/jffs2/xattr.c
@@ -50,9 +50,10 @@ #include "nodelist.h"
  *   is used to write xdatum to medium. xd->version will be incremented.
  * create_xattr_datum(c, xprefix, xname, xvalue, xsize)
  *   is used to create new xdatum and write to medium.
- * delete_xattr_datum(c, xd)
- *   is used to delete a xdatum. It marks xd JFFS2_XFLAGS_DEAD, and allows
- *   GC to reclaim those physical nodes.
+ * unrefer_xattr_datum(c, xd)
+ *   is used to delete a xdatum. When nobody refers this xdatum, JFFS2_XFLAGS_DEAD
+ *   is set on xd->flags and chained xattr_dead_list or release it immediately.
+ *   In the first case, the garbage collector release it later.
  * -------------------------------------------------- */
 static uint32_t xattr_datum_hashkey(int xprefix, const char *xname, const char *xvalue, int xsize)
 {
@@ -394,22 +395,24 @@ static struct jffs2_xattr_datum *create_
 	return xd;
 }
 
-static void delete_xattr_datum(struct jffs2_sb_info *c, struct jffs2_xattr_datum *xd)
+static void unrefer_xattr_datum(struct jffs2_sb_info *c, struct jffs2_xattr_datum *xd)
 {
 	/* must be called under down_write(xattr_sem) */
-	BUG_ON(atomic_read(&xd->refcnt));
+	if (atomic_dec_and_lock(&xd->refcnt, &c->erase_completion_lock)) {
+		uint32_t xid = xd->xid, version = xd->version;
 
-	unload_xattr_datum(c, xd);
-	xd->flags |= JFFS2_XFLAGS_DEAD;
-	spin_lock(&c->erase_completion_lock);
-	if (xd->node == (void *)xd) {
-		BUG_ON(!(xd->flags & JFFS2_XFLAGS_INVALID));
-		jffs2_free_xattr_datum(xd);
-	} else {
-		list_add(&xd->xindex, &c->xattr_dead_list);
+		unload_xattr_datum(c, xd);
+		xd->flags |= JFFS2_XFLAGS_DEAD;
+		if (xd->node == (void *)xd) {
+			BUG_ON(!(xd->flags & JFFS2_XFLAGS_INVALID));
+			jffs2_free_xattr_datum(xd);
+		} else {
+			list_add(&xd->xindex, &c->xattr_dead_list);
+		}
+		spin_unlock(&c->erase_completion_lock);
+
+		dbg_xattr("xdatum(xid=%u, version=%u) was removed.\n", xid, version);
 	}
-	spin_unlock(&c->erase_completion_lock);
-	dbg_xattr("xdatum(xid=%u, version=%u) was removed.\n", xd->xid, xd->version);
 }
 
 /* -------- xref related functions ------------------
@@ -580,8 +583,7 @@ static void delete_xattr_ref(struct jffs
 	dbg_xattr("xref(ino=%u, xid=%u, xseqno=%u) was removed.\n",
 		  ref->ino, ref->xid, ref->xseqno);
 
-	if (atomic_dec_and_test(&xd->refcnt))
-		delete_xattr_datum(c, xd);
+	unrefer_xattr_datum(c, xd);
 }
 
 void jffs2_xattr_delete_inode(struct jffs2_sb_info *c, struct jffs2_inode_cache *ic)
@@ -1119,8 +1121,7 @@ int do_jffs2_setxattr(struct inode *inod
 					ref->next = c->xref_dead_list;
 					c->xref_dead_list = ref;
 					spin_unlock(&c->erase_completion_lock);
-					if (atomic_dec_and_test(&xd->refcnt))
-						delete_xattr_datum(c, xd);
+					unrefer_xattr_datum(c, xd);
 				} else {
 					ref->ic = ic;
 					ref->xd = xd;
@@ -1156,8 +1157,7 @@ int do_jffs2_setxattr(struct inode *inod
 	down_write(&c->xattr_sem);
 	if (rc) {
 		JFFS2_WARNING("jffs2_reserve_space()=%d, request=%u\n", rc, request);
-		if (atomic_dec_and_test(&xd->refcnt))
-			delete_xattr_datum(c, xd);
+		unrefer_xattr_datum(c, xd);
 		up_write(&c->xattr_sem);
 		return rc;
 	}
@@ -1170,8 +1170,7 @@ int do_jffs2_setxattr(struct inode *inod
 			ic->xref = ref;
 		}
 		rc = PTR_ERR(newref);
-		if (atomic_dec_and_test(&xd->refcnt))
-			delete_xattr_datum(c, xd);
+		unrefer_xattr_datum(c, xd);
 	} else if (ref) {
 		delete_xattr_ref(c, ref);
 	}