linux-kernel - Re: [PATCH] scsi/fcoe: simplify fcoe_select

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <aEDmXvKAvJfjMrCk@yury>
Date: Wed, 4 Jun 2025 20:35:42 -0400
From: Yury Norov <yury.norov@...il.com>
To: Bart Van Assche <bvanassche@....org>
Cc: Hannes Reinecke <hare@...e.de>,
	"James E.J. Bottomley" <James.Bottomley@...senpartnership.com>,
	"Martin K. Petersen" <martin.petersen@...cle.com>,
	linux-scsi@...r.kernel.org, linux-kernel@...r.kernel.org,
	Tejun Heo <tj@...nel.org>, Lai Jiangshan <jiangshanlai@...il.com>
Subject: Re: [PATCH] scsi/fcoe: simplify fcoe_select_cpu()

+ Tejun, Lai

On Thu, Jun 05, 2025 at 08:13:53AM +0800, Bart Van Assche wrote:
> On 6/5/25 7:42 AM, Yury Norov wrote:
> > diff --git a/drivers/scsi/fcoe/fcoe.c b/drivers/scsi/fcoe/fcoe.c
> > index b911fdb387f3..07eddafe52ff 100644
> > --- a/drivers/scsi/fcoe/fcoe.c
> > +++ b/drivers/scsi/fcoe/fcoe.c
> > @@ -1312,10 +1312,7 @@ static inline unsigned int fcoe_select_cpu(void)
> >   {
> >   	static unsigned int selected_cpu;
> > -	selected_cpu = cpumask_next(selected_cpu, cpu_online_mask);
> > -	if (selected_cpu >= nr_cpu_ids)
> > -		selected_cpu = cpumask_first(cpu_online_mask);
> > -
> > +	selected_cpu = cpumask_next_wrap(selected_cpu, cpu_online_mask);
> >   	return selected_cpu;
> >   }
> 
> Why does this algorithm occur in the FCoE driver? Isn't
> WORK_CPU_UNBOUND good enough for this driver? And if it isn't
> good enough, shouldn't this kind of functionality be integrated in
> kernel/workqueue.c rather than having the above algorithm in a
> kernel driver?

(I'm obviously not an expert in this driver, and just wanted to cleanup
the cpumask API usage.)

It looks like the intention is to distribute the workload among CPUs
sequentially. If you move this function out of the driver, someone
else may call the function, and sequential distribution may get
broken.

If sequential distribution doesn't matter here, and the real
intention is just to distribute workload more or less evenly,
we already have cpumask_any_distribute() for this.

Thanks,
Yury