Re: [PATCH] scsi/fcoe: simplify fcoe_select_cpu()

[Yury Norov <yury.norov@gmail.com>]

+ 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