2022-09-09 09:30:22

by Miaohe Lin

Subject: [PATCH 09/16] mm/page_alloc: use local variable zone_idx directly

Use local variable zone_idx directly since it holds the exact value of
zone_idx(). No functional change intended.

Signed-off-by: Miaohe Lin <[email protected]>
---
mm/page_alloc.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/mm/page_alloc.c b/mm/page_alloc.c
index 94baf33da865..3417232afa45 100644
--- a/mm/page_alloc.c
+++ b/mm/page_alloc.c
@@ -6878,7 +6878,7 @@ void __ref memmap_init_zone_device(struct zone *zone,
unsigned long start = jiffies;
int nid = pgdat->node_id;

- if (WARN_ON_ONCE(!pgmap || zone_idx(zone) != ZONE_DEVICE))
+ if (WARN_ON_ONCE(!pgmap || zone_idx != ZONE_DEVICE))
return;

/*
--
2.23.0

2022-09-09 12:01:35

by David Hildenbrand

Subject: Re: [PATCH 09/16] mm/page_alloc: use local variable zone_idx directly

On 09.09.22 11:24, Miaohe Lin wrote:
> Use local variable zone_idx directly since it holds the exact value of
> zone_idx(). No functional change intended.
>
> Signed-off-by: Miaohe Lin <[email protected]>
> ---
> mm/page_alloc.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/mm/page_alloc.c b/mm/page_alloc.c
> index 94baf33da865..3417232afa45 100644
> --- a/mm/page_alloc.c
> +++ b/mm/page_alloc.c
> @@ -6878,7 +6878,7 @@ void __ref memmap_init_zone_device(struct zone *zone,
> unsigned long start = jiffies;
> int nid = pgdat->node_id;
>
> - if (WARN_ON_ONCE(!pgmap || zone_idx(zone) != ZONE_DEVICE))
> + if (WARN_ON_ONCE(!pgmap || zone_idx != ZONE_DEVICE))
> return;
>
> /*

Reviewed-by: David Hildenbrand <[email protected]>

--
Thanks,

David / dhildenb

2022-09-13 08:59:54

by Anshuman Khandual

Subject: Re: [PATCH 09/16] mm/page_alloc: use local variable zone_idx directly

On 9/9/22 14:54, Miaohe Lin wrote:
> Use local variable zone_idx directly since it holds the exact value of
> zone_idx(). No functional change intended.
>
> Signed-off-by: Miaohe Lin <[email protected]>

Reviewed-by: Anshuman Khandual <[email protected]>

> ---
> mm/page_alloc.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/mm/page_alloc.c b/mm/page_alloc.c
> index 94baf33da865..3417232afa45 100644
> --- a/mm/page_alloc.c
> +++ b/mm/page_alloc.c
> @@ -6878,7 +6878,7 @@ void __ref memmap_init_zone_device(struct zone *zone,
> unsigned long start = jiffies;
> int nid = pgdat->node_id;
>
> - if (WARN_ON_ONCE(!pgmap || zone_idx(zone) != ZONE_DEVICE))
> + if (WARN_ON_ONCE(!pgmap || zone_idx != ZONE_DEVICE))
> return;
>
> /*

2022-09-15 07:47:19

by Oscar Salvador

Subject: Re: [PATCH 09/16] mm/page_alloc: use local variable zone_idx directly

On Fri, Sep 09, 2022 at 05:24:44PM +0800, Miaohe Lin wrote:
> Use local variable zone_idx directly since it holds the exact value of
> zone_idx(). No functional change intended.
>
> Signed-off-by: Miaohe Lin <[email protected]>

By the way, zone_idx being "int" should be enough?

Reviewed-by: Oscar Salvador <[email protected]>

> ---
> mm/page_alloc.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/mm/page_alloc.c b/mm/page_alloc.c
> index 94baf33da865..3417232afa45 100644
> --- a/mm/page_alloc.c
> +++ b/mm/page_alloc.c
> @@ -6878,7 +6878,7 @@ void __ref memmap_init_zone_device(struct zone *zone,
> unsigned long start = jiffies;
> int nid = pgdat->node_id;
>
> - if (WARN_ON_ONCE(!pgmap || zone_idx(zone) != ZONE_DEVICE))
> + if (WARN_ON_ONCE(!pgmap || zone_idx != ZONE_DEVICE))
> return;
>
> /*
> --
> 2.23.0
>

--
Oscar Salvador
SUSE Labs

2022-09-15 08:20:28

by Miaohe Lin

Subject: Re: [PATCH 09/16] mm/page_alloc: use local variable zone_idx directly

On 2022/9/15 15:36, Oscar Salvador wrote:
> On Fri, Sep 09, 2022 at 05:24:44PM +0800, Miaohe Lin wrote:
>> Use local variable zone_idx directly since it holds the exact value of
>> zone_idx(). No functional change intended.
>>
>> Signed-off-by: Miaohe Lin <[email protected]>
>
> By the way, zone_idx being "int" should be enough?

Yes, it should be. But zone_idx is used as "unsigned long" around this function,
e.g. __init_zone_device_page expects zone_idx as "unsigned long", so it should be
better to keep it as is ?

>
> Reviewed-by: Oscar Salvador <[email protected]>

Many thanks for your review and comment in this series.

Thanks,
Miaohe Lin


>
>> ---
>> mm/page_alloc.c | 2 +-
>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/mm/page_alloc.c b/mm/page_alloc.c
>> index 94baf33da865..3417232afa45 100644
>> --- a/mm/page_alloc.c
>> +++ b/mm/page_alloc.c
>> @@ -6878,7 +6878,7 @@ void __ref memmap_init_zone_device(struct zone *zone,
>> unsigned long start = jiffies;
>> int nid = pgdat->node_id;
>>
>> - if (WARN_ON_ONCE(!pgmap || zone_idx(zone) != ZONE_DEVICE))
>> + if (WARN_ON_ONCE(!pgmap || zone_idx != ZONE_DEVICE))
>> return;
>>
>> /*
>> --
>> 2.23.0
>>
>