All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 00/39] seq: obsolete change of address limit
@ 2016-08-07  9:48 Takashi Sakamoto
  2016-08-07  9:48 ` [PATCH 01/39] ALSA: seq: add const qualifier to table of functions for ioctl Takashi Sakamoto
                   ` (38 more replies)
  0 siblings, 39 replies; 63+ messages in thread
From: Takashi Sakamoto @ 2016-08-07  9:48 UTC (permalink / raw)
  To: clemens, tiwai; +Cc: alsa-devel

Hi,

ALSA sequencer core has two types of client; user application and kernel
driver. The core allows both types of client to do relevant operations,
thus it's required for the core to handle data in both user and kernel
spaces.

Currently, this is achieved by changing address limit of running task.
This is a well-known technique to suppress address check, while it's
just a suppression and unfriendly to readers or static code parsers. 

This patchset obsoletes the usage. In process context, data in user space
is once copied to kernel space, then operated and copied to user space.
As a result, actual operations for each ioctl command handle data in kernel
space, and '__user' qualifier is useless.

In this series, patch 1-8 just apply above design. The rest changes each
operation following to the design. Therefore, static code parser like
sparce generates warnings temporarily in a way to apply these patches.

There's a concern of this solution. The data for ioctl is always copied to
kernel space or to kernel space even when it's read-only or write-only. But
this brings no severe issue as long as I read ALSA sequencer core.

(This patchset is a part of my work to introduce EPIPE into ALSA rawmidi
core.)

Takashi Sakamoto (39):
  ALSA: seq: add const qualifier to table of functions for ioctl
  ALSA: seq: apply shorter name for file local functions
  ALSA: seq: fulfill callback entry for ioctl
  ALSA: seq: copy ioctl data from user space to kernel stack
  ALSA: seq: add documentation for snd_seq_kernel_client_ctl
  ALSA: seq: obsolete address mode in compatibility layer
  ALSA: seq: obsolete change of address limit in in-kernel path for
    ioctl
  ALSA: seq: obsolete address limit helper
  ALSA: seq: optimize pversion function to new design
  ALSA: seq: optimize client_id function to new design
  ALSA: seq: optimize system_info function to new design
  ALSA: seq: optimize running mode function to new design
  ALSA: seq: optimize client_info function to new design
  ALSA: seq: optimize set_client_info function to new design
  ALSA: seq: optimize create_port function to new design
  ALSA: seq: optimize delete_port function to new design
  ALSA: seq: optimize get_port_info function to new design
  ALSA: seq: optimize seq_port_info function to new design
  ALSA: seq: optimize subscribe_port function to new design
  ALSA: seq: optimize unsubscribe_port function to new design
  ALSA: seq: optimize create_queue function to new design
  ALSA: seq: optimize delete_queue function to new design
  ALSA: seq: optimize get_queue_info function to new design
  ALSA: seq: optimize seq_queue_info function to new design
  ALSA: seq: optimize get_named_queue function to new design
  ALSA: seq: optimize get_queue_status function to new design
  ALSA: seq: optimize get_queue_tempo function to new design
  ALSA: seq: optimize set_queue_tempo function to new design
  ALSA: seq: optimize get_queue_timer function to new design
  ALSA: seq: optimize seq_queue_timer function to new design
  ALSA: seq: optimize get_queue_client function to new design
  ALSA: seq: optimize set_queue_client function to new design
  ALSA: seq: optimize get_client_pool function to new design
  ALSA: seq: optimize seq_client_pool function to new design
  ALSA: seq: optimize remove_events function to new design
  ALSA: seq: optimize get_subscription function to new design
  ALSA: seq: optimize query_subs function to new design
  ALSA: seq: optimize query_next_client function to new design
  ALSA: seq: optimize query_next_port function to new design

 sound/core/seq/seq_clientmgr.c | 854 +++++++++++++++++++----------------------
 sound/core/seq/seq_compat.c    |  26 +-
 2 files changed, 407 insertions(+), 473 deletions(-)

-- 
2.7.4

^ permalink raw reply	[flat|nested] 63+ messages in thread

* [PATCH 01/39] ALSA: seq: add const qualifier to table of functions for ioctl
  2016-08-07  9:48 [PATCH 00/39] seq: obsolete change of address limit Takashi Sakamoto
@ 2016-08-07  9:48 ` Takashi Sakamoto
  2016-08-07  9:48 ` [PATCH 02/39] ALSA: seq: apply shorter name for file local functions Takashi Sakamoto
                   ` (37 subsequent siblings)
  38 siblings, 0 replies; 63+ messages in thread
From: Takashi Sakamoto @ 2016-08-07  9:48 UTC (permalink / raw)
  To: clemens, tiwai; +Cc: alsa-devel

Each entry in this table is never changed in runtime. This commit moves it
from .data to .rodata section.

Signed-off-by: Takashi Sakamoto <o-takashi@sakamocchi.jp>
---
 sound/core/seq/seq_clientmgr.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/sound/core/seq/seq_clientmgr.c b/sound/core/seq/seq_clientmgr.c
index d6d9419..42be1e5 100644
--- a/sound/core/seq/seq_clientmgr.c
+++ b/sound/core/seq/seq_clientmgr.c
@@ -2168,7 +2168,7 @@ static int snd_seq_ioctl_query_next_port(struct snd_seq_client *client,
 
 /* -------------------------------------------------------- */
 
-static struct seq_ioctl_table {
+static const struct seq_ioctl_table {
 	unsigned int cmd;
 	int (*func)(struct snd_seq_client *client, void __user * arg);
 } ioctl_tables[] = {
@@ -2207,7 +2207,7 @@ static struct seq_ioctl_table {
 static int snd_seq_do_ioctl(struct snd_seq_client *client, unsigned int cmd,
 			    void __user *arg)
 {
-	struct seq_ioctl_table *p;
+	const struct seq_ioctl_table *p;
 
 	switch (cmd) {
 	case SNDRV_SEQ_IOCTL_PVERSION:
-- 
2.7.4

^ permalink raw reply related	[flat|nested] 63+ messages in thread

* [PATCH 02/39] ALSA: seq: apply shorter name for file local functions
  2016-08-07  9:48 [PATCH 00/39] seq: obsolete change of address limit Takashi Sakamoto
  2016-08-07  9:48 ` [PATCH 01/39] ALSA: seq: add const qualifier to table of functions for ioctl Takashi Sakamoto
@ 2016-08-07  9:48 ` Takashi Sakamoto
  2016-08-08  6:56   ` Takashi Iwai
  2016-08-07  9:48 ` [PATCH 03/39] ALSA: seq: fulfill callback entry for ioctl Takashi Sakamoto
                   ` (36 subsequent siblings)
  38 siblings, 1 reply; 63+ messages in thread
From: Takashi Sakamoto @ 2016-08-07  9:48 UTC (permalink / raw)
  To: clemens, tiwai; +Cc: alsa-devel

Exported symbols should have 'snd_' prefix to avoid name conflict, while
file local symbols have no need to have. Shorter names are generally
preferable due to '80 characters in a line' rule.

This commit renames some local functions.

Signed-off-by: Takashi Sakamoto <o-takashi@sakamocchi.jp>
---
 sound/core/seq/seq_clientmgr.c | 180 +++++++++++++++++++++--------------------
 sound/core/seq/seq_compat.c    |  19 ++---
 2 files changed, 101 insertions(+), 98 deletions(-)

diff --git a/sound/core/seq/seq_clientmgr.c b/sound/core/seq/seq_clientmgr.c
index 42be1e5..1331103 100644
--- a/sound/core/seq/seq_clientmgr.c
+++ b/sound/core/seq/seq_clientmgr.c
@@ -1130,7 +1130,8 @@ static unsigned int snd_seq_poll(struct file *file, poll_table * wait)
 
 
 /* SYSTEM_INFO ioctl() */
