[alsa-devel] [PATCH 0/6] sequencer fixes
One bug fix, and various unimportant cleanups for -next.
include/sound/seq_kernel.h | 9 +-------- sound/core/seq/seq_clientmgr.c | 3 +-- sound/core/seq/seq_dummy.c | 31 ------------------------------- sound/core/seq/seq_ports.c | 9 +++------ sound/core/seq/seq_ports.h | 1 - 5 files changed, 5 insertions(+), 48 deletions(-)
When the last subscriber to a "Through" port has been removed, the subscribed destination ports might still be active, so it would be wrong to send "all sounds off" and "reset controller" events to them. The proper place for such a shutdown would be the closing of the actual MIDI port (and close_substream() in rawmidi.c already can do this).
This also fixes a deadlock when dummy_unuse() tries to send events to its own port that is already locked because it is being freed.
Reported-by: Peter Billam peter@www.pjb.com.au Fixes: 1da177e4c3f4 Cc: stable@vger.kernel.org Signed-off-by: Clemens Ladisch clemens@ladisch.de --- sound/core/seq/seq_dummy.c | 31 ------------------------------- 1 file changed, 31 deletions(-)
diff --git a/sound/core/seq/seq_dummy.c b/sound/core/seq/seq_dummy.c index ec667f1..5d905d9 100644 --- a/sound/core/seq/seq_dummy.c +++ b/sound/core/seq/seq_dummy.c @@ -82,36 +82,6 @@ struct snd_seq_dummy_port { static int my_client = -1;
/* - * unuse callback - send ALL_SOUNDS_OFF and RESET_CONTROLLERS events - * to subscribers. - * Note: this callback is called only after all subscribers are removed. - */ -static int -dummy_unuse(void *private_data, struct snd_seq_port_subscribe *info) -{ - struct snd_seq_dummy_port *p; - int i; - struct snd_seq_event ev; - - p = private_data; - memset(&ev, 0, sizeof(ev)); - if (p->duplex) - ev.source.port = p->connect; - else - ev.source.port = p->port; - ev.dest.client = SNDRV_SEQ_ADDRESS_SUBSCRIBERS; - ev.type = SNDRV_SEQ_EVENT_CONTROLLER; - for (i = 0; i < 16; i++) { - ev.data.control.channel = i; - ev.data.control.param = MIDI_CTL_ALL_SOUNDS_OFF; - snd_seq_kernel_client_dispatch(p->client, &ev, 0, 0); - ev.data.control.param = MIDI_CTL_RESET_CONTROLLERS; - snd_seq_kernel_client_dispatch(p->client, &ev, 0, 0); - } - return 0; -} - -/* * event input callback - just redirect events to subscribers */ static int @@ -175,7 +145,6 @@ create_port(int idx, int type) | SNDRV_SEQ_PORT_TYPE_PORT; memset(&pcb, 0, sizeof(pcb)); pcb.owner = THIS_MODULE; - pcb.unuse = dummy_unuse; pcb.event_input = dummy_input; pcb.private_free = dummy_free; pcb.private_data = rec;
At Sun, 25 Jan 2015 14:34:29 +0100, Clemens Ladisch wrote:
When the last subscriber to a "Through" port has been removed, the subscribed destination ports might still be active, so it would be wrong to send "all sounds off" and "reset controller" events to them. The proper place for such a shutdown would be the closing of the actual MIDI port (and close_substream() in rawmidi.c already can do this).
This also fixes a deadlock when dummy_unuse() tries to send events to its own port that is already locked because it is being freed.
Reported-by: Peter Billam peter@www.pjb.com.au Fixes: 1da177e4c3f4 Cc: stable@vger.kernel.org Signed-off-by: Clemens Ladisch clemens@ladisch.de
Applied to for-linus branch, but I dropped the fixes tag that points to 2.6.12-rc2, which is obviously not so useful.
thanks,
Takashi
sound/core/seq/seq_dummy.c | 31 ------------------------------- 1 file changed, 31 deletions(-)
diff --git a/sound/core/seq/seq_dummy.c b/sound/core/seq/seq_dummy.c index ec667f1..5d905d9 100644 --- a/sound/core/seq/seq_dummy.c +++ b/sound/core/seq/seq_dummy.c @@ -82,36 +82,6 @@ struct snd_seq_dummy_port { static int my_client = -1;
/*
- unuse callback - send ALL_SOUNDS_OFF and RESET_CONTROLLERS events
- to subscribers.
- Note: this callback is called only after all subscribers are removed.
- */
-static int -dummy_unuse(void *private_data, struct snd_seq_port_subscribe *info) -{
- struct snd_seq_dummy_port *p;
- int i;
- struct snd_seq_event ev;
- p = private_data;
- memset(&ev, 0, sizeof(ev));
- if (p->duplex)
ev.source.port = p->connect;- else
ev.source.port = p->port;- ev.dest.client = SNDRV_SEQ_ADDRESS_SUBSCRIBERS;
- ev.type = SNDRV_SEQ_EVENT_CONTROLLER;
- for (i = 0; i < 16; i++) {
ev.data.control.channel = i;ev.data.control.param = MIDI_CTL_ALL_SOUNDS_OFF;snd_seq_kernel_client_dispatch(p->client, &ev, 0, 0);ev.data.control.param = MIDI_CTL_RESET_CONTROLLERS;snd_seq_kernel_client_dispatch(p->client, &ev, 0, 0);- }
- return 0;
-}
-/*
- event input callback - just redirect events to subscribers
*/ static int @@ -175,7 +145,6 @@ create_port(int idx, int type) | SNDRV_SEQ_PORT_TYPE_PORT; memset(&pcb, 0, sizeof(pcb)); pcb.owner = THIS_MODULE;
- pcb.unuse = dummy_unuse; pcb.event_input = dummy_input; pcb.private_free = dummy_free; pcb.private_data = rec;
Due to SNDRV_SEQ_ADDRESS_BROADCAST, not all 256 port number values can be used.
Signed-off-by: Clemens Ladisch clemens@ladisch.de --- sound/core/seq/seq_clientmgr.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/sound/core/seq/seq_clientmgr.c b/sound/core/seq/seq_clientmgr.c index 225c7315..808918a 100644 --- a/sound/core/seq/seq_clientmgr.c +++ b/sound/core/seq/seq_clientmgr.c @@ -1133,7 +1133,7 @@ static int snd_seq_ioctl_system_info(struct snd_seq_client *client, void __user /* fill the info fields */ info.queues = SNDRV_SEQ_MAX_QUEUES; info.clients = SNDRV_SEQ_MAX_CLIENTS; - info.ports = 256; /* fixed limit */ + info.ports = SNDRV_SEQ_MAX_PORTS; info.channels = 256; /* fixed limit */ info.cur_clients = client_usage.cur; info.cur_queues = snd_seq_queue_get_cur_queues();
At Sun, 25 Jan 2015 14:34:57 +0100, Clemens Ladisch wrote:
Due to SNDRV_SEQ_ADDRESS_BROADCAST, not all 256 port number values can be used.
Signed-off-by: Clemens Ladisch clemens@ladisch.de
Thanks, applied all the rest five patches to for-next.
Takashi
sound/core/seq/seq_clientmgr.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/sound/core/seq/seq_clientmgr.c b/sound/core/seq/seq_clientmgr.c index 225c7315..808918a 100644 --- a/sound/core/seq/seq_clientmgr.c +++ b/sound/core/seq/seq_clientmgr.c @@ -1133,7 +1133,7 @@ static int snd_seq_ioctl_system_info(struct snd_seq_client *client, void __user /* fill the info fields */ info.queues = SNDRV_SEQ_MAX_QUEUES; info.clients = SNDRV_SEQ_MAX_CLIENTS;
- info.ports = 256; /* fixed limit */
- info.ports = SNDRV_SEQ_MAX_PORTS; info.channels = 256; /* fixed limit */ info.cur_clients = client_usage.cur; info.cur_queues = snd_seq_queue_get_cur_queues();
Signed-off-by: Clemens Ladisch clemens@ladisch.de --- sound/core/seq/seq_ports.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/sound/core/seq/seq_ports.c b/sound/core/seq/seq_ports.c index 794a341..52b279b 100644 --- a/sound/core/seq/seq_ports.c +++ b/sound/core/seq/seq_ports.c @@ -134,7 +134,7 @@ struct snd_seq_client_port *snd_seq_create_port(struct snd_seq_client *client, if (snd_BUG_ON(!client)) return NULL;
- if (client->num_ports >= SNDRV_SEQ_MAX_PORTS - 1) { + if (client->num_ports >= SNDRV_SEQ_MAX_PORTS) { pr_warn("ALSA: seq: too many ports for client %d\n", client->number); return NULL; }
Signed-off-by: Clemens Ladisch clemens@ladisch.de --- include/sound/seq_kernel.h | 6 ------ 1 file changed, 6 deletions(-)
diff --git a/include/sound/seq_kernel.h b/include/sound/seq_kernel.h index eea5400..ab8ddd9 100644 --- a/include/sound/seq_kernel.h +++ b/include/sound/seq_kernel.h @@ -27,9 +27,6 @@ typedef struct snd_seq_real_time snd_seq_real_time_t; typedef union snd_seq_timestamp snd_seq_timestamp_t;
-/* maximum number of events dequeued per schedule interval */ -#define SNDRV_SEQ_MAX_DEQUEUE 50 - /* maximum number of queues */ #define SNDRV_SEQ_MAX_QUEUES 8
@@ -42,9 +39,6 @@ typedef union snd_seq_timestamp snd_seq_timestamp_t; /* max number of events in memory pool */ #define SNDRV_SEQ_MAX_EVENTS 2000
-/* default number of events in memory chunk */ -#define SNDRV_SEQ_DEFAULT_CHUNK_EVENTS 64 - /* default number of events in memory pool */ #define SNDRV_SEQ_DEFAULT_EVENTS 500
Signed-off-by: Clemens Ladisch clemens@ladisch.de --- include/sound/seq_kernel.h | 1 - sound/core/seq/seq_clientmgr.c | 1 - sound/core/seq/seq_ports.c | 7 ++----- sound/core/seq/seq_ports.h | 1 - 4 files changed, 2 insertions(+), 8 deletions(-)
diff --git a/include/sound/seq_kernel.h b/include/sound/seq_kernel.h index ab8ddd9..f1c8e94 100644 --- a/include/sound/seq_kernel.h +++ b/include/sound/seq_kernel.h @@ -64,7 +64,6 @@ struct snd_seq_port_callback { int (*unuse)(void *private_data, struct snd_seq_port_subscribe *info); int (*event_input)(struct snd_seq_event *ev, int direct, void *private_data, int atomic, int hop); void (*private_free)(void *private_data); - unsigned int callback_all; /* call subscribe callbacks at each connection/disconnection */ /*...*/ };
diff --git a/sound/core/seq/seq_clientmgr.c b/sound/core/seq/seq_clientmgr.c index 808918a..29182f5 100644 --- a/sound/core/seq/seq_clientmgr.c +++ b/sound/core/seq/seq_clientmgr.c @@ -1279,7 +1279,6 @@ static int snd_seq_ioctl_create_port(struct snd_seq_client *client, port->owner = callback->owner; port->private_data = callback->private_data; port->private_free = callback->private_free; - port->callback_all = callback->callback_all; port->event_input = callback->event_input; port->c_src.open = callback->subscribe; port->c_src.close = callback->unsubscribe; diff --git a/sound/core/seq/seq_ports.c b/sound/core/seq/seq_ports.c index 52b279b..46ff593 100644 --- a/sound/core/seq/seq_ports.c +++ b/sound/core/seq/seq_ports.c @@ -411,9 +411,6 @@ int snd_seq_get_port_info(struct snd_seq_client_port * port, * invoked. * This feature is useful if these callbacks are associated with * initialization or termination of devices (see seq_midi.c). - * - * If callback_all option is set, the callback function is invoked - * at each connection/disconnection. */
static int subscribe_port(struct snd_seq_client *client, @@ -427,7 +424,7 @@ static int subscribe_port(struct snd_seq_client *client, if (!try_module_get(port->owner)) return -EFAULT; grp->count++; - if (grp->open && (port->callback_all || grp->count == 1)) { + if (grp->open && grp->count == 1) { err = grp->open(port->private_data, info); if (err < 0) { module_put(port->owner); @@ -452,7 +449,7 @@ static int unsubscribe_port(struct snd_seq_client *client, if (! grp->count) return -EINVAL; grp->count--; - if (grp->close && (port->callback_all || grp->count == 0)) + if (grp->close && grp->count == 0) err = grp->close(port->private_data, info); if (send_ack && client->type == USER_CLIENT) snd_seq_client_notify_subscription(port->addr.client, port->addr.port, diff --git a/sound/core/seq/seq_ports.h b/sound/core/seq/seq_ports.h index 9d71171..26bd71f 100644 --- a/sound/core/seq/seq_ports.h +++ b/sound/core/seq/seq_ports.h @@ -73,7 +73,6 @@ struct snd_seq_client_port { int atomic, int hop); void (*private_free)(void *private_data); void *private_data; - unsigned int callback_all : 1; unsigned int closing : 1; unsigned int timestamping: 1; unsigned int time_real: 1;
Queues are used both for scheduling playback events and for assigning timestamps to recorded events, so it is easy to need quite a lot of them, especially on a multi-user system. Additionally, the actual queue objects are allocated dynamically, so it does not really make sense to have a low limit. Increase it to something still sane.
Signed-off-by: Clemens Ladisch clemens@ladisch.de --- include/sound/seq_kernel.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/include/sound/seq_kernel.h b/include/sound/seq_kernel.h index f1c8e94..18a2ac5 100644 --- a/include/sound/seq_kernel.h +++ b/include/sound/seq_kernel.h @@ -28,7 +28,7 @@ typedef struct snd_seq_real_time snd_seq_real_time_t; typedef union snd_seq_timestamp snd_seq_timestamp_t;
/* maximum number of queues */ -#define SNDRV_SEQ_MAX_QUEUES 8 +#define SNDRV_SEQ_MAX_QUEUES 32
/* max number of concurrent clients */ #define SNDRV_SEQ_MAX_CLIENTS 192
participants (2)
-
Clemens Ladisch -
Takashi Iwai