[PATCH v2 1/1] mm: page_alloc: Avoid defining unused function

[PATCH v2 1/1] mm: page_alloc: Avoid defining unused function

[Andy Shevchenko]

In some configurations I got
mm/page_alloc.c:656:20: warning: unused function 'add_to_free_list' [-Wunused-function]
Becuase the only user is guarged with a certain ifdeffery,
do the same for add_to_free_list().

Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
v2: moved the function down to the respective ifdeffery block (Liam)
 mm/page_alloc.c | 16 ++++++++--------
 1 file changed, 8 insertions(+), 8 deletions(-)

diff --git a/mm/page_alloc.c b/mm/page_alloc.c
index 33d4a1be927b..cd584aace6bf 100644
--- a/mm/page_alloc.c
+++ b/mm/page_alloc.c
@@ -653,14 +653,6 @@ static inline void __add_to_free_list(struct page *page, struct zone *zone,
 	area->nr_free++;
 }
 
-static inline void add_to_free_list(struct page *page, struct zone *zone,
-				    unsigned int order, int migratetype,
-				    bool tail)
-{
-	__add_to_free_list(page, zone, order, migratetype, tail);
-	account_freepages(zone, 1 << order, migratetype);
-}
-
 /*
  * Used for pages which are on another list. Move the pages to the tail
  * of the list - so the moved pages won't immediately be considered for
@@ -6776,6 +6768,14 @@ bool is_free_buddy_page(const struct page *page)
 EXPORT_SYMBOL(is_free_buddy_page);
 
 #ifdef CONFIG_MEMORY_FAILURE
+static inline void add_to_free_list(struct page *page, struct zone *zone,
+				    unsigned int order, int migratetype,
+				    bool tail)
+{
+	__add_to_free_list(page, zone, order, migratetype, tail);
+	account_freepages(zone, 1 << order, migratetype);
+}
+
 /*
  * Break down a higher-order page in sub-pages, and keep our target out of
  * buddy allocator.
-- 
2.43.0.rc1.1336.g36b5255a03ac

Re: [PATCH v2 1/1] mm: page_alloc: Avoid defining unused function

[Andrew Morton]

On Tue, 23 Apr 2024 19:14:43 +0300 Andy Shevchenko <andriy.shevchenko@linux.intel.com> wrote:

> In some configurations I got
> mm/page_alloc.c:656:20: warning: unused function 'add_to_free_list' [-Wunused-function]
> Becuase the only user is guarged with a certain ifdeffery,
> do the same for add_to_free_list().
> 
> --- a/mm/page_alloc.c
> +++ b/mm/page_alloc.c
> @@ -653,14 +653,6 @@ static inline void __add_to_free_list(struct page *page, struct zone *zone,
>  	area->nr_free++;
>  }
>  
> -static inline void add_to_free_list(struct page *page, struct zone *zone,
> -				    unsigned int order, int migratetype,
> -				    bool tail)
> -{
> -	__add_to_free_list(page, zone, order, migratetype, tail);
> -	account_freepages(zone, 1 << order, migratetype);
> -}
> -
>  /*
>   * Used for pages which are on another list. Move the pages to the tail
>   * of the list - so the moved pages won't immediately be considered for
> @@ -6776,6 +6768,14 @@ bool is_free_buddy_page(const struct page *page)
>  EXPORT_SYMBOL(is_free_buddy_page);
>  
>  #ifdef CONFIG_MEMORY_FAILURE
> +static inline void add_to_free_list(struct page *page, struct zone *zone,
> +				    unsigned int order, int migratetype,
> +				    bool tail)
> +{
> +	__add_to_free_list(page, zone, order, migratetype, tail);
> +	account_freepages(zone, 1 << order, migratetype);
> +}
> +
>  /*
>   * Break down a higher-order page in sub-pages, and keep our target out of
>   * buddy allocator.

Thanks, I'll queue this as a fix against "mm: page_alloc: consolidate
free page accounting".

Please do tell us the config when fixing these things.  That way I can
do a little bisect to ensure that I correctly identified the offending
patch.

Re: [PATCH v2 1/1] mm: page_alloc: Avoid defining unused function

[Andy Shevchenko]

On Tue, Apr 23, 2024 at 11:10:00AM -0700, Andrew Morton wrote:
> On Tue, 23 Apr 2024 19:14:43 +0300 Andy Shevchenko <andriy.shevchenko@linux.intel.com> wrote:
> > In some configurations I got
> > mm/page_alloc.c:656:20: warning: unused function 'add_to_free_list' [-Wunused-function]
> > Becuase the only user is guarged with a certain ifdeffery,
> > do the same for add_to_free_list().

...

> Thanks, I'll queue this as a fix against "mm: page_alloc: consolidate
> free page accounting".

Thank you!

> Please do tell us the config when fixing these things.  That way I can
> do a little bisect to ensure that I correctly identified the offending
> patch.

Hmm... You mean defconfig? I can share it.

I built this with `make W=1`, probably that one helps, but the MM parts of
the defconfig have not been altered by me from the x86_64_defconfig.

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH v2 1/1] mm: page_alloc: Avoid defining unused function

[Johannes Weiner]

On Tue, Apr 23, 2024 at 09:27:07PM +0300, Andy Shevchenko wrote:
> On Tue, Apr 23, 2024 at 11:10:00AM -0700, Andrew Morton wrote:
> > On Tue, 23 Apr 2024 19:14:43 +0300 Andy Shevchenko <andriy.shevchenko@linux.intel.com> wrote:
> > > In some configurations I got
> > > mm/page_alloc.c:656:20: warning: unused function 'add_to_free_list' [-Wunused-function]
> > > Becuase the only user is guarged with a certain ifdeffery,
> > > do the same for add_to_free_list().
> 
> ...
> 
> > Thanks, I'll queue this as a fix against "mm: page_alloc: consolidate
> > free page accounting".
> 
> Thank you!

Thanks for the fix. We switched most callsites to __add_to_free_list()
now, I didn't realize the memory failure code was the only one left.

Acked-by: Johannes Weiner <hannes@cmpxchg.org>

Re: [PATCH v2 1/1] mm: page_alloc: Avoid defining unused function

[Andy Shevchenko]

On Tue, Apr 23, 2024 at 04:30:20PM -0400, Johannes Weiner wrote:
> On Tue, Apr 23, 2024 at 09:27:07PM +0300, Andy Shevchenko wrote:
> > On Tue, Apr 23, 2024 at 11:10:00AM -0700, Andrew Morton wrote:
> > > On Tue, 23 Apr 2024 19:14:43 +0300 Andy Shevchenko <andriy.shevchenko@linux.intel.com> wrote:
> > > > In some configurations I got
> > > > mm/page_alloc.c:656:20: warning: unused function 'add_to_free_list' [-Wunused-function]
> > > > Becuase the only user is guarged with a certain ifdeffery,
> > > > do the same for add_to_free_list().
> > 
> > ...
> > 
> > > Thanks, I'll queue this as a fix against "mm: page_alloc: consolidate
> > > free page accounting".
> > 
> > Thank you!
> 
> Thanks for the fix. We switched most callsites to __add_to_free_list()
> now, I didn't realize the memory failure code was the only one left.

You're welcome! Hint to the future `make W=1` should be a must during
development.

> Acked-by: Johannes Weiner <hannes@cmpxchg.org>

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH v2 1/1] mm: page_alloc: Avoid defining unused function

[Miaohe Lin]

On 2024/4/24 0:14, Andy Shevchenko wrote:
> In some configurations I got
> mm/page_alloc.c:656:20: warning: unused function 'add_to_free_list' [-Wunused-function]
> Becuase the only user is guarged with a certain ifdeffery,

guarged? ifdeffery?

> do the same for add_to_free_list().

A Fixes tag might be needed.

> 
> Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>

Anyway, this patch looks good to me. Thanks.

Reviewed-by: Miaohe Lin <linmiaohe@huawei.com>
Thanks.
.

Re: [PATCH v2 1/1] mm: page_alloc: Avoid defining unused function

[Andy Shevchenko]

On Thu, Apr 25, 2024 at 02:25:24PM +0800, Miaohe Lin wrote:
> On 2024/4/24 0:14, Andy Shevchenko wrote:
> > In some configurations I got
> > mm/page_alloc.c:656:20: warning: unused function 'add_to_free_list' [-Wunused-function]
> > Becuase the only user is guarged with a certain ifdeffery,
> 
> guarged?

A typo, thanks!

> ifdeffery?

Yes, this is established term.

> > do the same for add_to_free_list().
> 
> A Fixes tag might be needed.

First of all, it's not really critical issue to fix. Also see below.

> > Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> 
> Anyway, this patch looks good to me. Thanks.
> 
> Reviewed-by: Miaohe Lin <linmiaohe@huawei.com>

Thank you, I believe it will be squashed to the original one as Andrew already
accepted it. But at the same time his workflow allows to update the commit
message in case it's going to be a separate change.

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH v2 1/1] mm: page_alloc: Avoid defining unused function

[Andy Shevchenko]

On Thu, Apr 25, 2024 at 12:15:32PM +0300, Andy Shevchenko wrote:
> On Thu, Apr 25, 2024 at 02:25:24PM +0800, Miaohe Lin wrote:
> > On 2024/4/24 0:14, Andy Shevchenko wrote:

...

> > > In some configurations I got
> > > mm/page_alloc.c:656:20: warning: unused function 'add_to_free_list' [-Wunused-function]
> > > Becuase the only user is guarged with a certain ifdeffery,
> > 
> > guarged?
> 
> A typo, thanks!

FWIW, Andrew fixed this along with "Because". Thank you, Andrew!

> > ifdeffery?
> 
> Yes, this is established term.
> 
> > > do the same for add_to_free_list().

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH v2 1/1] mm: page_alloc: Avoid defining unused function

[Miaohe Lin]

On 2024/4/25 17:45, Andy Shevchenko wrote:
> On Thu, Apr 25, 2024 at 12:15:32PM +0300, Andy Shevchenko wrote:
>> On Thu, Apr 25, 2024 at 02:25:24PM +0800, Miaohe Lin wrote:
>>> On 2024/4/24 0:14, Andy Shevchenko wrote:
> 
> ...
> 
>>>> In some configurations I got
>>>> mm/page_alloc.c:656:20: warning: unused function 'add_to_free_list' [-Wunused-function]
>>>> Becuase the only user is guarged with a certain ifdeffery,
>>>
>>> guarged?
>>
>> A typo, thanks!
> 
> FWIW, Andrew fixed this along with "Because". Thank you, Andrew!

Got it. Thanks both.
.

> 
>>> ifdeffery?
>>
>> Yes, this is established term.
>>
>>>> do the same for add_to_free_list().
>