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