On Thu, 25 Mar 2021 17:13:41 +0100, Andrey Grodzovsky wrote: > > > > On 2021-03-25 2:58 a.m., Takashi Iwai wrote: > > On Wed, 24 Mar 2021 22:43:24 +0100, > > Andrey Grodzovsky wrote: > >> > >> Few comments - > >> > >> 1) Why we don't use snd_power_wait_and_ref in patch 3 in the common > >> handler ? > >> Don't we want the PCI rescan sequence to 'wait for' any in flight > >> taks that might be accessing registers and not only read/write/tlv > >> accesses ? > > > > Right, we don't need to block all ioctls, but only the ones that may > > access the hardware. So basically the patches 3-5 can be dropped if > > we take the patch 6. The current patch was written on top of the > > previous series, that's why it has both. > > > >> 2) Possible deadlock - > >> In azx_rescan_prepare - you put the card into SNDRV_CTL_POWER_D3hot > >> first and then 'wait for' all in flight tasks with the refcount. > >> The in flight tasks on the other hand, using snd_power_wait_and_ref, > >> may have already bumped up the refcount and now 'wait for' the card > >> to go into SNDRV_CTL_POWER_D0 which can't happen since PCI rescan > >> waits for the refocunt to drop to 0 before proceeding. > >> > >> Instead of snd_power_wait_and_ref can't we just call snd_power_ref > >> in common IOCTL before checking for power_state != SNDRV_CTL_POWER_D0 ? > >> Or is it because you don't want to fail IOCTLs ? > > > > No, this is the intended behavior and should work as-is because > > snd_power_wait_and_ref() drops the refcount in the loop before > > sleeping. The inc before the state check is a must for covering the > > possible race, and ditto for changing the power_state to D3hot before > > syncing. > > Ohh, missed the refcount dec in the wait loop... My bad. > > On boot the latest patch-set was throwing refcount warnings since > u are not supposed to inc from zerro count and so I fixed > with the attached patch. That way seems to work fine. Ah, I completely forgot about the oddness of refcount_t. Then I guess it's simpler to replace with atomic_*() instead. thanks, Takashi
