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 51A0C3F8894 for ; Fri, 26 Jun 2026 14:29:06 +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=1782484155; cv=none; b=CRc1ceF4N3HaGM8QG4HXkJFPKXj2XjkF2bSFKBIkCrCL2uhkJIwPHMkmqL9c6W37gFD/fuscXsi9RzNLSLqjqu44Q1j+hl+MH7/xM0xdOVMmTONtbFBAky4rWiGXaenv3s6bpYMIWI/Uz6HKPTlONe7ANvJhkjavuhP68+BxBCI= ARC-Message-Signature:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1782484155; c=relaxed/simple; bh=70SpPLv+LwvPrvs8YX5JvyLDNxFPv+BzVeJaHNADt0A=; h=From:To:Cc:Subject:In-Reply-To:References:Date:Message-ID: MIME-Version:Content-Type; b=QPVOOdVZlT/lF4wPw8UXGCDmcvOtQGuXoPqFcnXPm6f4YsNI8reQK5QRtA0ejHBo+/7HBytGEjMUvEP6eeLdvGZZDcpX5e4CCep523ykx6um8/jImvwzhdzWx0hKmIqinUOggGgSPsL1g97D0W68XtkjKcw1pjyPQ4yUL0nwvXw= ARC-Authentication-Results:i=1; smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=kernel.org header.i=@kernel.org header.b=dbkhIiIf; 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="dbkhIiIf" Received: by smtp.kernel.org (Postfix) with ESMTPSA id E7F081F000E9; Fri, 26 Jun 2026 14:29:05 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=kernel.org; s=k20260515; t=1782484146; bh=IuOBA+Nb6St/JHjWqROUDSu2nAbcvZ0TquOVdBRoSnA=; h=From:To:Cc:Subject:In-Reply-To:References:Date; b=dbkhIiIfOQaMHe8eO+iZ9qh1sBcy4u7nsWCx943i7xmXAqjFTJcnKA/75EV554wsl zhi4nDjss0EkA3PNjLQkdu74LUSrY3GDiJqMjluRjILZABUkS0o192a/X308HkWg2E lvEPwAqWFoNbSNHHkBjkiR7Z2AkLqsl91aV3jJqO3fiVQZ5NggSPO1MqQw0mjJO55g 3D5WBYgmxknWClC1iQjQjZCvjwQxT0aVduaWybggUK/rgl5FDCHPK0GRhBZV2g51sc cKVFsB2r60W32XOpWNSwiNJYmJhZAzNcDnJdVme4LAQyygwzODYg8l8pJ8fVH1PCWS pRclyi57OkGaQ== 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 04/14] smp: Use task-local IPI cpumask in smp_call_function_many_cond() In-Reply-To: <20260616111127.966468-5-zhouchuyi@bytedance.com> References: <20260616111127.966468-1-zhouchuyi@bytedance.com> <20260616111127.966468-5-zhouchuyi@bytedance.com> Date: Fri, 26 Jun 2026 16:29:03 +0200 Message-ID: <871pdtjryo.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: > This patch prepares the task-local IPI cpumask during thread creation, and > uses the local cpumask to replace the percpu cfd cpumask in > smp_call_function_many_cond(). We will enable preemption during > csd_lock_wait() later, and this can prevent concurrent access to the > cfd->cpumask from other tasks on the current CPU. For cases where > cpumask_size() is smaller than or equal to the pointer size, it tries to > stash the cpumask in the pointer itself to avoid extra memory allocations. This one fails the comprehensible test and also does not match the rules of how change logs should be written. > +#if defined(CONFIG_SMP) && defined(CONFIG_PREEMPTION) > + union { > + cpumask_t *ipi_mask_ptr; > + unsigned long ipi_mask_val; Indentation of the variable name wants TABs not spaces > @@ -933,10 +934,14 @@ static struct task_struct *dup_task_struct(struct task_struct *orig, int node) > #endif > account_kernel_stack(tsk, 1); > > - err = scs_prepare(tsk, node); > + err = smp_task_ipi_mask_alloc(tsk); Hrm. So we unconditionally allocate another per task CPU mask. How many task actually utilize it? We keep making task_struct and the related things larger every other release without actually looking at the resulting overall memory consumption. > +static DEFINE_STATIC_KEY_FALSE(ipi_mask_inlined); > + > +#ifdef CONFIG_PREEMPTION > + > +int smp_task_ipi_mask_alloc(struct task_struct *task) > +{ > + if (static_branch_unlikely(&ipi_mask_inlined)) > + return 0; > + > + task->ipi_mask_ptr = kmalloc(cpumask_size(), GFP_KERNEL); > + if (!task->ipi_mask_ptr) > + return -ENOMEM; > + > + return 0; > +} > + > +void smp_task_ipi_mask_free(struct task_struct *task) > +{ > + if (static_branch_unlikely(&ipi_mask_inlined)) > + return; > + > + kfree(task->ipi_mask_ptr); > +} > + > +static cpumask_t *smp_task_ipi_mask(struct task_struct *cur) > +{ > + /* > + * If cpumask_size() is smaller than or equal to the pointer > + * size, it stashes the cpumask in the pointer itself to > + * avoid extra memory allocations. > + */ > + if (static_branch_unlikely(&ipi_mask_inlined)) > + return (cpumask_t *)&cur->ipi_mask_val; > + > + return cur->ipi_mask_ptr; > +} > +#else > +static cpumask_t *smp_task_ipi_mask(struct task_struct *cur) > +{ > + return NULL; > +} > +#endif > + > /* > * Flags to be used as scf_flags argument of smp_call_function_many_cond(). > * > @@ -811,11 +855,19 @@ static void smp_call_function_many_cond(const struct cpumask *mask, > int cpu, last_cpu, this_cpu = smp_processor_id(); > struct call_function_data *cfd; > bool wait = scf_flags & SCF_WAIT; > + struct cpumask *cpumask, *task_mask; > int nr_cpus = 0; > bool run_remote = false; While at it please fix up the variable declaration according to Documentation so it becomes reverse fir tree layout. > > lockdep_assert_preemption_disabled(); > > + task_mask = smp_task_ipi_mask(current); > + cfd = this_cpu_ptr(&cfd_data); > + if (task_mask) > + cpumask = task_mask; > + else > + cpumask = cfd->cpumask; Glueing the cfd initialization between task_mask and the conditional is pointlessly hard to follow. Keep related things together. Thanks, tglx