git: a17a41ffde0d - main - sound: Use bus_child_deleted methods to free ivars for children

From: John Baldwin <jhb_at_FreeBSD.org>
Date: Fri, 01 Nov 2024 14:13:04 UTC
The branch main has been updated by jhb:

URL: https://cgit.FreeBSD.org/src/commit/?id=a17a41ffde0d1c579dcd7cef7900c3b44be0ec81

commit a17a41ffde0d1c579dcd7cef7900c3b44be0ec81
Author:     John Baldwin <jhb@FreeBSD.org>
AuthorDate: 2024-11-01 14:09:11 +0000
Commit:     John Baldwin <jhb@FreeBSD.org>
CommitDate: 2024-11-01 14:09:11 +0000

    sound: Use bus_child_deleted methods to free ivars for children
    
    Note that hdsp and hdspe were just leaking the ivars on detach
    previously.
    
    While here, use M_WAITOK to allocate ivars since attach routines are
    sleepable.  hdsp and hdspe were using M_NOWAIT without checking the
    return value.
    
    Reviewed by:    imp
    Differential Revision:  https://reviews.freebsd.org/D47366
---
 sys/dev/sound/pci/csa.c     |  26 +++------
 sys/dev/sound/pci/emu10kx.c | 134 ++++++++++----------------------------------
 sys/dev/sound/pci/hdsp.c    |   9 ++-
 sys/dev/sound/pci/hdspe.c   |   9 ++-
 4 files changed, 54 insertions(+), 124 deletions(-)

diff --git a/sys/dev/sound/pci/csa.c b/sys/dev/sound/pci/csa.c
index 6440b73f6341..d62759e22dcb 100644
--- a/sys/dev/sound/pci/csa.c
+++ b/sys/dev/sound/pci/csa.c
@@ -273,22 +273,14 @@ csa_attach(device_t dev)
 	/* Attach the children. */
 
 	/* PCM Audio */
-	func = malloc(sizeof(struct sndcard_func), M_DEVBUF, M_NOWAIT | M_ZERO);
-	if (func == NULL) {
-		error = ENOMEM;
-		goto err_teardown;
-	}
+	func = malloc(sizeof(struct sndcard_func), M_DEVBUF, M_WAITOK | M_ZERO);
 	func->varinfo = &scp->binfo;
 	func->func = SCF_PCM;
 	scp->pcm = device_add_child(dev, "pcm", DEVICE_UNIT_ANY);
 	device_set_ivars(scp->pcm, func);
 
 	/* Midi Interface */
-	func = malloc(sizeof(struct sndcard_func), M_DEVBUF, M_NOWAIT | M_ZERO);
-	if (func == NULL) {
-		error = ENOMEM;
-		goto err_teardown;
-	}
+	func = malloc(sizeof(struct sndcard_func), M_DEVBUF, M_WAITOK | M_ZERO);
 	func->varinfo = &scp->binfo;
 	func->func = SCF_MIDI;
 	scp->midi = device_add_child(dev, "midi", DEVICE_UNIT_ANY);
@@ -309,34 +301,33 @@ err_io:
 	return (error);
 }
 
