* [PATCH] Revert "mm/page_alloc: make should_fail_alloc_page() static"
@ 2021-07-08 19:11 Matteo Croce
2021-07-09 9:08 ` Mel Gorman
0 siblings, 1 reply; 6+ messages in thread
From: Matteo Croce @ 2021-07-08 19:11 UTC (permalink / raw)
To: Mel Gorman, Linux-MM
Cc: Andrew Morton, Michal Hocko, David Hildenbrand, Vlastimil Babka,
Dan Streetman, Yang Shi, LKML
From: Matteo Croce <mcroce@microsoft.com>
This reverts commit f7173090033c70886d925995e9dfdfb76dbb2441.
Fix an unresolved symbol error when CONFIG_DEBUG_INFO_BTF=y:
LD vmlinux
BTFIDS vmlinux
FAILED unresolved symbol should_fail_alloc_page
make: *** [Makefile:1199: vmlinux] Error 255
make: *** Deleting file 'vmlinux'
Fixes: f7173090033c ("mm/page_alloc: make should_fail_alloc_page() static")
Signed-off-by: Matteo Croce <mcroce@microsoft.com>
---
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 d6e94cc8066c..7b6405f8ff48 100644
--- a/mm/page_alloc.c
+++ b/mm/page_alloc.c
@@ -3831,7 +3831,7 @@ static inline bool __should_fail_alloc_page(gfp_t gfp_mask, unsigned int order)
#endif /* CONFIG_FAIL_PAGE_ALLOC */
-static noinline bool should_fail_alloc_page(gfp_t gfp_mask, unsigned int order)
+noinline bool should_fail_alloc_page(gfp_t gfp_mask, unsigned int order)
{
return __should_fail_alloc_page(gfp_mask, order);
}
--
2.31.1
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH] Revert "mm/page_alloc: make should_fail_alloc_page() static"
2021-07-08 19:11 [PATCH] Revert "mm/page_alloc: make should_fail_alloc_page() static" Matteo Croce
@ 2021-07-09 9:08 ` Mel Gorman
0 siblings, 0 replies; 6+ messages in thread
From: Mel Gorman @ 2021-07-09 9:08 UTC (permalink / raw)
To: Matteo Croce
Cc: Linux-MM, Andrew Morton, Michal Hocko, David Hildenbrand,
Vlastimil Babka, Dan Streetman, Yang Shi, LKML
On Thu, Jul 08, 2021 at 09:11:28PM +0200, Matteo Croce wrote:
> From: Matteo Croce <mcroce@microsoft.com>
>
> This reverts commit f7173090033c70886d925995e9dfdfb76dbb2441.
>
> Fix an unresolved symbol error when CONFIG_DEBUG_INFO_BTF=y:
>
> LD vmlinux
> BTFIDS vmlinux
> FAILED unresolved symbol should_fail_alloc_page
> make: *** [Makefile:1199: vmlinux] Error 255
> make: *** Deleting file 'vmlinux'
>
> Fixes: f7173090033c ("mm/page_alloc: make should_fail_alloc_page() static")
> Signed-off-by: Matteo Croce <mcroce@microsoft.com>
Acked-by: Mel Gorman <mgorman@techsingularity.net>
--
Mel Gorman
SUSE Labs
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] Revert "mm/page_alloc: make should_fail_alloc_page() static"
2021-07-05 10:38 Marco Elver
2021-07-05 11:37 ` Mel Gorman
@ 2021-07-05 11:55 ` Christoph Hellwig
1 sibling, 0 replies; 6+ messages in thread
From: Christoph Hellwig @ 2021-07-05 11:55 UTC (permalink / raw)
To: Marco Elver
Cc: akpm, glider, dvyukov, linux-kernel, linux-mm, kasan-dev,
Andrii Nakryiko, Daniel Borkmann, Vlastimil Babka, Yang Shi, bpf,
Mel Gorman, Alexei Starovoitov
On Mon, Jul 05, 2021 at 12:38:06PM +0200, Marco Elver wrote:
> This reverts commit f7173090033c70886d925995e9dfdfb76dbb2441.
>
> Commit 76cd61739fd1 ("mm/error_inject: Fix allow_error_inject function
> signatures") explicitly made should_fail_alloc_page() non-static, due to
> worries of remaining compiler optimizations in the absence of function
> side-effects while being noinline.
>
> Furthermore, kernel/bpf/verifier.c pushes should_fail_alloc_page onto
> the btf_non_sleepable_error_inject BTF IDs set, which when enabling
> CONFIG_DEBUG_INFO_BTF results in an error at the BTFIDS stage:
>
> FAILED unresolved symbol should_fail_alloc_page
>
> To avoid the W=1 warning, add a function declaration right above the
> function itself, with a comment it is required in a BTF IDs set.
NAK. We're not going to make symbols pointlessly global for broken
instrumentation coe. Someone needs to fixthis eBPF mess as we had
the same kind of issue before already.
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] Revert "mm/page_alloc: make should_fail_alloc_page() static"
2021-07-05 11:37 ` Mel Gorman
@ 2021-07-05 11:44 ` Marco Elver
0 siblings, 0 replies; 6+ messages in thread
From: Marco Elver @ 2021-07-05 11:44 UTC (permalink / raw)
To: Mel Gorman
Cc: akpm, glider, dvyukov, linux-kernel, linux-mm, kasan-dev,
Andrii Nakryiko, Daniel Borkmann, Vlastimil Babka, Yang Shi, bpf,
Alexei Starovoitov
On Mon, Jul 05, 2021 at 12:37PM +0100, Mel Gorman wrote:
> On Mon, Jul 05, 2021 at 12:38:06PM +0200, Marco Elver wrote:
> > This reverts commit f7173090033c70886d925995e9dfdfb76dbb2441.
> >
> > Commit 76cd61739fd1 ("mm/error_inject: Fix allow_error_inject function
> > signatures") explicitly made should_fail_alloc_page() non-static, due to
> > worries of remaining compiler optimizations in the absence of function
> > side-effects while being noinline.
> >
> > Furthermore, kernel/bpf/verifier.c pushes should_fail_alloc_page onto
> > the btf_non_sleepable_error_inject BTF IDs set, which when enabling
> > CONFIG_DEBUG_INFO_BTF results in an error at the BTFIDS stage:
> >
> > FAILED unresolved symbol should_fail_alloc_page
> >
> > To avoid the W=1 warning, add a function declaration right above the
> > function itself, with a comment it is required in a BTF IDs set.
> >
> > Fixes: f7173090033c ("mm/page_alloc: make should_fail_alloc_page() static")
> > Cc: Mel Gorman <mgorman@techsingularity.net>
> > Cc: Alexei Starovoitov <ast@kernel.org>
> > Signed-off-by: Marco Elver <elver@google.com>
>
> Acked-by: Mel Gorman <mgorman@techsingularity.net>
>
> Out of curiousity though, why does block/blk-core.c not require
> something similar for should_fail_bio?
It seems kernel/bpf/verifier.c doesn't refer to it in an BTF IDs set.
Looks like should_fail_alloc_page is special for BPF purposes. I'm not a
BPF maintainer, so hopefully someone can explain why
should_fail_alloc_page is special for BPF.
Thanks,
-- Marco
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] Revert "mm/page_alloc: make should_fail_alloc_page() static"
2021-07-05 10:38 Marco Elver
@ 2021-07-05 11:37 ` Mel Gorman
2021-07-05 11:44 ` Marco Elver
2021-07-05 11:55 ` Christoph Hellwig
1 sibling, 1 reply; 6+ messages in thread
From: Mel Gorman @ 2021-07-05 11:37 UTC (permalink / raw)
To: Marco Elver
Cc: akpm, glider, dvyukov, linux-kernel, linux-mm, kasan-dev,
Andrii Nakryiko, Daniel Borkmann, Vlastimil Babka, Yang Shi, bpf,
Alexei Starovoitov
On Mon, Jul 05, 2021 at 12:38:06PM +0200, Marco Elver wrote:
> This reverts commit f7173090033c70886d925995e9dfdfb76dbb2441.
>
> Commit 76cd61739fd1 ("mm/error_inject: Fix allow_error_inject function
> signatures") explicitly made should_fail_alloc_page() non-static, due to
> worries of remaining compiler optimizations in the absence of function
> side-effects while being noinline.
>
> Furthermore, kernel/bpf/verifier.c pushes should_fail_alloc_page onto
> the btf_non_sleepable_error_inject BTF IDs set, which when enabling
> CONFIG_DEBUG_INFO_BTF results in an error at the BTFIDS stage:
>
> FAILED unresolved symbol should_fail_alloc_page
>
> To avoid the W=1 warning, add a function declaration right above the
> function itself, with a comment it is required in a BTF IDs set.
>
> Fixes: f7173090033c ("mm/page_alloc: make should_fail_alloc_page() static")
> Cc: Mel Gorman <mgorman@techsingularity.net>
> Cc: Alexei Starovoitov <ast@kernel.org>
> Signed-off-by: Marco Elver <elver@google.com>
Acked-by: Mel Gorman <mgorman@techsingularity.net>
Out of curiousity though, why does block/blk-core.c not require
something similar for should_fail_bio?
--
Mel Gorman
SUSE Labs
^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH] Revert "mm/page_alloc: make should_fail_alloc_page() static"
@ 2021-07-05 10:38 Marco Elver
2021-07-05 11:37 ` Mel Gorman
2021-07-05 11:55 ` Christoph Hellwig
0 siblings, 2 replies; 6+ messages in thread
From: Marco Elver @ 2021-07-05 10:38 UTC (permalink / raw)
To: elver, akpm
Cc: glider, dvyukov, linux-kernel, linux-mm, kasan-dev,
Andrii Nakryiko, Daniel Borkmann, Vlastimil Babka, Yang Shi, bpf,
Mel Gorman, Alexei Starovoitov
This reverts commit f7173090033c70886d925995e9dfdfb76dbb2441.
Commit 76cd61739fd1 ("mm/error_inject: Fix allow_error_inject function
signatures") explicitly made should_fail_alloc_page() non-static, due to
worries of remaining compiler optimizations in the absence of function
side-effects while being noinline.
Furthermore, kernel/bpf/verifier.c pushes should_fail_alloc_page onto
the btf_non_sleepable_error_inject BTF IDs set, which when enabling
CONFIG_DEBUG_INFO_BTF results in an error at the BTFIDS stage:
FAILED unresolved symbol should_fail_alloc_page
To avoid the W=1 warning, add a function declaration right above the
function itself, with a comment it is required in a BTF IDs set.
Fixes: f7173090033c ("mm/page_alloc: make should_fail_alloc_page() static")
Cc: Mel Gorman <mgorman@techsingularity.net>
Cc: Alexei Starovoitov <ast@kernel.org>
Signed-off-by: Marco Elver <elver@google.com>
---
mm/page_alloc.c | 8 +++++++-
1 file changed, 7 insertions(+), 1 deletion(-)
diff --git a/mm/page_alloc.c b/mm/page_alloc.c
index d6e94cc8066c..16e71d48d84e 100644
--- a/mm/page_alloc.c
+++ b/mm/page_alloc.c
@@ -3831,7 +3831,13 @@ static inline bool __should_fail_alloc_page(gfp_t gfp_mask, unsigned int order)
#endif /* CONFIG_FAIL_PAGE_ALLOC */
-static noinline bool should_fail_alloc_page(gfp_t gfp_mask, unsigned int order)
+/*
+ * should_fail_alloc_page() is only called by page_alloc.c, however, is also
+ * included in a BTF IDs set and must remain non-static. Declare it to avoid a
+ * "missing prototypes" warning, and make it clear this is intentional.
+ */
+bool should_fail_alloc_page(gfp_t gfp_mask, unsigned int order);
+noinline bool should_fail_alloc_page(gfp_t gfp_mask, unsigned int order)
{
return __should_fail_alloc_page(gfp_mask, order);
}
--
2.32.0.93.g670b81a890-goog
^ permalink raw reply related [flat|nested] 6+ messages in thread
end of thread, other threads:[~2021-07-09 9:08 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-07-08 19:11 [PATCH] Revert "mm/page_alloc: make should_fail_alloc_page() static" Matteo Croce
2021-07-09 9:08 ` Mel Gorman
-- strict thread matches above, loose matches on Subject: below --
2021-07-05 10:38 Marco Elver
2021-07-05 11:37 ` Mel Gorman
2021-07-05 11:44 ` Marco Elver
2021-07-05 11:55 ` Christoph Hellwig
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).