diff mbox series

[09/11] ALSA: control: Introduce unlocked version for snd_ctl_find_*() helpers

Message ID 20230718141304.1032-10-tiwai@suse.de
State Accepted
Commit b1e055f67611daf098e27e8731386eeb5257bde3
Headers show
Series ALSA: Make control API taking controls_rwsem consistently | expand

Commit Message

Takashi Iwai July 18, 2023, 2:13 p.m. UTC 
For reducing the unnecessary use of controls_rwsem in the drivers,
this patch adds a new variant for snd_ctl_find_*() helpers:
snd_ctl_find_id_locked() and snd_ctl_find_numid_locked() look for a
kctl element inside the card->controls_rwsem -- that is, doing the
very same as what snd_ctl_find_id() and snd_ctl_find_numid() did until
now.  snd_ctl_find_id() and snd_ctl_find_numid() remain same,
i.e. still unlocked version, but they will be switched to locked
version once after all callers are replaced.

The patch also replaces the calls of snd_ctl_find_id() and
snd_ctl_find_numid() in a few places; all of those are places where we
know that the functions are called properly with controls_rwsem held.
All others are without rwsem (although they should have been).

After this patch, we'll turn on the locking in snd_ctl_find_id() and
snd_ctl_find_numid() to be more race-free.

Signed-off-by: Takashi Iwai <tiwai@suse.de>
---
 include/sound/control.h     |  4 ++-
 sound/core/control.c        | 69 +++++++++++++++++++++++++++----------
 sound/core/control_compat.c |  2 +-
 sound/core/control_led.c    |  2 +-
 sound/core/oss/mixer_oss.c  | 10 +++---
 sound/pci/emu10k1/emufx.c   |  2 +-
 6 files changed, 61 insertions(+), 28 deletions(-)
diff mbox series

Patch

diff --git a/include/sound/control.h b/include/sound/control.h
index e61b749bf204..42e8dbb22d8e 100644
--- a/include/sound/control.h
+++ b/include/sound/control.h
@@ -140,7 +140,9 @@  int snd_ctl_remove_id(struct snd_card * card, struct snd_ctl_elem_id *id);
 int snd_ctl_rename_id(struct snd_card * card, struct snd_ctl_elem_id *src_id, struct snd_ctl_elem_id *dst_id);
 void snd_ctl_rename(struct snd_card *card, struct snd_kcontrol *kctl, const char *name);
 int snd_ctl_activate_id(struct snd_card *card, struct snd_ctl_elem_id *id, int active);
-struct snd_kcontrol *snd_ctl_find_numid(struct snd_card * card, unsigned int numid);
+struct snd_kcontrol *snd_ctl_find_numid_locked(struct snd_card *card, unsigned int numid);
+struct snd_kcontrol *snd_ctl_find_numid(struct snd_card *card, unsigned int numid);
+struct snd_kcontrol *snd_ctl_find_id_locked(struct snd_card *card, const struct snd_ctl_elem_id *id);
 struct snd_kcontrol *snd_ctl_find_id(struct snd_card *card, const struct snd_ctl_elem_id *id);
 
 int snd_ctl_create(struct snd_card *card);