+static void
+csa_child_deleted(device_t dev, device_t child)
+{
+	free(device_get_ivars(child), M_DEVBUF);
+}
+
 static int
 csa_detach(device_t dev)
 {
 	csa_res *resp;
 	sc_p scp;
-	struct sndcard_func *func;
 	int err;
 
 	scp = device_get_softc(dev);
 	resp = &scp->res;
 
 	if (scp->midi != NULL) {
-		func = device_get_ivars(scp->midi);
 		err = device_delete_child(dev, scp->midi);
 		if (err != 0)
 			return err;
-		if (func != NULL)
-			free(func, M_DEVBUF);
 		scp->midi = NULL;
 	}
 
 	if (scp->pcm != NULL) {
-		func = device_get_ivars(scp->pcm);
 		err = device_delete_child(dev, scp->pcm);
 		if (err != 0)
 			return err;
-		if (func != NULL)
-			free(func, M_DEVBUF);
 		scp->pcm = NULL;
 	}
 
@@ -1060,6 +1051,7 @@ static device_method_t csa_methods[] = {
 	DEVMETHOD(device_resume,	csa_resume),
 
 	/* Bus interface */
+	DEVMETHOD(bus_child_deleted,	csa_child_deleted),
 	DEVMETHOD(bus_alloc_resource,	csa_alloc_resource),
 	DEVMETHOD(bus_release_resource,	csa_release_resource),
 	DEVMETHOD(bus_activate_resource, bus_generic_activate_resource),
diff --git a/sys/dev/sound/pci/emu10kx.c b/sys/dev/sound/pci/emu10kx.c
index e642d334f9e8..b35a5e3139cb 100644
--- a/sys/dev/sound/pci/emu10kx.c
+++ b/sys/dev/sound/pci/emu10kx.c
@@ -3211,16 +3211,8 @@ emu_pci_attach(device_t dev)
 		sc->pcm[i] = NULL;
 
 	/* FRONT */
-	func = malloc(sizeof(struct sndcard_func), M_DEVBUF, M_NOWAIT | M_ZERO);
-	if (func == NULL) {
-		error = ENOMEM;
-		goto bad;
-	}
-	pcminfo = malloc(sizeof(struct emu_pcminfo), M_DEVBUF, M_NOWAIT | M_ZERO);
-	if (pcminfo == NULL) {
-		error = ENOMEM;
-		goto bad;
-	}
+	func = malloc(sizeof(struct sndcard_func), M_DEVBUF, M_WAITOK | M_ZERO);
+	pcminfo = malloc(sizeof(struct emu_pcminfo), M_DEVBUF, M_WAITOK | M_ZERO);
 	pcminfo->card = sc;
 	pcminfo->route = RT_FRONT;
 
@@ -3231,16 +3223,8 @@ emu_pci_attach(device_t dev)
 
 	if (!(sc->mch_disabled)) {
 		/* REAR */
-		func = malloc(sizeof(struct sndcard_func), M_DEVBUF, M_NOWAIT | M_ZERO);
-		if (func == NULL) {
-			error = ENOMEM;
-			goto bad;
-		}
-		pcminfo = malloc(sizeof(struct emu_pcminfo), M_DEVBUF, M_NOWAIT | M_ZERO);
-		if (pcminfo == NULL) {
-			error = ENOMEM;
-			goto bad;
-		}
+		func = malloc(sizeof(struct sndcard_func), M_DEVBUF, M_WAITOK | M_ZERO);
+		pcminfo = malloc(sizeof(struct emu_pcminfo), M_DEVBUF, M_WAITOK | M_ZERO);
 		pcminfo->card = sc;
 		pcminfo->route = RT_REAR;
 
@@ -3250,16 +3234,8 @@ emu_pci_attach(device_t dev)
 		device_set_ivars(sc->pcm[RT_REAR], func);
 		if (sc->has_51) {
 			/* CENTER */
-			func = malloc(sizeof(struct sndcard_func), M_DEVBUF, M_NOWAIT | M_ZERO);
-			if (func == NULL) {
-				error = ENOMEM;
-				goto bad;
-			}
-			pcminfo = malloc(sizeof(struct emu_pcminfo), M_DEVBUF, M_NOWAIT | M_ZERO);
-			if (pcminfo == NULL) {
-				error = ENOMEM;
-				goto bad;
-			}
+			func = malloc(sizeof(struct sndcard_func), M_DEVBUF, M_WAITOK | M_ZERO);
+			pcminfo = malloc(sizeof(struct emu_pcminfo), M_DEVBUF, M_WAITOK | M_ZERO);
 			pcminfo->card = sc;
 			pcminfo->route = RT_CENTER;
 
@@ -3268,16 +3244,8 @@ emu_pci_attach(device_t dev)
 			sc->pcm[RT_CENTER] = device_add_child(dev, "pcm", DEVICE_UNIT_ANY);
 			device_set_ivars(sc->pcm[RT_CENTER], func);
 			/* SUB */
-			func = malloc(sizeof(struct sndcard_func), M_DEVBUF, M_NOWAIT | M_ZERO);
-			if (func == NULL) {
-				error = ENOMEM;
-				goto bad;
-			}
-			pcminfo = malloc(sizeof(struct emu_pcminfo), M_DEVBUF, M_NOWAIT | M_ZERO);
-			if (pcminfo == NULL) {
-				error = ENOMEM;
-				goto bad;
-			}
+			func = malloc(sizeof(struct sndcard_func), M_DEVBUF, M_WAITOK | M_ZERO);
+			pcminfo = malloc(sizeof(struct emu_pcminfo), M_DEVBUF, M_WAITOK | M_ZERO);
 			pcminfo->card = sc;
 			pcminfo->route = RT_SUB;
 
@@ -3288,16 +3256,8 @@ emu_pci_attach(device_t dev)
 		}
 		if (sc->has_71) {
 			/* SIDE */
-			func = malloc(sizeof(struct sndcard_func), M_DEVBUF, M_NOWAIT | M_ZERO);
-			if (func == NULL) {
-				error = ENOMEM;
-				goto bad;
-			}
-			pcminfo = malloc(sizeof(struct emu_pcminfo), M_DEVBUF, M_NOWAIT | M_ZERO);
-			if (pcminfo == NULL) {
-				error = ENOMEM;
-				goto bad;
-			}
+			func = malloc(sizeof(struct sndcard_func), M_DEVBUF, M_WAITOK | M_ZERO);
+			pcminfo = malloc(sizeof(struct emu_pcminfo), M_DEVBUF, M_WAITOK | M_ZERO);
 			pcminfo->card = sc;
 			pcminfo->route = RT_SIDE;
 
@@ -3309,16 +3269,8 @@ emu_pci_attach(device_t dev)
 	} /* mch_disabled */
 
 	if (sc->mch_rec) {
-		func = malloc(sizeof(struct sndcard_func), M_DEVBUF, M_NOWAIT | M_ZERO);
-		if (func == NULL) {
-			error = ENOMEM;
-			goto bad;
-		}
-		pcminfo = malloc(sizeof(struct emu_pcminfo), M_DEVBUF, M_NOWAIT | M_ZERO);
-		if (pcminfo == NULL) {
-			error = ENOMEM;
-			goto bad;
-		}
+		func = malloc(sizeof(struct sndcard_func), M_DEVBUF, M_WAITOK | M_ZERO);
+		pcminfo = malloc(sizeof(struct emu_pcminfo), M_DEVBUF, M_WAITOK | M_ZERO);
 		pcminfo->card = sc;
 		pcminfo->route = RT_MCHRECORD;
 
@@ -3335,16 +3287,8 @@ emu_pci_attach(device_t dev)
 #if 0
 	/* Midi Interface 1: Live!, Audigy, Audigy 2 */
 	if ((sc->is_emu10k1) || (sc->is_emu10k2) || (sc->is_ca0102)) {
-		func = malloc(sizeof(struct sndcard_func), M_DEVBUF, M_NOWAIT | M_ZERO);
-		if (func == NULL) {
-			error = ENOMEM;
-			goto bad;
-		}
-		midiinfo = malloc(sizeof(struct emu_midiinfo), M_DEVBUF, M_NOWAIT | M_ZERO);
-		if (midiinfo == NULL) {
-			error = ENOMEM;
-			goto bad;
-		}
+		func = malloc(sizeof(struct sndcard_func), M_DEVBUF, M_WAITOK | M_ZERO);
+		midiinfo = malloc(sizeof(struct emu_midiinfo), M_DEVBUF, M_WAITOK | M_ZERO);
 		midiinfo->card = sc;
 		if (sc->is_emu10k2 || (sc->is_ca0102)) {
 			midiinfo->port = EMU_A_MUDATA1;
@@ -3361,16 +3305,8 @@ emu_pci_attach(device_t dev)
 	}
 	/* Midi Interface 2: Audigy, Audigy 2 (on AudigyDrive) */
 	if (sc->is_emu10k2 || (sc->is_ca0102)) {
-		func = malloc(sizeof(struct sndcard_func), M_DEVBUF, M_NOWAIT | M_ZERO);
-		if (func == NULL) {
-			error = ENOMEM;
-			goto bad;
-		}
-		midiinfo = malloc(sizeof(struct emu_midiinfo), M_DEVBUF, M_NOWAIT | M_ZERO);
-		if (midiinfo == NULL) {
-			error = ENOMEM;
-			goto bad;
-		}
+		func = malloc(sizeof(struct sndcard_func), M_DEVBUF, M_WAITOK | M_ZERO);
+		midiinfo = malloc(sizeof(struct emu_midiinfo), M_DEVBUF, M_WAITOK | M_ZERO);
 		midiinfo->card = sc;
 
 		midiinfo->port = EMU_A_MUDATA2;
@@ -3401,11 +3337,22 @@ bad:
 	return (error);
 }
 
+static void
+emu_pci_child_deleted(device_t dev, device_t child)
+{
+	struct sndcard_func *func;
+
+	func = device_get_ivars(child);
+	if (func != NULL) {
+		free(func->varinfo, M_DEVBUF);
+		free(func, M_DEVBUF);
+	}
+}
+
 static int
 emu_pci_detach(device_t dev)
 {
 	struct emu_sc_info *sc;
-	struct sndcard_func *func;
 	int devcount, i;
 	device_t *childlist;
 	int r = 0;
@@ -3414,35 +3361,17 @@ emu_pci_detach(device_t dev)
 
 	for (i = 0; i < RT_COUNT; i++) {
 		if (sc->pcm[i] != NULL) {
-			func = device_get_ivars(sc->pcm[i]);
-			if (func != NULL && func->func == SCF_PCM) {
-				device_set_ivars(sc->pcm[i], NULL);
-				free(func->varinfo, M_DEVBUF);
-				free(func, M_DEVBUF);
-			}
 			r = device_delete_child(dev, sc->pcm[i]);
 			if (r)	return (r);
 		}
 	}
 
 	if (sc->midi[0] != NULL) {
-		func = device_get_ivars(sc->midi[0]);
-		if (func != NULL && func->func == SCF_MIDI) {
-			device_set_ivars(sc->midi[0], NULL);
-			free(func->varinfo, M_DEVBUF);
-			free(func, M_DEVBUF);
-		}
 		r = device_delete_child(dev, sc->midi[0]);
 		if (r)	return (r);
 	}
 
 	if (sc->midi[1] != NULL) {
-		func = device_get_ivars(sc->midi[1]);
-		if (func != NULL && func->func == SCF_MIDI) {
-			device_set_ivars(sc->midi[1], NULL);
-			free(func->varinfo, M_DEVBUF);
-			free(func, M_DEVBUF);
-		}
 		r = device_delete_child(dev, sc->midi[1]);
 		if (r)	return (r);
 	}
@@ -3450,12 +3379,6 @@ emu_pci_detach(device_t dev)
 	if (device_get_children(dev, &childlist, &devcount) == 0)
 		for (i = 0; i < devcount - 1; i++) {
 			device_printf(dev, "removing stale child %d (unit %d)\n", i, device_get_unit(childlist[i]));
-			func = device_get_ivars(childlist[i]);
-			if (func != NULL && (func->func == SCF_MIDI || func->func == SCF_PCM)) {
-				device_set_ivars(childlist[i], NULL);
-				free(func->varinfo, M_DEVBUF);
-				free(func, M_DEVBUF);
-			}
 			device_delete_child(dev, childlist[i]);
 		}
 	if (childlist != NULL)
@@ -3488,6 +3411,7 @@ static device_method_t emu_methods[] = {
 	DEVMETHOD(device_attach, emu_pci_attach),
 	DEVMETHOD(device_detach, emu_pci_detach),
 	/* Bus methods */
+	DEVMETHOD(bus_child_deleted, emu_pci_child_deleted),
 	DEVMETHOD(bus_read_ivar, emu_read_ivar),
 	DEVMETHOD(bus_write_ivar, emu_write_ivar),
 
diff --git a/sys/dev/sound/pci/hdsp.c b/sys/dev/sound/pci/hdsp.c
index 03e885df63aa..d678fed527dd 100644
--- a/sys/dev/sound/pci/hdsp.c
+++ b/sys/dev/sound/pci/hdsp.c
@@ -887,7 +887,7 @@ hdsp_attach(device_t dev)
 		return (ENXIO);
 
 	for (i = 0; i < HDSP_MAX_CHANS && chan_map[i].descr != NULL; i++) {
-		scp = malloc(sizeof(struct sc_pcminfo), M_DEVBUF, M_NOWAIT | M_ZERO);
+		scp = malloc(sizeof(struct sc_pcminfo), M_DEVBUF, M_WAITOK | M_ZERO);
 		scp->hc = &chan_map[i];
 		scp->sc = sc;
 		scp->dev = device_add_child(dev, "pcm", -1);
@@ -955,6 +955,12 @@ hdsp_attach(device_t dev)
 	return (bus_generic_attach(dev));
 }
 
+static void
+hdsp_child_deleted(device_t dev, device_t child)
+{
+	free(device_get_ivars(child), M_DEVBUF);
+}
+
 static void
 hdsp_dmafree(struct sc_info *sc)
 {
@@ -1002,6 +1008,7 @@ static device_method_t hdsp_methods[] = {
 	DEVMETHOD(device_probe,     hdsp_probe),
 	DEVMETHOD(device_attach,    hdsp_attach),
 	DEVMETHOD(device_detach,    hdsp_detach),
+	DEVMETHOD(bus_child_deleted, hdsp_child_deleted),
 	{ 0, 0 }
 };
 
diff --git a/sys/dev/sound/pci/hdspe.c b/sys/dev/sound/pci/hdspe.c
index 0e25e9599328..2561fcdebb1c 100644
--- a/sys/dev/sound/pci/hdspe.c
+++ b/sys/dev/sound/pci/hdspe.c
@@ -578,7 +578,7 @@ hdspe_attach(device_t dev)
 		return (ENXIO);
 
 	for (i = 0; i < HDSPE_MAX_CHANS && chan_map[i].descr != NULL; i++) {
-		scp = malloc(sizeof(struct sc_pcminfo), M_DEVBUF, M_NOWAIT | M_ZERO);
+		scp = malloc(sizeof(struct sc_pcminfo), M_DEVBUF, M_WAITOK | M_ZERO);
 		scp->hc = &chan_map[i];
 		scp->sc = sc;
 		scp->dev = device_add_child(dev, "pcm", DEVICE_UNIT_ANY);
@@ -626,6 +626,12 @@ hdspe_attach(device_t dev)
 	return (bus_generic_attach(dev));
 }
 
+static void
+hdspe_child_deleted(device_t dev, device_t child)
+{
+	free(device_get_ivars(child), M_DEVBUF);
+}
+
 static void
 hdspe_dmafree(struct sc_info *sc)
 {
@@ -673,6 +679,7 @@ static device_method_t hdspe_methods[] = {
 	DEVMETHOD(device_probe,     hdspe_probe),
 	DEVMETHOD(device_attach,    hdspe_attach),
 	DEVMETHOD(device_detach,    hdspe_detach),
+	DEVMETHOD(bus_child_deleted, hdspe_child_deleted),
 	{ 0, 0 }
 };