Hi,
On 2/21/21 7:01 PM, Jaroslav Kysela wrote:
> Dne 21. 02. 21 v 14:14 Hans de Goede napsal(a):
>
>>> v2 changes:
>>> - fix the locking - remove the controls_rwsem read lock
>>> in the element get (the consistency is already protected
>>> with the global snd_ctl_led_mutex and possible partial
>>> value writes are catched with the next value change
>>> notification callback)
>>
>> I'm afraid that lockdep still is unhappy. With v2 there is a new
>> (different) lockdep warning.
>
>> If you can send me another fixup-diff then I'll make sure to
>> test this before you do a v3, so that we can be sure that
>> all cases which my setup catches are resolved before sending
>> out v3.
>
> Thank you for your test. This change (on top of v2) should resolve this
> remaining lockdep:
I can confirm that I'm not seeing any more lockdeps warning after
applying this on top of v2 of the series.
Regards,
Hans
> diff --git a/sound/core/control.c b/sound/core/control.c
> index c9f062fada0a..494f0154e8be 100644
> --- a/sound/core/control.c
> +++ b/sound/core/control.c
> @@ -2051,7 +2051,9 @@ void snd_ctl_register_layer(struct snd_ctl_layer_ops *lops)
> for (card_number = 0; card_number < SNDRV_CARDS; card_number++) {
> card = snd_card_ref(card_number);
> if (card) {
> + down_read(&card->controls_rwsem);
> lops->lregister(card);
> + up_read(&card->controls_rwsem);
> snd_card_unref(card);
> }
> }
> @@ -2113,10 +2115,12 @@ static int snd_ctl_dev_register(struct snd_device *device)
> &snd_ctl_f_ops, card, &card->ctl_dev);
> if (err < 0)
> return err;
> + down_read(&card->controls_rwsem);
> down_read(&snd_ctl_layer_rwsem);
> for (lops = snd_ctl_layer; lops; lops = lops->next)
> lops->lregister(card);
> up_read(&snd_ctl_layer_rwsem);
> + up_read(&card->controls_rwsem);
> return 0;
> }
>
> @@ -2137,10 +2141,12 @@ static int snd_ctl_dev_disconnect(struct snd_device
> *device)
> }
> read_unlock_irqrestore(&card->ctl_files_rwlock, flags);
>
> + down_read(&card->controls_rwsem);
> down_read(&snd_ctl_layer_rwsem);
> for (lops = snd_ctl_layer; lops; lops = lops->next)
> lops->ldisconnect(card);
> up_read(&snd_ctl_layer_rwsem);
> + up_read(&card->controls_rwsem);
>
> return snd_unregister_device(&card->ctl_dev);
> }
> diff --git a/sound/core/control_led.c b/sound/core/control_led.c
> index cafe4c82ca35..b8bb8fd46686 100644
> --- a/sound/core/control_led.c
> +++ b/sound/core/control_led.c
> @@ -235,11 +235,10 @@ static void snd_ctl_led_register(struct snd_card *card)
> mutex_lock(&snd_ctl_led_mutex);
> snd_ctl_led_card_valid[card->number] = true;
> mutex_unlock(&snd_ctl_led_mutex);
> - down_read(&card->controls_rwsem);
> + /* the register callback is already called with held card->controls_rwsem */
> list_for_each_entry(kctl, &card->controls, list)
> for (ioff = 0; ioff < kctl->count; ioff++)
> snd_ctl_led_notify(card, SNDRV_CTL_EVENT_MASK_VALUE, kctl, ioff);
> - up_read(&card->controls_rwsem);
> snd_ctl_led_refresh();
> }
>
>
> Jaroslav
>
