On Mon, 02 Aug 2021 17:21:17 +0200, folkert wrote:
[ kernel bug if repeatingly aconnect'ing midi devices ]
Does this happen if you do reconnect of kernel sequencer client? You can use snd-virmidi as well as snd-dummy. I'm asking it because it'll simplify the test a lot, which will be almost self-contained.
Like this?
root@lappiemctopface:~# aplaymidi -l Port Client name Port name 14:0 Midi Through Midi Through Port-0 20:0 Virtual Raw MIDI 1-0 VirMIDI 1-0 21:0 Virtual Raw MIDI 1-1 VirMIDI 1-1 22:0 Virtual Raw MIDI 1-2 VirMIDI 1-2 23:0 Virtual Raw MIDI 1-3 VirMIDI 1-3 128:0 rtpmidi lappiemctopface Network 128:1 rtpmidi lappiemctopface metronoom 128:2 rtpmidi lappiemctopface AppleMidi2IPMidiBridge 128:3 rtpmidi lappiemctopface oensoens 130:0 FLUID Synth (11462) Synth input port (11462:0)
and then:
root@lappiemctopface:~# cat test.sh while true do aconnect 20:0 21:0 aconnect -d 20:0 21:0 done
root@lappiemctopface:~# for i in `seq 0 3` ; do (./test.sh &) ; done
This hard locks-up my laptop: it doesn't even respond to capslock (led on/off) anymore nor the ctrl+prtscr+alt+b combination.
Thanks, that's really helpful! I see the possible race now.
Could you try the quick fix below?
Takashi
--- --- a/sound/core/seq/seq_ports.c +++ b/sound/core/seq/seq_ports.c @@ -200,6 +200,17 @@ get_subscriber(struct list_head *p, bool is_src) return list_entry(p, struct snd_seq_subscribers, dest_list); }
+static void subscriber_get(struct snd_seq_subscribers *subs) +{ + atomic_inc(&subs->ref_count); +} + +static void subscriber_put(struct snd_seq_subscribers *subs) +{ + if (atomic_dec_and_test(&subs->ref_count)) + kfree(subs); +} + /* * remove all subscribers on the list * this is called from port_delete, for each src and dest list. @@ -228,8 +239,7 @@ static void clear_subscriber_list(struct snd_seq_client *client, * we decrease the counter, and when both ports are deleted * remove the subscriber info */ - if (atomic_dec_and_test(&subs->ref_count)) - kfree(subs); + subscriber_put(subs); continue; }
@@ -489,6 +499,8 @@ static int check_and_subscribe_port(struct snd_seq_client *client, s = get_subscriber(p, is_src); if (match_subs_info(&subs->info, &s->info)) goto __error; + if (!subs->ready) + goto __error; } }
@@ -505,7 +517,7 @@ static int check_and_subscribe_port(struct snd_seq_client *client, else list_add_tail(&subs->dest_list, &grp->list_head); grp->exclusive = exclusive; - atomic_inc(&subs->ref_count); + subscriber_get(subs); write_unlock_irq(&grp->list_lock); err = 0;
@@ -536,6 +548,7 @@ static void delete_and_unsubscribe_port(struct snd_seq_client *client, if (!empty) unsubscribe_port(client, port, grp, &subs->info, ack); up_write(&grp->list_mutex); + subscriber_put(subs); }
/* connect two ports */ @@ -555,7 +568,7 @@ int snd_seq_port_connect(struct snd_seq_client *connector, return -ENOMEM;
subs->info = *info; - atomic_set(&subs->ref_count, 0); + atomic_set(&subs->ref_count, 1); INIT_LIST_HEAD(&subs->src_list); INIT_LIST_HEAD(&subs->dest_list);
@@ -572,13 +585,14 @@ int snd_seq_port_connect(struct snd_seq_client *connector, if (err < 0) goto error_dest;
+ subs->ready = true; return 0;
error_dest: delete_and_unsubscribe_port(src_client, src_port, subs, true, connector->number != src_client->number); error: - kfree(subs); + subscriber_put(subs); return err; }
@@ -597,8 +611,8 @@ int snd_seq_port_disconnect(struct snd_seq_client *connector, down_write(&src->list_mutex); /* look for the connection */ list_for_each_entry(subs, &src->list_head, src_list) { - if (match_subs_info(info, &subs->info)) { - atomic_dec(&subs->ref_count); /* mark as not ready */ + if (match_subs_info(info, &subs->info) && subs->ready) { + subs->ready = false; err = 0; break; } @@ -611,7 +625,7 @@ int snd_seq_port_disconnect(struct snd_seq_client *connector, connector->number != src_client->number); delete_and_unsubscribe_port(dest_client, dest_port, subs, false, connector->number != dest_client->number); - kfree(subs); + subscriber_put(subs); return 0; }
diff --git a/sound/core/seq/seq_ports.h b/sound/core/seq/seq_ports.h index b1f2c4943174..d9644d5f109e 100644 --- a/sound/core/seq/seq_ports.h +++ b/sound/core/seq/seq_ports.h @@ -30,6 +30,7 @@ struct snd_seq_subscribers { struct list_head src_list; /* link of sources */ struct list_head dest_list; /* link of destinations */ atomic_t ref_count; + bool ready; };
struct snd_seq_port_subs_info {