Hi Bjorn, On 9/29/2016 5:49 PM, Bjorn Helgaas wrote: >> + } > This pattern of "unlock, do something, relock" needs some > justification. In general it's unsafe because the lock is protecting > *something*, and you have to assume that something can change as soon > as you unlock. Maybe you know it's safe in this situation, and if so, > the explanation of why it's safe is what I'm looking for. Agreed. The problem is that save and restore routines obtain the lock again and they fails as the lock is already held. The alternative is to change the dev_locks in save and restore to try_lock so that it will work if locks were previously obtained or not. > > Also, you're now calling pci_reset_bridge_secondary_bus() with the dev > unlocked, where we called it with the dev locked before. Some (but > worryingly, not all) of the other pci_reset_bridge_secondary_bus() > callers also have the dev locked. I didn't look long enough to figure > out if there is a strategy there or if these inconsistencies are > latent bugs. > The goal of this routine is to reset the device not the bridge and the code will use FLR or others if available. Therefore, it makes perfect sense to obtain the device lock while doing this. The code tries to reset the bus if none of the other resets work. This is where the problem is. It destroys the context for other devices. I can fix get rid of this unlock, do something and then lock again business by rewriting the locks in save and restore. Sinan -- Sinan Kaya Qualcomm Datacenter Technologies, Inc. as an affiliate of Qualcomm Technologies, Inc. Qualcomm Technologies, Inc. is a member of the Code Aurora Forum, a Linux Foundation Collaborative Project. -- To unsubscribe from this list: send the line "unsubscribe linux-pci" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
