From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from smtp.kernel.org (aws-us-west-2-korg-mail-alma10-1.taild15c8.ts.net [100.103.45.18]) (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 D48E93F8ED8 for ; Fri, 26 Jun 2026 14:40:56 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=100.103.45.18 ARC-Seal:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1782484859; cv=none; b=VFl3FvMGLuFNGc6OrmPvIBADum5m2L9AvVXPq3v1L3rgaCK33SK/lHXjGJVcF/hkcM6rF9uHgi4nVnQhZ3dtt3FBpnft0b8PWUbAYLjpZ0A9xFAvKtyTVoeVpCB514Gj4W7/51hE7Xq2G7fCFWlaEAB1jXQXgpPR4w1fYwqWX/M= ARC-Message-Signature:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1782484859; c=relaxed/simple; bh=JeYI86Nhi4QugXJEeBuM2YuUmYnsLRyH8tO/eO1WuPQ=; h=From:To:Cc:Subject:In-Reply-To:References:Date:Message-ID: MIME-Version:Content-Type; b=a6PMsxf51UqlSWqOFnKB3z1ClqZ5Y9Fjt/oAHCxxkIavblmy47fj42oAVBsNz/nSkZs9fe2wdB1wFRLnRNyuZJeoYKv5cTyF4UmuCwUSsvrl2qIxvpfaSsQRPn2FRdP2ZIp7Vx1yFCNy+L0m83re1d9xyI0yXpvaTgGlS7/rJVE= ARC-Authentication-Results:i=1; smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=kernel.org header.i=@kernel.org header.b=LUbmaCah; arc=none smtp.client-ip=100.103.45.18 Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=kernel.org header.i=@kernel.org header.b="LUbmaCah" Received: by smtp.kernel.org (Postfix) with ESMTPSA id AE99F1F000E9; Fri, 26 Jun 2026 14:40:55 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=kernel.org; s=k20260515; t=1782484856; bh=oGGzudP1swJuh3KTWNu1mcjno5Q5hxzYI2yFF/im5mk=; h=From:To:Cc:Subject:In-Reply-To:References:Date; b=LUbmaCahS4NQoC8OZqTUC6dVgfrteuD0/2r3XFlEC9bnfDnJlpx+vSL+IrVhtlJBw QLtlD/eDT5QdWLIKAazivaCUVgcIWOSFbiFTT0XP42Bn3pFRfUddGfxnzCUJb1/Cvr iUAjlfCiNwK0IyHvROLi+3MXMGOzVdmbumXevUPwcZ3ySl9IRoB8W49wIeEybzKilx Nv8EEMUZAiZuEWRdnxy6DB10V+gyGK+HEaJY3T0uK6hjgMjvan6zNP8YVl1cf7irn7 AMs6LnFIiQLPIEee7rmgCmOo0epOF/2s7UbiZScfQa7wqTLqRtKvPB+l0/IL1ZraDM FLZbmirfhjkEA== From: Thomas Gleixner To: Chuyi Zhou , mingo@redhat.com, luto@kernel.org, peterz@infradead.org, paulmck@kernel.org, muchun.song@linux.dev, bp@alien8.de, dave.hansen@linux.intel.com, pbonzini@redhat.com, bigeasy@linutronix.de, clrkwllms@kernel.org, rostedt@goodmis.org, nadav.amit@gmail.com, vkuznets@redhat.com Cc: linux-kernel@vger.kernel.org, Chuyi Zhou Subject: Re: [PATCH v8 06/14] smp: Enable preemption early in smp_call_function_many_cond() In-Reply-To: <20260616111127.966468-7-zhouchuyi@bytedance.com> References: <20260616111127.966468-1-zhouchuyi@bytedance.com> <20260616111127.966468-7-zhouchuyi@bytedance.com> Date: Fri, 26 Jun 2026 16:40:53 +0200 Message-ID: <87v7b5icui.ffs@fw13> Precedence: bulk X-Mailing-List: linux-kernel@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 Content-Type: text/plain On Tue, Jun 16 2026 at 19:11, Chuyi Zhou wrote: > Disabling preemption entirely during smp_call_function_many_cond() was > primarily for the following reasons: > > - To prevent the remote online CPU from going offline. Specifically, we > want to ensure that no new csds are queued after smpcfd_dying_cpu() has > finished. Therefore, preemption must be disabled until all necessary IPIs > are sent. > > - To prevent current CPU from going offline. Being migrated to another CPU > and calling csd_lock_wait() may cause UAF due to smpcfd_dead_cpu() during > the current CPU offline process. > > - To protect the per-cpu cfd_data from concurrent modification by other > tasks on the current CPU. cfd_data contains cpumasks and per-cpu csds. > Before enqueueing a csd, we block on the csd_lock() to ensure the > previous async csd->func() has completed, and then initialize csd->func and > csd->info. After sending the IPI, we spin-wait for the remote CPU to call > csd_unlock(). Actually the csd_lock mechanism already guarantees csd > serialization. If preemption occurs during csd_lock_wait, other concurrent > smp_call_function_many_cond calls will simply block until the previous > csd->func() completes: Please format properly. > task A task B > > sd->func = fun_a > send ipis > > preempted by B > ---------------> > csd_lock(csd); // block until last > // fun_a finished > > csd->func = func_b; > csd->info = info; > ... > send ipis > > switch back to A > <--------------- > > csd_lock_wait(csd); // block until remote finish func_* > > Previous patches replaced the per-cpu cfd->cpumask with task-local cpumask, The per CPU cfd->cpumask has been replaced with a task local cpumask.... > and the percpu csd is allocated only once and is never freed to ensure > we can safely access csd. Now we can enable preemption before > csd_lock_wait() which makes the potentially unpredictable csd_lock_wait() > preemptible and migratable. With that in place enable preemption before .... > + this_cpu = get_cpu(); > task_mask = smp_task_ipi_mask(current); > cfd = this_cpu_ptr(&cfd_data); > if (task_mask) > @@ -953,6 +952,17 @@ static void smp_call_function_many_cond(const struct cpumask *mask, > local_irq_restore(flags); > } > > + /* > + * Waiting for completion can take time, especially with many CPUs. > + * On a PREEMPT kernel a per-task cpumask is used to track CPUs with > + * pending IPI requests. This allows preemption to be enabled before > + * waiting. On a !PREEMPT kernel the cpumask is shared and the call > + * must block until completion to avoid modifications by another caller > + * on this CPU. > + */ > + if (task_mask) > + put_cpu(); What's this conditional for?. If CONFIG_PREEMPTION is disabled preempt_enable() never results in preemption, which means the shared per CPU mask is implicitely protected and get/put_cpu() are completely unrelated to that. So please make this unconditional end rewrite this completely misleading comment. Thanks, tglx