mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
* [PATCH 0/5] ALSA: seq: Optimization with RCU
@ 2026-08-10 13:37 Takashi Iwai
  2026-08-10 13:37 ` [PATCH 1/5] ALSA: seq: Use RCU for the port subscriber list Takashi Iwai
                   ` (4 more replies)
  0 siblings, 5 replies; 6+ messages in thread
From: Takashi Iwai @ 2026-08-10 13:37 UTC (permalink / raw)
  To: linux-sound; +Cc: linux-kernel

Hi,

here are a set of (rather small) patches for optimizing lockings in
ALSA sequencer core by replacing with RCU.  Most of changes are
straight-forward conversions from read-lock to RCU, or a simple
pointer assignment.


Takashi

===

Takashi Iwai (5):
  ALSA: seq: Use RCU for the port subscriber list
  ALSA: seq: Use RCU for the client port list
  ALSA: seq: Use RCU for the client table
  ALSA: seq: Use RCU for the virmidi file list
  ALSA: seq: Use RCU for the UMP client output substream

 include/sound/seq_virmidi.h     |   1 -
 sound/core/seq/seq_clientmgr.c  |  69 +++++++++++++--------
 sound/core/seq/seq_clientmgr.h  |   1 -
 sound/core/seq/seq_ports.c      | 103 +++++++++++++++-----------------
 sound/core/seq/seq_ports.h      |   8 +--
 sound/core/seq/seq_ump_client.c |  38 +++++++-----
 sound/core/seq/seq_virmidi.c    |  19 +++---
 7 files changed, 127 insertions(+), 112 deletions(-)

-- 
2.55.0


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

* [PATCH 1/5] ALSA: seq: Use RCU for the port subscriber list
  2026-08-10 13:37 [PATCH 0/5] ALSA: seq: Optimization with RCU Takashi Iwai
