[patch] 2.3.20 simple fs-corruption fix [Re: [patch] [possible racein ext2] Re: how to write get_block?]

[Andrea Arcangeli]

On Sun, 10 Oct 1999, Andrea Arcangeli wrote:

>As the fix I am proposing is only a bit more than a one liner, I think I
>can implement it. Of course if there is a better fix I won't want my

I implemented it. Here my simple fix against 2.3.20:

--- 2.3.20/fs/buffer.c  Sun Oct 10 16:59:57 1999
+++ 2.3.20-pagecache/fs/buffer.c        Sun Oct 10 19:57:36 1999
@@ -1205,6 +1205,18 @@
        return 0;
 }
 
+static void unmap_buffer(struct buffer_head * bh)
+{
+       if (buffer_mapped(bh))
+       {
+               mark_buffer_clean(bh);
+               wait_on_buffer(bh);
+               clear_bit(BH_Uptodate, &bh->b_state);
+               clear_bit(BH_Mapped, &bh->b_state);
+               clear_bit(BH_Req, &bh->b_state);
+       }
+}
+
 /*
  * We don't have to release all buffers here, but
  * we have to be sure that no dirty buffer is left
@@ -1231,16 +1243,8 @@
                /*
                 * is this block fully flushed?
                 */
-               if (offset <= curr_off) {
-                       if (buffer_mapped(bh)) {
-                               mark_buffer_clean(bh);
-                               wait_on_buffer(bh);
-                               clear_bit(BH_Uptodate, &bh->b_state);
-                               clear_bit(BH_Mapped, &bh->b_state);
-                               clear_bit(BH_Req, &bh->b_state);
-                               bh->b_blocknr = 0;
-                       }
-               }
+               if (offset <= curr_off)
+                       unmap_buffer(bh);
                curr_off = next_off;
                bh = next;
        } while (bh != head);
@@ -1286,6 +1290,19 @@
        get_page(page);
 }
 
+static void unmap_underlying_metadata(struct buffer_head * bh)
+{
+       bh = get_hash_table(bh->b_dev, bh->b_blocknr, bh->b_size);
+       if (bh)
+       {
+               unmap_buffer(bh);
+               /* Here we could run brelse or bforget. We use
+                  bforget because it will try to put the buffer
+                  in the freelist. */
+               __bforget(bh);
+       }
+}
+
 /*
  * block_write_full_page() is SMP-safe - currently it's still
  * being called with the kernel lock held, but the code is ready.
@@ -1331,6 +1348,7 @@
                        err = inode->i_op->get_block(inode, block, bh, 1);
                        if (err)
                                goto out;
+                       unmap_underlying_metadata(bh);
                }
                set_bit(BH_Uptodate, &bh->b_state);
                mark_buffer_dirty(bh,0);
@@ -1420,6 +1438,7 @@
                        err = inode->i_op->get_block(inode, block, bh, 1);
                        if (err)
                                goto out;
+                       unmap_underlying_metadata(bh);
                }
 
                if (!buffer_uptodate(bh) && (start_offset || (end_bytes && (i == 
end_block)))) {
@@ -1582,6 +1601,7 @@
                        err = inode->i_op->get_block(inode, block, bh, 1);
                        if (err)
                                goto out;
+                       unmap_underlying_metadata(bh);
                }
 
                if (!buffer_uptodate(bh) && (start_offset || (end_bytes && (i == 
end_block)))) {



I know it has the downside of doing an hash lookup for each buffer we want
overlap on the pagecache for writing into it.

Andrea