From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from stravinsky.debian.org (stravinsky.debian.org [82.195.75.108]) (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 8642F37DE81 for ; Fri, 10 Apr 2026 10:42:16 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=82.195.75.108 ARC-Seal:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1775817738; cv=none; b=qOVGwlh4ojOVJvKSL3tHt5y7CazGaZtnT361UfBvg8N3zF5Ht6YAgxaI48fS4VqA6f5GCj6PIdYARTO+gn7piTScHSto4mVTgi6bYP3bMmHJJXnMqGNnyToq4OxbH2RZZuPHRD2cDJctQtu6OPiXQhLs11B483tYHsmNqS83njo= ARC-Message-Signature:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1775817738; c=relaxed/simple; bh=BWjsoP3LbXumeg5SWSgoFPbMXhIiugyfxSrrCtLbgjk=; h=Date:From:To:Cc:Subject:Message-ID:References:MIME-Version: Content-Type:Content-Disposition:In-Reply-To; b=PwtpxGt9q5xKiNBgTWkug4VEU/F/TqB26G4pIBnwhE2im09l6S9VyK9uTeKhRvVj/i+F9hKVkj4N4JGnosHh2GfD+sf9ZwYOjXnDcN+8sbsZXqlu18YGCB5eLsUd13DDiOI9uI1mu9WvS6dk5bH8aYALZWoheGXbM1aMMbMhJ5M= ARC-Authentication-Results:i=1; smtp.subspace.kernel.org; dmarc=none (p=none dis=none) header.from=debian.org; spf=none smtp.mailfrom=debian.org; dkim=pass (2048-bit key) header.d=debian.org header.i=@debian.org header.b=ci6e1++8; arc=none smtp.client-ip=82.195.75.108 Authentication-Results: smtp.subspace.kernel.org; dmarc=none (p=none dis=none) header.from=debian.org Authentication-Results: smtp.subspace.kernel.org; spf=none smtp.mailfrom=debian.org Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=debian.org header.i=@debian.org header.b="ci6e1++8" DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=debian.org; s=smtpauto.stravinsky; h=X-Debian-User:In-Reply-To:Content-Type:MIME-Version: References:Message-ID:Subject:Cc:To:From:Date:Reply-To: Content-Transfer-Encoding:Content-ID:Content-Description; bh=/tDwItey/SJ4uwQlkYh39gp/hoZ0HlTlWt4PqqzvcME=; b=ci6e1++86xbOCRYHzAeb63ZZx+ ltfw8xC2PaNBPEL97jkMg8Uh/gXBvnvdAIV+Gw2mcWB2smnXaEpLgPMF8wRnma9azCRqfgLJWy5Pf 7OI3fFaFqvSkEeW65LQx34lEY0zDT5X4EZOYQDDdRrwuOt0r0duBZ5UkFR84p1cSb7b0zy5PWkD7O g1Qd1Ufc5XEf103dJ+VoQ6ZrNl5rILUjW4zaqy2vkAlq9XW93uyNKEfo/P7n3vJbgzMLiqXRjeizp u3e97nn0ehboLEx5i+kowfXh01kFdVrsExtUT1ib4bWWNG+lj9zQ9aGC4yaVWj2Ot4rjhNjgd5XAQ vroOEYfA==; Received: from authenticated user by stravinsky.debian.org with esmtpsa (TLS1.3:ECDHE_X25519__RSA_PSS_RSAE_SHA256__AES_256_GCM:256) (Exim 4.96) (envelope-from ) id 1wB9JN-009vGB-0j; Fri, 10 Apr 2026 10:42:05 +0000 Date: Fri, 10 Apr 2026 03:42:00 -0700 From: Breno Leitao To: Tejun Heo Cc: Lai Jiangshan , linux-kernel@vger.kernel.org, kernel-team@meta.com, kernel test robot Subject: Re: [PATCH] workqueue: validate cpumask_first() result in llc_populate_cpu_shard_id() Message-ID: References: <20260410-workqueue_fix_nios-v1-1-abbb26575b1b@debian.org> 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=us-ascii Content-Disposition: inline In-Reply-To: X-Debian-User: leitao Hello Tejun, On Thu, Apr 09, 2026 at 11:00:30PM -1000, Tejun Heo wrote: > On Fri, Apr 10, 2026 at 01:49:50AM -0700, Breno Leitao wrote: > > In llc_populate_cpu_shard_id(), cpumask_first(sibling_cpus) is used to > > find the leader CPU, and the result is then used to index into > > cpu_shard_id[]. Add a bounds check with WARN_ON_ONCE to guard against > > unexpected values before using it as an array index. > > > > Store the result in a local variable to make the code clearer, as also > > to avoid calling cpumask_first() twice. > > > > Fixes: 5920d046f7ae3 ("workqueue: add WQ_AFFN_CACHE_SHARD affinity scope") > ... > > @@ -8318,7 +8319,11 @@ static void __init llc_populate_cpu_shard_id(const struct cpumask *pod_cpus, > > * The siblings' shard MUST be the same as the leader. > > * never split threads in the same core. > > */ > > - cpu_shard_id[c] = cpu_shard_id[cpumask_first(sibling_cpus)]; > > + leader = cpumask_first(sibling_cpus); > > + > > + if (WARN_ON_ONCE(leader >= nr_cpu_ids)) > > + continue; > > + cpu_shard_id[c] = cpu_shard_id[leader]; > > sibling_cpus can't be empty, right? Correct. sibling_cpus will have, at least, 'c' set. > This is mostly to shut up the reported > compiler warning? If so, can you please note that in a ocmment and the > description? Sure. Is something like the following acceptable? diff --git a/kernel/workqueue.c b/kernel/workqueue.c index 083d8fe301f46..5dc304cdfa7f9 100644 --- a/kernel/workqueue.c +++ b/kernel/workqueue.c @@ -8300,6 +8300,7 @@ static void __init llc_populate_cpu_shard_id(const struct cpumask *pod_cpus, int cores_in_shard = 0; /* This is a cursor for the shards. Go from zero to nr_shards - 1*/ int shard_id = 0; + int leader; int c; /* Iterate at every CPU for a given LLC pod, and assign it a shard */ @@ -8318,7 +8319,17 @@ static void __init llc_populate_cpu_shard_id(const struct cpumask *pod_cpus, * The siblings' shard MUST be the same as the leader. * never split threads in the same core. */ - cpu_shard_id[c] = cpu_shard_id[cpumask_first(sibling_cpus)]; + leader = cpumask_first(sibling_cpus); + + /* + * sibling_cpus cannot be empty here since 'c' + * is always set in it. This check silences a + * compiler warning about using the unchecked + * cpumask_first() result as an array index. + */ + if (WARN_ON_ONCE(leader >= nr_cpu_ids)) + continue; + cpu_shard_id[c] = cpu_shard_id[leader]; } }