diff mbox series

[03/14] mm/page_alloc: Make should_fail_alloc_page a static function should_fail_alloc_page static

Message ID 20210520084809.8576-4-mgorman@techsingularity.net (mailing list archive)
State New, archived
Headers show
Series Clean W=1 build warnings for mm/ | expand

Commit Message

Mel Gorman May 20, 2021, 8:47 a.m. UTC
make W=1 generates the following warning for mm/page_alloc.c

  mm/page_alloc.c:3651:15: warning: no previous prototype for ‘should_fail_alloc_page’ [-Wmissing-prototypes]
   noinline bool should_fail_alloc_page(gfp_t gfp_mask, unsigned int order)
                 ^~~~~~~~~~~~~~~~~~~~~~

This function is deliberately split out for BPF to allow errors to be
injected. The function is not used anywhere else so it is local to
the file. Make it static which should still allow error injection
to be used similar to how block/blk-core.c:should_fail_bio() works.

Signed-off-by: Mel Gorman <mgorman@techsingularity.net>
---
 mm/page_alloc.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Comments

Matteo Croce July 8, 2021, 7:18 p.m. UTC | #1
On Thu, 20 May 2021 09:47:58 +0100
Mel Gorman <mgorman@techsingularity.net> wrote:

> make W=1 generates the following warning for mm/page_alloc.c
> 
>   mm/page_alloc.c:3651:15: warning: no previous prototype for
> ‘should_fail_alloc_page’ [-Wmissing-prototypes] noinline bool
> should_fail_alloc_page(gfp_t gfp_mask, unsigned int order)
> ^~~~~~~~~~~~~~~~~~~~~~
> 
> This function is deliberately split out for BPF to allow errors to be
> injected. The function is not used anywhere else so it is local to
> the file. Make it static which should still allow error injection
> to be used similar to how block/blk-core.c:should_fail_bio() works.
> 
> Signed-off-by: Mel Gorman <mgorman@techsingularity.net>
> ---
>  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 aaa1655cf682..26cc1a4e639b 100644
> --- a/mm/page_alloc.c
> +++ b/mm/page_alloc.c
> @@ -3648,7 +3648,7 @@ static inline bool
> __should_fail_alloc_page(gfp_t gfp_mask, unsigned int order) 
>  #endif /* CONFIG_FAIL_PAGE_ALLOC */
>  
> -noinline bool should_fail_alloc_page(gfp_t gfp_mask, unsigned int
> order) +static noinline bool should_fail_alloc_page(gfp_t gfp_mask,
> unsigned int order) {
>  	return __should_fail_alloc_page(gfp_mask, order);
>  }


Hi Mel,

It seems that this breaks builds with CONFIG_DEBUG_INFO_BTF=y.
Maybe that warning was a false positive because
should_fail_alloc_page() is referenced via a macro?

I proposed to revert it, feel free to propose another fix.

Regards,
Mel Gorman July 9, 2021, 9:30 a.m. UTC | #2
On Thu, Jul 08, 2021 at 09:18:44PM +0200, Matteo Croce wrote:
> On Thu, 20 May 2021 09:47:58 +0100
> Mel Gorman <mgorman@techsingularity.net> wrote:
> 
> > make W=1 generates the following warning for mm/page_alloc.c
> > 
> >   mm/page_alloc.c:3651:15: warning: no previous prototype for
> > ???should_fail_alloc_page??? [-Wmissing-prototypes] noinline bool
> > should_fail_alloc_page(gfp_t gfp_mask, unsigned int order)
> > ^~~~~~~~~~~~~~~~~~~~~~
> > 
> > This function is deliberately split out for BPF to allow errors to be
> > injected. The function is not used anywhere else so it is local to
> > the file. Make it static which should still allow error injection
> > to be used similar to how block/blk-core.c:should_fail_bio() works.
> > 
> > Signed-off-by: Mel Gorman <mgorman@techsingularity.net>
> > ---
> >  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 aaa1655cf682..26cc1a4e639b 100644
> > --- a/mm/page_alloc.c
> > +++ b/mm/page_alloc.c
> > @@ -3648,7 +3648,7 @@ static inline bool
> > __should_fail_alloc_page(gfp_t gfp_mask, unsigned int order) 
> >  #endif /* CONFIG_FAIL_PAGE_ALLOC */
> >  
> > -noinline bool should_fail_alloc_page(gfp_t gfp_mask, unsigned int
> > order) +static noinline bool should_fail_alloc_page(gfp_t gfp_mask,
> > unsigned int order) {
> >  	return __should_fail_alloc_page(gfp_mask, order);
> >  }
> 
> 
> Hi Mel,
> 
> It seems that this breaks builds with CONFIG_DEBUG_INFO_BTF=y.
> Maybe that warning was a false positive because
> should_fail_alloc_page() is referenced via a macro?
> 
> I proposed to revert it, feel free to propose another fix.
> 

The alternative fix of making the symbol global was rejected. eBPF needs
to figure out a way of instrumenting code that is is unused by the kernel
and not globally visible but I don't know how that might be achieved.
diff mbox series

Patch

diff --git a/mm/page_alloc.c b/mm/page_alloc.c
index aaa1655cf682..26cc1a4e639b 100644
--- a/mm/page_alloc.c
+++ b/mm/page_alloc.c
@@ -3648,7 +3648,7 @@  static inline bool __should_fail_alloc_page(gfp_t gfp_mask, unsigned int order)
 
 #endif /* CONFIG_FAIL_PAGE_ALLOC */
 
-noinline bool should_fail_alloc_page(gfp_t gfp_mask, unsigned int order)
+static noinline bool should_fail_alloc_page(gfp_t gfp_mask, unsigned int order)
 {
 	return __should_fail_alloc_page(gfp_mask, order);
 }