-static int snd_seq_ioctl_system_info(struct snd_seq_client *client, void __user *arg)
+static int seq_ioctl_system_info(struct snd_seq_client *client,
+				 void __user *arg)
 {
 	struct snd_seq_system_info info;
 
@@ -1150,7 +1151,8 @@ static int snd_seq_ioctl_system_info(struct snd_seq_client *client, void __user
 
 
 /* RUNNING_MODE ioctl() */
-static int snd_seq_ioctl_running_mode(struct snd_seq_client *client, void __user *arg)
+static int seq_ioctl_running_mode(struct snd_seq_client *client,
+				  void __user *arg)
 {
 	struct snd_seq_running_info info;
 	struct snd_seq_client *cptr;
@@ -1213,8 +1215,8 @@ static void get_client_info(struct snd_seq_client *cptr,
 	memset(info->reserved, 0, sizeof(info->reserved));
 }
 
-static int snd_seq_ioctl_get_client_info(struct snd_seq_client *client,
-					 void __user *arg)
+static int seq_ioctl_get_client_info(struct snd_seq_client *client,
+				     void __user *arg)
 {
 	struct snd_seq_client *cptr;
 	struct snd_seq_client_info client_info;
@@ -1237,8 +1239,8 @@ static int snd_seq_ioctl_get_client_info(struct snd_seq_client *client,
 
 
 /* CLIENT_INFO ioctl() */
-static int snd_seq_ioctl_set_client_info(struct snd_seq_client *client,
-					 void __user *arg)
+static int seq_ioctl_set_client_info(struct snd_seq_client *client,
+				     void __user *arg)
 {
 	struct snd_seq_client_info client_info;
 
@@ -1267,8 +1269,8 @@ static int snd_seq_ioctl_set_client_info(struct snd_seq_client *client,
 /* 
  * CREATE PORT ioctl() 
  */
-static int snd_seq_ioctl_create_port(struct snd_seq_client *client,
-				     void __user *arg)
+static int seq_ioctl_create_port(struct snd_seq_client *client,
+				 void __user *arg)
 {
 	struct snd_seq_client_port *port;
 	struct snd_seq_port_info info;
@@ -1317,8 +1319,8 @@ static int snd_seq_ioctl_create_port(struct snd_seq_client *client,
 /* 
  * DELETE PORT ioctl() 
  */
-static int snd_seq_ioctl_delete_port(struct snd_seq_client *client,
-				     void __user *arg)
+static int seq_ioctl_delete_port(struct snd_seq_client *client,
+				 void __user *arg)
 {
 	struct snd_seq_port_info info;
 	int err;
@@ -1341,8 +1343,8 @@ static int snd_seq_ioctl_delete_port(struct snd_seq_client *client,
 /* 
  * GET_PORT_INFO ioctl() (on any client) 
  */
-static int snd_seq_ioctl_get_port_info(struct snd_seq_client *client,
-				       void __user *arg)
+static int seq_ioctl_get_port_info(struct snd_seq_client *client,
+				   void __user *arg)
 {
 	struct snd_seq_client *cptr;
 	struct snd_seq_client_port *port;
@@ -1374,8 +1376,8 @@ static int snd_seq_ioctl_get_port_info(struct snd_seq_client *client,
 /* 
  * SET_PORT_INFO ioctl() (only ports on this/own client) 
  */
-static int snd_seq_ioctl_set_port_info(struct snd_seq_client *client,
-				       void __user *arg)
+static int seq_ioctl_set_port_info(struct snd_seq_client *client,
+				   void __user *arg)
 {
 	struct snd_seq_client_port *port;
 	struct snd_seq_port_info info;
@@ -1452,8 +1454,8 @@ int snd_seq_client_notify_subscription(int client, int port,
 /* 
  * add to port's subscription list IOCTL interface 
  */
-static int snd_seq_ioctl_subscribe_port(struct snd_seq_client *client,
-					void __user *arg)
+static int seq_ioctl_subscribe_port(struct snd_seq_client *client,
+				    void __user *arg)
 {
 	int result = -EINVAL;
 	struct snd_seq_client *receiver = NULL, *sender = NULL;
@@ -1497,8 +1499,8 @@ static int snd_seq_ioctl_subscribe_port(struct snd_seq_client *client,
 /* 
  * remove from port's subscription list 
  */
-static int snd_seq_ioctl_unsubscribe_port(struct snd_seq_client *client,
-					  void __user *arg)
+static int seq_ioctl_unsubscribe_port(struct snd_seq_client *client,
+				      void __user *arg)
 {
 	int result = -ENXIO;
 	struct snd_seq_client *receiver = NULL, *sender = NULL;
@@ -1539,8 +1541,8 @@ static int snd_seq_ioctl_unsubscribe_port(struct snd_seq_client *client,
 
 
 /* CREATE_QUEUE ioctl() */
-static int snd_seq_ioctl_create_queue(struct snd_seq_client *client,
-				      void __user *arg)
+static int seq_ioctl_create_queue(struct snd_seq_client *client,
+				  void __user *arg)
 {
 	struct snd_seq_queue_info info;
 	int result;
@@ -1574,8 +1576,8 @@ static int snd_seq_ioctl_create_queue(struct snd_seq_client *client,
 }
 
 /* DELETE_QUEUE ioctl() */
-static int snd_seq_ioctl_delete_queue(struct snd_seq_client *client,
-				      void __user *arg)
+static int seq_ioctl_delete_queue(struct snd_seq_client *client,
+				  void __user *arg)
 {
 	struct snd_seq_queue_info info;
 
@@ -1586,8 +1588,8 @@ static int snd_seq_ioctl_delete_queue(struct snd_seq_client *client,
 }
 
 /* GET_QUEUE_INFO ioctl() */
-static int snd_seq_ioctl_get_queue_info(struct snd_seq_client *client,
-					void __user *arg)
+static int seq_ioctl_get_queue_info(struct snd_seq_client *client,
+				    void __user *arg)
 {
 	struct snd_seq_queue_info info;
 	struct snd_seq_queue *q;
@@ -1613,8 +1615,8 @@ static int snd_seq_ioctl_get_queue_info(struct snd_seq_client *client,
 }
 
 /* SET_QUEUE_INFO ioctl() */
-static int snd_seq_ioctl_set_queue_info(struct snd_seq_client *client,
-					void __user *arg)
+static int seq_ioctl_set_queue_info(struct snd_seq_client *client,
+				    void __user *arg)
 {
 	struct snd_seq_queue_info info;
 	struct snd_seq_queue *q;
@@ -1649,7 +1651,8 @@ static int snd_seq_ioctl_set_queue_info(struct snd_seq_client *client,
 }
 
 /* GET_NAMED_QUEUE ioctl() */
-static int snd_seq_ioctl_get_named_queue(struct snd_seq_client *client, void __user *arg)
+static int seq_ioctl_get_named_queue(struct snd_seq_client *client,
+				     void __user *arg)
 {
 	struct snd_seq_queue_info info;
 	struct snd_seq_queue *q;
@@ -1672,8 +1675,8 @@ static int snd_seq_ioctl_get_named_queue(struct snd_seq_client *client, void __u
 }
 
 /* GET_QUEUE_STATUS ioctl() */
-static int snd_seq_ioctl_get_queue_status(struct snd_seq_client *client,
-					  void __user *arg)
+static int seq_ioctl_get_queue_status(struct snd_seq_client *client,
+				      void __user *arg)
 {
 	struct snd_seq_queue_status status;
 	struct snd_seq_queue *queue;
@@ -1706,8 +1709,8 @@ static int snd_seq_ioctl_get_queue_status(struct snd_seq_client *client,
 
 
 /* GET_QUEUE_TEMPO ioctl() */
-static int snd_seq_ioctl_get_queue_tempo(struct snd_seq_client *client,
-					 void __user *arg)
+static int seq_ioctl_get_queue_tempo(struct snd_seq_client *client,
+				     void __user *arg)
 {
 	struct snd_seq_queue_tempo tempo;
 	struct snd_seq_queue *queue;
@@ -1746,8 +1749,8 @@ int snd_seq_set_queue_tempo(int client, struct snd_seq_queue_tempo *tempo)
 
 EXPORT_SYMBOL(snd_seq_set_queue_tempo);
 
-static int snd_seq_ioctl_set_queue_tempo(struct snd_seq_client *client,
-					 void __user *arg)
+static int seq_ioctl_set_queue_tempo(struct snd_seq_client *client,
+				     void __user *arg)
 {
 	int result;
 	struct snd_seq_queue_tempo tempo;
@@ -1761,8 +1764,8 @@ static int snd_seq_ioctl_set_queue_tempo(struct snd_seq_client *client,
 
 
 /* GET_QUEUE_TIMER ioctl() */
-static int snd_seq_ioctl_get_queue_timer(struct snd_seq_client *client,
-					 void __user *arg)
+static int seq_ioctl_get_queue_timer(struct snd_seq_client *client,
+				     void __user *arg)
 {
 	struct snd_seq_queue_timer timer;
 	struct snd_seq_queue *queue;
@@ -1798,8 +1801,8 @@ static int snd_seq_ioctl_get_queue_timer(struct snd_seq_client *client,
 
 
 /* SET_QUEUE_TIMER ioctl() */
-static int snd_seq_ioctl_set_queue_timer(struct snd_seq_client *client,
-					 void __user *arg)
+static int seq_ioctl_set_queue_timer(struct snd_seq_client *client,
+				     void __user *arg)
 {
 	int result = 0;
 	struct snd_seq_queue_timer timer;
@@ -1840,8 +1843,8 @@ static int snd_seq_ioctl_set_queue_timer(struct snd_seq_client *client,
 
 
 /* GET_QUEUE_CLIENT ioctl() */
-static int snd_seq_ioctl_get_queue_client(struct snd_seq_client *client,
-					  void __user *arg)
+static int seq_ioctl_get_queue_client(struct snd_seq_client *client,
+				      void __user *arg)
 {
 	struct snd_seq_queue_client info;
 	int used;
@@ -1862,8 +1865,8 @@ static int snd_seq_ioctl_get_queue_client(struct snd_seq_client *client,
 
 
 /* SET_QUEUE_CLIENT ioctl() */
-static int snd_seq_ioctl_set_queue_client(struct snd_seq_client *client,
-					  void __user *arg)
+static int seq_ioctl_set_queue_client(struct snd_seq_client *client,
+				      void __user *arg)
 {
 	int err;
 	struct snd_seq_queue_client info;
@@ -1877,13 +1880,13 @@ static int snd_seq_ioctl_set_queue_client(struct snd_seq_client *client,
 			return err;
 	}
 
-	return snd_seq_ioctl_get_queue_client(client, arg);
+	return seq_ioctl_get_queue_client(client, arg);
 }
 
 
 /* GET_CLIENT_POOL ioctl() */
-static int snd_seq_ioctl_get_client_pool(struct snd_seq_client *client,
-					 void __user *arg)
+static int seq_ioctl_get_client_pool(struct snd_seq_client *client,
+				     void __user *arg)
 {
 	struct snd_seq_client_pool info;
 	struct snd_seq_client *cptr;
@@ -1917,8 +1920,8 @@ static int snd_seq_ioctl_get_client_pool(struct snd_seq_client *client,
 }
 
 /* SET_CLIENT_POOL ioctl() */
-static int snd_seq_ioctl_set_client_pool(struct snd_seq_client *client,
-					 void __user *arg)
+static int seq_ioctl_set_client_pool(struct snd_seq_client *client,
+				     void __user *arg)
 {
 	struct snd_seq_client_pool info;
 	int rc;
@@ -1957,13 +1960,13 @@ static int snd_seq_ioctl_set_client_pool(struct snd_seq_client *client,
 		client->pool->room  = info.output_room;
 	}
 
-	return snd_seq_ioctl_get_client_pool(client, arg);
+	return seq_ioctl_get_client_pool(client, arg);
 }
 
 
 /* REMOVE_EVENTS ioctl() */
-static int snd_seq_ioctl_remove_events(struct snd_seq_client *client,
-				       void __user *arg)
+static int seq_ioctl_remove_events(struct snd_seq_client *client,
+				   void __user *arg)
 {
 	struct snd_seq_remove_events info;
 
@@ -1992,8 +1995,8 @@ static int snd_seq_ioctl_remove_events(struct snd_seq_client *client,
 /*
  * get subscription info
  */
-static int snd_seq_ioctl_get_subscription(struct snd_seq_client *client,
-					  void __user *arg)
+static int seq_ioctl_get_subscription(struct snd_seq_client *client,
+				      void __user *arg)
 {
 	int result;
 	struct snd_seq_client *sender = NULL;
@@ -2032,8 +2035,7 @@ static int snd_seq_ioctl_get_subscription(struct snd_seq_client *client,
 /*
  * get subscription info - check only its presence
  */
-static int snd_seq_ioctl_query_subs(struct snd_seq_client *client,
-				    void __user *arg)
+static int seq_ioctl_query_subs(struct snd_seq_client *client, void __user *arg)
 {
 	int result = -ENXIO;
 	struct snd_seq_client *cptr = NULL;
@@ -2102,8 +2104,8 @@ static int snd_seq_ioctl_query_subs(struct snd_seq_client *client,
 /*
  * query next client
  */
-static int snd_seq_ioctl_query_next_client(struct snd_seq_client *client,
-					   void __user *arg)
+static int seq_ioctl_query_next_client(struct snd_seq_client *client,
+				       void __user *arg)
 {
 	struct snd_seq_client *cptr = NULL;
 	struct snd_seq_client_info info;
@@ -2134,8 +2136,8 @@ static int snd_seq_ioctl_query_next_client(struct snd_seq_client *client,
 /* 
  * query next port
  */
-static int snd_seq_ioctl_query_next_port(struct snd_seq_client *client,
-					 void __user *arg)
+static int seq_ioctl_query_next_port(struct snd_seq_client *client,
+				     void __user *arg)
 {
 	struct snd_seq_client *cptr;
 	struct snd_seq_client_port *port = NULL;
@@ -2172,39 +2174,39 @@ static const struct seq_ioctl_table {
 	unsigned int cmd;
 	int (*func)(struct snd_seq_client *client, void __user * arg);
 } ioctl_tables[] = {
-	{ SNDRV_SEQ_IOCTL_SYSTEM_INFO, snd_seq_ioctl_system_info },
-	{ SNDRV_SEQ_IOCTL_RUNNING_MODE, snd_seq_ioctl_running_mode },
-	{ SNDRV_SEQ_IOCTL_GET_CLIENT_INFO, snd_seq_ioctl_get_client_info },
-	{ SNDRV_SEQ_IOCTL_SET_CLIENT_INFO, snd_seq_ioctl_set_client_info },
-	{ SNDRV_SEQ_IOCTL_CREATE_PORT, snd_seq_ioctl_create_port },
-	{ SNDRV_SEQ_IOCTL_DELETE_PORT, snd_seq_ioctl_delete_port },
-	{ SNDRV_SEQ_IOCTL_GET_PORT_INFO, snd_seq_ioctl_get_port_info },
-	{ SNDRV_SEQ_IOCTL_SET_PORT_INFO, snd_seq_ioctl_set_port_info },
-	{ SNDRV_SEQ_IOCTL_SUBSCRIBE_PORT, snd_seq_ioctl_subscribe_port },
-	{ SNDRV_SEQ_IOCTL_UNSUBSCRIBE_PORT, snd_seq_ioctl_unsubscribe_port },
-	{ SNDRV_SEQ_IOCTL_CREATE_QUEUE, snd_seq_ioctl_create_queue },
-	{ SNDRV_SEQ_IOCTL_DELETE_QUEUE, snd_seq_ioctl_delete_queue },
-	{ SNDRV_SEQ_IOCTL_GET_QUEUE_INFO, snd_seq_ioctl_get_queue_info },
-	{ SNDRV_SEQ_IOCTL_SET_QUEUE_INFO, snd_seq_ioctl_set_queue_info },
-	{ SNDRV_SEQ_IOCTL_GET_NAMED_QUEUE, snd_seq_ioctl_get_named_queue },
-	{ SNDRV_SEQ_IOCTL_GET_QUEUE_STATUS, snd_seq_ioctl_get_queue_status },
-	{ SNDRV_SEQ_IOCTL_GET_QUEUE_TEMPO, snd_seq_ioctl_get_queue_tempo },
-	{ SNDRV_SEQ_IOCTL_SET_QUEUE_TEMPO, snd_seq_ioctl_set_queue_tempo },
-	{ SNDRV_SEQ_IOCTL_GET_QUEUE_TIMER, snd_seq_ioctl_get_queue_timer },
-	{ SNDRV_SEQ_IOCTL_SET_QUEUE_TIMER, snd_seq_ioctl_set_queue_timer },
-	{ SNDRV_SEQ_IOCTL_GET_QUEUE_CLIENT, snd_seq_ioctl_get_queue_client },
-	{ SNDRV_SEQ_IOCTL_SET_QUEUE_CLIENT, snd_seq_ioctl_set_queue_client },
-	{ SNDRV_SEQ_IOCTL_GET_CLIENT_POOL, snd_seq_ioctl_get_client_pool },
-	{ SNDRV_SEQ_IOCTL_SET_CLIENT_POOL, snd_seq_ioctl_set_client_pool },
-	{ SNDRV_SEQ_IOCTL_GET_SUBSCRIPTION, snd_seq_ioctl_get_subscription },
-	{ SNDRV_SEQ_IOCTL_QUERY_NEXT_CLIENT, snd_seq_ioctl_query_next_client },
-	{ SNDRV_SEQ_IOCTL_QUERY_NEXT_PORT, snd_seq_ioctl_query_next_port },
-	{ SNDRV_SEQ_IOCTL_REMOVE_EVENTS, snd_seq_ioctl_remove_events },
-	{ SNDRV_SEQ_IOCTL_QUERY_SUBS, snd_seq_ioctl_query_subs },
+	{ SNDRV_SEQ_IOCTL_SYSTEM_INFO, seq_ioctl_system_info },
+	{ SNDRV_SEQ_IOCTL_RUNNING_MODE, seq_ioctl_running_mode },
+	{ SNDRV_SEQ_IOCTL_GET_CLIENT_INFO, seq_ioctl_get_client_info },
+	{ SNDRV_SEQ_IOCTL_SET_CLIENT_INFO, seq_ioctl_set_client_info },
+	{ SNDRV_SEQ_IOCTL_CREATE_PORT, seq_ioctl_create_port },
+	{ SNDRV_SEQ_IOCTL_DELETE_PORT, seq_ioctl_delete_port },
+	{ SNDRV_SEQ_IOCTL_GET_PORT_INFO, seq_ioctl_get_port_info },
+	{ SNDRV_SEQ_IOCTL_SET_PORT_INFO, seq_ioctl_set_port_info },
+	{ SNDRV_SEQ_IOCTL_SUBSCRIBE_PORT, seq_ioctl_subscribe_port },
+	{ SNDRV_SEQ_IOCTL_UNSUBSCRIBE_PORT, seq_ioctl_unsubscribe_port },
+	{ SNDRV_SEQ_IOCTL_CREATE_QUEUE, seq_ioctl_create_queue },
+	{ SNDRV_SEQ_IOCTL_DELETE_QUEUE, seq_ioctl_delete_queue },
+	{ SNDRV_SEQ_IOCTL_GET_QUEUE_INFO, seq_ioctl_get_queue_info },
+	{ SNDRV_SEQ_IOCTL_SET_QUEUE_INFO, seq_ioctl_set_queue_info },
+	{ SNDRV_SEQ_IOCTL_GET_NAMED_QUEUE, seq_ioctl_get_named_queue },
+	{ SNDRV_SEQ_IOCTL_GET_QUEUE_STATUS, seq_ioctl_get_queue_status },
+	{ SNDRV_SEQ_IOCTL_GET_QUEUE_TEMPO, seq_ioctl_get_queue_tempo },
+	{ SNDRV_SEQ_IOCTL_SET_QUEUE_TEMPO, seq_ioctl_set_queue_tempo },
+	{ SNDRV_SEQ_IOCTL_GET_QUEUE_TIMER, seq_ioctl_get_queue_timer },
+	{ SNDRV_SEQ_IOCTL_SET_QUEUE_TIMER, seq_ioctl_set_queue_timer },
+	{ SNDRV_SEQ_IOCTL_GET_QUEUE_CLIENT, seq_ioctl_get_queue_client },
+	{ SNDRV_SEQ_IOCTL_SET_QUEUE_CLIENT, seq_ioctl_set_queue_client },
+	{ SNDRV_SEQ_IOCTL_GET_CLIENT_POOL, seq_ioctl_get_client_pool },
+	{ SNDRV_SEQ_IOCTL_SET_CLIENT_POOL, seq_ioctl_set_client_pool },
+	{ SNDRV_SEQ_IOCTL_GET_SUBSCRIPTION, seq_ioctl_get_subscription },
+	{ SNDRV_SEQ_IOCTL_QUERY_NEXT_CLIENT, seq_ioctl_query_next_client },
+	{ SNDRV_SEQ_IOCTL_QUERY_NEXT_PORT, seq_ioctl_query_next_port },
+	{ SNDRV_SEQ_IOCTL_REMOVE_EVENTS, seq_ioctl_remove_events },
+	{ SNDRV_SEQ_IOCTL_QUERY_SUBS, seq_ioctl_query_subs },
 	{ 0, NULL },
 };
 
-static int snd_seq_do_ioctl(struct snd_seq_client *client, unsigned int cmd,
+static int seq_do_ioctl(struct snd_seq_client *client, unsigned int cmd,
 			    void __user *arg)
 {
 	const struct seq_ioctl_table *p;
@@ -2237,7 +2239,7 @@ static long snd_seq_ioctl(struct file *file, unsigned int cmd, unsigned long arg
 	if (snd_BUG_ON(!client))
 		return -ENXIO;
 		
-	return snd_seq_do_ioctl(client, cmd, (void __user *) arg);
+	return seq_do_ioctl(client, cmd, (void __user *) arg);
 }
 
 #ifdef CONFIG_COMPAT
@@ -2437,7 +2439,7 @@ int snd_seq_kernel_client_ctl(int clientid, unsigned int cmd, void *arg)
 	if (client == NULL)
 		return -ENXIO;
 	fs = snd_enter_user();
-	result = snd_seq_do_ioctl(client, cmd, (void __force __user *)arg);
+	result = seq_do_ioctl(client, cmd, (void __force __user *)arg);
 	snd_leave_user(fs);
 	return result;
 }
diff --git a/sound/core/seq/seq_compat.c b/sound/core/seq/seq_compat.c
index 6517590..70d3ddb 100644
--- a/sound/core/seq/seq_compat.c
+++ b/sound/core/seq/seq_compat.c
@@ -42,8 +42,9 @@ struct snd_seq_port_info32 {
 	char reserved[59];		/* for future use */
 };
 
-static int snd_seq_call_port_info_ioctl(struct snd_seq_client *client, unsigned int cmd,
-					struct snd_seq_port_info32 __user *data32)
+static int seq_call_port_info_ioctl(struct snd_seq_client *client,
+				    unsigned int cmd,
+				    struct snd_seq_port_info32 __user *data32)
 {
 	int err = -EFAULT;
 	struct snd_seq_port_info *data;
@@ -60,7 +61,7 @@ static int snd_seq_call_port_info_ioctl(struct snd_seq_client *client, unsigned
 	data->kernel = NULL;
 
 	fs = snd_enter_user();
-	err = snd_seq_do_ioctl(client, cmd, data);
+	err = seq_do_ioctl(client, cmd, data);
 	snd_leave_user(fs);
 	if (err < 0)
 		goto error;
@@ -123,17 +124,17 @@ static long snd_seq_ioctl_compat(struct file *file, unsigned int cmd, unsigned l
 	case SNDRV_SEQ_IOCTL_GET_SUBSCRIPTION:
 	case SNDRV_SEQ_IOCTL_QUERY_NEXT_CLIENT:
 	case SNDRV_SEQ_IOCTL_RUNNING_MODE:
-		return snd_seq_do_ioctl(client, cmd, argp);
+		return seq_do_ioctl(client, cmd, argp);
 	case SNDRV_SEQ_IOCTL_CREATE_PORT32:
-		return snd_seq_call_port_info_ioctl(client, SNDRV_SEQ_IOCTL_CREATE_PORT, argp);
+		return seq_call_port_info_ioctl(client, SNDRV_SEQ_IOCTL_CREATE_PORT, argp);
 	case SNDRV_SEQ_IOCTL_DELETE_PORT32:
-		return snd_seq_call_port_info_ioctl(client, SNDRV_SEQ_IOCTL_DELETE_PORT, argp);
+		return seq_call_port_info_ioctl(client, SNDRV_SEQ_IOCTL_DELETE_PORT, argp);
 	case SNDRV_SEQ_IOCTL_GET_PORT_INFO32:
-		return snd_seq_call_port_info_ioctl(client, SNDRV_SEQ_IOCTL_GET_PORT_INFO, argp);
+		return seq_call_port_info_ioctl(client, SNDRV_SEQ_IOCTL_GET_PORT_INFO, argp);
 	case SNDRV_SEQ_IOCTL_SET_PORT_INFO32:
-		return snd_seq_call_port_info_ioctl(client, SNDRV_SEQ_IOCTL_SET_PORT_INFO, argp);
+		return seq_call_port_info_ioctl(client, SNDRV_SEQ_IOCTL_SET_PORT_INFO, argp);
 	case SNDRV_SEQ_IOCTL_QUERY_NEXT_PORT32:
-		return snd_seq_call_port_info_ioctl(client, SNDRV_SEQ_IOCTL_QUERY_NEXT_PORT, argp);
+		return seq_call_port_info_ioctl(client, SNDRV_SEQ_IOCTL_QUERY_NEXT_PORT, argp);
 	}
 	return -ENOIOCTLCMD;
 }
-- 
2.7.4

^ permalink raw reply related	[flat|nested] 63+ messages in thread

* [PATCH 03/39] ALSA: seq: fulfill callback entry for ioctl
  2016-08-07  9:48 [PATCH 00/39] seq: obsolete change of address limit Takashi Sakamoto
  2016-08-07  9:48 ` [PATCH 01/39] ALSA: seq: add const qualifier to table of functions for ioctl Takashi Sakamoto
  2016-08-07  9:48 ` [PATCH 02/39] ALSA: seq: apply shorter name for file local functions Takashi Sakamoto
@ 2016-08-07  9:48 ` Takashi Sakamoto
  2016-08-07  9:48 ` [PATCH 04/39] ALSA: seq: copy ioctl data from user space to kernel stack Takashi Sakamoto
                   ` (35 subsequent siblings)
  38 siblings, 0 replies; 63+ messages in thread
From: Takashi Sakamoto @ 2016-08-07  9:48 UTC (permalink / raw)
  To: clemens, tiwai; +Cc: alsa-devel

Most of callback functions corresponding to each ioctl command are in local
table, while two of them are not. This is a bit worse to handle the command
in a consistent way.

This commit adds entries for these two functions in the table.

Signed-off-by: Takashi Sakamoto <o-takashi@sakamocchi.jp>
---
 sound/core/seq/seq_clientmgr.c | 21 ++++++++++++---------
 1 file changed, 12 insertions(+), 9 deletions(-)

diff --git a/sound/core/seq/seq_clientmgr.c b/sound/core/seq/seq_clientmgr.c
index 1331103..d6c1219 100644
--- a/sound/core/seq/seq_clientmgr.c
+++ b/sound/core/seq/seq_clientmgr.c
@@ -1129,6 +1129,16 @@ static unsigned int snd_seq_poll(struct file *file, poll_table * wait)
 /*-----------------------------------------------------*/
 
 
+static int seq_ioctl_pversion(struct snd_seq_client *client, void __user *arg)
+{
+	return put_user(SNDRV_SEQ_VERSION, (int __user *)arg) ? -EFAULT : 0;
+}
+
+static int seq_ioctl_client_id(struct snd_seq_client *client, void __user *arg)
+{
+	return put_user(client->number, (int __user *)arg) ? -EFAULT : 0;
+}
+
 /* SYSTEM_INFO ioctl() */
 static int seq_ioctl_system_info(struct snd_seq_client *client,
 				 void __user *arg)
@@ -2174,6 +2184,8 @@ static const struct seq_ioctl_table {
 	unsigned int cmd;
 	int (*func)(struct snd_seq_client *client, void __user * arg);
 } ioctl_tables[] = {
+	{ SNDRV_SEQ_IOCTL_PVERSION, seq_ioctl_pversion },
+	{ SNDRV_SEQ_IOCTL_CLIENT_ID, seq_ioctl_client_id },
 	{ SNDRV_SEQ_IOCTL_SYSTEM_INFO, seq_ioctl_system_info },
 	{ SNDRV_SEQ_IOCTL_RUNNING_MODE, seq_ioctl_running_mode },
 	{ SNDRV_SEQ_IOCTL_GET_CLIENT_INFO, seq_ioctl_get_client_info },
@@ -2211,15 +2223,6 @@ static int seq_do_ioctl(struct snd_seq_client *client, unsigned int cmd,
 {
 	const struct seq_ioctl_table *p;
 
-	switch (cmd) {
-	case SNDRV_SEQ_IOCTL_PVERSION:
-		/* return sequencer version number */
-		return put_user(SNDRV_SEQ_VERSION, (int __user *)arg) ? -EFAULT : 0;
-	case SNDRV_SEQ_IOCTL_CLIENT_ID:
-		/* return the id of this client */
-		return put_user(client->number, (int __user *)arg) ? -EFAULT : 0;
-	}
-
 	if (! arg)
 		return -EFAULT;
 	for (p = ioctl_tables; p->cmd; p++) {
-- 
2.7.4

^ permalink raw reply related	[flat|nested] 63+ messages in thread

* [PATCH 04/39] ALSA: seq: copy ioctl data from user space to kernel stack
  2016-08-07  9:48 [PATCH 00/39] seq: obsolete change of address limit Takashi Sakamoto
                   ` (2 preceding siblings ...)
  2016-08-07  9:48 ` [PATCH 03/39] ALSA: seq: fulfill callback entry for ioctl Takashi Sakamoto
@ 2016-08-07  9:48 ` Takashi Sakamoto
  2016-08-07 10:15   ` Clemens Ladisch
  2016-08-07  9:48 ` [PATCH 05/39] ALSA: seq: add documentation for snd_seq_kernel_client_ctl Takashi Sakamoto
                   ` (34 subsequent siblings)
  38 siblings, 1 reply; 63+ messages in thread
From: Takashi Sakamoto @ 2016-08-07  9:48 UTC (permalink / raw)
  To: clemens, tiwai; +Cc: alsa-devel

ALSA sequencer core allows kernel-mode tasks to execute operations
corresponding to ioctls by user-mode tasks. This is for compatibility
layers such as ABIs for 32/64 bit and I/O operations via Open Sound
System.

This design requires to handle ioctl data in kernel space. For this
requirement, ALSA sequencer core temporarily changes address limit for the
task by get_fs()/set_fs() macro.

Although, this way get shape of code worse, because even when an pointer
has '__user' qualifier, actually it refers to kernel space, depending on
the task. This easily misleads readers.

This commit is a preparation for following patches to solve this issue.
Data from user space is once copied to kernel stack, then operated and
copied to user space, in a consistent manner. This manner forces all ioctl
operations to copy the data from/to user space, even if it's read-only or
write-only. Thus, it has an overhead for simpler ioctl commands.

Signed-off-by: Takashi Sakamoto <o-takashi@sakamocchi.jp>
---
 sound/core/seq/seq_clientmgr.c | 136 ++++++++++++++++++++++++++++++-----------
 1 file changed, 101 insertions(+), 35 deletions(-)

diff --git a/sound/core/seq/seq_clientmgr.c b/sound/core/seq/seq_clientmgr.c
index d6c1219..17d988a 100644
--- a/sound/core/seq/seq_clientmgr.c
+++ b/sound/core/seq/seq_clientmgr.c
@@ -2182,40 +2182,70 @@ static int seq_ioctl_query_next_port(struct snd_seq_client *client,
 
 static const struct seq_ioctl_table {
 	unsigned int cmd;
-	int (*func)(struct snd_seq_client *client, void __user * arg);
+	int (*func)(struct snd_seq_client *client, void *arg);
+	size_t arg_size;
 } ioctl_tables[] = {
-	{ SNDRV_SEQ_IOCTL_PVERSION, seq_ioctl_pversion },
-	{ SNDRV_SEQ_IOCTL_CLIENT_ID, seq_ioctl_client_id },
-	{ SNDRV_SEQ_IOCTL_SYSTEM_INFO, seq_ioctl_system_info },
-	{ SNDRV_SEQ_IOCTL_RUNNING_MODE, seq_ioctl_running_mode },
-	{ SNDRV_SEQ_IOCTL_GET_CLIENT_INFO, seq_ioctl_get_client_info },
-	{ SNDRV_SEQ_IOCTL_SET_CLIENT_INFO, seq_ioctl_set_client_info },
-	{ SNDRV_SEQ_IOCTL_CREATE_PORT, seq_ioctl_create_port },
-	{ SNDRV_SEQ_IOCTL_DELETE_PORT, seq_ioctl_delete_port },
-	{ SNDRV_SEQ_IOCTL_GET_PORT_INFO, seq_ioctl_get_port_info },
-	{ SNDRV_SEQ_IOCTL_SET_PORT_INFO, seq_ioctl_set_port_info },
-	{ SNDRV_SEQ_IOCTL_SUBSCRIBE_PORT, seq_ioctl_subscribe_port },
-	{ SNDRV_SEQ_IOCTL_UNSUBSCRIBE_PORT, seq_ioctl_unsubscribe_port },
-	{ SNDRV_SEQ_IOCTL_CREATE_QUEUE, seq_ioctl_create_queue },
-	{ SNDRV_SEQ_IOCTL_DELETE_QUEUE, seq_ioctl_delete_queue },
-	{ SNDRV_SEQ_IOCTL_GET_QUEUE_INFO, seq_ioctl_get_queue_info },
-	{ SNDRV_SEQ_IOCTL_SET_QUEUE_INFO, seq_ioctl_set_queue_info },
-	{ SNDRV_SEQ_IOCTL_GET_NAMED_QUEUE, seq_ioctl_get_named_queue },
-	{ SNDRV_SEQ_IOCTL_GET_QUEUE_STATUS, seq_ioctl_get_queue_status },
-	{ SNDRV_SEQ_IOCTL_GET_QUEUE_TEMPO, seq_ioctl_get_queue_tempo },
-	{ SNDRV_SEQ_IOCTL_SET_QUEUE_TEMPO, seq_ioctl_set_queue_tempo },
-	{ SNDRV_SEQ_IOCTL_GET_QUEUE_TIMER, seq_ioctl_get_queue_timer },
-	{ SNDRV_SEQ_IOCTL_SET_QUEUE_TIMER, seq_ioctl_set_queue_timer },
-	{ SNDRV_SEQ_IOCTL_GET_QUEUE_CLIENT, seq_ioctl_get_queue_client },
-	{ SNDRV_SEQ_IOCTL_SET_QUEUE_CLIENT, seq_ioctl_set_queue_client },
-	{ SNDRV_SEQ_IOCTL_GET_CLIENT_POOL, seq_ioctl_get_client_pool },
-	{ SNDRV_SEQ_IOCTL_SET_CLIENT_POOL, seq_ioctl_set_client_pool },
-	{ SNDRV_SEQ_IOCTL_GET_SUBSCRIPTION, seq_ioctl_get_subscription },
-	{ SNDRV_SEQ_IOCTL_QUERY_NEXT_CLIENT, seq_ioctl_query_next_client },
-	{ SNDRV_SEQ_IOCTL_QUERY_NEXT_PORT, seq_ioctl_query_next_port },
-	{ SNDRV_SEQ_IOCTL_REMOVE_EVENTS, seq_ioctl_remove_events },
-	{ SNDRV_SEQ_IOCTL_QUERY_SUBS, seq_ioctl_query_subs },
-	{ 0, NULL },
+	{ SNDRV_SEQ_IOCTL_PVERSION, seq_ioctl_pversion, sizeof(int) },
+	{ SNDRV_SEQ_IOCTL_CLIENT_ID, seq_ioctl_client_id, sizeof(int) },
+	{ SNDRV_SEQ_IOCTL_SYSTEM_INFO, seq_ioctl_system_info,
+	  sizeof(struct snd_seq_system_info) },
+	{ SNDRV_SEQ_IOCTL_RUNNING_MODE, seq_ioctl_running_mode,
+	  sizeof(struct snd_seq_running_info) },
+	{ SNDRV_SEQ_IOCTL_GET_CLIENT_INFO, seq_ioctl_get_client_info,
+	  sizeof(struct snd_seq_client_info) },
+	{ SNDRV_SEQ_IOCTL_SET_CLIENT_INFO, seq_ioctl_set_client_info,
+	  sizeof(struct snd_seq_client_info) },
+	{ SNDRV_SEQ_IOCTL_CREATE_PORT, seq_ioctl_create_port,
+	  sizeof(struct snd_seq_port_info) },
+	{ SNDRV_SEQ_IOCTL_DELETE_PORT, seq_ioctl_delete_port,
+	  sizeof(struct snd_seq_port_info) },
+	{ SNDRV_SEQ_IOCTL_GET_PORT_INFO, seq_ioctl_get_port_info,
+	  sizeof(struct snd_seq_port_info) },
+	{ SNDRV_SEQ_IOCTL_SET_PORT_INFO, seq_ioctl_set_port_info,
+	  sizeof(struct snd_seq_port_info) },
+	{ SNDRV_SEQ_IOCTL_SUBSCRIBE_PORT, seq_ioctl_subscribe_port,
+	  sizeof(struct snd_seq_port_subscribe) },
+	{ SNDRV_SEQ_IOCTL_UNSUBSCRIBE_PORT, seq_ioctl_unsubscribe_port,
+	  sizeof(struct snd_seq_port_subscribe) },
+	{ SNDRV_SEQ_IOCTL_CREATE_QUEUE, seq_ioctl_create_queue,
+	  sizeof(struct snd_seq_queue_info) },
+	{ SNDRV_SEQ_IOCTL_DELETE_QUEUE, seq_ioctl_delete_queue,
+	  sizeof(struct snd_seq_queue_info) },
+	{ SNDRV_SEQ_IOCTL_GET_QUEUE_INFO, seq_ioctl_get_queue_info,
+	  sizeof(struct snd_seq_queue_info) },
+	{ SNDRV_SEQ_IOCTL_SET_QUEUE_INFO, seq_ioctl_set_queue_info,
+	  sizeof(struct snd_seq_queue_info) },
+	{ SNDRV_SEQ_IOCTL_GET_NAMED_QUEUE, seq_ioctl_get_named_queue,
+	  sizeof(struct snd_seq_queue_info) },
+	{ SNDRV_SEQ_IOCTL_GET_QUEUE_STATUS, seq_ioctl_get_queue_status,
+	  sizeof(struct snd_seq_queue_status) },
+	{ SNDRV_SEQ_IOCTL_GET_QUEUE_TEMPO, seq_ioctl_get_queue_tempo,
+	  sizeof(struct snd_seq_queue_tempo) },
+	{ SNDRV_SEQ_IOCTL_SET_QUEUE_TEMPO, seq_ioctl_set_queue_tempo,
+	  sizeof(struct snd_seq_queue_tempo) },
+	{ SNDRV_SEQ_IOCTL_GET_QUEUE_TIMER, seq_ioctl_get_queue_timer,
+	  sizeof(struct snd_seq_queue_timer) },
+	{ SNDRV_SEQ_IOCTL_SET_QUEUE_TIMER, seq_ioctl_set_queue_timer,
+	  sizeof(struct snd_seq_queue_timer) },
+	{ SNDRV_SEQ_IOCTL_GET_QUEUE_CLIENT, seq_ioctl_get_queue_client,
+	  sizeof(struct snd_seq_queue_client) },
+	{ SNDRV_SEQ_IOCTL_SET_QUEUE_CLIENT, seq_ioctl_set_queue_client,
+	  sizeof(struct snd_seq_queue_client) },
+	{ SNDRV_SEQ_IOCTL_GET_CLIENT_POOL, seq_ioctl_get_client_pool,
+	  sizeof(struct snd_seq_client_pool) },
+	{ SNDRV_SEQ_IOCTL_SET_CLIENT_POOL, seq_ioctl_set_client_pool,
+	  sizeof(struct snd_seq_client_pool) },
+	{ SNDRV_SEQ_IOCTL_GET_SUBSCRIPTION, seq_ioctl_get_subscription,
+	  sizeof(struct snd_seq_port_subscribe) },
+	{ SNDRV_SEQ_IOCTL_QUERY_NEXT_CLIENT, seq_ioctl_query_next_client,
+	  sizeof(struct snd_seq_client_info) },
+	{ SNDRV_SEQ_IOCTL_QUERY_NEXT_PORT, seq_ioctl_query_next_port,
+	  sizeof(struct snd_seq_port_info) },
+	{ SNDRV_SEQ_IOCTL_REMOVE_EVENTS, seq_ioctl_remove_events,
+	  sizeof(struct snd_seq_remove_events) },
+	{ SNDRV_SEQ_IOCTL_QUERY_SUBS, seq_ioctl_query_subs,
+	  sizeof(struct snd_seq_query_subs) },
+	{ 0, NULL, 0 },
 };
 
 static int seq_do_ioctl(struct snd_seq_client *client, unsigned int cmd,
@@ -2235,14 +2265,50 @@ static int seq_do_ioctl(struct snd_seq_client *client, unsigned int cmd,
 }
 
 
-static long snd_seq_ioctl(struct file *file, unsigned int cmd, unsigned long arg)
+static long snd_seq_ioctl(struct file *file, unsigned int cmd,
+			  unsigned long arg)
 {
 	struct snd_seq_client *client = file->private_data;
+	const struct seq_ioctl_table *p;
+	union ioctl_arg {
+		int pversion;
+		int client_id;
+		struct snd_seq_system_info	system_info;
+		struct snd_seq_running_info	running_info;
+		struct snd_seq_client_info	client_info;
+		struct snd_seq_port_info	port_info;
+		struct snd_seq_port_subscribe	port_subscribe;
+		struct snd_seq_queue_info	queue_info;
+		struct snd_seq_queue_status	queue_status;
+		struct snd_seq_queue_tempo	tempo;
+		struct snd_seq_queue_timer	queue_timer;
+		struct snd_seq_queue_client	queue_client;
+		struct snd_seq_client_pool	client_pool;
+		struct snd_seq_remove_events	remove_events;
+		struct snd_seq_query_subs	query_subs;
+	} buf = {0};
+	int err;
 
 	if (snd_BUG_ON(!client))
 		return -ENXIO;
+
+	for (p = ioctl_tables; p->cmd > 0; ++p) {
+		if (p->cmd == cmd)
+			break;
+	}
+	if (p->cmd == 0)
+		return -ENOTTY;
+
+	if (copy_from_user(&buf, (const void __user *)arg, p->arg_size))
+		return -EFAULT;
 		
-	return seq_do_ioctl(client, cmd, (void __user *) arg);
+	err = p->func(client, &buf);
+	if (err >= 0) {
+		if (copy_to_user((void __user *)arg, &buf, p->arg_size))
+			return -EFAULT;
+	}
+
+	return err;
 }
 
 #ifdef CONFIG_COMPAT
-- 
2.7.4

^ permalink raw reply related	[flat|nested] 63+ messages in thread

* [PATCH 05/39] ALSA: seq: add documentation for snd_seq_kernel_client_ctl
  2016-08-07  9:48 [PATCH 00/39] seq: obsolete change of address limit Takashi Sakamoto
                   ` (3 preceding siblings ...)
  2016-08-07  9:48 ` [PATCH 04/39] ALSA: seq: copy ioctl data from user space to kernel stack Takashi Sakamoto
@ 2016-08-07  9:48 ` Takashi Sakamoto
  2016-08-08  6:58   ` Takashi Iwai
  2016-08-07  9:48 ` [PATCH 06/39] ALSA: seq: obsolete address mode in compatibility layer Takashi Sakamoto
                   ` (33 subsequent siblings)
  38 siblings, 1 reply; 63+ messages in thread
From: Takashi Sakamoto @ 2016-08-07  9:48 UTC (permalink / raw)
  To: clemens, tiwai; +Cc: alsa-devel

This kernel API is used by kernel implementation. Currently, it's used for
kernel clients of ALSA sequencer, while it can be used for application
clients. The difference is just on address spaces of argument. In short,
this kernel API can be available for application client with data in kernel
space.

This commit adds a document about this.

Signed-off-by: Takashi Sakamoto <o-takashi@sakamocchi.jp>
---
 sound/core/seq/seq_clientmgr.c | 14 +++++++++++---
 1 file changed, 11 insertions(+), 3 deletions(-)

diff --git a/sound/core/seq/seq_clientmgr.c b/sound/core/seq/seq_clientmgr.c
index 17d988a..a09cb84 100644
--- a/sound/core/seq/seq_clientmgr.c
+++ b/sound/core/seq/seq_clientmgr.c
@@ -2494,9 +2494,17 @@ int snd_seq_kernel_client_dispatch(int client, struct snd_seq_event * ev,
 
 EXPORT_SYMBOL(snd_seq_kernel_client_dispatch);
 
-/*
- * exported, called by kernel clients to perform same functions as with
- * userland ioctl() 
+/**
+ * snd_seq_kernel_client_ctl - operate a command for a client with data in
+ *			       kernel space.
+ * @clientid:	A numerical ID for a client.
+ * @cmd:	An ioctl(2) command for ALSA sequencer operation.
+ * @arg:	A pointer to data in kernel space.
+ *
+ * Against its name, both kernel/application client can be handled by this
+ * kernel API. Address space for 'arg' argument should be in kernel space.
+ *
+ * Return: 0 at success. Minus error code at failure.
  */
 int snd_seq_kernel_client_ctl(int clientid, unsigned int cmd, void *arg)
 {
-- 
2.7.4

^ permalink raw reply related	[flat|nested] 63+ messages in thread

* [PATCH 06/39] ALSA: seq: obsolete address mode in compatibility layer
  2016-08-07  9:48 [PATCH 00/39] seq: obsolete change of address limit Takashi Sakamoto
                   ` (4 preceding siblings ...)
  2016-08-07  9:48 ` [PATCH 05/39] ALSA: seq: add documentation for snd_seq_kernel_client_ctl Takashi Sakamoto
@ 2016-08-07  9:48 ` Takashi Sakamoto
  2016-08-08  7:09   ` Takashi Iwai
  2016-08-07  9:48 ` [PATCH 07/39] ALSA: seq: obsolete change of address limit in in-kernel path for ioctl Takashi Sakamoto
                   ` (32 subsequent siblings)
  38 siblings, 1 reply; 63+ messages in thread
From: Takashi Sakamoto @ 2016-08-07  9:48 UTC (permalink / raw)
  To: clemens, tiwai; +Cc: alsa-devel

In a 32/64 bit compatibility layer of ALSA sequencer core, data for some
ioctls is copied to kernel stack and passed to core operations. Then,
address limit of running task is changed because core implementation
expected arguments in userspace.

In this case, snd_seq_kernel_client_ctl() is available. This commit
replaces with it.

Signed-off-by: Takashi Sakamoto <o-takashi@sakamocchi.jp>
---
 sound/core/seq/seq_compat.c | 25 +++++++++++++------------
 1 file changed, 13 insertions(+), 12 deletions(-)

diff --git a/sound/core/seq/seq_compat.c b/sound/core/seq/seq_compat.c
index 70d3ddb..6cc7302 100644
--- a/sound/core/seq/seq_compat.c
+++ b/sound/core/seq/seq_compat.c
@@ -42,13 +42,11 @@ struct snd_seq_port_info32 {
 	char reserved[59];		/* for future use */
 };
 
-static int seq_call_port_info_ioctl(struct snd_seq_client *client,
-				    unsigned int cmd,
+static int seq_call_port_info_ioctl(int clientid, unsigned int cmd,
 				    struct snd_seq_port_info32 __user *data32)
 {
 	int err = -EFAULT;
 	struct snd_seq_port_info *data;
-	mm_segment_t fs;
 
 	data = kmalloc(sizeof(*data), GFP_KERNEL);
 	if (!data)
@@ -60,9 +58,7 @@ static int seq_call_port_info_ioctl(struct snd_seq_client *client,
 		goto error;
 	data->kernel = NULL;
 
-	fs = snd_enter_user();
-	err = seq_do_ioctl(client, cmd, data);
-	snd_leave_user(fs);
+	err = snd_seq_kernel_client_ctl(clientid, cmd, data);
 	if (err < 0)
 		goto error;
 
@@ -124,17 +120,22 @@ static long snd_seq_ioctl_compat(struct file *file, unsigned int cmd, unsigned l
 	case SNDRV_SEQ_IOCTL_GET_SUBSCRIPTION:
 	case SNDRV_SEQ_IOCTL_QUERY_NEXT_CLIENT:
 	case SNDRV_SEQ_IOCTL_RUNNING_MODE:
-		return seq_do_ioctl(client, cmd, argp);
+		return snd_seq_ioctl(file, cmd, (unsigned long)arg);
 	case SNDRV_SEQ_IOCTL_CREATE_PORT32:
-		return seq_call_port_info_ioctl(client, SNDRV_SEQ_IOCTL_CREATE_PORT, argp);
+		return seq_call_port_info_ioctl(client->number,
+					SNDRV_SEQ_IOCTL_CREATE_PORT, argp);
 	case SNDRV_SEQ_IOCTL_DELETE_PORT32:
-		return seq_call_port_info_ioctl(client, SNDRV_SEQ_IOCTL_DELETE_PORT, argp);
+		return seq_call_port_info_ioctl(client->number,
+					SNDRV_SEQ_IOCTL_DELETE_PORT, argp);
 	case SNDRV_SEQ_IOCTL_GET_PORT_INFO32:
-		return seq_call_port_info_ioctl(client, SNDRV_SEQ_IOCTL_GET_PORT_INFO, argp);
+		return seq_call_port_info_ioctl(client->number,
+					SNDRV_SEQ_IOCTL_GET_PORT_INFO, argp);
 	case SNDRV_SEQ_IOCTL_SET_PORT_INFO32:
-		return seq_call_port_info_ioctl(client, SNDRV_SEQ_IOCTL_SET_PORT_INFO, argp);
+		return seq_call_port_info_ioctl(client->number,
+					SNDRV_SEQ_IOCTL_SET_PORT_INFO, argp);
 	case SNDRV_SEQ_IOCTL_QUERY_NEXT_PORT32:
-		return seq_call_port_info_ioctl(client, SNDRV_SEQ_IOCTL_QUERY_NEXT_PORT, argp);
+		return seq_call_port_info_ioctl(client->number,
+					SNDRV_SEQ_IOCTL_QUERY_NEXT_PORT, argp);
 	}
 	return -ENOIOCTLCMD;
 }
-- 
2.7.4

^ permalink raw reply related	[flat|nested] 63+ messages in thread

* [PATCH 07/39] ALSA: seq: obsolete change of address limit in in-kernel path for ioctl
  2016-08-07  9:48 [PATCH 00/39] seq: obsolete change of address limit Takashi Sakamoto
                   ` (5 preceding siblings ...)
  2016-08-07  9:48 ` [PATCH 06/39] ALSA: seq: obsolete address mode in compatibility layer Takashi Sakamoto
@ 2016-08-07  9:48 ` Takashi Sakamoto
  2016-08-07  9:48 ` [PATCH 08/39] ALSA: seq: obsolete address limit helper Takashi Sakamoto
                   ` (31 subsequent siblings)
  38 siblings, 0 replies; 63+ messages in thread
From: Takashi Sakamoto @ 2016-08-07  9:48 UTC (permalink / raw)
  To: clemens, tiwai; +Cc: alsa-devel

In former commit, actual operations for each ioctl is re-designed to get
data in kernel space. Therefore, no need to cast pointer in kernel space
to user space.

This commit applies optimization to in-kernel path of ioctl.

Signed-off-by: Takashi Sakamoto <o-takashi@sakamocchi.jp>
---
 sound/core/seq/seq_clientmgr.c | 36 +++++++++++++-----------------------
 1 file changed, 13 insertions(+), 23 deletions(-)

diff --git a/sound/core/seq/seq_clientmgr.c b/sound/core/seq/seq_clientmgr.c
index a09cb84..2002dac 100644
--- a/sound/core/seq/seq_clientmgr.c
+++ b/sound/core/seq/seq_clientmgr.c
@@ -2248,23 +2248,6 @@ static const struct seq_ioctl_table {
 	{ 0, NULL, 0 },
 };
 
-static int seq_do_ioctl(struct snd_seq_client *client, unsigned int cmd,
-			    void __user *arg)
-{
-	const struct seq_ioctl_table *p;
-
-	if (! arg)
-		return -EFAULT;
-	for (p = ioctl_tables; p->cmd; p++) {
-		if (p->cmd == cmd)
-			return p->func(client, arg);
-	}
-	pr_debug("ALSA: seq unknown ioctl() 0x%x (type='%c', number=0x%02x)\n",
-		   cmd, _IOC_TYPE(cmd), _IOC_NR(cmd));
-	return -ENOTTY;
-}
-
-
 static long snd_seq_ioctl(struct file *file, unsigned int cmd,
 			  unsigned long arg)
 {
@@ -2509,16 +2492,23 @@ EXPORT_SYMBOL(snd_seq_kernel_client_dispatch);
 int snd_seq_kernel_client_ctl(int clientid, unsigned int cmd, void *arg)
 {
 	struct snd_seq_client *client;
-	mm_segment_t fs;
-	int result;
+	const struct seq_ioctl_table *p;
 
 	client = clientptr(clientid);
 	if (client == NULL)
 		return -ENXIO;
-	fs = snd_enter_user();
-	result = seq_do_ioctl(client, cmd, (void __force __user *)arg);
-	snd_leave_user(fs);
-	return result;
+
+	for (p = ioctl_tables; p->cmd > 0; ++p) {
+		if (p->cmd == cmd)
+			break;
+	}
+	if (p->cmd == 0) {
+		pr_debug("ALSA: seq unknown ioctl() 0x%x (type='%c', number=0x%02x)\n",
+			 cmd, _IOC_TYPE(cmd), _IOC_NR(cmd));
+		return -ENOTTY;
+	}
+
+	return p->func(client, arg);
 }
 
 EXPORT_SYMBOL(snd_seq_kernel_client_ctl);
-- 
2.7.4

^ permalink raw reply related	[flat|nested] 63+ messages in thread

* [PATCH 08/39] ALSA: seq: obsolete address limit helper
  2016-08-07  9:48 [PATCH 00/39] seq: obsolete change of address limit Takashi Sakamoto
                   ` (6 preceding siblings ...)
  2016-08-07  9:48 ` [PATCH 07/39] ALSA: seq: obsolete change of address limit in in-kernel path for ioctl Takashi Sakamoto
@ 2016-08-07  9:48 ` Takashi Sakamoto
  2016-08-07  9:48 ` [PATCH 09/39] ALSA: seq: optimize pversion function to new design Takashi Sakamoto
                   ` (30 subsequent siblings)
  38 siblings, 0 replies; 63+ messages in thread
From: Takashi Sakamoto @ 2016-08-07  9:48 UTC (permalink / raw)
  To: clemens, tiwai; +Cc: alsa-devel

Former commits make it useless to change address limit of running task.
This commit obsoletes helper functions for it.

Signed-off-by: Takashi Sakamoto <o-takashi@sakamocchi.jp>
---
 sound/core/seq/seq_clientmgr.c | 15 ---------------
 1 file changed, 15 deletions(-)

diff --git a/sound/core/seq/seq_clientmgr.c b/sound/core/seq/seq_clientmgr.c
index 2002dac..a643b4e 100644
--- a/sound/core/seq/seq_clientmgr.c
+++ b/sound/core/seq/seq_clientmgr.c
@@ -87,21 +87,6 @@ static int snd_seq_deliver_single_event(struct snd_seq_client *client,
 
 /*
  */
- 
-static inline mm_segment_t snd_enter_user(void)
-{
-	mm_segment_t fs = get_fs();
-	set_fs(get_ds());
-	return fs;
-}
-
-static inline void snd_leave_user(mm_segment_t fs)
-{
-	set_fs(fs);
-}
-
-/*
- */
 static inline unsigned short snd_seq_file_flags(struct file *file)
 {
         switch (file->f_mode & (FMODE_READ | FMODE_WRITE)) {
-- 
2.7.4

^ permalink raw reply related	[flat|nested] 63+ messages in thread

* [PATCH 09/39] ALSA: seq: optimize pversion function to new design
  2016-08-07  9:48 [PATCH 00/39] seq: obsolete change of address limit Takashi Sakamoto
                   ` (7 preceding siblings ...)
  2016-08-07  9:48 ` [PATCH 08/39] ALSA: seq: obsolete address limit helper Takashi Sakamoto
@ 2016-08-07  9:48 ` Takashi Sakamoto
  2016-08-07  9:48 ` [PATCH 10/39] ALSA: seq: optimize client_id " Takashi Sakamoto
                   ` (29 subsequent siblings)
  38 siblings, 0 replies; 63+ messages in thread
From: Takashi Sakamoto @ 2016-08-07  9:48 UTC (permalink / raw)
  To: clemens, tiwai; +Cc: alsa-devel

In former commit, actual operations of each ioctl command get argument
in kernel space. Copying from/to user space is performed outside of
the function.

This commit optimizes to the new design.

Signed-off-by: Takashi Sakamoto <o-takashi@sakamocchi.jp>
---
 sound/core/seq/seq_clientmgr.c | 7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/sound/core/seq/seq_clientmgr.c b/sound/core/seq/seq_clientmgr.c
index a643b4e..5e81d4f 100644
--- a/sound/core/seq/seq_clientmgr.c
+++ b/sound/core/seq/seq_clientmgr.c
@@ -1114,9 +1114,12 @@ static unsigned int snd_seq_poll(struct file *file, poll_table * wait)
 /*-----------------------------------------------------*/
 
 
-static int seq_ioctl_pversion(struct snd_seq_client *client, void __user *arg)
+static int seq_ioctl_pversion(struct snd_seq_client *client, void *arg)
 {
-	return put_user(SNDRV_SEQ_VERSION, (int __user *)arg) ? -EFAULT : 0;
+	int *version = arg;
+
+	*version = SNDRV_SEQ_VERSION;
+	return 0;
 }
 
 static int seq_ioctl_client_id(struct snd_seq_client *client, void __user *arg)
-- 
2.7.4

^ permalink raw reply related	[flat|nested] 63+ messages in thread

* [PATCH 10/39] ALSA: seq: optimize client_id function to new design
  2016-08-07  9:48 [PATCH 00/39] seq: obsolete change of address limit Takashi Sakamoto
                   ` (8 preceding siblings ...)
  2016-08-07  9:48 ` [PATCH 09/39] ALSA: seq: optimize pversion function to new design Takashi Sakamoto
@ 2016-08-07  9:48 ` Takashi Sakamoto
  2016-08-07  9:48 ` [PATCH 11/39] ALSA: seq: optimize system_info " Takashi Sakamoto
                   ` (28 subsequent siblings)
  38 siblings, 0 replies; 63+ messages in thread
From: Takashi Sakamoto @ 2016-08-07  9:48 UTC (permalink / raw)
  To: clemens, tiwai; +Cc: alsa-devel

In former commit, actual operations of each ioctl command get argument
in kernel space. Copying from/to user space is performed outside of
the function.

This commit optimizes to the new design.

Signed-off-by: Takashi Sakamoto <o-takashi@sakamocchi.jp>
---
 sound/core/seq/seq_clientmgr.c | 7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/sound/core/seq/seq_clientmgr.c b/sound/core/seq/seq_clientmgr.c
index 5e81d4f..123d8a1 100644
--- a/sound/core/seq/seq_clientmgr.c
+++ b/sound/core/seq/seq_clientmgr.c
@@ -1122,9 +1122,12 @@ static int seq_ioctl_pversion(struct snd_seq_client *client, void *arg)
 	return 0;
 }
 
-static int seq_ioctl_client_id(struct snd_seq_client *client, void __user *arg)
+static int seq_ioctl_client_id(struct snd_seq_client *client, void *arg)
 {
-	return put_user(client->number, (int __user *)arg) ? -EFAULT : 0;
+	int *number = arg;
+
+	*number = client->number;
+	return 0;
 }
 
 /* SYSTEM_INFO ioctl() */
-- 
2.7.4

^ permalink raw reply related	[flat|nested] 63+ messages in thread

* [PATCH 11/39] ALSA: seq: optimize system_info function to new design
  2016-08-07  9:48 [PATCH 00/39] seq: obsolete change of address limit Takashi Sakamoto
                   ` (9 preceding siblings ...)
  2016-08-07  9:48 ` [PATCH 10/39] ALSA: seq: optimize client_id " Takashi Sakamoto
@ 2016-08-07  9:48 ` Takashi Sakamoto
  2016-08-08  7:04   ` Takashi Iwai
  2016-08-07  9:48 ` [PATCH 12/39] ALSA: seq: optimize running mode " Takashi Sakamoto
                   ` (27 subsequent siblings)
  38 siblings, 1 reply; 63+ messages in thread
From: Takashi Sakamoto @ 2016-08-07  9:48 UTC (permalink / raw)
  To: clemens, tiwai; +Cc: alsa-devel

In former commit, actual operations of each ioctl command get argument
in kernel space. Copying from/to user space is performed outside of
the function.

This commit optimizes to the new design.

Signed-off-by: Takashi Sakamoto <o-takashi@sakamocchi.jp>
---
 sound/core/seq/seq_clientmgr.c | 21 +++++++++------------
 1 file changed, 9 insertions(+), 12 deletions(-)

diff --git a/sound/core/seq/seq_clientmgr.c b/sound/core/seq/seq_clientmgr.c
index 123d8a1..cb86190 100644
--- a/sound/core/seq/seq_clientmgr.c
+++ b/sound/core/seq/seq_clientmgr.c
@@ -1131,22 +1131,19 @@ static int seq_ioctl_client_id(struct snd_seq_client *client, void *arg)
 }
 
 /* SYSTEM_INFO ioctl() */
-static int seq_ioctl_system_info(struct snd_seq_client *client,
-				 void __user *arg)
+static int seq_ioctl_system_info(struct snd_seq_client *client, void *arg)
 {
-	struct snd_seq_system_info info;
+	struct snd_seq_system_info *info = arg;
 
-	memset(&info, 0, sizeof(info));
+	memset(info, 0, sizeof(*info));
 	/* fill the info fields */
-	info.queues = SNDRV_SEQ_MAX_QUEUES;
-	info.clients = SNDRV_SEQ_MAX_CLIENTS;
-	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();
+	info->queues = SNDRV_SEQ_MAX_QUEUES;
+	info->clients = SNDRV_SEQ_MAX_CLIENTS;
+	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();
 
-	if (copy_to_user(arg, &info, sizeof(info)))
-		return -EFAULT;
 	return 0;
 }
 
-- 
2.7.4

^ permalink raw reply related	[flat|nested] 63+ messages in thread

* [PATCH 12/39] ALSA: seq: optimize running mode function to new design
  2016-08-07  9:48 [PATCH 00/39] seq: obsolete change of address limit Takashi Sakamoto
                   ` (10 preceding siblings ...)
  2016-08-07  9:48 ` [PATCH 11/39] ALSA: seq: optimize system_info " Takashi Sakamoto
@ 2016-08-07  9:48 ` Takashi Sakamoto
  2016-08-07  9:48 ` [PATCH 13/39] ALSA: seq: optimize client_info " Takashi Sakamoto
                   ` (26 subsequent siblings)
  38 siblings, 0 replies; 63+ messages in thread
From: Takashi Sakamoto @ 2016-08-07  9:48 UTC (permalink / raw)
  To: clemens, tiwai; +Cc: alsa-devel

In former commit, actual operations of each ioctl command get argument
in kernel space. Copying from/to user space is performed outside of
the function.

This commit optimizes to the new design.

Signed-off-by: Takashi Sakamoto <o-takashi@sakamocchi.jp>
---
 sound/core/seq/seq_clientmgr.c | 18 +++++++-----------
 1 file changed, 7 insertions(+), 11 deletions(-)

diff --git a/sound/core/seq/seq_clientmgr.c b/sound/core/seq/seq_clientmgr.c
index cb86190..10a2e43 100644
--- a/sound/core/seq/seq_clientmgr.c
+++ b/sound/core/seq/seq_clientmgr.c
@@ -1149,38 +1149,34 @@ static int seq_ioctl_system_info(struct snd_seq_client *client, void *arg)
 
 
 /* RUNNING_MODE ioctl() */
-static int seq_ioctl_running_mode(struct snd_seq_client *client,
-				  void __user *arg)
+static int seq_ioctl_running_mode(struct snd_seq_client *client, void *arg)
 {
-	struct snd_seq_running_info info;
+	struct snd_seq_running_info *info = arg;
 	struct snd_seq_client *cptr;
 	int err = 0;
 
-	if (copy_from_user(&info, arg, sizeof(info)))
-		return -EFAULT;
-
 	/* requested client number */
-	cptr = snd_seq_client_use_ptr(info.client);
+	cptr = snd_seq_client_use_ptr(info->client);
 	if (cptr == NULL)
 		return -ENOENT;		/* don't change !!! */
 
 #ifdef SNDRV_BIG_ENDIAN
-	if (! info.big_endian) {
+	if (!info->big_endian) {
 		err = -EINVAL;
 		goto __err;
 	}
 #else
-	if (info.big_endian) {
+	if (info->big_endian) {
 		err = -EINVAL;
 		goto __err;
 	}
 
 #endif
-	if (info.cpu_mode > sizeof(long)) {
+	if (info->cpu_mode > sizeof(long)) {
 		err = -EINVAL;
 		goto __err;
 	}
-	cptr->convert32 = (info.cpu_mode < sizeof(long));
+	cptr->convert32 = (info->cpu_mode < sizeof(long));
  __err:
 	snd_seq_client_unlock(cptr);
 	return err;
-- 
2.7.4

^ permalink raw reply related	[flat|nested] 63+ messages in thread

* [PATCH 13/39] ALSA: seq: optimize client_info function to new design
  2016-08-07  9:48 [PATCH 00/39] seq: obsolete change of address limit Takashi Sakamoto
                   ` (11 preceding siblings ...)
  2016-08-07  9:48 ` [PATCH 12/39] ALSA: seq: optimize running mode " Takashi Sakamoto
@ 2016-08-07  9:48 ` Takashi Sakamoto
  2016-08-07  9:48 ` [PATCH 14/39] ALSA: seq: optimize set_client_info " Takashi Sakamoto
                   ` (25 subsequent siblings)
  38 siblings, 0 replies; 63+ messages in thread
From: Takashi Sakamoto @ 2016-08-07  9:48 UTC (permalink / raw)
  To: clemens, tiwai; +Cc: alsa-devel

In former commit, actual operations of each ioctl command get argument
in kernel space. Copying from/to user space is performed outside of
the function.

This commit optimizes to the new design.

Signed-off-by: Takashi Sakamoto <o-takashi@sakamocchi.jp>
---
 sound/core/seq/seq_clientmgr.c | 14 ++++----------
 1 file changed, 4 insertions(+), 10 deletions(-)

diff --git a/sound/core/seq/seq_clientmgr.c b/sound/core/seq/seq_clientmgr.c
index 10a2e43..3e7431e 100644
--- a/sound/core/seq/seq_clientmgr.c
+++ b/sound/core/seq/seq_clientmgr.c
@@ -1209,25 +1209,19 @@ static void get_client_info(struct snd_seq_client *cptr,
 	memset(info->reserved, 0, sizeof(info->reserved));
 }
 
-static int seq_ioctl_get_client_info(struct snd_seq_client *client,
-				     void __user *arg)
+static int seq_ioctl_get_client_info(struct snd_seq_client *client, void *arg)
 {
+	struct snd_seq_client_info *client_info = arg;
 	struct snd_seq_client *cptr;
-	struct snd_seq_client_info client_info;
-
-	if (copy_from_user(&client_info, arg, sizeof(client_info)))
-		return -EFAULT;
 
 	/* requested client number */
-	cptr = snd_seq_client_use_ptr(client_info.client);
+	cptr = snd_seq_client_use_ptr(client_info->client);
 	if (cptr == NULL)
 		return -ENOENT;		/* don't change !!! */
 
-	get_client_info(cptr, &client_info);
+	get_client_info(cptr, client_info);
 	snd_seq_client_unlock(cptr);
 
-	if (copy_to_user(arg, &client_info, sizeof(client_info)))
-		return -EFAULT;
 	return 0;
 }
 
-- 
2.7.4

^ permalink raw reply related	[flat|nested] 63+ messages in thread

* [PATCH 14/39] ALSA: seq: optimize set_client_info function to new design
  2016-08-07  9:48 [PATCH 00/39] seq: obsolete change of address limit Takashi Sakamoto
                   ` (12 preceding siblings ...)
  2016-08-07  9:48 ` [PATCH 13/39] ALSA: seq: optimize client_info " Takashi Sakamoto
@ 2016-08-07  9:48 ` Takashi Sakamoto
  2016-08-07  9:48 ` [PATCH 15/39] ALSA: seq: optimize create_port " Takashi Sakamoto
                   ` (24 subsequent siblings)
  38 siblings, 0 replies; 63+ messages in thread
From: Takashi Sakamoto @ 2016-08-07  9:48 UTC (permalink / raw)
  To: clemens, tiwai; +Cc: alsa-devel

In former commit, actual operations of each ioctl command get argument
in kernel space. Copying from/to user space is performed outside of
the function.

This commit optimizes to the new design.

Signed-off-by: Takashi Sakamoto <o-takashi@sakamocchi.jp>
---
 sound/core/seq/seq_clientmgr.c | 22 +++++++++-------------
 1 file changed, 9 insertions(+), 13 deletions(-)

diff --git a/sound/core/seq/seq_clientmgr.c b/sound/core/seq/seq_clientmgr.c
index 3e7431e..46e040b 100644
--- a/sound/core/seq/seq_clientmgr.c
+++ b/sound/core/seq/seq_clientmgr.c
@@ -1227,28 +1227,24 @@ static int seq_ioctl_get_client_info(struct snd_seq_client *client, void *arg)
 
 
 /* CLIENT_INFO ioctl() */
-static int seq_ioctl_set_client_info(struct snd_seq_client *client,
-				     void __user *arg)
+static int seq_ioctl_set_client_info(struct snd_seq_client *client, void *arg)
 {
-	struct snd_seq_client_info client_info;
-
-	if (copy_from_user(&client_info, arg, sizeof(client_info)))
-		return -EFAULT;
+	struct snd_seq_client_info *client_info = arg;
 
 	/* it is not allowed to set the info fields for an another client */
-	if (client->number != client_info.client)
+	if (client->number != client_info->client)
 		return -EPERM;
 	/* also client type must be set now */
-	if (client->type != client_info.type)
+	if (client->type != client_info->type)
 		return -EINVAL;
 
 	/* fill the info fields */
-	if (client_info.name[0])
-		strlcpy(client->name, client_info.name, sizeof(client->name));
+	if (client_info->name[0])
+		strlcpy(client->name, client_info->name, sizeof(client->name));
 
-	client->filter = client_info.filter;
-	client->event_lost = client_info.event_lost;
-	memcpy(client->event_filter, client_info.event_filter, 32);
+	client->filter = client_info->filter;
+	client->event_lost = client_info->event_lost;
+	memcpy(client->event_filter, client_info->event_filter, 32);
 
 	return 0;
 }
-- 
2.7.4

^ permalink raw reply related	[flat|nested] 63+ messages in thread

* [PATCH 15/39] ALSA: seq: optimize create_port function to new design
  2016-08-07  9:48 [PATCH 00/39] seq: obsolete change of address limit Takashi Sakamoto
                   ` (13 preceding siblings ...)
  2016-08-07  9:48 ` [PATCH 14/39] ALSA: seq: optimize set_client_info " Takashi Sakamoto
@ 2016-08-07  9:48 ` Takashi Sakamoto
  2016-08-07  9:48 ` [PATCH 16/39] ALSA: seq: optimize delete_port " Takashi Sakamoto
                   ` (23 subsequent siblings)
  38 siblings, 0 replies; 63+ messages in thread
From: Takashi Sakamoto @ 2016-08-07  9:48 UTC (permalink / raw)
  To: clemens, tiwai; +Cc: alsa-devel

In former commit, actual operations of each ioctl command get argument
in kernel space. Copying from/to user space is performed outside of
the function.

This commit optimizes to the new design.

Signed-off-by: Takashi Sakamoto <o-takashi@sakamocchi.jp>
---
 sound/core/seq/seq_clientmgr.c | 24 +++++++++---------------
 1 file changed, 9 insertions(+), 15 deletions(-)

diff --git a/sound/core/seq/seq_clientmgr.c b/sound/core/seq/seq_clientmgr.c
index 46e040b..c2bb93b 100644
--- a/sound/core/seq/seq_clientmgr.c
+++ b/sound/core/seq/seq_clientmgr.c
@@ -1253,30 +1253,27 @@ static int seq_ioctl_set_client_info(struct snd_seq_client *client, void *arg)
 /* 
  * CREATE PORT ioctl() 
  */
-static int seq_ioctl_create_port(struct snd_seq_client *client,
-				 void __user *arg)
+static int seq_ioctl_create_port(struct snd_seq_client *client, void *arg)
 {
+	struct snd_seq_port_info *info = arg;
 	struct snd_seq_client_port *port;
-	struct snd_seq_port_info info;
 	struct snd_seq_port_callback *callback;
 
-	if (copy_from_user(&info, arg, sizeof(info)))
-		return -EFAULT;
-
 	/* it is not allowed to create the port for an another client */
-	if (info.addr.client != client->number)
+	if (info->addr.client != client->number)
 		return -EPERM;
 
-	port = snd_seq_create_port(client, (info.flags & SNDRV_SEQ_PORT_FLG_GIVEN_PORT) ? info.addr.port : -1);
+	port = snd_seq_create_port(client, (info->flags & SNDRV_SEQ_PORT_FLG_GIVEN_PORT) ? info->addr.port : -1);
 	if (port == NULL)
 		return -ENOMEM;
 
-	if (client->type == USER_CLIENT && info.kernel) {
+	if (client->type == USER_CLIENT && info->kernel) {
 		snd_seq_delete_port(client, port->addr.port);
 		return -EINVAL;
 	}
 	if (client->type == KERNEL_CLIENT) {
-		if ((callback = info.kernel) != NULL) {
+		callback = info->kernel;
+		if (callback != NULL) {
 			if (callback->owner)
 				port->owner = callback->owner;
 			port->private_data = callback->private_data;
@@ -1289,14 +1286,11 @@ static int seq_ioctl_create_port(struct snd_seq_client *client,
 		}
 	}
 
-	info.addr = port->addr;
+	info->addr = port->addr;
 
-	snd_seq_set_port_info(port, &info);
+	snd_seq_set_port_info(port, info);
 	snd_seq_system_client_ev_port_start(port->addr.client, port->addr.port);
 
-	if (copy_to_user(arg, &info, sizeof(info)))
-		return -EFAULT;
-
 	return 0;
 }
 
-- 
2.7.4

^ permalink raw reply related	[flat|nested] 63+ messages in thread

* [PATCH 16/39] ALSA: seq: optimize delete_port function to new design
  2016-08-07  9:48 [PATCH 00/39] seq: obsolete change of address limit Takashi Sakamoto
                   ` (14 preceding siblings ...)
  2016-08-07  9:48 ` [PATCH 15/39] ALSA: seq: optimize create_port " Takashi Sakamoto
@ 2016-08-07  9:48 ` Takashi Sakamoto
  2016-08-07  9:48 ` [PATCH 17/39] ALSA: seq: optimize get_port_info " Takashi Sakamoto
                   ` (22 subsequent siblings)
  38 siblings, 0 replies; 63+ messages in thread
From: Takashi Sakamoto @ 2016-08-07  9:48 UTC (permalink / raw)
  To: clemens, tiwai; +Cc: alsa-devel

In former commit, actual operations of each ioctl command get argument
in kernel space. Copying from/to user space is performed outside of
the function.

This commit optimizes to the new design.

Signed-off-by: Takashi Sakamoto <o-takashi@sakamocchi.jp>
---
 sound/core/seq/seq_clientmgr.c | 16 ++++++----------
 1 file changed, 6 insertions(+), 10 deletions(-)

diff --git a/sound/core/seq/seq_clientmgr.c b/sound/core/seq/seq_clientmgr.c
index c2bb93b..3acec92 100644
--- a/sound/core/seq/seq_clientmgr.c
+++ b/sound/core/seq/seq_clientmgr.c
@@ -1297,23 +1297,19 @@ static int seq_ioctl_create_port(struct snd_seq_client *client, void *arg)
 /* 
  * DELETE PORT ioctl() 
  */
-static int seq_ioctl_delete_port(struct snd_seq_client *client,
-				 void __user *arg)
+static int seq_ioctl_delete_port(struct snd_seq_client *client, void *arg)
 {
-	struct snd_seq_port_info info;
+	struct snd_seq_port_info *info = arg;
 	int err;
 
-	/* set passed parameters */
-	if (copy_from_user(&info, arg, sizeof(info)))
-		return -EFAULT;
-	
 	/* it is not allowed to remove the port for an another client */
-	if (info.addr.client != client->number)
+	if (info->addr.client != client->number)
 		return -EPERM;
 
-	err = snd_seq_delete_port(client, info.addr.port);
+	err = snd_seq_delete_port(client, info->addr.port);
 	if (err >= 0)
-		snd_seq_system_client_ev_port_exit(client->number, info.addr.port);
+		snd_seq_system_client_ev_port_exit(client->number,
+						   info->addr.port);
 	return err;
 }
 
-- 
2.7.4

^ permalink raw reply related	[flat|nested] 63+ messages in thread

* [PATCH 17/39] ALSA: seq: optimize get_port_info function to new design
  2016-08-07  9:48 [PATCH 00/39] seq: obsolete change of address limit Takashi Sakamoto
                   ` (15 preceding siblings ...)
  2016-08-07  9:48 ` [PATCH 16/39] ALSA: seq: optimize delete_port " Takashi Sakamoto
@ 2016-08-07  9:48 ` Takashi Sakamoto
  2016-08-07  9:48 ` [PATCH 18/39] ALSA: seq: optimize seq_port_info " Takashi Sakamoto
                   ` (21 subsequent siblings)
  38 siblings, 0 replies; 63+ messages in thread
From: Takashi Sakamoto @ 2016-08-07  9:48 UTC (permalink / raw)
  To: clemens, tiwai; +Cc: alsa-devel

In former commit, actual operations of each ioctl command get argument
in kernel space. Copying from/to user space is performed outside of
the function.

This commit optimizes to the new design.

Signed-off-by: Takashi Sakamoto <o-takashi@sakamocchi.jp>
---
 sound/core/seq/seq_clientmgr.c | 15 +++++----------
 1 file changed, 5 insertions(+), 10 deletions(-)

diff --git a/sound/core/seq/seq_clientmgr.c b/sound/core/seq/seq_clientmgr.c
index 3acec92..1e7b2ac 100644
--- a/sound/core/seq/seq_clientmgr.c
+++ b/sound/core/seq/seq_clientmgr.c
@@ -1317,32 +1317,27 @@ static int seq_ioctl_delete_port(struct snd_seq_client *client, void *arg)
 /* 
  * GET_PORT_INFO ioctl() (on any client) 
  */
-static int seq_ioctl_get_port_info(struct snd_seq_client *client,
-				   void __user *arg)
+static int seq_ioctl_get_port_info(struct snd_seq_client *client, void *arg)
 {
+	struct snd_seq_port_info *info = arg;
 	struct snd_seq_client *cptr;
 	struct snd_seq_client_port *port;
-	struct snd_seq_port_info info;
 
-	if (copy_from_user(&info, arg, sizeof(info)))
-		return -EFAULT;
-	cptr = snd_seq_client_use_ptr(info.addr.client);
+	cptr = snd_seq_client_use_ptr(info->addr.client);
 	if (cptr == NULL)
 		return -ENXIO;
 
-	port = snd_seq_port_use_ptr(cptr, info.addr.port);
+	port = snd_seq_port_use_ptr(cptr, info->addr.port);
 	if (port == NULL) {
 		snd_seq_client_unlock(cptr);
 		return -ENOENT;			/* don't change */
 	}
 
 	/* get port info */
-	snd_seq_get_port_info(port, &info);
+	snd_seq_get_port_info(port, info);
 	snd_seq_port_unlock(port);
 	snd_seq_client_unlock(cptr);
 
-	if (copy_to_user(arg, &info, sizeof(info)))
-		return -EFAULT;
 	return 0;
 }
 
-- 
2.7.4

^ permalink raw reply related	[flat|nested] 63+ messages in thread

* [PATCH 18/39] ALSA: seq: optimize seq_port_info function to new design
  2016-08-07  9:48 [PATCH 00/39] seq: obsolete change of address limit Takashi Sakamoto
                   ` (16 preceding siblings ...)
  2016-08-07  9:48 ` [PATCH 17/39] ALSA: seq: optimize get_port_info " Takashi Sakamoto
@ 2016-08-07  9:48 ` Takashi Sakamoto
  2016-08-07  9:48 ` [PATCH 19/39] ALSA: seq: optimize subscribe_port " Takashi Sakamoto
                   ` (20 subsequent siblings)
  38 siblings, 0 replies; 63+ messages in thread
From: Takashi Sakamoto @ 2016-08-07  9:48 UTC (permalink / raw)
  To: clemens, tiwai; +Cc: alsa-devel

In former commit, actual operations of each ioctl command get argument
in kernel space. Copying from/to user space is performed outside of
the function.

This commit optimizes to the new design.

Signed-off-by: Takashi Sakamoto <o-takashi@sakamocchi.jp>
---
 sound/core/seq/seq_clientmgr.c | 14 +++++---------
 1 file changed, 5 insertions(+), 9 deletions(-)

diff --git a/sound/core/seq/seq_clientmgr.c b/sound/core/seq/seq_clientmgr.c
index 1e7b2ac..4b4cd66 100644
--- a/sound/core/seq/seq_clientmgr.c
+++ b/sound/core/seq/seq_clientmgr.c
@@ -1345,20 +1345,16 @@ static int seq_ioctl_get_port_info(struct snd_seq_client *client, void *arg)
 /* 
  * SET_PORT_INFO ioctl() (only ports on this/own client) 
  */
-static int seq_ioctl_set_port_info(struct snd_seq_client *client,
-				   void __user *arg)
+static int seq_ioctl_set_port_info(struct snd_seq_client *client, void *arg)
 {
+	struct snd_seq_port_info *info = arg;
 	struct snd_seq_client_port *port;
-	struct snd_seq_port_info info;
-
-	if (copy_from_user(&info, arg, sizeof(info)))
-		return -EFAULT;
 
-	if (info.addr.client != client->number) /* only set our own ports ! */
+	if (info->addr.client != client->number) /* only set our own ports ! */
 		return -EPERM;
-	port = snd_seq_port_use_ptr(client, info.addr.port);
+	port = snd_seq_port_use_ptr(client, info->addr.port);
 	if (port) {
-		snd_seq_set_port_info(port, &info);
+		snd_seq_set_port_info(port, info);
 		snd_seq_port_unlock(port);
 	}
 	return 0;
-- 
2.7.4

^ permalink raw reply related	[flat|nested] 63+ messages in thread

* [PATCH 19/39] ALSA: seq: optimize subscribe_port function to new design
  2016-08-07  9:48 [PATCH 00/39] seq: obsolete change of address limit Takashi Sakamoto
                   ` (17 preceding siblings ...)
  2016-08-07  9:48 ` [PATCH 18/39] ALSA: seq: optimize seq_port_info " Takashi Sakamoto
@ 2016-08-07  9:48 ` Takashi Sakamoto
  2016-08-07  9:48 ` [PATCH 20/39] ALSA: seq: optimize unsubscribe_port " Takashi Sakamoto
                   ` (19 subsequent siblings)
  38 siblings, 0 replies; 63+ messages in thread
From: Takashi Sakamoto @ 2016-08-07  9:48 UTC (permalink / raw)
  To: clemens, tiwai; +Cc: alsa-devel

In former commit, actual operations of each ioctl command get argument
in kernel space. Copying from/to user space is performed outside of
the function.

This commit optimizes to the new design.

Signed-off-by: Takashi Sakamoto <o-takashi@sakamocchi.jp>
---
 sound/core/seq/seq_clientmgr.c | 32 +++++++++++++++++---------------
 1 file changed, 17 insertions(+), 15 deletions(-)

diff --git a/sound/core/seq/seq_clientmgr.c b/sound/core/seq/seq_clientmgr.c
index 4b4cd66..8681f90 100644
--- a/sound/core/seq/seq_clientmgr.c
+++ b/sound/core/seq/seq_clientmgr.c
@@ -1419,35 +1419,37 @@ int snd_seq_client_notify_subscription(int client, int port,
 /* 
  * add to port's subscription list IOCTL interface 
  */
-static int seq_ioctl_subscribe_port(struct snd_seq_client *client,
-				    void __user *arg)
+static int seq_ioctl_subscribe_port(struct snd_seq_client *client, void *arg)
 {
+	struct snd_seq_port_subscribe *subs = arg;
 	int result = -EINVAL;
 	struct snd_seq_client *receiver = NULL, *sender = NULL;
 	struct snd_seq_client_port *sport = NULL, *dport = NULL;
-	struct snd_seq_port_subscribe subs;
-
-	if (copy_from_user(&subs, arg, sizeof(subs)))
-		return -EFAULT;
 
-	if ((receiver = snd_seq_client_use_ptr(subs.dest.client)) == NULL)
+	receiver = snd_seq_client_use_ptr(subs->dest.client);
+	if (receiver == NULL)
 		goto __end;
-	if ((sender = snd_seq_client_use_ptr(subs.sender.client)) == NULL)
+	sender = snd_seq_client_use_ptr(subs->sender.client);
+	if (sender == NULL)
 		goto __end;
-	if ((sport = snd_seq_port_use_ptr(sender, subs.sender.port)) == NULL)
+	sport = snd_seq_port_use_ptr(sender, subs->sender.port);
+	if (sport == NULL)
 		goto __end;
-	if ((dport = snd_seq_port_use_ptr(receiver, subs.dest.port)) == NULL)
+	dport = snd_seq_port_use_ptr(receiver, subs->dest.port);
+	if (dport == NULL)
 		goto __end;
 
-	result = check_subscription_permission(client, sport, dport, &subs);
+	result = check_subscription_permission(client, sport, dport, subs);
 	if (result < 0)
 		goto __end;
 
 	/* connect them */
-	result = snd_seq_port_connect(client, sender, sport, receiver, dport, &subs);
-	if (! result) /* broadcast announce */
-		snd_seq_client_notify_subscription(SNDRV_SEQ_ADDRESS_SUBSCRIBERS, 0,
-						   &subs, SNDRV_SEQ_EVENT_PORT_SUBSCRIBED);
+	result = snd_seq_port_connect(client, sender, sport, receiver, dport,
+				      subs);
+	if (!result) /* broadcast announce */
+		snd_seq_client_notify_subscription(
+					SNDRV_SEQ_ADDRESS_SUBSCRIBERS, 0,
+					subs, SNDRV_SEQ_EVENT_PORT_SUBSCRIBED);
       __end:
       	if (sport)
 		snd_seq_port_unlock(sport);
-- 
2.7.4

^ permalink raw reply related	[flat|nested] 63+ messages in thread

* [PATCH 20/39] ALSA: seq: optimize unsubscribe_port function to new design
  2016-08-07  9:48 [PATCH 00/39] seq: obsolete change of address limit Takashi Sakamoto
                   ` (18 preceding siblings ...)
  2016-08-07  9:48 ` [PATCH 19/39] ALSA: seq: optimize subscribe_port " Takashi Sakamoto
@ 2016-08-07  9:48 ` Takashi Sakamoto
  2016-08-07  9:48 ` [PATCH 21/39] ALSA: seq: optimize create_queue " Takashi Sakamoto
                   ` (18 subsequent siblings)
  38 siblings, 0 replies; 63+ messages in thread
From: Takashi Sakamoto @ 2016-08-07  9:48 UTC (permalink / raw)
  To: clemens, tiwai; +Cc: alsa-devel

In former commit, actual operations of each ioctl command get argument
in kernel space. Copying from/to user space is performed outside of
the function.

This commit optimizes to the new design.

Signed-off-by: Takashi Sakamoto <o-takashi@sakamocchi.jp>
---
 sound/core/seq/seq_clientmgr.c | 32 +++++++++++++++++---------------
 1 file changed, 17 insertions(+), 15 deletions(-)

diff --git a/sound/core/seq/seq_clientmgr.c b/sound/core/seq/seq_clientmgr.c
index 8681f90..9f4bcf2 100644
--- a/sound/core/seq/seq_clientmgr.c
+++ b/sound/core/seq/seq_clientmgr.c
@@ -1466,34 +1466,36 @@ static int seq_ioctl_subscribe_port(struct snd_seq_client *client, void *arg)
 /* 
  * remove from port's subscription list 
  */
-static int seq_ioctl_unsubscribe_port(struct snd_seq_client *client,
-				      void __user *arg)
+static int seq_ioctl_unsubscribe_port(struct snd_seq_client *client, void *arg)
 {
+	struct snd_seq_port_subscribe *subs = arg;
 	int result = -ENXIO;
 	struct snd_seq_client *receiver = NULL, *sender = NULL;
 	struct snd_seq_client_port *sport = NULL, *dport = NULL;
-	struct snd_seq_port_subscribe subs;
-
-	if (copy_from_user(&subs, arg, sizeof(subs)))
-		return -EFAULT;
 
-	if ((receiver = snd_seq_client_use_ptr(subs.dest.client)) == NULL)
+	receiver = snd_seq_client_use_ptr(subs->dest.client);
+	if (receiver == NULL)
 		goto __end;
-	if ((sender = snd_seq_client_use_ptr(subs.sender.client)) == NULL)
+	sender = snd_seq_client_use_ptr(subs->sender.client);
+	if (receiver == NULL)
 		goto __end;
-	if ((sport = snd_seq_port_use_ptr(sender, subs.sender.port)) == NULL)
+	sport = snd_seq_port_use_ptr(sender, subs->sender.port);
+	if (sport == NULL)
 		goto __end;
-	if ((dport = snd_seq_port_use_ptr(receiver, subs.dest.port)) == NULL)
+	dport = snd_seq_port_use_ptr(receiver, subs->dest.port);
+	if (dport == NULL)
 		goto __end;
 
-	result = check_subscription_permission(client, sport, dport, &subs);
+	result = check_subscription_permission(client, sport, dport, subs);
 	if (result < 0)
 		goto __end;
 
-	result = snd_seq_port_disconnect(client, sender, sport, receiver, dport, &subs);
-	if (! result) /* broadcast announce */
-		snd_seq_client_notify_subscription(SNDRV_SEQ_ADDRESS_SUBSCRIBERS, 0,
-						   &subs, SNDRV_SEQ_EVENT_PORT_UNSUBSCRIBED);
+	result = snd_seq_port_disconnect(client, sender, sport, receiver, dport,
+					 subs);
+	if (!result) /* broadcast announce */
+		snd_seq_client_notify_subscription(
+					SNDRV_SEQ_ADDRESS_SUBSCRIBERS, 0, subs,
+					SNDRV_SEQ_EVENT_PORT_UNSUBSCRIBED);
       __end:
       	if (sport)
 		snd_seq_port_unlock(sport);
-- 
2.7.4

^ permalink raw reply related	[flat|nested] 63+ messages in thread

* [PATCH 21/39] ALSA: seq: optimize create_queue function to new design
  2016-08-07  9:48 [PATCH 00/39] seq: obsolete change of address limit Takashi Sakamoto
                   ` (19 preceding siblings ...)
  2016-08-07  9:48 ` [PATCH 20/39] ALSA: seq: optimize unsubscribe_port " Takashi Sakamoto
@ 2016-08-07  9:48 ` Takashi Sakamoto
  2016-08-07  9:48 ` [PATCH 22/39] ALSA: seq: optimize delete_queue " Takashi Sakamoto
                   ` (17 subsequent siblings)
  38 siblings, 0 replies; 63+ messages in thread
From: Takashi Sakamoto @ 2016-08-07  9:48 UTC (permalink / raw)
  To: clemens, tiwai; +Cc: alsa-devel

In former commit, actual operations of each ioctl command get argument
in kernel space. Copying from/to user space is performed outside of
the function.

This commit optimizes to the new design.

Signed-off-by: Takashi Sakamoto <o-takashi@sakamocchi.jp>
---
 sound/core/seq/seq_clientmgr.c | 25 +++++++++----------------
 1 file changed, 9 insertions(+), 16 deletions(-)

diff --git a/sound/core/seq/seq_clientmgr.c b/sound/core/seq/seq_clientmgr.c
index 9f4bcf2..aa937c0 100644
--- a/sound/core/seq/seq_clientmgr.c
+++ b/sound/core/seq/seq_clientmgr.c
@@ -1510,17 +1510,13 @@ static int seq_ioctl_unsubscribe_port(struct snd_seq_client *client, void *arg)
 
 
 /* CREATE_QUEUE ioctl() */
-static int seq_ioctl_create_queue(struct snd_seq_client *client,
-				  void __user *arg)
+static int seq_ioctl_create_queue(struct snd_seq_client *client, void *arg)
 {
-	struct snd_seq_queue_info info;
+	struct snd_seq_queue_info *info = arg;
 	int result;
 	struct snd_seq_queue *q;
 
-	if (copy_from_user(&info, arg, sizeof(info)))
-		return -EFAULT;
-
-	result = snd_seq_queue_alloc(client->number, info.locked, info.flags);
+	result = snd_seq_queue_alloc(client->number, info->locked, info->flags);
 	if (result < 0)
 		return result;
 
@@ -1528,19 +1524,16 @@ static int seq_ioctl_create_queue(struct snd_seq_client *client,
 	if (q == NULL)
 		return -EINVAL;
 
-	info.queue = q->queue;
-	info.locked = q->locked;
-	info.owner = q->owner;
+	info->queue = q->queue;
+	info->locked = q->locked;
+	info->owner = q->owner;
 
 	/* set queue name */
-	if (! info.name[0])
-		snprintf(info.name, sizeof(info.name), "Queue-%d", q->queue);
-	strlcpy(q->name, info.name, sizeof(q->name));
+	if (!info->name[0])
+		snprintf(info->name, sizeof(info->name), "Queue-%d", q->queue);
+	strlcpy(q->name, info->name, sizeof(q->name));
 	queuefree(q);
 
-	if (copy_to_user(arg, &info, sizeof(info)))
-		return -EFAULT;
-
 	return 0;
 }
 
-- 
2.7.4

^ permalink raw reply related	[flat|nested] 63+ messages in thread

* [PATCH 22/39] ALSA: seq: optimize delete_queue function to new design
  2016-08-07  9:48 [PATCH 00/39] seq: obsolete change of address limit Takashi Sakamoto
                   ` (20 preceding siblings ...)
  2016-08-07  9:48 ` [PATCH 21/39] ALSA: seq: optimize create_queue " Takashi Sakamoto
@ 2016-08-07  9:48 ` Takashi Sakamoto
  2016-08-07  9:48 ` [PATCH 23/39] ALSA: seq: optimize get_queue_info " Takashi Sakamoto
                   ` (16 subsequent siblings)
  38 siblings, 0 replies; 63+ messages in thread
From: Takashi Sakamoto @ 2016-08-07  9:48 UTC (permalink / raw)
  To: clemens, tiwai; +Cc: alsa-devel

In former commit, actual operations of each ioctl command get argument
in kernel space. Copying from/to user space is performed outside of
the function.

This commit optimizes to the new design.

Signed-off-by: Takashi Sakamoto <o-takashi@sakamocchi.jp>
---
 sound/core/seq/seq_clientmgr.c | 10 +++-------
 1 file changed, 3 insertions(+), 7 deletions(-)

diff --git a/sound/core/seq/seq_clientmgr.c b/sound/core/seq/seq_clientmgr.c
index aa937c0..167be82 100644
--- a/sound/core/seq/seq_clientmgr.c
+++ b/sound/core/seq/seq_clientmgr.c
@@ -1538,15 +1538,11 @@ static int seq_ioctl_create_queue(struct snd_seq_client *client, void *arg)
 }
 
 /* DELETE_QUEUE ioctl() */
-static int seq_ioctl_delete_queue(struct snd_seq_client *client,
-				  void __user *arg)
+static int seq_ioctl_delete_queue(struct snd_seq_client *client, void *arg)
 {
-	struct snd_seq_queue_info info;
-
-	if (copy_from_user(&info, arg, sizeof(info)))
-		return -EFAULT;
+	struct snd_seq_queue_info *info = arg;
 
-	return snd_seq_queue_delete(client->number, info.queue);
+	return snd_seq_queue_delete(client->number, info->queue);
 }
 
 /* GET_QUEUE_INFO ioctl() */
-- 
2.7.4

^ permalink raw reply related	[flat|nested] 63+ messages in thread

* [PATCH 23/39] ALSA: seq: optimize get_queue_info function to new design
  2016-08-07  9:48 [PATCH 00/39] seq: obsolete change of address limit Takashi Sakamoto
                   ` (21 preceding siblings ...)
  2016-08-07  9:48 ` [PATCH 22/39] ALSA: seq: optimize delete_queue " Takashi Sakamoto
@ 2016-08-07  9:48 ` Takashi Sakamoto
  2016-08-07  9:49 ` [PATCH 24/39] ALSA: seq: optimize seq_queue_info " Takashi Sakamoto
                   ` (15 subsequent siblings)
  38 siblings, 0 replies; 63+ messages in thread
From: Takashi Sakamoto @ 2016-08-07  9:48 UTC (permalink / raw)
  To: clemens, tiwai; +Cc: alsa-devel

In former commit, actual operations of each ioctl command get argument
in kernel space. Copying from/to user space is performed outside of
the function.

This commit optimizes to the new design.

Signed-off-by: Takashi Sakamoto <o-takashi@sakamocchi.jp>
---
 sound/core/seq/seq_clientmgr.c | 23 ++++++++---------------
 1 file changed, 8 insertions(+), 15 deletions(-)

diff --git a/sound/core/seq/seq_clientmgr.c b/sound/core/seq/seq_clientmgr.c
index 167be82..5756e2c 100644
--- a/sound/core/seq/seq_clientmgr.c
+++ b/sound/core/seq/seq_clientmgr.c
@@ -1546,29 +1546,22 @@ static int seq_ioctl_delete_queue(struct snd_seq_client *client, void *arg)
 }
 
 /* GET_QUEUE_INFO ioctl() */
-static int seq_ioctl_get_queue_info(struct snd_seq_client *client,
-				    void __user *arg)
+static int seq_ioctl_get_queue_info(struct snd_seq_client *client, void *arg)
 {
-	struct snd_seq_queue_info info;
+	struct snd_seq_queue_info *info = arg;
 	struct snd_seq_queue *q;
 
-	if (copy_from_user(&info, arg, sizeof(info)))
-		return -EFAULT;
-
-	q = queueptr(info.queue);
+	q = queueptr(info->queue);
 	if (q == NULL)
 		return -EINVAL;
 
-	memset(&info, 0, sizeof(info));
-	info.queue = q->queue;
-	info.owner = q->owner;
-	info.locked = q->locked;
-	strlcpy(info.name, q->name, sizeof(info.name));
+	memset(info, 0, sizeof(*info));
+	info->queue = q->queue;
+	info->owner = q->owner;
+	info->locked = q->locked;
+	strlcpy(info->name, q->name, sizeof(info->name));
 	queuefree(q);
 
-	if (copy_to_user(arg, &info, sizeof(info)))
-		return -EFAULT;
-
 	return 0;
 }
 
-- 
2.7.4

^ permalink raw reply related	[flat|nested] 63+ messages in thread

* [PATCH 24/39] ALSA: seq: optimize seq_queue_info function to new design
  2016-08-07  9:48 [PATCH 00/39] seq: obsolete change of address limit Takashi Sakamoto
                   ` (22 preceding siblings ...)
  2016-08-07  9:48 ` [PATCH 23/39] ALSA: seq: optimize get_queue_info " Takashi Sakamoto
@ 2016-08-07  9:49 ` Takashi Sakamoto
  2016-08-07  9:49 ` [PATCH 25/39] ALSA: seq: optimize get_named_queue " Takashi Sakamoto
                   ` (14 subsequent siblings)
  38 siblings, 0 replies; 63+ messages in thread
From: Takashi Sakamoto @ 2016-08-07  9:49 UTC (permalink / raw)
  To: clemens, tiwai; +Cc: alsa-devel

In former commit, actual operations of each ioctl command get argument
in kernel space. Copying from/to user space is performed outside of
the function.

This commit optimizes to the new design.

Signed-off-by: Takashi Sakamoto <o-takashi@sakamocchi.jp>
---
 sound/core/seq/seq_clientmgr.c | 24 ++++++++++--------------
 1 file changed, 10 insertions(+), 14 deletions(-)

diff --git a/sound/core/seq/seq_clientmgr.c b/sound/core/seq/seq_clientmgr.c
index 5756e2c..70f75ec 100644
--- a/sound/core/seq/seq_clientmgr.c
+++ b/sound/core/seq/seq_clientmgr.c
@@ -1566,36 +1566,32 @@ static int seq_ioctl_get_queue_info(struct snd_seq_client *client, void *arg)
 }
 
 /* SET_QUEUE_INFO ioctl() */
-static int seq_ioctl_set_queue_info(struct snd_seq_client *client,
-				    void __user *arg)
+static int seq_ioctl_set_queue_info(struct snd_seq_client *client, void *arg)
 {
-	struct snd_seq_queue_info info;
+	struct snd_seq_queue_info *info = arg;
 	struct snd_seq_queue *q;
 
-	if (copy_from_user(&info, arg, sizeof(info)))
-		return -EFAULT;
-
-	if (info.owner != client->number)
+	if (info->owner != client->number)
 		return -EINVAL;
 
 	/* change owner/locked permission */
-	if (snd_seq_queue_check_access(info.queue, client->number)) {
-		if (snd_seq_queue_set_owner(info.queue, client->number, info.locked) < 0)
+	if (snd_seq_queue_check_access(info->queue, client->number)) {
+		if (snd_seq_queue_set_owner(info->queue, client->number, info->locked) < 0)
 			return -EPERM;
-		if (info.locked)
-			snd_seq_queue_use(info.queue, client->number, 1);
+		if (info->locked)
+			snd_seq_queue_use(info->queue, client->number, 1);
 	} else {
 		return -EPERM;
 	}	
 
-	q = queueptr(info.queue);
-	if (! q)
+	q = queueptr(info->queue);
+	if (!q)
 		return -EINVAL;
 	if (q->owner != client->number) {
 		queuefree(q);
 		return -EPERM;
 	}
-	strlcpy(q->name, info.name, sizeof(q->name));
+	strlcpy(q->name, info->name, sizeof(q->name));
 	queuefree(q);
 
 	return 0;
-- 
2.7.4

^ permalink raw reply related	[flat|nested] 63+ messages in thread

* [PATCH 25/39] ALSA: seq: optimize get_named_queue function to new design
  2016-08-07  9:48 [PATCH 00/39] seq: obsolete change of address limit Takashi Sakamoto
                   ` (23 preceding siblings ...)
  2016-08-07  9:49 ` [PATCH 24/39] ALSA: seq: optimize seq_queue_info " Takashi Sakamoto
@ 2016-08-07  9:49 ` Takashi Sakamoto
  2016-08-07  9:49 ` [PATCH 26/39] ALSA: seq: optimize get_queue_status " Takashi Sakamoto
                   ` (13 subsequent siblings)
  38 siblings, 0 replies; 63+ messages in thread
From: Takashi Sakamoto @ 2016-08-07  9:49 UTC (permalink / raw)
  To: clemens, tiwai; +Cc: alsa-devel

In former commit, actual operations of each ioctl command get argument
in kernel space. Copying from/to user space is performed outside of
the function.

This commit optimizes to the new design.

Signed-off-by: Takashi Sakamoto <o-takashi@sakamocchi.jp>
---
 sound/core/seq/seq_clientmgr.c | 19 ++++++-------------
 1 file changed, 6 insertions(+), 13 deletions(-)

diff --git a/sound/core/seq/seq_clientmgr.c b/sound/core/seq/seq_clientmgr.c
index 70f75ec..9afb0aa 100644
--- a/sound/core/seq/seq_clientmgr.c
+++ b/sound/core/seq/seq_clientmgr.c
@@ -1598,26 +1598,19 @@ static int seq_ioctl_set_queue_info(struct snd_seq_client *client, void *arg)
 }
 
 /* GET_NAMED_QUEUE ioctl() */
-static int seq_ioctl_get_named_queue(struct snd_seq_client *client,
-				     void __user *arg)
+static int seq_ioctl_get_named_queue(struct snd_seq_client *client, void *arg)
 {
-	struct snd_seq_queue_info info;
+	struct snd_seq_queue_info *info = arg;
 	struct snd_seq_queue *q;
 
-	if (copy_from_user(&info, arg, sizeof(info)))
-		return -EFAULT;
-
-	q = snd_seq_queue_find_name(info.name);
+	q = snd_seq_queue_find_name(info->name);
 	if (q == NULL)
 		return -EINVAL;
-	info.queue = q->queue;
-	info.owner = q->owner;
-	info.locked = q->locked;
+	info->queue = q->queue;
+	info->owner = q->owner;
+	info->locked = q->locked;
 	queuefree(q);
 
-	if (copy_to_user(arg, &info, sizeof(info)))
-		return -EFAULT;
-
 	return 0;
 }
 
-- 
2.7.4

^ permalink raw reply related	[flat|nested] 63+ messages in thread

* [PATCH 26/39] ALSA: seq: optimize get_queue_status function to new design
  2016-08-07  9:48 [PATCH 00/39] seq: obsolete change of address limit Takashi Sakamoto
                   ` (24 preceding siblings ...)
  2016-08-07  9:49 ` [PATCH 25/39] ALSA: seq: optimize get_named_queue " Takashi Sakamoto
@ 2016-08-07  9:49 ` Takashi Sakamoto
  2016-08-07  9:49 ` [PATCH 27/39] ALSA: seq: optimize get_queue_tempo " Takashi Sakamoto
                   ` (12 subsequent siblings)
  38 siblings, 0 replies; 63+ messages in thread
From: Takashi Sakamoto @ 2016-08-07  9:49 UTC (permalink / raw)
  To: clemens, tiwai; +Cc: alsa-devel

In former commit, actual operations of each ioctl command get argument
in kernel space. Copying from/to user space is performed outside of
the function.

This commit optimizes to the new design.

Signed-off-by: Takashi Sakamoto <o-takashi@sakamocchi.jp>
---
 sound/core/seq/seq_clientmgr.c | 26 ++++++++++----------------
 1 file changed, 10 insertions(+), 16 deletions(-)

diff --git a/sound/core/seq/seq_clientmgr.c b/sound/core/seq/seq_clientmgr.c
index 9afb0aa..40346f2 100644
--- a/sound/core/seq/seq_clientmgr.c
+++ b/sound/core/seq/seq_clientmgr.c
@@ -1615,35 +1615,29 @@ static int seq_ioctl_get_named_queue(struct snd_seq_client *client, void *arg)
 }
 
 /* GET_QUEUE_STATUS ioctl() */
-static int seq_ioctl_get_queue_status(struct snd_seq_client *client,
-				      void __user *arg)
+static int seq_ioctl_get_queue_status(struct snd_seq_client *client, void *arg)
 {
-	struct snd_seq_queue_status status;
+	struct snd_seq_queue_status *status = arg;
 	struct snd_seq_queue *queue;
 	struct snd_seq_timer *tmr;
 
-	if (copy_from_user(&status, arg, sizeof(status)))
-		return -EFAULT;
-
-	queue = queueptr(status.queue);
+	queue = queueptr(status->queue);
 	if (queue == NULL)
 		return -EINVAL;
-	memset(&status, 0, sizeof(status));
-	status.queue = queue->queue;
+	memset(status, 0, sizeof(*status));
+	status->queue = queue->queue;
 	
 	tmr = queue->timer;
-	status.events = queue->tickq->cells + queue->timeq->cells;
+	status->events = queue->tickq->cells + queue->timeq->cells;
 
-	status.time = snd_seq_timer_get_cur_time(tmr);
-	status.tick = snd_seq_timer_get_cur_tick(tmr);
+	status->time = snd_seq_timer_get_cur_time(tmr);
+	status->tick = snd_seq_timer_get_cur_tick(tmr);
 
-	status.running = tmr->running;
+	status->running = tmr->running;
 
-	status.flags = queue->flags;
+	status->flags = queue->flags;
 	queuefree(queue);
 
-	if (copy_to_user(arg, &status, sizeof(status)))
-		return -EFAULT;
 	return 0;
 }
 
-- 
2.7.4

^ permalink raw reply related	[flat|nested] 63+ messages in thread

* [PATCH 27/39] ALSA: seq: optimize get_queue_tempo function to new design
  2016-08-07  9:48 [PATCH 00/39] seq: obsolete change of address limit Takashi Sakamoto
                   ` (25 preceding siblings ...)
  2016-08-07  9:49 ` [PATCH 26/39] ALSA: seq: optimize get_queue_status " Takashi Sakamoto
@ 2016-08-07  9:49 ` Takashi Sakamoto
  2016-08-07  9:49 ` [PATCH 28/39] ALSA: seq: optimize set_queue_tempo " Takashi Sakamoto
                   ` (11 subsequent siblings)
  38 siblings, 0 replies; 63+ messages in thread
From: Takashi Sakamoto @ 2016-08-07  9:49 UTC (permalink / raw)
  To: clemens, tiwai; +Cc: alsa-devel

In former commit, actual operations of each ioctl command get argument
in kernel space. Copying from/to user space is performed outside of
the function.

This commit optimizes to the new design.

Signed-off-by: Takashi Sakamoto <o-takashi@sakamocchi.jp>
---
 sound/core/seq/seq_clientmgr.c | 24 +++++++++---------------
 1 file changed, 9 insertions(+), 15 deletions(-)

diff --git a/sound/core/seq/seq_clientmgr.c b/sound/core/seq/seq_clientmgr.c
index 40346f2..2338468 100644
--- a/sound/core/seq/seq_clientmgr.c
+++ b/sound/core/seq/seq_clientmgr.c
@@ -1643,32 +1643,26 @@ static int seq_ioctl_get_queue_status(struct snd_seq_client *client, void *arg)
 
 
 /* GET_QUEUE_TEMPO ioctl() */
-static int seq_ioctl_get_queue_tempo(struct snd_seq_client *client,
-				     void __user *arg)
+static int seq_ioctl_get_queue_tempo(struct snd_seq_client *client, void *arg)
 {
-	struct snd_seq_queue_tempo tempo;
+	struct snd_seq_queue_tempo *tempo = arg;
 	struct snd_seq_queue *queue;
 	struct snd_seq_timer *tmr;
 
-	if (copy_from_user(&tempo, arg, sizeof(tempo)))
-		return -EFAULT;
-
-	queue = queueptr(tempo.queue);
+	queue = queueptr(tempo->queue);
 	if (queue == NULL)
 		return -EINVAL;
-	memset(&tempo, 0, sizeof(tempo));
-	tempo.queue = queue->queue;
+	memset(tempo, 0, sizeof(*tempo));
+	tempo->queue = queue->queue;
 	
 	tmr = queue->timer;
 
-	tempo.tempo = tmr->tempo;
-	tempo.ppq = tmr->ppq;
-	tempo.skew_value = tmr->skew;
-	tempo.skew_base = tmr->skew_base;
+	tempo->tempo = tmr->tempo;
+	tempo->ppq = tmr->ppq;
+	tempo->skew_value = tmr->skew;
+	tempo->skew_base = tmr->skew_base;
 	queuefree(queue);
 
-	if (copy_to_user(arg, &tempo, sizeof(tempo)))
-		return -EFAULT;
 	return 0;
 }
 
-- 
2.7.4

^ permalink raw reply related	[flat|nested] 63+ messages in thread

* [PATCH 28/39] ALSA: seq: optimize set_queue_tempo function to new design
  2016-08-07  9:48 [PATCH 00/39] seq: obsolete change of address limit Takashi Sakamoto
                   ` (26 preceding siblings ...)
  2016-08-07  9:49 ` [PATCH 27/39] ALSA: seq: optimize get_queue_tempo " Takashi Sakamoto
@ 2016-08-07  9:49 ` Takashi Sakamoto
  2016-08-07  9:49 ` [PATCH 29/39] ALSA: seq: optimize get_queue_timer " Takashi Sakamoto
                   ` (10 subsequent siblings)
  38 siblings, 0 replies; 63+ messages in thread
From: Takashi Sakamoto @ 2016-08-07  9:49 UTC (permalink / raw)
  To: clemens, tiwai; +Cc: alsa-devel

In former commit, actual operations of each ioctl command get argument
in kernel space. Copying from/to user space is performed outside of
the function.

This commit optimizes to the new design.

Signed-off-by: Takashi Sakamoto <o-takashi@sakamocchi.jp>
---
 sound/core/seq/seq_clientmgr.c | 10 +++-------
 1 file changed, 3 insertions(+), 7 deletions(-)

diff --git a/sound/core/seq/seq_clientmgr.c b/sound/core/seq/seq_clientmgr.c
index 2338468..a90a4a3 100644
--- a/sound/core/seq/seq_clientmgr.c
+++ b/sound/core/seq/seq_clientmgr.c
@@ -1677,16 +1677,12 @@ int snd_seq_set_queue_tempo(int client, struct snd_seq_queue_tempo *tempo)
 
 EXPORT_SYMBOL(snd_seq_set_queue_tempo);
 
-static int seq_ioctl_set_queue_tempo(struct snd_seq_client *client,
-				     void __user *arg)
+static int seq_ioctl_set_queue_tempo(struct snd_seq_client *client, void *arg)
 {
+	struct snd_seq_queue_tempo *tempo = arg;
 	int result;
-	struct snd_seq_queue_tempo tempo;
-
-	if (copy_from_user(&tempo, arg, sizeof(tempo)))
-		return -EFAULT;
 
-	result = snd_seq_set_queue_tempo(client->number, &tempo);
+	result = snd_seq_set_queue_tempo(client->number, tempo);
 	return result < 0 ? result : 0;
 }
 
-- 
2.7.4

^ permalink raw reply related	[flat|nested] 63+ messages in thread

* [PATCH 29/39] ALSA: seq: optimize get_queue_timer function to new design
  2016-08-07  9:48 [PATCH 00/39] seq: obsolete change of address limit Takashi Sakamoto
                   ` (27 preceding siblings ...)
  2016-08-07  9:49 ` [PATCH 28/39] ALSA: seq: optimize set_queue_tempo " Takashi Sakamoto
@ 2016-08-07  9:49 ` Takashi Sakamoto
  2016-08-07  9:49 ` [PATCH 30/39] ALSA: seq: optimize seq_queue_timer " Takashi Sakamoto
                   ` (9 subsequent siblings)
  38 siblings, 0 replies; 63+ messages in thread
From: Takashi Sakamoto @ 2016-08-07  9:49 UTC (permalink / raw)
  To: clemens, tiwai; +Cc: alsa-devel

In former commit, actual operations of each ioctl command get argument
in kernel space. Copying from/to user space is performed outside of
the function.

This commit optimizes to the new design.

Signed-off-by: Takashi Sakamoto <o-takashi@sakamocchi.jp>
---
 sound/core/seq/seq_clientmgr.c | 22 ++++++++--------------
 1 file changed, 8 insertions(+), 14 deletions(-)

diff --git a/sound/core/seq/seq_clientmgr.c b/sound/core/seq/seq_clientmgr.c
index a90a4a3..383f5d3 100644
--- a/sound/core/seq/seq_clientmgr.c
+++ b/sound/core/seq/seq_clientmgr.c
@@ -1688,17 +1688,13 @@ static int seq_ioctl_set_queue_tempo(struct snd_seq_client *client, void *arg)
 
 
 /* GET_QUEUE_TIMER ioctl() */
-static int seq_ioctl_get_queue_timer(struct snd_seq_client *client,
-				     void __user *arg)
+static int seq_ioctl_get_queue_timer(struct snd_seq_client *client, void *arg)
 {
-	struct snd_seq_queue_timer timer;
+	struct snd_seq_queue_timer *timer = arg;
 	struct snd_seq_queue *queue;
 	struct snd_seq_timer *tmr;
 
-	if (copy_from_user(&timer, arg, sizeof(timer)))
-		return -EFAULT;
-
-	queue = queueptr(timer.queue);
+	queue = queueptr(timer->queue);
 	if (queue == NULL)
 		return -EINVAL;
 
@@ -1707,19 +1703,17 @@ static int seq_ioctl_get_queue_timer(struct snd_seq_client *client,
 		return -ERESTARTSYS;
 	}
 	tmr = queue->timer;
-	memset(&timer, 0, sizeof(timer));
-	timer.queue = queue->queue;
+	memset(timer, 0, sizeof(*timer));
+	timer->queue = queue->queue;
 
-	timer.type = tmr->type;
+	timer->type = tmr->type;
 	if (tmr->type == SNDRV_SEQ_TIMER_ALSA) {
-		timer.u.alsa.id = tmr->alsa_id;
-		timer.u.alsa.resolution = tmr->preferred_resolution;
+		timer->u.alsa.id = tmr->alsa_id;
+		timer->u.alsa.resolution = tmr->preferred_resolution;
 	}
 	mutex_unlock(&queue->timer_mutex);
 	queuefree(queue);
 	
-	if (copy_to_user(arg, &timer, sizeof(timer)))
-		return -EFAULT;
 	return 0;
 }
 
-- 
2.7.4

^ permalink raw reply related	[flat|nested] 63+ messages in thread

* [PATCH 30/39] ALSA: seq: optimize seq_queue_timer function to new design
  2016-08-07  9:48 [PATCH 00/39] seq: obsolete change of address limit Takashi Sakamoto
                   ` (28 preceding siblings ...)
  2016-08-07  9:49 ` [PATCH 29/39] ALSA: seq: optimize get_queue_timer " Takashi Sakamoto
@ 2016-08-07  9:49 ` Takashi Sakamoto
  2016-08-07  9:49 ` [PATCH 31/39] ALSA: seq: optimize get_queue_client " Takashi Sakamoto
                   ` (8 subsequent siblings)
  38 siblings, 0 replies; 63+ messages in thread
From: Takashi Sakamoto @ 2016-08-07  9:49 UTC (permalink / raw)
  To: clemens, tiwai; +Cc: alsa-devel

In former commit, actual operations of each ioctl command get argument
in kernel space. Copying from/to user space is performed outside of
the function.

This commit optimizes to the new design.

Signed-off-by: Takashi Sakamoto <o-takashi@sakamocchi.jp>
---
 sound/core/seq/seq_clientmgr.c | 24 ++++++++++--------------
 1 file changed, 10 insertions(+), 14 deletions(-)

diff --git a/sound/core/seq/seq_clientmgr.c b/sound/core/seq/seq_clientmgr.c
index 383f5d3..667ad23 100644
--- a/sound/core/seq/seq_clientmgr.c
+++ b/sound/core/seq/seq_clientmgr.c
@@ -1719,23 +1719,19 @@ static int seq_ioctl_get_queue_timer(struct snd_seq_client *client, void *arg)
 
 
 /* SET_QUEUE_TIMER ioctl() */
-static int seq_ioctl_set_queue_timer(struct snd_seq_client *client,
-				     void __user *arg)
+static int seq_ioctl_set_queue_timer(struct snd_seq_client *client, void *arg)
 {
+	struct snd_seq_queue_timer *timer = arg;
 	int result = 0;
-	struct snd_seq_queue_timer timer;
-
-	if (copy_from_user(&timer, arg, sizeof(timer)))
-		return -EFAULT;
 
-	if (timer.type != SNDRV_SEQ_TIMER_ALSA)
+	if (timer->type != SNDRV_SEQ_TIMER_ALSA)
 		return -EINVAL;
 
-	if (snd_seq_queue_check_access(timer.queue, client->number)) {
+	if (snd_seq_queue_check_access(timer->queue, client->number)) {
 		struct snd_seq_queue *q;
 		struct snd_seq_timer *tmr;
 
-		q = queueptr(timer.queue);
+		q = queueptr(timer->queue);
 		if (q == NULL)
 			return -ENXIO;
 		if (mutex_lock_interruptible(&q->timer_mutex)) {
@@ -1743,13 +1739,13 @@ static int seq_ioctl_set_queue_timer(struct snd_seq_client *client,
 			return -ERESTARTSYS;
 		}
 		tmr = q->timer;
-		snd_seq_queue_timer_close(timer.queue);
-		tmr->type = timer.type;
+		snd_seq_queue_timer_close(timer->queue);
+		tmr->type = timer->type;
 		if (tmr->type == SNDRV_SEQ_TIMER_ALSA) {
-			tmr->alsa_id = timer.u.alsa.id;
-			tmr->preferred_resolution = timer.u.alsa.resolution;
+			tmr->alsa_id = timer->u.alsa.id;
+			tmr->preferred_resolution = timer->u.alsa.resolution;
 		}
-		result = snd_seq_queue_timer_open(timer.queue);
+		result = snd_seq_queue_timer_open(timer->queue);
 		mutex_unlock(&q->timer_mutex);
 		queuefree(q);
 	} else {
-- 
2.7.4

^ permalink raw reply related	[flat|nested] 63+ messages in thread

* [PATCH 31/39] ALSA: seq: optimize get_queue_client function to new design
  2016-08-07  9:48 [PATCH 00/39] seq: obsolete change of address limit Takashi Sakamoto
                   ` (29 preceding siblings ...)
  2016-08-07  9:49 ` [PATCH 30/39] ALSA: seq: optimize seq_queue_timer " Takashi Sakamoto
@ 2016-08-07  9:49 ` Takashi Sakamoto
  2016-08-07  9:49 ` [PATCH 32/39] ALSA: seq: optimize set_queue_client " Takashi Sakamoto
                   ` (7 subsequent siblings)
  38 siblings, 0 replies; 63+ messages in thread
From: Takashi Sakamoto @ 2016-08-07  9:49 UTC (permalink / raw)
  To: clemens, tiwai; +Cc: alsa-devel

In former commit, actual operations of each ioctl command get argument
in kernel space. Copying from/to user space is performed outside of
the function.

This commit optimizes to the new design.

Signed-off-by: Takashi Sakamoto <o-takashi@sakamocchi.jp>
---
 sound/core/seq/seq_clientmgr.c | 16 +++++-----------
 1 file changed, 5 insertions(+), 11 deletions(-)

diff --git a/sound/core/seq/seq_clientmgr.c b/sound/core/seq/seq_clientmgr.c
index 667ad23..30aac28 100644
--- a/sound/core/seq/seq_clientmgr.c
+++ b/sound/core/seq/seq_clientmgr.c
@@ -1757,23 +1757,17 @@ static int seq_ioctl_set_queue_timer(struct snd_seq_client *client, void *arg)
 
 
 /* GET_QUEUE_CLIENT ioctl() */
-static int seq_ioctl_get_queue_client(struct snd_seq_client *client,
-				      void __user *arg)
+static int seq_ioctl_get_queue_client(struct snd_seq_client *client, void *arg)
 {
-	struct snd_seq_queue_client info;
+	struct snd_seq_queue_client *info = arg;
 	int used;
 
-	if (copy_from_user(&info, arg, sizeof(info)))
-		return -EFAULT;
-
-	used = snd_seq_queue_is_used(info.queue, client->number);
+	used = snd_seq_queue_is_used(info->queue, client->number);
 	if (used < 0)
 		return -EINVAL;
-	info.used = used;
-	info.client = client->number;
+	info->used = used;
+	info->client = client->number;
 
-	if (copy_to_user(arg, &info, sizeof(info)))
-		return -EFAULT;
 	return 0;
 }
 
-- 
2.7.4

^ permalink raw reply related	[flat|nested] 63+ messages in thread

* [PATCH 32/39] ALSA: seq: optimize set_queue_client function to new design
  2016-08-07  9:48 [PATCH 00/39] seq: obsolete change of address limit Takashi Sakamoto
                   ` (30 preceding siblings ...)
  2016-08-07  9:49 ` [PATCH 31/39] ALSA: seq: optimize get_queue_client " Takashi Sakamoto
@ 2016-08-07  9:49 ` Takashi Sakamoto
  2016-08-07  9:49 ` [PATCH 33/39] ALSA: seq: optimize get_client_pool " Takashi Sakamoto
                   ` (6 subsequent siblings)
  38 siblings, 0 replies; 63+ messages in thread
From: Takashi Sakamoto @ 2016-08-07  9:49 UTC (permalink / raw)
  To: clemens, tiwai; +Cc: alsa-devel

In former commit, actual operations of each ioctl command get argument
in kernel space. Copying from/to user space is performed outside of
the function.

This commit optimizes to the new design.

Signed-off-by: Takashi Sakamoto <o-takashi@sakamocchi.jp>
---
 sound/core/seq/seq_clientmgr.c | 13 +++++--------
 1 file changed, 5 insertions(+), 8 deletions(-)

diff --git a/sound/core/seq/seq_clientmgr.c b/sound/core/seq/seq_clientmgr.c
index 30aac28..6c1b406 100644
--- a/sound/core/seq/seq_clientmgr.c
+++ b/sound/core/seq/seq_clientmgr.c
@@ -1773,17 +1773,14 @@ static int seq_ioctl_get_queue_client(struct snd_seq_client *client, void *arg)
 
 
 /* SET_QUEUE_CLIENT ioctl() */
-static int seq_ioctl_set_queue_client(struct snd_seq_client *client,
-				      void __user *arg)
+static int seq_ioctl_set_queue_client(struct snd_seq_client *client, void *arg)
 {
+	struct snd_seq_queue_client *info = arg;
 	int err;
-	struct snd_seq_queue_client info;
-
-	if (copy_from_user(&info, arg, sizeof(info)))
-		return -EFAULT;
 
-	if (info.used >= 0) {
-		err = snd_seq_queue_use(info.queue, client->number, info.used);
+	if (info->used >= 0) {
+		err = snd_seq_queue_use(info->queue, client->number,
+					info->used);
 		if (err < 0)
 			return err;
 	}
-- 
2.7.4

^ permalink raw reply related	[flat|nested] 63+ messages in thread

* [PATCH 33/39] ALSA: seq: optimize get_client_pool function to new design
  2016-08-07  9:48 [PATCH 00/39] seq: obsolete change of address limit Takashi Sakamoto
                   ` (31 preceding siblings ...)
  2016-08-07  9:49 ` [PATCH 32/39] ALSA: seq: optimize set_queue_client " Takashi Sakamoto
@ 2016-08-07  9:49 ` Takashi Sakamoto
  2016-08-07  9:49 ` [PATCH 34/39] ALSA: seq: optimize seq_client_pool " Takashi Sakamoto
                   ` (5 subsequent siblings)
  38 siblings, 0 replies; 63+ messages in thread
From: Takashi Sakamoto @ 2016-08-07  9:49 UTC (permalink / raw)
  To: clemens, tiwai; +Cc: alsa-devel

In former commit, actual operations of each ioctl command get argument
in kernel space. Copying from/to user space is performed outside of
the function.

This commit optimizes to the new design.

Signed-off-by: Takashi Sakamoto <o-takashi@sakamocchi.jp>
---
 sound/core/seq/seq_clientmgr.c | 34 ++++++++++++++--------------------
 1 file changed, 14 insertions(+), 20 deletions(-)

diff --git a/sound/core/seq/seq_clientmgr.c b/sound/core/seq/seq_clientmgr.c
index 6c1b406..2e63082 100644
--- a/sound/core/seq/seq_clientmgr.c
+++ b/sound/core/seq/seq_clientmgr.c
@@ -1790,37 +1790,31 @@ static int seq_ioctl_set_queue_client(struct snd_seq_client *client, void *arg)
 
 
 /* GET_CLIENT_POOL ioctl() */
-static int seq_ioctl_get_client_pool(struct snd_seq_client *client,
-				     void __user *arg)
+static int seq_ioctl_get_client_pool(struct snd_seq_client *client, void *arg)
 {
-	struct snd_seq_client_pool info;
+	struct snd_seq_client_pool *info = arg;
 	struct snd_seq_client *cptr;
 
-	if (copy_from_user(&info, arg, sizeof(info)))
-		return -EFAULT;
-
-	cptr = snd_seq_client_use_ptr(info.client);
+	cptr = snd_seq_client_use_ptr(info->client);
 	if (cptr == NULL)
 		return -ENOENT;
-	memset(&info, 0, sizeof(info));
-	info.client = cptr->number;
-	info.output_pool = cptr->pool->size;
-	info.output_room = cptr->pool->room;
-	info.output_free = info.output_pool;
-	info.output_free = snd_seq_unused_cells(cptr->pool);
+	memset(info, 0, sizeof(*info));
+	info->client = cptr->number;
+	info->output_pool = cptr->pool->size;
+	info->output_room = cptr->pool->room;
+	info->output_free = info->output_pool;
+	info->output_free = snd_seq_unused_cells(cptr->pool);
 	if (cptr->type == USER_CLIENT) {
-		info.input_pool = cptr->data.user.fifo_pool_size;
-		info.input_free = info.input_pool;
+		info->input_pool = cptr->data.user.fifo_pool_size;
+		info->input_free = info->input_pool;
 		if (cptr->data.user.fifo)
-			info.input_free = snd_seq_unused_cells(cptr->data.user.fifo->pool);
+			info->input_free = snd_seq_unused_cells(cptr->data.user.fifo->pool);
 	} else {
-		info.input_pool = 0;
-		info.input_free = 0;
+		info->input_pool = 0;
+		info->input_free = 0;
 	}
 	snd_seq_client_unlock(cptr);
 	
-	if (copy_to_user(arg, &info, sizeof(info)))
-		return -EFAULT;
 	return 0;
 }
 
-- 
2.7.4

^ permalink raw reply related	[flat|nested] 63+ messages in thread

* [PATCH 34/39] ALSA: seq: optimize seq_client_pool function to new design
  2016-08-07  9:48 [PATCH 00/39] seq: obsolete change of address limit Takashi Sakamoto
                   ` (32 preceding siblings ...)
  2016-08-07  9:49 ` [PATCH 33/39] ALSA: seq: optimize get_client_pool " Takashi Sakamoto
@ 2016-08-07  9:49 ` Takashi Sakamoto
  2016-08-07  9:49 ` [PATCH 35/39] ALSA: seq: optimize remove_events " Takashi Sakamoto
                   ` (4 subsequent siblings)
  38 siblings, 0 replies; 63+ messages in thread
From: Takashi Sakamoto @ 2016-08-07  9:49 UTC (permalink / raw)
  To: clemens, tiwai; +Cc: alsa-devel

In former commit, actual operations of each ioctl command get argument
in kernel space. Copying from/to user space is performed outside of
the function.

This commit optimizes to the new design.

Signed-off-by: Takashi Sakamoto <o-takashi@sakamocchi.jp>
---
 sound/core/seq/seq_clientmgr.c | 36 +++++++++++++++++-------------------
 1 file changed, 17 insertions(+), 19 deletions(-)

diff --git a/sound/core/seq/seq_clientmgr.c b/sound/core/seq/seq_clientmgr.c
index 2e63082..bf86ca3 100644
--- a/sound/core/seq/seq_clientmgr.c
+++ b/sound/core/seq/seq_clientmgr.c
@@ -1819,44 +1819,42 @@ static int seq_ioctl_get_client_pool(struct snd_seq_client *client, void *arg)
 }
 
 /* SET_CLIENT_POOL ioctl() */
-static int seq_ioctl_set_client_pool(struct snd_seq_client *client,
-				     void __user *arg)
+static int seq_ioctl_set_client_pool(struct snd_seq_client *client, void *arg)
 {
-	struct snd_seq_client_pool info;
+	struct snd_seq_client_pool *info = arg;
 	int rc;
 
-	if (copy_from_user(&info, arg, sizeof(info)))
-		return -EFAULT;
-
-	if (client->number != info.client)
+	if (client->number != info->client)
 		return -EINVAL; /* can't change other clients */
 
-	if (info.output_pool >= 1 && info.output_pool <= SNDRV_SEQ_MAX_EVENTS &&
-	    (! snd_seq_write_pool_allocated(client) ||
-	     info.output_pool != client->pool->size)) {
+	if (info->output_pool >= 1 &&
+	    info->output_pool <= SNDRV_SEQ_MAX_EVENTS &&
+	    (!snd_seq_write_pool_allocated(client) ||
+	     info->output_pool != client->pool->size)) {
 		if (snd_seq_write_pool_allocated(client)) {
 			/* remove all existing cells */
 			snd_seq_queue_client_leave_cells(client->number);
 			snd_seq_pool_done(client->pool);
 		}
-		client->pool->size = info.output_pool;
+		client->pool->size = info->output_pool;
 		rc = snd_seq_pool_init(client->pool);
 		if (rc < 0)
 			return rc;
 	}
 	if (client->type == USER_CLIENT && client->data.user.fifo != NULL &&
-	    info.input_pool >= 1 &&
-	    info.input_pool <= SNDRV_SEQ_MAX_CLIENT_EVENTS &&
-	    info.input_pool != client->data.user.fifo_pool_size) {
+	    info->input_pool >= 1 &&
+	    info->input_pool <= SNDRV_SEQ_MAX_CLIENT_EVENTS &&
+	    info->input_pool != client->data.user.fifo_pool_size) {
 		/* change pool size */
-		rc = snd_seq_fifo_resize(client->data.user.fifo, info.input_pool);
+		rc = snd_seq_fifo_resize(client->data.user.fifo,
+					 info->input_pool);
 		if (rc < 0)
 			return rc;
-		client->data.user.fifo_pool_size = info.input_pool;
+		client->data.user.fifo_pool_size = info->input_pool;
 	}
-	if (info.output_room >= 1 &&
-	    info.output_room <= client->pool->size) {
-		client->pool->room  = info.output_room;
+	if (info->output_room >= 1 &&
+	    info->output_room <= client->pool->size) {
+		client->pool->room  = info->output_room;
 	}
 
 	return seq_ioctl_get_client_pool(client, arg);
-- 
2.7.4

^ permalink raw reply related	[flat|nested] 63+ messages in thread

* [PATCH 35/39] ALSA: seq: optimize remove_events function to new design
  2016-08-07  9:48 [PATCH 00/39] seq: obsolete change of address limit Takashi Sakamoto
                   ` (33 preceding siblings ...)
  2016-08-07  9:49 ` [PATCH 34/39] ALSA: seq: optimize seq_client_pool " Takashi Sakamoto
@ 2016-08-07  9:49 ` Takashi Sakamoto
  2016-08-07  9:49 ` [PATCH 36/39] ALSA: seq: optimize get_subscription " Takashi Sakamoto
                   ` (3 subsequent siblings)
  38 siblings, 0 replies; 63+ messages in thread
From: Takashi Sakamoto @ 2016-08-07  9:49 UTC (permalink / raw)
  To: clemens, tiwai; +Cc: alsa-devel

In former commit, actual operations of each ioctl command get argument
in kernel space. Copying from/to user space is performed outside of
the function.

This commit optimizes to the new design.

Signed-off-by: Takashi Sakamoto <o-takashi@sakamocchi.jp>
---
 sound/core/seq/seq_clientmgr.c | 14 +++++---------
 1 file changed, 5 insertions(+), 9 deletions(-)

diff --git a/sound/core/seq/seq_clientmgr.c b/sound/core/seq/seq_clientmgr.c
index bf86ca3..b608521 100644
--- a/sound/core/seq/seq_clientmgr.c
+++ b/sound/core/seq/seq_clientmgr.c
@@ -1862,18 +1862,14 @@ static int seq_ioctl_set_client_pool(struct snd_seq_client *client, void *arg)
 
 
 /* REMOVE_EVENTS ioctl() */
-static int seq_ioctl_remove_events(struct snd_seq_client *client,
-				   void __user *arg)
+static int seq_ioctl_remove_events(struct snd_seq_client *client, void *arg)
 {
-	struct snd_seq_remove_events info;
-
-	if (copy_from_user(&info, arg, sizeof(info)))
-		return -EFAULT;
+	struct snd_seq_remove_events *info = arg;
 
 	/*
 	 * Input mostly not implemented XXX.
 	 */
-	if (info.remove_mode & SNDRV_SEQ_REMOVE_INPUT) {
+	if (info->remove_mode & SNDRV_SEQ_REMOVE_INPUT) {
 		/*
 		 * No restrictions so for a user client we can clear
 		 * the whole fifo
@@ -1882,8 +1878,8 @@ static int seq_ioctl_remove_events(struct snd_seq_client *client,
 			snd_seq_fifo_clear(client->data.user.fifo);
 	}
 
-	if (info.remove_mode & SNDRV_SEQ_REMOVE_OUTPUT)
-		snd_seq_queue_remove_cells(client->number, &info);
+	if (info->remove_mode & SNDRV_SEQ_REMOVE_OUTPUT)
+		snd_seq_queue_remove_cells(client->number, info);
 
 	return 0;
 }
-- 
2.7.4

^ permalink raw reply related	[flat|nested] 63+ messages in thread

* [PATCH 36/39] ALSA: seq: optimize get_subscription function to new design
  2016-08-07  9:48 [PATCH 00/39] seq: obsolete change of address limit Takashi Sakamoto
                   ` (34 preceding siblings ...)
  2016-08-07  9:49 ` [PATCH 35/39] ALSA: seq: optimize remove_events " Takashi Sakamoto
@ 2016-08-07  9:49 ` Takashi Sakamoto
  2016-08-07  9:49 ` [PATCH 37/39] ALSA: seq: optimize query_subs " Takashi Sakamoto
                   ` (2 subsequent siblings)
  38 siblings, 0 replies; 63+ messages in thread
From: Takashi Sakamoto @ 2016-08-07  9:49 UTC (permalink / raw)
  To: clemens, tiwai; +Cc: alsa-devel

In former commit, actual operations of each ioctl command get argument
in kernel space. Copying from/to user space is performed outside of
the function.

This commit optimizes to the new design.

Signed-off-by: Takashi Sakamoto <o-takashi@sakamocchi.jp>
---
 sound/core/seq/seq_clientmgr.c | 23 +++++++++--------------
 1 file changed, 9 insertions(+), 14 deletions(-)

diff --git a/sound/core/seq/seq_clientmgr.c b/sound/core/seq/seq_clientmgr.c
index b608521..cd4073d 100644
--- a/sound/core/seq/seq_clientmgr.c
+++ b/sound/core/seq/seq_clientmgr.c
@@ -1888,27 +1888,25 @@ static int seq_ioctl_remove_events(struct snd_seq_client *client, void *arg)
 /*
  * get subscription info
  */
-static int seq_ioctl_get_subscription(struct snd_seq_client *client,
-				      void __user *arg)
+static int seq_ioctl_get_subscription(struct snd_seq_client *client, void *arg)
 {
+	struct snd_seq_port_subscribe *subs = arg;
 	int result;
 	struct snd_seq_client *sender = NULL;
 	struct snd_seq_client_port *sport = NULL;
-	struct snd_seq_port_subscribe subs;
 	struct snd_seq_subscribers *p;
 
-	if (copy_from_user(&subs, arg, sizeof(subs)))
-		return -EFAULT;
-
 	result = -EINVAL;
-	if ((sender = snd_seq_client_use_ptr(subs.sender.client)) == NULL)
+	sender = snd_seq_client_use_ptr(subs->sender.client);
+	if (sender == NULL)
 		goto __end;
-	if ((sport = snd_seq_port_use_ptr(sender, subs.sender.port)) == NULL)
+	sport = snd_seq_port_use_ptr(sender, subs->sender.port);
+	if (sport == NULL)
 		goto __end;
-	p = snd_seq_port_get_subscription(&sport->c_src, &subs.dest);
+	p = snd_seq_port_get_subscription(&sport->c_src, &subs->dest);
 	if (p) {
 		result = 0;
-		subs = p->info;
+		*subs = p->info;
 	} else
 		result = -ENOENT;
 
@@ -1917,10 +1915,7 @@ static int seq_ioctl_get_subscription(struct snd_seq_client *client,
 		snd_seq_port_unlock(sport);
 	if (sender)
 		snd_seq_client_unlock(sender);
-	if (result >= 0) {
-		if (copy_to_user(arg, &subs, sizeof(subs)))
-			return -EFAULT;
-	}
+
 	return result;
 }
 
-- 
2.7.4

^ permalink raw reply related	[flat|nested] 63+ messages in thread

* [PATCH 37/39] ALSA: seq: optimize query_subs function to new design
  2016-08-07  9:48 [PATCH 00/39] seq: obsolete change of address limit Takashi Sakamoto
                   ` (35 preceding siblings ...)
  2016-08-07  9:49 ` [PATCH 36/39] ALSA: seq: optimize get_subscription " Takashi Sakamoto
@ 2016-08-07  9:49 ` Takashi Sakamoto
  2016-08-07  9:49 ` [PATCH 38/39] ALSA: seq: optimize query_next_client " Takashi Sakamoto
  2016-08-07  9:49 ` [PATCH 39/39] ALSA: seq: optimize query_next_port " Takashi Sakamoto
  38 siblings, 0 replies; 63+ messages in thread
From: Takashi Sakamoto @ 2016-08-07  9:49 UTC (permalink / raw)
  To: clemens, tiwai; +Cc: alsa-devel

In former commit, actual operations of each ioctl command get argument
in kernel space. Copying from/to user space is performed outside of
the function.

This commit optimizes to the new design.

Signed-off-by: Takashi Sakamoto <o-takashi@sakamocchi.jp>
---
 sound/core/seq/seq_clientmgr.c | 34 +++++++++++++++-------------------
 1 file changed, 15 insertions(+), 19 deletions(-)

diff --git a/sound/core/seq/seq_clientmgr.c b/sound/core/seq/seq_clientmgr.c
index cd4073d..e4a9754 100644
--- a/sound/core/seq/seq_clientmgr.c
+++ b/sound/core/seq/seq_clientmgr.c
@@ -1923,25 +1923,24 @@ static int seq_ioctl_get_subscription(struct snd_seq_client *client, void *arg)
 /*
  * get subscription info - check only its presence
  */
-static int seq_ioctl_query_subs(struct snd_seq_client *client, void __user *arg)
+static int seq_ioctl_query_subs(struct snd_seq_client *client, void *arg)
 {
+	struct snd_seq_query_subs *subs = arg;
 	int result = -ENXIO;
 	struct snd_seq_client *cptr = NULL;
 	struct snd_seq_client_port *port = NULL;
-	struct snd_seq_query_subs subs;
 	struct snd_seq_port_subs_info *group;
 	struct list_head *p;
 	int i;
 
-	if (copy_from_user(&subs, arg, sizeof(subs)))
-		return -EFAULT;
-
-	if ((cptr = snd_seq_client_use_ptr(subs.root.client)) == NULL)
+	cptr = snd_seq_client_use_ptr(subs->root.client);
+	if (cptr == NULL)
 		goto __end;
-	if ((port = snd_seq_port_use_ptr(cptr, subs.root.port)) == NULL)
+	port = snd_seq_port_use_ptr(cptr, subs->root.port);
+	if (cptr == NULL)
 		goto __end;
 
-	switch (subs.type) {
+	switch (subs->type) {
 	case SNDRV_SEQ_QUERY_SUBS_READ:
 		group = &port->c_src;
 		break;
@@ -1954,22 +1953,22 @@ static int seq_ioctl_query_subs(struct snd_seq_client *client, void __user *arg)
 
 	down_read(&group->list_mutex);
 	/* search for the subscriber */
-	subs.num_subs = group->count;
+	subs->num_subs = group->count;
 	i = 0;
 	result = -ENOENT;
 	list_for_each(p, &group->list_head) {
-		if (i++ == subs.index) {
+		if (i++ == subs->index) {
 			/* found! */
 			struct snd_seq_subscribers *s;
-			if (subs.type == SNDRV_SEQ_QUERY_SUBS_READ) {
+			if (subs->type == SNDRV_SEQ_QUERY_SUBS_READ) {
 				s = list_entry(p, struct snd_seq_subscribers, src_list);
-				subs.addr = s->info.dest;
+				subs->addr = s->info.dest;
 			} else {
 				s = list_entry(p, struct snd_seq_subscribers, dest_list);
-				subs.addr = s->info.sender;
+				subs->addr = s->info.sender;
 			}
-			subs.flags = s->info.flags;
-			subs.queue = s->info.queue;
+			subs->flags = s->info.flags;
+			subs->queue = s->info.queue;
 			result = 0;
 			break;
 		}
@@ -1981,10 +1980,7 @@ static int seq_ioctl_query_subs(struct snd_seq_client *client, void __user *arg)
 		snd_seq_port_unlock(port);
 	if (cptr)
 		snd_seq_client_unlock(cptr);
-	if (result >= 0) {
-		if (copy_to_user(arg, &subs, sizeof(subs)))
-			return -EFAULT;
-	}
+
 	return result;
 }
 
-- 
2.7.4

^ permalink raw reply related	[flat|nested] 63+ messages in thread

* [PATCH 38/39] ALSA: seq: optimize query_next_client function to new design
  2016-08-07  9:48 [PATCH 00/39] seq: obsolete change of address limit Takashi Sakamoto
                   ` (36 preceding siblings ...)
  2016-08-07  9:49 ` [PATCH 37/39] ALSA: seq: optimize query_subs " Takashi Sakamoto
@ 2016-08-07  9:49 ` Takashi Sakamoto
  2016-08-07  9:49 ` [PATCH 39/39] ALSA: seq: optimize query_next_port " Takashi Sakamoto
  38 siblings, 0 replies; 63+ messages in thread
From: Takashi Sakamoto @ 2016-08-07  9:49 UTC (permalink / raw)
  To: clemens, tiwai; +Cc: alsa-devel

In former commit, actual operations of each ioctl command get argument
in kernel space. Copying from/to user space is performed outside of
the function.

This commit optimizes to the new design.

Signed-off-by: Takashi Sakamoto <o-takashi@sakamocchi.jp>
---
 sound/core/seq/seq_clientmgr.c | 22 ++++++++--------------
 1 file changed, 8 insertions(+), 14 deletions(-)

diff --git a/sound/core/seq/seq_clientmgr.c b/sound/core/seq/seq_clientmgr.c
index e4a9754..0f840ca 100644
--- a/sound/core/seq/seq_clientmgr.c
+++ b/sound/core/seq/seq_clientmgr.c
@@ -1988,32 +1988,26 @@ static int seq_ioctl_query_subs(struct snd_seq_client *client, void *arg)
 /*
  * query next client
  */
-static int seq_ioctl_query_next_client(struct snd_seq_client *client,
-				       void __user *arg)
+static int seq_ioctl_query_next_client(struct snd_seq_client *client, void *arg)
 {
+	struct snd_seq_client_info *info = arg;
 	struct snd_seq_client *cptr = NULL;
-	struct snd_seq_client_info info;
-
-	if (copy_from_user(&info, arg, sizeof(info)))
-		return -EFAULT;
 
 	/* search for next client */
-	info.client++;
-	if (info.client < 0)
-		info.client = 0;
-	for (; info.client < SNDRV_SEQ_MAX_CLIENTS; info.client++) {
-		cptr = snd_seq_client_use_ptr(info.client);
+	info->client++;
+	if (info->client < 0)
+		info->client = 0;
+	for (; info->client < SNDRV_SEQ_MAX_CLIENTS; info->client++) {
+		cptr = snd_seq_client_use_ptr(info->client);
 		if (cptr)
 			break; /* found */
 	}
 	if (cptr == NULL)
 		return -ENOENT;
 
-	get_client_info(cptr, &info);
+	get_client_info(cptr, info);
 	snd_seq_client_unlock(cptr);
 
-	if (copy_to_user(arg, &info, sizeof(info)))
-		return -EFAULT;
 	return 0;
 }
 
-- 
2.7.4

^ permalink raw reply related	[flat|nested] 63+ messages in thread

* [PATCH 39/39] ALSA: seq: optimize query_next_port function to new design
  2016-08-07  9:48 [PATCH 00/39] seq: obsolete change of address limit Takashi Sakamoto
                   ` (37 preceding siblings ...)
  2016-08-07  9:49 ` [PATCH 38/39] ALSA: seq: optimize query_next_client " Takashi Sakamoto
@ 2016-08-07  9:49 ` Takashi Sakamoto
  38 siblings, 0 replies; 63+ messages in thread
From: Takashi Sakamoto @ 2016-08-07  9:49 UTC (permalink / raw)
  To: clemens, tiwai; +Cc: alsa-devel

In former commit, actual operations of each ioctl command get argument
in kernel space. Copying from/to user space is performed outside of
the function.

This commit optimizes to the new design.

Signed-off-by: Takashi Sakamoto <o-takashi@sakamocchi.jp>
---
 sound/core/seq/seq_clientmgr.c | 19 +++++++------------
 1 file changed, 7 insertions(+), 12 deletions(-)

diff --git a/sound/core/seq/seq_clientmgr.c b/sound/core/seq/seq_clientmgr.c
index 0f840ca..6bc29b5 100644
--- a/sound/core/seq/seq_clientmgr.c
+++ b/sound/core/seq/seq_clientmgr.c
@@ -2014,35 +2014,30 @@ static int seq_ioctl_query_next_client(struct snd_seq_client *client, void *arg)
 /* 
  * query next port
  */
-static int seq_ioctl_query_next_port(struct snd_seq_client *client,
-				     void __user *arg)
+static int seq_ioctl_query_next_port(struct snd_seq_client *client, void *arg)
 {
+	struct snd_seq_port_info *info = arg;
 	struct snd_seq_client *cptr;
 	struct snd_seq_client_port *port = NULL;
-	struct snd_seq_port_info info;
 
-	if (copy_from_user(&info, arg, sizeof(info)))
-		return -EFAULT;
-	cptr = snd_seq_client_use_ptr(info.addr.client);
+	cptr = snd_seq_client_use_ptr(info->addr.client);
 	if (cptr == NULL)
 		return -ENXIO;
 
 	/* search for next port */
-	info.addr.port++;
-	port = snd_seq_port_query_nearest(cptr, &info);
+	info->addr.port++;
+	port = snd_seq_port_query_nearest(cptr, info);
 	if (port == NULL) {
 		snd_seq_client_unlock(cptr);
 		return -ENOENT;
 	}
 
 	/* get port info */
-	info.addr = port->addr;
-	snd_seq_get_port_info(port, &info);
+	info->addr = port->addr;
+	snd_seq_get_port_info(port, info);
 	snd_seq_port_unlock(port);
 	snd_seq_client_unlock(cptr);
 
-	if (copy_to_user(arg, &info, sizeof(info)))
-		return -EFAULT;
 	return 0;
 }
 
-- 
2.7.4

^ permalink raw reply related	[flat|nested] 63+ messages in thread

* Re: [PATCH 04/39] ALSA: seq: copy ioctl data from user space to kernel stack
  2016-08-07  9:48 ` [PATCH 04/39] ALSA: seq: copy ioctl data from user space to kernel stack Takashi Sakamoto
@ 2016-08-07 10:15   ` Clemens Ladisch
  2016-08-07 14:26     ` Takashi Sakamoto
  0 siblings, 1 reply; 63+ messages in thread
From: Clemens Ladisch @ 2016-08-07 10:15 UTC (permalink / raw)
  To: Takashi Sakamoto, tiwai; +Cc: alsa-devel

Takashi Sakamoto wrote:
> Data from user space is once copied to kernel stack, then operated and
> copied to user space, in a consistent manner. This manner forces all ioctl
> operations to copy the data from/to user space, even if it's read-only or
> write-only. Thus, it has an overhead for simpler ioctl commands.

The ioctl code itself already contains information about the direction
and size of the data to be copied (and in theory, these values are
correct).  See dispatch_ioctl() in drivers/firewire/core-cdev.c for an
example.


Regards,
Clemens

^ permalink raw reply	[flat|nested] 63+ messages in thread

* Re: [PATCH 04/39] ALSA: seq: copy ioctl data from user space to kernel stack
  2016-08-07 10:15   ` Clemens Ladisch
@ 2016-08-07 14:26     ` Takashi Sakamoto
  2016-08-08  7:10       ` Takashi Iwai
  0 siblings, 1 reply; 63+ messages in thread
From: Takashi Sakamoto @ 2016-08-07 14:26 UTC (permalink / raw)
  To: Clemens Ladisch, tiwai; +Cc: alsa-devel

Hi Clemens,

On Aug 7 2016 19:15, Clemens Ladisch wrote:
> Takashi Sakamoto wrote:
>> Data from user space is once copied to kernel stack, then operated and
>> copied to user space, in a consistent manner. This manner forces all ioctl
>> operations to copy the data from/to user space, even if it's read-only or
>> write-only. Thus, it has an overhead for simpler ioctl commands.
> 
> The ioctl code itself already contains information about the direction
> and size of the data to be copied (and in theory, these values are
> correct).  See dispatch_ioctl() in drivers/firewire/core-cdev.c for an
> example.

A nice idea.

_IOC_SIZE macro pick up 13 or 14 bits (architecture-dependent) in ioctl
command, which represents the size of argument. In my patch, the size of
'union ioctl_arg' is 188 (x86/x32) or 192 (x86_64) and there's enough
rest of the size field. So we can pick up the size from ioctl command by
the macro because the size represents the maximum bytes of argument for
all of sequencer ioctls.

I'll post revised version tomorrow. Thanks ;)


Takashi Sakamoto

^ permalink raw reply	[flat|nested] 63+ messages in thread

* Re: [PATCH 02/39] ALSA: seq: apply shorter name for file local functions
  2016-08-07  9:48 ` [PATCH 02/39] ALSA: seq: apply shorter name for file local functions Takashi Sakamoto
@ 2016-08-08  6:56   ` Takashi Iwai
  0 siblings, 0 replies; 63+ messages in thread
From: Takashi Iwai @ 2016-08-08  6:56 UTC (permalink / raw)
  To: Takashi Sakamoto; +Cc: alsa-devel, clemens

On Sun, 07 Aug 2016 11:48:38 +0200,
Takashi Sakamoto wrote:
> 
> Exported symbols should have 'snd_' prefix to avoid name conflict, while
> file local symbols have no need to have. Shorter names are generally
> preferable due to '80 characters in a line' rule.

I don't think such renames give much benefit unless it dramatically
improves the readability.


thanks,

Takashi

> 
> This commit renames some local functions.
> 
> Signed-off-by: Takashi Sakamoto <o-takashi@sakamocchi.jp>
> ---
>  sound/core/seq/seq_clientmgr.c | 180 +++++++++++++++++++++--------------------
>  sound/core/seq/seq_compat.c    |  19 ++---
>  2 files changed, 101 insertions(+), 98 deletions(-)
> 
> diff --git a/sound/core/seq/seq_clientmgr.c b/sound/core/seq/seq_clientmgr.c
> index 42be1e5..1331103 100644
> --- a/sound/core/seq/seq_clientmgr.c
> +++ b/sound/core/seq/seq_clientmgr.c
> @@ -1130,7 +1130,8 @@ static unsigned int snd_seq_poll(struct file *file, poll_table * wait)
>  
>  
>  /* SYSTEM_INFO ioctl() */
> -static int snd_seq_ioctl_system_info(struct snd_seq_client *client, void __user *arg)
> +static int seq_ioctl_system_info(struct snd_seq_client *client,
> +				 void __user *arg)
>  {
>  	struct snd_seq_system_info info;
>  
> @@ -1150,7 +1151,8 @@ static int snd_seq_ioctl_system_info(struct snd_seq_client *client, void __user
>  
>  
>  /* RUNNING_MODE ioctl() */
> -static int snd_seq_ioctl_running_mode(struct snd_seq_client *client, void __user *arg)
> +static int seq_ioctl_running_mode(struct snd_seq_client *client,
> +				  void __user *arg)
>  {
>  	struct snd_seq_running_info info;
>  	struct snd_seq_client *cptr;
> @@ -1213,8 +1215,8 @@ static void get_client_info(struct snd_seq_client *cptr,
>  	memset(info->reserved, 0, sizeof(info->reserved));
>  }
>  
> -static int snd_seq_ioctl_get_client_info(struct snd_seq_client *client,
> -					 void __user *arg)
> +static int seq_ioctl_get_client_info(struct snd_seq_client *client,
> +				     void __user *arg)
>  {
>  	struct snd_seq_client *cptr;
>  	struct snd_seq_client_info client_info;
> @@ -1237,8 +1239,8 @@ static int snd_seq_ioctl_get_client_info(struct snd_seq_client *client,
>  
>  
>  /* CLIENT_INFO ioctl() */
> -static int snd_seq_ioctl_set_client_info(struct snd_seq_client *client,
> -					 void __user *arg)
> +static int seq_ioctl_set_client_info(struct snd_seq_client *client,
> +				     void __user *arg)
>  {
>  	struct snd_seq_client_info client_info;
>  
> @@ -1267,8 +1269,8 @@ static int snd_seq_ioctl_set_client_info(struct snd_seq_client *client,
>  /* 
>   * CREATE PORT ioctl() 
>   */
> -static int snd_seq_ioctl_create_port(struct snd_seq_client *client,
> -				     void __user *arg)
> +static int seq_ioctl_create_port(struct snd_seq_client *client,
> +				 void __user *arg)
>  {
>  	struct snd_seq_client_port *port;
>  	struct snd_seq_port_info info;
> @@ -1317,8 +1319,8 @@ static int snd_seq_ioctl_create_port(struct snd_seq_client *client,
>  /* 
>   * DELETE PORT ioctl() 
>   */
> -static int snd_seq_ioctl_delete_port(struct snd_seq_client *client,
> -				     void __user *arg)
> +static int seq_ioctl_delete_port(struct snd_seq_client *client,
> +				 void __user *arg)
>  {
>  	struct snd_seq_port_info info;
>  	int err;
> @@ -1341,8 +1343,8 @@ static int snd_seq_ioctl_delete_port(struct snd_seq_client *client,
>  /* 
>   * GET_PORT_INFO ioctl() (on any client) 
>   */
> -static int snd_seq_ioctl_get_port_info(struct snd_seq_client *client,
> -				       void __user *arg)
> +static int seq_ioctl_get_port_info(struct snd_seq_client *client,
> +				   void __user *arg)
>  {
>  	struct snd_seq_client *cptr;
>  	struct snd_seq_client_port *port;
> @@ -1374,8 +1376,8 @@ static int snd_seq_ioctl_get_port_info(struct snd_seq_client *client,
>  /* 
>   * SET_PORT_INFO ioctl() (only ports on this/own client) 
>   */
> -static int snd_seq_ioctl_set_port_info(struct snd_seq_client *client,
> -				       void __user *arg)
> +static int seq_ioctl_set_port_info(struct snd_seq_client *client,
> +				   void __user *arg)
>  {
>  	struct snd_seq_client_port *port;
>  	struct snd_seq_port_info info;
> @@ -1452,8 +1454,8 @@ int snd_seq_client_notify_subscription(int client, int port,
>  /* 
>   * add to port's subscription list IOCTL interface 
>   */
> -static int snd_seq_ioctl_subscribe_port(struct snd_seq_client *client,
> -					void __user *arg)
> +static int seq_ioctl_subscribe_port(struct snd_seq_client *client,
> +				    void __user *arg)
>  {
>  	int result = -EINVAL;
>  	struct snd_seq_client *receiver = NULL, *sender = NULL;
> @@ -1497,8 +1499,8 @@ static int snd_seq_ioctl_subscribe_port(struct snd_seq_client *client,
>  /* 
>   * remove from port's subscription list 
>   */
> -static int snd_seq_ioctl_unsubscribe_port(struct snd_seq_client *client,
> -					  void __user *arg)
> +static int seq_ioctl_unsubscribe_port(struct snd_seq_client *client,
> +				      void __user *arg)
>  {
>  	int result = -ENXIO;
>  	struct snd_seq_client *receiver = NULL, *sender = NULL;
> @@ -1539,8 +1541,8 @@ static int snd_seq_ioctl_unsubscribe_port(struct snd_seq_client *client,
>  
>  
>  /* CREATE_QUEUE ioctl() */
> -static int snd_seq_ioctl_create_queue(struct snd_seq_client *client,
> -				      void __user *arg)
> +static int seq_ioctl_create_queue(struct snd_seq_client *client,
> +				  void __user *arg)
>  {
>  	struct snd_seq_queue_info info;
>  	int result;
> @@ -1574,8 +1576,8 @@ static int snd_seq_ioctl_create_queue(struct snd_seq_client *client,
>  }
>  
>  /* DELETE_QUEUE ioctl() */
> -static int snd_seq_ioctl_delete_queue(struct snd_seq_client *client,
> -				      void __user *arg)
> +static int seq_ioctl_delete_queue(struct snd_seq_client *client,
> +				  void __user *arg)
>  {
>  	struct snd_seq_queue_info info;
>  
> @@ -1586,8 +1588,8 @@ static int snd_seq_ioctl_delete_queue(struct snd_seq_client *client,
>  }
>  
>  /* GET_QUEUE_INFO ioctl() */
> -static int snd_seq_ioctl_get_queue_info(struct snd_seq_client *client,
> -					void __user *arg)
> +static int seq_ioctl_get_queue_info(struct snd_seq_client *client,
> +				    void __user *arg)
>  {
>  	struct snd_seq_queue_info info;
>  	struct snd_seq_queue *q;
> @@ -1613,8 +1615,8 @@ static int snd_seq_ioctl_get_queue_info(struct snd_seq_client *client,
>  }
>  
>  /* SET_QUEUE_INFO ioctl() */
> -static int snd_seq_ioctl_set_queue_info(struct snd_seq_client *client,
> -					void __user *arg)
> +static int seq_ioctl_set_queue_info(struct snd_seq_client *client,
> +				    void __user *arg)
>  {
>  	struct snd_seq_queue_info info;
>  	struct snd_seq_queue *q;
> @@ -1649,7 +1651,8 @@ static int snd_seq_ioctl_set_queue_info(struct snd_seq_client *client,
>  }
>  
>  /* GET_NAMED_QUEUE ioctl() */
> -static int snd_seq_ioctl_get_named_queue(struct snd_seq_client *client, void __user *arg)
> +static int seq_ioctl_get_named_queue(struct snd_seq_client *client,
> +				     void __user *arg)
>  {
>  	struct snd_seq_queue_info info;
>  	struct snd_seq_queue *q;
> @@ -1672,8 +1675,8 @@ static int snd_seq_ioctl_get_named_queue(struct snd_seq_client *client, void __u
>  }
>  
>  /* GET_QUEUE_STATUS ioctl() */
> -static int snd_seq_ioctl_get_queue_status(struct snd_seq_client *client,
> -					  void __user *arg)
> +static int seq_ioctl_get_queue_status(struct snd_seq_client *client,
> +				      void __user *arg)
>  {
>  	struct snd_seq_queue_status status;
>  	struct snd_seq_queue *queue;
> @@ -1706,8 +1709,8 @@ static int snd_seq_ioctl_get_queue_status(struct snd_seq_client *client,
>  
>  
>  /* GET_QUEUE_TEMPO ioctl() */
> -static int snd_seq_ioctl_get_queue_tempo(struct snd_seq_client *client,
> -					 void __user *arg)
> +static int seq_ioctl_get_queue_tempo(struct snd_seq_client *client,
> +				     void __user *arg)
>  {
>  	struct snd_seq_queue_tempo tempo;
>  	struct snd_seq_queue *queue;
> @@ -1746,8 +1749,8 @@ int snd_seq_set_queue_tempo(int client, struct snd_seq_queue_tempo *tempo)
>  
>  EXPORT_SYMBOL(snd_seq_set_queue_tempo);
>  
> -static int snd_seq_ioctl_set_queue_tempo(struct snd_seq_client *client,
> -					 void __user *arg)
> +static int seq_ioctl_set_queue_tempo(struct snd_seq_client *client,
> +				     void __user *arg)
>  {
>  	int result;
>  	struct snd_seq_queue_tempo tempo;
> @@ -1761,8 +1764,8 @@ static int snd_seq_ioctl_set_queue_tempo(struct snd_seq_client *client,
>  
>  
>  /* GET_QUEUE_TIMER ioctl() */
> -static int snd_seq_ioctl_get_queue_timer(struct snd_seq_client *client,
> -					 void __user *arg)
> +static int seq_ioctl_get_queue_timer(struct snd_seq_client *client,
> +				     void __user *arg)
>  {
>  	struct snd_seq_queue_timer timer;
>  	struct snd_seq_queue *queue;
> @@ -1798,8 +1801,8 @@ static int snd_seq_ioctl_get_queue_timer(struct snd_seq_client *client,
>  
>  
>  /* SET_QUEUE_TIMER ioctl() */
> -static int snd_seq_ioctl_set_queue_timer(struct snd_seq_client *client,
> -					 void __user *arg)
> +static int seq_ioctl_set_queue_timer(struct snd_seq_client *client,
> +				     void __user *arg)
>  {
>  	int result = 0;
>  	struct snd_seq_queue_timer timer;
> @@ -1840,8 +1843,8 @@ static int snd_seq_ioctl_set_queue_timer(struct snd_seq_client *client,
>  
>  
>  /* GET_QUEUE_CLIENT ioctl() */
> -static int snd_seq_ioctl_get_queue_client(struct snd_seq_client *client,
> -					  void __user *arg)
> +static int seq_ioctl_get_queue_client(struct snd_seq_client *client,
> +				      void __user *arg)
>  {
>  	struct snd_seq_queue_client info;
>  	int used;
> @@ -1862,8 +1865,8 @@ static int snd_seq_ioctl_get_queue_client(struct snd_seq_client *client,
>  
>  
>  /* SET_QUEUE_CLIENT ioctl() */
> -static int snd_seq_ioctl_set_queue_client(struct snd_seq_client *client,
> -					  void __user *arg)
> +static int seq_ioctl_set_queue_client(struct snd_seq_client *client,
> +				      void __user *arg)
>  {
>  	int err;
>  	struct snd_seq_queue_client info;
> @@ -1877,13 +1880,13 @@ static int snd_seq_ioctl_set_queue_client(struct snd_seq_client *client,
>  			return err;
>  	}
>  
> -	return snd_seq_ioctl_get_queue_client(client, arg);
> +	return seq_ioctl_get_queue_client(client, arg);
>  }
>  
>  
>  /* GET_CLIENT_POOL ioctl() */
> -static int snd_seq_ioctl_get_client_pool(struct snd_seq_client *client,
> -					 void __user *arg)
> +static int seq_ioctl_get_client_pool(struct snd_seq_client *client,
> +				     void __user *arg)
>  {
>  	struct snd_seq_client_pool info;
>  	struct snd_seq_client *cptr;
> @@ -1917,8 +1920,8 @@ static int snd_seq_ioctl_get_client_pool(struct snd_seq_client *client,
>  }
>  
>  /* SET_CLIENT_POOL ioctl() */
> -static int snd_seq_ioctl_set_client_pool(struct snd_seq_client *client,
> -					 void __user *arg)
> +static int seq_ioctl_set_client_pool(struct snd_seq_client *client,
> +				     void __user *arg)
>  {
>  	struct snd_seq_client_pool info;
>  	int rc;
> @@ -1957,13 +1960,13 @@ static int snd_seq_ioctl_set_client_pool(struct snd_seq_client *client,
>  		client->pool->room  = info.output_room;
>  	}
>  
> -	return snd_seq_ioctl_get_client_pool(client, arg);
> +	return seq_ioctl_get_client_pool(client, arg);
>  }
>  
>  
>  /* REMOVE_EVENTS ioctl() */
> -static int snd_seq_ioctl_remove_events(struct snd_seq_client *client,
> -				       void __user *arg)
> +static int seq_ioctl_remove_events(struct snd_seq_client *client,
> +				   void __user *arg)
>  {
>  	struct snd_seq_remove_events info;
>  
> @@ -1992,8 +1995,8 @@ static int snd_seq_ioctl_remove_events(struct snd_seq_client *client,
>  /*
>   * get subscription info
>   */
> -static int snd_seq_ioctl_get_subscription(struct snd_seq_client *client,
> -					  void __user *arg)
> +static int seq_ioctl_get_subscription(struct snd_seq_client *client,
> +				      void __user *arg)
>  {
>  	int result;
>  	struct snd_seq_client *sender = NULL;
> @@ -2032,8 +2035,7 @@ static int snd_seq_ioctl_get_subscription(struct snd_seq_client *client,
>  /*
>   * get subscription info - check only its presence
>   */
> -static int snd_seq_ioctl_query_subs(struct snd_seq_client *client,
> -				    void __user *arg)
> +static int seq_ioctl_query_subs(struct snd_seq_client *client, void __user *arg)
>  {
>  	int result = -ENXIO;
>  	struct snd_seq_client *cptr = NULL;
> @@ -2102,8 +2104,8 @@ static int snd_seq_ioctl_query_subs(struct snd_seq_client *client,
>  /*
>   * query next client
>   */
> -static int snd_seq_ioctl_query_next_client(struct snd_seq_client *client,
> -					   void __user *arg)
> +static int seq_ioctl_query_next_client(struct snd_seq_client *client,
> +				       void __user *arg)
>  {
>  	struct snd_seq_client *cptr = NULL;
>  	struct snd_seq_client_info info;
> @@ -2134,8 +2136,8 @@ static int snd_seq_ioctl_query_next_client(struct snd_seq_client *client,
>  /* 
>   * query next port
>   */
> -static int snd_seq_ioctl_query_next_port(struct snd_seq_client *client,
> -					 void __user *arg)
> +static int seq_ioctl_query_next_port(struct snd_seq_client *client,
> +				     void __user *arg)
>  {
>  	struct snd_seq_client *cptr;
>  	struct snd_seq_client_port *port = NULL;
> @@ -2172,39 +2174,39 @@ static const struct seq_ioctl_table {
>  	unsigned int cmd;
>  	int (*func)(struct snd_seq_client *client, void __user * arg);
>  } ioctl_tables[] = {
> -	{ SNDRV_SEQ_IOCTL_SYSTEM_INFO, snd_seq_ioctl_system_info },
> -	{ SNDRV_SEQ_IOCTL_RUNNING_MODE, snd_seq_ioctl_running_mode },
> -	{ SNDRV_SEQ_IOCTL_GET_CLIENT_INFO, snd_seq_ioctl_get_client_info },
> -	{ SNDRV_SEQ_IOCTL_SET_CLIENT_INFO, snd_seq_ioctl_set_client_info },
> -	{ SNDRV_SEQ_IOCTL_CREATE_PORT, snd_seq_ioctl_create_port },
> -	{ SNDRV_SEQ_IOCTL_DELETE_PORT, snd_seq_ioctl_delete_port },
> -	{ SNDRV_SEQ_IOCTL_GET_PORT_INFO, snd_seq_ioctl_get_port_info },
> -	{ SNDRV_SEQ_IOCTL_SET_PORT_INFO, snd_seq_ioctl_set_port_info },
> -	{ SNDRV_SEQ_IOCTL_SUBSCRIBE_PORT, snd_seq_ioctl_subscribe_port },
> -	{ SNDRV_SEQ_IOCTL_UNSUBSCRIBE_PORT, snd_seq_ioctl_unsubscribe_port },
> -	{ SNDRV_SEQ_IOCTL_CREATE_QUEUE, snd_seq_ioctl_create_queue },
> -	{ SNDRV_SEQ_IOCTL_DELETE_QUEUE, snd_seq_ioctl_delete_queue },
> -	{ SNDRV_SEQ_IOCTL_GET_QUEUE_INFO, snd_seq_ioctl_get_queue_info },
> -	{ SNDRV_SEQ_IOCTL_SET_QUEUE_INFO, snd_seq_ioctl_set_queue_info },
> -	{ SNDRV_SEQ_IOCTL_GET_NAMED_QUEUE, snd_seq_ioctl_get_named_queue },
> -	{ SNDRV_SEQ_IOCTL_GET_QUEUE_STATUS, snd_seq_ioctl_get_queue_status },
> -	{ SNDRV_SEQ_IOCTL_GET_QUEUE_TEMPO, snd_seq_ioctl_get_queue_tempo },
> -	{ SNDRV_SEQ_IOCTL_SET_QUEUE_TEMPO, snd_seq_ioctl_set_queue_tempo },
> -	{ SNDRV_SEQ_IOCTL_GET_QUEUE_TIMER, snd_seq_ioctl_get_queue_timer },
> -	{ SNDRV_SEQ_IOCTL_SET_QUEUE_TIMER, snd_seq_ioctl_set_queue_timer },
> -	{ SNDRV_SEQ_IOCTL_GET_QUEUE_CLIENT, snd_seq_ioctl_get_queue_client },
> -	{ SNDRV_SEQ_IOCTL_SET_QUEUE_CLIENT, snd_seq_ioctl_set_queue_client },
> -	{ SNDRV_SEQ_IOCTL_GET_CLIENT_POOL, snd_seq_ioctl_get_client_pool },
> -	{ SNDRV_SEQ_IOCTL_SET_CLIENT_POOL, snd_seq_ioctl_set_client_pool },
> -	{ SNDRV_SEQ_IOCTL_GET_SUBSCRIPTION, snd_seq_ioctl_get_subscription },
> -	{ SNDRV_SEQ_IOCTL_QUERY_NEXT_CLIENT, snd_seq_ioctl_query_next_client },
> -	{ SNDRV_SEQ_IOCTL_QUERY_NEXT_PORT, snd_seq_ioctl_query_next_port },
> -	{ SNDRV_SEQ_IOCTL_REMOVE_EVENTS, snd_seq_ioctl_remove_events },
> -	{ SNDRV_SEQ_IOCTL_QUERY_SUBS, snd_seq_ioctl_query_subs },
> +	{ SNDRV_SEQ_IOCTL_SYSTEM_INFO, seq_ioctl_system_info },
> +	{ SNDRV_SEQ_IOCTL_RUNNING_MODE, seq_ioctl_running_mode },
> +	{ SNDRV_SEQ_IOCTL_GET_CLIENT_INFO, seq_ioctl_get_client_info },
> +	{ SNDRV_SEQ_IOCTL_SET_CLIENT_INFO, seq_ioctl_set_client_info },
> +	{ SNDRV_SEQ_IOCTL_CREATE_PORT, seq_ioctl_create_port },
> +	{ SNDRV_SEQ_IOCTL_DELETE_PORT, seq_ioctl_delete_port },
> +	{ SNDRV_SEQ_IOCTL_GET_PORT_INFO, seq_ioctl_get_port_info },
> +	{ SNDRV_SEQ_IOCTL_SET_PORT_INFO, seq_ioctl_set_port_info },
> +	{ SNDRV_SEQ_IOCTL_SUBSCRIBE_PORT, seq_ioctl_subscribe_port },
> +	{ SNDRV_SEQ_IOCTL_UNSUBSCRIBE_PORT, seq_ioctl_unsubscribe_port },
> +	{ SNDRV_SEQ_IOCTL_CREATE_QUEUE, seq_ioctl_create_queue },
> +	{ SNDRV_SEQ_IOCTL_DELETE_QUEUE, seq_ioctl_delete_queue },
> +	{ SNDRV_SEQ_IOCTL_GET_QUEUE_INFO, seq_ioctl_get_queue_info },
> +	{ SNDRV_SEQ_IOCTL_SET_QUEUE_INFO, seq_ioctl_set_queue_info },
> +	{ SNDRV_SEQ_IOCTL_GET_NAMED_QUEUE, seq_ioctl_get_named_queue },
> +	{ SNDRV_SEQ_IOCTL_GET_QUEUE_STATUS, seq_ioctl_get_queue_status },
> +	{ SNDRV_SEQ_IOCTL_GET_QUEUE_TEMPO, seq_ioctl_get_queue_tempo },
> +	{ SNDRV_SEQ_IOCTL_SET_QUEUE_TEMPO, seq_ioctl_set_queue_tempo },
> +	{ SNDRV_SEQ_IOCTL_GET_QUEUE_TIMER, seq_ioctl_get_queue_timer },
> +	{ SNDRV_SEQ_IOCTL_SET_QUEUE_TIMER, seq_ioctl_set_queue_timer },
> +	{ SNDRV_SEQ_IOCTL_GET_QUEUE_CLIENT, seq_ioctl_get_queue_client },
> +	{ SNDRV_SEQ_IOCTL_SET_QUEUE_CLIENT, seq_ioctl_set_queue_client },
> +	{ SNDRV_SEQ_IOCTL_GET_CLIENT_POOL, seq_ioctl_get_client_pool },
> +	{ SNDRV_SEQ_IOCTL_SET_CLIENT_POOL, seq_ioctl_set_client_pool },
> +	{ SNDRV_SEQ_IOCTL_GET_SUBSCRIPTION, seq_ioctl_get_subscription },
> +	{ SNDRV_SEQ_IOCTL_QUERY_NEXT_CLIENT, seq_ioctl_query_next_client },
> +	{ SNDRV_SEQ_IOCTL_QUERY_NEXT_PORT, seq_ioctl_query_next_port },
> +	{ SNDRV_SEQ_IOCTL_REMOVE_EVENTS, seq_ioctl_remove_events },
> +	{ SNDRV_SEQ_IOCTL_QUERY_SUBS, seq_ioctl_query_subs },
>  	{ 0, NULL },
>  };
>  
> -static int snd_seq_do_ioctl(struct snd_seq_client *client, unsigned int cmd,
> +static int seq_do_ioctl(struct snd_seq_client *client, unsigned int cmd,
>  			    void __user *arg)
>  {
>  	const struct seq_ioctl_table *p;
> @@ -2237,7 +2239,7 @@ static long snd_seq_ioctl(struct file *file, unsigned int cmd, unsigned long arg
>  	if (snd_BUG_ON(!client))
>  		return -ENXIO;
>  		
> -	return snd_seq_do_ioctl(client, cmd, (void __user *) arg);
> +	return seq_do_ioctl(client, cmd, (void __user *) arg);
>  }
>  
>  #ifdef CONFIG_COMPAT
> @@ -2437,7 +2439,7 @@ int snd_seq_kernel_client_ctl(int clientid, unsigned int cmd, void *arg)
>  	if (client == NULL)
>  		return -ENXIO;
>  	fs = snd_enter_user();
> -	result = snd_seq_do_ioctl(client, cmd, (void __force __user *)arg);
> +	result = seq_do_ioctl(client, cmd, (void __force __user *)arg);
>  	snd_leave_user(fs);
>  	return result;
>  }
> diff --git a/sound/core/seq/seq_compat.c b/sound/core/seq/seq_compat.c
> index 6517590..70d3ddb 100644
> --- a/sound/core/seq/seq_compat.c
> +++ b/sound/core/seq/seq_compat.c
> @@ -42,8 +42,9 @@ struct snd_seq_port_info32 {
>  	char reserved[59];		/* for future use */
>  };
>  
> -static int snd_seq_call_port_info_ioctl(struct snd_seq_client *client, unsigned int cmd,
> -					struct snd_seq_port_info32 __user *data32)
> +static int seq_call_port_info_ioctl(struct snd_seq_client *client,
> +				    unsigned int cmd,
> +				    struct snd_seq_port_info32 __user *data32)
>  {
>  	int err = -EFAULT;
>  	struct snd_seq_port_info *data;
> @@ -60,7 +61,7 @@ static int snd_seq_call_port_info_ioctl(struct snd_seq_client *client, unsigned
>  	data->kernel = NULL;
>  
>  	fs = snd_enter_user();
> -	err = snd_seq_do_ioctl(client, cmd, data);
> +	err = seq_do_ioctl(client, cmd, data);
>  	snd_leave_user(fs);
>  	if (err < 0)
>  		goto error;
> @@ -123,17 +124,17 @@ static long snd_seq_ioctl_compat(struct file *file, unsigned int cmd, unsigned l
>  	case SNDRV_SEQ_IOCTL_GET_SUBSCRIPTION:
>  	case SNDRV_SEQ_IOCTL_QUERY_NEXT_CLIENT:
>  	case SNDRV_SEQ_IOCTL_RUNNING_MODE:
> -		return snd_seq_do_ioctl(client, cmd, argp);
> +		return seq_do_ioctl(client, cmd, argp);
>  	case SNDRV_SEQ_IOCTL_CREATE_PORT32:
> -		return snd_seq_call_port_info_ioctl(client, SNDRV_SEQ_IOCTL_CREATE_PORT, argp);
> +		return seq_call_port_info_ioctl(client, SNDRV_SEQ_IOCTL_CREATE_PORT, argp);
>  	case SNDRV_SEQ_IOCTL_DELETE_PORT32:
> -		return snd_seq_call_port_info_ioctl(client, SNDRV_SEQ_IOCTL_DELETE_PORT, argp);
> +		return seq_call_port_info_ioctl(client, SNDRV_SEQ_IOCTL_DELETE_PORT, argp);
>  	case SNDRV_SEQ_IOCTL_GET_PORT_INFO32:
> -		return snd_seq_call_port_info_ioctl(client, SNDRV_SEQ_IOCTL_GET_PORT_INFO, argp);
> +		return seq_call_port_info_ioctl(client, SNDRV_SEQ_IOCTL_GET_PORT_INFO, argp);
>  	case SNDRV_SEQ_IOCTL_SET_PORT_INFO32:
> -		return snd_seq_call_port_info_ioctl(client, SNDRV_SEQ_IOCTL_SET_PORT_INFO, argp);
> +		return seq_call_port_info_ioctl(client, SNDRV_SEQ_IOCTL_SET_PORT_INFO, argp);
>  	case SNDRV_SEQ_IOCTL_QUERY_NEXT_PORT32:
> -		return snd_seq_call_port_info_ioctl(client, SNDRV_SEQ_IOCTL_QUERY_NEXT_PORT, argp);
> +		return seq_call_port_info_ioctl(client, SNDRV_SEQ_IOCTL_QUERY_NEXT_PORT, argp);
>  	}
>  	return -ENOIOCTLCMD;
>  }
> -- 
> 2.7.4
> 
> _______________________________________________
> Alsa-devel mailing list
> Alsa-devel@alsa-project.org
> http://mailman.alsa-project.org/mailman/listinfo/alsa-devel
> 

^ permalink raw reply	[flat|nested] 63+ messages in thread

* Re: [PATCH 05/39] ALSA: seq: add documentation for snd_seq_kernel_client_ctl
  2016-08-07  9:48 ` [PATCH 05/39] ALSA: seq: add documentation for snd_seq_kernel_client_ctl Takashi Sakamoto
@ 2016-08-08  6:58   ` Takashi Iwai
  0 siblings, 0 replies; 63+ messages in thread
From: Takashi Iwai @ 2016-08-08  6:58 UTC (permalink / raw)
  To: Takashi Sakamoto; +Cc: alsa-devel, clemens

On Sun, 07 Aug 2016 11:48:41 +0200,
Takashi Sakamoto wrote:
> 
> This kernel API is used by kernel implementation. Currently, it's used for
> kernel clients of ALSA sequencer, while it can be used for application
> clients. The difference is just on address spaces of argument. In short,
> this kernel API can be available for application client with data in kernel
> space.
> 
> This commit adds a document about this.
> 
> Signed-off-by: Takashi Sakamoto <o-takashi@sakamocchi.jp>
> ---
>  sound/core/seq/seq_clientmgr.c | 14 +++++++++++---
>  1 file changed, 11 insertions(+), 3 deletions(-)
> 
> diff --git a/sound/core/seq/seq_clientmgr.c b/sound/core/seq/seq_clientmgr.c
> index 17d988a..a09cb84 100644
> --- a/sound/core/seq/seq_clientmgr.c
> +++ b/sound/core/seq/seq_clientmgr.c
> @@ -2494,9 +2494,17 @@ int snd_seq_kernel_client_dispatch(int client, struct snd_seq_event * ev,
>  
>  EXPORT_SYMBOL(snd_seq_kernel_client_dispatch);
>  
> -/*
> - * exported, called by kernel clients to perform same functions as with
> - * userland ioctl() 
> +/**
> + * snd_seq_kernel_client_ctl - operate a command for a client with data in
> + *			       kernel space.
> + * @clientid:	A numerical ID for a client.
> + * @cmd:	An ioctl(2) command for ALSA sequencer operation.
> + * @arg:	A pointer to data in kernel space.
> + *
> + * Against its name, both kernel/application client can be handled by this
> + * kernel API. Address space for 'arg' argument should be in kernel space.
> + *
> + * Return: 0 at success. Minus error code at failure.

s/Minus/Negative/


Takashi

^ permalink raw reply	[flat|nested] 63+ messages in thread

* Re: [PATCH 11/39] ALSA: seq: optimize system_info function to new design
  2016-08-07  9:48 ` [PATCH 11/39] ALSA: seq: optimize system_info " Takashi Sakamoto
@ 2016-08-08  7:04   ` Takashi Iwai
  2016-08-08  7:50     ` Takashi Iwai
  2016-08-08  8:37     ` Takashi Sakamoto
  0 siblings, 2 replies; 63+ messages in thread
From: Takashi Iwai @ 2016-08-08  7:04 UTC (permalink / raw)
  To: Takashi Sakamoto; +Cc: alsa-devel, clemens

On Sun, 07 Aug 2016 11:48:47 +0200,
Takashi Sakamoto wrote:
> 
> In former commit, actual operations of each ioctl command get argument
> in kernel space. Copying from/to user space is performed outside of
> the function.
> 
> This commit optimizes to the new design.
> 
> Signed-off-by: Takashi Sakamoto <o-takashi@sakamocchi.jp>

While it's OK to split to small patches if you prefer, you don't have
to do so.  Basically all the rest are doing the same thing (strip
copy_*_user() and replace to the pointer accesses), and it's rather
boring to read repeated mails.


thanks,

Takashi

^ permalink raw reply	[flat|nested] 63+ messages in thread

* Re: [PATCH 06/39] ALSA: seq: obsolete address mode in compatibility layer
  2016-08-07  9:48 ` [PATCH 06/39] ALSA: seq: obsolete address mode in compatibility layer Takashi Sakamoto
@ 2016-08-08  7:09   ` Takashi Iwai
  2016-08-08 12:19     ` Takashi Sakamoto
  0 siblings, 1 reply; 63+ messages in thread
From: Takashi Iwai @ 2016-08-08  7:09 UTC (permalink / raw)
  To: Takashi Sakamoto; +Cc: alsa-devel, clemens

On Sun, 07 Aug 2016 11:48:42 +0200,
Takashi Sakamoto wrote:
> 
> In a 32/64 bit compatibility layer of ALSA sequencer core, data for some
> ioctls is copied to kernel stack and passed to core operations. Then,
> address limit of running task is changed because core implementation
> expected arguments in userspace.
> 
> In this case, snd_seq_kernel_client_ctl() is available. This commit
> replaces with it.
> 
> Signed-off-by: Takashi Sakamoto <o-takashi@sakamocchi.jp>
> ---
>  sound/core/seq/seq_compat.c | 25 +++++++++++++------------
>  1 file changed, 13 insertions(+), 12 deletions(-)
> 
> diff --git a/sound/core/seq/seq_compat.c b/sound/core/seq/seq_compat.c
> index 70d3ddb..6cc7302 100644
> --- a/sound/core/seq/seq_compat.c
> +++ b/sound/core/seq/seq_compat.c
> @@ -42,13 +42,11 @@ struct snd_seq_port_info32 {
>  	char reserved[59];		/* for future use */
>  };
>  
> -static int seq_call_port_info_ioctl(struct snd_seq_client *client,
> -				    unsigned int cmd,
> +static int seq_call_port_info_ioctl(int clientid, unsigned int cmd,
>  				    struct snd_seq_port_info32 __user *data32)
>  {
>  	int err = -EFAULT;
>  	struct snd_seq_port_info *data;
> -	mm_segment_t fs;
>  
>  	data = kmalloc(sizeof(*data), GFP_KERNEL);
>  	if (!data)
> @@ -60,9 +58,7 @@ static int seq_call_port_info_ioctl(struct snd_seq_client *client,
>  		goto error;
>  	data->kernel = NULL;
>  
> -	fs = snd_enter_user();
> -	err = seq_do_ioctl(client, cmd, data);
> -	snd_leave_user(fs);
> +	err = snd_seq_kernel_client_ctl(clientid, cmd, data);
>  	if (err < 0)
>  		goto error;

It's better to pass a snd_seq_client pointer to this function and
evaluate client->port in the function instead of referencing to
client->port from each caller.


Takashi

^ permalink raw reply	[flat|nested] 63+ messages in thread

* Re: [PATCH 04/39] ALSA: seq: copy ioctl data from user space to kernel stack
  2016-08-07 14:26     ` Takashi Sakamoto
@ 2016-08-08  7:10       ` Takashi Iwai
  2016-08-08 13:46         ` Takashi Sakamoto
  0 siblings, 1 reply; 63+ messages in thread
From: Takashi Iwai @ 2016-08-08  7:10 UTC (permalink / raw)
  To: Takashi Sakamoto; +Cc: alsa-devel, Clemens Ladisch

On Sun, 07 Aug 2016 16:26:35 +0200,
Takashi Sakamoto wrote:
> 
> Hi Clemens,
> 
> On Aug 7 2016 19:15, Clemens Ladisch wrote:
> > Takashi Sakamoto wrote:
> >> Data from user space is once copied to kernel stack, then operated and
> >> copied to user space, in a consistent manner. This manner forces all ioctl
> >> operations to copy the data from/to user space, even if it's read-only or
> >> write-only. Thus, it has an overhead for simpler ioctl commands.
> > 
> > The ioctl code itself already contains information about the direction
> > and size of the data to be copied (and in theory, these values are
> > correct).  See dispatch_ioctl() in drivers/firewire/core-cdev.c for an
> > example.
> 
> A nice idea.
> 
> _IOC_SIZE macro pick up 13 or 14 bits (architecture-dependent) in ioctl
> command, which represents the size of argument. In my patch, the size of
> 'union ioctl_arg' is 188 (x86/x32) or 192 (x86_64) and there's enough
> rest of the size field. So we can pick up the size from ioctl command by
> the macro because the size represents the maximum bytes of argument for
> all of sequencer ioctls.

It's not only about the size.  It contains the r/w bits, so you can
avoid the unnecessary user-copy calls, too.


Takashi

^ permalink raw reply	[flat|nested] 63+ messages in thread

* Re: [PATCH 11/39] ALSA: seq: optimize system_info function to new design
  2016-08-08  7:04   ` Takashi Iwai
@ 2016-08-08  7:50     ` Takashi Iwai
  2016-08-08 12:22       ` Takashi Sakamoto
  2016-08-08  8:37     ` Takashi Sakamoto
  1 sibling, 1 reply; 63+ messages in thread
From: Takashi Iwai @ 2016-08-08  7:50 UTC (permalink / raw)
  To: Takashi Sakamoto; +Cc: alsa-devel, clemens

On Mon, 08 Aug 2016 09:04:55 +0200,
Takashi Iwai wrote:
> 
> On Sun, 07 Aug 2016 11:48:47 +0200,
> Takashi Sakamoto wrote:
> > 
> > In former commit, actual operations of each ioctl command get argument
> > in kernel space. Copying from/to user space is performed outside of
> > the function.
> > 
> > This commit optimizes to the new design.
> > 
> > Signed-off-by: Takashi Sakamoto <o-takashi@sakamocchi.jp>
> 
> While it's OK to split to small patches if you prefer, you don't have
> to do so.  Basically all the rest are doing the same thing (strip
> copy_*_user() and replace to the pointer accesses), and it's rather
> boring to read repeated mails.

BTW, I'm afraid that the patch series breaks bisection.
We need to consider rearranging the changes if we want to keep
bisectionability.


Takashi

^ permalink raw reply	[flat|nested] 63+ messages in thread

* Re: [PATCH 11/39] ALSA: seq: optimize system_info function to new design
  2016-08-08  7:04   ` Takashi Iwai
  2016-08-08  7:50     ` Takashi Iwai
@ 2016-08-08  8:37     ` Takashi Sakamoto
  2016-08-08 15:07       ` Takashi Iwai
  1 sibling, 1 reply; 63+ messages in thread
From: Takashi Sakamoto @ 2016-08-08  8:37 UTC (permalink / raw)
  To: Takashi Iwai; +Cc: alsa-devel, clemens

On Aug 8 2016 16:04, Takashi Iwai wrote:
> On Sun, 07 Aug 2016 11:48:47 +0200,
> Takashi Sakamoto wrote:
>>
>> In former commit, actual operations of each ioctl command get argument
>> in kernel space. Copying from/to user space is performed outside of
>> the function.
>>
>> This commit optimizes to the new design.
>>
>> Signed-off-by: Takashi Sakamoto <o-takashi@sakamocchi.jp>
>
> While it's OK to split to small patches if you prefer, you don't have
> to do so.  Basically all the rest are doing the same thing (strip
> copy_*_user() and replace to the pointer accesses), and it's rather
> boring to read repeated mails.

The mail server of alsa-project.org has a limitation of the size of one 
message. It shrinks developers to split commit to a small parts. Not 
from my taste.


Regards

Takashi Sakamoto

^ permalink raw reply	[flat|nested] 63+ messages in thread

* Re: [PATCH 06/39] ALSA: seq: obsolete address mode in compatibility layer
  2016-08-08  7:09   ` Takashi Iwai
@ 2016-08-08 12:19     ` Takashi Sakamoto
  2016-08-08 14:59       ` Takashi Iwai
  0 siblings, 1 reply; 63+ messages in thread
From: Takashi Sakamoto @ 2016-08-08 12:19 UTC (permalink / raw)
  To: Takashi Iwai; +Cc: alsa-devel, clemens

On Aug 8 2016 16:09, Takashi Iwai wrote:
> It's better to pass a snd_seq_client pointer to this function and
> evaluate client->port in the function instead of referencing to
> client->port from each caller.

?

I don't understand to what you addressed. Neither snd_seq_do_ioctl() and 
snd_seq_kernel_client_ctl() have differences about port referencing.


Regards

Takashi Sakamoto

^ permalink raw reply	[flat|nested] 63+ messages in thread

* Re: [PATCH 11/39] ALSA: seq: optimize system_info function to new design
  2016-08-08  7:50     ` Takashi Iwai
@ 2016-08-08 12:22       ` Takashi Sakamoto
  2016-08-08 15:18         ` Takashi Iwai
  0 siblings, 1 reply; 63+ messages in thread
From: Takashi Sakamoto @ 2016-08-08 12:22 UTC (permalink / raw)
  To: Takashi Iwai; +Cc: alsa-devel, clemens

On Aug 8 2016 16:50, Takashi Iwai wrote:
> On Mon, 08 Aug 2016 09:04:55 +0200,
> Takashi Iwai wrote:
>>
>> On Sun, 07 Aug 2016 11:48:47 +0200,
>> Takashi Sakamoto wrote:
>>>
>>> In former commit, actual operations of each ioctl command get argument
>>> in kernel space. Copying from/to user space is performed outside of
>>> the function.
>>>
>>> This commit optimizes to the new design.
>>>
>>> Signed-off-by: Takashi Sakamoto <o-takashi@sakamocchi.jp>
>>
>> While it's OK to split to small patches if you prefer, you don't have
>> to do so.  Basically all the rest are doing the same thing (strip
>> copy_*_user() and replace to the pointer accesses), and it's rather
>> boring to read repeated mails.
>
> BTW, I'm afraid that the patch series breaks bisection.
> We need to consider rearranging the changes if we want to keep
> bisectionability.

As long as it's possible. But in this case, it's difficult. The relation 
between ioctl table and each functions is one to N. If we change them in 
one patch, the size is quite large (and alsa-project.org will skip it to 
deliver.). It's unavoidable.


Regards

Takashi Sakamoto

^ permalink raw reply	[flat|nested] 63+ messages in thread

* Re: [PATCH 04/39] ALSA: seq: copy ioctl data from user space to kernel stack
  2016-08-08  7:10       ` Takashi Iwai
@ 2016-08-08 13:46         ` Takashi Sakamoto
  2016-08-08 14:58           ` Takashi Iwai
  0 siblings, 1 reply; 63+ messages in thread
From: Takashi Sakamoto @ 2016-08-08 13:46 UTC (permalink / raw)
  To: Takashi Iwai; +Cc: alsa-devel, Clemens Ladisch

On Aug 8 2016 16:10, Takashi Iwai wrote:
>> _IOC_SIZE macro pick up 13 or 14 bits (architecture-dependent) in ioctl
>> command, which represents the size of argument. In my patch, the size of
>> 'union ioctl_arg' is 188 (x86/x32) or 192 (x86_64) and there's enough
>> rest of the size field. So we can pick up the size from ioctl command by
>> the macro because the size represents the maximum bytes of argument for
>> all of sequencer ioctls.
> 
> It's not only about the size.  It contains the r/w bits, so you can
> avoid the unnecessary user-copy calls, too.

SET_QUEUE_CLIENT ioctl command is defined as 'W', while a corresponding
function writes to userspace. This is unavoidable bug.


Regards

Takashi Sakamoto

^ permalink raw reply	[flat|nested] 63+ messages in thread

* Re: [PATCH 04/39] ALSA: seq: copy ioctl data from user space to kernel stack
  2016-08-08 13:46         ` Takashi Sakamoto
@ 2016-08-08 14:58           ` Takashi Iwai
  0 siblings, 0 replies; 63+ messages in thread
From: Takashi Iwai @ 2016-08-08 14:58 UTC (permalink / raw)
  To: Takashi Sakamoto; +Cc: alsa-devel, Clemens Ladisch

On Mon, 08 Aug 2016 15:46:21 +0200,
Takashi Sakamoto wrote:
> 
> On Aug 8 2016 16:10, Takashi Iwai wrote:
> >> _IOC_SIZE macro pick up 13 or 14 bits (architecture-dependent) in ioctl
> >> command, which represents the size of argument. In my patch, the size of
> >> 'union ioctl_arg' is 188 (x86/x32) or 192 (x86_64) and there's enough
> >> rest of the size field. So we can pick up the size from ioctl command by
> >> the macro because the size represents the maximum bytes of argument for
> >> all of sequencer ioctls.
> > 
> > It's not only about the size.  It contains the r/w bits, so you can
> > avoid the unnecessary user-copy calls, too.
> 
> SET_QUEUE_CLIENT ioctl command is defined as 'W', while a corresponding
> function writes to userspace. This is unavoidable bug.

Yes, there are a few things to be corrected.  That is, some ioctls
need to be remapped to right ioctl numbers beforehand.


Takashi

^ permalink raw reply	[flat|nested] 63+ messages in thread

* Re: [PATCH 06/39] ALSA: seq: obsolete address mode in compatibility layer
  2016-08-08 12:19     ` Takashi Sakamoto
@ 2016-08-08 14:59       ` Takashi Iwai
  2016-08-09 12:04         ` Takashi Sakamoto
  0 siblings, 1 reply; 63+ messages in thread
From: Takashi Iwai @ 2016-08-08 14:59 UTC (permalink / raw)
  To: Takashi Sakamoto; +Cc: alsa-devel, clemens

On Mon, 08 Aug 2016 14:19:50 +0200,
Takashi Sakamoto wrote:
> 
> On Aug 8 2016 16:09, Takashi Iwai wrote:
> > It's better to pass a snd_seq_client pointer to this function and
> > evaluate client->port in the function instead of referencing to
> > client->port from each caller.
> 
> ?
> 
> I don't understand to what you addressed. Neither snd_seq_do_ioctl()
> and snd_seq_kernel_client_ctl() have differences about port
> referencing.

Put in this way: why did you change seq_call_port_ioctl() to take an
clientid integer instead of client pointer?  Due to this change, each
caller has to deduce the client number by referencing client->number.
Which merit does it give at all?


Takashi

^ permalink raw reply	[flat|nested] 63+ messages in thread

* Re: [PATCH 11/39] ALSA: seq: optimize system_info function to new design
  2016-08-08  8:37     ` Takashi Sakamoto
@ 2016-08-08 15:07       ` Takashi Iwai
  2016-08-09 12:15         ` Takashi Sakamoto
  0 siblings, 1 reply; 63+ messages in thread
From: Takashi Iwai @ 2016-08-08 15:07 UTC (permalink / raw)
  To: Takashi Sakamoto; +Cc: alsa-devel, clemens

On Mon, 08 Aug 2016 10:37:21 +0200,
Takashi Sakamoto wrote:
> 
> On Aug 8 2016 16:04, Takashi Iwai wrote:
> > On Sun, 07 Aug 2016 11:48:47 +0200,
> > Takashi Sakamoto wrote:
> >>
> >> In former commit, actual operations of each ioctl command get argument
> >> in kernel space. Copying from/to user space is performed outside of
> >> the function.
> >>
> >> This commit optimizes to the new design.
> >>
> >> Signed-off-by: Takashi Sakamoto <o-takashi@sakamocchi.jp>
> >
> > While it's OK to split to small patches if you prefer, you don't have
> > to do so.  Basically all the rest are doing the same thing (strip
> > copy_*_user() and replace to the pointer accesses), and it's rather
> > boring to read repeated mails.
> 
> The mail server of alsa-project.org has a limitation of the size of
> one message. It shrinks developers to split commit to a small
> parts. Not from my taste.

Yeah, that's sometimes annoying, indeed.  Jaroslav, could you raise
the bar to allow larger patches, or drop this restriction?


Meanwhile, the current limitation is reasonable in most cases.
Patches with thousands of lines make review more difficult.  If your
patch exceeds the size, it already indicates that it may be
potentially a bad patch.  The logic is something similar like 80 chars
limitation we have in general.

Sometimes a big patch is unavoidable (e.g. writing a new driver from
scratch).  Or sometimes a bigger patch is acceptable (e.g. when
changes are machinery, done purely by a script).  But if you wrote
some patch by hands and it exceeds the size, you should doubt it at
first.


Takashi

^ permalink raw reply	[flat|nested] 63+ messages in thread

* Re: [PATCH 11/39] ALSA: seq: optimize system_info function to new design
  2016-08-08 12:22       ` Takashi Sakamoto
@ 2016-08-08 15:18         ` Takashi Iwai
  0 siblings, 0 replies; 63+ messages in thread
From: Takashi Iwai @ 2016-08-08 15:18 UTC (permalink / raw)
  To: Takashi Sakamoto; +Cc: alsa-devel, clemens

On Mon, 08 Aug 2016 14:22:47 +0200,
Takashi Sakamoto wrote:
> 
> On Aug 8 2016 16:50, Takashi Iwai wrote:
> > On Mon, 08 Aug 2016 09:04:55 +0200,
> > Takashi Iwai wrote:
> >>
> >> On Sun, 07 Aug 2016 11:48:47 +0200,
> >> Takashi Sakamoto wrote:
> >>>
> >>> In former commit, actual operations of each ioctl command get argument
> >>> in kernel space. Copying from/to user space is performed outside of
> >>> the function.
> >>>
> >>> This commit optimizes to the new design.
> >>>
> >>> Signed-off-by: Takashi Sakamoto <o-takashi@sakamocchi.jp>
> >>
> >> While it's OK to split to small patches if you prefer, you don't have
> >> to do so.  Basically all the rest are doing the same thing (strip
> >> copy_*_user() and replace to the pointer accesses), and it's rather
> >> boring to read repeated mails.
> >
> > BTW, I'm afraid that the patch series breaks bisection.
> > We need to consider rearranging the changes if we want to keep
> > bisectionability.
> 
> As long as it's possible. But in this case, it's difficult. The
> relation between ioctl table and each functions is one to N. If we
> change them in one patch, the size is quite large (and
> alsa-project.org will skip it to deliver.). It's unavoidable.

The standard technique is transitional changes: namely, introduce a
new function table while keeping both old and new tables, move the
entries gradually, and finally clean up the old table.

In your case, try to start cleaning up the user-copy at first.  That
is,

- Create a function to do user-copy and call function pointer with a
  new function table.  It's called from snd_seq_do_ioctl(), and
  snd_seq_do_ioctl() processes the old function table as fallback.

- Move each function to the new function.

- After moving all functions, replace snd_seq_do_ioctl() with the new
  one you created in the first step.

At this moment, snd_seq_kernel_client_ctl() still calls
snd_seq_do_ioctl().  Now we can clean up the segment workaround.
At best, retrieve a lookup function for obtaining the function pointer
from the given ioctl number.  Then both snd_seq_kernel_client_ctl()
and seq_seq_do_ioctl() can call it and delegate to the function.

At last, clean up the compat code.


Takashi

^ permalink raw reply	[flat|nested] 63+ messages in thread

* Re: [PATCH 06/39] ALSA: seq: obsolete address mode in compatibility layer
  2016-08-08 14:59       ` Takashi Iwai
@ 2016-08-09 12:04         ` Takashi Sakamoto
  2016-08-09 12:15           ` Takashi Iwai
  0 siblings, 1 reply; 63+ messages in thread
From: Takashi Sakamoto @ 2016-08-09 12:04 UTC (permalink / raw)
  To: Takashi Iwai; +Cc: alsa-devel, clemens

On Aug 8 2016 23:59, Takashi Iwai wrote:
> On Mon, 08 Aug 2016 14:19:50 +0200,
> Takashi Sakamoto wrote:
>>
>> On Aug 8 2016 16:09, Takashi Iwai wrote:
>>> It's better to pass a snd_seq_client pointer to this function and
>>> evaluate client->port in the function instead of referencing to
>>> client->port from each caller.
>>
>> ?
>>
>> I don't understand to what you addressed. Neither snd_seq_do_ioctl()
>> and snd_seq_kernel_client_ctl() have differences about port
>> referencing.
> 
> Put in this way: why did you change seq_call_port_ioctl() to take an
> clientid integer instead of client pointer?  Due to this change, each
> caller has to deduce the client number by referencing client->number.
> Which merit does it give at all?

Just to call snd_seq_kernel_client_ctl(). The callers are in 32/64 bit
compatibility code and the function is not exported.

I don't realize what you concern yet.


Regards

Takashi Sakamoto

^ permalink raw reply	[flat|nested] 63+ messages in thread

* Re: [PATCH 11/39] ALSA: seq: optimize system_info function to new design
  2016-08-08 15:07       ` Takashi Iwai
@ 2016-08-09 12:15         ` Takashi Sakamoto
  2016-08-09 12:24           ` Takashi Iwai
  0 siblings, 1 reply; 63+ messages in thread
From: Takashi Sakamoto @ 2016-08-09 12:15 UTC (permalink / raw)
  To: Takashi Iwai; +Cc: alsa-devel, clemens

On Aug 9 2016 00:07, Takashi Iwai wrote:
> On Mon, 08 Aug 2016 10:37:21 +0200,
> Takashi Sakamoto wrote:
>>
>> On Aug 8 2016 16:04, Takashi Iwai wrote:
>>> On Sun, 07 Aug 2016 11:48:47 +0200,
>>> Takashi Sakamoto wrote:
>>>>
>>>> In former commit, actual operations of each ioctl command get argument
>>>> in kernel space. Copying from/to user space is performed outside of
>>>> the function.
>>>>
>>>> This commit optimizes to the new design.
>>>>
>>>> Signed-off-by: Takashi Sakamoto <o-takashi@sakamocchi.jp>
>>>
>>> While it's OK to split to small patches if you prefer, you don't have
>>> to do so.  Basically all the rest are doing the same thing (strip
>>> copy_*_user() and replace to the pointer accesses), and it's rather
>>> boring to read repeated mails.
>>
>> The mail server of alsa-project.org has a limitation of the size of
>> one message. It shrinks developers to split commit to a small
>> parts. Not from my taste.
> 
> Yeah, that's sometimes annoying, indeed.  Jaroslav, could you raise
> the bar to allow larger patches, or drop this restriction?

It's not what I want. I've already followed to the restriction for this
several years. The restriction is still OK as long as I can split my
patches as small parts and I'm not objected for them.

You completely puzzled me. Finally, what you'd like me to do?????


Regards

Takashi Sakamoto

^ permalink raw reply	[flat|nested] 63+ messages in thread

* Re: [PATCH 06/39] ALSA: seq: obsolete address mode in compatibility layer
  2016-08-09 12:04         ` Takashi Sakamoto
@ 2016-08-09 12:15           ` Takashi Iwai
  2016-08-09 12:32             ` Takashi Sakamoto
  0 siblings, 1 reply; 63+ messages in thread
From: Takashi Iwai @ 2016-08-09 12:15 UTC (permalink / raw)
  To: Takashi Sakamoto; +Cc: alsa-devel, clemens

On Tue, 09 Aug 2016 14:04:37 +0200,
Takashi Sakamoto wrote:
> 
> On Aug 8 2016 23:59, Takashi Iwai wrote:
> > On Mon, 08 Aug 2016 14:19:50 +0200,
> > Takashi Sakamoto wrote:
> >>
> >> On Aug 8 2016 16:09, Takashi Iwai wrote:
> >>> It's better to pass a snd_seq_client pointer to this function and
> >>> evaluate client->port in the function instead of referencing to
> >>> client->port from each caller.
> >>
> >> ?
> >>
> >> I don't understand to what you addressed. Neither snd_seq_do_ioctl()
> >> and snd_seq_kernel_client_ctl() have differences about port
> >> referencing.
> > 
> > Put in this way: why did you change seq_call_port_ioctl() to take an
> > clientid integer instead of client pointer?  Due to this change, each
> > caller has to deduce the client number by referencing client->number.
> > Which merit does it give at all?
> 
> Just to call snd_seq_kernel_client_ctl(). The callers are in 32/64 bit
> compatibility code and the function is not exported.
> 
> I don't realize what you concern yet.

Please read back your patch again.  You changed the function argument
like:

================================================================
@@ -42,13 +42,11 @@ struct snd_seq_port_info32 {
 	char reserved[59];		/* for future use */
 };
 
-static int seq_call_port_info_ioctl(struct snd_seq_client *client,
-				    unsigned int cmd,
+static int seq_call_port_info_ioctl(int clientid, unsigned int cmd,
 				    struct snd_seq_port_info32 __user *data32)
================================================================

And this requires the changes in all its callers as below:

================================================================
 	case SNDRV_SEQ_IOCTL_CREATE_PORT32:
-		return seq_call_port_info_ioctl(client, SNDRV_SEQ_IOCTL_CREATE_PORT, argp);
+		return seq_call_port_info_ioctl(client->number,
+					SNDRV_SEQ_IOCTL_CREATE_PORT, argp);
 	case SNDRV_SEQ_IOCTL_DELETE_PORT32:
-		return seq_call_port_info_ioctl(client, SNDRV_SEQ_IOCTL_DELETE_PORT, argp);
+		return seq_call_port_info_ioctl(client->number,
+					SNDRV_SEQ_IOCTL_DELETE_PORT, argp);
 	case SNDRV_SEQ_IOCTL_GET_PORT_INFO32:
-		return seq_call_port_info_ioctl(client, SNDRV_SEQ_IOCTL_GET_PORT_INFO, argp);
+		return seq_call_port_info_ioctl(client->number,
+					SNDRV_SEQ_IOCTL_GET_PORT_INFO, argp);
 	case SNDRV_SEQ_IOCTL_SET_PORT_INFO32:
-		return seq_call_port_info_ioctl(client, SNDRV_SEQ_IOCTL_SET_PORT_INFO, argp);
+		return seq_call_port_info_ioctl(client->number,
+					SNDRV_SEQ_IOCTL_SET_PORT_INFO, argp);
 	case SNDRV_SEQ_IOCTL_QUERY_NEXT_PORT32:
-		return seq_call_port_info_ioctl(client, SNDRV_SEQ_IOCTL_QUERY_NEXT_PORT, argp);
+		return seq_call_port_info_ioctl(client->number,
+					SNDRV_SEQ_IOCTL_QUERY_NEXT_PORT, argp);
================================================================

But, why do you need to change these changes at all?  What you
basically need is to change the call inside seq_call_port_info_ioctl()
like:

-	err = snd_seq_do_ioctl(client, cmd, data);
+	err = snd_seq_kernel_client_ctl(client->number, cmd, data);

Then the rest can remain intact.  Referencing client->number in each
caller side just results in inefficient codes, obviously.


Takashi

^ permalink raw reply	[flat|nested] 63+ messages in thread

* Re: [PATCH 11/39] ALSA: seq: optimize system_info function to new design
  2016-08-09 12:15         ` Takashi Sakamoto
@ 2016-08-09 12:24           ` Takashi Iwai
  2016-08-09 13:01             ` Takashi Sakamoto
  0 siblings, 1 reply; 63+ messages in thread
From: Takashi Iwai @ 2016-08-09 12:24 UTC (permalink / raw)
  To: Takashi Sakamoto; +Cc: alsa-devel, clemens

On Tue, 09 Aug 2016 14:15:03 +0200,
Takashi Sakamoto wrote:
> 
> On Aug 9 2016 00:07, Takashi Iwai wrote:
> > On Mon, 08 Aug 2016 10:37:21 +0200,
> > Takashi Sakamoto wrote:
> >>
> >> On Aug 8 2016 16:04, Takashi Iwai wrote:
> >>> On Sun, 07 Aug 2016 11:48:47 +0200,
> >>> Takashi Sakamoto wrote:
> >>>>
> >>>> In former commit, actual operations of each ioctl command get argument
> >>>> in kernel space. Copying from/to user space is performed outside of
> >>>> the function.
> >>>>
> >>>> This commit optimizes to the new design.
> >>>>
> >>>> Signed-off-by: Takashi Sakamoto <o-takashi@sakamocchi.jp>
> >>>
> >>> While it's OK to split to small patches if you prefer, you don't have
> >>> to do so.  Basically all the rest are doing the same thing (strip
> >>> copy_*_user() and replace to the pointer accesses), and it's rather
> >>> boring to read repeated mails.
> >>
> >> The mail server of alsa-project.org has a limitation of the size of
> >> one message. It shrinks developers to split commit to a small
> >> parts. Not from my taste.
> > 
> > Yeah, that's sometimes annoying, indeed.  Jaroslav, could you raise
> > the bar to allow larger patches, or drop this restriction?
> 
> It's not what I want. I've already followed to the restriction for this
> several years. The restriction is still OK as long as I can split my
> patches as small parts and I'm not objected for them.
> 
> You completely puzzled me. Finally, what you'd like me to do?????

You've cut into too small patches.  Damn too small.

Of course, too small is much better than too big.  That's why I wrote
"it's OK".  But it's also a bad taste, after all -- it annoys readers,
since they need to look over the same boring changes again and again.

For avoiding annoyance, such tasks would be better to be grouped in a
certain level, so that readers can take a glance over wider ranges, as
long as all the changes are the more or less same procedure.


Takashi

^ permalink raw reply	[flat|nested] 63+ messages in thread

* Re: [PATCH 06/39] ALSA: seq: obsolete address mode in compatibility layer
  2016-08-09 12:15           ` Takashi Iwai
@ 2016-08-09 12:32             ` Takashi Sakamoto
  2016-08-09 12:58               ` Takashi Iwai
  0 siblings, 1 reply; 63+ messages in thread
From: Takashi Sakamoto @ 2016-08-09 12:32 UTC (permalink / raw)
  To: Takashi Iwai; +Cc: alsa-devel, clemens

On Aug 9 2016 21:15, Takashi Iwai wrote:
> But, why do you need to change these changes at all?  What you
> basically need is to change the call inside seq_call_port_info_ioctl()
> like:
> 
> -	err = snd_seq_do_ioctl(client, cmd, data);
> +	err = snd_seq_kernel_client_ctl(client->number, cmd, data);
> 
> Then the rest can remain intact.  Referencing client->number in each
> caller side just results in inefficient codes, obviously.

In seq_call_port_info_ioctl(), no members except for the 'number' are
used. So no need to get a pointer to the client data as an argument.

This change makes it simpler for readers to follow code of the function.
They have no need to think about the whole data, it's OK just to focus
on the numerical ID for client.


Regards

Takashi Sakamoto

^ permalink raw reply	[flat|nested] 63+ messages in thread

* Re: [PATCH 06/39] ALSA: seq: obsolete address mode in compatibility layer
  2016-08-09 12:32             ` Takashi Sakamoto
@ 2016-08-09 12:58               ` Takashi Iwai
  0 siblings, 0 replies; 63+ messages in thread
From: Takashi Iwai @ 2016-08-09 12:58 UTC (permalink / raw)
  To: Takashi Sakamoto; +Cc: alsa-devel, clemens

On Tue, 09 Aug 2016 14:32:30 +0200,
Takashi Sakamoto wrote:
> 
> On Aug 9 2016 21:15, Takashi Iwai wrote:
> > But, why do you need to change these changes at all?  What you
> > basically need is to change the call inside seq_call_port_info_ioctl()
> > like:
> > 
> > -	err = snd_seq_do_ioctl(client, cmd, data);
> > +	err = snd_seq_kernel_client_ctl(client->number, cmd, data);
> > 
> > Then the rest can remain intact.  Referencing client->number in each
> > caller side just results in inefficient codes, obviously.
> 
> In seq_call_port_info_ioctl(), no members except for the 'number' are
> used. So no need to get a pointer to the client data as an argument.
> 
> This change makes it simpler for readers to follow code of the function.
> They have no need to think about the whole data, it's OK just to focus
> on the numerical ID for client.

I don't buy this argument, it's no obvious improvement about
readability at all, sorry.  Why delegating the object becomes so
difficult to follow suddenly from that point?  IOW, why passing a
client number there is easier to understand?  The caller doesn't need
to care which field is evaluated.  C isn't OOP language, but still
most of Linux codes are in such a style.

Unless any obvious code readability improvement is done, usually the
merit of the code optimization wins.  It even avoids the unnecessary
changes, makes easier to follow the change history in git tree in this
case.  It's often user-friendlier than small frequent changes here and
there "just for a taste".


Takashi

^ permalink raw reply	[flat|nested] 63+ messages in thread

* Re: [PATCH 11/39] ALSA: seq: optimize system_info function to new design
  2016-08-09 12:24           ` Takashi Iwai
@ 2016-08-09 13:01             ` Takashi Sakamoto
  0 siblings, 0 replies; 63+ messages in thread
From: Takashi Sakamoto @ 2016-08-09 13:01 UTC (permalink / raw)
  To: Takashi Iwai; +Cc: alsa-devel, clemens

On Aug 9 2016 21:24, Takashi Iwai wrote:
>> You completely puzzled me. Finally, what you'd like me to do?????
> 
> You've cut into too small patches.  Damn too small.
> 
> Of course, too small is much better than too big.  That's why I wrote
> "it's OK".  But it's also a bad taste, after all -- it annoys readers,
> since they need to look over the same boring changes again and again.
> 
> For avoiding annoyance, such tasks would be better to be grouped in a
> certain level, so that readers can take a glance over wider ranges, as
> long as all the changes are the more or less same procedure.

And bisect operations have a practical effect.

Hm, OK. I see. Depending on the purpose of patches, I'm likely to create
large patches.

Honestly, I have a long concern about the granularity of the size of
each patch. For example, change log of the latest alsa-lib in
alsa-project.org is quite bad and I was a bit disappointed.


Regards

Takashi Sakamoto

^ permalink raw reply	[flat|nested] 63+ messages in thread

end of thread, other threads:[~2016-08-09 13:01 UTC | newest]

Thread overview: 63+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-08-07  9:48 [PATCH 00/39] seq: obsolete change of address limit Takashi Sakamoto
2016-08-07  9:48 ` [PATCH 01/39] ALSA: seq: add const qualifier to table of functions for ioctl Takashi Sakamoto
2016-08-07  9:48 ` [PATCH 02/39] ALSA: seq: apply shorter name for file local functions Takashi Sakamoto
2016-08-08  6:56   ` Takashi Iwai
2016-08-07  9:48 ` [PATCH 03/39] ALSA: seq: fulfill callback entry for ioctl Takashi Sakamoto
2016-08-07  9:48 ` [PATCH 04/39] ALSA: seq: copy ioctl data from user space to kernel stack Takashi Sakamoto
2016-08-07 10:15   ` Clemens Ladisch
2016-08-07 14:26     ` Takashi Sakamoto
2016-08-08  7:10       ` Takashi Iwai
2016-08-08 13:46         ` Takashi Sakamoto
2016-08-08 14:58           ` Takashi Iwai
2016-08-07  9:48 ` [PATCH 05/39] ALSA: seq: add documentation for snd_seq_kernel_client_ctl Takashi Sakamoto
2016-08-08  6:58   ` Takashi Iwai
2016-08-07  9:48 ` [PATCH 06/39] ALSA: seq: obsolete address mode in compatibility layer Takashi Sakamoto
2016-08-08  7:09   ` Takashi Iwai
2016-08-08 12:19     ` Takashi Sakamoto
2016-08-08 14:59       ` Takashi Iwai
2016-08-09 12:04         ` Takashi Sakamoto
2016-08-09 12:15           ` Takashi Iwai
2016-08-09 12:32             ` Takashi Sakamoto
2016-08-09 12:58               ` Takashi Iwai
2016-08-07  9:48 ` [PATCH 07/39] ALSA: seq: obsolete change of address limit in in-kernel path for ioctl Takashi Sakamoto
2016-08-07  9:48 ` [PATCH 08/39] ALSA: seq: obsolete address limit helper Takashi Sakamoto
2016-08-07  9:48 ` [PATCH 09/39] ALSA: seq: optimize pversion function to new design Takashi Sakamoto
2016-08-07  9:48 ` [PATCH 10/39] ALSA: seq: optimize client_id " Takashi Sakamoto
2016-08-07  9:48 ` [PATCH 11/39] ALSA: seq: optimize system_info " Takashi Sakamoto
2016-08-08  7:04   ` Takashi Iwai
2016-08-08  7:50     ` Takashi Iwai
2016-08-08 12:22       ` Takashi Sakamoto
2016-08-08 15:18         ` Takashi Iwai
2016-08-08  8:37     ` Takashi Sakamoto
2016-08-08 15:07       ` Takashi Iwai
2016-08-09 12:15         ` Takashi Sakamoto
2016-08-09 12:24           ` Takashi Iwai
2016-08-09 13:01             ` Takashi Sakamoto
2016-08-07  9:48 ` [PATCH 12/39] ALSA: seq: optimize running mode " Takashi Sakamoto
2016-08-07  9:48 ` [PATCH 13/39] ALSA: seq: optimize client_info " Takashi Sakamoto
2016-08-07  9:48 ` [PATCH 14/39] ALSA: seq: optimize set_client_info " Takashi Sakamoto
2016-08-07  9:48 ` [PATCH 15/39] ALSA: seq: optimize create_port " Takashi Sakamoto
2016-08-07  9:48 ` [PATCH 16/39] ALSA: seq: optimize delete_port " Takashi Sakamoto
2016-08-07  9:48 ` [PATCH 17/39] ALSA: seq: optimize get_port_info " Takashi Sakamoto
2016-08-07  9:48 ` [PATCH 18/39] ALSA: seq: optimize seq_port_info " Takashi Sakamoto
2016-08-07  9:48 ` [PATCH 19/39] ALSA: seq: optimize subscribe_port " Takashi Sakamoto
2016-08-07  9:48 ` [PATCH 20/39] ALSA: seq: optimize unsubscribe_port " Takashi Sakamoto
2016-08-07  9:48 ` [PATCH 21/39] ALSA: seq: optimize create_queue " Takashi Sakamoto
2016-08-07  9:48 ` [PATCH 22/39] ALSA: seq: optimize delete_queue " Takashi Sakamoto
2016-08-07  9:48 ` [PATCH 23/39] ALSA: seq: optimize get_queue_info " Takashi Sakamoto
2016-08-07  9:49 ` [PATCH 24/39] ALSA: seq: optimize seq_queue_info " Takashi Sakamoto
2016-08-07  9:49 ` [PATCH 25/39] ALSA: seq: optimize get_named_queue " Takashi Sakamoto
2016-08-07  9:49 ` [PATCH 26/39] ALSA: seq: optimize get_queue_status " Takashi Sakamoto
2016-08-07  9:49 ` [PATCH 27/39] ALSA: seq: optimize get_queue_tempo " Takashi Sakamoto
2016-08-07  9:49 ` [PATCH 28/39] ALSA: seq: optimize set_queue_tempo " Takashi Sakamoto
2016-08-07  9:49 ` [PATCH 29/39] ALSA: seq: optimize get_queue_timer " Takashi Sakamoto
2016-08-07  9:49 ` [PATCH 30/39] ALSA: seq: optimize seq_queue_timer " Takashi Sakamoto
2016-08-07  9:49 ` [PATCH 31/39] ALSA: seq: optimize get_queue_client " Takashi Sakamoto
2016-08-07  9:49 ` [PATCH 32/39] ALSA: seq: optimize set_queue_client " Takashi Sakamoto
2016-08-07  9:49 ` [PATCH 33/39] ALSA: seq: optimize get_client_pool " Takashi Sakamoto
2016-08-07  9:49 ` [PATCH 34/39] ALSA: seq: optimize seq_client_pool " Takashi Sakamoto
2016-08-07  9:49 ` [PATCH 35/39] ALSA: seq: optimize remove_events " Takashi Sakamoto
2016-08-07  9:49 ` [PATCH 36/39] ALSA: seq: optimize get_subscription " Takashi Sakamoto
2016-08-07  9:49 ` [PATCH 37/39] ALSA: seq: optimize query_subs " Takashi Sakamoto
2016-08-07  9:49 ` [PATCH 38/39] ALSA: seq: optimize query_next_client " Takashi Sakamoto
2016-08-07  9:49 ` [PATCH 39/39] ALSA: seq: optimize query_next_port " Takashi Sakamoto

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.