[PATCH v2 0/3] ALSA: pcm: Retain components across codec teardown

[PATCH v2 0/3] ALSA: pcm: Retain components across codec teardown

[Cen Zhang]

A pending PCM opener can retain the card without retaining a PCM removed
by codec-only unbind. A late codec probe can also publish PCM devices
before a subsequent registration failure starts cleanup.

Retain native and OSS PCM lookups through file release, close admission
at component disconnect, and drain codec users before probe-error cleanup.
Keep the existing proc rundown outside open_mutex at disconnect, and wait
for PCM references in final cleanup after the lookup gate is closed.

---
Cen Zhang (3):
  ALSA: pcm: Retain components across native minor lookups
  ALSA: pcm: oss: Retain the PCM from minor lookup through release
  ALSA: hda: Drain PCM users before cleaning up a failed codec probe

 include/sound/pcm.h      | 11 ++++++
 sound/core/oss/pcm_oss.c | 20 +++++++----
 sound/core/pcm.c         | 74 ++++++++++++++++++++++++++++++++++++++--
 sound/core/pcm_native.c  | 44 ++++++++++++++++--------
 sound/hda/common/bind.c  |  3 ++
 5 files changed, 128 insertions(+), 24 deletions(-)

Changes in v2:
- Replace the setup-list-only fix with separate native PCM, OSS PCM and HDA
  probe-error lifetime fixes.
- Retain PCM references from minor lookup through file release and roll back
  partially registered minors.
- Rebase onto current for-next while preserving proc rundown outside
  open_mutex at device disconnect.
- Drain codec PCM users before probe-error cleanup removes driver state.
- Describe the native HDA codec sysfs-unbind and PCM cleanup sequence
  explicitly in the core patch.

Link to v1: https://lore.kernel.org/r/pm-pmb-a4703edca61d34aa679b-v6-680f350d78e44f9de7a7@gmail.com

[PATCH v2 1/3] ALSA: pcm: Retain components across native minor lookups

[Cen Zhang]

The card reference acquired by snd_lookup_minor_data() does not retain a
PCM component that its owner explicitly frees on a live card. A native
open can obtain the PCM and be delayed before substream attachment or
the driver open callback acquires its own usage reference.

For example, writing a codec name to its HDA driver's sysfs unbind
attribute disconnects and frees that codec's PCMs while the card remains
registered. The codec PCM usage count does not cover an opener that has
not yet entered azx_pcm_open():

  Native PCM open                       HDA codec unbind
  ---------------                       ----------------
  look up minor, retain card
  retain raw PCM pointer
                                        snd_hda_codec_disconnect_pcms()
                                          disconnect PCM
                                        wait for codec PCM references
                                        codec_release_pcms()
                                          snd_device_free()
                                            snd_pcm_free()
  use PCM in substream attachment

Acquire a PCM usage reference while still holding register_mutex across
minor lookup. Transfer it to the file on successful open and release it
after substream cleanup, before dropping the file's card reference.
Mappings also keep this reference through the file they retain. Balance
the temporary references used for PCM file checks and failed opens.

Keep proc rundown in the existing component disconnect path, after
open_mutex has been dropped. The PCM usage wait belongs in final free,
with no registration or open mutex held while users finish.

Close lookup admission and reject new substream attachments on component
disconnect. Final free closes admission even for BUILD-state cleanup,
wakes waiting openers and waits for existing PCM users without holding
register_mutex or open_mutex.

Keep registration under the same lookup gate and roll back earlier
direction minors if a later registration fails. Otherwise a failed
BUILD-state component can leave a minor pointing at freed storage.
Initialize PCM timers only after both direction registrations succeed,
so a failed registration leaves no partially initialized timers to
replace on retry.

Assisted-by: LLM
Signed-off-by: Cen Zhang <zzzccc427@gmail.com>
---
 include/sound/pcm.h     |  9 +++++++
 sound/core/pcm.c        | 59 ++++++++++++++++++++++++++++++++++++++---
 sound/core/pcm_native.c | 44 ++++++++++++++++++++----------
 3 files changed, 95 insertions(+), 17 deletions(-)

