Skip to content

Commit

Permalink
ALSA: control_led: Use guard() for locking
Browse files Browse the repository at this point in the history 
We can simplify the code gracefully with new guard() macro and co for
automatic cleanup of locks.

A couple of functions that use snd_card_ref() and *_unref() are also
cleaned up with a defined class, too.

Only the code refactoring, and no functional changes.

Signed-off-by: Takashi Iwai <[email protected]>
Link: https://lore.kernel.org/r/[email protected]
  • Loading branch information
@tiwai
tiwai committed Feb 28, 2024
1 parent 3923de0 commit 7dba48a
Showing 1 changed file with 65 additions and 85 deletions.
150 changes: 65 additions & 85 deletions sound/core/control_led.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -147,29 +147,27 @@ static void snd_ctl_led_set_state(struct snd_card *card, unsigned int access,
return;
route = -1;
found = false;
mutex_lock(&snd_ctl_led_mutex);
/* the card may not be registered (active) at this point */
if (card && !snd_ctl_led_card_valid[card->number]) {
mutex_unlock(&snd_ctl_led_mutex);
return;
}
list_for_each_entry(lctl, &led->controls, list) {
if (lctl->kctl == kctl && lctl->index_offset == ioff)
found = true;
UPDATE_ROUTE(route, snd_ctl_led_get(lctl));
}
if (!found && kctl && card) {
lctl = kzalloc(sizeof(*lctl), GFP_KERNEL);
if (lctl) {
lctl->card = card;
lctl->access = access;
lctl->kctl = kctl;
lctl->index_offset = ioff;
list_add(&lctl->list, &led->controls);
scoped_guard(mutex, &snd_ctl_led_mutex) {
/* the card may not be registered (active) at this point */
if (card && !snd_ctl_led_card_valid[card->number])
return;
list_for_each_entry(lctl, &led->controls, list) {
if (lctl->kctl == kctl && lctl->index_offset == ioff)
found = true;
UPDATE_ROUTE(route, snd_ctl_led_get(lctl));
}
if (!found && kctl && card) {
lctl = kzalloc(sizeof(*lctl), GFP_KERNEL);
if (lctl) {
lctl->card = card;
lctl->access = access;
lctl->kctl = kctl;
lctl->index_offset = ioff;
list_add(&lctl->list, &led->controls);
UPDATE_ROUTE(route, snd_ctl_led_get(lctl));
}
}
}
mutex_unlock(&snd_ctl_led_mutex);
switch (led->mode) {
case MODE_OFF: route = 1; break;
case MODE_ON: route = 0; break;
Expand Down Expand Up @@ -201,14 +199,13 @@ static unsigned int snd_ctl_led_remove(struct snd_kcontrol *kctl, unsigned int i
struct snd_ctl_led_ctl *lctl;
unsigned int ret = 0;

mutex_lock(&snd_ctl_led_mutex);
guard(mutex)(&snd_ctl_led_mutex);
lctl = snd_ctl_led_find(kctl, ioff);
if (lctl && (access == 0 || access != lctl->access)) {
ret = lctl->access;
list_del(&lctl->list);
kfree(lctl);
}
mutex_unlock(&snd_ctl_led_mutex);
return ret;
}

Expand Down Expand Up @@ -239,44 +236,36 @@ static void snd_ctl_led_notify(struct snd_card *card, unsigned int mask,
}
}

DEFINE_FREE(snd_card_unref, struct snd_card *, if (_T) snd_card_unref(_T))

static int snd_ctl_led_set_id(int card_number, struct snd_ctl_elem_id *id,
unsigned int group, bool set)
{
struct snd_card *card;
struct snd_card *card __free(snd_card_unref) = NULL;
struct snd_kcontrol *kctl;
struct snd_kcontrol_volatile *vd;
unsigned int ioff, access, new_access;
int err = 0;

card = snd_card_ref(card_number);
if (card) {
down_write(&card->controls_rwsem);
kctl = snd_ctl_find_id_locked(card, id);
if (kctl) {
ioff = snd_ctl_get_ioff(kctl, id);
vd = &kctl->vd[ioff];
access = vd->access & SNDRV_CTL_ELEM_ACCESS_LED_MASK;
if (access != 0 && access != group_to_access(group)) {
err = -EXDEV;
goto unlock;
}
new_access = vd->access & ~SNDRV_CTL_ELEM_ACCESS_LED_MASK;
if (set)
new_access |= group_to_access(group);
if (new_access != vd->access) {
vd->access = new_access;
snd_ctl_led_notify(card, SNDRV_CTL_EVENT_MASK_INFO, kctl, ioff);
}
} else {
err = -ENOENT;
}
unlock:
up_write(&card->controls_rwsem);
snd_card_unref(card);
} else {
err = -ENXIO;
if (!card)
return -ENXIO;
guard(rwsem_write)(&card->controls_rwsem);
kctl = snd_ctl_find_id_locked(card, id);
if (!kctl)
return -ENOENT;
ioff = snd_ctl_get_ioff(kctl, id);
vd = &kctl->vd[ioff];
access = vd->access & SNDRV_CTL_ELEM_ACCESS_LED_MASK;
if (access != 0 && access != group_to_access(group))
return -EXDEV;
new_access = vd->access & ~SNDRV_CTL_ELEM_ACCESS_LED_MASK;
if (set)
new_access |= group_to_access(group);
if (new_access != vd->access) {
vd->access = new_access;
snd_ctl_led_notify(card, SNDRV_CTL_EVENT_MASK_INFO, kctl, ioff);
}
return err;
return 0;
}

static void snd_ctl_led_refresh(void)
Expand Down Expand Up @@ -312,7 +301,7 @@ static void snd_ctl_led_clean(struct snd_card *card)

static int snd_ctl_led_reset(int card_number, unsigned int group)
{
struct snd_card *card;
struct snd_card *card __free(snd_card_unref) = NULL;
struct snd_ctl_led *led;
struct snd_ctl_led_ctl *lctl;
struct snd_kcontrol_volatile *vd;
Expand All @@ -322,26 +311,22 @@ static int snd_ctl_led_reset(int card_number, unsigned int group)
if (!card)
return -ENXIO;

mutex_lock(&snd_ctl_led_mutex);
if (!snd_ctl_led_card_valid[card_number]) {
mutex_unlock(&snd_ctl_led_mutex);
snd_card_unref(card);
return -ENXIO;
}
led = &snd_ctl_leds[group];
scoped_guard(mutex, &snd_ctl_led_mutex) {
if (!snd_ctl_led_card_valid[card_number])
return -ENXIO;
led = &snd_ctl_leds[group];
repeat:
list_for_each_entry(lctl, &led->controls, list)
if (lctl->card == card) {
vd = &lctl->kctl->vd[lctl->index_offset];
vd->access &= ~group_to_access(group);
snd_ctl_led_ctl_destroy(lctl);
change = true;
goto repeat;
}
mutex_unlock(&snd_ctl_led_mutex);
list_for_each_entry(lctl, &led->controls, list)
if (lctl->card == card) {
vd = &lctl->kctl->vd[lctl->index_offset];
vd->access &= ~group_to_access(group);
snd_ctl_led_ctl_destroy(lctl);
change = true;
goto repeat;
}
}
if (change)
snd_ctl_led_set_state(NULL, group_to_access(group), NULL, 0);
snd_card_unref(card);
return 0;
}

Expand All @@ -353,9 +338,8 @@ static void snd_ctl_led_register(struct snd_card *card)
if (snd_BUG_ON(card->number < 0 ||
card->number >= ARRAY_SIZE(snd_ctl_led_card_valid)))
return;
mutex_lock(&snd_ctl_led_mutex);
snd_ctl_led_card_valid[card->number] = true;
mutex_unlock(&snd_ctl_led_mutex);
scoped_guard(mutex, &snd_ctl_led_mutex)
snd_ctl_led_card_valid[card->number] = true;
/* 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++)
Expand All @@ -367,10 +351,10 @@ static void snd_ctl_led_register(struct snd_card *card)
static void snd_ctl_led_disconnect(struct snd_card *card)
{
snd_ctl_led_sysfs_remove(card);
mutex_lock(&snd_ctl_led_mutex);
snd_ctl_led_card_valid[card->number] = false;
snd_ctl_led_clean(card);
mutex_unlock(&snd_ctl_led_mutex);
scoped_guard(mutex, &snd_ctl_led_mutex) {
snd_ctl_led_card_valid[card->number] = false;
snd_ctl_led_clean(card);
}
snd_ctl_led_refresh();
}

Expand Down Expand Up @@ -430,9 +414,8 @@ static ssize_t mode_store(struct device *dev,
else
return count;

mutex_lock(&snd_ctl_led_mutex);
led->mode = mode;
mutex_unlock(&snd_ctl_led_mutex);
scoped_guard(mutex, &snd_ctl_led_mutex)
led->mode = mode;

snd_ctl_led_set_state(NULL, group_to_access(led->group), NULL, 0);
return count;
Expand Down Expand Up @@ -615,15 +598,15 @@ static ssize_t list_show(struct device *dev,
struct device_attribute *attr, char *buf)
{
struct snd_ctl_led_card *led_card = container_of(dev, struct snd_ctl_led_card, dev);
struct snd_card *card;
struct snd_card *card __free(snd_card_unref) = NULL;
struct snd_ctl_led_ctl *lctl;
size_t l = 0;

card = snd_card_ref(led_card->number);
if (!card)
return -ENXIO;
down_read(&card->controls_rwsem);
mutex_lock(&snd_ctl_led_mutex);
guard(rwsem_read)(&card->controls_rwsem);
guard(mutex)(&snd_ctl_led_mutex);
if (snd_ctl_led_card_valid[led_card->number]) {
list_for_each_entry(lctl, &led_card->led->controls, list) {
if (lctl->card != card)
Expand All @@ -634,9 +617,6 @@ static ssize_t list_show(struct device *dev,
lctl->kctl->id.numid + lctl->index_offset);
}
}
mutex_unlock(&snd_ctl_led_mutex);
up_read(&card->controls_rwsem);
snd_card_unref(card);
return l;
}

Expand Down

0 comments on commit 7dba48a

Please sign in to comment.