diff mbox series

mm/page_isolation: fix clang deadcode warning

Message ID 20221021030953.34925-1-quic_aiquny@quicinc.com
State New
Headers
Series mm/page_isolation: fix clang deadcode warning |

Commit Message

Aiqun Yu (Maria) Oct. 21, 2022, 3:09 a.m. UTC 
  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 <quic_aiquny@quicinc.com>
---
 mm/page_isolation.c | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

Comments

Matthew Wilcox Oct. 21, 2022, 3:19 a.m. UTC | #1 
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))));
Aiqun Yu (Maria) Oct. 21, 2022, 3:26 a.m. UTC | #2 
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))));
Andrew Morton Oct. 21, 2022, 4:17 a.m. UTC | #3 
On Fri, 21 Oct 2022 11:26:22 +0800 "Aiqun(Maria) Yu" <quic_aiquny@quicinc.com> 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...
Aiqun Yu (Maria) Oct. 21, 2022, 5:21 a.m. UTC | #4 
On 10/21/2022 12:17 PM, Andrew Morton wrote:
> On Fri, 21 Oct 2022 11:26:22 +0800 "Aiqun(Maria) Yu" <quic_aiquny@quicinc.com> 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.
diff mbox series

Patch

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);