@ 2026-08-10 13:37 ` Takashi Iwai
  2026-08-10 13:37 ` [PATCH 2/5] ALSA: seq: Use RCU for the client port list Takashi Iwai
                   ` (3 subsequent siblings)
  4 siblings, 0 replies; 6+ messages in thread
From: Takashi Iwai @ 2026-08-10 13:37 UTC (permalink / raw)
  To: linux-sound; +Cc: linux-kernel

Each sequencer port keeps two subscriber groups (c_src and c_dest),
each protected by both an rwlock (list_lock) and a rw_semaphore
(list_mutex).  The rwlock is taken read-side in the event delivery hot
path (__deliver_to_subscribers()) for delivering every event to
subscribers, while the mutex serializes subscribe/unsubscribe and
covers the sleepable delivery and query walks.

Subscriptions change rarely but delivery happens constantly, so this is
a textbook read-mostly case.  Convert the subscriber list traversal to
RCU and drop the rwlock entirely while keeping the existing list_mutex
for serializing the writers.  The atomic delivery path now runs
lock-free under rcu_read_lock() instead of contending on the shared
rwlock.

Along with the conversion to RCU, the subscriber lists are switched
from list_head to hlist so that removal can use hlist_del_init_rcu():
it keeps the ->next pointer intact for concurrent readers while
clearing ->pprev, which lets the double-deletion guard (added in
commit 13d5e5d4725c) keep detecting an already-removed entry via
hlist_unhashed().

Dropping write_lock_irq() from the writers is safe: no writer runs in
atomic/IRQ context, and the sole atomic reader now uses RCU, which is
IRQ-safe.  Port lifetime handling (use_lock/closing drain in
port_delete()) is orthogonal and unchanged.

Note that the conversion to RCU has another merit: it automatically
"fixes" the (rather false) lockdep warnings for the doubly read-locks
of the same subscriber list, too.

Signed-off-by: Takashi Iwai <tiwai@suse.de>
---
 sound/core/seq/seq_clientmgr.c | 25 ++++++++--------
 sound/core/seq/seq_ports.c     | 52 ++++++++++++++++------------------
 sound/core/seq/seq_ports.h     |  8 +++---
 3 files changed, 41 insertions(+), 44 deletions(-)

diff --git a/sound/core/seq/seq_clientmgr.c b/sound/core/seq/seq_clientmgr.c
index 23ec239640c3..77f5020f1873 100644
--- a/sound/core/seq/seq_clientmgr.c
+++ b/sound/core/seq/seq_clientmgr.c
@@ -717,10 +717,11 @@ static int __deliver_to_subscribers(struct snd_seq_client *client,
 	
 	/* lock list */
 	if (atomic)
-		read_lock(&grp->list_lock);
+		rcu_read_lock();
 	else
 		down_read_nested(&grp->list_mutex, hop);
-	list_for_each_entry(subs, &grp->list_head, src_list) {
+	hlist_for_each_entry_rcu(subs, &grp->list_head, src_list,
+				 lockdep_is_held(&grp->list_mutex)) {
 		/* both ports ready? */
 		if (atomic_read(&subs->ref_count) != 2)
 			continue;
@@ -741,7 +742,7 @@ static int __deliver_to_subscribers(struct snd_seq_client *client,
 		memcpy(event, &event_saved, saved_size);
 	}
 	if (atomic)
-		read_unlock(&grp->list_lock);
+		rcu_read_unlock();
 	else
 		up_read(&grp->list_mutex);
 	memcpy(event, &event_saved, saved_size);
@@ -1938,7 +1939,7 @@ static int snd_seq_ioctl_query_subs(struct snd_seq_client *client, void *arg)
 {
 	struct snd_seq_query_subs *subs = arg;
 	struct snd_seq_port_subs_info *group;
-	struct list_head *p;
+	struct hlist_node *p;
 	int i;
 
 	struct snd_seq_client *cptr __free(snd_seq_client) =
@@ -1965,15 +1966,15 @@ static int snd_seq_ioctl_query_subs(struct snd_seq_client *client, void *arg)
 	/* search for the subscriber */
 	subs->num_subs = group->count;
 	i = 0;
-	list_for_each(p, &group->list_head) {
+	hlist_for_each(p, &group->list_head) {
 		if (i++ == subs->index) {
 			/* found! */
 			struct snd_seq_subscribers *s;
 			if (subs->type == SNDRV_SEQ_QUERY_SUBS_READ) {
-				s = list_entry(p, struct snd_seq_subscribers, src_list);
+				s = hlist_entry(p, struct snd_seq_subscribers, src_list);
 				subs->addr = s->info.dest;
 			} else {
-				s = list_entry(p, struct snd_seq_subscribers, dest_list);
+				s = hlist_entry(p, struct snd_seq_subscribers, dest_list);
 				subs->addr = s->info.sender;
 			}
 			subs->flags = s->info.flags;
@@ -2529,19 +2530,19 @@ static void snd_seq_info_dump_subscribers(struct snd_info_buffer *buffer,
 					  struct snd_seq_port_subs_info *group,
 					  int is_src, char *msg)
 {
-	struct list_head *p;
+	struct hlist_node *p;
 	struct snd_seq_subscribers *s;
 	int count = 0;
 
 	guard(rwsem_read)(&group->list_mutex);
-	if (list_empty(&group->list_head))
+	if (hlist_empty(&group->list_head))
 		return;
 	snd_iprintf(buffer, msg);
-	list_for_each(p, &group->list_head) {
+	hlist_for_each(p, &group->list_head) {
 		if (is_src)
-			s = list_entry(p, struct snd_seq_subscribers, src_list);
+			s = hlist_entry(p, struct snd_seq_subscribers, src_list);
 		else
-			s = list_entry(p, struct snd_seq_subscribers, dest_list);
+			s = hlist_entry(p, struct snd_seq_subscribers, dest_list);
 		if (count++)
 			snd_iprintf(buffer, ", ");
 		snd_iprintf(buffer, "%d:%d",
diff --git a/sound/core/seq/seq_ports.c b/sound/core/seq/seq_ports.c
index 6612e92d801f..357c72ed0d3b 100644
--- a/sound/core/seq/seq_ports.c
+++ b/sound/core/seq/seq_ports.c
@@ -98,10 +98,9 @@ struct snd_seq_client_port *snd_seq_port_query_nearest(struct snd_seq_client *cl
 /* initialize snd_seq_port_subs_info */
 static void port_subs_info_init(struct snd_seq_port_subs_info *grp)
 {
-	INIT_LIST_HEAD(&grp->list_head);
+	INIT_HLIST_HEAD(&grp->list_head);
 	grp->count = 0;
 	grp->exclusive = 0;
-	rwlock_init(&grp->list_lock);
 	init_rwsem(&grp->list_mutex);
 	grp->open = NULL;
 	grp->close = NULL;
@@ -202,12 +201,12 @@ static void delete_and_unsubscribe_port(struct snd_seq_client *client,
 					bool is_src, bool ack);
 
 static inline struct snd_seq_subscribers *
-get_subscriber(struct list_head *p, bool is_src)
+get_subscriber(struct hlist_node *p, bool is_src)
 {
 	if (is_src)
-		return list_entry(p, struct snd_seq_subscribers, src_list);
+		return hlist_entry(p, struct snd_seq_subscribers, src_list);
 	else
-		return list_entry(p, struct snd_seq_subscribers, dest_list);
+		return hlist_entry(p, struct snd_seq_subscribers, dest_list);
 }
 
 /*
@@ -219,9 +218,9 @@ static void clear_subscriber_list(struct snd_seq_client *client,
 				  struct snd_seq_port_subs_info *grp,
 				  int is_src)
 {
-	struct list_head *p, *n;
+	struct hlist_node *p, *n;
 
-	list_for_each_safe(p, n, &grp->list_head) {
+	hlist_for_each_safe(p, n, &grp->list_head) {
 		struct snd_seq_subscribers *subs;
 
 		subs = get_subscriber(p, is_src);
@@ -238,13 +237,13 @@ static void clear_subscriber_list(struct snd_seq_client *client,
 			 * remove the subscriber info
 			 */
 			if (atomic_dec_and_test(&subs->ref_count))
-				kfree(subs);
+				kfree_rcu(subs, rcu);
 			continue;
 		}
 
 		/* ok we got the connected port */
 		delete_and_unsubscribe_port(c, aport, subs, !is_src, true);
-		kfree(subs);
+		kfree_rcu(subs, rcu);
 	}
 }
 
@@ -499,20 +498,20 @@ static int check_and_subscribe_port(struct snd_seq_client *client,
 				    bool is_src, bool exclusive, bool ack)
 {
 	struct snd_seq_port_subs_info *grp;
-	struct list_head *p;
+	struct hlist_node *p;
 	struct snd_seq_subscribers *s;
 	int err;
 
 	grp = is_src ? &port->c_src : &port->c_dest;
 	guard(rwsem_write)(&grp->list_mutex);
 	if (exclusive) {
-		if (!list_empty(&grp->list_head))
+		if (!hlist_empty(&grp->list_head))
 			return -EBUSY;
 	} else {
 		if (grp->exclusive)
 			return -EBUSY;
 		/* check whether already exists */
-		list_for_each(p, &grp->list_head) {
+		hlist_for_each(p, &grp->list_head) {
 			s = get_subscriber(p, is_src);
 			if (match_subs_info(&subs->info, &s->info))
 				return -EBUSY;
@@ -526,11 +525,10 @@ static int check_and_subscribe_port(struct snd_seq_client *client,
 	}
 
 	/* add to list */
-	guard(write_lock_irq)(&grp->list_lock);
 	if (is_src)
-		list_add_tail(&subs->src_list, &grp->list_head);
+		hlist_add_tail_rcu(&subs->src_list, &grp->list_head);
 	else
-		list_add_tail(&subs->dest_list, &grp->list_head);
+		hlist_add_tail_rcu(&subs->dest_list, &grp->list_head);
 	grp->exclusive = exclusive;
 	atomic_inc(&subs->ref_count);
 
@@ -544,17 +542,15 @@ static void __delete_and_unsubscribe_port(struct snd_seq_client *client,
 					  bool is_src, bool ack)
 {
 	struct snd_seq_port_subs_info *grp;
-	struct list_head *list;
+	struct hlist_node *list;
 	bool empty;
 
 	grp = is_src ? &port->c_src : &port->c_dest;
 	list = is_src ? &subs->src_list : &subs->dest_list;
-	scoped_guard(write_lock_irq, &grp->list_lock) {
-		empty = list_empty(list);
-		if (!empty)
-			list_del_init(list);
-		grp->exclusive = 0;
-	}
+	empty = hlist_unhashed(list);
+	if (!empty)
+		hlist_del_init_rcu(list);
+	grp->exclusive = 0;
 
 	if (!empty)
 		unsubscribe_port(client, port, grp, &subs->info, ack);
@@ -590,8 +586,8 @@ int snd_seq_port_connect(struct snd_seq_client *connector,
 
 	subs->info = *info;
 	atomic_set(&subs->ref_count, 0);
-	INIT_LIST_HEAD(&subs->src_list);
-	INIT_LIST_HEAD(&subs->dest_list);
+	INIT_HLIST_NODE(&subs->src_list);
+	INIT_HLIST_NODE(&subs->dest_list);
 
 	exclusive = !!(info->flags & SNDRV_SEQ_PORT_SUBS_EXCLUSIVE);
 
@@ -612,7 +608,7 @@ int snd_seq_port_connect(struct snd_seq_client *connector,
 	delete_and_unsubscribe_port(src_client, src_port, subs, true,
 				    connector->number != src_client->number);
  error:
-	kfree(subs);
+	kfree_rcu(subs, rcu);
 	return err;
 }
 
@@ -633,7 +629,7 @@ int snd_seq_port_disconnect(struct snd_seq_client *connector,
 	 */
 	scoped_guard(rwsem_write, &dest->list_mutex) {
 		/* look for the connection */
-		list_for_each_entry(subs, &dest->list_head, dest_list) {
+		hlist_for_each_entry(subs, &dest->list_head, dest_list) {
 			if (match_subs_info(info, &subs->info)) {
 				__delete_and_unsubscribe_port(dest_client, dest_port,
 							      subs, false,
@@ -648,7 +644,7 @@ int snd_seq_port_disconnect(struct snd_seq_client *connector,
 
 	delete_and_unsubscribe_port(src_client, src_port, subs, true,
 				    connector->number != src_client->number);
-	kfree(subs);
+	kfree_rcu(subs, rcu);
 	return 0;
 }
 
@@ -662,7 +658,7 @@ int snd_seq_port_get_subscription(struct snd_seq_port_subs_info *src_grp,
 	int err = -ENOENT;
 
 	guard(rwsem_read)(&src_grp->list_mutex);
-	list_for_each_entry(s, &src_grp->list_head, src_list) {
+	hlist_for_each_entry(s, &src_grp->list_head, src_list) {
 		if (addr_match(dest_addr, &s->info.dest)) {
 			*subs = s->info;
 			err = 0;
diff --git a/sound/core/seq/seq_ports.h b/sound/core/seq/seq_ports.h
index b689c0f4867c..12ad86bf1489 100644
--- a/sound/core/seq/seq_ports.h
+++ b/sound/core/seq/seq_ports.h
@@ -28,17 +28,17 @@
 
 struct snd_seq_subscribers {
 	struct snd_seq_port_subscribe info;	/* additional info */
-	struct list_head src_list;	/* link of sources */
-	struct list_head dest_list;	/* link of destinations */
+	struct hlist_node src_list;	/* link of sources */
+	struct hlist_node dest_list;	/* link of destinations */
 	atomic_t ref_count;
+	struct rcu_head rcu;		/* for deferred free */
 };
 
 struct snd_seq_port_subs_info {
-	struct list_head list_head;	/* list of subscribed ports */
+	struct hlist_head list_head;	/* list of subscribed ports */
 	unsigned int count;		/* count of subscribers */
 	unsigned int exclusive: 1;	/* exclusive mode */
 	struct rw_semaphore list_mutex;
-	rwlock_t list_lock;
 	int (*open)(void *private_data, struct snd_seq_port_subscribe *info);
 	int (*close)(void *private_data, struct snd_seq_port_subscribe *info);
 };
-- 
2.55.0


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

* [PATCH 2/5] ALSA: seq: Use RCU for the client port list
  2026-08-10 13:37 [PATCH 0/5] ALSA: seq: Optimization with RCU Takashi Iwai
  2026-08-10 13:37 ` [PATCH 1/5] ALSA: seq: Use RCU for the port subscriber list Takashi Iwai
