From mboxrd@z Thu Jan 1 00:00:00 1970 From: David Woodhouse Subject: JFFS2 update for 2.4: read_inode/clear_inode race. Date: Thu, 10 Oct 2002 15:46:38 +0100 Sender: linux-fsdevel-owner@vger.kernel.org Message-ID: <3054.1034261198@passion.cambridge.redhat.com> Mime-Version: 1.0 Content-Type: text/plain; charset=us-ascii Cc: linux-fsdevel@vger.kernel.org Return-path: To: marcelo@conectiva.com.br List-Id: linux-fsdevel.vger.kernel.org JFFS2 gets somewhat unhappy when the VFS calls read_inode() and clear_inode() for the same inode simultaneously. This changeset (which can be fed through 'grep ^# | cut -c2-' to get a unified diff if anyone cares) fixes that particular case. I still haven't decided what to do in the stable branch about the other less likely case where the VFS calls read_inode() read_inode() clear_inode() clear_inode() in that order, all for the same inode number. By the time the last clear_inode() is called, some of the data structures which belonged to the physical inode in question may have been freed, although it's unlikely as it actually requires a flash block erase to happen in the meantime too. This BitKeeper patch contains the following changesets: 1.737 # This is a BitKeeper patch. What follows are the unified diffs for the # set of deltas contained in the patch. The rest of the patch, the part # that BitKeeper cares about, is below these diffs. # User: dwmw2 # Host: infradead.org # Root: /mnt/bk/jffs2-2.4 # #--- 1.4/fs/jffs2/gc.c Sun May 12 19:34:14 2002 #+++ 1.5/fs/jffs2/gc.c Thu Oct 10 15:34:48 2002 #@@ -31,7 +31,7 @@ # * provisions above, a recipient may use your version of this file # * under either the RHEPL or the GPL. # * #- * $Id: gc.c,v 1.52.2.3 2002/05/12 17:27:08 dwmw2 Exp $ #+ * $Id: gc.c,v 1.52.2.5 2002/10/10 13:18:38 dwmw2 Exp $ # * # */ # #@@ -134,8 +134,10 @@ # # D1(printk(KERN_DEBUG "garbage collect from block at phys 0x%08x\n", jeb->offset)); # #- if (!jeb->used_size) #+ if (!jeb->used_size) { #+ up(&c->alloc_sem); # goto eraseit; #+ } # # raw = jeb->gc_node; # #@@ -156,6 +158,7 @@ # /* Inode-less node. Clean marker, snapshot or something like that */ # spin_unlock_bh(&c->erase_completion_lock); # jffs2_mark_node_obsolete(c, raw); #+ up(&c->alloc_sem); # goto eraseit_lock; # } # #@@ -170,8 +173,8 @@ # if (is_bad_inode(inode)) { # printk(KERN_NOTICE "Eep. read_inode() failed for ino #%u\n", inum); # /* NB. This will happen again. We need to do something appropriate here. */ #- iput(inode); # up(&c->alloc_sem); #+ iput(inode); # return -EIO; # } # #@@ -234,6 +237,7 @@ # } # upnout: # up(&f->sem); #+ up(&c->alloc_sem); # iput(inode); # # eraseit_lock: #@@ -250,7 +254,6 @@ # jffs2_erase_pending_trigger(c); # } # spin_unlock_bh(&c->erase_completion_lock); #- up(&c->alloc_sem); # # return ret; # } #@@ -507,7 +510,7 @@ # * number as before. (Except in case of error -- see 'goto fill;' # * above.) # */ #- D1(if(unlikely(fn->frags <= 1)) { #+ D1(if(fn->frags <= 1) { # printk(KERN_WARNING "jffs2_garbage_collect_hole: Replacing fn with %d frag(s) but new ver %d != highest_version %d of ino #%d\n", # fn->frags, ri.version, f->highest_version, ri.ino); # }); # #--- 1.4/fs/jffs2/readinode.c Fri Mar 22 09:01:31 2002 #+++ 1.5/fs/jffs2/readinode.c Thu Oct 10 15:34:49 2002 #@@ -31,7 +31,7 @@ # * provisions above, a recipient may use your version of this file # * under either the RHEPL or the GPL. # * #- * $Id: readinode.c,v 1.58.2.5 2002/03/05 22:40:03 dwmw2 Exp $ #+ * $Id: readinode.c,v 1.58.2.6 2002/10/10 13:18:38 dwmw2 Exp $ # * # */ # #@@ -468,15 +468,28 @@ # struct jffs2_node_frag *frag, *frags; # struct jffs2_full_dirent *fd, *fds; # struct jffs2_inode_info *f = JFFS2_INODE_INFO(inode); #+ /* I don't think we care about the potential race due to reading this #+ without f->sem. It can never get undeleted. */ #+ int deleted = f->inocache && !f->inocache->nlink; # # D1(printk(KERN_DEBUG "jffs2_clear_inode(): ino #%lu mode %o\n", inode->i_ino, inode->i_mode)); # #+ /* If it's a deleted inode, grab the alloc_sem. This prevents #+ jffs2_garbage_collect_pass() from deciding that it wants to #+ garbage collect one of the nodes we're just about to mark #+ obsolete -- by the time we drop alloc_sem and return, all #+ the nodes are marked obsolete, and jffs2_g_c_pass() won't #+ call iget() for the inode in question. #+ */ #+ if (deleted) #+ down(&c->alloc_sem); #+ # down(&f->sem); # # frags = f->fraglist; # fds = f->dents; # if (f->metadata) { #- if (!f->inocache->nlink) #+ if (deleted) # jffs2_mark_node_obsolete(c, f->metadata->raw); # jffs2_free_full_dnode(f->metadata); # } #@@ -488,7 +501,7 @@ # # if (frag->node && !(--frag->node->frags)) { # /* Not a hole, and it's the final remaining frag of this node. Free the node */ #- if (!f->inocache->nlink) #+ if (deleted) # jffs2_mark_node_obsolete(c, frag->node->raw); # # jffs2_free_full_dnode(frag->node); #@@ -502,5 +515,8 @@ # } # # up(&f->sem); #+ #+ if(deleted) #+ up(&c->alloc_sem); # }; # # # Diff checksum=73bb1fd7 # Patch vers: 1.3 # Patch type: REGULAR == ChangeSet == torvalds@athlon.transmeta.com|ChangeSet|20020205173056|16047|c1d11a41ed024864 marcelo@freak.distro.conectiva|ChangeSet|20021010052138|26379 D 1.737 02/10/10 15:35:41+01:00 dwmw2@infradead.org +2 -0 B torvalds@athlon.transmeta.com|ChangeSet|20020205173056|16047|c1d11a41ed024864 C c Deal with VFS calling clear_inode() and read_inode() simultaneously for the same inode. K 25252 P ChangeSet ------------------------------------------------ 0a0 > patch@athlon.transmeta.com|fs/jffs2/gc.c|20020205201839|10209|b6698581d0f53a8b dwmw2@infradead.org|fs/jffs2/gc.c|20021010143448|25453 > patch@athlon.transmeta.com|fs/jffs2/readinode.c|20020205201839|13519|d8985d847ab6293a dwmw2@infradead.org|fs/jffs2/readinode.c|20021010143449|21243 == fs/jffs2/gc.c == patch@athlon.transmeta.com|fs/jffs2/gc.c|20020205201839|10209|b6698581d0f53a8b dwmw2@dwmw2.baythorne.internal|fs/jffs2/gc.c|20020512183414|22949 D 1.5 02/10/10 15:34:48+01:00 dwmw2@infradead.org +8 -5 B torvalds@athlon.transmeta.com|ChangeSet|20020205173056|16047|c1d11a41ed024864 C c Rearrange the dropping of alloc_sem and iput() to allow for clear_inode() now requiring the alloc_sem. K 25453 O -rw-rw-r-- P fs/jffs2/gc.c ------------------------------------------------ D34 1 I34 1 * $Id: gc.c,v 1.52.2.5 2002/10/10 13:18:38 dwmw2 Exp $ D137 1 I137 2 if (!jeb->used_size) { up(&c->alloc_sem); I138 1 } I158 1 up(&c->alloc_sem); D173 1 I174 1 iput(inode); I236 1 up(&c->alloc_sem); D253 1 D510 1 I510 1 D1(if(fn->frags <= 1) { == fs/jffs2/readinode.c == patch@athlon.transmeta.com|fs/jffs2/readinode.c|20020205201839|13519|d8985d847ab6293a dwmw2@infradead.org|fs/jffs2/readinode.c|20020322090131|37120 D 1.5 02/10/10 15:34:49+01:00 dwmw2@infradead.org +19 -3 B torvalds@athlon.transmeta.com|ChangeSet|20020205173056|16047|c1d11a41ed024864 C c Grab alloc_sem in clear_inode(). K 21243 O -rw-rw-r-- P fs/jffs2/readinode.c ------------------------------------------------ D34 1 I34 1 * $Id: readinode.c,v 1.58.2.6 2002/10/10 13:18:38 dwmw2 Exp $ I470 3 /* I don't think we care about the potential race due to reading this without f->sem. It can never get undeleted. */ int deleted = f->inocache && !f->inocache->nlink; I473 10 /* If it's a deleted inode, grab the alloc_sem. This prevents jffs2_garbage_collect_pass() from deciding that it wants to garbage collect one of the nodes we're just about to mark obsolete -- by the time we drop alloc_sem and return, all the nodes are marked obsolete, and jffs2_g_c_pass() won't call iget() for the inode in question. */ if (deleted) down(&c->alloc_sem); \ D479 1 I479 1 if (deleted) D491 1 I491 1 if (deleted) I504 3 \ if(deleted) up(&c->alloc_sem); # Patch checksum=272430c7 -- dwmw2