Linux Sound subsystem development
 help / color / mirror / Atom feed
From: Takashi Iwai <tiwai@suse.de>
To: linux-sound@vger.kernel.org
Subject: [PATCH 09/11] ALSA: seq: oss/midi: Cleanup with guard and auto-cleanup
Date: Mon, 11 Aug 2025 12:32:45 +0200	[thread overview]
Message-ID: <20250811103249.10644-10-tiwai@suse.de> (raw)
In-Reply-To: <20250811103249.10644-1-tiwai@suse.de>

Use the auto-cleanup for the refcount management of seq_oss_midi
object.  The explicit call of snd_use_lock_free() is dropped by the
magic __free(seq_oss_midi) attribute.

Along with that, replace the manual mutex and spin locks with
guard().

Only code refactoring, and no behavior change.

Signed-off-by: Takashi Iwai <tiwai@suse.de>
---
 sound/core/seq/oss/seq_oss_midi.c | 125 ++++++++++--------------------
 1 file changed, 40 insertions(+), 85 deletions(-)

diff --git a/sound/core/seq/oss/seq_oss_midi.c b/sound/core/seq/oss/seq_oss_midi.c
index f8e247d9e5c9..6d6a1a0dc6c0 100644
--- a/sound/core/seq/oss/seq_oss_midi.c
+++ b/sound/core/seq/oss/seq_oss_midi.c
@@ -40,6 +40,7 @@ struct seq_oss_midi {
 	struct mutex open_mutex;
 };
 
+DEFINE_FREE(seq_oss_midi, struct seq_oss_midi *, if (!IS_ERR_OR_NULL(_T)) snd_use_lock_free(&(_T)->use_lock))
 
 /*
  * midi device table
@@ -90,13 +91,11 @@ static struct seq_oss_midi *
 get_mdev(int dev)
 {
 	struct seq_oss_midi *mdev;
-	unsigned long flags;
 
-	spin_lock_irqsave(&register_lock, flags);
+	guard(spinlock_irqsave)(&register_lock);
 	mdev = midi_devs[dev];
 	if (mdev)
 		snd_use_lock_use(&mdev->use_lock);
-	spin_unlock_irqrestore(&register_lock, flags);
 	return mdev;
 }
 
@@ -108,19 +107,16 @@ find_slot(int client, int port)
 {
 	int i;
 	struct seq_oss_midi *mdev;
-	unsigned long flags;
 
-	spin_lock_irqsave(&register_lock, flags);
+	guard(spinlock_irqsave)(&register_lock);
 	for (i = 0; i < max_midi_devs; i++) {
 		mdev = midi_devs[i];
 		if (mdev && mdev->client == client && mdev->port == port) {
 			/* found! */
 			snd_use_lock_use(&mdev->use_lock);
-			spin_unlock_irqrestore(&register_lock, flags);
 			return mdev;
 		}
 	}
-	spin_unlock_irqrestore(&register_lock, flags);
 	return NULL;
 }
 
