invalidate buffers on blkdev_put
PATCH 1/1 invalidate buffers on blkdev_put
Hi guys,
I had sent this to Al, but couldn't tell if he hated it, thought it was broken, or just didn't think it was required. So, I opted for wider testing. This only affects 2.4.10pre15+, as it was caused by the blkdev changes there.
Anyway, the bug looks like this:
dd if=ext2-image-1 of=/dev/ram0
mount /dev/ram0 /mnt
umount /mnt
dd if=ext2-image-2 of=/dev/ram0
mount /dev/ram0 /mnt
ls -la /mnt (FS looks corrupted and wrong).
The problem is that on unmount, the ramdisk's buffer cache isn't cleared because bd_openers is still one. So, even if a new image is copied in, you still see the old image's superblock/inodes etc on mount.
In this case, we want to leave the dirty pages in the page cache, but get rid of the buffer cache copies.
This should not drop any updated data in the ramdisk because rd_blkdev_pagecache_IO dirties pages as the higher layers send down modified buffer heads (and other rd.c funcs do the same). Patch is below. Linus, please consider (pending lack of nays from Al).
An additional patch is probably to remove the second invalidate_buffers call, since only the filesystems should have buffer cache entries.
-chris
fs/block_dev.c Mon, 24 Sep 2001 15:11:49 -0400 renamed from fs/block_dev.c Sun, 23 Sep 2001 20:11:16 -0400 +3 -1
@@ -802,8 +802,10 @@ unlock_super(sb); drop_super(sb); } - } else if (kind == BDEV_FS) + } else if (kind == BDEV_FS) { fsync_no_super(rdev); + invalidate_buffers(rdev); + } if (!--bdev->bd_openers) { truncate_inode_pages(bd_inode->i_mapping, 0); invalidate_buffers(rdev);
Quoted 22 lines · Chris Mason
Hi guys,
I had sent this to Al, but couldn't tell if he hated it, thought it was broken, or just didn't think it was required. So, I opted for wider testing. This only affects 2.4.10pre15+, as it was caused by the blkdev changes there.
Anyway, the bug looks like this:
dd if=ext2-image-1 of=/dev/ram0
mount/dev/ram0/mnt
umount /mntdd if=ext2-image-2 of=/dev/ram0
mount/dev/ram0/mnt
ls -la /mnt (FS looks corrupted and wrong).The problem is that on unmount, the ramdisk's buffer cache isn't cleared because
bd_openersis still one. So, even if a new image is copied in, you still see the old image's superblock/inodes etc on mount.
Quoted 7 lines
In this case, we want to leave the dirty pages in the page cache, but get rid of the buffer cache copies.
This should not drop any updated data in the ramdisk because
rd_blkdev_pagecache_IOdirties pages as the higher layers send down modified buffer heads (and otherrd.cfuncs do the same). Patch is below. Linus, please consider (pending lack of nays from Al).
OK, not exactly nay, but... What you are trying to do is a workaround for problem that can be solved in somewhat saner way. Namely, we can make getblk() return buffres backed by pages from device page cache.
It's _not_ an obvious step. The most sensitive parts are
* need to allocate all bdev pages with GFP_BUFFER. Otherwise
we'll eat flaming death on VM deadlocks. Doable (we can set ->i_data.gfp_mask) but may lead to interesting effects wrt VM balancing.
* implementation of bforget()
* we'll need to kill buffer hash or deal with the new access path
to page-private buffer_heads.
It's solvable, but not obvious. It _does_ solve coherency problems between device page cache and buffer cache (thus killing update_buffers() and its ilk), but the last issue (new access path to page-private buffer_heads) may be rather nasty.
Quoted 4 lines · Alexander Viro
OK, not exactly nay, but... What you are trying to do is a workaround for problem that can be solved in somewhat saner way. Namely, we can make
getblk()return buffres backed by pages from device page cache.
I now have the patches for this, but I have to fix up fs/block_dev.c to also honour the block size thing because otherwise the two are still not in sync.
I'll send out a test-patch later this evening, I hope.
Quoted 4 lines
It's solvable, but not obvious. It _does_ solve coherency problems between device page cache and buffer cache (thus killing
update_buffers()and its ilk), but the last issue (new access path to page-privatebuffer_heads) may be rather nasty.
It's certainly solvable, but it is also certainly very fraught with tons of small details. I'll be very happy if people end up looking through the patches _very_ critically (and don't even bother testing them if you don't have a machine where you can lose a filesystem or two).
Hopefully in another hour or two (but the first version is going to have some ugly stuff in it still).
Linus
Hi!
Quoted 15 lines
Quoted 3 lines
OK, not exactly nay, but... What you are trying to do is a workaround for problem that can be solved in somewhat saner way. Namely, we can make
getblk()return buffres backed by pages from device page cache.I now have the patches for this, but I have to fix up
fs/block_dev.cto also honour the block size thing because otherwise the two are still not in sync.I'll send out a test-patch later this evening, I hope.
Quoted 4 lines
It's solvable, but not obvious. It _does_ solve coherency problems between device page cache and buffer cache (thus killing
update_buffers()and its ilk), but the last issue (new access path to page-privatebuffer_heads) may be rather nasty.
Quoted 4 lines
It's certainly solvable, but it is also certainly very fraught with tons of small details. I'll be very happy if people end up looking through the patches _very_ critically (and don't even bother testing them if you don't have a machine where you can lose a filesystem or two).
Time to rename 2.4.10 to 2.5.0? ;-)
Pavel
Signature · 2 lines
Philips Velo 1: 1"x4"x8", 300gram, 60, 12MB, 40bogomips, linux, mutt, details at http://atrey.karlin.mff.cuni.cz/~pavel/velo/index.html.
Quoted 7 lines · Pavel Machek
Hi!
Quoted 5 lines
Quoted 4 lines
It's solvable, but not obvious. It _does_ solve coherency problems between device page cache and buffer cache (thus killing
update_buffers()and its ilk), but the last issue (new access path to page-privatebuffer_heads) may be rather nasty.
Quoted 6 lines
Quoted 4 lines
It's certainly solvable, but it is also certainly very fraught with tons of small details. I'll be very happy if people end up looking through the patches _very_ critically (and don't even bother testing them if you don't have a machine where you can lose a filesystem or two).
Time to rename 2.4.10 to 2.5.0? ;-)
<Waving hand> It broke my feature patch. Good enough for me. ; >
Signature · 1 line
The time is now 22:19 (Totalitarian) - http://www.ccops.org/clock.html