[PATCH v2 00/11] ALSA: seq: Use auto-cleanup macros

linux-sound.vger.kernel.org archive mirror
 help / color / mirror / Atom feed

* [PATCH v2 00/11] ALSA: seq: Use auto-cleanup macros
@ 2025-08-27  8:05 Takashi Iwai
  2025-08-27  8:05 ` [PATCH v2 01/11] ALSA: seq: Simplify internal command operation from OSS layer Takashi Iwai
                   ` (10 more replies)
  0 siblings, 11 replies; 12+ messages in thread
From: Takashi Iwai @ 2025-08-27  8:05 UTC (permalink / raw)
  To: linux-sound

This is a revised patch set for ALSA sequencer code to clean up with
the auto-cleanup macros like guard() and CLASS().
Only code refactoring, and no behavior changes intended.

v1->v2:
* Drop buggy conversion in oss/seq_oss_midi.c
* Fix build errors with clang W=1


Takashi

===

Takashi Iwai (11):
  ALSA: seq: Simplify internal command operation from OSS layer
  ALSA: seq: Clean up spin lock with guard()
  ALSA: seq: Use guard() for mutex and rwsem locks
  ALSA: seq: Use auto-cleanup for client refcounting
  ALSA: seq: Clean up port locking with auto cleanup
  ALSA: seq: Clean up queue locking with auto cleanup
  ALSA: seq: Clean up fifo locking with guard
  ALSA: seq: oss: Clean up core code with guard()
  ALSA: seq: oss/midi: Cleanup with guard and auto-cleanup
  ALSA: seq: oss/synth: Clean up with guard and auto cleanup
  ALSA: seq: oss/rw: Cleanup with guard

 sound/core/seq/oss/seq_oss.c        |  24 +-
 sound/core/seq/oss/seq_oss_device.h |   7 +-
 sound/core/seq/oss/seq_oss_midi.c   | 116 ++---
 sound/core/seq/oss/seq_oss_readq.c  |  10 +-
 sound/core/seq/oss/seq_oss_synth.c  | 125 ++---
 sound/core/seq/oss/seq_oss_writeq.c |   5 +-
 sound/core/seq/seq_clientmgr.c      | 759 ++++++++++++----------------
 sound/core/seq/seq_clientmgr.h      |  17 +-
 sound/core/seq/seq_fifo.c           |  15 +-
 sound/core/seq/seq_fifo.h           |   1 +
 sound/core/seq/seq_ports.c          |  19 +-
 sound/core/seq/seq_ports.h          |   2 +
 sound/core/seq/seq_queue.c          |  76 +--
 sound/core/seq/seq_queue.h          |   2 +
 sound/core/seq/seq_timer.c          |   5 +-
 15 files changed, 463 insertions(+), 720 deletions(-)

-- 
2.50.1


^ permalink raw reply	[flat|nested] 12+ messages in thread

* [PATCH v2 01/11] ALSA: seq: Simplify internal command operation from OSS layer
  2025-08-27  8:05 [PATCH v2 00/11] ALSA: seq: Use auto-cleanup macros Takashi Iwai
@ 2025-08-27  8:05 ` Takashi Iwai
  2025-08-27  8:05 ` [PATCH v2 02/11] ALSA: seq: Clean up spin lock with guard() Takashi Iwai
                   ` (9 subsequent siblings)
  10 siblings, 0 replies; 12+ messages in thread
From: Takashi Iwai @ 2025-08-27  8:05 UTC (permalink / raw)
  To: linux-sound

snd_seq_client_ioctl_lock() and *_unlock() are used only from a single
function of the OSS layer, and it's just to wrap the call of
snd_seq_kernel_client_ctl().

Provide another variant of snd_seq_kernel_client_ctl() that takes the
locks internally and drop the ugly snd_seq_client_ioctl_lock() and
*_unlock() implementations, instead.

Signed-off-by: Takashi Iwai <tiwai@suse.de>
---
 sound/core/seq/oss/seq_oss_device.h |  7 +--
 sound/core/seq/seq_clientmgr.c      | 77 +++++++++++++----------------
 sound/core/seq/seq_clientmgr.h      |  3 +-
 3 files changed, 35 insertions(+), 52 deletions(-)

diff --git a/sound/core/seq/oss/seq_oss_device.h b/sound/core/seq/oss/seq_oss_device.h
index 6163a00bc8de..935cf3df0b30 100644
--- a/sound/core/seq/oss/seq_oss_device.h
+++ b/sound/core/seq/oss/seq_oss_device.h
@@ -137,12 +137,7 @@ snd_seq_oss_dispatch(struct seq_oss_devinfo *dp, struct snd_seq_event *ev, int a
 static inline int
 snd_seq_oss_control(struct seq_oss_devinfo *dp, unsigned int type, void *arg)
 {
-	int err;
-
-	snd_seq_client_ioctl_lock(dp->cseq);
-	err = snd_seq_kernel_client_ctl(dp->cseq, type, arg);
-	snd_seq_client_ioctl_unlock(dp->cseq);
-	return err;
+	return snd_seq_kernel_client_ioctl(dp->cseq, type, arg);
 }
 
 /* fill the addresses in header */
diff --git a/sound/core/seq/seq_clientmgr.c b/sound/core/seq/seq_clientmgr.c
index aa9c956d2581..7787f4661626 100644
--- a/sound/core/seq/seq_clientmgr.c
+++ b/sound/core/seq/seq_clientmgr.c
@@ -182,41 +182,6 @@ static struct snd_seq_client *client_load_and_use_ptr(int clientid)
 	return client_use_ptr(clientid, IS_ENABLED(CONFIG_MODULES));
 }
 
-/* Take refcount and perform ioctl_mutex lock on the given client;
- * used only for OSS sequencer
- * Unlock via snd_seq_client_ioctl_unlock() below
- */
-bool snd_seq_client_ioctl_lock(int clientid)
-{
-	struct snd_seq_client *client;
-
-	client = client_load_and_use_ptr(clientid);
-	if (!client)
-		return false;
-	mutex_lock(&client->ioctl_mutex);
-	/* The client isn't unrefed here; see snd_seq_client_ioctl_unlock() */
-	return true;
-}
-EXPORT_SYMBOL_GPL(snd_seq_client_ioctl_lock);
-
-/* Unlock and unref the given client; for OSS sequencer use only */
-void snd_seq_client_ioctl_unlock(int clientid)
-{
-	struct snd_seq_client *client;
-
-	client = snd_seq_client_use_ptr(clientid);
-	if (WARN_ON(!client))
-		return;
-	mutex_unlock(&client->ioctl_mutex);
-	/* The doubly unrefs below are intentional; the first one releases the
-	 * leftover from snd_seq_client_ioctl_lock() above, and the second one
-	 * is for releasing snd_seq_client_use_ptr() in this function
-	 */
-	snd_seq_client_unlock(client);
-	snd_seq_client_unlock(client);
-}
-EXPORT_SYMBOL_GPL(snd_seq_client_ioctl_unlock);
-
 static void usage_alloc(struct snd_seq_usage *res, int num)
 {
 	res->cur += num;
@@ -2558,6 +2523,21 @@ int snd_seq_kernel_client_dispatch(int client, struct snd_seq_event * ev,
 }
 EXPORT_SYMBOL(snd_seq_kernel_client_dispatch);
 
+static int call_seq_client_ctl(struct snd_seq_client *client,
+			       unsigned int cmd, void *arg)
+{
+	const struct ioctl_handler *handler;
+
+	for (handler = ioctl_handlers; handler->cmd > 0; ++handler) {
+		if (handler->cmd == cmd)
+			return handler->func(client, arg);
+	}
+
+	pr_debug("ALSA: seq unknown ioctl() 0x%x (type='%c', number=0x%02x)\n",
+		 cmd, _IOC_TYPE(cmd), _IOC_NR(cmd));
+	return -ENOTTY;
+}
+
 /**
  * snd_seq_kernel_client_ctl - operate a command for a client with data in
  *			       kernel space.
@@ -2572,24 +2552,33 @@ EXPORT_SYMBOL(snd_seq_kernel_client_dispatch);
  */
 int snd_seq_kernel_client_ctl(int clientid, unsigned int cmd, void *arg)
 {
-	const struct ioctl_handler *handler;
 	struct snd_seq_client *client;
 
 	client = clientptr(clientid);
 	if (client == NULL)
 		return -ENXIO;
 
-	for (handler = ioctl_handlers; handler->cmd > 0; ++handler) {
-		if (handler->cmd == cmd)
-			return handler->func(client, arg);
-	}
-
-	pr_debug("ALSA: seq unknown ioctl() 0x%x (type='%c', number=0x%02x)\n",
-		 cmd, _IOC_TYPE(cmd), _IOC_NR(cmd));
-	return -ENOTTY;
+	return call_seq_client_ctl(client, cmd, arg);
 }
 EXPORT_SYMBOL(snd_seq_kernel_client_ctl);
 
+/* a similar like above but taking locks; used only from OSS sequencer layer */
+int snd_seq_kernel_client_ioctl(int clientid, unsigned int cmd, void *arg)
+{
+	struct snd_seq_client *client;
+	int ret;
+
+	client = client_load_and_use_ptr(clientid);
+	if (!client)
+		return -ENXIO;
+	mutex_lock(&client->ioctl_mutex);
+	ret = call_seq_client_ctl(client, cmd, arg);
+	mutex_unlock(&client->ioctl_mutex);
+	snd_seq_client_unlock(client);
+	return ret;
+}
+EXPORT_SYMBOL_GPL(snd_seq_kernel_client_ioctl);
+
 /* exported (for OSS emulator) */
 int snd_seq_kernel_client_write_poll(int clientid, struct file *file, poll_table *wait)
 {
diff --git a/sound/core/seq/seq_clientmgr.h b/sound/core/seq/seq_clientmgr.h
index 915b1017286e..7d0758e72aa9 100644
--- a/sound/core/seq/seq_clientmgr.h
+++ b/sound/core/seq/seq_clientmgr.h
@@ -94,8 +94,7 @@ int __snd_seq_deliver_single_event(struct snd_seq_client *dest,
 				   int atomic, int hop);
 
 /* only for OSS sequencer */
-bool snd_seq_client_ioctl_lock(int clientid);
-void snd_seq_client_ioctl_unlock(int clientid);
+int snd_seq_kernel_client_ioctl(int clientid, unsigned int cmd, void *arg);
 
 extern int seq_client_load[15];
 
-- 
2.50.1


^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [PATCH v2 02/11] ALSA: seq: Clean up spin lock with guard()
  2025-08-27  8:05 [PATCH v2 00/11] ALSA: seq: Use auto-cleanup macros Takashi Iwai
  2025-08-27  8:05 ` [PATCH v2 01/11] ALSA: seq: Simplify internal command operation from OSS layer Takashi Iwai
@ 2025-08-27  8:05 ` Takashi Iwai
  2025-08-27  8:05 ` [PATCH v2 03/11] ALSA: seq: Use guard() for mutex and rwsem locks Takashi Iwai
                   ` (8 subsequent siblings)
  10 siblings, 0 replies; 12+ messages in thread
From: Takashi Iwai @ 2025-08-27  8:05 UTC (permalink / raw)
  To: linux-sound

Use guard() for spin locks to manage the sequencer client locking.

The code about the refcounting was modified with the new
snd_seq_client_ref() and *_unref() helpers instead of the ugly goto,
too.

Only code refactoring, and no behavior change.

Signed-off-by: Takashi Iwai <tiwai@suse.de>
---
 sound/core/seq/seq_clientmgr.c | 77 +++++++++++++++-------------------
 sound/core/seq/seq_clientmgr.h | 14 ++++++-
 2 files changed, 47 insertions(+), 44 deletions(-)

diff --git a/sound/core/seq/seq_clientmgr.c b/sound/core/seq/seq_clientmgr.c
index 7787f4661626..89f99e713a2f 100644
--- a/sound/core/seq/seq_clientmgr.c
+++ b/sound/core/seq/seq_clientmgr.c
@@ -108,7 +108,6 @@ static struct snd_seq_client *clientptr(int clientid)
 
 static struct snd_seq_client *client_use_ptr(int clientid, bool load_module)
 {
-	unsigned long flags;
 	struct snd_seq_client *client;
 
 	if (clientid < 0 || clientid >= SNDRV_SEQ_MAX_CLIENTS) {
@@ -116,15 +115,13 @@ static struct snd_seq_client *client_use_ptr(int clientid, bool load_module)
 			   clientid);
 		return NULL;
 	}
-	spin_lock_irqsave(&clients_lock, flags);
-	client = clientptr(clientid);
-	if (client)
-		goto __lock;
-	if (clienttablock[clientid]) {
-		spin_unlock_irqrestore(&clients_lock, flags);
-		return NULL;
+	scoped_guard(spinlock_irqsave, &clients_lock) {
+		client = clientptr(clientid);
+		if (client)
+			return snd_seq_client_ref(client);
+		if (clienttablock[clientid])
+			return NULL;
 	}
-	spin_unlock_irqrestore(&clients_lock, flags);
 #ifdef CONFIG_MODULES
 	if (load_module) {
 		static DECLARE_BITMAP(client_requested, SNDRV_SEQ_GLOBAL_CLIENTS);
@@ -153,19 +150,14 @@ static struct snd_seq_client *client_use_ptr(int clientid, bool load_module)
 				snd_seq_device_load_drivers();
 			}
 		}
-		spin_lock_irqsave(&clients_lock, flags);
-		client = clientptr(clientid);
-		if (client)
-			goto __lock;
-		spin_unlock_irqrestore(&clients_lock, flags);
+		scoped_guard(spinlock_irqsave, &clients_lock) {
+			client = clientptr(clientid);
+			if (client)
+				return snd_seq_client_ref(client);
+		}
 	}
 #endif
 	return NULL;
-
-      __lock:
-	snd_use_lock_use(&client->use_lock);
-	spin_unlock_irqrestore(&clients_lock, flags);
-	return client;
 }
 
 /* get snd_seq_client object for the given id quickly */
@@ -227,25 +219,24 @@ static struct snd_seq_client *seq_create_client1(int client_index, int poolsize)
 	client->ump_endpoint_port = -1;
 
 	/* find free slot in the client table */
-	spin_lock_irq(&clients_lock);
-	if (client_index < 0) {
-		for (c = SNDRV_SEQ_DYNAMIC_CLIENTS_BEGIN;
-		     c < SNDRV_SEQ_MAX_CLIENTS;
-		     c++) {
-			if (clienttab[c] || clienttablock[c])
-				continue;
-			clienttab[client->number = c] = client;
-			spin_unlock_irq(&clients_lock);
-			return client;
-		}
-	} else {
-		if (clienttab[client_index] == NULL && !clienttablock[client_index]) {
-			clienttab[client->number = client_index] = client;
-			spin_unlock_irq(&clients_lock);
-			return client;
+	scoped_guard(spinlock_irq, &clients_lock) {
+		if (client_index < 0) {
+			for (c = SNDRV_SEQ_DYNAMIC_CLIENTS_BEGIN;
+			     c < SNDRV_SEQ_MAX_CLIENTS;
+			     c++) {
+				if (clienttab[c] || clienttablock[c])
+					continue;
+				clienttab[client->number = c] = client;
+				return client;
+			}
+		} else {
+			if (clienttab[client_index] == NULL && !clienttablock[client_index]) {
+				clienttab[client->number = client_index] = client;
+				return client;
+			}
 		}
 	}
-	spin_unlock_irq(&clients_lock);
+
 	snd_seq_pool_delete(&client->pool);
 	kfree(client);
 	return NULL;	/* no free slot found or busy, return failure code */
@@ -256,18 +247,18 @@ static int seq_free_client1(struct snd_seq_client *client)
 {
 	if (!client)
 		return 0;
-	spin_lock_irq(&clients_lock);
-	clienttablock[client->number] = 1;
-	clienttab[client->number] = NULL;
-	spin_unlock_irq(&clients_lock);
+	scoped_guard(spinlock_irq, &clients_lock) {
+		clienttablock[client->number] = 1;
+		clienttab[client->number] = NULL;
+	}
 	snd_seq_delete_all_ports(client);
 	snd_seq_queue_client_leave(client->number);
 	snd_use_lock_sync(&client->use_lock);
 	if (client->pool)
 		snd_seq_pool_delete(&client->pool);
-	spin_lock_irq(&clients_lock);
-	clienttablock[client->number] = 0;
-	spin_unlock_irq(&clients_lock);
+	scoped_guard(spinlock_irq, &clients_lock) {
+		clienttablock[client->number] = 0;
+	}
 	return 0;
 }
 
diff --git a/sound/core/seq/seq_clientmgr.h b/sound/core/seq/seq_clientmgr.h
index 7d0758e72aa9..b42afb887daf 100644
--- a/sound/core/seq/seq_clientmgr.h
+++ b/sound/core/seq/seq_clientmgr.h
@@ -78,8 +78,20 @@ void snd_sequencer_device_done(void);
 /* get locked pointer to client */
 struct snd_seq_client *snd_seq_client_use_ptr(int clientid);
 
+static inline struct snd_seq_client *
+snd_seq_client_ref(struct snd_seq_client *client)
+{
+	snd_use_lock_use(&client->use_lock);
+	return client;
+}
+
 /* unlock pointer to client */
-#define snd_seq_client_unlock(client) snd_use_lock_free(&(client)->use_lock)
+static inline void snd_seq_client_unref(struct snd_seq_client *client)
+{
+	snd_use_lock_free(&client->use_lock);
+}
+
+#define snd_seq_client_unlock(c)	snd_seq_client_unref(c)
 
 /* dispatch event to client(s) */
 int snd_seq_dispatch_event(struct snd_seq_event_cell *cell, int atomic, int hop);
-- 
2.50.1


^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [PATCH v2 03/11] ALSA: seq: Use guard() for mutex and rwsem locks
  2025-08-27  8:05 [PATCH v2 00/11] ALSA: seq: Use auto-cleanup macros Takashi Iwai
  2025-08-27  8:05 ` [PATCH v2 01/11] ALSA: seq: Simplify internal command operation from OSS layer Takashi Iwai
  2025-08-27  8:05 ` [PATCH v2 02/11] ALSA: seq: Clean up spin lock with guard() Takashi Iwai
@ 2025-08-27  8:05 ` Takashi Iwai
  2025-08-27  8:05 ` [PATCH v2 04/11] ALSA: seq: Use auto-cleanup for client refcounting Takashi Iwai
                   ` (7 subsequent siblings)
  10 siblings, 0 replies; 12+ messages in thread
