spin_lock() needed ?

["Artem B. Bityuckiy" <abityuckiy@yandex.ru>]

Hello,

In JFFS2 I have mentioned the following:

When new node is successfully written to the flash, its node_ref is 
insert to the correspondent inode's node_ref list. Tis is done as following:

jffs2_add_physical_node_ref(c, raw);
raw->next_in_ino = f->inocache->nodes;
f->inocache->nodes = raw;



For example, see functions jffs2_write_dirent() and jffs2_write_dnode() 
in the write.c file.

I am not sure, but it seems there is a race here. The f->inocache->nodes 
may be obsolete node which is in the block pending for erase. So, this 
node may be removed when the correspondent block is erased.



I mean the following.

Suppose we have inode with two nodes. The first node is obsolete and is 
physically located to the block (say block number A), which is currently 
in the c->erase_pending_list. So, suppose:


jffs2_add_physical_node_ref(c, raw);
raw->next_in_ino = f->inocache->nodes; /* We save the address of the 
first obsolete node */

/* Suppose we are preempted here and the another process calls the 
jffs2_erase_pending_blocks() function, which erases the block A. Before 
erasing, it removes all the node_ref structures corresponding to nodes 
in this block A (see the implementation of jffs2_erase_pending_blocks(), 
i.e., the call to jffs2_free_all_node_refs()). Thus, the first node will 
be removed from list */

f->inocache->nodes = raw;
/* Now the first node_ref corresponds to new (3rd) node, but 
f->inocache->nodes->next_in_ino points to wrong place */



So, I think we should hold the c->erase_completion_lock here. I mean:

jffs2_add_physical_node_ref(c, raw);
spin_lock(&c->erase_completion_lock); /* <--------- this */
raw->next_in_ino = f->inocache->nodes;
f->inocache->nodes = raw;
spin_unlock(&c->erase_completion_lock); /* <--------- and this */


Can anybody comment this please?

-- 
Best Regards,
Artem B. Bityuckiy,
St.-Petersburg, Russia.