On Mon, Jun 25, 2012 at 3:24 PM, Bjørn Mork <bjorn@xxxxxxx> wrote: > > True, but irrelevant. The pointer is either valid or NULL. We don't > need to care about synchronizing the exact time it is set to NULL. > > The locking in cdc-wdm will ensure that the pointer is valid while it is > in use by .manage_power, because usbnet_disconnect is prevented from > continuing with free_netdev() while any caller of .manage_power is > running. What I mean is that the situation is just what moving the set to NULL is doing. >> So it is only the sync mechanism that works on the race even the check is >> added in the patch. Putting usb_set_intfdata(, NULL) after driver_info->unbind >> should be OK, and it is a general solution for the problem. > > There is no problem wrt qmi_wwan and intfdata as long as the NULL test > is added to .manage_power. It depends on the ARCH or compiler. Considered there is not any locking/memory barrier between the set to NULL and read the pointer, also no ACCESS_ONCE on read or store the pointer, reading in .manage_power may see a invalid pointer if the CPU doesn't support Store Atomicity or the compiler does a byte-at-a-time optimization on the store[1]. So why not take the correct way in theory? also it is a general solution, and we can document its usage. [1], Paul mentioned it in the previous discussion http://lkml.org/lkml/2012/6/6/280 Thanks, -- Ming Lei -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