@@ -134,8 +130,7 @@ int
 snd_seq_oss_midi_check_new_port(struct snd_seq_port_info *pinfo)
 {
 	int i;
-	struct seq_oss_midi *mdev;
-	unsigned long flags;
+	struct seq_oss_midi *mdev __free(seq_oss_midi) = NULL;
 
 	/* the port must include generic midi */
 	if (! (pinfo->type & SNDRV_SEQ_PORT_TYPE_MIDI_GENERIC))
@@ -151,7 +146,6 @@ snd_seq_oss_midi_check_new_port(struct snd_seq_port_info *pinfo)
 	mdev = find_slot(pinfo->addr.client, pinfo->addr.port);
 	if (mdev) {
 		/* already exists */
-		snd_use_lock_free(&mdev->use_lock);
 		return 0;
 	}
 
@@ -176,7 +170,7 @@ snd_seq_oss_midi_check_new_port(struct snd_seq_port_info *pinfo)
 	/* create MIDI coder */
 	if (snd_midi_event_new(MAX_MIDI_EVENT_BUF, &mdev->coder) < 0) {
 		pr_err("ALSA: seq_oss: can't malloc midi coder\n");
-		kfree(mdev);
+		kfree(no_free_ptr(mdev));
 		return -ENOMEM;
 	}
 	/* OSS sequencer adds running status to all sequences */
@@ -185,23 +179,21 @@ snd_seq_oss_midi_check_new_port(struct snd_seq_port_info *pinfo)
 	/*
 	 * look for en empty slot
 	 */
-	spin_lock_irqsave(&register_lock, flags);
+	guard(spinlock_irqsave)(&register_lock);
 	for (i = 0; i < max_midi_devs; i++) {
 		if (midi_devs[i] == NULL)
 			break;
 	}
 	if (i >= max_midi_devs) {
 		if (max_midi_devs >= SNDRV_SEQ_OSS_MAX_MIDI_DEVS) {
-			spin_unlock_irqrestore(&register_lock, flags);
 			snd_midi_event_free(mdev->coder);
-			kfree(mdev);
+			kfree(no_free_ptr(mdev));
 			return -ENOMEM;
 		}
 		max_midi_devs++;
 	}
 	mdev->seq_device = i;
-	midi_devs[mdev->seq_device] = mdev;
-	spin_unlock_irqrestore(&register_lock, flags);
+	midi_devs[mdev->seq_device] = no_free_ptr(mdev);
 
 	return 0;
 }
@@ -213,26 +205,24 @@ int
 snd_seq_oss_midi_check_exit_port(int client, int port)
 {
 	struct seq_oss_midi *mdev;
-	unsigned long flags;
 	int index;
 
 	mdev = find_slot(client, port);
 	if (mdev) {
-		spin_lock_irqsave(&register_lock, flags);
-		midi_devs[mdev->seq_device] = NULL;
-		spin_unlock_irqrestore(&register_lock, flags);
+		scoped_guard(spinlock_irqsave, &register_lock) {
+			midi_devs[mdev->seq_device] = NULL;
+		}
 		snd_use_lock_free(&mdev->use_lock);
 		snd_use_lock_sync(&mdev->use_lock);
 		snd_midi_event_free(mdev->coder);
 		kfree(mdev);
 	}
-	spin_lock_irqsave(&register_lock, flags);
+	guard(spinlock_irqsave)(&register_lock);
 	for (index = max_midi_devs - 1; index >= 0; index--) {
 		if (midi_devs[index])
 			break;
 	}
 	max_midi_devs = index + 1;
-	spin_unlock_irqrestore(&register_lock, flags);
 	return 0;
 }
 
@@ -245,9 +235,8 @@ snd_seq_oss_midi_clear_all(void)
 {
 	int i;
 	struct seq_oss_midi *mdev;
-	unsigned long flags;
 
-	spin_lock_irqsave(&register_lock, flags);
+	guard(spinlock_irqsave)(&register_lock);
 	for (i = 0; i < max_midi_devs; i++) {
 		mdev = midi_devs[i];
 		if (mdev) {
@@ -257,7 +246,6 @@ snd_seq_oss_midi_clear_all(void)
 		}
 	}
 	max_midi_devs = 0;
-	spin_unlock_irqrestore(&register_lock, flags);
 }
 
 
