Re: [PATCH RFC] preempt: Put preempt_enable() within an instrumentation*() section.

[Sebastian Andrzej Siewior <bigeasy@linutronix.de>]

[-- Attachment #1: Type: text/plain, Size: 1971 bytes --]

On 2023-04-18 14:16:11 [+0200], Thomas Gleixner wrote:
> On Thu, Mar 09 2023 at 08:27, Sebastian Andrzej Siewior wrote:
> > Callers of preempt_enable() can be within an noinstr section leading to:
> > | vmlinux.o: warning: objtool: native_sched_clock+0x97: call to preempt_schedule_notrace_thunk() leaves .noinstr.text section
> > | vmlinux.o: warning: objtool: kvm_clock_read+0x22: call to preempt_schedule_notrace_thunk() leaves .noinstr.text section
> > | vmlinux.o: warning: objtool: local_clock+0xb4: call to preempt_schedule_notrace_thunk() leaves .noinstr.text section
> 
> > | vmlinux.o: warning: objtool: enter_from_user_mode+0xea: call to preempt_schedule_thunk() leaves .noinstr.text section
> > | vmlinux.o: warning: objtool: syscall_enter_from_user_mode+0x140: call to preempt_schedule_thunk() leaves .noinstr.text section
> > | vmlinux.o: warning: objtool: syscall_enter_from_user_mode_prepare+0xf2: call to preempt_schedule_thunk() leaves .noinstr.text section
> > | vmlinux.o: warning: objtool: irqentry_enter_from_user_mode+0xea: call to preempt_schedule_thunk() leaves .noinstr.text section 
> 
> I'm confused where this preempt_enable is in those
> *_enter_from_user_mode() functions.

These are gone since commit
	f87d28673b71b ("entry: Fix noinstr warning in __enter_from_user_mode()")

which is part of v6.3-rc4.

> Kernel config and compiler version please.

The former three still exist as of v6.3-rc7. I've attached the kernel
config. I'm using gcc version 12.2.0 (Debian 12.2.0-14).

> >  #define preempt_enable() \
> >  do { \
> >  	barrier(); \
> > -	if (unlikely(preempt_count_dec_and_test())) \
> > +	if (unlikely(preempt_count_dec_and_test())) { \
> > +		instrumentation_begin(); \
> >  		__preempt_schedule(); \
> > +		instrumentation_end(); \
> > +	} \
> >  } while (0)
> 
> This would paper over a misplaced preempt_disable/enable() pair in
> noinstr code. I'm not really happy about that.

okay.

> Thanks,
> 
>         tglx

Sebastian

[-- Attachment #2: CONFIG-noinstr.xz --]
[-- Type: application/x-xz, Size: 27328 bytes --]