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 {