diff --git a/sound/core/control.c b/sound/core/control.c
index 180e5768a10f..30741293708d 100644
--- a/sound/core/control.c
+++ b/sound/core/control.c
@@ -475,7 +475,7 @@  static int __snd_ctl_add_replace(struct snd_card *card,
 	if (id.index > UINT_MAX - kcontrol->count)
 		return -EINVAL;
 
-	old = snd_ctl_find_id(card, &id);
+	old = snd_ctl_find_id_locked(card, &id);
 	if (!old) {
 		if (mode == CTL_REPLACE)
 			return -EINVAL;
@@ -641,7 +641,7 @@  int snd_ctl_remove_id(struct snd_card *card, struct snd_ctl_elem_id *id)
 	int ret;
 
 	down_write(&card->controls_rwsem);
-	kctl = snd_ctl_find_id(card, id);
+	kctl = snd_ctl_find_id_locked(card, id);
 	if (kctl == NULL) {
 		up_write(&card->controls_rwsem);
 		return -ENOENT;
@@ -670,7 +670,7 @@  static int snd_ctl_remove_user_ctl(struct snd_ctl_file * file,
 	int idx, ret;
 
 	down_write(&card->controls_rwsem);
-	kctl = snd_ctl_find_id(card, id);
+	kctl = snd_ctl_find_id_locked(card, id);
 	if (kctl == NULL) {
 		ret = -ENOENT;
 		goto error;
@@ -711,7 +711,7 @@  int snd_ctl_activate_id(struct snd_card *card, struct snd_ctl_elem_id *id,
 	int ret;
 
 	down_write(&card->controls_rwsem);
-	kctl = snd_ctl_find_id(card, id);
+	kctl = snd_ctl_find_id_locked(card, id);
 	if (kctl == NULL) {
 		ret = -ENOENT;
 		goto unlock;
@@ -765,7 +765,7 @@  int snd_ctl_rename_id(struct snd_card *card, struct snd_ctl_elem_id *src_id,
 	int saved_numid;
 
 	down_write(&card->controls_rwsem);
-	kctl = snd_ctl_find_id(card, src_id);
+	kctl = snd_ctl_find_id_locked(card, src_id);
 	if (kctl == NULL) {
 		up_write(&card->controls_rwsem);
 		return -ENOENT;
@@ -820,7 +820,7 @@  snd_ctl_find_numid_slow(struct snd_card *card, unsigned int numid)
 #endif /* !CONFIG_SND_CTL_FAST_LOOKUP */
 
 /**
- * snd_ctl_find_numid - find the control instance with the given number-id
+ * snd_ctl_find_numid_locked - find the control instance with the given number-id
  * @card: the card instance
  * @numid: the number-id to search
  *
@@ -830,9 +830,9 @@  snd_ctl_find_numid_slow(struct snd_card *card, unsigned int numid)
  * (if the race condition can happen).
  *
  * Return: The pointer of the instance if found, or %NULL if not.
- *
  */
-struct snd_kcontrol *snd_ctl_find_numid(struct snd_card *card, unsigned int numid)
+struct snd_kcontrol *
+snd_ctl_find_numid_locked(struct snd_card *card, unsigned int numid)
 {
 	if (snd_BUG_ON(!card || !numid))
 		return NULL;
@@ -842,10 +842,26 @@  struct snd_kcontrol *snd_ctl_find_numid(struct snd_card *card, unsigned int numi
 	return snd_ctl_find_numid_slow(card, numid);
 #endif
 }
+EXPORT_SYMBOL(snd_ctl_find_numid_locked);
+
+/**
+ * snd_ctl_find_numid - find the control instance with the given number-id
+ * @card: the card instance
+ * @numid: the number-id to search
+ *
+ * Finds the control instance with the given number-id from the card.
+ *
+ * Return: The pointer of the instance if found, or %NULL if not.
+ */
+struct snd_kcontrol *snd_ctl_find_numid(struct snd_card *card,
+					unsigned int numid)
+{
+	return snd_ctl_find_numid_locked(card, numid);
+}
 EXPORT_SYMBOL(snd_ctl_find_numid);
 
 /**
- * snd_ctl_find_id - find the control instance with the given id
+ * snd_ctl_find_id_locked - find the control instance with the given id
  * @card: the card instance
  * @id: the id to search
  *
@@ -855,17 +871,16 @@  EXPORT_SYMBOL(snd_ctl_find_numid);
  * (if the race condition can happen).
  *
  * Return: The pointer of the instance if found, or %NULL if not.
- *
  */
-struct snd_kcontrol *snd_ctl_find_id(struct snd_card *card,
-				     const struct snd_ctl_elem_id *id)
+struct snd_kcontrol *snd_ctl_find_id_locked(struct snd_card *card,
+					    const struct snd_ctl_elem_id *id)
 {
 	struct snd_kcontrol *kctl;
 
 	if (snd_BUG_ON(!card || !id))
 		return NULL;
 	if (id->numid != 0)
-		return snd_ctl_find_numid(card, id->numid);
+		return snd_ctl_find_numid_locked(card, id->numid);
 #ifdef CONFIG_SND_CTL_FAST_LOOKUP
 	kctl = xa_load(&card->ctl_hash, get_ctl_id_hash(id));
 	if (kctl && elem_id_matches(kctl, id))
@@ -880,6 +895,22 @@  struct snd_kcontrol *snd_ctl_find_id(struct snd_card *card,
 
 	return NULL;
 }
+EXPORT_SYMBOL(snd_ctl_find_id_locked);
+
+/**
+ * snd_ctl_find_id - find the control instance with the given id
+ * @card: the card instance
+ * @id: the id to search
+ *
+ * Finds the control instance with the given id from the card.
+ *
+ * Return: The pointer of the instance if found, or %NULL if not.
+ */
+struct snd_kcontrol *snd_ctl_find_id(struct snd_card *card,
+				     const struct snd_ctl_elem_id *id)
+{
+	return snd_ctl_find_id_locked(card, id);
+}
 EXPORT_SYMBOL(snd_ctl_find_id);
 
 static int snd_ctl_card_info(struct snd_card *card, struct snd_ctl_file * ctl,
@@ -1194,7 +1225,7 @@  static int snd_ctl_elem_info(struct snd_ctl_file *ctl,
 	int result;
 
 	down_read(&card->controls_rwsem);
-	kctl = snd_ctl_find_id(card, &info->id);
+	kctl = snd_ctl_find_id_locked(card, &info->id);
 	if (kctl == NULL)
 		result = -ENOENT;
 	else
@@ -1233,7 +1264,7 @@  static int snd_ctl_elem_read(struct snd_card *card,
 	int ret;
 
 	down_read(&card->controls_rwsem);
-	kctl = snd_ctl_find_id(card, &control->id);
+	kctl = snd_ctl_find_id_locked(card, &control->id);
 	if (kctl == NULL) {
 		ret = -ENOENT;
 		goto unlock;
@@ -1310,7 +1341,7 @@  static int snd_ctl_elem_write(struct snd_card *card, struct snd_ctl_file *file,
 	int result;
 
 	down_write(&card->controls_rwsem);
-	kctl = snd_ctl_find_id(card, &control->id);
+	kctl = snd_ctl_find_id_locked(card, &control->id);
 	if (kctl == NULL) {
 		up_write(&card->controls_rwsem);
 		return -ENOENT;
@@ -1391,7 +1422,7 @@  static int snd_ctl_elem_lock(struct snd_ctl_file *file,
 	if (copy_from_user(&id, _id, sizeof(id)))
 		return -EFAULT;
 	down_write(&card->controls_rwsem);
-	kctl = snd_ctl_find_id(card, &id);
+	kctl = snd_ctl_find_id_locked(card, &id);
 	if (kctl == NULL) {
 		result = -ENOENT;
 	} else {
@@ -1419,7 +1450,7 @@  static int snd_ctl_elem_unlock(struct snd_ctl_file *file,
 	if (copy_from_user(&id, _id, sizeof(id)))
 		return -EFAULT;
 	down_write(&card->controls_rwsem);
-	kctl = snd_ctl_find_id(card, &id);
+	kctl = snd_ctl_find_id_locked(card, &id);
 	if (kctl == NULL) {
 		result = -ENOENT;
 	} else {
@@ -1927,7 +1958,7 @@  static int snd_ctl_tlv_ioctl(struct snd_ctl_file *file,
 	container_size = header.length;
 	container = buf->tlv;
 
-	kctl = snd_ctl_find_numid(file->card, header.numid);
+	kctl = snd_ctl_find_numid_locked(file->card, header.numid);
 	if (kctl == NULL)
 		return -ENOENT;
 
diff --git a/sound/core/control_compat.c b/sound/core/control_compat.c
index 9cae5d74335c..0e8b1bfb040e 100644
--- a/sound/core/control_compat.c
+++ b/sound/core/control_compat.c
@@ -173,7 +173,7 @@  static int get_ctl_type(struct snd_card *card, struct snd_ctl_elem_id *id,
 	int err;
 
 	down_read(&card->controls_rwsem);
-	kctl = snd_ctl_find_id(card, id);
+	kctl = snd_ctl_find_id_locked(card, id);
 	if (! kctl) {
 		up_read(&card->controls_rwsem);
 		return -ENOENT;
diff --git a/sound/core/control_led.c b/sound/core/control_led.c
index ee77547bf8dc..67fc2a1dcf7a 100644
--- a/sound/core/control_led.c
+++ b/sound/core/control_led.c
@@ -251,7 +251,7 @@  static int snd_ctl_led_set_id(int card_number, struct snd_ctl_elem_id *id,
 	card = snd_card_ref(card_number);
 	if (card) {
 		down_write(&card->controls_rwsem);
-		kctl = snd_ctl_find_id(card, id);
+		kctl = snd_ctl_find_id_locked(card, id);
 		if (kctl) {
 			ioff = snd_ctl_get_ioff(kctl, id);
 			vd = &kctl->vd[ioff];
diff --git a/sound/core/oss/mixer_oss.c b/sound/core/oss/mixer_oss.c
index 9620115cfdc0..dae2da380835 100644
--- a/sound/core/oss/mixer_oss.c
+++ b/sound/core/oss/mixer_oss.c
@@ -524,7 +524,7 @@  static struct snd_kcontrol *snd_mixer_oss_test_id(struct snd_mixer_oss *mixer, c
 	id.iface = SNDRV_CTL_ELEM_IFACE_MIXER;
 	strscpy(id.name, name, sizeof(id.name));
 	id.index = index;
-	return snd_ctl_find_id(card, &id);
+	return snd_ctl_find_id_locked(card, &id);
 }
 
 static void snd_mixer_oss_get_volume1_vol(struct snd_mixer_oss_file *fmixer,
@@ -540,7 +540,7 @@  static void snd_mixer_oss_get_volume1_vol(struct snd_mixer_oss_file *fmixer,
 	if (numid == ID_UNKNOWN)
 		return;
 	down_read(&card->controls_rwsem);
-	kctl = snd_ctl_find_numid(card, numid);
+	kctl = snd_ctl_find_numid_locked(card, numid);
 	if (!kctl) {
 		up_read(&card->controls_rwsem);
 		return;
@@ -579,7 +579,7 @@  static void snd_mixer_oss_get_volume1_sw(struct snd_mixer_oss_file *fmixer,
 	if (numid == ID_UNKNOWN)
 		return;
 	down_read(&card->controls_rwsem);
-	kctl = snd_ctl_find_numid(card, numid);
+	kctl = snd_ctl_find_numid_locked(card, numid);
 	if (!kctl) {
 		up_read(&card->controls_rwsem);
 		return;
@@ -645,7 +645,7 @@  static void snd_mixer_oss_put_volume1_vol(struct snd_mixer_oss_file *fmixer,
 	if (numid == ID_UNKNOWN)
 		return;
 	down_read(&card->controls_rwsem);
-	kctl = snd_ctl_find_numid(card, numid);
+	kctl = snd_ctl_find_numid_locked(card, numid);
 	if (!kctl) {
 		up_read(&card->controls_rwsem);
 		return;
@@ -688,7 +688,7 @@  static void snd_mixer_oss_put_volume1_sw(struct snd_mixer_oss_file *fmixer,
 	if (numid == ID_UNKNOWN)
 		return;
 	down_read(&card->controls_rwsem);
-	kctl = snd_ctl_find_numid(card, numid);
+	kctl = snd_ctl_find_numid_locked(card, numid);
 	if (!kctl) {
 		up_read(&card->controls_rwsem);
 		return;
diff --git a/sound/pci/emu10k1/emufx.c b/sound/pci/emu10k1/emufx.c
index 70c8252a92d9..318a064f2cec 100644
--- a/sound/pci/emu10k1/emufx.c
+++ b/sound/pci/emu10k1/emufx.c
@@ -800,7 +800,7 @@  static int snd_emu10k1_verify_controls(struct snd_emu10k1 *emu,
 			continue;
 		gctl_id = (struct snd_ctl_elem_id *)&gctl->id;
 		down_read(&emu->card->controls_rwsem);
-		if (snd_ctl_find_id(emu->card, gctl_id)) {
+		if (snd_ctl_find_id_locked(emu->card, gctl_id)) {
 			up_read(&emu->card->controls_rwsem);
 			err = -EEXIST;
 			goto __error;