public inbox for linux-kernel@vger.kernel.org
 help / color / mirror / Atom feed
From: Thomas Gleixner <tglx@linutronix.de>
To: Mathieu Desnoyers <mathieu.desnoyers@efficios.com>,
	LKML <linux-kernel@vger.kernel.org>
Cc: Michael Jeanson <mjeanson@efficios.com>,
	Jens Axboe <axboe@kernel.dk>,
	Peter Zijlstra <peterz@infradead.org>,
	"Paul E. McKenney" <paulmck@kernel.org>,
	Boqun Feng <boqun.feng@gmail.com>,
	Paolo Bonzini <pbonzini@redhat.com>,
	Sean Christopherson <seanjc@google.com>,
	Wei Liu <wei.liu@kernel.org>, Dexuan Cui <decui@microsoft.com>,
	x86@kernel.org, Arnd Bergmann <arnd@arndb.de>,
	Heiko Carstens <hca@linux.ibm.com>,
	Christian Borntraeger <borntraeger@linux.ibm.com>,
	Sven Schnelle <svens@linux.ibm.com>,
	Huacai Chen <chenhuacai@kernel.org>,
	Paul Walmsley <paul.walmsley@sifive.com>,
	Palmer Dabbelt <palmer@dabbelt.com>
Subject: Re: [patch V4 28/36] rseq: Switch to fast path processing on exit to user
Date: Thu, 11 Sep 2025 18:47:32 +0200	[thread overview]
Message-ID: <87v7lpq6l7.ffs@tglx> (raw)
In-Reply-To: <84d9beb2-85e7-4fc0-b403-0514bd87ae8b@efficios.com>

On Thu, Sep 11 2025 at 10:44, Mathieu Desnoyers wrote:
> On 2025-09-08 17:32, Thomas Gleixner wrote:
>>   void __rseq_signal_deliver(int sig, struct pt_regs *regs);
>> @@ -142,11 +148,16 @@ static inline void rseq_fork(struct task
>>   	} else {
>>   		t->rseq = current->rseq;
>>   		t->rseq.ids.cpu_cid = ~0ULL;
>
> As discussed earlier, do we really want to clear cpu_cid here, or
> copy from parent ? If we keep the parent's cached values, I suspect

Suspicion is not really a good engineering principle.

> we can skip the page fault on return from fork in many cases.

I doubt that it's many cases, see below.

>> +		/*
>> +		 * If it has rseq, force it into the slow path right away
>> +		 * because it is guaranteed to fault.
>> +		 */
>> +		t->rseq.event.slowpath = t->rseq.event.has_rseq;
>
> I think we can do better here. It's only guaranteed to fault if:
>
> - has_rseq is set, AND
>    - cpu or cid has changed compared to the cached value OR
>    - rseq_cs user pointer is non-NULL.
>
> Otherwise we should be able to handle the return from fork from the fast
> path just with loads from the rseq area, or am I missing something ?

The normal case is that the fault is pretty much guaranteed to happen
because the scheduler places the child on a different CPU and therefore
the CPU/MM IDs need to be updated anyway.

The only cases where this is not true, are when there is no capacity to
do so or on UP or the parent was affine to a single CPU, which is what
the child inherits.

Now for those cases the only time, where such an optimization would be
meaningful is when the child is so short lived, that it does not
schedule.  Otherwise you just procrastinate the fault to a later point
in time.

The point is that it will fault in the vast majority of cases and then
the fault will happen in the fast path first, which means the exit code
has to go another round through the work loop instead of forcing the
fault right away on the first exit in the slowpath, where it can be
actually resolved.

> Thanks,
>
> Mathieu

Your method of ending a mail with a greeting and then continuing to
reply downwards is pretty annoying. I almost missed the below if I
wouldn't have tried to trim the reply.

>>   	}
>>   }
>>   
>>   #else /* CONFIG_RSEQ */
>> -static inline void rseq_handle_notify_resume(struct ksignal *ksig, struct pt_regs *regs) { }
>> +static inline void rseq_handle_slowpath(struct pt_regs *regs) { }
>>   static inline void rseq_signal_deliver(struct ksignal *ksig, struct pt_regs *regs) { }
>>   static inline void rseq_sched_switch_event(struct task_struct *t) { }
>>   static inline void rseq_sched_set_task_cpu(struct task_struct *t, unsigned int cpu) { }
>> --- a/init/Kconfig
>> +++ b/init/Kconfig
>> @@ -1911,7 +1911,7 @@ config RSEQ_DEBUG_DEFAULT_ENABLE
>>   config DEBUG_RSEQ
>>   	default n
>>   	bool "Enable debugging of rseq() system call" if EXPERT
>> -	depends on RSEQ && DEBUG_KERNEL
>> +	depends on RSEQ && DEBUG_KERNEL && !GENERIC_ENTRY
>
> I'm confused about this hunk. Perhaps this belongs to a different
> commit ?