diff --git a/include/sound/pcm.h b/include/sound/pcm.h
index 1cf511f8d880..8fdb07f2f542 100644
--- a/include/sound/pcm.h
+++ b/include/sound/pcm.h
@@ -543,11 +543,13 @@ struct snd_pcm {
 	struct snd_pcm_str streams[2];
 	struct mutex open_mutex;
 	wait_queue_head_t open_wait;
+	struct snd_refcount users; /* minor lookups and open files */
 	void *private_data;
 	void (*private_free) (struct snd_pcm *pcm);
 	bool internal; /* pcm is for internal use only */
 	bool nonatomic; /* whole PCM operations are in non-atomic context */
 	bool no_device_suspend; /* don't invoke device PM suspend */
+	bool disconnected; /* no new substream may be attached */
 #if IS_ENABLED(CONFIG_SND_PCM_OSS)
 	struct snd_pcm_oss oss;
 #endif
@@ -559,6 +561,13 @@ struct snd_pcm {
 
 extern const struct file_operations snd_pcm_f_ops[2];
 
+struct snd_pcm *snd_pcm_get_by_minor(unsigned int minor, int type);
+
+static inline void snd_pcm_put(struct snd_pcm *pcm)
+{
+	snd_refcount_put(&pcm->users);
+}
+
 int snd_pcm_new(struct snd_card *card, const char *id, int device,
 		int playback_count, int capture_count,
 		struct snd_pcm **rpcm);
diff --git a/sound/core/pcm.c b/sound/core/pcm.c
index 064df4cd3b50..17cd60e8825d 100644
--- a/sound/core/pcm.c
+++ b/sound/core/pcm.c
@@ -725,6 +725,7 @@ static int _snd_pcm_new(struct snd_card *card, const char *id, int device,
 	pcm->internal = internal;
 	mutex_init(&pcm->open_mutex);
 	init_waitqueue_head(&pcm->open_wait);
+	snd_refcount_init(&pcm->users);
 	INIT_LIST_HEAD(&pcm->list);
 	if (id)
 		strscpy(pcm->id, id, sizeof(pcm->id));
@@ -862,6 +863,14 @@ static int snd_pcm_free(struct snd_pcm *pcm)
 	if (!pcm)
 		return 0;
 	if (!pcm->internal) {
+		/* Also close lookup admission for BUILD-state cleanup. */
+		scoped_guard(mutex, &register_mutex) {
+			scoped_guard(mutex, &pcm->open_mutex) {
+				pcm->disconnected = true;
+				wake_up(&pcm->open_wait);
+			}
+		}
+		snd_refcount_sync(&pcm->users);
 		scoped_guard(mutex, &register_mutex)
 			pcm_call_notify(pcm, n_unregister);
 	}
@@ -897,6 +906,8 @@ int snd_pcm_attach_substream(struct snd_pcm *pcm, int stream,
 		       stream != SNDRV_PCM_STREAM_CAPTURE))
 		return -EINVAL;
 	*rsubstream = NULL;
+	if (pcm->disconnected)
+		return -ENODEV;
 	pstr = &pcm->streams[stream];
 	if (pstr->substream == NULL || pstr->substream_count == 0)
 		return -ENODEV;
@@ -1015,6 +1026,44 @@ void snd_pcm_detach_substream(struct snd_pcm_substream *substream)
 	substream->pstr->substream_opened--;
 }
 
+static struct snd_pcm *pcm_get_from_minor(unsigned int minor, int type,
+					  void *(*lookup)(unsigned int, int))
+{
+	struct snd_pcm *pcm;
+	struct snd_card *card;
+
+	scoped_guard(mutex, &register_mutex) {
+		pcm = lookup(minor, type);
+		if (!pcm)
+			return NULL;
+		if (!pcm->disconnected) {
+			snd_refcount_get(&pcm->users);
+			return pcm;
+		}
+		card = pcm->card;
+	}
+	snd_card_unref(card);
+	return NULL;
+}
+
+/**
+ * snd_pcm_get_by_minor - look up and retain a native PCM component
+ * @minor: native ALSA minor number
+ * @type: playback or capture PCM device type
+ *
+ * The lookup holds the PCM registration mutex through reference acquisition.
+ * The caller must release both the PCM reference with snd_pcm_put() and the
+ * card reference with snd_card_unref(). An open file retains its PCM reference
+ * until release, including any mappings that keep the file alive.
+ *
+ * Return: A referenced PCM, or NULL if the minor is absent or disconnected.
+ */
+struct snd_pcm *snd_pcm_get_by_minor(unsigned int minor, int type)
+{
+	return pcm_get_from_minor(minor, type, snd_lookup_minor_data);
+}
+EXPORT_SYMBOL_GPL(snd_pcm_get_by_minor);
+
 static ssize_t pcm_class_show(struct device *dev,
 			      struct device_attribute *attr, char *buf)
 {
@@ -1081,14 +1130,17 @@ static int snd_pcm_dev_register(struct snd_device *device)
 					  &snd_pcm_f_ops[cidx], pcm,
 					  pcm->streams[cidx].dev);
 		if (err < 0) {
+			while (--cidx >= 0)
+				if (pcm->streams[cidx].substream)
+					snd_unregister_device(pcm->streams[cidx].dev);
 			list_del_init(&pcm->list);
 			return err;
 		}
-
-		for (substream = pcm->streams[cidx].substream; substream; substream = substream->next)
-			snd_pcm_timer_init(substream);
 	}
 
