]> git.neil.brown.name Git - LaFS.git/commitdiff
Fix incorrect de-ref of ->my_inode
authorNeilBrown <neilb@suse.de>
Mon, 28 Jun 2010 02:33:18 +0000 (12:33 +1000)
committerNeilBrown <neilb@suse.de>
Mon, 28 Jun 2010 02:33:18 +0000 (12:33 +1000)
my_inode may not be set on a block on writeout - e.g. if it was
just cleaned.
As my_inode does not disappear once set (while a ref is held
on the block) it is safe to simply test it.

Signed-off-by: NeilBrown <neilb@suse.de>
README
cluster.c

diff --git a/README b/README
index 13078e40b4907e8af8c9d49422a5619e88aef60d..c5cd5c053de761a4d13134e781d66a6a27cf13ed 100644 (file)
--- a/README
+++ b/README
@@ -4836,9 +4836,11 @@ DONE 7a/ block.c:507 in lafs_dirty_dblock - no credits for 0/2
                i.e. 1510, 1945-2038, 2448 of 5378
     Appear to be looping in first loop of try_clean, maybe
      group_size_words == 0 ??
+    Add BUGON and wait.
 
- 7c/ NULL pointer deref - 000001b4
+DONE 7c/ NULL pointer deref - 000001b4
      Could be cluster_flush finds inode dblock without inode.
+     Have a BUG_ON of this now.
 
  7d/ paging request at 6b6b6bfb. 
     invalidate_inode_buffers called, so inode_has_buffers,
@@ -4896,7 +4898,7 @@ DONE 8/ looping in do_checkpoint
                     [cfa5fc58]0/2(2852)r0E:Valid,Dirty,SegRef,CN,CNI,UninCredit,PhysValid
                     [cfa53c58]0/2(3570)r0E:Valid,Dirty,CN,CNI,UninCredit,PhysValid
                     [cfa53828]0/2(2969)r0E:Valid,Dirty,CN,CNI,UninCredit,PhysValid
-                    [cfa57c58]0/2(579)r0E:Valid,Dirty,UninCredit,PhysValid
+                    [cfa75c58]0/2(579)r0E:Valid,Dirty,UninCredit,PhysValid
     maybe dir-orphan handling stuffed up
 
 12/ timeout/showstate in unmount
@@ -5178,3 +5180,16 @@ DONE 15d/ What does I_Dirty mean - and implement it.
     So just set I_Dirty and use that to flush inode to db at writeout.
     Any changes which must be in the next phase will come via setattr and
     so will wait for incompatible changes to be written out.
+
+ Reflecting on 7c - cluster_flush might find ->my_inode is NULL.
+  my_inode is set
+     lafs_import_inode
+         iget and mount-time stuff
+     lafs_inode_dblock
+
+  my_inode is cleared
+    When I_Destroyed is set and the last ref on the block is dropped
+    When inode_map_new_prepare claims an inodeblock
+
+  So we could easily not have a my_inode - e.g. just cleaning the data block.
+  ->my_inode cannot disappear while we hold the block, so a test is safe.
index d2467c9bf1c3e148c9fecd6279eceafbd89cdc29..188f3fcba53f274b3791e8d5c62a3faa7c26e743 100644 (file)
--- a/cluster.c
+++ b/cluster.c
@@ -677,7 +677,7 @@ unsigned long long lafs_cluster_allocate(struct block *b, int cnum)
        if (!test_bit(B_Index, &b->flags) &&
            LAFSI(b->inode)->type == TypeInodeFile) {
                struct inode *myi = dblk(b)->my_inode;
-               if (test_and_clear_bit(I_Dirty, &LAFSI(myi)->iflags))
+               if (myi && test_and_clear_bit(I_Dirty, &LAFSI(myi)->iflags))
                        lafs_inode_fillblock(myi);
        }
 
@@ -1254,9 +1254,9 @@ static void cluster_flush(struct fs *fs, int cnum)
                WARN_ON(credits == 0);
                lafs_space_return(fs, credits);
                if (!test_bit(B_Index, &b->flags) &&
-                   LAFSI(b->inode)->type == TypeInodeFile) {
+                   LAFSI(b->inode)->type == TypeInodeFile &&
+                   dblk(b)->my_inode) {
                        struct indexblock *ib;
-                       LAFS_BUG(!dblk(b)->my_inode, b);
                        spin_lock(&dblk(b)->my_inode->i_data.private_lock);
                        ib = getiref_locked(LAFSI(dblk(b)->my_inode)->iblock,
                                            MKREF(cflush));