[PATCH v2 0/5] ALSA: control - add generic LED API

[Jaroslav Kysela]

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:

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

-- 
Jaroslav Kysela <perex at perex.cz>
Linux Sound Maintainer; ALSA Project; Red Hat, Inc.