@@ -267,9 +255,8 @@ snd_seq_oss_midi_clear_all(void)
 void
 snd_seq_oss_midi_setup(struct seq_oss_devinfo *dp)
 {
-	spin_lock_irq(&register_lock);
+	guard(spinlock_irq)(&register_lock);
 	dp->max_mididev = max_midi_devs;
-	spin_unlock_irq(&register_lock);
 }
 
 /*
@@ -317,20 +304,17 @@ int
 snd_seq_oss_midi_open(struct seq_oss_devinfo *dp, int dev, int fmode)
 {
 	int perm;
-	struct seq_oss_midi *mdev;
+	struct seq_oss_midi *mdev __free(seq_oss_midi) = NULL;
 	struct snd_seq_port_subscribe subs;
-	int err;
 
 	mdev = get_mididev(dp, dev);
 	if (!mdev)
 		return -ENODEV;
 
-	mutex_lock(&mdev->open_mutex);
+	guard(mutex)(&mdev->open_mutex);
 	/* already used? */
-	if (mdev->opened && mdev->devinfo != dp) {
-		err = -EBUSY;
-		goto unlock;
-	}
+	if (mdev->opened && mdev->devinfo != dp)
+		return -EBUSY;
 
 	perm = 0;
 	if (is_write_mode(fmode))
@@ -338,16 +322,12 @@ snd_seq_oss_midi_open(struct seq_oss_devinfo *dp, int dev, int fmode)
 	if (is_read_mode(fmode))
 		perm |= PERM_READ;
 	perm &= mdev->flags;
-	if (perm == 0) {
-		err = -ENXIO;
-		goto unlock;
-	}
+	if (perm == 0)
+		return -ENXIO;
 
 	/* already opened? */
-	if ((mdev->opened & perm) == perm) {
-		err = 0;
-		goto unlock;
-	}
+	if ((mdev->opened & perm) == perm)
+		return 0;
 
 	perm &= ~mdev->opened;
 
@@ -370,18 +350,11 @@ snd_seq_oss_midi_open(struct seq_oss_devinfo *dp, int dev, int fmode)
 			mdev->opened |= PERM_READ;
 	}
 
-	if (! mdev->opened) {
-		err = -ENXIO;
-		goto unlock;
-	}
+	if (!mdev->opened)
+		return -ENXIO;
 
 	mdev->devinfo = dp;