This is attached to wiring up the fast path in GENERIC_ENTRY, because
the fastpath, which does debug when enabled through the static key, is
at the end of the exit to user work loop and therefore the extra round
through the detached debug muck can be avoided.

While on !GENERIC_ENTRY architectures the processing happens always in
the TIF_NOTIFY_RESUME slowpath, so the extra debug muck is required, no?

> Mathieu

<TRIM pointless leftovers>

      Can you please trim your replies?

</>

Thanks,

        tglx

  parent reply	other threads:[~2025-09-11 16:47 UTC|newest]

Thread overview: 83+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2025-09-08 21:31 [patch V4 00/36] rseq: Optimize exit to user space Thomas Gleixner
2025-09-08 21:31 ` [patch V4 01/36] rseq: Avoid pointless evaluation in __rseq_notify_resume() Thomas Gleixner
2025-09-08 21:31 ` [patch V4 02/36] rseq: Condense the inline stubs Thomas Gleixner
2025-09-08 21:31 ` [patch V4 03/36] rseq: Move algorithm comment to top Thomas Gleixner
2025-09-08 21:31 ` [patch V4 04/36] rseq: Remove the ksig argument from rseq_handle_notify_resume() Thomas Gleixner
2025-09-08 21:31 ` [patch V4 05/36] rseq: Simplify registration Thomas Gleixner
2025-09-08 21:31 ` [patch V4 06/36] rseq: Simplify the event notification Thomas Gleixner
2025-09-09 13:18   ` Mathieu Desnoyers
2025-09-08 21:31 ` [patch V4 07/36] rseq, virt: Retrigger RSEQ after vcpu_run() Thomas Gleixner
2025-09-09  0:00   ` Sean Christopherson
2025-09-09 12:10     ` Thomas Gleixner
2025-09-09 13:21   ` Mathieu Desnoyers
2025-09-08 21:31 ` [patch V4 08/36] rseq: Avoid CPU/MM CID updates when no event pending Thomas Gleixner
2025-09-09 13:25   ` Mathieu Desnoyers
2025-09-08 21:31 ` [patch V4 09/36] rseq: Introduce struct rseq_data Thomas Gleixner
2025-09-09 13:30   ` Mathieu Desnoyers
2025-09-12 20:44     ` Thomas Gleixner
2025-09-12 21:33       ` Mathieu Desnoyers
2025-09-08 21:31 ` [patch V4 10/36] entry: Cleanup header Thomas Gleixner
2025-09-08 21:31 ` [patch V4 11/36] entry: Remove syscall_enter_from_user_mode_prepare() Thomas Gleixner
2025-09-09 13:33   ` Mathieu Desnoyers
2025-09-08 21:31 ` [patch V4 12/36] entry: Inline irqentry_enter/exit_from/to_user_mode() Thomas Gleixner
2025-09-09 13:38   ` Mathieu Desnoyers
2025-09-09 14:10     ` Thomas Gleixner
2025-09-09 14:59       ` Mathieu Desnoyers
2025-09-08 21:31 ` [patch V4 13/36] sched: Move MM CID related functions to sched.h Thomas Gleixner
2025-09-08 21:31 ` [patch V4 14/36] rseq: Cache CPU ID and MM CID values Thomas Gleixner
2025-09-09 13:43   ` Mathieu Desnoyers
2025-09-09 14:13     ` Thomas Gleixner
2025-09-09 15:01       ` Mathieu Desnoyers
2025-09-08 21:31 ` [patch V4 15/36] rseq: Record interrupt from user space Thomas Gleixner
2025-09-09 13:53   ` Mathieu Desnoyers
2025-09-09 14:17     ` Thomas Gleixner
2025-09-09 15:05       ` Mathieu Desnoyers
2025-09-08 21:31 ` [patch V4 16/36] rseq: Provide tracepoint wrappers for inline code Thomas Gleixner
2025-09-08 21:31 ` [patch V4 17/36] rseq: Expose lightweight statistics in debugfs Thomas Gleixner
2025-09-08 21:32 ` [patch V4 18/36] rseq: Provide static branch for runtime debugging Thomas Gleixner
2025-09-08 21:32 ` [patch V4 19/36] rseq: Provide and use rseq_update_user_cs() Thomas Gleixner
2025-09-09 15:11   ` Mathieu Desnoyers
2025-09-08 21:32 ` [patch V4 20/36] rseq: Replace the original debug implementation Thomas Gleixner
2025-09-08 21:32 ` [patch V4 21/36] rseq: Make exit debugging static branch based Thomas Gleixner
2025-09-08 21:32 ` [patch V4 22/36] rseq: Use static branch for syscall exit debug when GENERIC_IRQ_ENTRY=y Thomas Gleixner
2025-09-08 21:32 ` [patch V4 23/36] rseq: Provide and use rseq_set_ids() Thomas Gleixner
2025-09-11 13:40   ` Mathieu Desnoyers
2025-09-11 16:02     ` Thomas Gleixner
2025-09-11 17:13       ` Mathieu Desnoyers
2025-09-08 21:32 ` [patch V4 24/36] rseq: Separate the signal delivery path Thomas Gleixner
2025-09-08 21:32 ` [patch V4 25/36] rseq: Rework the TIF_NOTIFY handler Thomas Gleixner
2025-09-08 21:32 ` [patch V4 26/36] rseq: Optimize event setting Thomas Gleixner
2025-09-11 14:03   ` Mathieu Desnoyers
2025-09-11 16:06     ` Thomas Gleixner
2025-09-11 17:15       ` Mathieu Desnoyers
2025-09-12  6:58         ` Thomas Gleixner
2025-09-08 21:32 ` [patch V4 27/36] rseq: Implement fast path for exit to user Thomas Gleixner
2025-09-11 14:27   ` Mathieu Desnoyers
2025-09-11 16:08     ` Thomas Gleixner
2025-09-08 21:32 ` [patch V4 28/36] rseq: Switch to fast path processing on " Thomas Gleixner
2025-09-11 14:44   ` Mathieu Desnoyers
2025-09-11 14:45     ` Mathieu Desnoyers
2025-09-11 16:50       ` Thomas Gleixner
2025-09-11 16:47     ` Thomas Gleixner [this message]
2025-09-11 20:00       ` Mathieu Desnoyers
2025-09-12 14:22         ` Thomas Gleixner
2025-09-12 15:44           ` Mathieu Desnoyers
2025-09-08 21:32 ` [patch V4 29/36] entry: Split up exit_to_user_mode_prepare() Thomas Gleixner
2025-09-08 21:32 ` [patch V4 30/36] rseq: Split up rseq_exit_to_user_mode() Thomas Gleixner
2025-09-08 21:32 ` [patch V4 31/36] asm-generic: Provide generic TIF infrastructure Thomas Gleixner
2025-09-17  6:16   ` [tip: core/core] " tip-bot2 for Thomas Gleixner
2025-09-08 21:32 ` [patch V4 32/36] x86: Use generic TIF bits Thomas Gleixner
2025-09-17  6:16   ` [tip: core/core] " tip-bot2 for Thomas Gleixner
2025-09-08 21:32 ` [patch V4 33/36] s390: " Thomas Gleixner
2025-09-11  9:11   ` Sven Schnelle
2025-09-11 11:03   ` Heiko Carstens
2025-09-17  6:16   ` [tip: core/core] " tip-bot2 for Thomas Gleixner
2025-09-08 21:32 ` [patch V4 34/36] loongarch: " Thomas Gleixner
2025-09-17  6:16   ` [tip: core/core] " tip-bot2 for Thomas Gleixner
2025-09-08 21:32 ` [patch V4 35/36] riscv: " Thomas Gleixner
2025-09-17  6:16   ` [tip: core/core] " tip-bot2 for Thomas Gleixner
2025-09-08 21:32 ` [patch V4 36/36] rseq: Switch to TIF_RSEQ if supported Thomas Gleixner
2025-09-10 13:55 ` [patch V4 00/36] rseq: Optimize exit to user space Jens Axboe
2025-09-10 14:45   ` Michael Jeanson
2025-09-10 15:34     ` Jens Axboe
2025-09-10 14:54   ` Thomas Gleixner

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=87v7lpq6l7.ffs@tglx \
    --to=tglx@linutronix.de \
    --cc=arnd@arndb.de \
    --cc=axboe@kernel.dk \
    --cc=boqun.feng@gmail.com \
    --cc=borntraeger@linux.ibm.com \
    --cc=chenhuacai@kernel.org \
    --cc=decui@microsoft.com \
    --cc=hca@linux.ibm.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mathieu.desnoyers@efficios.com \
    --cc=mjeanson@efficios.com \
    --cc=palmer@dabbelt.com \
    --cc=paul.walmsley@sifive.com \
    --cc=paulmck@kernel.org \
    --cc=pbonzini@redhat.com \
    --cc=peterz@infradead.org \
    --cc=seanjc@google.com \
    --cc=svens@linux.ibm.com \
    --cc=wei.liu@kernel.org \
    --cc=x86@kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox