* [PATCH v2] ext4: Remove unnecessary assignments
@ 2021-11-04 6:36 cgel.zte
2022-01-05 23:24 ` Theodore Ts'o
0 siblings, 1 reply; 2+ messages in thread
From: cgel.zte @ 2021-11-04 6:36 UTC (permalink / raw)
To: Theodore Ts'o
Cc: Andreas Dilger, linux-ext4, linux-kernel, luo penghao, Zeal Robot
From: luo penghao <luo.penghao@zte.com.cn>
The assignment at the end of the function is not necessary
The clang_analyzer complains as follows:
fs/ext4/mballoc.c:3889:3 warning:
Value stored to 'err' is never read
change in v2:
Repair the sending email box
Reported-by: Zeal Robot <zealci@zte.com.cn>
Signed-off-by: luo penghao <luo.penghao@zte.com.cn>
---
fs/ext4/mballoc.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
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:
--
2.15.2
^ permalink raw reply related [flat|nested] 2+ messages in thread
* Re: [PATCH v2] ext4: Remove unnecessary assignments
2021-11-04 6:36 [PATCH v2] ext4: Remove unnecessary assignments cgel.zte
@ 2022-01-05 23:24 ` Theodore Ts'o
0 siblings, 0 replies; 2+ messages in thread
From: Theodore Ts'o @ 2022-01-05 23:24 UTC (permalink / raw)
To: cgel.zte
Cc: Andreas Dilger, linux-ext4, linux-kernel, luo penghao,
Zeal Robot, Harshad Shirwadkar
On Thu, Nov 04, 2021 at 06:36:37AM +0000, cgel.zte@gmail.com wrote:
> From: luo penghao <luo.penghao@zte.com.cn>
>
> 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
^ permalink raw reply [flat|nested] 2+ messages in thread
end of thread, other threads:[~2022-01-05 23:24 UTC | newest]
Thread overview: 2+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-11-04 6:36 [PATCH v2] ext4: Remove unnecessary assignments cgel.zte
2022-01-05 23:24 ` Theodore Ts'o
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).