diff mbox series

[v2,1/5] jump_label: Don't warn on __exit jump entries

Message ID 20190731161256.22210-2-andrew.murray@arm.com (mailing list archive)
State New, archived
Headers show
Series arm64: avoid out-of-line ll/sc atomics | expand

Commit Message

Andrew Murray July 31, 2019, 4:12 p.m. UTC
On architectures that discard .exit.* sections at runtime, a
warning is printed for each jump label that is used within an
in-kernel __exit annotated function:

can't patch jump_label at ehci_hcd_cleanup+0x8/0x3c
WARNING: CPU: 0 PID: 1 at kernel/jump_label.c:410 __jump_label_update+0x12c/0x138

As these functions will never get executed (they are free'd along
with the rest of initmem) - we do not need to patch them and should
not display any warnings.

The warning is displayed because the test required to satisfy
jump_entry_is_init is based on init_section_contains (__init_begin to
__init_end) whereas the test in __jump_label_update is based on
init_kernel_text (_sinittext to _einittext) via kernel_text_address).

In addition to fixing this, we also remove an out-of-date comment
and use a WARN instead of a WARN_ONCE.

Fixes: 19483677684b ("jump_label: Annotate entries that operate on __init code earlier")
Signed-off-by: Andrew Murray <andrew.murray@arm.com>
---
 kernel/jump_label.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

Comments

Peter Zijlstra July 31, 2019, 4:41 p.m. UTC | #1
On Wed, Jul 31, 2019 at 05:12:52PM +0100, Andrew Murray wrote:
> On architectures that discard .exit.* sections at runtime, a
> warning is printed for each jump label that is used within an
> in-kernel __exit annotated function:
> 
> can't patch jump_label at ehci_hcd_cleanup+0x8/0x3c
> WARNING: CPU: 0 PID: 1 at kernel/jump_label.c:410 __jump_label_update+0x12c/0x138
> 
> As these functions will never get executed (they are free'd along
> with the rest of initmem) - we do not need to patch them and should
> not display any warnings.
> 
> The warning is displayed because the test required to satisfy
> jump_entry_is_init is based on init_section_contains (__init_begin to
> __init_end) whereas the test in __jump_label_update is based on
> init_kernel_text (_sinittext to _einittext) via kernel_text_address).
> 
> In addition to fixing this, we also remove an out-of-date comment
> and use a WARN instead of a WARN_ONCE.
> 
> Fixes: 19483677684b ("jump_label: Annotate entries that operate on __init code earlier")
> Signed-off-by: Andrew Murray <andrew.murray@arm.com>
> ---
>  kernel/jump_label.c | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
> 
> diff --git a/kernel/jump_label.c b/kernel/jump_label.c
> index df3008419a1d..c984078a5c28 100644
> --- a/kernel/jump_label.c
> +++ b/kernel/jump_label.c
> @@ -407,7 +407,9 @@ static bool jump_label_can_update(struct jump_entry *entry, bool init)
>  		return false;
>  
>  	if (!kernel_text_address(jump_entry_code(entry))) {
> -		WARN_ONCE(1, "can't patch jump_label at %pS", (void *)jump_entry_code(entry));
> +		if (!jump_entry_is_init(entry))
> +			WARN_ONCE(1, "can't patch jump_label at %pS",
> +				  (void *)jump_entry_code(entry));

It seems to me we can writes that as:

		WARN_ONCE(!jump_entry_is_init(entry),
			  "can't patch jump_label at %pS",
			  (void *)jump_entry_code(entry));

>  		return false;
>  	}

Other than that,

Acked-by: Peter Zijlstra (Intel) <peterz@infradead.org>
Andrew Murray Aug. 2, 2019, 8:22 a.m. UTC | #2
On Wed, Jul 31, 2019 at 06:41:56PM +0200, Peter Zijlstra wrote:
> On Wed, Jul 31, 2019 at 05:12:52PM +0100, Andrew Murray wrote:
> > On architectures that discard .exit.* sections at runtime, a
> > warning is printed for each jump label that is used within an
> > in-kernel __exit annotated function:
> > 
> > can't patch jump_label at ehci_hcd_cleanup+0x8/0x3c
> > WARNING: CPU: 0 PID: 1 at kernel/jump_label.c:410 __jump_label_update+0x12c/0x138
> > 
> > As these functions will never get executed (they are free'd along
> > with the rest of initmem) - we do not need to patch them and should
> > not display any warnings.
> > 
> > The warning is displayed because the test required to satisfy
> > jump_entry_is_init is based on init_section_contains (__init_begin to
> > __init_end) whereas the test in __jump_label_update is based on
> > init_kernel_text (_sinittext to _einittext) via kernel_text_address).
> > 
> > In addition to fixing this, we also remove an out-of-date comment
> > and use a WARN instead of a WARN_ONCE.
> > 
> > Fixes: 19483677684b ("jump_label: Annotate entries that operate on __init code earlier")
> > Signed-off-by: Andrew Murray <andrew.murray@arm.com>
> > ---
> >  kernel/jump_label.c | 4 +++-
> >  1 file changed, 3 insertions(+), 1 deletion(-)
> > 
> > diff --git a/kernel/jump_label.c b/kernel/jump_label.c
> > index df3008419a1d..c984078a5c28 100644
> > --- a/kernel/jump_label.c
> > +++ b/kernel/jump_label.c
> > @@ -407,7 +407,9 @@ static bool jump_label_can_update(struct jump_entry *entry, bool init)
> >  		return false;
> >  
> >  	if (!kernel_text_address(jump_entry_code(entry))) {
> > -		WARN_ONCE(1, "can't patch jump_label at %pS", (void *)jump_entry_code(entry));
> > +		if (!jump_entry_is_init(entry))
> > +			WARN_ONCE(1, "can't patch jump_label at %pS",
> > +				  (void *)jump_entry_code(entry));
> 
> It seems to me we can writes that as:
> 
> 		WARN_ONCE(!jump_entry_is_init(entry),
> 			  "can't patch jump_label at %pS",
> 			  (void *)jump_entry_code(entry));
> 
> >  		return false;
> >  	}
> 
> Other than that,
> 
> Acked-by: Peter Zijlstra (Intel) <peterz@infradead.org>

Ah of course!

I'll make this change on my respin and add your ack. Thanks.

Thanks,

Andrew Murray
diff mbox series

Patch

diff --git a/kernel/jump_label.c b/kernel/jump_label.c
index df3008419a1d..c984078a5c28 100644
--- a/kernel/jump_label.c
+++ b/kernel/jump_label.c
@@ -407,7 +407,9 @@  static bool jump_label_can_update(struct jump_entry *entry, bool init)
 		return false;
 
 	if (!kernel_text_address(jump_entry_code(entry))) {
-		WARN_ONCE(1, "can't patch jump_label at %pS", (void *)jump_entry_code(entry));
+		if (!jump_entry_is_init(entry))
+			WARN_ONCE(1, "can't patch jump_label at %pS",
+				  (void *)jump_entry_code(entry));
 		return false;
 	}