From nobody Wed Dec 17 15:53:03 2025 Received: from galois.linutronix.de (Galois.linutronix.de [193.142.43.55]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id AA05919B3F7 for ; Mon, 24 Jun 2024 14:54:19 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=193.142.43.55 ARC-Seal: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1719240861; cv=none; b=CJHcow5ClRtRl26UDMS+z2fI+SjIicnUSVJMKjsvbhr1v3hkDrDj6hLmrSCsuaHTAizTMQlwnQSDVtF9ViwMKeLELHuzkoFmekb47Wa9OiyewM3TJhwhYH9RJreKFRWdE42+biRqDIJL8JAf94Y5PSxaPQZXQ50LD6JdUwDbylo= ARC-Message-Signature: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1719240861; c=relaxed/simple; bh=ZCv5paGxTG7crUbvML1vFLJHx5FyrLUJsMMZPSG3ywI=; h=From:Date:Subject:MIME-Version:Content-Type:Message-Id:References: In-Reply-To:To:Cc; b=uRB9ml4iAw+3iraELLlt0eHngyBVPjMTSgUuYDD1bNKW2CZW1hLDEiMasbxAw6DFLSEXmW0NNVFBhAfHqN6gSoWlNf28qNJG8lnifURSpZsfnSqUIdVmiuyLFEPxRPOezQbDHPVycRs+apxHRg/aU4ZyirTAvFF+6HlecTr+4Y8= ARC-Authentication-Results: i=1; smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=linutronix.de; spf=pass smtp.mailfrom=linutronix.de; dkim=pass (2048-bit key) header.d=linutronix.de header.i=@linutronix.de header.b=Zl9VGHM3; dkim=permerror (0-bit key) header.d=linutronix.de header.i=@linutronix.de header.b=qoMeqJ4D; arc=none smtp.client-ip=193.142.43.55 Authentication-Results: smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=linutronix.de Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=linutronix.de Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=linutronix.de header.i=@linutronix.de header.b="Zl9VGHM3"; dkim=permerror (0-bit key) header.d=linutronix.de header.i=@linutronix.de header.b="qoMeqJ4D" From: Anna-Maria Behnsen DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linutronix.de; s=2020; t=1719240858; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=Jc+Xr3oxnKbk8xCRoHeTtoyZgCdu4Q1jJQGfCbv0wdA=; b=Zl9VGHM3bYO6+ACHHHRbqnOsU+IUbDzo4sazZxXR0MOZLH5dTaRvHoMznZvPTbduZs/VDc j6baRaC7YWr3PqJHDsbnZQUBkyRYP7BQL0nGWRnsYHMeIZ0ATmRdN1dkCIEMPYpQlpoMMg Z97y2q8juI8H7kNBeaG8L9jMyTNZLcKjrRcBtIYGgvIWNxABKbk5KG0s/Rngb68L74hMu6 AdEjiDpPBuItgILMRP3jMo7zru2lmS1A59PJcnGcyi1b1PxaUeWkVPq1A9aCjUDbIE/Z/P Ubu5697FfY6RQ7/BH5YhM/OaYzMeyn0rdt5F8shHoeF+Ex6aqYy0naAMkKo8ig== DKIM-Signature: v=1; a=ed25519-sha256; c=relaxed/relaxed; d=linutronix.de; s=2020e; t=1719240858; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=Jc+Xr3oxnKbk8xCRoHeTtoyZgCdu4Q1jJQGfCbv0wdA=; b=qoMeqJ4DFic2sASBo64Wclns9x+1k1UseBD4lM3sO+3pos6UVQh7yWqjBstKjViu8MTnRz X55IALn64Ly4UYCg== Date: Mon, 24 Jun 2024 16:53:53 +0200 Subject: [PATCH v2 1/5] timer_migration: Do not rely always on group->parent Precedence: bulk X-Mailing-List: linux-kernel@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: quoted-printable Message-Id: <20240624-tmigr-fixes-v2-1-3eb4c0604790@linutronix.de> References: <20240624-tmigr-fixes-v2-0-3eb4c0604790@linutronix.de> In-Reply-To: <20240624-tmigr-fixes-v2-0-3eb4c0604790@linutronix.de> To: Frederic Weisbecker , Thomas Gleixner , linux-kernel@vger.kernel.org Cc: Borislav Petkov , Anna-Maria Behnsen When reading group->parent without holding the group lock it is racy against CPUs coming online the first time and thereby creating another level of the hierarchy. This is not a problem when this value is read once to decide whether to abort a propagation or not. The worst outcome is an unnecessary/early CPU wake up. But it is racy when reading it several times during a single 'action' (like activation, deactivation, checking for remote timer expiry,...) and relying on the consitency of this value without holding the lock. This happens at the moment e.g. in tmigr_inactive_up() which is also calling tmigr_udpate_events(). Code relys on group->parent not to change during this 'action'. Update parent struct member description to explain the above only once. Remove parent pointer checks when they are not mandatory (like update of data->childmask). Remove a warning, which would be nice but the trigger of this warning is not reliable and add expand the data structure member description instead. Expand a comment, why it is safe to rely on parent pointer here (inside hierarchy update). Fixes: 7ee988770326 ("timers: Implement the hierarchical pull model") Reported-by: Borislav Petkov Signed-off-by: Anna-Maria Behnsen Reviewed-by: Frederic Weisbecker --- kernel/time/timer_migration.c | 33 +++++++++++++++------------------ kernel/time/timer_migration.h | 12 +++++++++++- 2 files changed, 26 insertions(+), 19 deletions(-) diff --git a/kernel/time/timer_migration.c b/kernel/time/timer_migration.c index 84413114db5c..d91efe1dc3bf 100644 --- a/kernel/time/timer_migration.c +++ b/kernel/time/timer_migration.c @@ -507,7 +507,14 @@ static void walk_groups(up_f up, void *data, struct tm= igr_cpu *tmc) * (get_next_timer_interrupt()) * @firstexp: Contains the first event expiry information when last * active CPU of hierarchy is on the way to idle to make - * sure CPU will be back in time. + * sure CPU will be back in time. It is updated in top + * level group only. Be aware, there could occur a new top + * level of the hierarchy between the 'top level call' in + * tmigr_update_events() and the check for the parent group + * in walk_groups(). Then @firstexp might contain a value + * !=3D KTIME_MAX even if it was not the final top + * level. This is not a problem, as the worst outcome is a + * CPU which might wake up a little early. * @evt: Pointer to tmigr_event which needs to be queued (of idle * child group) * @childmask: childmask of child group @@ -649,7 +656,7 @@ static bool tmigr_active_up(struct tmigr_group *group, =20 } while (!atomic_try_cmpxchg(&group->migr_state, &curstate.state, newstat= e.state)); =20 - if ((walk_done =3D=3D false) && group->parent) + if (walk_done =3D=3D false) data->childmask =3D group->childmask; =20 /* @@ -1317,20 +1324,9 @@ static bool tmigr_inactive_up(struct tmigr_group *gr= oup, /* Event Handling */ tmigr_update_events(group, child, data); =20 - if (group->parent && (walk_done =3D=3D false)) + if (walk_done =3D=3D false) data->childmask =3D group->childmask; =20 - /* - * data->firstexp was set by tmigr_update_events() and contains the - * expiry of the first global event which needs to be handled. It - * differs from KTIME_MAX if: - * - group is the top level group and - * - group is idle (which means CPU was the last active CPU in the - * hierarchy) and - * - there is a pending event in the hierarchy - */ - WARN_ON_ONCE(data->firstexp !=3D KTIME_MAX && group->parent); - trace_tmigr_group_set_cpu_inactive(group, newstate, childmask); =20 return walk_done; @@ -1552,10 +1548,11 @@ static void tmigr_connect_child_parent(struct tmigr= _group *child, data.childmask =3D child->childmask; =20 /* - * There is only one new level per time. When connecting the - * child and the parent and set the child active when the parent - * is inactive, the parent needs to be the uppermost - * level. Otherwise there went something wrong! + * There is only one new level per time (which is protected by + * tmigr_mutex). When connecting the child and the parent and + * set the child active when the parent is inactive, the parent + * needs to be the uppermost level. Otherwise there went + * something wrong! */ WARN_ON(!tmigr_active_up(parent, child, &data) && parent->parent); } diff --git a/kernel/time/timer_migration.h b/kernel/time/timer_migration.h index 6c37d94a37d9..494f68cc13f4 100644 --- a/kernel/time/timer_migration.h +++ b/kernel/time/timer_migration.h @@ -22,7 +22,17 @@ struct tmigr_event { * struct tmigr_group - timer migration hierarchy group * @lock: Lock protecting the event information and group hierarchy * information during setup - * @parent: Pointer to the parent group + * @parent: Pointer to the parent group. Pointer is updated when a + * new hierarchy level is added because of a CPU coming + * online the first time. Once it is set, the pointer will + * not be removed or updated. When accessing parent pointer + * lock less to decide whether to abort a propagation or + * not, it is not a problem. The worst outcome is an + * unnecessary/early CPU wake up. But do not access parent + * pointer several times in the same 'action' (like + * activation, deactivation, check for remote expiry,...) + * without holding the lock as it is not ensured that value + * will not change. * @groupevt: Next event of the group which is only used when the * group is !active. The group event is then queued into * the parent timer queue. --=20 2.39.2 From nobody Wed Dec 17 15:53:03 2025 Received: from galois.linutronix.de (Galois.linutronix.de [193.142.43.55]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id F1E7519D063 for ; Mon, 24 Jun 2024 14:54:19 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=193.142.43.55 ARC-Seal: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1719240861; cv=none; b=A/uilJcuSyxJ/faVYmMweYe2EHcVH5DqTfebgExVGeiozEHmqvtB4A4CdnyqaGh0sWc52vPPQkF3Z8m0hmeTRm8zmvzOfMfpsV/gUPbfj+I8pWZl8qUOojVQr9NqCkN2PoAL1vX/8fiycI/jpC/lAhdAtYUyF+JVNv3ikkssFYM= ARC-Message-Signature: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1719240861; c=relaxed/simple; bh=NzMYnlU25XlcmunTuNeMNqwHFYLTvlMOwqg3Y6KHY+8=; h=From:Date:Subject:MIME-Version:Content-Type:Message-Id:References: In-Reply-To:To:Cc; b=dmzHSuwmRQJT/dLv8crVN6mlruP5sF9xvXoswJhAD06oOnKRfAm/W1GxloGlmLnSJp64YoikW1r/Jpga+fFHnfORyFimyf10tyEMvWplMorKOv7ucUdRIQ5wqTI2T+HqRYIYP1hznjO/m16V/2f+YYyPdFWdCQAedCoyrsaU6rU= ARC-Authentication-Results: i=1; smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=linutronix.de; spf=pass smtp.mailfrom=linutronix.de; dkim=pass (2048-bit key) header.d=linutronix.de header.i=@linutronix.de header.b=QQ0wBYMT; dkim=permerror (0-bit key) header.d=linutronix.de header.i=@linutronix.de header.b=VSv9lJ/A; arc=none smtp.client-ip=193.142.43.55 Authentication-Results: smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=linutronix.de Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=linutronix.de Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=linutronix.de header.i=@linutronix.de header.b="QQ0wBYMT"; dkim=permerror (0-bit key) header.d=linutronix.de header.i=@linutronix.de header.b="VSv9lJ/A" From: Anna-Maria Behnsen DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linutronix.de; s=2020; t=1719240858; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=jRwb6bZH+C76mHYt3MDxKdtn1jFC4cvRGSrCMq7mXCs=; b=QQ0wBYMTwJkEyIGivDQrQWLyruVTMPFqOMVSxCuskrTXzawKRI8IUPns3iq/cbrNfeD9Dw Q9AcAziqyRdx27p+8QJwGv9vWCG3edDIFVwGJVuOOgczvNLnFYc19G2ZzxgFwonhkO0UIh KQdvZOgtAAhgdku2aWE4qrRrxpAVT1iqQr5LvtS+Sqaf63qVuszFZm/1I5kRgLbC9uDJ4z CmPHqQ0mzj8gwcYa5GezxlzHSiwIPSJoBD7U1gWtGQ0FtCBoYlbV5ms0VqeXrjYmc1a81M VynFhsNGvVYrvj87umCXeYWtp1bJbpVbZtZszR5LnMoX1M/VWg3D0jFei/VcqA== DKIM-Signature: v=1; a=ed25519-sha256; c=relaxed/relaxed; d=linutronix.de; s=2020e; t=1719240858; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=jRwb6bZH+C76mHYt3MDxKdtn1jFC4cvRGSrCMq7mXCs=; b=VSv9lJ/AQDL1mSFpFZbJc4viptpvb+gqENT54ulqh3u4MfKJXaUAA5DiYpNnWgwjTa/5x6 Ym2ug4XstfGgpWDA== Date: Mon, 24 Jun 2024 16:53:54 +0200 Subject: [PATCH v2 2/5] timer_migration: Improve tracing Precedence: bulk X-Mailing-List: linux-kernel@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: quoted-printable Message-Id: <20240624-tmigr-fixes-v2-2-3eb4c0604790@linutronix.de> References: <20240624-tmigr-fixes-v2-0-3eb4c0604790@linutronix.de> In-Reply-To: <20240624-tmigr-fixes-v2-0-3eb4c0604790@linutronix.de> To: Frederic Weisbecker , Thomas Gleixner , linux-kernel@vger.kernel.org Cc: Anna-Maria Behnsen Trace points of inactive and active propagation are located at the end of the related functions. The interesting information of those trace points is the updated group state. When trace points are not located directly at the place where group state changed, order of trace points in traces could be confusing. Move inactive and active propagation trace points directly after update of group state values. This change is also a preparation to be able to split out update of group state information easily into a separate function. Which in turn is required to fix a possible race in setup code. Fixes: 7ee988770326 ("timers: Implement the hierarchical pull model") Signed-off-by: Anna-Maria Behnsen Reviewed-by: Frederic Weisbecker --- kernel/time/timer_migration.c | 11 +++++------ 1 file changed, 5 insertions(+), 6 deletions(-) diff --git a/kernel/time/timer_migration.c b/kernel/time/timer_migration.c index d91efe1dc3bf..600c9e866fae 100644 --- a/kernel/time/timer_migration.c +++ b/kernel/time/timer_migration.c @@ -656,6 +656,8 @@ static bool tmigr_active_up(struct tmigr_group *group, =20 } while (!atomic_try_cmpxchg(&group->migr_state, &curstate.state, newstat= e.state)); =20 + trace_tmigr_group_set_cpu_active(group, newstate, childmask); + if (walk_done =3D=3D false) data->childmask =3D group->childmask; =20 @@ -673,8 +675,6 @@ static bool tmigr_active_up(struct tmigr_group *group, */ group->groupevt.ignore =3D true; =20 - trace_tmigr_group_set_cpu_active(group, newstate, childmask); - return walk_done; } =20 @@ -1306,9 +1306,10 @@ static bool tmigr_inactive_up(struct tmigr_group *gr= oup, =20 WARN_ON_ONCE((newstate.migrator !=3D TMIGR_NONE) && !(newstate.active)); =20 - if (atomic_try_cmpxchg(&group->migr_state, &curstate.state, - newstate.state)) + if (atomic_try_cmpxchg(&group->migr_state, &curstate.state, newstate.sta= te)) { + trace_tmigr_group_set_cpu_inactive(group, newstate, childmask); break; + } =20 /* * The memory barrier is paired with the cmpxchg() in @@ -1327,8 +1328,6 @@ static bool tmigr_inactive_up(struct tmigr_group *gro= up, if (walk_done =3D=3D false) data->childmask =3D group->childmask; =20 - trace_tmigr_group_set_cpu_inactive(group, newstate, childmask); - return walk_done; } =20 --=20 2.39.2 From nobody Wed Dec 17 15:53:03 2025 Received: from galois.linutronix.de (Galois.linutronix.de [193.142.43.55]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id 6826919D06B for ; Mon, 24 Jun 2024 14:54:20 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=193.142.43.55 ARC-Seal: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1719240862; cv=none; b=MzNdAnZLZ7Iny/v6bVuUtmG2KsadAN9jTotGvRRF3pw8FcJW5Hiy6wyK8LEIUBJJEApJnQQRWBtSqdvcs7Hvyj/u9dq+aoTwm+3ea5ZhYRJA3HKlJu08aAlsEYgA3PYA9AX2liEtw25UgCss5B7SKizIoilqqRchCl9uiVbU1+M= ARC-Message-Signature: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1719240862; c=relaxed/simple; bh=4yBjlfA0tuIAEuVSIqkRa/8T70pDH/sbNUKs5SlgSHE=; h=From:Date:Subject:MIME-Version:Content-Type:Message-Id:References: In-Reply-To:To:Cc; b=i/6KoaPjFkA5owUK9/Nsr6sLbP9PDY5peNLJrhDRUatsxQKD4fxt+5mKCM/M9Y3ZUr7s4BJcaKnILOYh9KlZ+LomqGH3xK7zkpNqgWhi9ea4dlOLrBH27d4gCETPDT9jqpMK7jGJncqY4LO1tmPsj+SRvtuy8F2LpzQJ7Ge3Yes= ARC-Authentication-Results: i=1; smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=linutronix.de; spf=pass smtp.mailfrom=linutronix.de; dkim=pass (2048-bit key) header.d=linutronix.de header.i=@linutronix.de header.b=yqbeOPlP; dkim=permerror (0-bit key) header.d=linutronix.de header.i=@linutronix.de header.b=j5EHRPt8; arc=none smtp.client-ip=193.142.43.55 Authentication-Results: smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=linutronix.de Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=linutronix.de Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=linutronix.de header.i=@linutronix.de header.b="yqbeOPlP"; dkim=permerror (0-bit key) header.d=linutronix.de header.i=@linutronix.de header.b="j5EHRPt8" From: Anna-Maria Behnsen DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linutronix.de; s=2020; t=1719240858; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=XQfLpd4KsucHQv4vv56t9DwxntT2d0zNXi6vClOfi/4=; b=yqbeOPlP88EL8vhlLsfiDaJH8OX/MfrZTt0M457CTvKY/8ZfECuFjTPYOGFIJNOmQcS/iE K8PrWfN/rvtzEaYcJrBgxFWy7VRiX1LmFUHIpC8ZHm/NjtXvBZxlBjzQH0rHdqXkM+In9v iNNTAix2bnGedV7SOavx88ARayQPNyVOJdsFjGVBWvhDKiAOCpH3hw6EWBCWxqcxGI3b18 Z4RkX5RXY82zKRsiuuTYaHWHr03a/tvVyBwaYy/saXemXXT8/258Ef3jouF4Sll9yFCL0N zHmKRETKIkpOT+ujG7XIGMVhzSL5JwlhALGjLYwxMyOrkBG2h7ZHDPiOZxadYA== DKIM-Signature: v=1; a=ed25519-sha256; c=relaxed/relaxed; d=linutronix.de; s=2020e; t=1719240858; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=XQfLpd4KsucHQv4vv56t9DwxntT2d0zNXi6vClOfi/4=; b=j5EHRPt8ry18cHOtg2ynonAvD7DWnKhC8XRuOrve8z0V6hqf5LgcOUw1yPeN6rwVEJQ5xL vkvMW6mznYS6vnBQ== Date: Mon, 24 Jun 2024 16:53:55 +0200 Subject: [PATCH v2 3/5] timer_migration: Split out state update of tmigr_active_up() Precedence: bulk X-Mailing-List: linux-kernel@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: quoted-printable Message-Id: <20240624-tmigr-fixes-v2-3-3eb4c0604790@linutronix.de> References: <20240624-tmigr-fixes-v2-0-3eb4c0604790@linutronix.de> In-Reply-To: <20240624-tmigr-fixes-v2-0-3eb4c0604790@linutronix.de> To: Frederic Weisbecker , Thomas Gleixner , linux-kernel@vger.kernel.org Cc: Anna-Maria Behnsen The functionality of the state update is split into __tmigr_active_up() to be reusable by other callsites. This is a preparation for a fix of a potential race when a CPU comes online the first time and creates a new top level. No functional change. Fixes: 7ee988770326 ("timers: Implement the hierarchical pull model") Signed-off-by: Anna-Maria Behnsen Reviewed-by: Frederic Weisbecker --- kernel/time/timer_migration.c | 47 ++++++++++++++++++++++++++-------------= ---- 1 file changed, 29 insertions(+), 18 deletions(-) diff --git a/kernel/time/timer_migration.c b/kernel/time/timer_migration.c index 600c9e866fae..0e1c53dac390 100644 --- a/kernel/time/timer_migration.c +++ b/kernel/time/timer_migration.c @@ -623,12 +623,38 @@ static u64 tmigr_next_groupevt_expires(struct tmigr_g= roup *group) return evt->nextevt.expires; } =20 +static bool __tmigr_active_up(struct tmigr_group *group, bool *walk_done, + union tmigr_state *curstate, u8 childmask) +{ + union tmigr_state newstate; + + newstate =3D *curstate; + *walk_done =3D true; + + if (newstate.migrator =3D=3D TMIGR_NONE) { + newstate.migrator =3D childmask; + + /* Changes need to be propagated */ + *walk_done =3D false; + } + + newstate.active |=3D childmask; + newstate.seq++; + + if (atomic_try_cmpxchg(&group->migr_state, &curstate->state, newstate.sta= te)) { + trace_tmigr_group_set_cpu_active(group, newstate, childmask); + return true; + } + + return false; +} + static bool tmigr_active_up(struct tmigr_group *group, struct tmigr_group *child, void *ptr) { - union tmigr_state curstate, newstate; struct tmigr_walk *data =3D ptr; + union tmigr_state curstate; bool walk_done; u8 childmask; =20 @@ -640,23 +666,8 @@ static bool tmigr_active_up(struct tmigr_group *group, */ curstate.state =3D atomic_read(&group->migr_state); =20 - do { - newstate =3D curstate; - walk_done =3D true; - - if (newstate.migrator =3D=3D TMIGR_NONE) { - newstate.migrator =3D childmask; - - /* Changes need to be propagated */ - walk_done =3D false; - } - - newstate.active |=3D childmask; - newstate.seq++; - - } while (!atomic_try_cmpxchg(&group->migr_state, &curstate.state, newstat= e.state)); - - trace_tmigr_group_set_cpu_active(group, newstate, childmask); + while (!__tmigr_active_up(group, &walk_done, &curstate, childmask)) + ; =20 if (walk_done =3D=3D false) data->childmask =3D group->childmask; --=20 2.39.2 From nobody Wed Dec 17 15:53:03 2025 Received: from galois.linutronix.de (Galois.linutronix.de [193.142.43.55]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id B7A5519D076 for ; Mon, 24 Jun 2024 14:54:20 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=193.142.43.55 ARC-Seal: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1719240862; cv=none; b=Sz7UfJtJqOvEOn46EzT/OIyr7NdhAXm0Gj4tuPGx7EWomqMpVonR0hk8eYkgcoT17Zr2TnZJyNooU8T2zf7LhFcDLAttHZpBFc4zIUDsCnTfDbxiJvSvaJ3y53ckXBH8vkHhZN3+P82EhuqQEqYiKQh3tEKmnl0i+vT4QCqpIHM= ARC-Message-Signature: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1719240862; c=relaxed/simple; bh=ALyMCnPuofxu9ffvPlOcV1t2j2NwHcomo3j39FHLctU=; h=From:Date:Subject:MIME-Version:Content-Type:Message-Id:References: In-Reply-To:To:Cc; b=m1D3kTuYeUPrqIrl/h7PjxCGfkbLWadOPzxdIb0eAJjZJD0tVX8GCyvztVB7nryUnexhGYQreWKZqas8klzaTo7g1DAYVovnSWOg8ssjQEiIWcxGqaTaUCOqaAnQohq23pq5RpDDiippW4IXk28cYw72UgX1Tz8lhAyEZTmVamw= ARC-Authentication-Results: i=1; smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=linutronix.de; spf=pass smtp.mailfrom=linutronix.de; dkim=pass (2048-bit key) header.d=linutronix.de header.i=@linutronix.de header.b=SkCxa4B3; dkim=permerror (0-bit key) header.d=linutronix.de header.i=@linutronix.de header.b=tsxt/pp1; arc=none smtp.client-ip=193.142.43.55 Authentication-Results: smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=linutronix.de Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=linutronix.de Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=linutronix.de header.i=@linutronix.de header.b="SkCxa4B3"; dkim=permerror (0-bit key) header.d=linutronix.de header.i=@linutronix.de header.b="tsxt/pp1" From: Anna-Maria Behnsen DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linutronix.de; s=2020; t=1719240858; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=vjbihVcK59OxT8itrX6DwAUHPlbzLTEjwCtwqlkepCk=; b=SkCxa4B3j8G2Gg3BJL0idiyzmLruXC9Z2IRCnPx2SzSaMhDg69/LMaqd6t0upQKiNBb+50 8BbJELU/gDlXlheCstF/i+hokcT0vWAax0I4NPwG+xt7mynbs42KZcEMZDEwuhb8v53cHc phsz8yfj3oneIBsY2N3c/jhe/QSmbPyPGFOwtVZe+6UwKYQ367OjQHbPvOYtUVua6HT/0C W7WdggDfSrBEyp7W25oazjEjbZ1hCrWze0WbMD/TUwG+CgyfpGY1CIrbMEjGWazV3JxKGn P2e7yOi1on0HEUnNNKijiLexPT4rttYjWRr9K9wSkz2KqP2n9Pz+E0s4HYGByQ== DKIM-Signature: v=1; a=ed25519-sha256; c=relaxed/relaxed; d=linutronix.de; s=2020e; t=1719240858; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=vjbihVcK59OxT8itrX6DwAUHPlbzLTEjwCtwqlkepCk=; b=tsxt/pp1zL6CmjT2y6aMnOXFcTZWQJccl5x6eNMUDU/l7UD7glOgawL0crqnJE78CjWxjY 5PhdscQxhkkRmnCQ== Date: Mon, 24 Jun 2024 16:53:56 +0200 Subject: [PATCH v2 4/5] timer_migration: Fix possible race in tmigr_active_up() in setup path Precedence: bulk X-Mailing-List: linux-kernel@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: quoted-printable Message-Id: <20240624-tmigr-fixes-v2-4-3eb4c0604790@linutronix.de> References: <20240624-tmigr-fixes-v2-0-3eb4c0604790@linutronix.de> In-Reply-To: <20240624-tmigr-fixes-v2-0-3eb4c0604790@linutronix.de> To: Frederic Weisbecker , Thomas Gleixner , linux-kernel@vger.kernel.org Cc: Anna-Maria Behnsen Frederic reported the following possible race: [GRP0:0] migrator =3D 0 active =3D 0 nextevt =3D KTIME_MAX / \ 0 1 .. 7 active idle 0) Hierarchy has for now only 8 CPUs and CPU 0 is the only active CPU. [GRP1:0] migrator =3D TMIGR_NONE active =3D NONE nextevt =3D KTIME_MAX \ [GRP0:0] [GRP0:1] migrator =3D 0 migrator =3D TMIGR_NONE active =3D 0 active =3D NONE nextevt =3D KTIME_MAX nextevt =3D KTIME_MAX / \ 0 1 .. 7 8 active idle !online 1) CPU 8 is booting and creates a new group in first level GRP0:1 and therefore also a new top group GRP1:0. For now the setup code proceeded only until the connected between GRP0:1 to the new top group. The connection between CPU8 and GRP0:1 is not yet established and CPU 8 is still !online. [GRP1:0] migrator =3D TMIGR_NONE active =3D NONE nextevt =3D KTIME_MAX / \ [GRP0:0] [GRP0:1] migrator =3D 0 migrator =3D TMIGR_NONE active =3D 0 active =3D NONE nextevt =3D KTIME_MAX nextevt =3D KTIME_MAX / \ 0 1 .. 7 8 active idle !online 2) Setup code now connects GRP0:0 to GRP1:0 and observes while in tmigr_connect_child_parent() that GRP0:0 is not TMIGR_NONE. So it prepares to call tmigr_active_up() on it. It hasn't done it yet. [GRP1:0] migrator =3D TMIGR_NONE active =3D NONE nextevt =3D KTIME_MAX / \ [GRP0:0] [GRP0:1] migrator =3D TMIGR_NONE migrator =3D TMIGR_NONE active =3D NONE active =3D NONE nextevt =3D KTIME_MAX nextevt =3D KTIME_MAX / \ 0 1 .. 7 8 idle idle !online 3) CPU 0 goes idle. Since GRP0:0->parent has been updated by CPU 8 with GRP0:0->lock held, CPU 0 observes GRP1:0 after calling tmigr_update_events() and it propagates the change to the top (no change there and no wakeup programmed since there is no timer). [GRP1:0] migrator =3D GRP0:0 active =3D GRP0:0 nextevt =3D KTIME_MAX / \ [GRP0:0] [GRP0:1] migrator =3D TMIGR_NONE migrator =3D TMIGR_NONE active =3D NONE active =3D NONE nextevt =3D KTIME_MAX nextevt =3D KTIME_MAX / \ 0 1 .. 7 8 idle idle !online 4) Now the setup code finally calls tmigr_active_up() to and sets GRP0:0 active in GRP1:0 [GRP1:0] migrator =3D GRP0:0 active =3D GRP0:0, GRP0:1 nextevt =3D KTIME_MAX / \ [GRP0:0] [GRP0:1] migrator =3D TMIGR_NONE migrator =3D 8 active =3D NONE active =3D 8 nextevt =3D KTIME_MAX nextevt =3D KTIME_MAX / \ | 0 1 .. 7 8 idle idle active 5) Now CPU 8 is connected with GRP0:1 and CPU 8 calls tmigr_active_up() out of tmigr_cpu_online(). [GRP1:0] migrator =3D GRP0:0 active =3D GRP0:0 nextevt =3D T8 / \ [GRP0:0] [GRP0:1] migrator =3D TMIGR_NONE migrator =3D TMIGR_NONE active =3D NONE active =3D NONE nextevt =3D KTIME_MAX nextevt =3D T8 / \ | 0 1 .. 7 8 idle idle idle 5) CPU 8 goes idle with a timer T8 and relies on GRP0:0 as the migrator. But it's not really active, so T8 gets ignored. The update which is done in third step is not noticed by setup code. So a wrong migrator is set to top level group and a timer could get ignored. Rework the activation of an already existing group in setup code after adding a new top level group and use memory barriers (as already used in tmigr_inactive_up()) to be sure that child state updates and group state updates are ordered properly. The update of the group event ignore bit is now ignored. But this is fine, as this bit is only required when queueing the event into the timer queue of the parent group. As this is always the update of the top level group, it doesn't matter. Fixes: 7ee988770326 ("timers: Implement the hierarchical pull model") Reported-by: Frederic Weisbecker Signed-off-by: Anna-Maria Behnsen Reviewed-by: Frederic Weisbecker --- kernel/time/timer_migration.c | 49 ++++++++++++++++++++++++++++-----------= ---- 1 file changed, 32 insertions(+), 17 deletions(-) diff --git a/kernel/time/timer_migration.c b/kernel/time/timer_migration.c index 0e1c53dac390..7971512a60b0 100644 --- a/kernel/time/timer_migration.c +++ b/kernel/time/timer_migration.c @@ -1448,6 +1448,37 @@ u64 tmigr_quick_check(u64 nextevt) return KTIME_MAX; } =20 +static void tmigr_setup_active_up(struct tmigr_group *group, struct tmigr_= group *child) +{ + union tmigr_state curstate, childstate; + bool walk_done; + + /* + * Memory barrier is paired with the cmpxchg in tmigr_inactive_up() to + * make sure updates of child and group states are ordered. The ordering + * is mandatory, as the update of the group state is only valid for when + * child state is active. + */ + curstate.state =3D atomic_read_acquire(&group->migr_state); + + for (;;) { + childstate.state =3D atomic_read(&child->migr_state); + if (!childstate.active) + return; + + if (__tmigr_active_up(group, &walk_done, &curstate, child->childmask)) + break; + + /* + * The memory barrier is paired with the cmpxchg() in + * tmigr_inactive_up() to make sure the updates of child and + * group states are ordered. It is required only when the + * try_cmpxchg() in __tmigr_active_up() fails. + */ + smp_mb__after_atomic(); + } +} + static void tmigr_init_group(struct tmigr_group *group, unsigned int lvl, int node) { @@ -1522,8 +1553,6 @@ static struct tmigr_group *tmigr_get_group(unsigned i= nt cpu, int node, static void tmigr_connect_child_parent(struct tmigr_group *child, struct tmigr_group *parent) { - union tmigr_state childstate; - raw_spin_lock_irq(&child->lock); raw_spin_lock_nested(&parent->lock, SINGLE_DEPTH_NESTING); =20 @@ -1551,21 +1580,7 @@ static void tmigr_connect_child_parent(struct tmigr_= group *child, * executed with the formerly top level group (child) and the newly * created group (parent). */ - childstate.state =3D atomic_read(&child->migr_state); - if (childstate.migrator !=3D TMIGR_NONE) { - struct tmigr_walk data; - - data.childmask =3D child->childmask; - - /* - * There is only one new level per time (which is protected by - * tmigr_mutex). When connecting the child and the parent and - * set the child active when the parent is inactive, the parent - * needs to be the uppermost level. Otherwise there went - * something wrong! - */ - WARN_ON(!tmigr_active_up(parent, child, &data) && parent->parent); - } + tmigr_setup_active_up(parent, child); } =20 static int tmigr_setup_groups(unsigned int cpu, unsigned int node) --=20 2.39.2 From nobody Wed Dec 17 15:53:03 2025 Received: from galois.linutronix.de (Galois.linutronix.de [193.142.43.55]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id 01CFF19D88C for ; Mon, 24 Jun 2024 14:54:22 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=193.142.43.55 ARC-Seal: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1719240864; cv=none; b=gX+5xZSHagpmeJP9EWNEbpoGs/SKf2kOJfejwgmvyF2FUJZj6avotjoSA+SwQXU4g34YprZvQu5ehbl8rcC2hcRr9wzOUpyUnAIrnCsJHQn6DITOtKyerKwBQWv3x1bSj/MMiAYIsqohHVCsJNqVYD4EZmMkBkAvXFqECt8KXGE= ARC-Message-Signature: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1719240864; c=relaxed/simple; bh=sOD5/KUI5C/W7gKfhdqKa1clLl9j4FoXxPG9kCjGskc=; h=From:Date:Subject:MIME-Version:Content-Type:Message-Id:References: In-Reply-To:To:Cc; b=ZVTtsymrGTAcbRKjJcfDrrZDDJ+IaMxwoDKkjqmbDHch2gJWwCgLcbynxZxrT3oNCPNfhmTSHVO4jXOdZSp5qEfX83d9YK49RS+jC+UpSsmvnai4RZXAr2IOXdUyUXTNHsQGxEF+OXporArSP9Hxj+3jDKGePipStugKuROusv4= ARC-Authentication-Results: i=1; smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=linutronix.de; spf=pass smtp.mailfrom=linutronix.de; dkim=pass (2048-bit key) header.d=linutronix.de header.i=@linutronix.de header.b=tEW5r+9r; dkim=permerror (0-bit key) header.d=linutronix.de header.i=@linutronix.de header.b=3Im6ORrd; arc=none smtp.client-ip=193.142.43.55 Authentication-Results: smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=linutronix.de Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=linutronix.de Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=linutronix.de header.i=@linutronix.de header.b="tEW5r+9r"; dkim=permerror (0-bit key) header.d=linutronix.de header.i=@linutronix.de header.b="3Im6ORrd" From: Anna-Maria Behnsen DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linutronix.de; s=2020; t=1719240859; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=Op6walJII/gVaxljfpCXLxR/HhgFf5B7QUNkyDEsexo=; b=tEW5r+9riqeajpxUUvRgdNcYb85XusNKpjA/FWBOiJCk1sd0iUqYEBDNuruj/6fIl+1m7B 73WQ2i5Shx5Endd2q/HB9mlU5kJc5StXwc1soFgLnIN0XtCS2a44wip/cuTy4wBdu67ltu K1idILjobmLStEHSMRwlNc8853+c1zmIlP8+YOgBnmlTqEbBLgqjluqnsWGzx5ICSQyiCF 3z3ZNI5SQqlPTuj+FiQmnjcPxwrb0W9IOhEhfW5dgl4s0weTKRDqqGGNSZVwQEslBMVzWv r40A7h9Ei4t9h3ckxGtuBGT+apTcjZsUmXRLuEsG09e1rkzKsdMxmb7J39qPQQ== DKIM-Signature: v=1; a=ed25519-sha256; c=relaxed/relaxed; d=linutronix.de; s=2020e; t=1719240859; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=Op6walJII/gVaxljfpCXLxR/HhgFf5B7QUNkyDEsexo=; b=3Im6ORrd0BV9fThFY2aELbdk1VADrfIJlxTmDPEzU3ZbVD4YytSq32f6KqRybp+tHylYos hvOdksFTaylQTfBA== Date: Mon, 24 Jun 2024 16:53:57 +0200 Subject: [PATCH v2 5/5] timer_migration: Spare write when nothing changed Precedence: bulk X-Mailing-List: linux-kernel@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: quoted-printable Message-Id: <20240624-tmigr-fixes-v2-5-3eb4c0604790@linutronix.de> References: <20240624-tmigr-fixes-v2-0-3eb4c0604790@linutronix.de> In-Reply-To: <20240624-tmigr-fixes-v2-0-3eb4c0604790@linutronix.de> To: Frederic Weisbecker , Thomas Gleixner , linux-kernel@vger.kernel.org Cc: Anna-Maria Behnsen The wakeup value is written unconditionally in tmigr_cpu_new_timer(). When there was no new next timer expiry that needs to be propagated, then the value that was read before is written. This is not required. Move write to the place where wakeup value could have changed. Signed-off-by: Anna-Maria Behnsen Reviewed-by: Frederic Weisbecker --- kernel/time/timer_migration.c | 11 +++++------ 1 file changed, 5 insertions(+), 6 deletions(-) diff --git a/kernel/time/timer_migration.c b/kernel/time/timer_migration.c index 7971512a60b0..3fb47e4b4ce4 100644 --- a/kernel/time/timer_migration.c +++ b/kernel/time/timer_migration.c @@ -1248,14 +1248,13 @@ u64 tmigr_cpu_new_timer(u64 nextexp) if (nextexp !=3D tmc->cpuevt.nextevt.expires || tmc->cpuevt.ignore) { ret =3D tmigr_new_timer(tmc, nextexp); + /* + * Make sure the reevaluation of timers in idle path + * will not miss an event. + */ + WRITE_ONCE(tmc->wakeup, ret); } } - /* - * Make sure the reevaluation of timers in idle path will not miss an - * event. - */ - WRITE_ONCE(tmc->wakeup, ret); - trace_tmigr_cpu_new_timer_idle(tmc, nextexp); raw_spin_unlock(&tmc->lock); return ret; --=20 2.39.2