On Mon, Nov 29, 2010 at 12:10 PM, Josef Bacik <josef@xxxxxxxxxx> wrote: > On Thu, Nov 25, 2010 at 05:52:47PM +0800, Miao Xie wrote: >> Btrfs has a number of BUG_ON()s, which may lead btrfs to unpleasant panic. >> Meanwhile, they are very ugly and should be handled more propriately. >> >> There are mainly two ways to deal with these BUG_ON()s. >> >> 1. For those errors which can be handled well by callers, we just return their >> error number to callers. >> >> 2. For others, We can force the filesystem readonly when it hits errors, which >> Âis what this patchset has done. Replaced BUG_ON() with the interface provided >> Âin this patchset, we will get error infomation via dmesg. Since btrfs is now >> readonly, we can save our data safely and umount it, then a btrfsck is >> recommended. >> >> By these ways, we can protect our filesystem from panic caused by those >> BUG_ONs. >> >> --- >> Âfs/btrfs/ctree.h    |  21 ++++++++++ >> Âfs/btrfs/disk-io.c   |  23 +++++++++++ >> Âfs/btrfs/super.c    | Â100 ++++++++++++++++++++++++++++++++++++++++++++++- >> Âfs/btrfs/transaction.c |  Â7 +++ >> Â4 files changed, 148 insertions(+), 3 deletions(-) >> > > Overall seems sane, but what about kernels that don't make these checks? ÂI'm ok > with "well sucks for them" as an answer, just want to make sure we've at least > though about it. > > Also I'm not sure marking the fs as broken is the right move here. ÂExt3/4 don't > do this, they just mount read-only, as long as you can still unmount the > filesystem everything comes out ok. ÂThink of the case where we just get a > spurious EIO, the fs should be fine the next time around, there's reason to > force the user to run fsck in this case. > Did you mean "there's no reason to"? Also I guess you mean this in the case when there is no redundancy (single and raid0) as the other cases should recover from spurious EIO at run time. -- To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
