Re: [PATCH v2 3/3] Drivers: hv: vmbus: Check for pending channel interrupts before taking a CPU offline

[Andrea Parri <parri.andrea@gmail.com>]

> > @@ -336,6 +372,19 @@ int hv_synic_cleanup(unsigned int cpu)
> >  	if (channel_found && vmbus_connection.conn_state == CONNECTED)
> >  		return -EBUSY;
> > 
> > +	if (vmbus_proto_version >= VERSION_WIN10_V4_1) {
> > +		/*
> > +		 * channel_found == false means that any channels that were previously
> > +		 * assigned to the CPU have been reassigned elsewhere with a call of
> > +		 * vmbus_send_modifychannel().  Scan the event flags page looking for
> > +		 * bits that are set and waiting with a timeout for vmbus_chan_sched()
> > +		 * to process such bits.  If bits are still set after this operation
> > +		 * and VMBus is connected, fail the CPU offlining operation.
> > +		 */
> > +		if (hv_synic_event_pending() && vmbus_connection.conn_state == CONNECTED)
> > +			return -EBUSY;
> > +	}
> > +
> 
> Perhaps the test for conn_state == CONNECTED could be factored out as follows.  If we're
> not CONNECTED (i.e., in the panic or kexec path) we should be able to also skip the search
> for channels that are bound to the CPU since we will ignore the result anyway.
> 
> 	if (vmbus_connection.conn_state != CONNECTED)
> 		goto always_cleanup;
> 
> 	if (cpu == VMBUS_CONNECT_CPU)
> 		return -EBUSY;
> 
> 	[Code to search for channels that are bound to the CPU we're about to clean up]
> 	
> 	if (channel_found)
> 		return -EBUSY;
> 
> 	/*
> 	 * channel_found == false means that any channels that were previously
> 	 * assigned to the CPU have been reassigned elsewhere with a call of
> 	 * vmbus_send_modifychannel().  Scan the event flags page looking for
> 	 * bits that are set and waiting with a timeout for vmbus_chan_sched()
> 	 * to process such bits.  If bits are still set after this operation
> 	 * and VMBus is connected, fail the CPU offlining operation.
> 	 */
> 	if (vmbus_proto_version >= VERSION_WIN10_V4_1 && hv_synic_event_pending())
> 		return -EBUSY;
> 
> always_cleanup:

Agreed, applied.  Thank you for the suggestion,

  Andrea