+	for_each_pcm_substream(pcm, cidx, substream)
+		snd_pcm_timer_init(substream);
+
 	pcm_call_notify(pcm, n_register);
 	return err;
 }
@@ -1101,6 +1153,7 @@ static int snd_pcm_dev_disconnect(struct snd_device *device)
 
 	guard(mutex)(&register_mutex);
 	scoped_guard(mutex, &pcm->open_mutex) {
+		pcm->disconnected = true;
 		wake_up(&pcm->open_wait);
 		list_del_init(&pcm->list);
 
diff --git a/sound/core/pcm_native.c b/sound/core/pcm_native.c
index 35152fae74ca..ba544fb4bc6c 100644
--- a/sound/core/pcm_native.c
+++ b/sound/core/pcm_native.c
@@ -2355,17 +2355,20 @@ static bool is_pcm_file(struct file *file)
 {
 	struct inode *inode = file_inode(file);
 	struct snd_pcm *pcm;
+	struct snd_card *card;
 	unsigned int minor;
 
 	if (!S_ISCHR(inode->i_mode) || imajor(inode) != snd_major)
 		return false;
 	minor = iminor(inode);
-	pcm = snd_lookup_minor_data(minor, SNDRV_DEVICE_TYPE_PCM_PLAYBACK);
+	pcm = snd_pcm_get_by_minor(minor, SNDRV_DEVICE_TYPE_PCM_PLAYBACK);
 	if (!pcm)
-		pcm = snd_lookup_minor_data(minor, SNDRV_DEVICE_TYPE_PCM_CAPTURE);
+		pcm = snd_pcm_get_by_minor(minor, SNDRV_DEVICE_TYPE_PCM_CAPTURE);
 	if (!pcm)
 		return false;
-	snd_card_unref(pcm->card);
+	card = pcm->card;
+	snd_pcm_put(pcm);
+	snd_card_unref(card);
 	return true;
 }
 
@@ -2942,11 +2945,16 @@ static int snd_pcm_playback_open(struct inode *inode, struct file *file)
 
 	nonseekable_open(inode, file);
 
-	pcm = snd_lookup_minor_data(iminor(inode),
-				    SNDRV_DEVICE_TYPE_PCM_PLAYBACK);
+	pcm = snd_pcm_get_by_minor(iminor(inode),
+				   SNDRV_DEVICE_TYPE_PCM_PLAYBACK);
 	err = snd_pcm_open(file, pcm, SNDRV_PCM_STREAM_PLAYBACK);
-	if (pcm)
-		snd_card_unref(pcm->card);
+	if (pcm) {
+		struct snd_card *card = pcm->card;
+
+		if (err < 0)
+			snd_pcm_put(pcm);
+		snd_card_unref(card);
+	}
 	return err;
 }
 
