[PATCH 1/5] ALSA: seq: Use RCU for the port subscriber list

[Takashi Iwai]

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@xxxxxxx>
---
 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