[PATCH 0/6] sequencer fixes

[PATCH 0/6] sequencer fixes

[Clemens Ladisch]

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(-)

[PATCH 1/6] ALSA: seq-dummy: remove deadlock-causing events on close

[Clemens Ladisch]

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;

[PATCH 2/6] ALSA: seq: correctly report maximum number of ports

[Clemens Ladisch]

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();

[PATCH 3/6] ALSA: seq: fix off-by-one error in port limit check

[Clemens Ladisch]

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;
 	}

[PATCH 4/6] ALSA: seq: remove unused symbols

[Clemens Ladisch]

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

[PATCH 5/6] ALSA: seq: remove unused callback_all field

[Clemens Ladisch]

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;

[PATCH 6/6] ALSA: seq: increase the maximum number of queues

[Clemens Ladisch]

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

Re: [PATCH 1/6] ALSA: seq-dummy: remove deadlock-causing events on close

[Takashi Iwai]

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;
> 

Re: [PATCH 2/6] ALSA: seq: correctly report maximum number of ports

[Takashi Iwai]

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();
>