On Fri, Apr 24, 2020 at 02:42:06PM +0100, John Garry wrote:
> On 24/04/2020 11:23, Ming Lei wrote:
> > static int blk_mq_hctx_notify_offline(unsigned int cpu, struct hlist_node *node)
> > {
> > + struct blk_mq_hw_ctx *hctx = hlist_entry_safe(node,
> > + struct blk_mq_hw_ctx, cpuhp_online);
> > +
> > + if (!cpumask_test_cpu(cpu, hctx->cpumask))
> > + return 0;
> > +
> > + if ((cpumask_next_and(-1, hctx->cpumask, cpu_online_mask) != cpu) ||
> > + (cpumask_next_and(cpu, hctx->cpumask, cpu_online_mask) < nr_cpu_ids))
> > + return 0;
>
>
> nit: personally I prefer what we had previously, as it was easier to read,
> even if it did cause the code to be indented:
>
> if ((cpumask_next_and(-1, cpumask, online_mask) == cpu) &&
> (cpumask_next_and(cpu, cpumask, online_mask) >= nr_cpu_ids)) {
> // do deactivate
> }
I will document what the check does, given it can save us one level of
indentation.
>
> return 0
>
> and it could avoid the cpumask_test_cpu() test, unless you want that as an
> optimisation. If so, a comment could help.
cpumask_test_cpu() is more readable, and should be pattern for doing
this stuff in cpuhp handler, cause the handler is called for any CPU
to be offline.
Thanks,
Ming