-	err = 0;
-
- unlock:
-	mutex_unlock(&mdev->open_mutex);
-	snd_use_lock_free(&mdev->use_lock);
-	return err;
+	return 0;
 }
 
 /*
@@ -390,15 +363,15 @@ snd_seq_oss_midi_open(struct seq_oss_devinfo *dp, int dev, int fmode)
 int
 snd_seq_oss_midi_close(struct seq_oss_devinfo *dp, int dev)
 {
-	struct seq_oss_midi *mdev;
+	struct seq_oss_midi *mdev __free(seq_oss_midi) = NULL;
 	struct snd_seq_port_subscribe subs;
 
 	mdev = get_mididev(dp, dev);
 	if (!mdev)
 		return -ENODEV;
-	mutex_lock(&mdev->open_mutex);
+	guard(mutex)(&mdev->open_mutex);
 	if (!mdev->opened || mdev->devinfo != dp)
-		goto unlock;
+		return 0;
 
 	memset(&subs, 0, sizeof(subs));
 	if (mdev->opened & PERM_WRITE) {
@@ -416,10 +389,6 @@ snd_seq_oss_midi_close(struct seq_oss_devinfo *dp, int dev)
 
 	mdev->opened = 0;
 	mdev->devinfo = NULL;
-
- unlock:
-	mutex_unlock(&mdev->open_mutex);
-	snd_use_lock_free(&mdev->use_lock);
 	return 0;
 }
 
@@ -429,7 +398,7 @@ snd_seq_oss_midi_close(struct seq_oss_devinfo *dp, int dev)
 int
 snd_seq_oss_midi_filemode(struct seq_oss_devinfo *dp, int dev)
 {
-	struct seq_oss_midi *mdev;
+	struct seq_oss_midi *mdev __free(seq_oss_midi) = NULL;
 	int mode;
 
 	mdev = get_mididev(dp, dev);
@@ -442,7 +411,6 @@ snd_seq_oss_midi_filemode(struct seq_oss_devinfo *dp, int dev)
 	if (mdev->opened & PERM_READ)
 		mode |= SNDRV_SEQ_OSS_FILE_READ;
 
-	snd_use_lock_free(&mdev->use_lock);
 	return mode;
 }
 
@@ -453,15 +421,13 @@ snd_seq_oss_midi_filemode(struct seq_oss_devinfo *dp, int dev)
 void
 snd_seq_oss_midi_reset(struct seq_oss_devinfo *dp, int dev)
 {
-	struct seq_oss_midi *mdev;
+	struct seq_oss_midi *mdev __free(seq_oss_midi) = NULL;
 
 	mdev = get_mididev(dp, dev);
 	if (!mdev)
 		return;
-	if (! mdev->opened) {
-		snd_use_lock_free(&mdev->use_lock);
+	if (!mdev->opened)
 		return;
-	}
 
 	if (mdev->opened & PERM_WRITE) {
 		struct snd_seq_event ev;
@@ -492,7 +458,6 @@ snd_seq_oss_midi_reset(struct seq_oss_devinfo *dp, int dev)
 		}
 	}
 	// snd_seq_oss_midi_close(dp, dev);
-	snd_use_lock_free(&mdev->use_lock);
 }
 
 
@@ -502,14 +467,13 @@ snd_seq_oss_midi_reset(struct seq_oss_devinfo *dp, int dev)
 void
 snd_seq_oss_midi_get_addr(struct seq_oss_devinfo *dp, int dev, struct snd_seq_addr *addr)
 {
-	struct seq_oss_midi *mdev;
+	struct seq_oss_midi *mdev __free(seq_oss_midi) = NULL;
 
 	mdev = get_mididev(dp, dev);
 	if (!mdev)
 		return;
 	addr->client = mdev->client;
 	addr->port = mdev->port;
-	snd_use_lock_free(&mdev->use_lock);
 }
 
 
@@ -520,26 +484,20 @@ int
 snd_seq_oss_midi_input(struct snd_seq_event *ev, int direct, void *private_data)
 {
 	struct seq_oss_devinfo *dp = (struct seq_oss_devinfo *)private_data;
-	struct seq_oss_midi *mdev;
-	int rc;
+	struct seq_oss_midi *mdev __free(seq_oss_midi) = NULL;
 
 	if (dp->readq == NULL)
 		return 0;
 	mdev = find_slot(ev->source.client, ev->source.port);
 	if (!mdev)
 		return 0;
-	if (! (mdev->opened & PERM_READ)) {
-		snd_use_lock_free(&mdev->use_lock);
+	if (!(mdev->opened & PERM_READ))
 		return 0;
-	}
 
 	if (dp->seq_mode == SNDRV_SEQ_OSS_MODE_MUSIC)
-		rc = send_synth_event(dp, ev, mdev->seq_device);
+		return send_synth_event(dp, ev, mdev->seq_device);
 	else
-		rc = send_midi_event(dp, ev, mdev);
-
-	snd_use_lock_free(&mdev->use_lock);
-	return rc;
+		return send_midi_event(dp, ev, mdev);
 }
 
 /*
@@ -636,17 +594,15 @@ send_midi_event(struct seq_oss_devinfo *dp, struct snd_seq_event *ev, struct seq
 int
 snd_seq_oss_midi_putc(struct seq_oss_devinfo *dp, int dev, unsigned char c, struct snd_seq_event *ev)
 {
-	struct seq_oss_midi *mdev;
+	struct seq_oss_midi *mdev __free(seq_oss_midi) = NULL;
 
 	mdev = get_mididev(dp, dev);
 	if (!mdev)
 		return -ENODEV;
 	if (snd_midi_event_encode_byte(mdev->coder, c, ev)) {
 		snd_seq_oss_fill_addr(dp, ev, mdev->client, mdev->port);
-		snd_use_lock_free(&mdev->use_lock);
 		return 0;
 	}
-	snd_use_lock_free(&mdev->use_lock);
 	return -EINVAL;
 }
 
@@ -656,7 +612,7 @@ snd_seq_oss_midi_putc(struct seq_oss_devinfo *dp, int dev, unsigned char c, stru
 int
 snd_seq_oss_midi_make_info(struct seq_oss_devinfo *dp, int dev, struct midi_info *inf)
 {
-	struct seq_oss_midi *mdev;
+	struct seq_oss_midi *mdev __free(seq_oss_midi) = NULL;
 
 	mdev = get_mididev(dp, dev);
 	if (!mdev)
@@ -665,7 +621,6 @@ snd_seq_oss_midi_make_info(struct seq_oss_devinfo *dp, int dev, struct midi_info
 	inf->dev_type = 0; /* FIXME: ?? */
 	inf->capabilities = 0; /* FIXME: ?? */
 	strscpy(inf->name, mdev->name, sizeof(inf->name));
