* [PATCH v4 1/3] ALSA: opl3: Initialize the timer before publishing a sequencer port
2026-10-24 16:49 [PATCH v4 0/3] ALSA: opl3: Fix sequencer timer and channel lifetimes Cen Zhang
@ 2026-10-24 16:49 ` Cen Zhang
2026-10-24 16:49 ` [PATCH v4 2/3] ALSA: opl3: Clear voice channels before freeing their sets Cen Zhang
` (2 subsequent siblings)
3 siblings, 0 replies; 5+ messages in thread
From: Cen Zhang @ 2026-10-24 16:49 UTC (permalink / raw)
To: perex, tiwai
Cc: linux-sound, linux-kernel, tiwai, baijiaju1990, jjzuming, zzzccc427
snd_opl3_seq_probe() initializes the delayed-note timer and its lock
after snd_opl3_synth_create_port() has made the native port reachable.
A direct NOTEON does not require a subscription, and a fixed-duration
instrument makes it enter snd_opl3_start_timer().
On sequencer-driver rebind, the card-owned instrument bank remains
available. An event delivered as soon as the port appears can race the
late initialization:
Sequencer driver probe Direct NOTEON
---------------------- -------------
snd_opl3_synth_create_port()
snd_seq_event_port_attach()
publish port
snd_opl3_synth_event_input()
snd_opl3_note_on()
snd_opl3_start_timer()
use tlist and sys_timer_lock
timer_setup(tlist)
spin_lock_init(sys_timer_lock)
sys_timer_status = 0
Initialize the timer, its lock and its status before creating the client
and attaching either port. No event can then reach partially initialized
timer state or have its newly armed timer reinitialized by the probe.
Assisted-by: LLM
Signed-off-by: Cen Zhang <zzzccc427@gmail.com>
---
sound/drivers/opl3/opl3_seq.c | 10 +++++-----
1 file changed, 5 insertions(+), 5 deletions(-)
diff --git a/sound/drivers/opl3/opl3_seq.c b/sound/drivers/opl3/opl3_seq.c
index b77d8e8f12fb..2bacefc3201d 100644
--- a/sound/drivers/opl3/opl3_seq.c
+++ b/sound/drivers/opl3/opl3_seq.c
@@ -214,6 +214,11 @@ static int snd_opl3_seq_probe(struct snd_seq_device *dev)
spin_lock_init(&opl3->voice_lock);
+ /* Initialize the timer before an attached port can receive events. */
+ timer_setup(&opl3->tlist, snd_opl3_timer_func, 0);
+ spin_lock_init(&opl3->sys_timer_lock);
+ opl3->sys_timer_status = 0;
+
opl3->seq_client = -1;
/* allocate new client */
@@ -232,11 +237,6 @@ static int snd_opl3_seq_probe(struct snd_seq_device *dev)
return err;
}
- /* setup system timer */
- timer_setup(&opl3->tlist, snd_opl3_timer_func, 0);
- spin_lock_init(&opl3->sys_timer_lock);
- opl3->sys_timer_status = 0;
-
#if IS_ENABLED(CONFIG_SND_SEQUENCER_OSS)
snd_opl3_init_seq_oss(opl3, name);
#endif
--
2.43.0
^ permalink raw reply [flat|nested] 5+ messages in thread* [PATCH v4 2/3] ALSA: opl3: Clear voice channels before freeing their sets
2026-10-24 16:49 [PATCH v4 0/3] ALSA: opl3: Fix sequencer timer and channel lifetimes Cen Zhang
2026-10-24 16:49 ` [PATCH v4 1/3] ALSA: opl3: Initialize the timer before publishing a sequencer port Cen Zhang
@ 2026-10-24 16:49 ` Cen Zhang
2026-10-24 16:49 ` [PATCH v4 3/3] ALSA: opl3: Shut down the timer after draining sequencer ports Cen Zhang
2026-10-26 8:01 ` [PATCH v4 0/3] ALSA: opl3: Fix sequencer timer and channel lifetimes Takashi Iwai
3 siblings, 0 replies; 5+ messages in thread
From: Cen Zhang @ 2026-10-24 16:49 UTC (permalink / raw)
To: perex, tiwai
Cc: linux-sound, linux-kernel, tiwai, baijiaju1990, jjzuming, zzzccc427
The card-owned OPL3 voice array survives sequencer-driver unbind, but
its chan pointers refer to the native or OSS port's channel set. Neither
port's private_free callback clears those pointers before freeing its set.
A direct NOTEON in the initial SYNTH mode can leave a voice allocated
without a subscription. Even with fix_dur zero and no timer, unbind and
rebind followed by direct PITCHBEND can read the old channel:
Sequencer driver unbind/rebind Direct PITCHBEND
----------------------------- ----------------
snd_seq_delete_kernel_client()
port_free()
free channel set
snd_opl3_seq_probe()
allocate new channel set
snd_opl3_pitch_ctrl()
snd_opl3_update_pitch()
old voice->chan is non-NULL
snd_opl3_calc_pitch()
read freed channel
Clearing only note_off_check or the voice state does not protect this
path: snd_opl3_update_pitch() tests the channel pointer itself.
Key off active voices before clearing their channel pointers, delayed-
note flags and state under voice_lock. Direct events need no
subscription, so port deletion need not call synth_unuse() to reset the
hardware. Merely marking a keyed-on voice off would retain hardware
KEYON and cause a later note to miss the key-on transition. Reuse
snd_opl3_kill_voice() before either channel set is freed.
Visit higher-numbered voices first. In initial SYNTH mode, direct
NOTEON events map channels to fixed voices without reserving them through
the allocator: a 4-op primary can coexist with an independently keyed-on
2-op partner at voice + 3. Killing the primary clears that partner's
software state without keying off its register. Ascending cleanup would
then skip the partner. Retiring the partner first guarantees its key-off
before the primary can change its state, on both register banks.
The lock also excludes a timer or pitch update already using a voice.
Both private_free callbacks must do the cleanup because an in-flight OSS
event can refill the shared voice array after the native port has been
drained and freed. That event can only reference its own still-live set;
the OSS callback's final cleanup retires that reference before freeing
its set as well.
Assisted-by: LLM
Signed-off-by: Cen Zhang <zzzccc427@gmail.com>
---
sound/drivers/opl3/opl3_midi.c | 15 +++++++++++++++
sound/drivers/opl3/opl3_oss.c | 1 +
sound/drivers/opl3/opl3_seq.c | 1 +
sound/drivers/opl3/opl3_voice.h | 1 +
4 files changed, 18 insertions(+)
diff --git a/sound/drivers/opl3/opl3_midi.c b/sound/drivers/opl3/opl3_midi.c
index 6d3c5b5a35ff..de611214cdac 100644
--- a/sound/drivers/opl3/opl3_midi.c
+++ b/sound/drivers/opl3/opl3_midi.c
@@ -632,6 +632,21 @@ static void snd_opl3_kill_voice(struct snd_opl3 *opl3, int voice)
}
+void snd_opl3_clear_voices(struct snd_opl3 *opl3)
+{
+ int i;
+
+ guard(spinlock_irqsave)(&opl3->voice_lock);
+ /* A 4-op primary clears its higher-numbered partner's state. */
+ for (i = MAX_OPL3_VOICES - 1; i >= 0; i--) {
+ if (opl3->voices[i].state > 0)
+ snd_opl3_kill_voice(opl3, i);
+ opl3->voices[i].chan = NULL;
+ opl3->voices[i].note_off_check = 0;
+ opl3->voices[i].state = SNDRV_OPL3_ST_OFF;
+ }
+}
+
/*
* Release a note in response to a midi note off.
*/
diff --git a/sound/drivers/opl3/opl3_oss.c b/sound/drivers/opl3/opl3_oss.c
index 6d39b2b77b80..1f01d27955fe 100644
--- a/sound/drivers/opl3/opl3_oss.c
+++ b/sound/drivers/opl3/opl3_oss.c
@@ -41,6 +41,7 @@ static void snd_opl3_oss_free_port(void *private_data)
{
struct snd_opl3 *opl3 = private_data;
+ snd_opl3_clear_voices(opl3);
snd_midi_channel_free_set(opl3->oss_chset);
}
diff --git a/sound/drivers/opl3/opl3_seq.c b/sound/drivers/opl3/opl3_seq.c
index 2bacefc3201d..e03c88377bb6 100644
--- a/sound/drivers/opl3/opl3_seq.c
+++ b/sound/drivers/opl3/opl3_seq.c
@@ -152,6 +152,7 @@ static void snd_opl3_synth_free_port(void *private_data)
{
struct snd_opl3 *opl3 = private_data;
+ snd_opl3_clear_voices(opl3);
snd_midi_channel_free_set(opl3->chset);
}
diff --git a/sound/drivers/opl3/opl3_voice.h b/sound/drivers/opl3/opl3_voice.h
index be9ccca2d952..aa81512d23c9 100644
--- a/sound/drivers/opl3/opl3_voice.h
+++ b/sound/drivers/opl3/opl3_voice.h
@@ -13,6 +13,7 @@ int snd_opl3_synth_use_inc(struct snd_opl3 * opl3);
void snd_opl3_synth_use_dec(struct snd_opl3 * opl3);
int snd_opl3_synth_setup(struct snd_opl3 * opl3);
void snd_opl3_synth_cleanup(struct snd_opl3 * opl3);
+void snd_opl3_clear_voices(struct snd_opl3 *opl3);
/* Prototypes for opl3_midi.c */
void snd_opl3_note_on(void *p, int note, int vel, struct snd_midi_channel *chan);
--
2.43.0
^ permalink raw reply [flat|nested] 5+ messages in thread* [PATCH v4 3/3] ALSA: opl3: Shut down the timer after draining sequencer ports
2026-10-24 16:49 [PATCH v4 0/3] ALSA: opl3: Fix sequencer timer and channel lifetimes Cen Zhang
2026-10-24 16:49 ` [PATCH v4 1/3] ALSA: opl3: Initialize the timer before publishing a sequencer port Cen Zhang
2026-10-24 16:49 ` [PATCH v4 2/3] ALSA: opl3: Clear voice channels before freeing their sets Cen Zhang
@ 2026-10-24 16:49 ` Cen Zhang
2026-10-26 8:01 ` [PATCH v4 0/3] ALSA: opl3: Fix sequencer timer and channel lifetimes Takashi Iwai
3 siblings, 0 replies; 5+ messages in thread
From: Cen Zhang @ 2026-10-24 16:49 UTC (permalink / raw)
To: perex, tiwai
Cc: linux-sound, linux-kernel, tiwai, baijiaju1990, jjzuming, zzzccc427
The delayed-note timer is independent of the sequencer ports' use-locks
and runs code in snd-opl3-synth. Deleting the client drains the event
callbacks but does not cancel this timer. A direct fixed-duration NOTEON
without a subscription can arm it without any unuse callback to stop it.
A shutdown in only the native port's private_free callback is too early
to finish shared voice cleanup when the OSS port is enabled:
Sequencer client deletion In-flight OSS NOTEON
------------------------- -------------------
drain native port
clear native voice aliases
snd_opl3_oss_event_input()
refill shared voice array
drain OSS port
clear voice aliases again
free OSS channel set
client deletion returns
The per-port voice cleanup prevents the timer from dereferencing a freed
channel during that sequence, but a queued or running timer can still
outlive client deletion and the sequencer driver itself.
Shut the timer down synchronously after snd_seq_delete_kernel_client()
has drained both ports. No event can then start it again, and shutdown
waits for a callback already running and prevents callback self-rearming.
Do this outside voice_lock and sys_timer_lock, which the callback takes.
The probe initializes timer state before publishing ports on rebind.
Assisted-by: LLM
Signed-off-by: Cen Zhang <zzzccc427@gmail.com>
---
sound/drivers/opl3/opl3_seq.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/sound/drivers/opl3/opl3_seq.c b/sound/drivers/opl3/opl3_seq.c
index e03c88377bb6..1004a7df1b8c 100644
--- a/sound/drivers/opl3/opl3_seq.c
+++ b/sound/drivers/opl3/opl3_seq.c
@@ -259,6 +259,8 @@ static void snd_opl3_seq_remove(struct snd_seq_device *dev)
snd_seq_delete_kernel_client(opl3->seq_client);
opl3->seq_client = -1;
}
+ /* Stop the shared timer after all event callbacks have been drained. */
+ timer_shutdown_sync(&opl3->tlist);
}
static struct snd_seq_driver opl3_seq_driver = {
--
2.43.0
^ permalink raw reply [flat|nested] 5+ messages in thread* Re: [PATCH v4 0/3] ALSA: opl3: Fix sequencer timer and channel lifetimes
2026-10-24 16:49 [PATCH v4 0/3] ALSA: opl3: Fix sequencer timer and channel lifetimes Cen Zhang
` (2 preceding siblings ...)
2026-10-24 16:49 ` [PATCH v4 3/3] ALSA: opl3: Shut down the timer after draining sequencer ports Cen Zhang
@ 2026-10-26 8:01 ` Takashi Iwai
3 siblings, 0 replies; 5+ messages in thread
From: Takashi Iwai @ 2026-10-26 8:01 UTC (permalink / raw)
To: Cen Zhang
Cc: perex, tiwai, linux-sound, linux-kernel, tiwai, baijiaju1990, jjzuming
On Sat, 24 Oct 2026 18:49:48 +0200,
Cen Zhang wrote:
>
> OPL3 can receive a fixed-duration note before its timer is initialized,
> and the card-owned voice array can retain channel pointers after either
> sequencer port has freed its channel set. The delayed-note timer can also
> remain queued after the sequencer client has been deleted.
>
> Initialize the timer before publishing ports, key off active voices and
> clear their aliases before freeing each channel set, and shut down the
> shared timer after client deletion has drained both event producers.
> Retire higher-numbered partners before their 4-op primaries so no active
> hardware KEYON is skipped when the primary changes its partner's state.
>
> This series applies to tiwai/sound.git for-next at d74eb09af56b.
>
> ---
> Cen Zhang (3):
> ALSA: opl3: Initialize the timer before publishing a sequencer port
> ALSA: opl3: Clear voice channels before freeing their sets
> ALSA: opl3: Shut down the timer after draining sequencer ports
Applied all three patches now. Thanks.
Takashi
^ permalink raw reply [flat|nested] 5+ messages in thread