2022-10-21 03:13:21

by Aiqun Yu (Maria)

Subject: [PATCH] mm/page_isolation: fix clang deadcode warning

When !CONFIG_VM_BUG_ON, there is warning of
clang-analyzer-deadcode.DeadStores:
Value stored to 'mt' during its initialization
is never read.

Signed-off-by: Maria Yu <[email protected]>
---
mm/page_isolation.c | 4 +---
1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/mm/page_isolation.c b/mm/page_isolation.c
index 04141a9bea70..51d5c8025f77 100644
--- a/mm/page_isolation.c
+++ b/mm/page_isolation.c
@@ -330,9 +330,7 @@ static int isolate_single_pageblock(unsigned long boundary_pfn, int flags,
zone->zone_start_pfn);

if (skip_isolation) {
- int mt = get_pageblock_migratetype(pfn_to_page(isolate_pageblock));
-
- VM_BUG_ON(!is_migrate_isolate(mt));
+ VM_BUG_ON(!is_migrate_isolate(get_pageblock_migratetype(pfn_to_page(isolate_pageblock))));
} else {
ret = set_migratetype_isolate(pfn_to_page(isolate_pageblock), migratetype,
flags, isolate_pageblock, isolate_pageblock + pageblock_nr_pages);
--
2.17.1

2022-10-21 03:46:53

by Aiqun Yu (Maria)

Subject: Re: [PATCH] mm/page_isolation: fix clang deadcode warning

On 10/21/2022 11:19 AM, Matthew Wilcox wrote:
> On Fri, Oct 21, 2022 at 11:09:53AM +0800, Maria Yu wrote:
>> When !CONFIG_VM_BUG_ON, there is warning of
>> clang-analyzer-deadcode.DeadStores:
>> Value stored to 'mt' during its initialization
>> is never read.
>
> Honestly, the cure is worse than the disease. I'd rather not have a
> line that's this long.
I don't like this long either. While the current Deadcode warning is
also annoying for our clang tidy check.

will the CONFIG check can be acceptable?
like:
#ifdef CONFIG_DEBUG_VM
or
if (IS_ENABLED(CONFIG_DEBUG_VM))

Any good ideas?

> >> - int mt = get_pageblock_migratetype(pfn_to_page(isolate_pageblock));
>> -
>> - VM_BUG_ON(!is_migrate_isolate(mt));
>> + VM_BUG_ON(!is_migrate_isolate(get_pageblock_migratetype(pfn_to_page(isolate_pageblock))));


--
Thx and BRs,
Aiqun(Maria) Yu

2022-10-21 03:52:06

by Matthew Wilcox

Subject: Re: [PATCH] mm/page_isolation: fix clang deadcode warning

On Fri, Oct 21, 2022 at 11:09:53AM +0800, Maria Yu wrote:
> When !CONFIG_VM_BUG_ON, there is warning of
> clang-analyzer-deadcode.DeadStores:
> Value stored to 'mt' during its initialization
> is never read.

Honestly, the cure is worse than the disease. I'd rather not have a
line that's this long.

> - int mt = get_pageblock_migratetype(pfn_to_page(isolate_pageblock));
> -
> - VM_BUG_ON(!is_migrate_isolate(mt));
> + VM_BUG_ON(!is_migrate_isolate(get_pageblock_migratetype(pfn_to_page(isolate_pageblock))));

2022-10-21 04:36:04

by Andrew Morton

Subject: Re: [PATCH] mm/page_isolation: fix clang deadcode warning

On Fri, 21 Oct 2022 11:26:22 +0800 "Aiqun(Maria) Yu" <[email protected]> wrote:

> > Honestly, the cure is worse than the disease. I'd rather not have a
> > line that's this long.
> I don't like this long either. While the current Deadcode warning is
> also annoying for our clang tidy check.
>
> will the CONFIG check can be acceptable?
> like:
> #ifdef CONFIG_DEBUG_VM
> or
> if (IS_ENABLED(CONFIG_DEBUG_VM))
>
> Any good ideas?

Put a newline in there?

Although the shortly following lines are nearly as long...

2022-10-21 05:39:48

by Aiqun Yu (Maria)

Subject: Re: [PATCH] mm/page_isolation: fix clang deadcode warning

On 10/21/2022 12:17 PM, Andrew Morton wrote:
> On Fri, 21 Oct 2022 11:26:22 +0800 "Aiqun(Maria) Yu" <[email protected]> wrote:
>
>>> Honestly, the cure is worse than the disease. I'd rather not have a
>>> line that's this long.
>> I don't like this long either. While the current Deadcode warning is
>> also annoying for our clang tidy check.
>>
>> will the CONFIG check can be acceptable?
>> like:
>> #ifdef CONFIG_DEBUG_VM
>> or
>> if (IS_ENABLED(CONFIG_DEBUG_VM))
>>
>> Any good ideas?
>
> Put a newline in there?
>
> Although the shortly following lines are nearly as long...
>
Thx for the suggestion. new patchset uploaded.

--
Thx and BRs,
Aiqun(Maria) Yu