Oliver Neukum <oliver@xxxxxxxxxx> writes: > Am Donnerstag, 2. Februar 2012, 11:34:23 schrieb Bjørn Mork: >> @@ -523,7 +535,7 @@ static int wdm_open(struct inode *inode, struct file *file) >> goto out; >> >> desc = usb_get_intfdata(intf); >> - if (test_bit(WDM_DISCONNECTING, &desc->flags)) >> + if (test_bit(WDM_DISCONNECTING, &desc->flags) || test_bit(WDM_RESETTING, &desc->flags)) >> goto out; >> file->private_data = desc; >> > > No, you are misusing the flags here. We need a way to have mutual exclusion. > The flags may be set after we have passed that point of execution. OK, forget the flag and take the global wdm_mutex during reset? Or do you need another device specific mutex for this purpose to avoid locking all opens of any cdc-wdm device while one is being reset? Or assume that any write blocking will be short, and use the wlock both for reset and open? Both solutions still need some way to break out of a blocking read and make further reads return an error. Or maybe just make read return the error when reset is finished? I am sorry, but I am starting to feel a bit lost here. The locking does not work as it is, but I don't know how to fix it. And I don't think I fully understand what you are trying to achieve by providing the pre/post_reset functions in the first place. Read/write/open should return errors - so why not just leave everything to the default unbind/bind? It's not like any users of the character device is any better off with all the added complexity of the reset handling. I guess I'm missing something, but that fact tells me that I should be careful messing with this at all. So I think I'll leave this bug for now and continue with the subdriver thing, which is completely unrelated and actually won't be affected by it as there is no reset support in usbnet. And I'm not intending to add any either. reset => unbind + bind work as I expect with an end user hat on. Bjørn -- 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