@ 2026-08-10 13:37 ` Takashi Iwai
  2026-08-10 13:37 ` [PATCH 3/5] ALSA: seq: Use RCU for the client table Takashi Iwai
                   ` (2 subsequent siblings)
  4 siblings, 0 replies; 6+ messages in thread
From: Takashi Iwai @ 2026-08-10 13:37 UTC (permalink / raw)
  To: linux-sound; +Cc: linux-kernel

Each sequencer client keeps a list of its ports (ports_list_head)
protected by both an rwlock (ports_lock) and a mutex (ports_mutex).
The rwlock is taken read-side on the event delivery hot path:
snd_seq_port_use_ptr() walks the list to resolve a port on every
dispatched event, while the mutex serializes port creation/deletion.

Ports change rarely but delivery happens constantly, so this is the
another read-mostly case as the port subscriber list.  Convert the
port list traversal to RCU and drop the rwlock entirely; the existing
ports_mutex keeps serializing the writers.  The atomic delivery path
(snd_seq_port_use_ptr(), snd_seq_port_query_nearest()) now runs
lock-free under rcu_read_lock() instead of contending on the shared
rwlock.

The writers switch to list_add_tail_rcu()/list_del_rcu().
snd_seq_insert_port() now stores the port number and name before
publishing the node so RCU readers only ever observe a fully
initialized port.  One drawback is that snd_seq_delete_all_ports()
drops the O(1) splice trick and unlinks each port individually,
though: the splice repointed the last port's ->next away from the list
head, which would send a concurrent lockless reader off the end of the
list.

Unlike the subscriber objects, ports are not freed via kfree_rcu():
port_delete() must drain outstanding use_lock references (and run
private_free()) synchronously.  The rwlock previously guaranteed that
no reader could take a new use_lock reference once the port was
unlinked -- list_del under write_lock excluded snd_use_lock_use()
under read_lock.  list_del_rcu() offers no such exclusion, so a reader
still traversing the list can grab a reference after the unlink.
port_delete() therefore calls synchronize_rcu() after the port has
been unlinked and before snd_use_lock_sync(): once the grace period
elapses no new reference can appear, and the existing drain then frees
the port safely.

Dropping write_lock_irq() from the writers is safe: no writer runs in
atomic/IRQ context, and the sole atomic reader now uses RCU, which is
IRQ-safe.

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

diff --git a/sound/core/seq/seq_clientmgr.c b/sound/core/seq/seq_clientmgr.c
index 77f5020f1873..b7cf14e3ddb3 100644
--- a/sound/core/seq/seq_clientmgr.c
+++ b/sound/core/seq/seq_clientmgr.c
@@ -212,7 +212,6 @@ static struct snd_seq_client *seq_create_client1(int client_index, int poolsize)
 	}
 	client->type = NO_CLIENT;
 	snd_use_lock_init(&client->use_lock);
-	rwlock_init(&client->ports_lock);
 	mutex_init(&client->ports_mutex);
 	INIT_LIST_HEAD(&client->ports_list_head);
 	mutex_init(&client->ioctl_mutex);
diff --git a/sound/core/seq/seq_clientmgr.h b/sound/core/seq/seq_clientmgr.h
index feea8bb7d987..d7ffc5c1ed61 100644
--- a/sound/core/seq/seq_clientmgr.h
+++ b/sound/core/seq/seq_clientmgr.h
@@ -49,7 +49,6 @@ struct snd_seq_client {
 	/* ports */
 	int num_ports;		/* number of ports */
 	struct list_head ports_list_head;
-	rwlock_t ports_lock;
 	struct mutex ports_mutex;
 	struct mutex ioctl_mutex;
 	int convert32;		/* convert 32->64bit */
diff --git a/sound/core/seq/seq_ports.c b/sound/core/seq/seq_ports.c
index 357c72ed0d3b..eb67eb0eeb14 100644
--- a/sound/core/seq/seq_ports.c
+++ b/sound/core/seq/seq_ports.c
@@ -48,8 +48,8 @@ struct snd_seq_client_port *snd_seq_port_use_ptr(struct snd_seq_client *client,
 
 	if (client == NULL)
 		return NULL;
-	guard(read_lock)(&client->ports_lock);
-	list_for_each_entry(port, &client->ports_list_head, list) {
+	guard(rcu)();
+	list_for_each_entry_rcu(port, &client->ports_list_head, list) {
 		if (port->addr.port == num) {
 			if (port->closing)
 				break; /* deleting now */
@@ -71,8 +71,8 @@ struct snd_seq_client_port *snd_seq_port_query_nearest(struct snd_seq_client *cl
 
 	num = pinfo->addr.port;
 	found = NULL;
-	guard(read_lock)(&client->ports_lock);
-	list_for_each_entry(port, &client->ports_list_head, list) {
+	guard(rcu)();
+	list_for_each_entry_rcu(port, &client->ports_list_head, list) {
 		if ((port->capability & SNDRV_SEQ_PORT_CAP_INACTIVE) &&
 		    !check_inactive)
 			continue; /* skip inactive ports */
@@ -153,7 +153,6 @@ int snd_seq_insert_port(struct snd_seq_client *client, int port,
 
 	num = max(port, 0);
 	guard(mutex)(&client->ports_mutex);
-	guard(write_lock_irq)(&client->ports_lock);
 	struct list_head *insert_before = &client->ports_list_head;
 	list_for_each_entry(p, &client->ports_list_head, list) {
 		if (p->addr.port == port)
@@ -165,12 +164,13 @@ int snd_seq_insert_port(struct snd_seq_client *client, int port,
 		if (port < 0) /* auto-probe mode */
 			num = p->addr.port + 1;
 	}
-	/* insert the new port */
-	list_add_tail(&new_port->list, insert_before);
-	client->num_ports++;
+	/* finish initializing the port before publishing it to RCU readers */
 	new_port->addr.port = num;	/* store the port number in the port */
 	if (!new_port->name[0])
 		sprintf(new_port->name, "port-%d", num);
+	/* insert the new port */
+	list_add_tail_rcu(&new_port->list, insert_before);
+	client->num_ports++;
 
 	return num;
 }
@@ -253,7 +253,13 @@ static int port_delete(struct snd_seq_client *client,
 {
 	/* set closing flag and wait for all port access are gone */
 	port->closing = 1;
-	snd_use_lock_sync(&port->use_lock); 
+	/* the port has already been unlinked from the client's port list;
+	 * wait for a grace period so that RCU readers still traversing the
+	 * list can no longer take a new use_lock reference, then drain the
+	 * outstanding references before freeing
+	 */
+	synchronize_rcu();
+	snd_use_lock_sync(&port->use_lock);
 
 	/* clear subscribers info */
 	clear_subscriber_list(client, port, &port->c_src, true);
@@ -276,11 +282,10 @@ int snd_seq_delete_port(struct snd_seq_client *client, int port)
 	struct snd_seq_client_port *found = NULL, *p;
 
 	scoped_guard(mutex, &client->ports_mutex) {
-		guard(write_lock_irq)(&client->ports_lock);
 		list_for_each_entry(p, &client->ports_list_head, list) {
 			if (p->addr.port == port) {
 				/* ok found.  delete from the list at first */
-				list_del(&p->list);
+				list_del_rcu(&p->list);
 				client->num_ports--;
 				found = p;
 				break;
@@ -296,26 +301,16 @@ int snd_seq_delete_port(struct snd_seq_client *client, int port)
 /* delete the all ports belonging to the given client */
 int snd_seq_delete_all_ports(struct snd_seq_client *client)
 {
-	struct list_head deleted_list;
 	struct snd_seq_client_port *port, *tmp;
-	
-	/* move the port list to deleted_list, and
-	 * clear the port list in the client data.
+
+	/* unlink and delete each port; port_delete() waits for an RCU grace
+	 * period before draining the port, so concurrent lockless readers can
+	 * no longer take a new use_lock reference on it
 	 */
 	guard(mutex)(&client->ports_mutex);
-	scoped_guard(write_lock_irq, &client->ports_lock) {
-		if (!list_empty(&client->ports_list_head)) {
-			list_add(&deleted_list, &client->ports_list_head);
-			list_del_init(&client->ports_list_head);
-		} else {
-			INIT_LIST_HEAD(&deleted_list);
-		}
-		client->num_ports = 0;
-	}
-
-	/* remove each port in deleted_list */
-	list_for_each_entry_safe(port, tmp, &deleted_list, list) {
-		list_del(&port->list);
+	list_for_each_entry_safe(port, tmp, &client->ports_list_head, list) {
+		list_del_rcu(&port->list);
+		client->num_ports--;
 		snd_seq_system_client_ev_port_exit(port->addr.client, port->addr.port);
 		port_delete(client, port);
 	}
-- 
2.55.0


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

* [PATCH 3/5] ALSA: seq: Use RCU for the client table
  2026-08-10 13:37 [PATCH 0/5] ALSA: seq: Optimization with RCU Takashi Iwai
  2026-08-10 13:37 ` [PATCH 1/5] ALSA: seq: Use RCU for the port subscriber list Takashi Iwai
  2026-08-10 13:37 ` [PATCH 2/5] ALSA: seq: Use RCU for the client port list Takashi Iwai
@ 2026-08-10 13:37 ` Takashi Iwai
  2026-08-10 13:37 ` [PATCH 4/5] ALSA: seq: Use RCU for the virmidi file list Takashi Iwai
  2026-08-10 13:37 ` [PATCH 5/5] ALSA: seq: Use RCU for the UMP client output substream Takashi Iwai
  4 siblings, 0 replies; 6+ messages in thread