@@ -2957,11 +2965,16 @@ static int snd_pcm_capture_open(struct inode *inode, struct file *file)
 
 	nonseekable_open(inode, file);
 
-	pcm = snd_lookup_minor_data(iminor(inode),
-				    SNDRV_DEVICE_TYPE_PCM_CAPTURE);
+	pcm = snd_pcm_get_by_minor(iminor(inode),
+				   SNDRV_DEVICE_TYPE_PCM_CAPTURE);
 	err = snd_pcm_open(file, pcm, SNDRV_PCM_STREAM_CAPTURE);
-	if (pcm)
-		snd_card_unref(pcm->card);
+	if (pcm) {
+		struct snd_card *card = pcm->card;
+
+		if (err < 0)
+			snd_pcm_put(pcm);
+		snd_card_unref(card);
+	}
 	return err;
 }
 
@@ -2999,7 +3012,7 @@ static int snd_pcm_open(struct file *file, struct snd_pcm *pcm, int stream)
 		mutex_unlock(&pcm->open_mutex);
 		schedule();
 		mutex_lock(&pcm->open_mutex);
-		if (pcm->card->shutdown) {
+		if (pcm->card->shutdown || pcm->disconnected) {
 			err = -ENODEV;
 			break;
 		}
@@ -3025,6 +3038,7 @@ static int snd_pcm_open(struct file *file, struct snd_pcm *pcm, int stream)
 static int snd_pcm_release(struct inode *inode, struct file *file)
 {
 	struct snd_pcm *pcm;
+	struct snd_card *card;
 	struct snd_pcm_substream *substream;
 	struct snd_pcm_file *pcm_file;
 
@@ -3033,6 +3047,7 @@ static int snd_pcm_release(struct inode *inode, struct file *file)
 	if (snd_BUG_ON(!substream))
 		return -ENXIO;
 	pcm = substream->pcm;
+	card = pcm->card;
 
 	/* block until the device gets woken up as it may touch the hardware */
 	snd_power_wait(pcm->card);
@@ -3042,8 +3057,9 @@ static int snd_pcm_release(struct inode *inode, struct file *file)
 		kfree(pcm_file);
 	}
 	wake_up(&pcm->open_wait);
-	module_put(pcm->card->module);
-	snd_card_file_remove(pcm->card, file);
+	module_put(card->module);
+	snd_pcm_put(pcm);
+	snd_card_file_remove(card, file);
 	return 0;
 }
 
-- 
2.43.0

[PATCH v2 2/3] ALSA: pcm: oss: Retain the PCM from minor lookup through release

[Cen Zhang]

An OSS open uses the PCM before acquiring open_mutex and before the HDA
driver's open callback takes a codec PCM reference. In particular,
snd_pcm_oss_look_for_setup() walks the stream's setup list during this
interval. The lookup's card reference does not retain a PCM component
removed by codec-only unbind on a registered card:

  OSS open                             HDA codec unbind
  --------                             ----------------
  lookup OSS minor
  retain card reference
  snd_pcm_oss_look_for_setup()
                                       disconnect PCM
                                       wait for codec PCM references
                                       snd_device_free()
                                         snd_pcm_free()
  use setup list or PCM in open

The OSS module pin retains its code, and proc disconnect drains proc
callbacks; neither retains the PCM for this early OSS open.

Use the PCM core's lookup-and-retain helper for the OSS minor. Hold the
PCM reference through the early setup lookup and transfer it to the file
on success. Drop it on every open failure or after substream release,
before the last card reference can invoke final card destruction.

The final PCM free now waits for that early opener before removing OSS
setup lists. A pending opener which resumes after disconnect is rejected
before entering the driver, and a waiting OSS opener observes the PCM's
component disconnection as well as whole-card shutdown.

Assisted-by: LLM
Signed-off-by: Cen Zhang <zzzccc427@gmail.com>
---
 include/sound/pcm.h      |  2 ++
 sound/core/oss/pcm_oss.c | 20 +++++++++++++-------
 sound/core/pcm.c         | 15 +++++++++++++++
 3 files changed, 30 insertions(+), 7 deletions(-)

