Re: [PATCH 1/2] staging: hv: Fix race condition in hv_utils moduleinitialization.

[Greg KH]

On Wed, May 19, 2010 at 08:30:25PM +0000, Haiyang Zhang wrote:
> > > +	/* Wait until all IC channels are initialized */
> > > +	while (atomic_read(&hv_utils_initcnt) < MAX_MSG_TYPES)
> > > +		msleep(100);
> > 
> > No, don't do this here, do something in your hv_vmbus core to handle
> > registering sub-drivers properly.  Perhaps you need to sleep there
> > before you can succeed on a initialization.
> 
> Thanks for the recommendation. I will put the sleep into vmbus_init to 
> ensure all channels are ready before the vmbus_init function exits.
> 
> > >  	hv_cb_utils[HV_SHUTDOWN_MSG].channel->OnChannelCallback =
> > >  		&shutdown_onchannelcallback;
> > 
> > The problem is that you just have a bunch of callbacks you are setting
> > up, it's not a "real" function call.  Please change it over to a
> > function call, like all other subsystems have.  Then, you can handle
> > any
> > "sleep until we are set up properly" issues in the vmbus code, not in
> > each and every individual bus driver.
> 
> Actually, we already assign a default callback function, chn_cb_negotiate(),
> when the channels are opened in vmbus module. It's a real function and can 
> handle common negotiation messages.

Then why don't you use it here?

> I will move the sleep into vmbus module as well.

I still think there's a real problem somewhere else in the architecture
if such a sleep is necessary...

Is the issue that the modprobe of the hv_vmbus can return before the bus
is really all set up and ready to go?  If so, just fix that, then you
will not need any "sleep" calls anywhere, right?

thanks,

greg k-h
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/