Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Date:   Wed, 2 Feb 2022 08:48:51 -0500 (EST)
From:   Mathieu Desnoyers <mathieu.desnoyers@efficios.com>
To:     Peter Zijlstra <peterz@infradead.org>
Cc:     linux-kernel <linux-kernel@vger.kernel.org>,
        Thomas Gleixner <tglx@linutronix.de>,
        paulmck <paulmck@kernel.org>, Boqun Feng <boqun.feng@gmail.com>,
        "H. Peter Anvin" <hpa@zytor.com>, Paul Turner <pjt@google.com>,
        linux-api <linux-api@vger.kernel.org>,
        Christian Brauner <christian.brauner@ubuntu.com>,
        Florian Weimer <fw@deneb.enyo.de>,
        David Laight <David.Laight@ACULAB.COM>,
        carlos <carlos@redhat.com>, Peter Oskolkov <posk@posk.io>
Message-ID: <465039213.27161.1643809731530.JavaMail.zimbra@efficios.com>
In-Reply-To: <20220202112343.GZ20638@worktop.programming.kicks-ass.net>
References: <20220201192540.10439-1-mathieu.desnoyers@efficios.com> <20220202112343.GZ20638@worktop.programming.kicks-ass.net>
Subject: Re: [RFC PATCH 1/3] Introduce per thread group current virtual cpu
 id
MIME-Version: 1.0
Content-Type: text/plain; charset=utf-8
Content-Transfer-Encoding: 7bit
Thread-Topic: Introduce per thread group current virtual cpu id
Thread-Index: yKlWk+fUpMMdDPyl2WKE2U6dJjx55g==
Precedence: bulk

----- On Feb 2, 2022, at 6:23 AM, Peter Zijlstra peterz@infradead.org wrote:

> On Tue, Feb 01, 2022 at 02:25:38PM -0500, Mathieu Desnoyers wrote:
> 
>> +static inline void tg_vcpu_get(struct task_struct *t)
>> +{
>> +	struct cpumask *cpumask = &t->signal->vcpu_mask;
>> +	unsigned int vcpu;
>> +
>> +	if (t->flags & PF_KTHREAD)
>> +		return;
>> +	/* Atomically reserve lowest available vcpu number. */
>> +	do {
>> +		vcpu = cpumask_first_zero(cpumask);
>> +		WARN_ON_ONCE(vcpu >= nr_cpu_ids);
>> +	} while (cpumask_test_and_set_cpu(vcpu, cpumask));
>> +	t->tg_vcpu = vcpu;
>> +}
>> +
>> +static inline void tg_vcpu_put(struct task_struct *t)
>> +{
>> +	if (t->flags & PF_KTHREAD)
>> +		return;
>> +	cpumask_clear_cpu(t->tg_vcpu, &t->signal->vcpu_mask);
>> +	t->tg_vcpu = 0;
>> +}
> 
>> diff --git a/kernel/sched/core.c b/kernel/sched/core.c
>> index 2e4ae00e52d1..2690e80977b1 100644
>> --- a/kernel/sched/core.c
>> +++ b/kernel/sched/core.c
>> @@ -4795,6 +4795,8 @@ prepare_task_switch(struct rq *rq, struct task_struct
>> *prev,
>>  	sched_info_switch(rq, prev, next);
>>  	perf_event_task_sched_out(prev, next);
>>  	rseq_preempt(prev);
>> +	tg_vcpu_put(prev);
>> +	tg_vcpu_get(next);
> 
> 
> URGGHHH!!! that's *2* atomics extra on the context switch path. Worse,
> that's on a line that's trivially contended with a few threads.

There is one obvious optimization that just begs to be done here: when
switching between threads belonging to the same process, we can simply
take the vcpu_id tag of the prev thread and use it for next,
without requiring any atomic operation.

This only leaves the overhead of added atomics when scheduling between
threads which belong to different processes. Does it matter as much ?
If it's the case, then we should really scratch our heads a little more
to come up with improvements.

Thanks,

Mathieu

-- 
Mathieu Desnoyers
EfficiOS Inc.
http://www.efficios.com