From: Takashi Iwai @ 2025-08-27  8:05 UTC (permalink / raw)
  To: linux-sound

There are a few manual calls of mutex and rwsem lock/unlock pairs in
seq_clientmngr.c, and those can be replaced nicely with guard().

Only code refactoring, and no behavior change.

Signed-off-by: Takashi Iwai <tiwai@suse.de>
---
 sound/core/seq/seq_clientmgr.c | 226 ++++++++++++++++-----------------
 1 file changed, 108 insertions(+), 118 deletions(-)

diff --git a/sound/core/seq/seq_clientmgr.c b/sound/core/seq/seq_clientmgr.c
index 89f99e713a2f..5a67c4b2b644 100644
--- a/sound/core/seq/seq_clientmgr.c
+++ b/sound/core/seq/seq_clientmgr.c
@@ -265,23 +265,23 @@ static int seq_free_client1(struct snd_seq_client *client)
 
 static void seq_free_client(struct snd_seq_client * client)
 {
-	mutex_lock(&register_mutex);
-	switch (client->type) {
-	case NO_CLIENT:
-		pr_warn("ALSA: seq: Trying to free unused client %d\n",
-			client->number);
-		break;
-	case USER_CLIENT:
-	case KERNEL_CLIENT:
-		seq_free_client1(client);
-		usage_free(&client_usage, 1);
-		break;
+	scoped_guard(mutex, &register_mutex) {
+		switch (client->type) {
+		case NO_CLIENT:
+			pr_warn("ALSA: seq: Trying to free unused client %d\n",
+				client->number);
+			break;
+		case USER_CLIENT:
+		case KERNEL_CLIENT:
+			seq_free_client1(client);
+			usage_free(&client_usage, 1);
+			break;
 
-	default:
-		pr_err("ALSA: seq: Trying to free client %d with undefined type = %d\n",
-			   client->number, client->type);
+		default:
+			pr_err("ALSA: seq: Trying to free client %d with undefined type = %d\n",
+			       client->number, client->type);
+		}
 	}
-	mutex_unlock(&register_mutex);
 
 	snd_seq_system_client_ev_client_exit(client->number);
 }
@@ -302,37 +302,34 @@ static int snd_seq_open(struct inode *inode, struct file *file)
 	if (err < 0)
 		return err;
 
-	mutex_lock(&register_mutex);
-	client = seq_create_client1(-1, SNDRV_SEQ_DEFAULT_EVENTS);
-	if (!client) {
-		mutex_unlock(&register_mutex);
-		return -ENOMEM;	/* failure code */
-	}
+	scoped_guard(mutex, &register_mutex) {
+		client = seq_create_client1(-1, SNDRV_SEQ_DEFAULT_EVENTS);
+		if (!client)
+			return -ENOMEM;	/* failure code */
 
-	mode = snd_seq_file_flags(file);
-	if (mode & SNDRV_SEQ_LFLG_INPUT)
-		client->accept_input = 1;
-	if (mode & SNDRV_SEQ_LFLG_OUTPUT)
-		client->accept_output = 1;
+		mode = snd_seq_file_flags(file);
+		if (mode & SNDRV_SEQ_LFLG_INPUT)
+			client->accept_input = 1;
+		if (mode & SNDRV_SEQ_LFLG_OUTPUT)
+			client->accept_output = 1;
 
-	user = &client->data.user;
-	user->fifo = NULL;
-	user->fifo_pool_size = 0;
+		user = &client->data.user;
+		user->fifo = NULL;
+		user->fifo_pool_size = 0;
 
-	if (mode & SNDRV_SEQ_LFLG_INPUT) {
-		user->fifo_pool_size = SNDRV_SEQ_DEFAULT_CLIENT_EVENTS;
-		user->fifo = snd_seq_fifo_new(user->fifo_pool_size);
-		if (user->fifo == NULL) {
-			seq_free_client1(client);
-			kfree(client);
-			mutex_unlock(&register_mutex);
-			return -ENOMEM;
+		if (mode & SNDRV_SEQ_LFLG_INPUT) {
+			user->fifo_pool_size = SNDRV_SEQ_DEFAULT_CLIENT_EVENTS;
+			user->fifo = snd_seq_fifo_new(user->fifo_pool_size);
+			if (user->fifo == NULL) {
+				seq_free_client1(client);
+				kfree(client);
+				return -ENOMEM;
+			}
 		}
-	}
 
-	usage_alloc(&client_usage, 1);
-	client->type = USER_CLIENT;
-	mutex_unlock(&register_mutex);
+		usage_alloc(&client_usage, 1);
+		client->type = USER_CLIENT;
+	}
 
 	c = client->number;
 	file->private_data = client;
@@ -2180,50 +2177,50 @@ static int snd_seq_ioctl_client_ump_info(struct snd_seq_client *caller,
 	if (!cptr)
 		return -ENOENT;
 
-	mutex_lock(&cptr->ioctl_mutex);
-	if (!cptr->midi_version) {
-		err = -EBADFD;
-		goto error;
-	}
-
-	if (cmd == SNDRV_SEQ_IOCTL_GET_CLIENT_UMP_INFO) {
-		if (!cptr->ump_info)
-			p = NULL;
-		else
-			p = cptr->ump_info[type];
-		if (!p) {
-			err = -ENODEV;
-			goto error;
-		}
-		if (copy_to_user(argp->info, p, size)) {
-			err = -EFAULT;
-			goto error;
-		}
-	} else {
-		if (cptr->type != USER_CLIENT) {
+	scoped_guard(mutex, &cptr->ioctl_mutex) {
+		if (!cptr->midi_version) {
 			err = -EBADFD;
 			goto error;
 		}
-		if (!cptr->ump_info) {
-			cptr->ump_info = kcalloc(NUM_UMP_INFOS,
-						 sizeof(void *), GFP_KERNEL);
-			if (!cptr->ump_info) {
-				err = -ENOMEM;
+
+		if (cmd == SNDRV_SEQ_IOCTL_GET_CLIENT_UMP_INFO) {
+			if (!cptr->ump_info)
+				p = NULL;
+			else
+				p = cptr->ump_info[type];
+			if (!p) {
+				err = -ENODEV;
 				goto error;
 			}
+			if (copy_to_user(argp->info, p, size)) {
+				err = -EFAULT;
+				goto error;
+			}
+		} else {
+			if (cptr->type != USER_CLIENT) {
+				err = -EBADFD;
+				goto error;
+			}
+			if (!cptr->ump_info) {
+				cptr->ump_info = kcalloc(NUM_UMP_INFOS,
+							 sizeof(void *), GFP_KERNEL);
+				if (!cptr->ump_info) {
+					err = -ENOMEM;
+					goto error;
+				}
+			}
+			p = memdup_user(argp->info, size);
+			if (IS_ERR(p)) {
+				err = PTR_ERR(p);
+				goto error;
+			}
+			kfree(cptr->ump_info[type]);
+			terminate_ump_info_strings(p, type);
+			cptr->ump_info[type] = p;
 		}
-		p = memdup_user(argp->info, size);
-		if (IS_ERR(p)) {
-			err = PTR_ERR(p);
-			goto error;
-		}
-		kfree(cptr->ump_info[type]);
-		terminate_ump_info_strings(p, type);
-		cptr->ump_info[type] = p;
 	}
 
  error:
-	mutex_unlock(&cptr->ioctl_mutex);
 	snd_seq_client_unlock(cptr);
 	if (!err && cmd == SNDRV_SEQ_IOCTL_SET_CLIENT_UMP_INFO) {
 		if (type == SNDRV_SEQ_CLIENT_UMP_INFO_ENDPOINT)
@@ -2337,9 +2334,9 @@ static long snd_seq_ioctl(struct file *file, unsigned int cmd,
 			return -EFAULT;
 	}
 
-	mutex_lock(&client->ioctl_mutex);
-	err = handler->func(client, &buf);
-	mutex_unlock(&client->ioctl_mutex);
+	scoped_guard(mutex, &client->ioctl_mutex) {
+		err = handler->func(client, &buf);
+	}
 	if (err >= 0) {
 		/* Some commands includes a bug in 'dir' field. */
 		if (handler->cmd == SNDRV_SEQ_IOCTL_SET_QUEUE_CLIENT ||
@@ -2376,34 +2373,32 @@ int snd_seq_create_kernel_client(struct snd_card *card, int client_index,
 	if (card == NULL && client_index >= SNDRV_SEQ_GLOBAL_CLIENTS)
 		return -EINVAL;
 
-	mutex_lock(&register_mutex);
+	scoped_guard(mutex, &register_mutex) {
 
-	if (card) {
-		client_index += SNDRV_SEQ_GLOBAL_CLIENTS
-			+ card->number * SNDRV_SEQ_CLIENTS_PER_CARD;
-		if (client_index >= SNDRV_SEQ_DYNAMIC_CLIENTS_BEGIN)
-			client_index = -1;
-	}
+		if (card) {
+			client_index += SNDRV_SEQ_GLOBAL_CLIENTS
+				+ card->number * SNDRV_SEQ_CLIENTS_PER_CARD;
+			if (client_index >= SNDRV_SEQ_DYNAMIC_CLIENTS_BEGIN)
+				client_index = -1;
+		}
 
-	/* empty write queue as default */
-	client = seq_create_client1(client_index, 0);
-	if (client == NULL) {
-		mutex_unlock(&register_mutex);
-		return -EBUSY;	/* failure code */
-	}
-	usage_alloc(&client_usage, 1);
+		/* empty write queue as default */
+		client = seq_create_client1(client_index, 0);
+		if (client == NULL)
+			return -EBUSY;	/* failure code */
+		usage_alloc(&client_usage, 1);
 
-	client->accept_input = 1;
-	client->accept_output = 1;
-	client->data.kernel.card = card;
-	client->user_pversion = SNDRV_SEQ_VERSION;
+		client->accept_input = 1;
+		client->accept_output = 1;
+		client->data.kernel.card = card;
+		client->user_pversion = SNDRV_SEQ_VERSION;
 		
-	va_start(args, name_fmt);
-	vsnprintf(client->name, sizeof(client->name), name_fmt, args);
-	va_end(args);
+		va_start(args, name_fmt);
+		vsnprintf(client->name, sizeof(client->name), name_fmt, args);
+		va_end(args);
 
-	client->type = KERNEL_CLIENT;
-	mutex_unlock(&register_mutex);
+		client->type = KERNEL_CLIENT;
+	}
 
 	/* make others aware this new client */
 	snd_seq_system_client_ev_client_start(client->number);
@@ -2465,11 +2460,10 @@ int snd_seq_kernel_client_enqueue(int client, struct snd_seq_event *ev,
 	if (!cptr->accept_output) {
 		result = -EPERM;
 	} else { /* send it */
-		mutex_lock(&cptr->ioctl_mutex);
+		guard(mutex)(&cptr->ioctl_mutex);
 		result = snd_seq_client_enqueue_event(cptr, ev, file, blocking,
 						      false, 0,
 						      &cptr->ioctl_mutex);
-		mutex_unlock(&cptr->ioctl_mutex);
 	}
 
 	snd_seq_client_unlock(cptr);
@@ -2562,9 +2556,9 @@ int snd_seq_kernel_client_ioctl(int clientid, unsigned int cmd, void *arg)
 	client = client_load_and_use_ptr(clientid);
 	if (!client)
 		return -ENXIO;
-	mutex_lock(&client->ioctl_mutex);
-	ret = call_seq_client_ctl(client, cmd, arg);
-	mutex_unlock(&client->ioctl_mutex);
+	scoped_guard(mutex, &client->ioctl_mutex) {
+		ret = call_seq_client_ctl(client, cmd, arg);
+	}
 	snd_seq_client_unlock(client);
 	return ret;
 }
@@ -2614,11 +2608,9 @@ static void snd_seq_info_dump_subscribers(struct snd_info_buffer *buffer,
 	struct snd_seq_subscribers *s;
 	int count = 0;
 
-	down_read(&group->list_mutex);
-	if (list_empty(&group->list_head)) {
-		up_read(&group->list_mutex);
+	guard(rwsem_read)(&group->list_mutex);
+	if (list_empty(&group->list_head))
 		return;
-	}
 	snd_iprintf(buffer, msg);
 	list_for_each(p, &group->list_head) {
 		if (is_src)
@@ -2635,7 +2627,6 @@ static void snd_seq_info_dump_subscribers(struct snd_info_buffer *buffer,
 		if (group->exclusive)
 			snd_iprintf(buffer, "[ex]");
 	}
-	up_read(&group->list_mutex);
 	snd_iprintf(buffer, "\n");
 }
 
@@ -2661,7 +2652,7 @@ static void snd_seq_info_dump_ports(struct snd_info_buffer *buffer,
 {
 	struct snd_seq_client_port *p;
 
-	mutex_lock(&client->ports_mutex);
+	guard(mutex)(&client->ports_mutex);
 	list_for_each_entry(p, &client->ports_list_head, list) {
 		if (p->capability & SNDRV_SEQ_PORT_CAP_INACTIVE)
 			continue;
@@ -2680,7 +2671,6 @@ static void snd_seq_info_dump_ports(struct snd_info_buffer *buffer,
 		snd_seq_info_dump_subscribers(buffer, &p->c_src, 1, "    Connecting To: ");
 		snd_seq_info_dump_subscribers(buffer, &p->c_dest, 0, "    Connected From: ");
 	}
-	mutex_unlock(&client->ports_mutex);
 }
 
 static const char *midi_version_string(unsigned int version)
@@ -2777,10 +2767,10 @@ int __init snd_sequencer_device_init(void)
 		return err;
 	dev_set_name(seq_dev, "seq");
 
-	mutex_lock(&register_mutex);
-	err = snd_register_device(SNDRV_DEVICE_TYPE_SEQUENCER, NULL, 0,
-				  &snd_seq_f_ops, NULL, seq_dev);
-	mutex_unlock(&register_mutex);
+	scoped_guard(mutex, &register_mutex) {
+		err = snd_register_device(SNDRV_DEVICE_TYPE_SEQUENCER, NULL, 0,
+					  &snd_seq_f_ops, NULL, seq_dev);
+	}
 	if (err < 0) {
 		put_device(seq_dev);
 		return err;
-- 
2.50.1


^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [PATCH v2 04/11] ALSA: seq: Use auto-cleanup for client refcounting
  2025-08-27  8:05 [PATCH v2 00/11] ALSA: seq: Use auto-cleanup macros Takashi Iwai
                   ` (2 preceding siblings ...)
  2025-08-27  8:05 ` [PATCH v2 03/11] ALSA: seq: Use guard() for mutex and rwsem locks Takashi Iwai
@ 2025-08-27  8:05 ` Takashi Iwai
  2025-08-27  8:05 ` [PATCH v2 05/11] ALSA: seq: Clean up port locking with auto cleanup Takashi Iwai
                   ` (6 subsequent siblings)
  10 siblings, 0 replies; 12+ messages in thread
From: Takashi Iwai @ 2025-08-27  8:05 UTC (permalink / raw)
  To: linux-sound

The current code manages the refcount of client in a way like:

	snd_seq_client *client;
	client = clientptr(id);
	....
	snd_seq_client_unlock(client);

Now we introduce an auto-cleanup macro to manage the unlock
implicitly, namely, the above will be replaced like:

	snd_seq_client *client __free(snd_seq_client) = NULL;
	client = clientptr(id);

and we can forget the unref call.

A part of the code in snd_seq_deliver_single_event() is factored out
to a function, so that the auto-cleanups can be applied cleanly.

This also allows us to replace some left mutex lock/unlock with
guard(), and also reduce scoped_guard() to the normal guard(), too.

Only code refactoring, and no behavior change.

Signed-off-by: Takashi Iwai <tiwai@suse.de>
---
 sound/core/seq/seq_clientmgr.c | 214 +++++++++++++--------------------
 sound/core/seq/seq_clientmgr.h |   2 +-
 sound/core/seq/seq_ports.c     |  16 +--
 3 files changed, 86 insertions(+), 146 deletions(-)

diff --git a/sound/core/seq/seq_clientmgr.c b/sound/core/seq/seq_clientmgr.c
index 5a67c4b2b644..5b14d70ba87a 100644
--- a/sound/core/seq/seq_clientmgr.c
+++ b/sound/core/seq/seq_clientmgr.c
@@ -495,24 +495,21 @@ static int check_port_perm(struct snd_seq_client_port *port, unsigned int flags)
  */
 static struct snd_seq_client *get_event_dest_client(struct snd_seq_event *event)
 {
-	struct snd_seq_client *dest;
+	struct snd_seq_client *dest __free(snd_seq_client) = NULL;
 
 	dest = snd_seq_client_use_ptr(event->dest.client);
 	if (dest == NULL)
 		return NULL;
 	if (! dest->accept_input)
-		goto __not_avail;
+		return NULL;
 	if (snd_seq_ev_is_ump(event))
-		return dest; /* ok - no filter checks */
+		return no_free_ptr(dest); /* ok - no filter checks */
 
 	if ((dest->filter & SNDRV_SEQ_FILTER_USE_EVENT) &&
 	    ! test_bit(event->type, dest->event_filter))
-		goto __not_avail;
+		return NULL;
 
-	return dest; /* ok - accessible */
-__not_avail:
-	snd_seq_client_unlock(dest);
-	return NULL;
+	return no_free_ptr(dest); /* ok - accessible */
 }
 
 
@@ -609,23 +606,14 @@ int __snd_seq_deliver_single_event(struct snd_seq_client *dest,
 	return 0;
 }
 
-/*
- * deliver an event to the specified destination.
- * if filter is non-zero, client filter bitmap is tested.
- *
- *  RETURN VALUE: 0 : if succeeded
- *		 <0 : error
- */
-static int snd_seq_deliver_single_event(struct snd_seq_client *client,
-					struct snd_seq_event *event,
-					int atomic, int hop)
+/* deliver a single event; called from snd_seq_deliver_single_event() */
+static int _snd_seq_deliver_single_event(struct snd_seq_client *client,
+					 struct snd_seq_event *event,
+					 int atomic, int hop)
 {
-	struct snd_seq_client *dest = NULL;
+	struct snd_seq_client *dest __free(snd_seq_client) = NULL;
 	struct snd_seq_client_port *dest_port = NULL;
 	int result = -ENOENT;
-	int direct;
-
-	direct = snd_seq_ev_is_direct(event);
 
 	dest = get_event_dest_client(event);
 	if (dest == NULL)
@@ -639,7 +627,7 @@ static int snd_seq_deliver_single_event(struct snd_seq_client *client,
 		result = -EPERM;
 		goto __skip;
 	}
-		
+
 	if (dest_port->timestamping)
 		update_timestamp_of_queue(event, dest_port->time_queue,
 					  dest_port->time_real);
@@ -670,12 +658,24 @@ static int snd_seq_deliver_single_event(struct snd_seq_client *client,
   __skip:
 	if (dest_port)
 		snd_seq_port_unlock(dest_port);
-	if (dest)
-		snd_seq_client_unlock(dest);
+	return result;
+}
 
-	if (result < 0 && !direct) {
-		result = bounce_error_event(client, event, result, atomic, hop);
-	}
+/*
+ * deliver an event to the specified destination.
+ * if filter is non-zero, client filter bitmap is tested.
+ *
+ *  RETURN VALUE: 0 : if succeeded
+ *		 <0 : error
+ */
+static int snd_seq_deliver_single_event(struct snd_seq_client *client,
+					struct snd_seq_event *event,
+					int atomic, int hop)
+{
+	int result = _snd_seq_deliver_single_event(client, event, atomic, hop);
+
+	if (result < 0 && !snd_seq_ev_is_direct(event))
+		return bounce_error_event(client, event, result, atomic, hop);
 	return result;
 }
 
@@ -817,7 +817,7 @@ static int snd_seq_deliver_event(struct snd_seq_client *client, struct snd_seq_e
  */
 int snd_seq_dispatch_event(struct snd_seq_event_cell *cell, int atomic, int hop)
 {
-	struct snd_seq_client *client;
+	struct snd_seq_client *client __free(snd_seq_client) = NULL;
 	int result;
 
 	if (snd_BUG_ON(!cell))
@@ -879,7 +879,6 @@ int snd_seq_dispatch_event(struct snd_seq_event_cell *cell, int atomic, int hop)
 		snd_seq_cell_free(cell);
 	}
 
-	snd_seq_client_unlock(client);
 	return result;
 }
 
@@ -1171,8 +1170,7 @@ static int snd_seq_ioctl_system_info(struct snd_seq_client *client, void *arg)
 static int snd_seq_ioctl_running_mode(struct snd_seq_client *client, void  *arg)
 {
 	struct snd_seq_running_info *info = arg;
-	struct snd_seq_client *cptr;
-	int err = 0;
+	struct snd_seq_client *cptr __free(snd_seq_client) = NULL;
 
 	/* requested client number */
 	cptr = client_load_and_use_ptr(info->client);
@@ -1180,25 +1178,16 @@ static int snd_seq_ioctl_running_mode(struct snd_seq_client *client, void  *arg)
 		return -ENOENT;		/* don't change !!! */
 
 #ifdef SNDRV_BIG_ENDIAN
-	if (!info->big_endian) {
-		err = -EINVAL;
-		goto __err;
-	}
+	if (!info->big_endian)
+		return -EINVAL;
 #else
-	if (info->big_endian) {
-		err = -EINVAL;
-		goto __err;
-	}
-
+	if (info->big_endian)
+		return -EINVAL;
 #endif
-	if (info->cpu_mode > sizeof(long)) {
-		err = -EINVAL;
-		goto __err;
-	}
+	if (info->cpu_mode > sizeof(long))
+		return -EINVAL;
 	cptr->convert32 = (info->cpu_mode < sizeof(long));
- __err:
-	snd_seq_client_unlock(cptr);
-	return err;
+	return 0;
 }
 
 /* CLIENT_INFO ioctl() */
@@ -1234,7 +1223,7 @@ static int snd_seq_ioctl_get_client_info(struct snd_seq_client *client,
 					 void *arg)
 {
 	struct snd_seq_client_info *client_info = arg;
-	struct snd_seq_client *cptr;
+	struct snd_seq_client *cptr __free(snd_seq_client) = NULL;
 
 	/* requested client number */
 	cptr = client_load_and_use_ptr(client_info->client);
@@ -1242,8 +1231,6 @@ static int snd_seq_ioctl_get_client_info(struct snd_seq_client *client,
 		return -ENOENT;		/* don't change !!! */
 
 	get_client_info(cptr, client_info);
-	snd_seq_client_unlock(cptr);
-
 	return 0;
 }
 
@@ -1373,7 +1360,7 @@ static int snd_seq_ioctl_delete_port(struct snd_seq_client *client, void *arg)
 static int snd_seq_ioctl_get_port_info(struct snd_seq_client *client, void *arg)
 {
 	struct snd_seq_port_info *info = arg;
-	struct snd_seq_client *cptr;
+	struct snd_seq_client *cptr __free(snd_seq_client) = NULL;
 	struct snd_seq_client_port *port;
 
 	cptr = client_load_and_use_ptr(info->addr.client);
@@ -1381,16 +1368,12 @@ static int snd_seq_ioctl_get_port_info(struct snd_seq_client *client, void *arg)
 		return -ENXIO;
 
 	port = snd_seq_port_use_ptr(cptr, info->addr.port);
-	if (port == NULL) {
-		snd_seq_client_unlock(cptr);
+	if (port == NULL)
 		return -ENOENT;			/* don't change */
-	}
 
 	/* get port info */
 	snd_seq_get_port_info(port, info);
 	snd_seq_port_unlock(port);
-	snd_seq_client_unlock(cptr);
-
 	return 0;
 }
 
@@ -1478,9 +1461,10 @@ int snd_seq_client_notify_subscription(int client, int port,
 static int snd_seq_ioctl_subscribe_port(struct snd_seq_client *client,
 					void *arg)
 {
-	struct snd_seq_port_subscribe *subs = arg;
 	int result = -EINVAL;
-	struct snd_seq_client *receiver = NULL, *sender = NULL;
+	struct snd_seq_port_subscribe *subs = arg;
+	struct snd_seq_client *receiver __free(snd_seq_client) = NULL;
+	struct snd_seq_client *sender __free(snd_seq_client) = NULL;
 	struct snd_seq_client_port *sport = NULL, *dport = NULL;
 
 	receiver = client_load_and_use_ptr(subs->dest.client);
@@ -1510,10 +1494,6 @@ static int snd_seq_ioctl_subscribe_port(struct snd_seq_client *client,
 		snd_seq_port_unlock(sport);
 	if (dport)
 		snd_seq_port_unlock(dport);
-	if (sender)
-		snd_seq_client_unlock(sender);
-	if (receiver)
-		snd_seq_client_unlock(receiver);
 	return result;
 }
 
@@ -1524,9 +1504,10 @@ static int snd_seq_ioctl_subscribe_port(struct snd_seq_client *client,
 static int snd_seq_ioctl_unsubscribe_port(struct snd_seq_client *client,
 					  void *arg)
 {
-	struct snd_seq_port_subscribe *subs = arg;
 	int result = -ENXIO;
-	struct snd_seq_client *receiver = NULL, *sender = NULL;
+	struct snd_seq_port_subscribe *subs = arg;
+	struct snd_seq_client *receiver __free(snd_seq_client) = NULL;
+	struct snd_seq_client *sender __free(snd_seq_client) = NULL;
 	struct snd_seq_client_port *sport = NULL, *dport = NULL;
 
 	receiver = snd_seq_client_use_ptr(subs->dest.client);
@@ -1555,10 +1536,6 @@ static int snd_seq_ioctl_unsubscribe_port(struct snd_seq_client *client,
 		snd_seq_port_unlock(sport);
 	if (dport)
 		snd_seq_port_unlock(dport);
-	if (sender)
-		snd_seq_client_unlock(sender);
-	if (receiver)
-		snd_seq_client_unlock(receiver);
 	return result;
 }
 
@@ -1849,7 +1826,7 @@ static int snd_seq_ioctl_get_client_pool(struct snd_seq_client *client,
 					 void *arg)
 {
 	struct snd_seq_client_pool *info = arg;
-	struct snd_seq_client *cptr;
+	struct snd_seq_client *cptr __free(snd_seq_client) = NULL;
 
 	cptr = client_load_and_use_ptr(info->client);
 	if (cptr == NULL)
@@ -1868,7 +1845,6 @@ static int snd_seq_ioctl_get_client_pool(struct snd_seq_client *client,
 		info->input_pool = 0;
 		info->input_free = 0;
 	}
-	snd_seq_client_unlock(cptr);
 	
 	return 0;
 }
@@ -1951,7 +1927,7 @@ static int snd_seq_ioctl_get_subscription(struct snd_seq_client *client,
 {
 	struct snd_seq_port_subscribe *subs = arg;
 	int result;
-	struct snd_seq_client *sender = NULL;
+	struct snd_seq_client *sender __free(snd_seq_client) = NULL;
 	struct snd_seq_client_port *sport = NULL;
 
 	result = -EINVAL;
@@ -1966,8 +1942,6 @@ static int snd_seq_ioctl_get_subscription(struct snd_seq_client *client,
       __end:
       	if (sport)
 		snd_seq_port_unlock(sport);
-	if (sender)
-		snd_seq_client_unlock(sender);
 
 	return result;
 }
@@ -1980,7 +1954,7 @@ static int snd_seq_ioctl_query_subs(struct snd_seq_client *client, void *arg)
 {
 	struct snd_seq_query_subs *subs = arg;
 	int result = -ENXIO;
-	struct snd_seq_client *cptr = NULL;
+	struct snd_seq_client *cptr __free(snd_seq_client) = NULL;
 	struct snd_seq_client_port *port = NULL;
 	struct snd_seq_port_subs_info *group;
 	struct list_head *p;
@@ -2031,8 +2005,6 @@ static int snd_seq_ioctl_query_subs(struct snd_seq_client *client, void *arg)
       __end:
    	if (port)
 		snd_seq_port_unlock(port);
-	if (cptr)
-		snd_seq_client_unlock(cptr);
 
 	return result;
 }
@@ -2045,7 +2017,7 @@ static int snd_seq_ioctl_query_next_client(struct snd_seq_client *client,
 					   void *arg)
 {
 	struct snd_seq_client_info *info = arg;
-	struct snd_seq_client *cptr = NULL;
+	struct snd_seq_client *cptr __free(snd_seq_client) = NULL;
 
 	/* search for next client */
 	if (info->client < INT_MAX)
@@ -2054,16 +2026,12 @@ static int snd_seq_ioctl_query_next_client(struct snd_seq_client *client,
 		info->client = 0;
 	for (; info->client < SNDRV_SEQ_MAX_CLIENTS; info->client++) {
 		cptr = client_load_and_use_ptr(info->client);
-		if (cptr)
-			break; /* found */
+		if (cptr) {
+			get_client_info(cptr, info);
+			return 0; /* found */
+		}
 	}
-	if (cptr == NULL)
-		return -ENOENT;
-
-	get_client_info(cptr, info);
-	snd_seq_client_unlock(cptr);
-
-	return 0;
+	return -ENOENT;
 }
 
 /* 
@@ -2073,7 +2041,7 @@ static int snd_seq_ioctl_query_next_port(struct snd_seq_client *client,
 					 void *arg)
 {
 	struct snd_seq_port_info *info = arg;
-	struct snd_seq_client *cptr;
+	struct snd_seq_client *cptr __free(snd_seq_client) = NULL;
 	struct snd_seq_client_port *port = NULL;
 
 	cptr = client_load_and_use_ptr(info->addr.client);
@@ -2083,16 +2051,13 @@ static int snd_seq_ioctl_query_next_port(struct snd_seq_client *client,
 	/* search for next port */
 	info->addr.port++;
 	port = snd_seq_port_query_nearest(cptr, info);
-	if (port == NULL) {
-		snd_seq_client_unlock(cptr);
+	if (port == NULL)
 		return -ENOENT;
-	}
 
 	/* get port info */
 	info->addr = port->addr;
 	snd_seq_get_port_info(port, info);
 	snd_seq_port_unlock(port);
-	snd_seq_client_unlock(cptr);
 
 	return 0;
 }
@@ -2157,7 +2122,7 @@ static int snd_seq_ioctl_client_ump_info(struct snd_seq_client *caller,
 {
 	struct snd_seq_client_ump_info __user *argp =
 		(struct snd_seq_client_ump_info __user *)arg;
-	struct snd_seq_client *cptr;
+	struct snd_seq_client *cptr __free(snd_seq_client) = NULL;
 	int client, type, err = 0;
 	size_t size;
 	void *p;
@@ -2180,7 +2145,7 @@ static int snd_seq_ioctl_client_ump_info(struct snd_seq_client *caller,
 	scoped_guard(mutex, &cptr->ioctl_mutex) {
 		if (!cptr->midi_version) {
 			err = -EBADFD;
-			goto error;
+			break;
 		}
 
 		if (cmd == SNDRV_SEQ_IOCTL_GET_CLIENT_UMP_INFO) {
@@ -2190,38 +2155,36 @@ static int snd_seq_ioctl_client_ump_info(struct snd_seq_client *caller,
 				p = cptr->ump_info[type];
 			if (!p) {
 				err = -ENODEV;
-				goto error;
+				break;
 			}
 			if (copy_to_user(argp->info, p, size)) {
 				err = -EFAULT;
-				goto error;
+				break;
 			}
 		} else {
 			if (cptr->type != USER_CLIENT) {
 				err = -EBADFD;
-				goto error;
+				break;
 			}
 			if (!cptr->ump_info) {
 				cptr->ump_info = kcalloc(NUM_UMP_INFOS,
 							 sizeof(void *), GFP_KERNEL);
 				if (!cptr->ump_info) {
 					err = -ENOMEM;
-					goto error;
+					break;
 				}
 			}
 			p = memdup_user(argp->info, size);
 			if (IS_ERR(p)) {
 				err = PTR_ERR(p);
-				goto error;
+				break;
 			}
 			kfree(cptr->ump_info[type]);
 			terminate_ump_info_strings(p, type);
 			cptr->ump_info[type] = p;
 		}
-	}
 
- error:
-	snd_seq_client_unlock(cptr);
+	}
 	if (!err && cmd == SNDRV_SEQ_IOCTL_SET_CLIENT_UMP_INFO) {
 		if (type == SNDRV_SEQ_CLIENT_UMP_INFO_ENDPOINT)
 			snd_seq_system_ump_notify(client, 0,
@@ -2434,8 +2397,7 @@ EXPORT_SYMBOL(snd_seq_delete_kernel_client);
 int snd_seq_kernel_client_enqueue(int client, struct snd_seq_event *ev,
 				  struct file *file, bool blocking)
 {
-	struct snd_seq_client *cptr;
-	int result;
+	struct snd_seq_client *cptr __free(snd_seq_client) = NULL;
 
 	if (snd_BUG_ON(!ev))
 		return -EINVAL;
@@ -2458,16 +2420,13 @@ int snd_seq_kernel_client_enqueue(int client, struct snd_seq_event *ev,
 		return -EINVAL;
 	
 	if (!cptr->accept_output) {
-		result = -EPERM;
+		return -EPERM;
 	} else { /* send it */
 		guard(mutex)(&cptr->ioctl_mutex);
-		result = snd_seq_client_enqueue_event(cptr, ev, file, blocking,
-						      false, 0,
-						      &cptr->ioctl_mutex);
+		return snd_seq_client_enqueue_event(cptr, ev, file, blocking,
+						    false, 0,
+						    &cptr->ioctl_mutex);
 	}
-
-	snd_seq_client_unlock(cptr);
-	return result;
 }
 EXPORT_SYMBOL(snd_seq_kernel_client_enqueue);
 
@@ -2481,8 +2440,7 @@ EXPORT_SYMBOL(snd_seq_kernel_client_enqueue);
 int snd_seq_kernel_client_dispatch(int client, struct snd_seq_event * ev,
 				   int atomic, int hop)
 {
-	struct snd_seq_client *cptr;
-	int result;
+	struct snd_seq_client *cptr __free(snd_seq_client) = NULL;
 
 	if (snd_BUG_ON(!ev))
 		return -EINVAL;
@@ -2499,12 +2457,9 @@ int snd_seq_kernel_client_dispatch(int client, struct snd_seq_event * ev,
 		return -EINVAL;
 
 	if (!cptr->accept_output)
-		result = -EPERM;
+		return -EPERM;
 	else
-		result = snd_seq_deliver_event(cptr, ev, atomic, hop);
-
-	snd_seq_client_unlock(cptr);
-	return result;
+		return snd_seq_deliver_event(cptr, ev, atomic, hop);
 }
 EXPORT_SYMBOL(snd_seq_kernel_client_dispatch);
 
@@ -2550,17 +2505,13 @@ EXPORT_SYMBOL(snd_seq_kernel_client_ctl);
 /* a similar like above but taking locks; used only from OSS sequencer layer */
 int snd_seq_kernel_client_ioctl(int clientid, unsigned int cmd, void *arg)
 {
-	struct snd_seq_client *client;
-	int ret;
+	struct snd_seq_client *client __free(snd_seq_client) = NULL;
 
 	client = client_load_and_use_ptr(clientid);
 	if (!client)
 		return -ENXIO;
-	scoped_guard(mutex, &client->ioctl_mutex) {
-		ret = call_seq_client_ctl(client, cmd, arg);
-	}
-	snd_seq_client_unlock(client);
-	return ret;
+	guard(mutex)(&client->ioctl_mutex);
+	return call_seq_client_ctl(client, cmd, arg);
 }
 EXPORT_SYMBOL_GPL(snd_seq_kernel_client_ioctl);
 
@@ -2590,7 +2541,7 @@ EXPORT_SYMBOL_GPL(snd_seq_kernel_client_get);
 void snd_seq_kernel_client_put(struct snd_seq_client *cptr)
 {
 	if (cptr)
-		snd_seq_client_unlock(cptr);
+		snd_seq_client_unref(cptr);
 }
 EXPORT_SYMBOL_GPL(snd_seq_kernel_client_put);
 
@@ -2692,7 +2643,6 @@ void snd_seq_info_clients_read(struct snd_info_entry *entry,
 			       struct snd_info_buffer *buffer)
 {
 	int c;
-	struct snd_seq_client *client;
 
 	snd_iprintf(buffer, "Client info\n");
 	snd_iprintf(buffer, "  cur  clients : %d\n", client_usage.cur);
@@ -2702,15 +2652,15 @@ void snd_seq_info_clients_read(struct snd_info_entry *entry,
 
 	/* list the client table */
 	for (c = 0; c < SNDRV_SEQ_MAX_CLIENTS; c++) {
+		struct snd_seq_client *client __free(snd_seq_client) = NULL;
+
 		client = client_load_and_use_ptr(c);
 		if (client == NULL)
 			continue;
-		if (client->type == NO_CLIENT) {
-			snd_seq_client_unlock(client);
+		if (client->type == NO_CLIENT)
 			continue;
-		}
 
-		mutex_lock(&client->ioctl_mutex);
+		guard(mutex)(&client->ioctl_mutex);
 		snd_iprintf(buffer, "Client %3d : \"%s\" [%s %s]\n",
 			    c, client->name,
 			    client->type == USER_CLIENT ? "User" : "Kernel",
@@ -2728,8 +2678,6 @@ void snd_seq_info_clients_read(struct snd_info_entry *entry,
 			snd_iprintf(buffer, "  Input pool :\n");
 			snd_seq_info_pool(buffer, client->data.user.fifo->pool, "    ");
 		}
-		mutex_unlock(&client->ioctl_mutex);
-		snd_seq_client_unlock(client);
 	}
 }
 #endif /* CONFIG_SND_PROC_FS */
diff --git a/sound/core/seq/seq_clientmgr.h b/sound/core/seq/seq_clientmgr.h
index b42afb887daf..ece02c58db70 100644
--- a/sound/core/seq/seq_clientmgr.h
+++ b/sound/core/seq/seq_clientmgr.h
@@ -91,7 +91,7 @@ static inline void snd_seq_client_unref(struct snd_seq_client *client)
 	snd_use_lock_free(&client->use_lock);
 }
 
-#define snd_seq_client_unlock(c)	snd_seq_client_unref(c)
+DEFINE_FREE(snd_seq_client, struct snd_seq_client *, if (!IS_ERR_OR_NULL(_T)) snd_seq_client_unref(_T))
 
 /* dispatch event to client(s) */
 int snd_seq_dispatch_event(struct snd_seq_event_cell *cell, int atomic, int hop);
diff --git a/sound/core/seq/seq_ports.c b/sound/core/seq/seq_ports.c
index cc2f8e846584..446d67c0fd67 100644
--- a/sound/core/seq/seq_ports.c
+++ b/sound/core/seq/seq_ports.c
@@ -178,17 +178,10 @@ static int unsubscribe_port(struct snd_seq_client *client,
 static struct snd_seq_client_port *get_client_port(struct snd_seq_addr *addr,
 						   struct snd_seq_client **cp)
 {
-	struct snd_seq_client_port *p;
 	*cp = snd_seq_client_use_ptr(addr->client);
-	if (*cp) {
-		p = snd_seq_port_use_ptr(*cp, addr->port);
-		if (! p) {
-			snd_seq_client_unlock(*cp);
-			*cp = NULL;
-		}
-		return p;
-	}
-	return NULL;
+	if (!*cp)
+		return NULL;
+	return snd_seq_port_use_ptr(*cp, addr->port);
 }
 
 static void delete_and_unsubscribe_port(struct snd_seq_client *client,
@@ -218,7 +211,7 @@ static void clear_subscriber_list(struct snd_seq_client *client,
 
 	list_for_each_safe(p, n, &grp->list_head) {
 		struct snd_seq_subscribers *subs;
-		struct snd_seq_client *c;
+		struct snd_seq_client *c __free(snd_seq_client) = NULL;
 		struct snd_seq_client_port *aport;
 
 		subs = get_subscriber(p, is_src);
@@ -242,7 +235,6 @@ static void clear_subscriber_list(struct snd_seq_client *client,
 		delete_and_unsubscribe_port(c, aport, subs, !is_src, true);
 		kfree(subs);
 		snd_seq_port_unlock(aport);
-		snd_seq_client_unlock(c);
 	}
 }
 
-- 
2.50.1


^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [PATCH v2 05/11] ALSA: seq: Clean up port locking with auto cleanup
  2025-08-27  8:05 [PATCH v2 00/11] ALSA: seq: Use auto-cleanup macros Takashi Iwai
                   ` (3 preceding siblings ...)
  2025-08-27  8:05 ` [PATCH v2 04/11] ALSA: seq: Use auto-cleanup for client refcounting Takashi Iwai
@ 2025-08-27  8:05 ` Takashi Iwai
  2025-08-27  8:05 ` [PATCH v2 06/11] ALSA: seq: Clean up queue " Takashi Iwai
                   ` (5 subsequent siblings)
  10 siblings, 0 replies; 12+ messages in thread
From: Takashi Iwai @ 2025-08-27  8:05 UTC (permalink / raw)
  To: linux-sound

Like the previous change in seq_clientmgr.c, introduce a new
auto-cleanup macro for the snd_seq_port_unlock(), and apply it
appropriately.

Only code refactoring, and no behavior change.

Signed-off-by: Takashi Iwai <tiwai@suse.de>
---
 sound/core/seq/seq_clientmgr.c | 141 ++++++++++++---------------------
 sound/core/seq/seq_ports.c     |   3 +-
 sound/core/seq/seq_ports.h     |   2 +
 3 files changed, 53 insertions(+), 93 deletions(-)

diff --git a/sound/core/seq/seq_clientmgr.c b/sound/core/seq/seq_clientmgr.c
index 5b14d70ba87a..18424f56251a 100644
--- a/sound/core/seq/seq_clientmgr.c
+++ b/sound/core/seq/seq_clientmgr.c
@@ -612,21 +612,18 @@ static int _snd_seq_deliver_single_event(struct snd_seq_client *client,
 					 int atomic, int hop)
 {
 	struct snd_seq_client *dest __free(snd_seq_client) = NULL;
-	struct snd_seq_client_port *dest_port = NULL;
-	int result = -ENOENT;
+	struct snd_seq_client_port *dest_port __free(snd_seq_port) = NULL;
 
 	dest = get_event_dest_client(event);
 	if (dest == NULL)
-		goto __skip;
+		return -ENOENT;
 	dest_port = snd_seq_port_use_ptr(dest, event->dest.port);
 	if (dest_port == NULL)
-		goto __skip;
+		return -ENOENT;
 
 	/* check permission */
-	if (! check_port_perm(dest_port, SNDRV_SEQ_PORT_CAP_WRITE)) {
-		result = -EPERM;
-		goto __skip;
-	}
+	if (!check_port_perm(dest_port, SNDRV_SEQ_PORT_CAP_WRITE))
+		return -EPERM;
 
 	if (dest_port->timestamping)
 		update_timestamp_of_queue(event, dest_port->time_queue,
@@ -634,31 +631,21 @@ static int _snd_seq_deliver_single_event(struct snd_seq_client *client,
 
 #if IS_ENABLED(CONFIG_SND_SEQ_UMP)
 	if (snd_seq_ev_is_ump(event)) {
-		if (!(dest->filter & SNDRV_SEQ_FILTER_NO_CONVERT)) {
-			result = snd_seq_deliver_from_ump(client, dest, dest_port,
-							  event, atomic, hop);
-			goto __skip;
-		} else if (dest->type == USER_CLIENT &&
-			   !snd_seq_client_is_ump(dest)) {
-			result = 0; // drop the event
-			goto __skip;
-		}
-	} else if (snd_seq_client_is_ump(dest)) {
-		if (!(dest->filter & SNDRV_SEQ_FILTER_NO_CONVERT)) {
-			result = snd_seq_deliver_to_ump(client, dest, dest_port,
+		if (!(dest->filter & SNDRV_SEQ_FILTER_NO_CONVERT))
+			return snd_seq_deliver_from_ump(client, dest, dest_port,
 							event, atomic, hop);
-			goto __skip;
-		}
+		else if (dest->type == USER_CLIENT &&
+			 !snd_seq_client_is_ump(dest))
+			return 0; // drop the event
+	} else if (snd_seq_client_is_ump(dest)) {
+		if (!(dest->filter & SNDRV_SEQ_FILTER_NO_CONVERT))
+			return snd_seq_deliver_to_ump(client, dest, dest_port,
+						      event, atomic, hop);
 	}
 #endif /* CONFIG_SND_SEQ_UMP */
 
-	result = __snd_seq_deliver_single_event(dest, dest_port, event,
-						atomic, hop);
-
-  __skip:
-	if (dest_port)
-		snd_seq_port_unlock(dest_port);
-	return result;
+	return __snd_seq_deliver_single_event(dest, dest_port, event,
+					      atomic, hop);
 }
 
 /*
@@ -687,7 +674,7 @@ static int __deliver_to_subscribers(struct snd_seq_client *client,
 				    struct snd_seq_event *event,
 				    int port, int atomic, int hop)
 {
-	struct snd_seq_client_port *src_port;
+	struct snd_seq_client_port *src_port __free(snd_seq_port) = NULL;
 	struct snd_seq_subscribers *subs;
 	int err, result = 0, num_ev = 0;
 	union __snd_seq_event event_saved;
@@ -734,7 +721,6 @@ static int __deliver_to_subscribers(struct snd_seq_client *client,
 		read_unlock(&grp->list_lock);
 	else
 		up_read(&grp->list_mutex);
-	snd_seq_port_unlock(src_port);
 	memcpy(event, &event_saved, saved_size);
 	return (result < 0) ? result : num_ev;
 }
@@ -902,10 +888,10 @@ static int snd_seq_client_enqueue_event(struct snd_seq_client *client,
 		event->queue = SNDRV_SEQ_QUEUE_DIRECT;
 	} else if (event->dest.client == SNDRV_SEQ_ADDRESS_SUBSCRIBERS) {
 		/* check presence of source port */
-		struct snd_seq_client_port *src_port = snd_seq_port_use_ptr(client, event->source.port);
-		if (src_port == NULL)
+		struct snd_seq_client_port *src_port __free(snd_seq_port) =
+			snd_seq_port_use_ptr(client, event->source.port);
+		if (!src_port)
 			return -EINVAL;
-		snd_seq_port_unlock(src_port);
 	}
 
 	/* direct event processing without enqueued */
@@ -1361,7 +1347,7 @@ static int snd_seq_ioctl_get_port_info(struct snd_seq_client *client, void *arg)
 {
 	struct snd_seq_port_info *info = arg;
 	struct snd_seq_client *cptr __free(snd_seq_client) = NULL;
-	struct snd_seq_client_port *port;
+	struct snd_seq_client_port *port __free(snd_seq_port) = NULL;
 
 	cptr = client_load_and_use_ptr(info->addr.client);
 	if (cptr == NULL)
@@ -1373,7 +1359,6 @@ static int snd_seq_ioctl_get_port_info(struct snd_seq_client *client, void *arg)
 
 	/* get port info */
 	snd_seq_get_port_info(port, info);
-	snd_seq_port_unlock(port);
 	return 0;
 }
 
@@ -1384,14 +1369,13 @@ static int snd_seq_ioctl_get_port_info(struct snd_seq_client *client, void *arg)
 static int snd_seq_ioctl_set_port_info(struct snd_seq_client *client, void *arg)
 {
 	struct snd_seq_port_info *info = arg;
-	struct snd_seq_client_port *port;
+	struct snd_seq_client_port *port __free(snd_seq_port) = NULL;
 
 	if (info->addr.client != client->number) /* only set our own ports ! */
 		return -EPERM;
 	port = snd_seq_port_use_ptr(client, info->addr.port);
 	if (port) {
 		snd_seq_set_port_info(port, info);
-		snd_seq_port_unlock(port);
 		/* notify the change */
 		snd_seq_system_client_ev_port_change(info->addr.client,
 						     info->addr.port);
@@ -1461,39 +1445,35 @@ int snd_seq_client_notify_subscription(int client, int port,
 static int snd_seq_ioctl_subscribe_port(struct snd_seq_client *client,
 					void *arg)
 {
-	int result = -EINVAL;
 	struct snd_seq_port_subscribe *subs = arg;
 	struct snd_seq_client *receiver __free(snd_seq_client) = NULL;
 	struct snd_seq_client *sender __free(snd_seq_client) = NULL;
-	struct snd_seq_client_port *sport = NULL, *dport = NULL;
+	struct snd_seq_client_port *sport __free(snd_seq_port) = NULL;
+	struct snd_seq_client_port *dport __free(snd_seq_port) = NULL;
+	int result;
 
 	receiver = client_load_and_use_ptr(subs->dest.client);
 	if (!receiver)
-		goto __end;
+		return -EINVAL;
 	sender = client_load_and_use_ptr(subs->sender.client);
 	if (!sender)
-		goto __end;
+		return -EINVAL;
 	sport = snd_seq_port_use_ptr(sender, subs->sender.port);
 	if (!sport)
-		goto __end;
+		return -EINVAL;
 	dport = snd_seq_port_use_ptr(receiver, subs->dest.port);
 	if (!dport)
-		goto __end;
+		return -EINVAL;
 
 	result = check_subscription_permission(client, sport, dport, subs);
 	if (result < 0)
-		goto __end;
+		return result;
 
 	/* connect them */
 	result = snd_seq_port_connect(client, sender, sport, receiver, dport, subs);
 	if (! result) /* broadcast announce */
 		snd_seq_client_notify_subscription(SNDRV_SEQ_ADDRESS_SUBSCRIBERS, 0,
 						   subs, SNDRV_SEQ_EVENT_PORT_SUBSCRIBED);
-      __end:
-      	if (sport)
-		snd_seq_port_unlock(sport);
-	if (dport)
-		snd_seq_port_unlock(dport);
 	return result;
 }
 
@@ -1504,38 +1484,34 @@ static int snd_seq_ioctl_subscribe_port(struct snd_seq_client *client,
 static int snd_seq_ioctl_unsubscribe_port(struct snd_seq_client *client,
 					  void *arg)
 {
-	int result = -ENXIO;
 	struct snd_seq_port_subscribe *subs = arg;
 	struct snd_seq_client *receiver __free(snd_seq_client) = NULL;
 	struct snd_seq_client *sender __free(snd_seq_client) = NULL;
-	struct snd_seq_client_port *sport = NULL, *dport = NULL;
+	struct snd_seq_client_port *sport __free(snd_seq_port) = NULL;
+	struct snd_seq_client_port *dport __free(snd_seq_port) = NULL;
+	int result;
 
 	receiver = snd_seq_client_use_ptr(subs->dest.client);
 	if (!receiver)
-		goto __end;
+		return -ENXIO;
 	sender = snd_seq_client_use_ptr(subs->sender.client);
 	if (!sender)
-		goto __end;
+		return -ENXIO;
 	sport = snd_seq_port_use_ptr(sender, subs->sender.port);
 	if (!sport)
-		goto __end;
+		return -ENXIO;
 	dport = snd_seq_port_use_ptr(receiver, subs->dest.port);
 	if (!dport)
-		goto __end;
+		return -ENXIO;
 
 	result = check_subscription_permission(client, sport, dport, subs);
 	if (result < 0)
-		goto __end;
+		return result;
 
 	result = snd_seq_port_disconnect(client, sender, sport, receiver, dport, subs);
 	if (! result) /* broadcast announce */
 		snd_seq_client_notify_subscription(SNDRV_SEQ_ADDRESS_SUBSCRIBERS, 0,
 						   subs, SNDRV_SEQ_EVENT_PORT_UNSUBSCRIBED);
-      __end:
-      	if (sport)
-		snd_seq_port_unlock(sport);
-	if (dport)
-		snd_seq_port_unlock(dport);
 	return result;
 }
 
@@ -1926,24 +1902,16 @@ static int snd_seq_ioctl_get_subscription(struct snd_seq_client *client,
 					  void *arg)
 {
 	struct snd_seq_port_subscribe *subs = arg;
-	int result;
 	struct snd_seq_client *sender __free(snd_seq_client) = NULL;
-	struct snd_seq_client_port *sport = NULL;
+	struct snd_seq_client_port *sport __free(snd_seq_port) = NULL;
 
-	result = -EINVAL;
 	sender = client_load_and_use_ptr(subs->sender.client);
 	if (!sender)
-		goto __end;
+		return -EINVAL;
 	sport = snd_seq_port_use_ptr(sender, subs->sender.port);
 	if (!sport)
-		goto __end;
-	result = snd_seq_port_get_subscription(&sport->c_src, &subs->dest,
-					       subs);
-      __end:
-      	if (sport)
-		snd_seq_port_unlock(sport);
-
-	return result;
+		return -EINVAL;
+	return snd_seq_port_get_subscription(&sport->c_src, &subs->dest, subs);
 }
 
 
@@ -1953,19 +1921,18 @@ static int snd_seq_ioctl_get_subscription(struct snd_seq_client *client,
 static int snd_seq_ioctl_query_subs(struct snd_seq_client *client, void *arg)
 {
 	struct snd_seq_query_subs *subs = arg;
-	int result = -ENXIO;
 	struct snd_seq_client *cptr __free(snd_seq_client) = NULL;
-	struct snd_seq_client_port *port = NULL;
+	struct snd_seq_client_port *port __free(snd_seq_port) = NULL;
 	struct snd_seq_port_subs_info *group;
 	struct list_head *p;
 	int i;
 
 	cptr = client_load_and_use_ptr(subs->root.client);
 	if (!cptr)
-		goto __end;
+		return -ENXIO;
 	port = snd_seq_port_use_ptr(cptr, subs->root.port);
 	if (!port)
-		goto __end;
+		return -ENXIO;
 
 	switch (subs->type) {
 	case SNDRV_SEQ_QUERY_SUBS_READ:
@@ -1975,14 +1942,13 @@ static int snd_seq_ioctl_query_subs(struct snd_seq_client *client, void *arg)
 		group = &port->c_dest;
 		break;
 	default:
-		goto __end;
+		return -ENXIO;
 	}
 
-	down_read(&group->list_mutex);
+	guard(rwsem_read)(&group->list_mutex);
 	/* search for the subscriber */
 	subs->num_subs = group->count;
 	i = 0;
-	result = -ENOENT;
 	list_for_each(p, &group->list_head) {
 		if (i++ == subs->index) {
 			/* found! */
@@ -1996,17 +1962,11 @@ static int snd_seq_ioctl_query_subs(struct snd_seq_client *client, void *arg)
 			}
 			subs->flags = s->info.flags;
 			subs->queue = s->info.queue;
-			result = 0;
-			break;
+			return 0;
 		}
 	}
-	up_read(&group->list_mutex);
 
-      __end:
-   	if (port)
-		snd_seq_port_unlock(port);
-
-	return result;
+	return -ENOENT;
 }
 
 
@@ -2042,7 +2002,7 @@ static int snd_seq_ioctl_query_next_port(struct snd_seq_client *client,
 {
 	struct snd_seq_port_info *info = arg;
 	struct snd_seq_client *cptr __free(snd_seq_client) = NULL;
-	struct snd_seq_client_port *port = NULL;
+	struct snd_seq_client_port *port __free(snd_seq_port) = NULL;
 
 	cptr = client_load_and_use_ptr(info->addr.client);
 	if (cptr == NULL)
@@ -2057,7 +2017,6 @@ static int snd_seq_ioctl_query_next_port(struct snd_seq_client *client,
 	/* get port info */
 	info->addr = port->addr;
 	snd_seq_get_port_info(port, info);
-	snd_seq_port_unlock(port);
 
 	return 0;
 }
diff --git a/sound/core/seq/seq_ports.c b/sound/core/seq/seq_ports.c
index 446d67c0fd67..40fa379847e5 100644
--- a/sound/core/seq/seq_ports.c
+++ b/sound/core/seq/seq_ports.c
@@ -212,7 +212,7 @@ static void clear_subscriber_list(struct snd_seq_client *client,
 	list_for_each_safe(p, n, &grp->list_head) {
 		struct snd_seq_subscribers *subs;
 		struct snd_seq_client *c __free(snd_seq_client) = NULL;
-		struct snd_seq_client_port *aport;
+		struct snd_seq_client_port *aport __free(snd_seq_port) = NULL;
 
 		subs = get_subscriber(p, is_src);
 		if (is_src)
@@ -234,7 +234,6 @@ static void clear_subscriber_list(struct snd_seq_client *client,
 		/* ok we got the connected port */
 		delete_and_unsubscribe_port(c, aport, subs, !is_src, true);
 		kfree(subs);
-		snd_seq_port_unlock(aport);
 	}
 }
 
diff --git a/sound/core/seq/seq_ports.h b/sound/core/seq/seq_ports.h
index b3b35018cb82..40ed6cf7cb90 100644
--- a/sound/core/seq/seq_ports.h
+++ b/sound/core/seq/seq_ports.h
@@ -96,6 +96,8 @@ struct snd_seq_client_port *snd_seq_port_query_nearest(struct snd_seq_client *cl
 /* unlock the port */
 #define snd_seq_port_unlock(port) snd_use_lock_free(&(port)->use_lock)
 
+DEFINE_FREE(snd_seq_port, struct snd_seq_client_port *, if (!IS_ERR_OR_NULL(_T)) snd_seq_port_unlock(_T))
+
 /* create a port, port number or a negative error code is returned */
 int snd_seq_create_port(struct snd_seq_client *client, int port_index,
 			struct snd_seq_client_port **port_ret);
-- 
2.50.1


^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [PATCH v2 06/11] ALSA: seq: Clean up queue locking with auto cleanup
  2025-08-27  8:05 [PATCH v2 00/11] ALSA: seq: Use auto-cleanup macros Takashi Iwai
                   ` (4 preceding siblings ...)
  2025-08-27  8:05 ` [PATCH v2 05/11] ALSA: seq: Clean up port locking with auto cleanup Takashi Iwai
@ 2025-08-27  8:05 ` Takashi Iwai
  2025-08-27  8:05 ` [PATCH v2 07/11] ALSA: seq: Clean up fifo locking with guard Takashi Iwai
                   ` (4 subsequent siblings)
  10 siblings, 0 replies; 12+ messages in thread
From: Takashi Iwai @ 2025-08-27  8:05 UTC (permalink / raw)
  To: linux-sound

Yet more cleanup with the auto-cleanup macro: now we replace the
queuefree() calls with the magic pointer attribute
__free(snd_seq_queue).

Only code refactoring, and no behavior change.

Signed-off-by: Takashi Iwai <tiwai@suse.de>
---
 sound/core/seq/seq_clientmgr.c | 37 ++++++-----------
 sound/core/seq/seq_queue.c     | 76 ++++++++++------------------------
 sound/core/seq/seq_queue.h     |  2 +
 sound/core/seq/seq_timer.c     |  5 +--
 4 files changed, 39 insertions(+), 81 deletions(-)

diff --git a/sound/core/seq/seq_clientmgr.c b/sound/core/seq/seq_clientmgr.c
index 18424f56251a..38ad5bbd2706 100644
--- a/sound/core/seq/seq_clientmgr.c
+++ b/sound/core/seq/seq_clientmgr.c
@@ -566,7 +566,7 @@ static int bounce_error_event(struct snd_seq_client *client,
 static int update_timestamp_of_queue(struct snd_seq_event *event,
 				     int queue, int real_time)
 {
-	struct snd_seq_queue *q;
+	struct snd_seq_queue *q __free(snd_seq_queue) = NULL;
 
 	q = queueptr(queue);
 	if (! q)
@@ -580,7 +580,6 @@ static int update_timestamp_of_queue(struct snd_seq_event *event,
 		event->time.tick = snd_seq_timer_get_cur_tick(q->timer);
 		event->flags |= SNDRV_SEQ_TIME_STAMP_TICK;
 	}
-	queuefree(q);
 	return 1;
 }
 
@@ -1520,7 +1519,7 @@ static int snd_seq_ioctl_unsubscribe_port(struct snd_seq_client *client,
 static int snd_seq_ioctl_create_queue(struct snd_seq_client *client, void *arg)
 {
 	struct snd_seq_queue_info *info = arg;
-	struct snd_seq_queue *q;
+	struct snd_seq_queue *q __free(snd_seq_queue) = NULL;
 
 	q = snd_seq_queue_alloc(client->number, info->locked, info->flags);
 	if (IS_ERR(q))
@@ -1534,7 +1533,6 @@ static int snd_seq_ioctl_create_queue(struct snd_seq_client *client, void *arg)
 	if (!info->name[0])
 		snprintf(info->name, sizeof(info->name), "Queue-%d", q->queue);
 	strscpy(q->name, info->name, sizeof(q->name));
-	snd_use_lock_free(&q->use_lock);
 
 	return 0;
 }
@@ -1552,7 +1550,7 @@ static int snd_seq_ioctl_get_queue_info(struct snd_seq_client *client,
 					void *arg)
 {
 	struct snd_seq_queue_info *info = arg;
-	struct snd_seq_queue *q;
+	struct snd_seq_queue *q __free(snd_seq_queue) = NULL;
 
 	q = queueptr(info->queue);
 	if (q == NULL)
@@ -1563,7 +1561,6 @@ static int snd_seq_ioctl_get_queue_info(struct snd_seq_client *client,
 	info->owner = q->owner;
 	info->locked = q->locked;
 	strscpy(info->name, q->name, sizeof(info->name));
-	queuefree(q);
 
 	return 0;
 }
@@ -1573,7 +1570,7 @@ static int snd_seq_ioctl_set_queue_info(struct snd_seq_client *client,
 					void *arg)
 {
 	struct snd_seq_queue_info *info = arg;
-	struct snd_seq_queue *q;
+	struct snd_seq_queue *q __free(snd_seq_queue) = NULL;
 
 	if (info->owner != client->number)
 		return -EINVAL;
@@ -1591,12 +1588,9 @@ static int snd_seq_ioctl_set_queue_info(struct snd_seq_client *client,
 	q = queueptr(info->queue);
 	if (! q)
 		return -EINVAL;
-	if (q->owner != client->number) {
-		queuefree(q);
+	if (q->owner != client->number)
 		return -EPERM;
-	}
 	strscpy(q->name, info->name, sizeof(q->name));
-	queuefree(q);
 
 	return 0;
 }
@@ -1606,7 +1600,7 @@ static int snd_seq_ioctl_get_named_queue(struct snd_seq_client *client,
 					 void *arg)
 {
 	struct snd_seq_queue_info *info = arg;
-	struct snd_seq_queue *q;
+	struct snd_seq_queue *q __free(snd_seq_queue) = NULL;
 
 	q = snd_seq_queue_find_name(info->name);
 	if (q == NULL)
@@ -1614,7 +1608,6 @@ static int snd_seq_ioctl_get_named_queue(struct snd_seq_client *client,
 	info->queue = q->queue;
 	info->owner = q->owner;
 	info->locked = q->locked;
-	queuefree(q);
 
 	return 0;
 }
@@ -1624,7 +1617,7 @@ static int snd_seq_ioctl_get_queue_status(struct snd_seq_client *client,
 					  void *arg)
 {
 	struct snd_seq_queue_status *status = arg;
-	struct snd_seq_queue *queue;
+	struct snd_seq_queue *queue __free(snd_seq_queue) = NULL;
 	struct snd_seq_timer *tmr;
 
 	queue = queueptr(status->queue);
@@ -1642,7 +1635,6 @@ static int snd_seq_ioctl_get_queue_status(struct snd_seq_client *client,
 	status->running = tmr->running;
 
 	status->flags = queue->flags;
-	queuefree(queue);
 
 	return 0;
 }
@@ -1653,7 +1645,7 @@ static int snd_seq_ioctl_get_queue_tempo(struct snd_seq_client *client,
 					 void *arg)
 {
 	struct snd_seq_queue_tempo *tempo = arg;
-	struct snd_seq_queue *queue;
+	struct snd_seq_queue *queue __free(snd_seq_queue) = NULL;
 	struct snd_seq_timer *tmr;
 
 	queue = queueptr(tempo->queue);
@@ -1670,7 +1662,6 @@ static int snd_seq_ioctl_get_queue_tempo(struct snd_seq_client *client,
 	tempo->skew_base = tmr->skew_base;
 	if (client->user_pversion >= SNDRV_PROTOCOL_VERSION(1, 0, 4))
 		tempo->tempo_base = tmr->tempo_base;
-	queuefree(queue);
 
 	return 0;
 }
@@ -1703,14 +1694,14 @@ static int snd_seq_ioctl_get_queue_timer(struct snd_seq_client *client,
 					 void *arg)
 {
 	struct snd_seq_queue_timer *timer = arg;
-	struct snd_seq_queue *queue;
+	struct snd_seq_queue *queue __free(snd_seq_queue) = NULL;
 	struct snd_seq_timer *tmr;
 
 	queue = queueptr(timer->queue);
 	if (queue == NULL)
 		return -EINVAL;
 
-	mutex_lock(&queue->timer_mutex);
+	guard(mutex)(&queue->timer_mutex);
 	tmr = queue->timer;
 	memset(timer, 0, sizeof(*timer));
 	timer->queue = queue->queue;
@@ -1720,8 +1711,6 @@ static int snd_seq_ioctl_get_queue_timer(struct snd_seq_client *client,
 		timer->u.alsa.id = tmr->alsa_id;
 		timer->u.alsa.resolution = tmr->preferred_resolution;
 	}
-	mutex_unlock(&queue->timer_mutex);
-	queuefree(queue);
 	
 	return 0;
 }
@@ -1738,13 +1727,13 @@ static int snd_seq_ioctl_set_queue_timer(struct snd_seq_client *client,
 		return -EINVAL;
 
 	if (snd_seq_queue_check_access(timer->queue, client->number)) {
-		struct snd_seq_queue *q;
+		struct snd_seq_queue *q __free(snd_seq_queue) = NULL;
 		struct snd_seq_timer *tmr;
 
 		q = queueptr(timer->queue);
 		if (q == NULL)
 			return -ENXIO;
-		mutex_lock(&q->timer_mutex);
+		guard(mutex)(&q->timer_mutex);
 		tmr = q->timer;
 		snd_seq_queue_timer_close(timer->queue);
 		tmr->type = timer->type;
@@ -1753,8 +1742,6 @@ static int snd_seq_ioctl_set_queue_timer(struct snd_seq_client *client,
 			tmr->preferred_resolution = timer->u.alsa.resolution;
 		}
 		result = snd_seq_queue_timer_open(timer->queue);
-		mutex_unlock(&q->timer_mutex);
-		queuefree(q);
 	} else {
 		return -EPERM;
 	}	
diff --git a/sound/core/seq/seq_queue.c b/sound/core/seq/seq_queue.c
index 10add922323d..f5c0e401c8ae 100644
--- a/sound/core/seq/seq_queue.c
+++ b/sound/core/seq/seq_queue.c
@@ -209,14 +209,13 @@ struct snd_seq_queue *queueptr(int queueid)
 struct snd_seq_queue *snd_seq_queue_find_name(char *name)
 {
 	int i;
-	struct snd_seq_queue *q;
 
 	for (i = 0; i < SNDRV_SEQ_MAX_QUEUES; i++) {
+		struct snd_seq_queue *q __free(snd_seq_queue) = NULL;
 		q = queueptr(i);
 		if (q) {
 			if (strncmp(q->name, name, sizeof(q->name)) == 0)
-				return q;
-			queuefree(q);
+				return no_free_ptr(q);
 		}
 	}
 	return NULL;
@@ -286,7 +285,7 @@ void snd_seq_check_queue(struct snd_seq_queue *q, int atomic, int hop)
 int snd_seq_enqueue_event(struct snd_seq_event_cell *cell, int atomic, int hop)
 {
 	int dest, err;
-	struct snd_seq_queue *q;
+	struct snd_seq_queue *q __free(snd_seq_queue) = NULL;
 
 	if (snd_BUG_ON(!cell))
 		return -EINVAL;
@@ -321,16 +320,12 @@ int snd_seq_enqueue_event(struct snd_seq_event_cell *cell, int atomic, int hop)
 		break;
 	}
 
-	if (err < 0) {
-		queuefree(q); /* unlock */
+	if (err < 0)
 		return err;
-	}
 
 	/* trigger dispatching */
 	snd_seq_check_queue(q, atomic, hop);
 
-	queuefree(q); /* unlock */
-
 	return 0;
 }
 
@@ -366,15 +361,12 @@ static inline void queue_access_unlock(struct snd_seq_queue *q)
 /* exported - only checking permission */
 int snd_seq_queue_check_access(int queueid, int client)
 {
-	struct snd_seq_queue *q = queueptr(queueid);
-	int access_ok;
+	struct snd_seq_queue *q __free(snd_seq_queue) = queueptr(queueid);
 
 	if (! q)
 		return 0;
-	scoped_guard(spinlock_irqsave, &q->owner_lock)
-		access_ok = check_access(q, client);
-	queuefree(q);
-	return access_ok;
+	guard(spinlock_irqsave)(&q->owner_lock);
+	return check_access(q, client);
 }
 
 /*----------------------------------------------------------------*/
@@ -384,22 +376,19 @@ int snd_seq_queue_check_access(int queueid, int client)
  */
 int snd_seq_queue_set_owner(int queueid, int client, int locked)
 {
-	struct snd_seq_queue *q = queueptr(queueid);
+	struct snd_seq_queue *q __free(snd_seq_queue) = queueptr(queueid);
 
 	if (q == NULL)
 		return -EINVAL;
 
-	if (! queue_access_lock(q, client)) {
-		queuefree(q);
+	if (!queue_access_lock(q, client))
 		return -EPERM;
-	}
 
 	scoped_guard(spinlock_irqsave, &q->owner_lock) {
 		q->locked = locked ? 1 : 0;
 		q->owner = client;
 	}
 	queue_access_unlock(q);
-	queuefree(q);
 
 	return 0;
 }
@@ -414,7 +403,7 @@ int snd_seq_queue_set_owner(int queueid, int client, int locked)
 int snd_seq_queue_timer_open(int queueid)
 {
 	int result = 0;
-	struct snd_seq_queue *queue;
+	struct snd_seq_queue *queue __free(snd_seq_queue) = NULL;
 	struct snd_seq_timer *tmr;
 
 	queue = queueptr(queueid);
@@ -426,7 +415,6 @@ int snd_seq_queue_timer_open(int queueid)
 		snd_seq_timer_defaults(tmr);
 		result = snd_seq_timer_open(queue);
 	}
-	queuefree(queue);
 	return result;
 }
 
@@ -435,14 +423,13 @@ int snd_seq_queue_timer_open(int queueid)
  */
 int snd_seq_queue_timer_close(int queueid)
 {
-	struct snd_seq_queue *queue;
+	struct snd_seq_queue *queue __free(snd_seq_queue) = NULL;
 	int result = 0;
 
 	queue = queueptr(queueid);
 	if (queue == NULL)
 		return -EINVAL;
 	snd_seq_timer_close(queue);
-	queuefree(queue);
 	return result;
 }
 
@@ -450,15 +437,13 @@ int snd_seq_queue_timer_close(int queueid)
 int snd_seq_queue_timer_set_tempo(int queueid, int client,
 				  struct snd_seq_queue_tempo *info)
 {
-	struct snd_seq_queue *q = queueptr(queueid);
+	struct snd_seq_queue *q __free(snd_seq_queue) = queueptr(queueid);
 	int result;
 
 	if (q == NULL)
 		return -EINVAL;
-	if (! queue_access_lock(q, client)) {
-		queuefree(q);
+	if (!queue_access_lock(q, client))
 		return -EPERM;
-	}
 
 	result = snd_seq_timer_set_tempo_ppq(q->timer, info->tempo, info->ppq,
 					     info->tempo_base);
@@ -466,7 +451,6 @@ int snd_seq_queue_timer_set_tempo(int queueid, int client,
 		result = snd_seq_timer_set_skew(q->timer, info->skew_value,
 						info->skew_base);
 	queue_access_unlock(q);
-	queuefree(q);
 	return result;
 }
 
@@ -495,15 +479,13 @@ static void queue_use(struct snd_seq_queue *queue, int client, int use)
  */
 int snd_seq_queue_use(int queueid, int client, int use)
 {
-	struct snd_seq_queue *queue;
+	struct snd_seq_queue *queue __free(snd_seq_queue) = NULL;
 
 	queue = queueptr(queueid);
 	if (queue == NULL)
 		return -EINVAL;
-	mutex_lock(&queue->timer_mutex);
+	guard(mutex)(&queue->timer_mutex);
 	queue_use(queue, client, use);
-	mutex_unlock(&queue->timer_mutex);
-	queuefree(queue);
 	return 0;
 }
 
@@ -514,15 +496,12 @@ int snd_seq_queue_use(int queueid, int client, int use)
  */
 int snd_seq_queue_is_used(int queueid, int client)
 {
-	struct snd_seq_queue *q;
-	int result;
+	struct snd_seq_queue *q __free(snd_seq_queue) = NULL;
 
 	q = queueptr(queueid);
 	if (q == NULL)
 		return -EINVAL; /* invalid queue */
-	result = test_bit(client, q->clients_bitmap) ? 1 : 0;
-	queuefree(q);
-	return result;
+	return test_bit(client, q->clients_bitmap) ? 1 : 0;
 }
 
 
@@ -535,11 +514,10 @@ int snd_seq_queue_is_used(int queueid, int client)
 void snd_seq_queue_client_leave(int client)
 {
 	int i;
-	struct snd_seq_queue *q;
 
 	/* delete own queues from queue list */
 	for (i = 0; i < SNDRV_SEQ_MAX_QUEUES; i++) {
-		q = queue_list_remove(i, client);
+		struct snd_seq_queue *q = queue_list_remove(i, client);
 		if (q)
 			queue_delete(q);
 	}
@@ -548,7 +526,7 @@ void snd_seq_queue_client_leave(int client)
 	 * they are not owned by this client
 	 */
 	for (i = 0; i < SNDRV_SEQ_MAX_QUEUES; i++) {
-		q = queueptr(i);
+		struct snd_seq_queue *q __free(snd_seq_queue) = queueptr(i);
 		if (!q)
 			continue;
 		if (test_bit(client, q->clients_bitmap)) {
@@ -556,7 +534,6 @@ void snd_seq_queue_client_leave(int client)
 			snd_seq_prioq_leave(q->timeq, client, 0);
 			snd_seq_queue_use(q->queue, client, 0);
 		}
-		queuefree(q);
 	}
 }
 
@@ -568,10 +545,9 @@ void snd_seq_queue_client_leave(int client)
 void snd_seq_queue_remove_cells(int client, struct snd_seq_remove_events *info)
 {
 	int i;
-	struct snd_seq_queue *q;
 
 	for (i = 0; i < SNDRV_SEQ_MAX_QUEUES; i++) {
-		q = queueptr(i);
+		struct snd_seq_queue *q __free(snd_seq_queue) = queueptr(i);
 		if (!q)
 			continue;
 		if (test_bit(client, q->clients_bitmap) &&
@@ -580,7 +556,6 @@ void snd_seq_queue_remove_cells(int client, struct snd_seq_remove_events *info)
 			snd_seq_prioq_remove_events(q->tickq, client, info);
 			snd_seq_prioq_remove_events(q->timeq, client, info);
 		}
-		queuefree(q);
 	}
 }
 
@@ -667,7 +642,7 @@ static void snd_seq_queue_process_event(struct snd_seq_queue *q,
  */
 int snd_seq_control_queue(struct snd_seq_event *ev, int atomic, int hop)
 {
-	struct snd_seq_queue *q;
+	struct snd_seq_queue *q __free(snd_seq_queue) = NULL;
 
 	if (snd_BUG_ON(!ev))
 		return -EINVAL;
@@ -676,15 +651,12 @@ int snd_seq_control_queue(struct snd_seq_event *ev, int atomic, int hop)
 	if (q == NULL)
 		return -EINVAL;
 
-	if (! queue_access_lock(q, ev->source.client)) {
-		queuefree(q);
+	if (!queue_access_lock(q, ev->source.client))
 		return -EPERM;
-	}
 
 	snd_seq_queue_process_event(q, ev, atomic, hop);
 
 	queue_access_unlock(q);
-	queuefree(q);
 	return 0;
 }
 
@@ -697,13 +669,12 @@ void snd_seq_info_queues_read(struct snd_info_entry *entry,
 			      struct snd_info_buffer *buffer)
 {
 	int i, bpm;
-	struct snd_seq_queue *q;
 	struct snd_seq_timer *tmr;
 	bool locked;
 	int owner;
 
 	for (i = 0; i < SNDRV_SEQ_MAX_QUEUES; i++) {
-		q = queueptr(i);
+		struct snd_seq_queue *q __free(snd_seq_queue) = queueptr(i);
 		if (!q)
 			continue;
 
@@ -731,7 +702,6 @@ void snd_seq_info_queues_read(struct snd_info_entry *entry,
 		snd_iprintf(buffer, "current time       : %d.%09d s\n", tmr->cur_time.tv_sec, tmr->cur_time.tv_nsec);
 		snd_iprintf(buffer, "current tick       : %d\n", tmr->tick.cur_tick);
 		snd_iprintf(buffer, "\n");
-		queuefree(q);
 	}
 }
 #endif /* CONFIG_SND_PROC_FS */
diff --git a/sound/core/seq/seq_queue.h b/sound/core/seq/seq_queue.h
index b81379c9af43..afcd3c5484a6 100644
--- a/sound/core/seq/seq_queue.h
+++ b/sound/core/seq/seq_queue.h
@@ -73,6 +73,8 @@ struct snd_seq_queue *queueptr(int queueid);
 /* unlock */
 #define queuefree(q) snd_use_lock_free(&(q)->use_lock)
 
+DEFINE_FREE(snd_seq_queue, struct snd_seq_queue *, if (!IS_ERR_OR_NULL(_T)) queuefree(_T))
+
 /* return the (first) queue matching with the specified name */
 struct snd_seq_queue *snd_seq_queue_find_name(char *name);
 
diff --git a/sound/core/seq/seq_timer.c b/sound/core/seq/seq_timer.c
index c9f0392ac7f1..29b018a212fc 100644
--- a/sound/core/seq/seq_timer.c
+++ b/sound/core/seq/seq_timer.c
@@ -440,13 +440,13 @@ void snd_seq_info_timer_read(struct snd_info_entry *entry,
 			     struct snd_info_buffer *buffer)
 {
 	int idx;
-	struct snd_seq_queue *q;
 	struct snd_seq_timer *tmr;
 	struct snd_timer_instance *ti;
 	unsigned long resolution;
 	
 	for (idx = 0; idx < SNDRV_SEQ_MAX_QUEUES; idx++) {
-		q = queueptr(idx);
+		struct snd_seq_queue *q __free(snd_seq_queue) = queueptr(idx);
+
 		if (q == NULL)
 			continue;
 		scoped_guard(mutex, &q->timer_mutex) {
@@ -461,7 +461,6 @@ void snd_seq_info_timer_read(struct snd_info_entry *entry,
 			snd_iprintf(buffer, "  Period time : %lu.%09lu\n", resolution / 1000000000, resolution % 1000000000);
 			snd_iprintf(buffer, "  Skew : %u / %u\n", tmr->skew, tmr->skew_base);
 		}
-		queuefree(q);
  	}
 }
 #endif /* CONFIG_SND_PROC_FS */
-- 
2.50.1


^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [PATCH v2 07/11] ALSA: seq: Clean up fifo locking with guard
  2025-08-27  8:05 [PATCH v2 00/11] ALSA: seq: Use auto-cleanup macros Takashi Iwai
                   ` (5 preceding siblings ...)
  2025-08-27  8:05 ` [PATCH v2 06/11] ALSA: seq: Clean up queue " Takashi Iwai
@ 2025-08-27  8:05 ` Takashi Iwai
  2025-08-27  8:05 ` [PATCH v2 08/11] ALSA: seq: oss: Clean up core code with guard() Takashi Iwai
                   ` (3 subsequent siblings)
  10 siblings, 0 replies; 12+ messages in thread
From: Takashi Iwai @ 2025-08-27  8:05 UTC (permalink / raw)
  To: linux-sound

Yet more cleanup, now for seq_fifo.c about its refcount calls; the
manual refcount calls (either snd_use_lock_*() or snd_seq_fifo_lock())
are replaced with guard(snd_seq_fifo).

Only code refactoring, and no behavior change.

Signed-off-by: Takashi Iwai <tiwai@suse.de>
---
 sound/core/seq/seq_clientmgr.c |  3 +--
 sound/core/seq/seq_fifo.c      | 15 ++++-----------
 sound/core/seq/seq_fifo.h      |  1 +
 3 files changed, 6 insertions(+), 13 deletions(-)

diff --git a/sound/core/seq/seq_clientmgr.c b/sound/core/seq/seq_clientmgr.c
index 38ad5bbd2706..f9a6e497f997 100644
--- a/sound/core/seq/seq_clientmgr.c
+++ b/sound/core/seq/seq_clientmgr.c
@@ -416,7 +416,7 @@ static ssize_t snd_seq_read(struct file *file, char __user *buf, size_t count,
 
 	cell = NULL;
 	err = 0;
-	snd_seq_fifo_lock(fifo);
+	guard(snd_seq_fifo)(fifo);
 
 	if (IS_ENABLED(CONFIG_SND_SEQ_UMP) && client->midi_version > 0)
 		aligned_size = sizeof(struct snd_seq_ump_event);
@@ -474,7 +474,6 @@ static ssize_t snd_seq_read(struct file *file, char __user *buf, size_t count,
 		if (err == -EAGAIN && result > 0)
 			err = 0;
 	}
-	snd_seq_fifo_unlock(fifo);
 
 	return (err < 0) ? err : result;
 }
diff --git a/sound/core/seq/seq_fifo.c b/sound/core/seq/seq_fifo.c
index 3a10b081f129..f23c6b7ae240 100644
--- a/sound/core/seq/seq_fifo.c
+++ b/sound/core/seq/seq_fifo.c
@@ -106,12 +106,11 @@ int snd_seq_fifo_event_in(struct snd_seq_fifo *f,
 	if (snd_BUG_ON(!f))
 		return -EINVAL;
 
-	snd_use_lock_use(&f->use_lock);
+	guard(snd_seq_fifo)(f);
 	err = snd_seq_event_dup(f->pool, event, &cell, 1, NULL, NULL); /* always non-blocking */
 	if (err < 0) {
 		if ((err == -ENOMEM) || (err == -EAGAIN))
 			atomic_inc(&f->overflow);
-		snd_use_lock_free(&f->use_lock);
 		return err;
 	}
 		
@@ -130,8 +129,6 @@ int snd_seq_fifo_event_in(struct snd_seq_fifo *f,
 	if (waitqueue_active(&f->input_sleep))
 		wake_up(&f->input_sleep);
 
-	snd_use_lock_free(&f->use_lock);
-
 	return 0; /* success */
 
 }
@@ -263,14 +260,10 @@ int snd_seq_fifo_resize(struct snd_seq_fifo *f, int poolsize)
 /* get the number of unused cells safely */
 int snd_seq_fifo_unused_cells(struct snd_seq_fifo *f)
 {
-	int cells;
-
 	if (!f)
 		return 0;
 
-	snd_use_lock_use(&f->use_lock);
-	scoped_guard(spinlock_irqsave, &f->lock)
-		cells = snd_seq_unused_cells(f->pool);
-	snd_use_lock_free(&f->use_lock);
-	return cells;
+	guard(snd_seq_fifo)(f);
+	guard(spinlock_irqsave)(&f->lock);
+	return snd_seq_unused_cells(f->pool);
 }
diff --git a/sound/core/seq/seq_fifo.h b/sound/core/seq/seq_fifo.h
index b56a7b897c9c..4c9c49127746 100644
--- a/sound/core/seq/seq_fifo.h
+++ b/sound/core/seq/seq_fifo.h
@@ -37,6 +37,7 @@ int snd_seq_fifo_event_in(struct snd_seq_fifo *f, struct snd_seq_event *event);
 /* lock fifo from release */
 #define snd_seq_fifo_lock(fifo)		snd_use_lock_use(&(fifo)->use_lock)
 #define snd_seq_fifo_unlock(fifo)	snd_use_lock_free(&(fifo)->use_lock)
+DEFINE_GUARD(snd_seq_fifo, struct snd_seq_fifo *, snd_seq_fifo_lock(_T), snd_seq_fifo_unlock(_T))
 
 /* get a cell from fifo - fifo should be locked */
 int snd_seq_fifo_cell_out(struct snd_seq_fifo *f, struct snd_seq_event_cell **cellp, int nonblock);
-- 
2.50.1


^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [PATCH v2 08/11] ALSA: seq: oss: Clean up core code with guard()
  2025-08-27  8:05 [PATCH v2 00/11] ALSA: seq: Use auto-cleanup macros Takashi Iwai
                   ` (6 preceding siblings ...)
  2025-08-27  8:05 ` [PATCH v2 07/11] ALSA: seq: Clean up fifo locking with guard Takashi Iwai
@ 2025-08-27  8:05 ` Takashi Iwai
  2025-08-27  8:05 ` [PATCH v2 09/11] ALSA: seq: oss/midi: Cleanup with guard and auto-cleanup Takashi Iwai
                   ` (2 subsequent siblings)
  10 siblings, 0 replies; 12+ messages in thread
From: Takashi Iwai @ 2025-08-27  8:05 UTC (permalink / raw)
  To: linux-sound

Replace the manual mutex lock/unlock pairs with guard() for code
simplification.

Only code refactoring, and no behavior change.

Signed-off-by: Takashi Iwai <tiwai@suse.de>
---
 sound/core/seq/oss/seq_oss.c | 24 +++++++-----------------
 1 file changed, 7 insertions(+), 17 deletions(-)

diff --git a/sound/core/seq/oss/seq_oss.c b/sound/core/seq/oss/seq_oss.c
index 77c1214acd90..02d30d8b6c3a 100644
--- a/sound/core/seq/oss/seq_oss.c
+++ b/sound/core/seq/oss/seq_oss.c
@@ -117,18 +117,15 @@ static DEFINE_MUTEX(register_mutex);
 static int
 odev_open(struct inode *inode, struct file *file)
 {
-	int level, rc;
+	int level;
 
 	if (iminor(inode) == SNDRV_MINOR_OSS_MUSIC)
 		level = SNDRV_SEQ_OSS_MODE_MUSIC;
 	else
 		level = SNDRV_SEQ_OSS_MODE_SYNTH;
 
-	mutex_lock(&register_mutex);
-	rc = snd_seq_oss_open(file, level);
-	mutex_unlock(&register_mutex);
-
-	return rc;
+	guard(mutex)(&register_mutex);
+	return snd_seq_oss_open(file, level);
 }
 
 static int
@@ -140,10 +137,8 @@ odev_release(struct inode *inode, struct file *file)
 	if (!dp)
 		return 0;
 
-	mutex_lock(&register_mutex);
+	guard(mutex)(&register_mutex);
 	snd_seq_oss_release(dp);
-	mutex_unlock(&register_mutex);
-
 	return 0;
 }
 
@@ -229,13 +224,12 @@ register_device(void)
 {
 	int rc;
 
-	mutex_lock(&register_mutex);
+	guard(mutex)(&register_mutex);
 	rc = snd_register_oss_device(SNDRV_OSS_DEVICE_TYPE_SEQUENCER,
 				     NULL, 0,
 				     &seq_oss_f_ops, NULL);
 	if (rc < 0) {
 		pr_err("ALSA: seq_oss: can't register device seq\n");
-		mutex_unlock(&register_mutex);
 		return rc;
 	}
 	rc = snd_register_oss_device(SNDRV_OSS_DEVICE_TYPE_MUSIC,
@@ -244,22 +238,19 @@ register_device(void)
 	if (rc < 0) {
 		pr_err("ALSA: seq_oss: can't register device music\n");
 		snd_unregister_oss_device(SNDRV_OSS_DEVICE_TYPE_SEQUENCER, NULL, 0);
-		mutex_unlock(&register_mutex);
 		return rc;
 	}
-	mutex_unlock(&register_mutex);
 	return 0;
 }
 
 static void
 unregister_device(void)
 {
-	mutex_lock(&register_mutex);
+	guard(mutex)(&register_mutex);
 	if (snd_unregister_oss_device(SNDRV_OSS_DEVICE_TYPE_MUSIC, NULL, 0) < 0)		
 		pr_err("ALSA: seq_oss: error unregister device music\n");
 	if (snd_unregister_oss_device(SNDRV_OSS_DEVICE_TYPE_SEQUENCER, NULL, 0) < 0)
 		pr_err("ALSA: seq_oss: error unregister device seq\n");
-	mutex_unlock(&register_mutex);
 }
 
 /*
@@ -273,12 +264,11 @@ static struct snd_info_entry *info_entry;
 static void
 info_read(struct snd_info_entry *entry, struct snd_info_buffer *buf)
 {
-	mutex_lock(&register_mutex);
+	guard(mutex)(&register_mutex);
 	snd_iprintf(buf, "OSS sequencer emulation version %s\n", SNDRV_SEQ_OSS_VERSION_STR);
 	snd_seq_oss_system_info_read(buf);
 	snd_seq_oss_synth_info_read(buf);
 	snd_seq_oss_midi_info_read(buf);
-	mutex_unlock(&register_mutex);
 }
 
 
-- 
2.50.1


^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [PATCH v2 09/11] ALSA: seq: oss/midi: Cleanup with guard and auto-cleanup
  2025-08-27  8:05 [PATCH v2 00/11] ALSA: seq: Use auto-cleanup macros Takashi Iwai
                   ` (7 preceding siblings ...)
  2025-08-27  8:05 ` [PATCH v2 08/11] ALSA: seq: oss: Clean up core code with guard() Takashi Iwai
@ 2025-08-27  8:05 ` Takashi Iwai
  2025-08-27  8:05 ` [PATCH v2 10/11] ALSA: seq: oss/synth: Clean up with guard and auto cleanup Takashi Iwai
  2025-08-27  8:05 ` [PATCH v2 11/11] ALSA: seq: oss/rw: Cleanup with guard Takashi Iwai
  10 siblings, 0 replies; 12+ messages in thread
From: Takashi Iwai @ 2025-08-27  8:05 UTC (permalink / raw)
  To: linux-sound

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 | 116 ++++++++++--------------------
 1 file changed, 36 insertions(+), 80 deletions(-)

diff --git a/sound/core/seq/oss/seq_oss_midi.c b/sound/core/seq/oss/seq_oss_midi.c
index f8e247d9e5c9..023e5d0a4351 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;
 }
 
@@ -135,7 +131,6 @@ snd_seq_oss_midi_check_new_port(struct snd_seq_port_info *pinfo)
 {
 	int i;
 	struct seq_oss_midi *mdev;
-	unsigned long flags;
 
 	/* the port must include generic midi */
 	if (! (pinfo->type & SNDRV_SEQ_PORT_TYPE_MIDI_GENERIC))
@@ -185,14 +180,13 @@ 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);
 			return -ENOMEM;
@@ -201,7 +195,6 @@ snd_seq_oss_midi_check_new_port(struct snd_seq_port_info *pinfo)
 	}
 	mdev->seq_device = i;
 	midi_devs[mdev->seq_device] = mdev;
-	spin_unlock_irqrestore(&register_lock, flags);
 
 	return 0;
 }
@@ -213,26 +206,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 +236,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 +247,6 @@ snd_seq_oss_midi_clear_all(void)
 		}
 	}
 	max_midi_devs = 0;
-	spin_unlock_irqrestore(&register_lock, flags);
 }
 
 
@@ -267,9 +256,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 +305,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 +323,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 +351,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 +364,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 +390,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 +399,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 +412,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 +422,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 +459,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 +468,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 +485,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 +595,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 +613,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 +622,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 +648,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 +664,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


^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [PATCH v2 10/11] ALSA: seq: oss/synth: Clean up with guard and auto cleanup
  2025-08-27  8:05 [PATCH v2 00/11] ALSA: seq: Use auto-cleanup macros Takashi Iwai
                   ` (8 preceding siblings ...)
  2025-08-27  8:05 ` [PATCH v2 09/11] ALSA: seq: oss/midi: Cleanup with guard and auto-cleanup Takashi Iwai
@ 2025-08-27  8:05 ` Takashi Iwai
  2025-08-27  8:05 ` [PATCH v2 11/11] ALSA: seq: oss/rw: Cleanup with guard Takashi Iwai
  10 siblings, 0 replies; 12+ messages in thread
From: Takashi Iwai @ 2025-08-27  8:05 UTC (permalink / raw)
  To: linux-sound

Use the auto-cleanup for the refcount management of seq_oss_synth
object.  The explicit call of snd_use_lock_free() is dropped by the
magic __free(seq_oss_synth) 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_synth.c | 125 ++++++++++++-----------------
 1 file changed, 52 insertions(+), 73 deletions(-)

diff --git a/sound/core/seq/oss/seq_oss_synth.c b/sound/core/seq/oss/seq_oss_synth.c
index 9de47e098b29..8c4e5913c7e6 100644
--- a/sound/core/seq/oss/seq_oss_synth.c
+++ b/sound/core/seq/oss/seq_oss_synth.c
@@ -44,6 +44,7 @@ struct seq_oss_synth {
 	snd_use_lock_t use_lock;
 };
 
+DEFINE_FREE(seq_oss_synth, struct seq_oss_synth *, if (!IS_ERR_OR_NULL(_T)) snd_use_lock_free(&(_T)->use_lock))
 
 /*
  * device table
@@ -85,7 +86,6 @@ snd_seq_oss_synth_probe(struct device *_dev)
 	int i;
 	struct seq_oss_synth *rec;
 	struct snd_seq_oss_reg *reg = SNDRV_SEQ_DEVICE_ARGPTR(dev);
-	unsigned long flags;
 
 	rec = kzalloc(sizeof(*rec), GFP_KERNEL);
 	if (!rec)
@@ -103,23 +103,22 @@ snd_seq_oss_synth_probe(struct device *_dev)
 	strscpy(rec->name, dev->name, sizeof(rec->name));
 
 	/* registration */
-	spin_lock_irqsave(&register_lock, flags);
-	for (i = 0; i < max_synth_devs; i++) {
-		if (synth_devs[i] == NULL)
-			break;
-	}
-	if (i >= max_synth_devs) {
-		if (max_synth_devs >= SNDRV_SEQ_OSS_MAX_SYNTH_DEVS) {
-			spin_unlock_irqrestore(&register_lock, flags);
-			pr_err("ALSA: seq_oss: no more synth slot\n");
-			kfree(rec);
-			return -ENOMEM;
+	scoped_guard(spinlock_irqsave, &register_lock) {
+		for (i = 0; i < max_synth_devs; i++) {
+			if (synth_devs[i] == NULL)
+				break;
 		}
-		max_synth_devs++;
+		if (i >= max_synth_devs) {
+			if (max_synth_devs >= SNDRV_SEQ_OSS_MAX_SYNTH_DEVS) {
+				pr_err("ALSA: seq_oss: no more synth slot\n");
+				kfree(rec);
+				return -ENOMEM;
+			}
+			max_synth_devs++;
+		}
+		rec->seq_device = i;
+		synth_devs[i] = rec;
 	}
-	rec->seq_device = i;
-	synth_devs[i] = rec;
-	spin_unlock_irqrestore(&register_lock, flags);
 	dev->driver_data = rec;
 #ifdef SNDRV_OSS_INFO_DEV_SYNTH
 	if (i < SNDRV_CARDS)
@@ -135,27 +134,25 @@ snd_seq_oss_synth_remove(struct device *_dev)
 	struct snd_seq_device *dev = to_seq_dev(_dev);
 	int index;
 	struct seq_oss_synth *rec = dev->driver_data;
-	unsigned long flags;
 
-	spin_lock_irqsave(&register_lock, flags);
-	for (index = 0; index < max_synth_devs; index++) {
-		if (synth_devs[index] == rec)
-			break;
-	}
-	if (index >= max_synth_devs) {
-		spin_unlock_irqrestore(&register_lock, flags);
-		pr_err("ALSA: seq_oss: can't unregister synth\n");
-		return -EINVAL;
-	}
-	synth_devs[index] = NULL;
-	if (index == max_synth_devs - 1) {
-		for (index--; index >= 0; index--) {
-			if (synth_devs[index])
+	scoped_guard(spinlock_irqsave, &register_lock) {
+		for (index = 0; index < max_synth_devs; index++) {
+			if (synth_devs[index] == rec)
 				break;
 		}
-		max_synth_devs = index + 1;
+		if (index >= max_synth_devs) {
+			pr_err("ALSA: seq_oss: can't unregister synth\n");
+			return -EINVAL;
+		}
+		synth_devs[index] = NULL;
+		if (index == max_synth_devs - 1) {
+			for (index--; index >= 0; index--) {
+				if (synth_devs[index])
+					break;
+			}
+			max_synth_devs = index + 1;
+		}
 	}
-	spin_unlock_irqrestore(&register_lock, flags);
 #ifdef SNDRV_OSS_INFO_DEV_SYNTH
 	if (rec->seq_device < SNDRV_CARDS)
 		snd_oss_info_unregister(SNDRV_OSS_INFO_DEV_SYNTH, rec->seq_device);
@@ -174,13 +171,11 @@ static struct seq_oss_synth *
 get_sdev(int dev)
 {
 	struct seq_oss_synth *rec;
-	unsigned long flags;
 
-	spin_lock_irqsave(&register_lock, flags);
+	guard(spinlock_irqsave)(&register_lock);
 	rec = synth_devs[dev];
 	if (rec)
 		snd_use_lock_use(&rec->use_lock);
-	spin_unlock_irqrestore(&register_lock, flags);
 	return rec;
 }
 
@@ -193,20 +188,18 @@ void
 snd_seq_oss_synth_setup(struct seq_oss_devinfo *dp)
 {
 	int i;
-	struct seq_oss_synth *rec;
 	struct seq_oss_synthinfo *info;
 
 	dp->max_synthdev = max_synth_devs;
 	dp->synth_opened = 0;
 	memset(dp->synths, 0, sizeof(dp->synths));
 	for (i = 0; i < dp->max_synthdev; i++) {
-		rec = get_sdev(i);
+		struct seq_oss_synth *rec __free(seq_oss_synth) = get_sdev(i);
+
 		if (rec == NULL)
 			continue;
-		if (rec->oper.open == NULL || rec->oper.close == NULL) {
-			snd_use_lock_free(&rec->use_lock);
+		if (rec->oper.open == NULL || rec->oper.close == NULL)
 			continue;
-		}
 		info = &dp->synths[i];
 		info->arg.app_index = dp->port;
 		info->arg.file_mode = dp->file_mode;
@@ -216,13 +209,10 @@ snd_seq_oss_synth_setup(struct seq_oss_devinfo *dp)
 		else
 			info->arg.event_passing = SNDRV_SEQ_OSS_PASS_EVENTS;
 		info->opened = 0;
-		if (!try_module_get(rec->oper.owner)) {
-			snd_use_lock_free(&rec->use_lock);
+		if (!try_module_get(rec->oper.owner))
 			continue;
-		}
 		if (rec->oper.open(&info->arg, rec->private_data) < 0) {
 			module_put(rec->oper.owner);
-			snd_use_lock_free(&rec->use_lock);
 			continue;
 		}
 		info->nr_voices = rec->nr_voices;
@@ -231,7 +221,6 @@ snd_seq_oss_synth_setup(struct seq_oss_devinfo *dp)
 			if (!info->ch) {
 				rec->oper.close(&info->arg);
 				module_put(rec->oper.owner);
-				snd_use_lock_free(&rec->use_lock);
 				continue;
 			}
 			reset_channels(info);
@@ -239,7 +228,6 @@ snd_seq_oss_synth_setup(struct seq_oss_devinfo *dp)
 		info->opened++;
 		rec->opened++;
 		dp->synth_opened++;
-		snd_use_lock_free(&rec->use_lock);
 	}
 }
 
@@ -286,7 +274,6 @@ void
 snd_seq_oss_synth_cleanup(struct seq_oss_devinfo *dp)
 {
 	int i;
-	struct seq_oss_synth *rec;
 	struct seq_oss_synthinfo *info;
 
 	if (snd_BUG_ON(dp->max_synthdev > SNDRV_SEQ_OSS_MAX_SYNTH_DEVS))
@@ -301,7 +288,9 @@ snd_seq_oss_synth_cleanup(struct seq_oss_devinfo *dp)
 				midi_synth_dev.opened--;
 			}
 		} else {
-			rec = get_sdev(i);
+			struct seq_oss_synth *rec __free(seq_oss_synth) =
+				get_sdev(i);
+
 			if (rec == NULL)
 				continue;
 			if (rec->opened > 0) {
@@ -309,7 +298,6 @@ snd_seq_oss_synth_cleanup(struct seq_oss_devinfo *dp)
 				module_put(rec->oper.owner);
 				rec->opened = 0;
 			}
-			snd_use_lock_free(&rec->use_lock);
 		}
 		kfree(info->ch);
 		info->ch = NULL;
@@ -380,7 +368,7 @@ reset_channels(struct seq_oss_synthinfo *info)
 void
 snd_seq_oss_synth_reset(struct seq_oss_devinfo *dp, int dev)
 {
-	struct seq_oss_synth *rec;
+	struct seq_oss_synth *rec __free(seq_oss_synth) = NULL;
 	struct seq_oss_synthinfo *info;
 
 	info = get_synthinfo_nospec(dp, dev);
@@ -416,7 +404,6 @@ snd_seq_oss_synth_reset(struct seq_oss_devinfo *dp, int dev)
 		ev.type = SNDRV_SEQ_EVENT_RESET;
 		snd_seq_oss_dispatch(dp, &ev, 0, 0);
 	}
-	snd_use_lock_free(&rec->use_lock);
 }
 
 
@@ -428,9 +415,8 @@ int
 snd_seq_oss_synth_load_patch(struct seq_oss_devinfo *dp, int dev, int fmt,
 			    const char __user *buf, int p, int c)
 {
-	struct seq_oss_synth *rec;
+	struct seq_oss_synth *rec __free(seq_oss_synth) = NULL;
 	struct seq_oss_synthinfo *info;
-	int rc;
 
 	info = get_synthinfo_nospec(dp, dev);
 	if (!info)
@@ -443,11 +429,9 @@ snd_seq_oss_synth_load_patch(struct seq_oss_devinfo *dp, int dev, int fmt,
 		return -ENXIO;
 
 	if (rec->oper.load_patch == NULL)
-		rc = -ENXIO;
+		return -ENXIO;
 	else
-		rc = rec->oper.load_patch(&info->arg, fmt, buf, p, c);
-	snd_use_lock_free(&rec->use_lock);
-	return rc;
+		return rec->oper.load_patch(&info->arg, fmt, buf, p, c);
 }
 
 /*
@@ -456,13 +440,11 @@ snd_seq_oss_synth_load_patch(struct seq_oss_devinfo *dp, int dev, int fmt,
 struct seq_oss_synthinfo *
 snd_seq_oss_synth_info(struct seq_oss_devinfo *dp, int dev)
 {
-	struct seq_oss_synth *rec;
+	struct seq_oss_synth *rec __free(seq_oss_synth) = NULL;
 
 	rec = get_synthdev(dp, dev);
-	if (rec) {
-		snd_use_lock_free(&rec->use_lock);
+	if (rec)
 		return get_synthinfo_nospec(dp, dev);
-	}
 	return NULL;
 }
 
@@ -513,9 +495,8 @@ snd_seq_oss_synth_addr(struct seq_oss_devinfo *dp, int dev, struct snd_seq_event
 int
 snd_seq_oss_synth_ioctl(struct seq_oss_devinfo *dp, int dev, unsigned int cmd, unsigned long addr)
 {
-	struct seq_oss_synth *rec;
+	struct seq_oss_synth *rec __free(seq_oss_synth) = NULL;
 	struct seq_oss_synthinfo *info;
-	int rc;
 
 	info = get_synthinfo_nospec(dp, dev);
 	if (!info || info->is_midi)
@@ -524,11 +505,9 @@ snd_seq_oss_synth_ioctl(struct seq_oss_devinfo *dp, int dev, unsigned int cmd, u
 	if (!rec)
 		return -ENXIO;
 	if (rec->oper.ioctl == NULL)
-		rc = -ENXIO;
+		return -ENXIO;
 	else
-		rc = rec->oper.ioctl(&info->arg, cmd, addr);
-	snd_use_lock_free(&rec->use_lock);
-	return rc;
+		return rec->oper.ioctl(&info->arg, cmd, addr);
 }
 
 
@@ -555,7 +534,6 @@ snd_seq_oss_synth_raw_event(struct seq_oss_devinfo *dp, int dev, unsigned char *
 int
 snd_seq_oss_synth_make_info(struct seq_oss_devinfo *dp, int dev, struct synth_info *inf)
 {
-	struct seq_oss_synth *rec;
 	struct seq_oss_synthinfo *info = get_synthinfo_nospec(dp, dev);
 
 	if (!info)
@@ -571,7 +549,9 @@ snd_seq_oss_synth_make_info(struct seq_oss_devinfo *dp, int dev, struct synth_in
 		inf->device = dev;
 		strscpy(inf->name, minf.name, sizeof(inf->name));
 	} else {
-		rec = get_synthdev(dp, dev);
+		struct seq_oss_synth *rec __free(seq_oss_synth) =
+			get_synthdev(dp, dev);
+
 		if (!rec)
 			return -ENXIO;
 		inf->synth_type = rec->synth_type;
@@ -579,7 +559,6 @@ snd_seq_oss_synth_make_info(struct seq_oss_devinfo *dp, int dev, struct synth_in
 		inf->nr_voices = rec->nr_voices;
 		inf->device = dev;
 		strscpy(inf->name, rec->name, sizeof(inf->name));
-		snd_use_lock_free(&rec->use_lock);
 	}
 	return 0;
 }
@@ -593,10 +572,11 @@ void
 snd_seq_oss_synth_info_read(struct snd_info_buffer *buf)
 {
 	int i;
-	struct seq_oss_synth *rec;
 
 	snd_iprintf(buf, "\nNumber of synth devices: %d\n", max_synth_devs);
 	for (i = 0; i < max_synth_devs; i++) {
+		struct seq_oss_synth *rec __free(seq_oss_synth) = NULL;
+
 		snd_iprintf(buf, "\nsynth %d: ", i);
 		rec = get_sdev(i);
 		if (rec == NULL) {
@@ -610,7 +590,6 @@ snd_seq_oss_synth_info_read(struct snd_info_buffer *buf)
 		snd_iprintf(buf, "  capabilities : ioctl %s / load_patch %s\n",
 			    str_enabled_disabled((long)rec->oper.ioctl),
 			    str_enabled_disabled((long)rec->oper.load_patch));
-		snd_use_lock_free(&rec->use_lock);
 	}
 }
 #endif /* CONFIG_SND_PROC_FS */
-- 
2.50.1


^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [PATCH v2 11/11] ALSA: seq: oss/rw: Cleanup with guard
  2025-08-27  8:05 [PATCH v2 00/11] ALSA: seq: Use auto-cleanup macros Takashi Iwai
                   ` (9 preceding siblings ...)
  2025-08-27  8:05 ` [PATCH v2 10/11] ALSA: seq: oss/synth: Clean up with guard and auto cleanup Takashi Iwai
@ 2025-08-27  8:05 ` Takashi Iwai
  10 siblings, 0 replies; 12+ messages in thread
From: Takashi Iwai @ 2025-08-27  8:05 UTC (permalink / raw)
  To: linux-sound

Replace the manual spin lock/unlock pairs with guard() for code
simplification.

Only code refactoring, and no behavior change.

Signed-off-by: Takashi Iwai <tiwai@suse.de>
---
 sound/core/seq/oss/seq_oss_readq.c  | 10 ++--------
 sound/core/seq/oss/seq_oss_writeq.c |  5 +----
 2 files changed, 3 insertions(+), 12 deletions(-)

diff --git a/sound/core/seq/oss/seq_oss_readq.c b/sound/core/seq/oss/seq_oss_readq.c
index f0db5d3dcba4..bbaf72e70b35 100644
--- a/sound/core/seq/oss/seq_oss_readq.c
+++ b/sound/core/seq/oss/seq_oss_readq.c
@@ -140,13 +140,9 @@ int snd_seq_oss_readq_sysex(struct seq_oss_readq *q, int dev,
 int
 snd_seq_oss_readq_put_event(struct seq_oss_readq *q, union evrec *ev)
 {
-	unsigned long flags;
-
-	spin_lock_irqsave(&q->lock, flags);
-	if (q->qlen >= q->maxlen - 1) {
-		spin_unlock_irqrestore(&q->lock, flags);
+	guard(spinlock_irqsave)(&q->lock);
+	if (q->qlen >= q->maxlen - 1)
 		return -ENOMEM;
-	}
 
 	memcpy(&q->q[q->tail], ev, sizeof(*ev));
 	q->tail = (q->tail + 1) % q->maxlen;
@@ -155,8 +151,6 @@ snd_seq_oss_readq_put_event(struct seq_oss_readq *q, union evrec *ev)
 	/* wake up sleeper */
 	wake_up(&q->midi_sleep);
 
-	spin_unlock_irqrestore(&q->lock, flags);
-
 	return 0;
 }
 
diff --git a/sound/core/seq/oss/seq_oss_writeq.c b/sound/core/seq/oss/seq_oss_writeq.c
index 3e3209ce53b1..a93ff8315b8e 100644
--- a/sound/core/seq/oss/seq_oss_writeq.c
+++ b/sound/core/seq/oss/seq_oss_writeq.c
@@ -122,13 +122,10 @@ snd_seq_oss_writeq_sync(struct seq_oss_writeq *q)
 void
 snd_seq_oss_writeq_wakeup(struct seq_oss_writeq *q, abstime_t time)
 {
-	unsigned long flags;
-
-	spin_lock_irqsave(&q->sync_lock, flags);
+	guard(spinlock_irqsave)(&q->sync_lock);
 	q->sync_time = time;
 	q->sync_event_put = 0;
 	wake_up(&q->sync_sleep);
-	spin_unlock_irqrestore(&q->sync_lock, flags);
 }
 
 
-- 
2.50.1


^ permalink raw reply related	[flat|nested] 12+ messages in thread

end of thread, other threads:[~2025-08-27  8:06 UTC | newest]

Thread overview: 12+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-08-27  8:05 [PATCH v2 00/11] ALSA: seq: Use auto-cleanup macros Takashi Iwai
2025-08-27  8:05 ` [PATCH v2 01/11] ALSA: seq: Simplify internal command operation from OSS layer Takashi Iwai
2025-08-27  8:05 ` [PATCH v2 02/11] ALSA: seq: Clean up spin lock with guard() Takashi Iwai
2025-08-27  8:05 ` [PATCH v2 03/11] ALSA: seq: Use guard() for mutex and rwsem locks Takashi Iwai
2025-08-27  8:05 ` [PATCH v2 04/11] ALSA: seq: Use auto-cleanup for client refcounting Takashi Iwai
2025-08-27  8:05 ` [PATCH v2 05/11] ALSA: seq: Clean up port locking with auto cleanup Takashi Iwai
2025-08-27  8:05 ` [PATCH v2 06/11] ALSA: seq: Clean up queue " Takashi Iwai
2025-08-27  8:05 ` [PATCH v2 07/11] ALSA: seq: Clean up fifo locking with guard Takashi Iwai
2025-08-27  8:05 ` [PATCH v2 08/11] ALSA: seq: oss: Clean up core code with guard() Takashi Iwai
2025-08-27  8:05 ` [PATCH v2 09/11] ALSA: seq: oss/midi: Cleanup with guard and auto-cleanup Takashi Iwai
2025-08-27  8:05 ` [PATCH v2 10/11] ALSA: seq: oss/synth: Clean up with guard and auto cleanup Takashi Iwai
2025-08-27  8:05 ` [PATCH v2 11/11] ALSA: seq: oss/rw: Cleanup with guard Takashi Iwai

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).