Re: [PATCH V8 07/11] blk-mq: stop to handle IO and drain IO before hctx becomes inactive

[Ming Lei <ming.lei@redhat.com>]

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