[PATCH] ALSA: ymfpci: avoid changing control access under spinlock
From: Runyu Xiao
Date: Sun Sep 27 2026 - 09:53:35 EST
The YMFPCI playback trigger changes a mixer control's access flags while
holding reg_lock. Direct access changes are not serialized with the control
core, and snd_ctl_activate_id() cannot be called under the spinlock because
it may sleep.
Track the desired control state under reg_lock and update it from a work
item. Use a generation counter to serialize prepare with a pending stop.
Cancel the work before the PCM private data is released.
Assisted-by: LLM
Fixes: 177a7cdbd1d8 ("[ALSA] ymfpci: fix volume handling of the 44.1 kHz slot")
Signed-off-by: Runyu Xiao <runyu.xiao@xxxxxxxxxx>
---
sound/pci/ymfpci/ymfpci.h | 4 ++
sound/pci/ymfpci/ymfpci_main.c | 69 ++++++++++++++++++++++++++--------
2 files changed, 57 insertions(+), 16 deletions(-)
diff --git a/sound/pci/ymfpci/ymfpci.h b/sound/pci/ymfpci/ymfpci.h
index a408785cf..33c7d4327 100644
--- a/sound/pci/ymfpci/ymfpci.h
+++ b/sound/pci/ymfpci/ymfpci.h
@@ -12,6 +12,7 @@
#include <sound/ac97_codec.h>
#include <sound/timer.h>
#include <linux/gameport.h>
+#include <linux/workqueue.h>
/*
* Direct registers
@@ -260,6 +261,9 @@ struct snd_ymfpci_pcm {
output_rear: 1,
swap_rear: 1;
unsigned int update_pcm_vol;
+ struct work_struct control_work;
+ unsigned int control_generation;
+ bool control_active;
u32 period_size; /* cached from runtime->period_size */
u32 buffer_size; /* cached from runtime->buffer_size */
u32 period_pos;
diff --git a/sound/pci/ymfpci/ymfpci_main.c b/sound/pci/ymfpci/ymfpci_main.c
index 2ccb976e6..e5c0fb78e 100644
--- a/sound/pci/ymfpci/ymfpci_main.c
+++ b/sound/pci/ymfpci/ymfpci_main.c
@@ -355,12 +355,52 @@ static void snd_ymfpci_pcm_capture_interrupt(struct snd_pcm_substream *substream
}
}
+static void snd_ymfpci_pcm_control_work(struct work_struct *work)
+{
+ struct snd_ymfpci_pcm *ypcm = container_of(work, struct snd_ymfpci_pcm,
+ control_work);
+ struct snd_ymfpci *chip = ypcm->chip;
+ struct snd_kcontrol *kctl;
+ unsigned int generation;
+ bool active;
+
+ kctl = chip->pcm_mixer[ypcm->substream->number].ctl;
+ if (!kctl)
+ return;
+
+ for (;;) {
+ scoped_guard(spinlock_irq, &chip->reg_lock) {
+ active = ypcm->control_active;
+ generation = ypcm->control_generation;
+ }
+ snd_ctl_activate_id(chip->card, &kctl->id, active);
+ scoped_guard(spinlock_irq, &chip->reg_lock) {
+ if (generation == ypcm->control_generation)
+ return;
+ }
+ }
+}
+
+static void snd_ymfpci_pcm_set_control_active(struct snd_ymfpci_pcm *ypcm,
+ bool active)
+{
+ struct snd_ymfpci *chip = ypcm->chip;
+ struct snd_kcontrol *kctl;
+
+ scoped_guard(spinlock_irq, &chip->reg_lock) {
+ ypcm->control_active = active;
+ ypcm->control_generation++;
+ }
+ kctl = chip->pcm_mixer[ypcm->substream->number].ctl;
+ if (kctl)
+ snd_ctl_activate_id(chip->card, &kctl->id, active);
+}
+
static int snd_ymfpci_playback_trigger(struct snd_pcm_substream *substream,
int cmd)
{
struct snd_ymfpci *chip = snd_pcm_substream_chip(substream);
struct snd_ymfpci_pcm *ypcm = substream->runtime->private_data;
- struct snd_kcontrol *kctl = NULL;
int result = 0;
guard(spinlock)(&chip->reg_lock);
@@ -377,8 +417,9 @@ static int snd_ymfpci_playback_trigger(struct snd_pcm_substream *substream,
break;
case SNDRV_PCM_TRIGGER_STOP:
if (substream->pcm == chip->pcm && !ypcm->use_441_slot) {
- kctl = chip->pcm_mixer[substream->number].ctl;
- kctl->vd[0].access |= SNDRV_CTL_ELEM_ACCESS_INACTIVE;
+ ypcm->control_active = false;
+ ypcm->control_generation++;
+ schedule_work(&ypcm->control_work);
}
fallthrough;
case SNDRV_PCM_TRIGGER_PAUSE_PUSH:
@@ -391,8 +432,6 @@ static int snd_ymfpci_playback_trigger(struct snd_pcm_substream *substream,
default:
return -EINVAL;
}
- if (kctl)
- snd_ctl_notify(chip->card, SNDRV_CTL_EVENT_MASK_INFO, &kctl->id);
return result;
}
static int snd_ymfpci_capture_trigger(struct snd_pcm_substream *substream,
@@ -636,7 +675,6 @@ static int snd_ymfpci_playback_prepare(struct snd_pcm_substream *substream)
struct snd_ymfpci *chip = snd_pcm_substream_chip(substream);
struct snd_pcm_runtime *runtime = substream->runtime;
struct snd_ymfpci_pcm *ypcm = runtime->private_data;
- struct snd_kcontrol *kctl;
unsigned int nvoice;
ypcm->period_size = runtime->period_size;
@@ -648,9 +686,7 @@ static int snd_ymfpci_playback_prepare(struct snd_pcm_substream *substream)
substream->pcm == chip->pcm);
if (substream->pcm == chip->pcm && !ypcm->use_441_slot) {
- kctl = chip->pcm_mixer[substream->number].ctl;
- kctl->vd[0].access &= ~SNDRV_CTL_ELEM_ACCESS_INACTIVE;
- snd_ctl_notify(chip->card, SNDRV_CTL_EVENT_MASK_INFO, &kctl->id);
+ snd_ymfpci_pcm_set_control_active(ypcm, true);
}
return 0;
}
@@ -843,7 +879,10 @@ static const struct snd_pcm_hardware snd_ymfpci_capture =
static void snd_ymfpci_pcm_free_substream(struct snd_pcm_runtime *runtime)
{
- kfree(runtime->private_data);
+ struct snd_ymfpci_pcm *ypcm = runtime->private_data;
+
+ cancel_work_sync(&ypcm->control_work);
+ kfree(ypcm);
}
static int snd_ymfpci_playback_open_1(struct snd_pcm_substream *substream)
@@ -870,6 +909,7 @@ static int snd_ymfpci_playback_open_1(struct snd_pcm_substream *substream)
ypcm->chip = chip;
ypcm->type = PLAYBACK_VOICE;
ypcm->substream = substream;
+ INIT_WORK(&ypcm->control_work, snd_ymfpci_pcm_control_work);
runtime->private_data = ypcm;
runtime->private_free = snd_ymfpci_pcm_free_substream;
return 0;
@@ -945,9 +985,7 @@ static int snd_ymfpci_playback_spdif_open(struct snd_pcm_substream *substream)
chip->spdif_opened++;
}
- chip->spdif_pcm_ctl->vd[0].access &= ~SNDRV_CTL_ELEM_ACCESS_INACTIVE;
- snd_ctl_notify(chip->card, SNDRV_CTL_EVENT_MASK_VALUE |
- SNDRV_CTL_EVENT_MASK_INFO, &chip->spdif_pcm_ctl->id);
+ snd_ctl_activate_id(chip->card, &chip->spdif_pcm_ctl->id, 1);
return 0;
}
@@ -997,6 +1035,7 @@ static int snd_ymfpci_capture_open(struct snd_pcm_substream *substream,
ypcm->type = capture_bank_number + CAPTURE_REC;
ypcm->substream = substream;
ypcm->capture_bank_number = capture_bank_number;
+ INIT_WORK(&ypcm->control_work, snd_ymfpci_pcm_control_work);
chip->capture_substream[capture_bank_number] = substream;
runtime->private_data = ypcm;
runtime->private_free = snd_ymfpci_pcm_free_substream;
@@ -1044,9 +1083,7 @@ static int snd_ymfpci_playback_spdif_close(struct snd_pcm_substream *substream)
snd_ymfpci_readw(chip, YDSXGR_SPDIFOUTCTRL) & ~2);
snd_ymfpci_writew(chip, YDSXGR_SPDIFOUTSTATUS, chip->spdif_bits);
}
- chip->spdif_pcm_ctl->vd[0].access |= SNDRV_CTL_ELEM_ACCESS_INACTIVE;
- snd_ctl_notify(chip->card, SNDRV_CTL_EVENT_MASK_VALUE |
- SNDRV_CTL_EVENT_MASK_INFO, &chip->spdif_pcm_ctl->id);
+ snd_ctl_activate_id(chip->card, &chip->spdif_pcm_ctl->id, 0);
return snd_ymfpci_playback_close_1(substream);
}
--
2.34.1