| Message ID | 1428371810-11085-1-git-send-email-david@fromorbit.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Tue, Apr 07, 2015 at 11:56:50AM +1000, Dave Chinner wrote: > From: Dave Chinner <dchinner@redhat.com> > > The "brd" kernel ram disk abuses BLKFLSBUF to mean "free all memory > in the ram drive" when in fact it actually means "flush all dirty > buffers to stable storage". the brd driver ignores BLKFLSBUF if > there is an active reference to the block device, (e.g. a fs is > mounted on it), but when a device is layered over the top of it > (e.g. dm-flakey, lvm devices, etc) then the applications and > filesystems hold references to the upper device, not the brd device. > Hence when the upper device passes down BLKFLSBUF to brd, it removes > all the pages in the brd, effectively erasing it. This causes all > sorts of problems..... > > Fix this by black listing "/dev/ramXXX" devices from tests that > require DM in some way. The _requires_sane_bdev_flush() macro is > called by the _requires_dm.... checks so that we don't have to > remember to add this to all new tests that use dm in some way. > > Signed-off-by: Dave Chinner <dchinner@redhat.com> > --- Sounds good to me... looks like we have a similar check in libxfs (platform_flush_device()). Reviewed-by: Brian Foster <bfoster@redhat.com> > common/rc | 34 +++++++++++++++++++++++----------- > tests/generic/081 | 1 - > 2 files changed, 23 insertions(+), 12 deletions(-) > > diff --git a/common/rc b/common/rc > index c5db0dd..ca8da7f 100644 > --- a/common/rc > +++ b/common/rc > @@ -1305,26 +1305,38 @@ _require_block_device() > fi > } > > +# brd based ram disks erase the device when they receive a flush command when no > +# active references are present. This causes problems for DM devices sitting on > +# top of brd devices as DM doesn't hold active references to the brd device. > +_require_sane_bdev_flush() > +{ > + echo $1 | grep -q "^/dev/ram[0-9]\+$" > + if [ $? -eq 0 ]; then > + _notrun "This test requires a sane block device flush" > + fi > +} > + > # this test requires the device mapper flakey target > # > _require_dm_flakey() > { > - # require SCRATCH_DEV to be a valid block device > - _require_block_device $SCRATCH_DEV > - _require_command "$DMSETUP_PROG" dmsetup > + # require SCRATCH_DEV to be a valid block device with sane BLKFLSBUF > + # behaviour > + _require_block_device $SCRATCH_DEV > + _require_sane_bdev_flush $SCRATCH_DEV > + _require_command "$DMSETUP_PROG" dmsetup > > - modprobe dm-flakey >/dev/null 2>&1 > - $DMSETUP_PROG targets | grep flakey >/dev/null 2>&1 > - if [ $? -eq 0 ] > - then > - : > - else > - _notrun "This test requires dm flakey support" > - fi > + modprobe dm-flakey >/dev/null 2>&1 > + $DMSETUP_PROG targets | grep flakey >/dev/null 2>&1 > + if [ $? -ne 0 ]; then > + _notrun "This test requires dm flakey support" > + fi > } > > _require_dm_snapshot() > { > + _require_block_device $SCRATCH_DEV > + _require_sane_bdev_flush $SCRATCH_DEV > _require_command "$DMSETUP_PROG" dmsetup > modprobe dm-snapshot >/dev/null 2>&1 > $DMSETUP_PROG targets | grep -q snapshot > diff --git a/tests/generic/081 b/tests/generic/081 > index e242c4c..5d38c11 100755 > --- a/tests/generic/081 > +++ b/tests/generic/081 > @@ -50,7 +50,6 @@ _supported_os Linux > _require_test > _require_scratch_nocheck > _require_dm_snapshot > -_require_block_device $SCRATCH_DEV > _require_command $LVM_PROG lvm > > echo "Silence is golden" > -- > 2.0.0 > > -- > To unsubscribe from this list: send the line "unsubscribe fstests" in > the body of a message to majordomo@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html -- To unsubscribe from this list: send the line "unsubscribe fstests" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Tue, Apr 07, 2015 at 11:56:50AM +1000, Dave Chinner wrote: > From: Dave Chinner <dchinner@redhat.com> > > The "brd" kernel ram disk abuses BLKFLSBUF to mean "free all memory > in the ram drive" when in fact it actually means "flush all dirty > buffers to stable storage". Actually, what BLKFLSBUF means is to "flush all dirty buffers to stable storage --- and then invalidate/kill all buffers for the underyling block device." The reason why the ramdisk driver has special semantics BLKFLSBUF is an accident of how it was originally implemented, back in the 0.1x days of Linux, where it was implemented as a set of buffers in the buffer cache that weren't actually backed by any device. Since they were marked dirty, they wouldn't actually get dropped by the buffer cache. It was a dirty hack, but back in the days where they were used to populate the ramdisk from a floppy disk, it worked well enough. It happened that how BLKFLSBUF was implemented, without having to do anything special, the fact that BLKFLSBUF would invalidate all of the buffers from the buffer cache, userspace code would use this as a way of causing the ram disk to release memory. Given the desire not to break userspace, when the brd device was implemented to no longer depend on the buffer cache, the "special" semantics of BLKFLSBUF were preserved. We probably should have allocated a new ioctl for the explicit purpose of releasing memory, and then tracked down all users of the magic BLKFLSBUF ioctl. I don't think there should be that many --- in fact, it may very well be that init/do_mounts_initrd.c in the kernel sources might be one of the few still around, since the days of boot/root floppies are long before us. Cheers, - Ted -- To unsubscribe from this list: send the line "unsubscribe fstests" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/common/rc b/common/rc index c5db0dd..ca8da7f 100644 --- a/common/rc +++ b/common/rc @@ -1305,26 +1305,38 @@ _require_block_device() fi } +# brd based ram disks erase the device when they receive a flush command when no +# active references are present. This causes problems for DM devices sitting on +# top of brd devices as DM doesn't hold active references to the brd device. +_require_sane_bdev_flush() +{ + echo $1 | grep -q "^/dev/ram[0-9]\+$" + if [ $? -eq 0 ]; then + _notrun "This test requires a sane block device flush" + fi +} + # this test requires the device mapper flakey target # _require_dm_flakey() { - # require SCRATCH_DEV to be a valid block device - _require_block_device $SCRATCH_DEV - _require_command "$DMSETUP_PROG" dmsetup + # require SCRATCH_DEV to be a valid block device with sane BLKFLSBUF + # behaviour + _require_block_device $SCRATCH_DEV + _require_sane_bdev_flush $SCRATCH_DEV + _require_command "$DMSETUP_PROG" dmsetup - modprobe dm-flakey >/dev/null 2>&1 - $DMSETUP_PROG targets | grep flakey >/dev/null 2>&1 - if [ $? -eq 0 ] - then - : - else - _notrun "This test requires dm flakey support" - fi + modprobe dm-flakey >/dev/null 2>&1 + $DMSETUP_PROG targets | grep flakey >/dev/null 2>&1 + if [ $? -ne 0 ]; then + _notrun "This test requires dm flakey support" + fi } _require_dm_snapshot() { + _require_block_device $SCRATCH_DEV + _require_sane_bdev_flush $SCRATCH_DEV _require_command "$DMSETUP_PROG" dmsetup modprobe dm-snapshot >/dev/null 2>&1 $DMSETUP_PROG targets | grep -q snapshot diff --git a/tests/generic/081 b/tests/generic/081 index e242c4c..5d38c11 100755 --- a/tests/generic/081 +++ b/tests/generic/081 @@ -50,7 +50,6 @@ _supported_os Linux _require_test _require_scratch_nocheck _require_dm_snapshot -_require_block_device $SCRATCH_DEV _require_command $LVM_PROG lvm echo "Silence is golden"