From: Takashi Iwai @ 2026-08-10 13:37 UTC (permalink / raw)
  To: linux-sound; +Cc: linux-kernel

The sequencer keeps a global table of clients (clienttab[]) indexed by
client id, protected by the global clients_lock spinlock.  The lookup
snd_seq_client_use_ptr() reads a slot and takes a use_lock reference on
the client, and this runs on the event delivery hot path: every
dispatched event resolves its destination (and often source) client
through it.  The spinlock's only job on the read side is to make the
"pointer is non-NULL" test and the reference increment indivisible with
respect to the writer that nulls the slot and then drains the refcount.

Clients come and go rarely but delivery happens constantly, so this is
yet another read-mostly case as the port and subscriber lists.
Convert the table to RCU: the read side now runs lock-free under
rcu_read_lock() and takes the use_lock reference via
rcu_dereference(), removing contention on the single global spinlock
from the delivery path.  The writers keep clients_lock (still needed
to serialize slot allocation) and publish / unpublish via
rcu_assign_pointer(); creation and destruction remain serialized at a
higher level by register_mutex.

As with the ports, the client is not freed via kfree_rcu(): its lifetime
is governed by the use_lock refcount drained in seq_free_client1().
list_del under the old spinlock excluded a concurrent lookup from taking
a new reference once the slot was nulled; rcu_assign_pointer(NULL) offers
no such exclusion, so a reader still holding the old pointer can grab a
reference after the unpublish.  seq_free_client1() therefore calls
synchronize_rcu() after nulling the slot and before snd_use_lock_sync():
once the grace period elapses no new reference can appear, and the
existing drain then frees the client safely.

