mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
From: Yao Kai <yaokai34@huawei.com>
To: Peter Zijlstra <peterz@infradead.org>
Cc: Thomas Gleixner <tglx@kernel.org>,
	Mathieu Desnoyers <mathieu.desnoyers@efficios.com>,
	Ingo Molnar <mingo@kernel.org>,
	Mark Rutland <mark.rutland@arm.com>,
	<linux-kernel@vger.kernel.org>, <liuyongqiang13@huawei.com>,
	<stable@vger.kernel.org>
Subject: Re: [PATCH] rseq: Prevent self-preemption when granting a time slice extension
Date: Tue, 27 Oct 2026 10:16:56 +0800	[thread overview]
Message-ID: <810801c8-55b2-45f9-81b8-cd95847fe530@huawei.com> (raw)
In-Reply-To: <20261026114038.GE2668200@noisy.programming.kicks-ass.net>



On 10/26/2026 7:40 PM, Peter Zijlstra wrote:
> On Fri, Oct 23, 2026 at 12:04:39PM +0800, Yao Kai wrote:
>> __rseq_grant_slice_extension() makes the grant visible to user space and
>> then reads the clock to compute the expiry time before it clears the
>> pending reschedule request. This runs in the exit to user mode loop with
>> interrupts enabled.
>>
>> Clocksource read functions are free to disable and enable preemption.
>> kvm_clock_read(), xen_clocksource_read() and the arm64 architected timer
>> counter accessors, which are used when a counter erratum workaround is
>> active, do so. With full or lazy preemption, when the request which
>> triggered the grant is a non-lazy TIF_NEED_RESCHED, e.g. a lazy request
>> upgraded by the scheduler tick, then preempt_enable_notrace() in the clock
>> read function schedules right away, e.g. in a KVM guest:
>>
>>    __schedule
>>    preempt_schedule_notrace
>>    preempt_schedule_notrace_thunk
>>    kvm_clock_get_cycles
>>    ktime_get_mono_fast_ns
>>    irqentry_exit
>>    asm_sysvec_apic_timer_interrupt
>>
>> The task is preempted with the grant already handed out to user space,
>> i.e. in the middle of the critical section which the extension is supposed
>> to protect. The stale grant is cleaned up when the task goes back to user
>> space, so the damage is limited to the extension silently not working for
>> such preemptions on systems which use one of these clocksources.
>>
>> Read the clock in the interrupt disabled section after the reschedule
>> request has been cleared.
>>
>> Fixes: dfb630f548a7 ("rseq: Implement rseq_grant_slice_extension()")
>> Cc: stable@vger.kernel.org
>> Signed-off-by: Yao Kai <yaokai34@huawei.com>
>> ---
>>   include/linux/rseq_entry.h | 14 ++++++++++++--
>>   1 file changed, 12 insertions(+), 2 deletions(-)
>>
>> diff --git a/include/linux/rseq_entry.h b/include/linux/rseq_entry.h
>> index 31ce349ed42c..3e6a339175f8 100644
>> --- a/include/linux/rseq_entry.h
>> +++ b/include/linux/rseq_entry.h
>> @@ -174,8 +174,6 @@ static __always_inline bool __rseq_grant_slice_extension(bool work_pending)
>>   	rseq_stat_inc(rseq_stats.s_granted);
>>   
>>   	curr->rseq.slice.state.granted = true;
>> -	/* Store expiry time for arming the timer on the way out */
>> -	curr->rseq.slice.expires = data_race(rseq_slice_ext_nsecs) + ktime_get_mono_fast_ns();
>>   	/*
>>   	 * This is racy against a remote CPU setting TIF_NEED_RESCHED in
>>   	 * several ways:
>> @@ -222,6 +220,18 @@ static __always_inline bool __rseq_grant_slice_extension(bool work_pending)
>>   	scoped_guard(irq) {
>>   		clear_tsk_need_resched(curr);
>>   		clear_preempt_need_resched();
>> +		/*
>> +		 * Store expiry time for arming the timer on the way out.
>> +		 *
>> +		 * Read the clock only after the reschedule request has been
>> +		 * cleared and with interrupts disabled. Clocksource read
>> +		 * functions, e.g. kvm_clock_read(), may disable and enable
>> +		 * preemption, which would otherwise schedule right here on a
>> +		 * pending request and void the grant which was just handed
>> +		 * out to user space.
>> +		 */
>> +		curr->rseq.slice.expires = data_race(rseq_slice_ext_nsecs) +
>> +					   ktime_get_mono_fast_ns();
>>   	}
>>   	return true;
> 
> I am confused; even with this patch on, a tick that happens after
> unsafe_put_user() can cause things to get rescheduled 'instantly'. That
> preemption happens from irqentry_exit_cond_resched(), and needs no help
> from any preempt_enable() usage.

Right, that window still exists and this patch does not close it. An
interrupt which hits between unsafe_put_user() and the interrupt
disabled section preempts via irqentry_exit_cond_resched() as before.
That's a race against a handful of instructions though.

The clock read does not need an interrupt at all. The request which the
grant is made for is still pending and folded into the preempt count when
preempt_enable_notrace() in kvm_clock_read() runs, so the task schedules
on every grant for a non-lazy request. I verified that in a KVM guest
with kvm-clock: without the patch the extension does not work there.

If that makes sense to you, I'll send a V2 which spells out that
distinction in the changelog.

Thanks,
Yao



      reply	other threads:[~2026-10-27  2:17 UTC|newest]

Thread overview: 3+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-10-23  4:04 Yao Kai
2026-10-26 11:40 ` Peter Zijlstra
2026-10-27  2:16   ` Yao Kai [this message]

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=810801c8-55b2-45f9-81b8-cd95847fe530@huawei.com \
    --to=yaokai34@huawei.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=liuyongqiang13@huawei.com \
    --cc=mark.rutland@arm.com \
    --cc=mathieu.desnoyers@efficios.com \
    --cc=mingo@kernel.org \
    --cc=peterz@infradead.org \
    --cc=stable@vger.kernel.org \
    --cc=tglx@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

all inboxes | Powered by JetHome®