Hello Andreas, Sorry for late reply, I've got some time to finish this: <...SNIP...> > > Suggested-by: Andreas Dilger <adilger.kernel@xxxxxxxxx> > Signed-off-by: Wang Shilong <wshilong@xxxxxxx> > --- I think this looks pretty reasonable, but the replacement of ext4_error() on some of the error paths with ext4_warning() seems like there is a chance that an error might be "lost" if one of the underlying functions misses its call to ext4_mark_group_bitmap_corrupted()->save_error_info(). I looked at the patch and the previous one together, and it _looks_ like all of the error paths are handled at some point lower down in the stack, but this has the potential to break in the future if a change doesn't call ext4_mark_group_bitmap_corrupted() itself for some reason. One option would be instead of ext4_warning() (or dropping completely) the ext4_error() messages, use ext4_mark_group_bitmap_corrupted() and change that function to not print an error message if the inode bitmap or block bitmap is already flagged as corrupted. That gives us code safety (the higher-level error check/message will catch any unhandled errors from below), while also reducing console spam (duplicate errors will not be printed, and the "most specific" error will print once to the console). --------> could you please more specific for this? I understood that we could add the check in ext4_mark_group_bitmap_corrupted() that only call ext4_error()/warnging() if necessary, but I did not catch your other point, you mean we should move ext4_mark_group_bitmap_corrupted() to higher-level call? Thanks, Shilong
