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 12:23:43 +0100
From:   Peter Zijlstra <peterz@infradead.org>
To:     Mathieu Desnoyers <mathieu.desnoyers@efficios.com>
Cc:     linux-kernel@vger.kernel.org, Thomas Gleixner <tglx@linutronix.de>,
        "Paul E . McKenney" <paulmck@kernel.org>,
        Boqun Feng <boqun.feng@gmail.com>,
        "H . Peter Anvin" <hpa@zytor.com>, Paul Turner <pjt@google.com>,
        linux-api@vger.kernel.org,
        Christian Brauner <christian.brauner@ubuntu.com>,
        Florian Weimer <fw@deneb.enyo.de>, David.Laight@ACULAB.COM,
        carlos@redhat.com, Peter Oskolkov <posk@posk.io>
Subject: Re: [RFC PATCH 1/3] Introduce per thread group current virtual cpu id
Message-ID: <20220202112343.GZ20638@worktop.programming.kicks-ass.net>
References: <20220201192540.10439-1-mathieu.desnoyers@efficios.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <20220201192540.10439-1-mathieu.desnoyers@efficios.com>
Precedence: bulk

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.