diff --git a/include/sound/pcm.h b/include/sound/pcm.h
index 8fdb07f2f542..12559c2144cf 100644
--- a/include/sound/pcm.h
+++ b/include/sound/pcm.h
@@ -577,6 +577,8 @@ int snd_pcm_new_internal(struct snd_card *card, const char *id, int device,
 int snd_pcm_new_stream(struct snd_pcm *pcm, int stream, int substream_count);
 
 #if IS_ENABLED(CONFIG_SND_PCM_OSS)
+struct snd_pcm *snd_pcm_get_by_oss_minor(unsigned int minor);
+
 struct snd_pcm_notify {
 	int (*n_register) (struct snd_pcm * pcm);
 	int (*n_disconnect) (struct snd_pcm * pcm);
diff --git a/sound/core/oss/pcm_oss.c b/sound/core/oss/pcm_oss.c
index e8df27d09b97..9f0dae051ecf 100644
--- a/sound/core/oss/pcm_oss.c
+++ b/sound/core/oss/pcm_oss.c
@@ -2497,8 +2497,7 @@ static int snd_pcm_oss_open(struct inode *inode, struct file *file)
 
 	nonseekable_open(inode, file);
 
-	pcm = snd_lookup_oss_minor_data(iminor(inode),
-					SNDRV_OSS_DEVICE_TYPE_PCM);
+	pcm = snd_pcm_get_by_oss_minor(iminor(inode));
 	if (pcm == NULL) {
 		err = -ENODEV;
 		goto __error1;
@@ -2545,7 +2544,7 @@ static int snd_pcm_oss_open(struct inode *inode, struct file *file)
 		mutex_unlock(&pcm->open_mutex);
 		schedule();
 		mutex_lock(&pcm->open_mutex);
-		if (pcm->card->shutdown) {
+		if (pcm->card->shutdown || pcm->disconnected) {
 			err = -ENODEV;
 			break;
 		}
@@ -2566,14 +2565,19 @@ static int snd_pcm_oss_open(struct inode *inode, struct file *file)
       __error2:
       	snd_card_file_remove(pcm->card, file);
       __error1:
-	if (pcm)
-		snd_card_unref(pcm->card);
+	if (pcm) {
+		struct snd_card *card = pcm->card;
+
+		snd_pcm_put(pcm);
+		snd_card_unref(card);
+	}
 	return err;
 }
 
 static int snd_pcm_oss_release(struct inode *inode, struct file *file)
 {
 	struct snd_pcm *pcm;
+	struct snd_card *card;
 	struct snd_pcm_substream *substream;
 	struct snd_pcm_oss_file *pcm_oss_file;
 
@@ -2584,14 +2588,16 @@ static int snd_pcm_oss_release(struct inode *inode, struct file *file)
 	if (snd_BUG_ON(!substream))
 		return -ENXIO;
 	pcm = substream->pcm;
+	card = pcm->card;
 	if (!pcm->card->shutdown)
 		snd_pcm_oss_sync(pcm_oss_file);
 	mutex_lock(&pcm->open_mutex);
 	snd_pcm_oss_release_file(pcm_oss_file);
 	mutex_unlock(&pcm->open_mutex);
 	wake_up(&pcm->open_wait);
-	module_put(pcm->card->module);
-	snd_card_file_remove(pcm->card, file);
+	module_put(card->module);
+	snd_pcm_put(pcm);
+	snd_card_file_remove(card, file);
 	return 0;
 }
 
diff --git a/sound/core/pcm.c b/sound/core/pcm.c
index 17cd60e8825d..380ce0781342 100644
--- a/sound/core/pcm.c
+++ b/sound/core/pcm.c
@@ -1064,6 +1064,21 @@ struct snd_pcm *snd_pcm_get_by_minor(unsigned int minor, int type)
 }
 EXPORT_SYMBOL_GPL(snd_pcm_get_by_minor);
 
+#if IS_ENABLED(CONFIG_SND_PCM_OSS)
+/**
+ * snd_pcm_get_by_oss_minor - look up and retain an OSS PCM component
+ * @minor: OSS minor number
+ *
+ * Returns the same PCM and card references as snd_pcm_get_by_minor().
+ */
+struct snd_pcm *snd_pcm_get_by_oss_minor(unsigned int minor)
+{
+	return pcm_get_from_minor(minor, SNDRV_OSS_DEVICE_TYPE_PCM,
+				  snd_lookup_oss_minor_data);
+}
+EXPORT_SYMBOL_GPL(snd_pcm_get_by_oss_minor);
+#endif
+
 static ssize_t pcm_class_show(struct device *dev,
 			      struct device_attribute *attr, char *buf)
 {
-- 
2.43.0

[PATCH v2 3/3] ALSA: hda: Drain PCM users before cleaning up a failed codec probe

[Cen Zhang]

A late codec probe on a registered card calls snd_card_register() after
building its PCMs. Some components can be registered and opened before
a later component registration fails. The error path then invokes the
driver's remove callback and frees the codec's PCM resources without
waiting for those users, unlike normal codec removal:

  Late HDA codec probe                  Native PCM user
  --------------------                  ---------------
  snd_card_register()
    register PCM A
                                        open PCM A
                                        driver takes codec PCM reference
    register PCM B fails
  driver->ops->remove()
  cleanup_for_unbind()
    codec_release_pcms()
                                        use PCM or enter driver close

Disconnect the codec's PCMs and wait for codec PCM references before
calling driver->ops->remove() on the error path. Disconnection drops
the construction references and blocks new driver opens; existing
users can finish while codec-specific state is still alive.

The PCM core's lookup reference and disconnection gate cover pending
openers which have not entered the driver. Waiting here additionally
covers users which already acquired the codec's own usage reference.
The later cleanup disconnection remains harmless because each PCM's
disconnected flag prevents dropping its construction reference twice.

Fixes: bcd96557bd0a ("ALSA: hda - Build PCMs and controls at codec driver probe")
Assisted-by: LLM
Signed-off-by: Cen Zhang <zzzccc427@gmail.com>
---
 sound/hda/common/bind.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/sound/hda/common/bind.c b/sound/hda/common/bind.c
index 957feace0f58..3c249d70ac1c 100644
--- a/sound/hda/common/bind.c
+++ b/sound/hda/common/bind.c
@@ -145,6 +145,9 @@ static int hda_codec_driver_probe(struct device *dev)
  error_module:
 	snd_hdac_device_disable_unsol(&codec->core);
 	cancel_delayed_work_sync(&codec->jackpoll_work);
+	/* A late probe can already have published working PCM components. */
+	snd_hda_codec_disconnect_pcms(codec);
+	snd_refcount_sync(&codec->pcm_ref);
 	if (probed && driver->ops->remove)
 		driver->ops->remove(codec);
  error_module_put:
-- 
2.43.0

Re: [PATCH v2 0/3] ALSA: pcm: Retain components across codec teardown

[Takashi Iwai]

On Sat, 24 Oct 2026 13:41:13 +0200,
Cen Zhang wrote:
> 
> A pending PCM opener can retain the card without retaining a PCM removed
> by codec-only unbind. A late codec probe can also publish PCM devices
> before a subsequent registration failure starts cleanup.
> 
> Retain native and OSS PCM lookups through file release, close admission
> at component disconnect, and drain codec users before probe-error cleanup.
> Keep the existing proc rundown outside open_mutex at disconnect, and wait
> for PCM references in final cleanup after the lookup gate is closed.
> 
> ---
> Cen Zhang (3):
>   ALSA: pcm: Retain components across native minor lookups
>   ALSA: pcm: oss: Retain the PCM from minor lookup through release
>   ALSA: hda: Drain PCM users before cleaning up a failed codec probe

I guess fixing in lower layer would become a whack-a-mole game.

My current thought is to rather plug on the upper layer, that is:

- Serialize concurrent probe and remove calls for the same HD-audio
  bus

- At remove, synchronize the all opened fds on the card; you can check
  list_empty(&card->files_list) and notify from snd_card_file_remove()
  call.

The proc files are removed and synced at disconnection, so the above
should cover most of corner cases, I suppose.


thanks,

Takashi