From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from va-1-112.ptr.blmpb.com (va-1-112.ptr.blmpb.com [209.127.230.112]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id 87F0E3D47CF for ; Fri, 22 May 2026 15:16:48 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=209.127.230.112 ARC-Seal:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1779463014; cv=none; b=DMkilfwflNCLrVlRXm9eMH50HrEb8Jsygs2B29NSLgfSJzZrE/urHr66G7nBonoYxyMY6EXRU/W+sYZKIdyLFSxpBcNoex7gHAMIdUKJhw8O6wxwWvbsuoetCUEY5ukrbjLLHgtAPKp5RwC5NBXlmHgIpCDbei2FLj0j3aUO7iU= ARC-Message-Signature:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1779463014; c=relaxed/simple; bh=i18cZyuPMqB25K6ZEYPXN6mKuuBAU/yZRS6GPyoTQY0=; h=From:In-Reply-To:References:Mime-Version:Subject:To:Cc:Date: Message-Id:Content-Type; b=ZeHwByzGWQOrqP+P16CMbYhSVjHoQGns/xitLn078bbZ15fGzm9cqbcBL15aH3kueZ7eeivUYPglglIucdXjbEucfiFsNVCZGefW69FgeW16W82WRd7jOYaOLns1/XKR5iMrUB0lArgB/Dkk/7XxW8RUKBpNL6GroOOE2rIUKt4= ARC-Authentication-Results:i=1; smtp.subspace.kernel.org; dmarc=pass (p=quarantine dis=none) header.from=bytedance.com; spf=pass smtp.mailfrom=bytedance.com; dkim=pass (2048-bit key) header.d=bytedance.com header.i=@bytedance.com header.b=nIZPT2/O; arc=none smtp.client-ip=209.127.230.112 Authentication-Results: smtp.subspace.kernel.org; dmarc=pass (p=quarantine dis=none) header.from=bytedance.com Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=bytedance.com Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=bytedance.com header.i=@bytedance.com header.b="nIZPT2/O" DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; s=2212171451; d=bytedance.com; t=1779463003; h=from:subject: mime-version:from:date:message-id:subject:to:cc:reply-to:content-type: mime-version:in-reply-to:message-id; bh=Jode59VZfu97jHeiSBue9m4y4PCZfeJaQlxwgRT3Gjw=; b=nIZPT2/OKg1n1QSVoKR+zO8GBm0hNqRPuZdzvTpHAex8k8Uz1Gp2j5MetR7XNiX6mu2P/v V5e/fB4qKAploZQws5UsLR+eBJv2gpIzHp/wuC+AYSvIifttEpAi8XwpxwXcGJ6MhrF3LD V1dqIFwxc23mK/EjFxUfm6R2Ss1zDh8tp5blFHmNVS0pzA8RHgKJgElsRWMujFn2tZdKis DjLv+MNly3AkBkraTNktJi1SLQbSNQ7Qjn6g+Bjz93v8OFQqMkyhdQSsTIo0b6wqzI8Hsv ze897xW+rvcr9olumIDutgEVaT4Y4Tg/LcOLbWEIooKRgd+PO+ozNjnsVEyjnw== From: "Chuyi Zhou" User-Agent: Mozilla Thunderbird In-Reply-To: <20260522100835.rgqCLkwa@linutronix.de> References: <20260513124524.2569867-1-zhouchuyi@bytedance.com> <20260513124524.2569867-7-zhouchuyi@bytedance.com> <20260522100835.rgqCLkwa@linutronix.de> Precedence: bulk X-Mailing-List: linux-kernel@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: Mime-Version: 1.0 X-Lms-Return-Path: Subject: Re: [RESEND PATCH v5 06/12] smp: Enable preemption early in smp_call_function_many_cond To: "Sebastian Andrzej Siewior" Cc: , , , , , , , , , , , , Date: Fri, 22 May 2026 23:16:22 +0800 Message-Id: Content-Transfer-Encoding: 7bit X-Original-From: Chuyi Zhou Content-Type: text/plain; charset=UTF-8 On 2026-05-22 6:08 p.m., Sebastian Andrzej Siewior wrote: > On 2026-05-13 20:45:18 [+0800], Chuyi Zhou wrote: >> --- a/kernel/smp.c >> +++ b/kernel/smp.c >> @@ -861,10 +861,10 @@ static void smp_call_function_many_cond(const struct cpumask *mask, >> int nr_cpus = 0; >> bool run_remote = false; >> >> - lockdep_assert_preemption_disabled(); >> - >> task_mask = smp_task_ipi_mask(current); >> - preemptible_wait = task_mask; >> + preemptible_wait = task_mask && preemptible(); > > Now that I stare at this again, why is preemptible() a thing here? > You care about doing put_cpu() below before csd_lock_wait(). This can > only be done if you get a cpumask from smp_task_ipi_mask(). If > preemption or interrupts is/are disabled then you still can use the > "private" cpumask and do the early put_cpu(). It simply is no > optimisation. > > This basically reduces the check to a CONFIG_PREEMPTION=y kernel because > otherwise you have no cpumask. And this is not done on !SMP kernels. > smp_task_ipi_mask() is only used here so there is no need to export it > via headers. > Agreed. The safety condition for the early put_cpu() is the availability of the task-local cpumask, not preemptible(). If an outer context already keeps preemption or interrupts disabled, the early put_cpu() simply does not buy anything. I will make the condition depend on task_mask directly, drop the preemptible() check, keep smp_task_ipi_mask() local to kernel/smp.c, and adjust the comment as suggested. Thanks. >> + >> + this_cpu = get_cpu(); >> cfd = this_cpu_ptr(&cfd_data); >> cpumask = preemptible_wait ? task_mask : cfd->cpumask; >> >> @@ -946,6 +946,19 @@ static void smp_call_function_many_cond(const struct cpumask *mask, >> local_irq_restore(flags); >> } >> >> + /* >> + * We may block in csd_lock_wait() for a significant amount of time, >> + * especially when interrupts are disabled or with a large number of >> + * remote CPUs. Try to enable preemption before csd_lock_wait(). > > If interrupts are disabled there is no gain. Also we sort of expect > interrupts to be enabled here. > > Waiting for completion can take time especially with many CPUs. On a > PREEMPTIBLE kernel a per-task cpumask is used to track CPUs with > pending IPI request. This allows to enable preemption and potentially > wait while allowing task preemption. On a !PREEMPTIBLE the cpumask is > shared and the call must block until completion to avoid modifications > by a another caller on this CPU. > >> + * >> + * Use the task_mask instead of cfd->cpumask to avoid concurrency >> + * modification from tasks on the same cpu. If preemption occurs during >> + * csd_lock_wait, other concurrent smp_call_function_many_cond() calls >> + * will simply block until the previous csd->func() completes. >> + */ >> + if (preemptible_wait) >> + put_cpu(); >> + >> if (run_remote && wait) { >> for_each_cpu(cpu, cpumask) { >> call_single_data_t *csd; >> @@ -954,6 +967,9 @@ static void smp_call_function_many_cond(const struct cpumask *mask, >> csd_lock_wait(csd); >> } >> } >> + >> + if (!preemptible_wait) >> + put_cpu(); >> } >> >> /** > > Sebastian