clienttablock[] keeps its slot-reservation role (create/free are
serialized by register_mutex); its read on the lookup path only gates
module autoload, so a lockless read is harmless.  Dropping the spinlock
from the read path is safe: clients_lock is now taken only by the
process-context writers, and the sole atomic reader uses RCU, which is
IRQ-safe.

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

diff --git a/sound/core/seq/seq_clientmgr.c b/sound/core/seq/seq_clientmgr.c
index b7cf14e3ddb3..d4cac594bc8f 100644
--- a/sound/core/seq/seq_clientmgr.c
+++ b/sound/core/seq/seq_clientmgr.c
@@ -59,7 +59,7 @@ static DEFINE_MUTEX(register_mutex);
  * client table
  */
 static char clienttablock[SNDRV_SEQ_MAX_CLIENTS];
-static struct snd_seq_client *clienttab[SNDRV_SEQ_MAX_CLIENTS];
+static struct snd_seq_client __rcu *clienttab[SNDRV_SEQ_MAX_CLIENTS];
 static struct snd_seq_usage client_usage;
 
 /*
@@ -95,15 +95,23 @@ static inline int snd_seq_write_pool_allocated(struct snd_seq_client *client)
 	return snd_seq_total_cells(client->pool) > 0;
 }
 
-/* return pointer to client structure for specified id */
-static struct snd_seq_client *clientptr(int clientid)
+/* return pointer to client structure for specified id; call under RCU read-lock */
+static struct snd_seq_client *__clientptr(int clientid)
 {
 	if (clientid < 0 || clientid >= SNDRV_SEQ_MAX_CLIENTS) {
 		pr_debug("ALSA: seq: oops. Trying to get pointer to client %d\n",
 			   clientid);
 		return NULL;
 	}
-	return clienttab[clientid];
+	return rcu_dereference_check(clienttab[clientid],
+				    lockdep_is_held(&clients_lock));
+}
+
+/* return pointer to client structure for specified id */
+static struct snd_seq_client *clientptr(int clientid)
+{
+	guard(rcu)();
+	return __clientptr(clientid);
 }
 
 static struct snd_seq_client *client_use_ptr(int clientid, bool load_module)
