Re: [PATCH v2 3/4] sched: Compact RSEQ concurrency IDs in batches

[Gabriele Monaco <gmonaco@redhat.com>]

On Tue, 2025-08-26 at 14:10 -0400, Mathieu Desnoyers wrote:
> On 2025-07-16 12:06, Gabriele Monaco wrote:
> > Currently, task_mm_cid_work() is called from
> > resume_user_mode_work().
> > This can delay the execution of the corresponding thread for the
> > entire duration of the function, negatively affecting the response
> > in case of real time tasks.
> > In practice, we observe task_mm_cid_work increasing the latency of
> > 30-35us on a 128 cores system, this order of magnitude is
> > meaningful under PREEMPT_RT.
> > 
> > Run the task_mm_cid_work in batches of up to
> > CONFIG_RSEQ_CID_SCAN_BATCH CPUs, this reduces the duration of the
> > delay for each scan.
> > 
> > The task_mm_cid_work contains a mechanism to avoid running more
> > frequently than every 100ms. Keep this pseudo-periodicity only on
> > complete scans.
> > This means each call to task_mm_cid_work returns prematurely if the
> > period did not elapse and a scan is not ongoing (i.e. the next
> > batch to scan is not the first).
> > This way full scans are not excessively delayed while still keeping
> > each run, and introduced latency, short.
> 
> With your test hardware/workload as reference, do you have an idea of
> how many CPUs would be needed to require more than 100ms to iterate
> on all CPUs with the default scan batch size (8) ?

As you guessed, this is strongly dependent on the workload, where
workloads with less threads are more likely to take longer.
I used cyclictest (threads with 100us period) and hackbench (processes)
on a 128 CPUs machine and measured the time to complete the scan (16
iterations) as well as the time between non-complete scans (not delayed
by 100ms):

cyclictest: delay 0-400 us , complete scan 1.5-2 ms
hackbench: delay 5us - 3ms , complete scan 1.5-15 ms

So to answer your question, in the observed worst case for hackbench,
it would take more than 800 CPUs to reach the 100ms limit.

That said, the problematic latency was observed on a full scan (128
CPUs), so perhaps the default of 8 is a bit too conservative and could
easily be doubled.

Measurements showed these durations for each call to task_mm_cid_scan:

batch size  8:  1-11 us (majority below 10)
batch size 16:  3-16 us (majority below 10)
batch size 32: 10-21 us (majority above 15)

20 us is considered a relevant latency on this machine, so 16 seems a
good tradeoff for a batch size to me.


I'm going to include those numbers in the next iteration of the series.

...
> > +cid_compact:
> > +	if (!try_cmpxchg(&mm->mm_cid_scan_batch, &this_batch,
> > next_batch))
> > +		return;
> >   	cidmask = mm_cidmask(mm);
> >   	/* Clear cids that were not recently used. */
> > -	for_each_possible_cpu(cpu)
> > +	idx = 0;
> > +	cpu = from_cpu;
> > +	for_each_cpu_from(cpu, cpu_possible_mask) {
> > +		if (idx == CONFIG_RSEQ_CID_SCAN_BATCH)
> 
> could do "if (idx++ == CONFIG_RSEQ_CID_SCAN_BATCH)"
> 
> > +			break;
> >   		sched_mm_cid_remote_clear_old(mm, cpu);
> > +		++idx;
> 
> and remove this ^
> 
> > +	}
> >   	weight = cpumask_weight(cidmask);
> >   	/*
> >   	 * Clear cids that are greater or equal to the cidmask
> > weight to
> >   	 * recompact it.
> >   	 */
> > -	for_each_possible_cpu(cpu)
> > +	idx = 0;
> > +	cpu = from_cpu;
> > +	for_each_cpu_from(cpu, cpu_possible_mask) {
> > +		if (idx == CONFIG_RSEQ_CID_SCAN_BATCH)
> 
> Likewise.
> 
> > +			break;
> >   		sched_mm_cid_remote_clear_weight(mm, cpu, weight);
> > +		++idx;
> 
> Likewise.

Sure, will do.

Thanks,
Gabriele