On 01/18/2011 03:56 AM, Chris Mason wrote: > Excerpts from liubo's message of 2011-01-06 06:30:25 -0500: >> This patch comes from "Forced readonly mounts on errors" ideas. >> >> As we know, this is the first step in being more fault tolerant of disk >> corruptions instead of just using BUG() statements. >> >> The major content: >> - add a framework for generating errors that should result in filesystems >> going readonly. >> - keep FS state in disk super block. >> - make sure that all of resource will be freed and released at umount time. >> - make sure that after FS is forced readonly on error, there will be no more >> disk change before FS is corrected. For this, we should stop write >> operation. >> >> After this patch is applied, the conversion from BUG() to such a framework >> can >> happen incrementally. > > I think this is a good overall framework and it will meet our needs > nicely as we scale up the error handling in the filesystem. > > One concern I have is where we save the error state to disk: > >> +static void __save_error_info(struct btrfs_fs_info *fs_info) >> +{ >> + struct btrfs_super_block *disk_super = &fs_info->super_copy; >> + >> + fs_info->fs_state = BTRFS_SUPER_FLAG_ERROR; >> + disk_super->flags |= cpu_to_le64(BTRFS_SUPER_FLAG_ERROR); >> + >> + mutex_lock(&fs_info->trans_mutex); >> + memcpy(&fs_info->super_for_commit, disk_super, >> + sizeof(fs_info->super_for_commit)); >> + mutex_unlock(&fs_info->trans_mutex); > > The super_for_commit isn't changed until we have a fully consistent set > of fields in the super block. The super_copy is changed as the > transaction progresses. > > So, this memcpy isn't quite safe. We should simply set the flag on the > super_for_commit and the super_copy individually. >
Got it, thanks for pointing it out. > I'll make this change and pull it in. We can build from here. > Great! thanks, Liubo > -chris > -- > To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in > the body of a message to majord...@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html > -- To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html