@@ -115,8 +123,8 @@ static struct snd_seq_client *client_use_ptr(int clientid, bool load_module)
 			   clientid);
 		return NULL;
 	}
-	scoped_guard(spinlock_irqsave, &clients_lock) {
-		client = clientptr(clientid);
+	scoped_guard(rcu) {
+		client = __clientptr(clientid);
 		if (client)
 			return snd_seq_client_ref(client);
 		if (clienttablock[clientid])
@@ -150,8 +158,8 @@ static struct snd_seq_client *client_use_ptr(int clientid, bool load_module)
 				snd_seq_device_load_drivers();
 			}
 		}
-		scoped_guard(spinlock_irqsave, &clients_lock) {
-			client = clientptr(clientid);
+		scoped_guard(rcu) {
+			client = __clientptr(clientid);
 			if (client)
 				return snd_seq_client_ref(client);
 		}
@@ -223,14 +231,17 @@ static struct snd_seq_client *seq_create_client1(int client_index, int poolsize)
 			for (c = SNDRV_SEQ_DYNAMIC_CLIENTS_BEGIN;
 			     c < SNDRV_SEQ_MAX_CLIENTS;
 			     c++) {
-				if (clienttab[c] || clienttablock[c])
+				if (rcu_access_pointer(clienttab[c]) || clienttablock[c])
 					continue;
-				clienttab[client->number = c] = client;
+				client->number = c;
+				rcu_assign_pointer(clienttab[c], client);
 				return client;
 			}
 		} else {
-			if (clienttab[client_index] == NULL && !clienttablock[client_index]) {
-				clienttab[client->number = client_index] = client;
+			if (rcu_access_pointer(clienttab[client_index]) == NULL &&
+			    !clienttablock[client_index]) {
+				client->number = client_index;
+				rcu_assign_pointer(clienttab[client_index], client);
 				return client;
 			}
 		}
@@ -248,10 +259,16 @@ static int seq_free_client1(struct snd_seq_client *client)
 		return 0;
 	scoped_guard(spinlock_irq, &clients_lock) {
 		clienttablock[client->number] = 1;
-		clienttab[client->number] = NULL;
+		rcu_assign_pointer(clienttab[client->number], NULL);
 	}
 	snd_seq_delete_all_ports(client);
 	snd_seq_queue_client_leave(client->number);
+	/* the client has been unpublished from the table; wait for a grace
+	 * period so that lockless readers (snd_seq_client_use_ptr()) that
+	 * observed the old pointer can no longer take a new use_lock
+	 * reference, then drain the outstanding references before freeing
+	 */
+	synchronize_rcu();
 	snd_use_lock_sync(&client->use_lock);
 	if (client->pool)
 		snd_seq_pool_delete(&client->pool);
-- 
2.55.0


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

* [PATCH 4/5] ALSA: seq: Use RCU for the virmidi file list
  2026-08-10 13:37 [PATCH 0/5] ALSA: seq: Optimization with RCU Takashi Iwai
                   ` (2 preceding siblings ...)
  2026-08-10 13:37 ` [PATCH 3/5] ALSA: seq: Use RCU for the client table Takashi Iwai
@ 2026-08-10 13:37 ` Takashi Iwai
  2026-08-10 13:37 ` [PATCH 5/5] ALSA: seq: Use RCU for the UMP client output substream Takashi Iwai
  4 siblings, 0 replies; 6+ messages in thread
From: Takashi Iwai @ 2026-08-10 13:37 UTC (permalink / raw)
  To: linux-sound; +Cc: linux-kernel

Each virmidi device keeps a list of its opened input files (filelist)
protected by both an rwlock (filelist_lock) and a rw_semaphore
(filelist_sem).  snd_virmidi_dev_receive_event() walks the list on the
sequencer event input path -- read_lock() when the event is delivered in
atomic context, down_read() otherwise -- decoding each incoming event
into the file's rawmidi buffer.  The writers (input open/close) take both
locks to add/remove entries.

This is another typical dual-lock read-mostly pattern as the port
subscriber list: files are opened/closed rarely while the receive
callback runs per event.  Let's convert the traversal to RCU and drop
the rwlock; the existing filelist_sem keeps serializing the writers.
The atomic input path now runs lock-free under rcu_read_lock(), and
both readers share a single list_for_each_entry_rcu() (valid under the
rwsem via lockdep_is_held()).  The writers switch to
list_add_tail_rcu() / list_del_rcu().

snd_virmidi_input_close() freed the entry (parser and struct)
immediately after list_del.  A concurrent lockless reader in the atomic
path may still be dereferencing it, so the close path now waits for an
RCU grace period after list_del_rcu() before freeing; synchronize_rcu()
is used rather than kfree_rcu() because the parser must also be released
after the grace period, not just the struct.  Non-atomic readers are
already excluded by the down_write, so only the atomic RCU readers need
the grace period.

Dropping write_lock_irq() from the writers is safe: no writer runs in
atomic/IRQ context, and the sole atomic reader now uses RCU, which is
IRQ-safe.

Signed-off-by: Takashi Iwai <tiwai@suse.de>
---
 include/sound/seq_virmidi.h  |  1 -
 sound/core/seq/seq_virmidi.c | 19 +++++++++++--------
 2 files changed, 11 insertions(+), 9 deletions(-)

diff --git a/include/sound/seq_virmidi.h b/include/sound/seq_virmidi.h
index 56a3f38df8c3..359cb363369d 100644
--- a/include/sound/seq_virmidi.h
+++ b/include/sound/seq_virmidi.h
@@ -46,7 +46,6 @@ struct snd_virmidi_dev {
 	int client;			/* created/attached client */
 	int port;			/* created/attached port */
 	unsigned int flags;		/* SNDRV_VIRMIDI_* */
-	rwlock_t filelist_lock;
 	struct rw_semaphore filelist_sem;
 	struct list_head filelist;
 };
diff --git a/sound/core/seq/seq_virmidi.c b/sound/core/seq/seq_virmidi.c
index 982828650d41..6208bf7f57bf 100644
--- a/sound/core/seq/seq_virmidi.c
+++ b/sound/core/seq/seq_virmidi.c
@@ -78,10 +78,11 @@ static int snd_virmidi_dev_receive_event(struct snd_virmidi_dev *rdev,
 	int len;
 
 	if (atomic)
-		read_lock(&rdev->filelist_lock);
+		rcu_read_lock();
 	else
 		down_read(&rdev->filelist_sem);
-	list_for_each_entry(vmidi, &rdev->filelist, list) {
+	list_for_each_entry_rcu(vmidi, &rdev->filelist, list,
+				lockdep_is_held(&rdev->filelist_sem)) {
 		if (!READ_ONCE(vmidi->trigger))
 			continue;
 		if (ev->type == SNDRV_SEQ_EVENT_SYSEX) {
@@ -96,7 +97,7 @@ static int snd_virmidi_dev_receive_event(struct snd_virmidi_dev *rdev,
 		}
 	}
 	if (atomic)
-		read_unlock(&rdev->filelist_lock);
+		rcu_read_unlock();
 	else
 		up_read(&rdev->filelist_sem);
 
@@ -200,8 +201,7 @@ static int snd_virmidi_input_open(struct snd_rawmidi_substream *substream)
 	vmidi->port = rdev->port;	
 	runtime->private_data = vmidi;
 	scoped_guard(rwsem_write, &rdev->filelist_sem) {
-		guard(write_lock_irq)(&rdev->filelist_lock);
-		list_add_tail(&vmidi->list, &rdev->filelist);
+		list_add_tail_rcu(&vmidi->list, &rdev->filelist);
 	}
 	vmidi->rdev = rdev;
 	return 0;
@@ -243,9 +243,13 @@ static int snd_virmidi_input_close(struct snd_rawmidi_substream *substream)
 	struct snd_virmidi *vmidi = substream->runtime->private_data;
 
 	scoped_guard(rwsem_write, &rdev->filelist_sem) {
-		guard(write_lock_irq)(&rdev->filelist_lock);
-		list_del(&vmidi->list);
+		list_del_rcu(&vmidi->list);
 	}
+	/* wait for a grace period so that lockless readers in the atomic
+	 * delivery path (snd_virmidi_dev_receive_event()) are no longer
+	 * traversing this entry before its parser and memory are freed
+	 */
+	synchronize_rcu();
 	snd_midi_event_free(vmidi->parser);
 	substream->runtime->private_data = NULL;
 	kfree(vmidi);
@@ -508,7 +512,6 @@ int snd_virmidi_new(struct snd_card *card, int device, struct snd_rawmidi **rrmi
 	rdev->device = device;
 	rdev->client = -1;
 	init_rwsem(&rdev->filelist_sem);
-	rwlock_init(&rdev->filelist_lock);
 	INIT_LIST_HEAD(&rdev->filelist);
 	rdev->seq_mode = SNDRV_VIRMIDI_SEQ_DISPATCH;
 	rmidi->private_data = rdev;
-- 
2.55.0


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

* [PATCH 5/5] ALSA: seq: Use RCU for the UMP client output substream
  2026-08-10 13:37 [PATCH 0/5] ALSA: seq: Optimization with RCU Takashi Iwai
                   ` (3 preceding siblings ...)
  2026-08-10 13:37 ` [PATCH 4/5] ALSA: seq: Use RCU for the virmidi file list Takashi Iwai
@ 2026-08-10 13:37 ` Takashi Iwai
  4 siblings, 0 replies; 6+ messages in thread
From: Takashi Iwai @ 2026-08-10 13:37 UTC (permalink / raw)
  To: linux-sound; +Cc: linux-kernel

The UMP sequencer client protects its output rawmidi file (out_rfile)
with an rwlock (output_lock).  seq_ump_process_event(), the port's
event_input callback, reads out_rfile.output under read_lock on every
delivered UMP event, while the open/close paths (serialized by
ump->open_mutex) publish and clear out_rfile under write_lock.

Output is opened/closed only on the subscribe/use lifecycle while
delivery happens per event, so this is another read-mostly hot path.
Convert it to RCU and drop the rwlock.  out_rfile is an embedded struct
rather than a pointer, so instead of restructuring it, add an
RCU-protected shadow of the substream (out_substream) for the reader;
out_rfile itself becomes writer-only state accessed solely under
open_mutex.  The reader now runs lock-free under rcu_read_lock() via
rcu_dereference(), and open publishes the substream with
rcu_assign_pointer().

On close the substream is cleared with rcu_assign_pointer(NULL) and the
rawmidi is released only after synchronize_rcu(), so no reader in the
delivery path can still be writing to the substream when
snd_rawmidi_kernel_release() runs.

Dropping write_lock_irqsave() from the writers is safe: they run in
process context under open_mutex, and the sole atomic reader now uses
RCU, which is IRQ-safe.

Signed-off-by: Takashi Iwai <tiwai@suse.de>
---
 sound/core/seq/seq_ump_client.c | 38 +++++++++++++++++++--------------
 1 file changed, 22 insertions(+), 16 deletions(-)

diff --git a/sound/core/seq/seq_ump_client.c b/sound/core/seq/seq_ump_client.c
index ccd93599b493..4c1e81db376f 100644
--- a/sound/core/seq/seq_ump_client.c
+++ b/sound/core/seq/seq_ump_client.c
@@ -37,8 +37,11 @@ struct seq_ump_client {
 	struct snd_ump_endpoint *ump;	/* assigned endpoint */
 	int seq_client;			/* sequencer client id */
 	int opened[2];			/* current opens for each direction */
-	rwlock_t output_lock;		/* protects out_rfile output access */
 	struct snd_rawmidi_file out_rfile; /* rawmidi for output */
+	/* RCU-protected shadow of out_rfile.output for the delivery hot path;
+	 * out_rfile itself is only touched by open/close under open_mutex
+	 */
+	struct snd_rawmidi_substream __rcu *out_substream;
 	struct seq_ump_input_buffer input; /* input parser context */
 	void *ump_info[SNDRV_UMP_MAX_BLOCKS + 1]; /* shadow of seq client ump_info */
 	struct work_struct group_notify_work; /* FB change notification */
@@ -89,8 +92,8 @@ static int seq_ump_process_event(struct snd_seq_event *ev, int direct,
 	unsigned char type;
 	int len;
 
-	guard(read_lock_irqsave)(&client->output_lock);
-	substream = client->out_rfile.output;
+	guard(rcu)();
+	substream = rcu_dereference(client->out_substream);
 	if (!substream)
 		return -ENODEV;
 	if (!snd_seq_ev_is_ump(ev))
@@ -108,19 +111,22 @@ static int seq_ump_process_event(struct snd_seq_event *ev, int direct,
 static int seq_ump_client_open(struct seq_ump_client *client, int dir)
 {
 	struct snd_ump_endpoint *ump = client->ump;
-	struct snd_rawmidi_file rfile = {};
 	int err;
 
 	guard(mutex)(&ump->open_mutex);
 	if (dir == STR_OUT && !client->opened[dir]) {
+		/* out_rfile is only accessed under open_mutex; the delivery
+		 * path reads out_substream via RCU, so open into out_rfile
+		 * directly and publish the substream afterwards
+		 */
 		err = snd_rawmidi_kernel_open(&ump->core, 0,
 					      SNDRV_RAWMIDI_LFLG_OUTPUT |
 					      SNDRV_RAWMIDI_LFLG_APPEND,
-					      &rfile);
+					      &client->out_rfile);
 		if (err < 0)
 			return err;
-		scoped_guard(write_lock_irqsave, &client->output_lock)
-			client->out_rfile = rfile;
+		rcu_assign_pointer(client->out_substream,
+				   client->out_rfile.output);
 	}
 	client->opened[dir]++;
 	return 0;
@@ -130,17 +136,18 @@ static int seq_ump_client_open(struct seq_ump_client *client, int dir)
 static int seq_ump_client_close(struct seq_ump_client *client, int dir)
 {
 	struct snd_ump_endpoint *ump = client->ump;
-	struct snd_rawmidi_file rfile = {};
 
 	guard(mutex)(&ump->open_mutex);
 	if (!--client->opened[dir]) {
-		if (dir == STR_OUT) {
-			scoped_guard(write_lock_irqsave, &client->output_lock) {
-				rfile = client->out_rfile;
-				client->out_rfile = (struct snd_rawmidi_file){};
-			}
-			if (rfile.rmidi)
-				snd_rawmidi_kernel_release(&rfile);
+		if (dir == STR_OUT && client->out_rfile.rmidi) {
+			rcu_assign_pointer(client->out_substream, NULL);
+			/* wait for a grace period so that no reader in the
+			 * delivery path is still writing to the substream
+			 * before it is released
+			 */
+			synchronize_rcu();
+			snd_rawmidi_kernel_release(&client->out_rfile);
+			client->out_rfile = (struct snd_rawmidi_file){};
 		}
 	}
 	return 0;
@@ -480,7 +487,6 @@ static int snd_seq_ump_probe(struct snd_seq_device *dev)
 
 	INIT_WORK(&client->group_notify_work, handle_group_notify);
 	client->ump = ump;
-	rwlock_init(&client->output_lock);
 
 	client->seq_client =
 		snd_seq_create_kernel_client(card, ump->core.device,
-- 
2.55.0


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

end of thread, other threads:[~2026-08-10 13:37 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2026-08-10 13:37 [PATCH 0/5] ALSA: seq: Optimization with RCU Takashi Iwai
2026-08-10 13:37 ` [PATCH 1/5] ALSA: seq: Use RCU for the port subscriber list Takashi Iwai
2026-08-10 13:37 ` [PATCH 2/5] ALSA: seq: Use RCU for the client port list Takashi Iwai
2026-08-10 13:37 ` [PATCH 3/5] ALSA: seq: Use RCU for the client table Takashi Iwai
2026-08-10 13:37 ` [PATCH 4/5] ALSA: seq: Use RCU for the virmidi file list Takashi Iwai
2026-08-10 13:37 ` [PATCH 5/5] ALSA: seq: Use RCU for the UMP client output substream Takashi Iwai

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox

all inboxes | Powered by JetHome®