On Thu, 18 Nov 2021 04:32:40 +0100, Vaibhav Agarwal wrote:
On Thu, Nov 18, 2021 at 3:25 AM Alex Elder firstname.lastname@example.org wrote:
On 11/17/21 3:02 PM, Takashi Iwai wrote:
On Wed, 17 Nov 2021 20:56:14 +0100, Alex Elder wrote:
On 11/16/21 1:20 AM, Takashi Iwai wrote:
snd_ctl_remove() has to be called with card->controls_rwsem held (when called after the card instantiation). This patch adds the missing rwsem calls around it.
I see the comment above snd_ctl_remove() that says you must hold the write lock. And given that, this seems correct to me.
I understand why you want to take the lock just once, rather than each time snd_ctl_remove() is called.
However I believe the acquisition and release of the lock belongs inside gbaudio_remove_controls(), not in its caller.
If you disagree, can you please explain why?
In general if the function returns an error and has a loop inside, taking a lock in the caller side avoids the forgotten unlock.
But taking the lock in the called function makes the caller not need to take the lock (which would be even more valuable if there were more than one caller).
I prefer having the lock acquisition in the called function. Please send version 2, as I suggested.
Thanks for sharing this patch. In reference to the suggestion from Alex, do you think replacing snd_ctl_find_id(), snd_ctl_remove() with snd_ctl_remove_id() inside gbaudio_remove_controls() would be an even better choice without worrying about locks?
Yeah, that sounds like a better plan, indeed.