-	snd_use_lock_free(&mdev->use_lock);
 	return 0;
 }
 
@@ -692,10 +647,11 @@ void
 snd_seq_oss_midi_info_read(struct snd_info_buffer *buf)
 {
 	int i;
-	struct seq_oss_midi *mdev;
 
 	snd_iprintf(buf, "\nNumber of MIDI devices: %d\n", max_midi_devs);
 	for (i = 0; i < max_midi_devs; i++) {
+		struct seq_oss_midi *mdev __free(seq_oss_midi) = NULL;
+
 		snd_iprintf(buf, "\nmidi %d: ", i);
 		mdev = get_mdev(i);
 		if (mdev == NULL) {
@@ -707,7 +663,6 @@ snd_seq_oss_midi_info_read(struct snd_info_buffer *buf)
 		snd_iprintf(buf, "  capability %s / opened %s\n",
 			    capmode_str(mdev->flags),
 			    capmode_str(mdev->opened));
-		snd_use_lock_free(&mdev->use_lock);
 	}
 }
 #endif /* CONFIG_SND_PROC_FS */
-- 
2.50.1


  parent reply	other threads:[~2025-08-11 10:33 UTC|newest]

Thread overview: 12+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2025-08-11 10:32 [PATCH 00/11] ALSA: seq: Use auto-cleanup macros Takashi Iwai
2025-08-11 10:32 ` [PATCH 01/11] ALSA: seq: Simplify internal command operation from OSS layer Takashi Iwai
2025-08-11 10:32 ` [PATCH 02/11] ALSA: seq: Clean up spin lock with guard() Takashi Iwai
2025-08-11 10:32 ` [PATCH 03/11] ALSA: seq: Use guard() for mutex and rwsem locks Takashi Iwai
2025-08-11 10:32 ` [PATCH 04/11] ALSA: seq: Use auto-cleanup for client refcounting Takashi Iwai
2025-08-11 10:32 ` [PATCH 05/11] ALSA: seq: Clean up port locking with auto cleanup Takashi Iwai
2025-08-11 10:32 ` [PATCH 06/11] ALSA: seq: Clean up queue " Takashi Iwai
2025-08-11 10:32 ` [PATCH 07/11] ALSA: seq: Clean up fifo locking with guard Takashi Iwai
2025-08-11 10:32 ` [PATCH 08/11] ALSA: seq: oss: Clean up core code with guard() Takashi Iwai
2025-08-11 10:32 ` Takashi Iwai [this message]
2025-08-11 10:32 ` [PATCH 10/11] ALSA: seq: oss/synth: Clean up with guard and auto cleanup Takashi Iwai
2025-08-11 10:32 ` [PATCH 11/11] ALSA: seq: oss/rw: Cleanup with guard Takashi Iwai

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20250811103249.10644-10-tiwai@suse.de \
    --to=tiwai@suse.de \
    --cc=linux-sound@vger.kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox