On Thu, Nov 04, 2021 at 06:36:37AM +0000, cgel.zte@xxxxxxxxx wrote: > From: luo penghao <luo.penghao@xxxxxxxxxx> > > The assignment at the end of the function is not necessary > > diff --git a/fs/ext4/mballoc.c b/fs/ext4/mballoc.c > index 089c958..f1258a7 100644 > --- a/fs/ext4/mballoc.c > +++ b/fs/ext4/mballoc.c > @@ -3886,7 +3886,7 @@ void ext4_mb_mark_bb(struct super_block *sb, ext4_fsblk_t block, > if (err) > goto out_err; > sync_dirty_buffer(bitmap_bh); > - err = ext4_handle_dirty_metadata(NULL, NULL, gdp_bh); > + ext4_handle_dirty_metadata(NULL, NULL, gdp_bh); > sync_dirty_buffer(gdp_bh); > > out_err: There's actually a bigger issue here than the Clang analyzer complaining about the unnecessasary assignment --- and that is we *should* be propagating the error up to ext4_mb_mark_bb's callers, and those callers should be logging errors and potentially aborting the fast_commit replay. There might be some errors that can be ignored, if an idempotent operation doesn't need to be redone. However, in cases like ext4_handle_dirty_metadata(), or ext4_read_block_bitmap(), any failures are probably due to something fatal happening --- either an ENOMEM, or an I/O error, etc., and simply silently aborting the current function without logging any kind of problem is going to make it much harder to root cause a potential fast commit replay failure. So what we should probably do is make ext4_mb_mark_bb return an error, and then we'll need to look at all of the callers of ext4_mb_mark_bb, and fix them up as necessary. - Ted
