[PATCH 4/5] ALSA: seq: Use RCU for the virmidi file list

[Takashi Iwai <tiwai@suse.de>]

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