* Re: [f2fs-dev] f2fs: factor out discard_cmd usage from general rb_tree use
@ 2023-03-24 11:21 Colin King (gmail)
2023-03-24 16:55 ` Jaegeuk Kim
0 siblings, 1 reply; 2+ messages in thread
From: Colin King (gmail) @ 2023-03-24 11:21 UTC (permalink / raw)
To: Jaegeuk Kim, Chao Yu, linux-f2fs-devel; +Cc: linux-kernel@vger.kernel.org
Hi,
static analysis with clang scan build has detected a potential issue
introduced by the following commit:
commit 7e9775a516ff6c1e73ee2b42ec563cafee38f42f
Author: Jaegeuk Kim <jaegeuk@kernel.org>
Date: Fri Mar 10 11:12:35 2023 -0800
f2fs: factor out discard_cmd usage from general rb_tree use
The warning is as follows:
fs/f2fs/segment.c:1425:4: warning: Value stored to 'tdc' is never read
[deadcode.DeadStores]
The while loop in function __update_discard_tree_range is as follows (+
my annotations):
while (1) {
struct rb_node *node;
struct discard_cmd *tdc = NULL;
### tdc is set to NULL
if (prev_dc) {
di.lstart = prev_dc->di.lstart + prev_dc->di.len;
if (di.lstart < lstart)
di.lstart = lstart;
if (di.lstart >= end)
break;
if (!next_dc || next_dc->di.lstart > end)
di.len = end - di.lstart;
else
di.len = next_dc->di.lstart - di.lstart;
di.start = start + di.lstart - lstart;
}
if (!di.len)
goto next;
if (prev_dc && prev_dc->state == D_PREP &&
prev_dc->bdev == bdev &&
__is_discard_back_mergeable(&di, &prev_dc->di,
max_discard_blocks)) {
prev_dc->di.len += di.len;
dcc->undiscard_blks += di.len;
__relocate_discard_cmd(dcc, prev_dc);
di = prev_dc->di;
tdc = prev_dc;
### tdc is set to prev_dc, however, it is not not read any more with th
introduction of the "goto next"" statement introduced in the commit
mentioned earlier
goto next;
}
if (next_dc && next_dc->state == D_PREP &&
next_dc->bdev == bdev &&
__is_discard_front_mergeable(&di, &next_dc->di,
max_discard_blocks)) {
next_dc->di.lstart = di.lstart;
next_dc->di.len += di.len;
next_dc->di.start = di.start;
dcc->undiscard_blks += di.len;
__relocate_discard_cmd(dcc, next_dc);
### tdc is always NULL, there is no path to this code where tdc is ever
set to a non-NULL value.
if (tdc)
__remove_discard_cmd(sbi, tdc);
goto next;
}
__insert_discard_cmd(sbi, bdev, di.lstart, di.start,
di.len);
next:
prev_dc = next_dc;
if (!prev_dc)
break;
node = rb_next(&prev_dc->rb_node);
next_dc = rb_entry_safe(node, struct discard_cmd, rb_node);
}
_______________________________________________
Linux-f2fs-devel mailing list
Linux-f2fs-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel
^ permalink raw reply [flat|nested] 2+ messages in thread* Re: [f2fs-dev] f2fs: factor out discard_cmd usage from general rb_tree use
2023-03-24 11:21 [f2fs-dev] f2fs: factor out discard_cmd usage from general rb_tree use Colin King (gmail)
@ 2023-03-24 16:55 ` Jaegeuk Kim
0 siblings, 0 replies; 2+ messages in thread
From: Jaegeuk Kim @ 2023-03-24 16:55 UTC (permalink / raw)
To: Colin King (gmail); +Cc: linux-kernel@vger.kernel.org, linux-f2fs-devel
On 03/24, Colin King (gmail) wrote:
> Hi,
>
> static analysis with clang scan build has detected a potential issue
> introduced by the following commit:
>
> commit 7e9775a516ff6c1e73ee2b42ec563cafee38f42f
> Author: Jaegeuk Kim <jaegeuk@kernel.org>
> Date: Fri Mar 10 11:12:35 2023 -0800
>
> f2fs: factor out discard_cmd usage from general rb_tree use
Good catch!
I found the bug and will post v2 soon.
>
>
> The warning is as follows:
>
> fs/f2fs/segment.c:1425:4: warning: Value stored to 'tdc' is never read
> [deadcode.DeadStores]
>
> The while loop in function __update_discard_tree_range is as follows (+ my
> annotations):
>
>
> while (1) {
> struct rb_node *node;
> struct discard_cmd *tdc = NULL;
>
> ### tdc is set to NULL
>
> if (prev_dc) {
> di.lstart = prev_dc->di.lstart + prev_dc->di.len;
> if (di.lstart < lstart)
> di.lstart = lstart;
> if (di.lstart >= end)
> break;
>
> if (!next_dc || next_dc->di.lstart > end)
> di.len = end - di.lstart;
> else
> di.len = next_dc->di.lstart - di.lstart;
> di.start = start + di.lstart - lstart;
> }
>
> if (!di.len)
> goto next;
>
> if (prev_dc && prev_dc->state == D_PREP &&
> prev_dc->bdev == bdev &&
> __is_discard_back_mergeable(&di, &prev_dc->di,
>
> max_discard_blocks)) {
> prev_dc->di.len += di.len;
> dcc->undiscard_blks += di.len;
> __relocate_discard_cmd(dcc, prev_dc);
> di = prev_dc->di;
> tdc = prev_dc;
>
> ### tdc is set to prev_dc, however, it is not not read any more with th
> introduction of the "goto next"" statement introduced in the commit
> mentioned earlier
>
> goto next;
> }
>
> if (next_dc && next_dc->state == D_PREP &&
> next_dc->bdev == bdev &&
> __is_discard_front_mergeable(&di, &next_dc->di,
>
> max_discard_blocks)) {
> next_dc->di.lstart = di.lstart;
> next_dc->di.len += di.len;
> next_dc->di.start = di.start;
> dcc->undiscard_blks += di.len;
> __relocate_discard_cmd(dcc, next_dc);
>
> ### tdc is always NULL, there is no path to this code where tdc is ever set
> to a non-NULL value.
>
> if (tdc)
> __remove_discard_cmd(sbi, tdc);
> goto next;
> }
>
> __insert_discard_cmd(sbi, bdev, di.lstart, di.start,
> di.len);
> next:
> prev_dc = next_dc;
> if (!prev_dc)
> break;
>
> node = rb_next(&prev_dc->rb_node);
> next_dc = rb_entry_safe(node, struct discard_cmd, rb_node);
> }
>
_______________________________________________
Linux-f2fs-devel mailing list
Linux-f2fs-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel
^ permalink raw reply [flat|nested] 2+ messages in thread
end of thread, other threads:[~2023-03-24 16:56 UTC | newest]
Thread overview: 2+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2023-03-24 11:21 [f2fs-dev] f2fs: factor out discard_cmd usage from general rb_tree use Colin King (gmail)
2023-03-24 16:55 ` Jaegeuk Kim
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).