2024-04-23 16:15:50

by Andy Shevchenko

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

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 <[email protected]>
---
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



2024-04-23 18:10:08

by Andrew Morton

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

On Tue, 23 Apr 2024 19:14:43 +0300 Andy Shevchenko <[email protected]> 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.


2024-04-23 18:32:41

by Andy Shevchenko

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

On Tue, Apr 23, 2024 at 11:10:00AM -0700, Andrew Morton wrote:
> On Tue, 23 Apr 2024 19:14:43 +0300 Andy Shevchenko <[email protected]> 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



2024-04-23 20:38:36

by Johannes Weiner

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

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 <[email protected]> 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 <[email protected]>

2024-04-24 00:46:40

by Andy Shevchenko

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

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 <[email protected]> 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 <[email protected]>

--
With Best Regards,
Andy Shevchenko



2024-04-25 06:25:35

by Miaohe Lin

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

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 <[email protected]>

Anyway, this patch looks good to me. Thanks.

Reviewed-by: Miaohe Lin <[email protected]>
Thanks.
.

2024-04-25 09:15:46

by Andy Shevchenko

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

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 <[email protected]>
>
> Anyway, this patch looks good to me. Thanks.
>
> Reviewed-by: Miaohe Lin <[email protected]>

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



2024-04-25 09:46:02

by Andy Shevchenko

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

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



2024-04-25 11:23:38

by Miaohe Lin

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

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().
>