struct jffs2_xattr_datum/jffs2_xattr_ref/jffs2_inode_cache

struct jffs2_xattr_datum/jffs2_xattr_ref/jffs2_inode_cache

[David Woodhouse]

Kaigai-san,

To fix the problems with xattr node deletion, I think you'll need to
just mark nodes obsolete like we do for inode nodes, and keep them in
_list_ of physical nodes attached to a struct jffs2_xattr_datum -- just
the same as the jffs2_inode_cache has a _list_ of nodes.

To avoid re-using xids too early, you'll need the jffs2_xattr_datum to
be removed only when that list is completely empty, in erase.c.

I've (helpfully) added a BUG_ON() in the two places it's likely to be
important, where we assume that what we find at the end of a
->next_in_ino list is a jffs2_inode_cache.

I've also shuffled the three structures so that they all start the
same...

{
	void *scan_dents; /* Unused in XATTR */
	struct jffs2_raw_node_ref *nodes;
	uint8_t class;
	...


I've also extended jffs2_link_node_ref() so that it adds the new
jffs2_raw_node_ref to a jffs2_inode_cache. It should _also_ be able to
add the ref to an xattr_ref or xattr_datum too.

Does that look OK?

I've done all this in the mtd-2.6.git tree -- I've pulled what was in
the jffs2-devel-2.6.git tree into that already.

-- 
dwmw2

Re: struct jffs2_xattr_datum/jffs2_xattr_ref/jffs2_inode_cache

[KaiGai Kohei]

Hi, David.

> To fix the problems with xattr node deletion, I think you'll need to
> just mark nodes obsolete like we do for inode nodes, and keep them in
> _list_ of physical nodes attached to a struct jffs2_xattr_datum -- just
> the same as the jffs2_inode_cache has a _list_ of nodes.
> 
> To avoid re-using xids too early, you'll need the jffs2_xattr_datum to
> be removed only when that list is completely empty, in erase.c.

I also think your suggestion is the most reasonable approch.

Now, I plan to implement that any versions of xattr_datum and
xattr_ref are chained to physical node list and a real deletion
of xattr_datum and xattr_ref should be called from
jffs2_remove_node_refs_from_ino_list().


By the way, I have a question about an usage for jffs2_reserve_space().

Is it permitted to allocate a space for multi nodes by a single
calling of jffs2_reserve_space()?
I tried to allocate a space for xattr_datum and xattr_ref by a single
calling of jffs2_reserve_space(), and it seems to work without any
troubles.

--[quich hack example]-----------
request = PAD(sizeof(struct jffs2_raw_xattr) + strlen(xname) + 1 + size)
          + PAD(sizeof(struct jffs2_raw_xref));
rc = jffs2_reserve_space(c, request, &phys_ofs, &length, ALLOC_NORMAL,
                         JFFS2_SUMMARY_XATTR_SIZE + JFFS2_SUMMARY_XREF_SIZE);
          :
xd = create_xattr_datum(..., &phys_ofs);  <- phys_ofs will be incremented by usage.
          :
newref = create_xattr_ref(..., phys_ofs);
          :
jffs2_complete_reservation(c);
----------------------------------

If it's permitted, it's useful to reduce complexity by exclusion.
What do you think about?

> I've also shuffled the three structures so that they all start the
> same...
> 
> {
> 	void *scan_dents; /* Unused in XATTR */
> 	struct jffs2_raw_node_ref *nodes;
> 	uint8_t class;
> 	...
> 
> 
> I've also extended jffs2_link_node_ref() so that it adds the new
> jffs2_raw_node_ref to a jffs2_inode_cache. It should _also_ be able to
> add the ref to an xattr_ref or xattr_datum too.
> 
> Does that look OK?

The xattr implementation depends on the format of those structures
only in gc.c. I could not think this shuffle has any bad effects.

> I've done all this in the mtd-2.6.git tree -- I've pulled what was in
> the jffs2-devel-2.6.git tree into that already.

I confirmed this. I'll use this tree as the development base.

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

Re: struct jffs2_xattr_datum/jffs2_xattr_ref/jffs2_inode_cache

[David Woodhouse]

On Tue, 2006-05-23 at 11:01 +0900, KaiGai Kohei wrote:
> Now, I plan to implement that any versions of xattr_datum and
> xattr_ref are chained to physical node list and a real deletion
> of xattr_datum and xattr_ref should be called from
> jffs2_remove_node_refs_from_ino_list().

OK.

> By the way, I have a question about an usage for jffs2_reserve_space().
> 
> Is it permitted to allocate a space for multi nodes by a single
> calling of jffs2_reserve_space()?
> I tried to allocate a space for xattr_datum and xattr_ref by a single
> calling of jffs2_reserve_space(), and it seems to work without any
> troubles.

Yes, it works. But if you have space for just the xattr_datum at the end
of the eraseblock, it's better to write that in the available space and
then write the xattr_ref to the next eraseblock.

By the way, we should implement support for retrying writes in
save_xattr_datum() and save_xattr_ref(). See how jffs2_write_dnode() and
jffs2_write_dirent() do it, for example.

> If it's permitted, it's useful to reduce complexity by exclusion.
> What do you think about?

Once you implement the retry support, you don't reduce complexity, do
you?

-- 
dwmw2

Re: struct jffs2_xattr_datum/jffs2_xattr_ref/jffs2_inode_cache

[KaiGai Kohei]

>> By the way, I have a question about an usage for jffs2_reserve_space().
>>
>> Is it permitted to allocate a space for multi nodes by a single
>> calling of jffs2_reserve_space()?
>> I tried to allocate a space for xattr_datum and xattr_ref by a single
>> calling of jffs2_reserve_space(), and it seems to work without any
>> troubles.
> 
> Yes, it works. But if you have space for just the xattr_datum at the end
> of the eraseblock, it's better to write that in the available space and
> then write the xattr_ref to the next eraseblock.

OK, I agreed an increasion of wasted size should be avoided if possible.

> By the way, we should implement support for retrying writes in
> save_xattr_datum() and save_xattr_ref(). See how jffs2_write_dnode() and
> jffs2_write_dirent() do it, for example.

When we implement support for retrying write in those functions,
xattr_sem must be released once at least. If there is no problem
related to exclusion, I'll adopt retrying approach.

Currently, I'm checking whether it's safe or not.

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