On Fri, May 21, 2010 at 10:07:17PM +0000, Haiyang Zhang wrote: > > From: Greg KH [mailto:gregkh@xxxxxxx] > > > +/* Counter of IC channels initialized */ > > > +atomic_t hv_utils_initcnt = ATOMIC_INIT(0); > > > > This doesn't need to be an atomic variable, does it really? > > > > Why not have a simple bool variable "vmbus_initialized" or something. > > It starts out as false, and then turns true when you are up and ready. > > Then provide a function that tests it: > > bool hv_vmbus_ready(void) > > { > > return vmbus_initialized > > } > > EXPORT_SYMBOL_GPL(hv_vmbus_ready); > > > > > > this turns into a simple function call, again, never needing to know > > about message types or any other mess. > > This looks good. I will add the hv_vmbus_ready() function. It doesn't even > have to be exported symbol, because it's only used in vmbus module to ensure > all channels are ready before vmbus_init() returns. Other modules won't get a > chance to see uninitialized channels after hv_vmbus is loaded. > > Also, I'll cleanup the printk in hv_utils load/unload. > > Regarding the atomic variable -- the channel offer processing function is > triggered by interrupts from host -- should we be concerned about "counter++" > racing with each other in two interrupts happening around the same time? If you are, having races like this, then you should be using a lock to protect lots of things, not just one single atomic variable, right? thanks, greg k-h _______________________________________________ devel mailing list devel@xxxxxxxxxxxxxxxxxxxxxx http://driverdev.linuxdriverproject.org/mailman/listinfo/devel
