[PATCH SPEAKUP v2 0/3] cleanup error and initilization

[PATCH SPEAKUP v2 0/3] cleanup error and initilization

[Pranay Kr. Srivastava]

Changelog from v1:
	1. fixed kbuild warning for i386 build as reported by kbuild robot
	2. split initialization code in two patches.

Pranay Kr. Srivastava (3):
  return same error value from spk_set_key_info
  remove unecessary initial allocation of vc
  use spkeaup_allocate as per required context

 drivers/staging/speakup/main.c | 46 +++++++++++++++++++++---------------------
 1 file changed, 23 insertions(+), 23 deletions(-)

-- 
2.10.2

[PATCH SPEAKUP v2 1/3] return same error value from spk_set_key_info

[Pranay Kr. Srivastava]

This patch makes spk_set_key_info return -EINVAL
in case of failure instead of returning 4 different
values for the type of error that occurred.

Print the offending values instead as debug message.

Signed-off-by: Pranay Kr. Srivastava <pranjas@gmail.com>
---
 drivers/staging/speakup/main.c | 27 +++++++++++++++++++--------
 1 file changed, 19 insertions(+), 8 deletions(-)

diff --git a/drivers/staging/speakup/main.c b/drivers/staging/speakup/main.c
index c2f70ef..a1d5b66 100644
--- a/drivers/staging/speakup/main.c
+++ b/drivers/staging/speakup/main.c
@@ -1216,13 +1216,19 @@ int spk_set_key_info(const u_char *key_info, u_char *k_buffer)
 	u_char ch, version, num_keys;
 
 	version = *cp++;
-	if (version != KEY_MAP_VER)
-		return -1;
+	if (version != KEY_MAP_VER) {
+		pr_debug("version found %d should be %d\n",
+			 version, KEY_MAP_VER);
+		return -EINVAL;
+	}
 	num_keys = *cp;
 	states = (int)cp[1];
 	key_data_len = (states + 1) * (num_keys + 1);
-	if (key_data_len + SHIFT_TBL_SIZE + 4 >= sizeof(spk_key_buf))
-		return -2;
+	if (key_data_len + SHIFT_TBL_SIZE + 4 >= sizeof(spk_key_buf)) {
+		pr_debug("too many key_infos (%d over %u)\n",
+			 key_data_len + SHIFT_TBL_SIZE + 4, (unsigned int)(sizeof(spk_key_buf)));
+		return -EINVAL;
+	}
 	memset(k_buffer, 0, SHIFT_TBL_SIZE);
 	memset(spk_our_keys, 0, sizeof(spk_our_keys));
 	spk_shift_table = k_buffer;
@@ -1233,14 +1239,19 @@ int spk_set_key_info(const u_char *key_info, u_char *k_buffer)
 	cp1 += 2;		/* now pointing at shift states */
 	for (i = 1; i <= states; i++) {
 		ch = *cp1++;
-		if (ch >= SHIFT_TBL_SIZE)
-			return -3;
+		if (ch >= SHIFT_TBL_SIZE) {
+			pr_debug("(%d) not valid shift state (max_allowed = %d)\n", ch,
+				 SHIFT_TBL_SIZE);
+			return -EINVAL;
+		}
 		spk_shift_table[ch] = i;
 	}
 	keymap_flags = *cp1++;
 	while ((ch = *cp1)) {
-		if (ch >= MAX_KEY)
-			return -4;
+		if (ch >= MAX_KEY) {
+			pr_debug("(%d), not valid key, (max_allowed = %d)\n", ch, MAX_KEY);
+			return -EINVAL;
+		}
 		spk_our_keys[ch] = cp1;
 		cp1 += states + 1;
 	}
-- 
2.10.2

[PATCH SPEAKUP v2 2/3] remove unecessary initial allocation of vc

[Pranay Kr. Srivastava]

This patch removes the unnecessary allocation of
current foreground vc during initialization.

This initialization is already handled in the loop
that follows it for all available virtual consoles.

Signed-off-by: Pranay Kr. Srivastava <pranjas@gmail.com>
---
 drivers/staging/speakup/main.c | 11 -----------
 1 file changed, 11 deletions(-)

diff --git a/drivers/staging/speakup/main.c b/drivers/staging/speakup/main.c
index a1d5b66..ca817ca 100644
--- a/drivers/staging/speakup/main.c
+++ b/drivers/staging/speakup/main.c
@@ -2317,7 +2317,6 @@ static int __init speakup_init(void)
 {
 	int i;
 	long err = 0;
-	struct st_spk_t *first_console;
 	struct vc_data *vc = vc_cons[fg_console].d;
 	struct var_t *var;
 
@@ -2342,15 +2341,6 @@ static int __init speakup_init(void)
 	if (err)
 		goto error_virtkeyboard;
 
-	first_console = kzalloc(sizeof(*first_console), GFP_KERNEL);
-	if (!first_console) {
-		err = -ENOMEM;
-		goto error_alloc;
-	}
-
-	speakup_console[vc->vc_num] = first_console;
-	speakup_date(vc);
-
 	for (i = 0; i < MAX_NR_CONSOLES; i++)
 		if (vc_cons[i].d) {
 			err = speakup_allocate(vc_cons[i].d);
@@ -2412,7 +2402,6 @@ static int __init speakup_init(void)
 	for (i = 0; i < MAX_NR_CONSOLES; i++)
 		kfree(speakup_console[i]);
 
-error_alloc:
 	speakup_remove_virtual_keyboard();
 
 error_virtkeyboard:
-- 
2.10.2

[PATCH SPEAKUP v2 3/3] use spkeaup_allocate as per required context

[Pranay Kr. Srivastava]

speakup_allocate used GFP_ATOMIC for allocations
even while during initialization due to it's use
in notifier call.

Pass GFP_ flags as well to speakup_allocate depending
on the context it is called in.

Signed-off-by: Pranay Kr. Srivastava <pranjas@gmail.com>
---
 drivers/staging/speakup/main.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/staging/speakup/main.c b/drivers/staging/speakup/main.c
index ca817ca..ede842e 100644
--- a/drivers/staging/speakup/main.c
+++ b/drivers/staging/speakup/main.c
@@ -1327,14 +1327,14 @@ static int edit_bits(struct vc_data *vc, u_char type, u_char ch, u_short key)
 }
 
 /* Allocation concurrency is protected by the console semaphore */
-static int speakup_allocate(struct vc_data *vc)
+static int speakup_allocate(struct vc_data *vc, gfp_t gfp_flags)
 {
 	int vc_num;
 
 	vc_num = vc->vc_num;
 	if (speakup_console[vc_num] == NULL) {
 		speakup_console[vc_num] = kzalloc(sizeof(*speakup_console[0]),
-						  GFP_ATOMIC);
+						  gfp_flags);
 		if (speakup_console[vc_num] == NULL)
 			return -ENOMEM;
 		speakup_date(vc);
@@ -2257,7 +2257,7 @@ static int vt_notifier_call(struct notifier_block *nb,
 	switch (code) {
 	case VT_ALLOCATE:
 		if (vc->vc_mode == KD_TEXT)
-			speakup_allocate(vc);
+			speakup_allocate(vc, GFP_ATOMIC);
 		break;
 	case VT_DEALLOCATE:
 		speakup_deallocate(vc);
@@ -2343,7 +2343,7 @@ static int __init speakup_init(void)
 
 	for (i = 0; i < MAX_NR_CONSOLES; i++)
 		if (vc_cons[i].d) {
-			err = speakup_allocate(vc_cons[i].d);
+			err = speakup_allocate(vc_cons[i].d, GFP_KERNEL);
 			if (err)
 				goto error_kobjects;
 		}
-- 
2.10.2

Re: [PATCH SPEAKUP v2 1/3] return same error value from spk_set_key_info

[Samuel Thibault]

Pranay Kr. Srivastava, on mar. 28 févr. 2017 13:57:53 +0530, wrote:
> This patch makes spk_set_key_info return -EINVAL
> in case of failure instead of returning 4 different
> values for the type of error that occurred.
> 
> Print the offending values instead as debug message.
> 
> Signed-off-by: Pranay Kr. Srivastava <pranjas@gmail.com>

Reviewed-by: Samuel Thibault <samuel.thibault@ens-lyon.org>

> ---
>  drivers/staging/speakup/main.c | 27 +++++++++++++++++++--------
>  1 file changed, 19 insertions(+), 8 deletions(-)
> 
> diff --git a/drivers/staging/speakup/main.c b/drivers/staging/speakup/main.c
> index c2f70ef..a1d5b66 100644
> --- a/drivers/staging/speakup/main.c
> +++ b/drivers/staging/speakup/main.c
> @@ -1216,13 +1216,19 @@ int spk_set_key_info(const u_char *key_info, u_char *k_buffer)
>  	u_char ch, version, num_keys;
>  
>  	version = *cp++;
> -	if (version != KEY_MAP_VER)
> -		return -1;
> +	if (version != KEY_MAP_VER) {
> +		pr_debug("version found %d should be %d\n",
> +			 version, KEY_MAP_VER);
> +		return -EINVAL;
> +	}
>  	num_keys = *cp;
>  	states = (int)cp[1];
>  	key_data_len = (states + 1) * (num_keys + 1);
> -	if (key_data_len + SHIFT_TBL_SIZE + 4 >= sizeof(spk_key_buf))
> -		return -2;
> +	if (key_data_len + SHIFT_TBL_SIZE + 4 >= sizeof(spk_key_buf)) {
> +		pr_debug("too many key_infos (%d over %u)\n",
> +			 key_data_len + SHIFT_TBL_SIZE + 4, (unsigned int)(sizeof(spk_key_buf)));
> +		return -EINVAL;
> +	}
>  	memset(k_buffer, 0, SHIFT_TBL_SIZE);
>  	memset(spk_our_keys, 0, sizeof(spk_our_keys));
>  	spk_shift_table = k_buffer;
> @@ -1233,14 +1239,19 @@ int spk_set_key_info(const u_char *key_info, u_char *k_buffer)
>  	cp1 += 2;		/* now pointing at shift states */
>  	for (i = 1; i <= states; i++) {
>  		ch = *cp1++;
> -		if (ch >= SHIFT_TBL_SIZE)
> -			return -3;
> +		if (ch >= SHIFT_TBL_SIZE) {
> +			pr_debug("(%d) not valid shift state (max_allowed = %d)\n", ch,
> +				 SHIFT_TBL_SIZE);
> +			return -EINVAL;
> +		}
>  		spk_shift_table[ch] = i;
>  	}
>  	keymap_flags = *cp1++;
>  	while ((ch = *cp1)) {
> -		if (ch >= MAX_KEY)
> -			return -4;
> +		if (ch >= MAX_KEY) {
> +			pr_debug("(%d), not valid key, (max_allowed = %d)\n", ch, MAX_KEY);
> +			return -EINVAL;
> +		}
>  		spk_our_keys[ch] = cp1;
>  		cp1 += states + 1;
>  	}
> -- 
> 2.10.2
> 

-- 
Samuel
Progress (n.): The process through which the Internet has evolved from
smart people in front of dumb terminals to dumb people in front of smart
terminals.

Re: [PATCH SPEAKUP v2 3/3] use spkeaup_allocate as per required context

[Samuel Thibault]

Pranay Kr. Srivastava, on mar. 28 févr. 2017 13:57:55 +0530, wrote:
> speakup_allocate used GFP_ATOMIC for allocations
> even while during initialization due to it's use
> in notifier call.
> 
> Pass GFP_ flags as well to speakup_allocate depending
> on the context it is called in.
> 
> Signed-off-by: Pranay Kr. Srivastava <pranjas@gmail.com>

Reviewed-by: Samuel Thibault <samuel.thibault@ens-lyon.org>

> ---
>  drivers/staging/speakup/main.c | 8 ++++----
>  1 file changed, 4 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/staging/speakup/main.c b/drivers/staging/speakup/main.c
> index ca817ca..ede842e 100644
> --- a/drivers/staging/speakup/main.c
> +++ b/drivers/staging/speakup/main.c
> @@ -1327,14 +1327,14 @@ static int edit_bits(struct vc_data *vc, u_char type, u_char ch, u_short key)
>  }
>  
>  /* Allocation concurrency is protected by the console semaphore */
> -static int speakup_allocate(struct vc_data *vc)
> +static int speakup_allocate(struct vc_data *vc, gfp_t gfp_flags)
>  {
>  	int vc_num;
>  
>  	vc_num = vc->vc_num;
>  	if (speakup_console[vc_num] == NULL) {
>  		speakup_console[vc_num] = kzalloc(sizeof(*speakup_console[0]),
> -						  GFP_ATOMIC);
> +						  gfp_flags);
>  		if (speakup_console[vc_num] == NULL)
>  			return -ENOMEM;
>  		speakup_date(vc);
> @@ -2257,7 +2257,7 @@ static int vt_notifier_call(struct notifier_block *nb,
>  	switch (code) {
>  	case VT_ALLOCATE:
>  		if (vc->vc_mode == KD_TEXT)
> -			speakup_allocate(vc);
> +			speakup_allocate(vc, GFP_ATOMIC);
>  		break;
>  	case VT_DEALLOCATE:
>  		speakup_deallocate(vc);
> @@ -2343,7 +2343,7 @@ static int __init speakup_init(void)
>  
>  	for (i = 0; i < MAX_NR_CONSOLES; i++)
>  		if (vc_cons[i].d) {
> -			err = speakup_allocate(vc_cons[i].d);
> +			err = speakup_allocate(vc_cons[i].d, GFP_KERNEL);
>  			if (err)
>  				goto error_kobjects;
>  		}
> -- 
> 2.10.2
> 

-- 
Samuel
(03:13:14) <j> bon
(03:13:19) <j> il est tard :p
(03:13:25) <g> c'est l'heure de manger
(03:13:38) <j> hm j'ai mangé à 1h moi, j'ai des horaires raisonnables

Re: [PATCH SPEAKUP v2 2/3] remove unecessary initial allocation of vc

[Samuel Thibault]

Pranay Kr. Srivastava, on mar. 28 févr. 2017 13:57:54 +0530, wrote:
> This patch removes the unnecessary allocation of
> current foreground vc during initialization.
> 
> This initialization is already handled in the loop
> that follows it for all available virtual consoles.
> 
> Signed-off-by: Pranay Kr. Srivastava <pranjas@gmail.com>

Reviewed-by: Samuel Thibault <samuel.thibault@ens-lyon.org>

> ---
>  drivers/staging/speakup/main.c | 11 -----------
>  1 file changed, 11 deletions(-)
> 
> diff --git a/drivers/staging/speakup/main.c b/drivers/staging/speakup/main.c
> index a1d5b66..ca817ca 100644
> --- a/drivers/staging/speakup/main.c
> +++ b/drivers/staging/speakup/main.c
> @@ -2317,7 +2317,6 @@ static int __init speakup_init(void)
>  {
>  	int i;
>  	long err = 0;
> -	struct st_spk_t *first_console;
>  	struct vc_data *vc = vc_cons[fg_console].d;
>  	struct var_t *var;
>  
> @@ -2342,15 +2341,6 @@ static int __init speakup_init(void)
>  	if (err)
>  		goto error_virtkeyboard;
>  
> -	first_console = kzalloc(sizeof(*first_console), GFP_KERNEL);
> -	if (!first_console) {
> -		err = -ENOMEM;
> -		goto error_alloc;
> -	}
> -
> -	speakup_console[vc->vc_num] = first_console;
> -	speakup_date(vc);
> -
>  	for (i = 0; i < MAX_NR_CONSOLES; i++)
>  		if (vc_cons[i].d) {
>  			err = speakup_allocate(vc_cons[i].d);
> @@ -2412,7 +2402,6 @@ static int __init speakup_init(void)
>  	for (i = 0; i < MAX_NR_CONSOLES; i++)
>  		kfree(speakup_console[i]);
>  
> -error_alloc:
>  	speakup_remove_virtual_keyboard();
>  
>  error_virtkeyboard:
> -- 
> 2.10.2
> 

-- 
Samuel
<i> ben oui ce serait idiot, mais osb
  -+- m'en fous de faire un truc débile ! -+-

Re: [PATCH SPEAKUP v2 1/3] return same error value from spk_set_key_info

[Greg KH]

On Tue, Feb 28, 2017 at 01:57:53PM +0530, Pranay Kr. Srivastava wrote:
> This patch makes spk_set_key_info return -EINVAL
> in case of failure instead of returning 4 different
> values for the type of error that occurred.
> 
> Print the offending values instead as debug message.
> 
> Signed-off-by: Pranay Kr. Srivastava <pranjas@gmail.com>
> ---
>  drivers/staging/speakup/main.c | 27 +++++++++++++++++++--------
>  1 file changed, 19 insertions(+), 8 deletions(-)

Please fix up your subject line to contain the subsystem and driver it
is modifying.  For example, this message should read:
	Subject: staging: speakup: return same error value from spk_set_key_info

Same thing goes for all 3 messages in this series.

Please fix up, keep the Ack, and resend.

thanks,

greg k-h

Re: [PATCH SPEAKUP v2 3/3] use spkeaup_allocate as per required context

[Greg KH]

On Tue, Feb 28, 2017 at 01:57:55PM +0530, Pranay Kr. Srivastava wrote:
> speakup_allocate used GFP_ATOMIC for allocations
> even while during initialization due to it's use
> in notifier call.
> 
> Pass GFP_ flags as well to speakup_allocate depending
> on the context it is called in.
> 
> Signed-off-by: Pranay Kr. Srivastava <pranjas@gmail.com>
> ---
>  drivers/staging/speakup/main.c | 8 ++++----
>  1 file changed, 4 insertions(+), 4 deletions(-)

Your subject line contains a misspelling :(

[PATCH staging/speakup v3 0/3] cleanup error and initilization

[Pranay Kr. Srivastava]

Changelog from v2:
	Fixed the message subject line.

Changelog from v1:
	1. fixed kbuild warning for i386 build as reported by kbuild robot
	2. split initialization code in two patches.



Pranay Kr. Srivastava (3):
  return same error value from spk_set_key_info
  remove unecessary initial allocation of vc
  use speakup_allocate as per required context

 drivers/staging/speakup/main.c | 46 +++++++++++++++++++++---------------------
 1 file changed, 23 insertions(+), 23 deletions(-)

-- 
2.10.2

[PATCH staging/speakup v3 1/3] return same error value from spk_set_key_info

[Pranay Kr. Srivastava]

This patch makes spk_set_key_info return -EINVAL
in case of failure instead of returning 4 different
values for the type of error that occurred.

Print the offending values instead as debug message.

Signed-off-by: Pranay Kr. Srivastava <pranjas@gmail.com>
---
 drivers/staging/speakup/main.c | 27 +++++++++++++++++++--------
 1 file changed, 19 insertions(+), 8 deletions(-)

diff --git a/drivers/staging/speakup/main.c b/drivers/staging/speakup/main.c
index c2f70ef..a1d5b66 100644
--- a/drivers/staging/speakup/main.c
+++ b/drivers/staging/speakup/main.c
@@ -1216,13 +1216,19 @@ int spk_set_key_info(const u_char *key_info, u_char *k_buffer)
 	u_char ch, version, num_keys;
 
 	version = *cp++;
-	if (version != KEY_MAP_VER)
-		return -1;
+	if (version != KEY_MAP_VER) {
+		pr_debug("version found %d should be %d\n",
+			 version, KEY_MAP_VER);
+		return -EINVAL;
+	}
 	num_keys = *cp;
 	states = (int)cp[1];
 	key_data_len = (states + 1) * (num_keys + 1);
-	if (key_data_len + SHIFT_TBL_SIZE + 4 >= sizeof(spk_key_buf))
-		return -2;
+	if (key_data_len + SHIFT_TBL_SIZE + 4 >= sizeof(spk_key_buf)) {
+		pr_debug("too many key_infos (%d over %u)\n",
+			 key_data_len + SHIFT_TBL_SIZE + 4, (unsigned int)(sizeof(spk_key_buf)));
+		return -EINVAL;
+	}
 	memset(k_buffer, 0, SHIFT_TBL_SIZE);
 	memset(spk_our_keys, 0, sizeof(spk_our_keys));
 	spk_shift_table = k_buffer;
@@ -1233,14 +1239,19 @@ int spk_set_key_info(const u_char *key_info, u_char *k_buffer)
 	cp1 += 2;		/* now pointing at shift states */
 	for (i = 1; i <= states; i++) {
 		ch = *cp1++;
-		if (ch >= SHIFT_TBL_SIZE)
-			return -3;
+		if (ch >= SHIFT_TBL_SIZE) {
+			pr_debug("(%d) not valid shift state (max_allowed = %d)\n", ch,
+				 SHIFT_TBL_SIZE);
+			return -EINVAL;
+		}
 		spk_shift_table[ch] = i;
 	}
 	keymap_flags = *cp1++;
 	while ((ch = *cp1)) {
-		if (ch >= MAX_KEY)
-			return -4;
+		if (ch >= MAX_KEY) {
+			pr_debug("(%d), not valid key, (max_allowed = %d)\n", ch, MAX_KEY);
+			return -EINVAL;
+		}
 		spk_our_keys[ch] = cp1;
 		cp1 += states + 1;
 	}
-- 
2.10.2

[PATCH staging/speakup v3 2/3] remove unnecessary initial allocation of vc

[Pranay Kr. Srivastava]

This patch removes the unnecessary allocation of
current foreground vc during initialization.

This initialization is already handled in the loop
that follows it for all available virtual consoles.

Signed-off-by: Pranay Kr. Srivastava <pranjas@gmail.com>
Reviewed-by: Samuel Thibault <samuel.thibault@ens-lyon.org>
---
 drivers/staging/speakup/main.c | 11 -----------
 1 file changed, 11 deletions(-)

diff --git a/drivers/staging/speakup/main.c b/drivers/staging/speakup/main.c
index a1d5b66..ca817ca 100644
--- a/drivers/staging/speakup/main.c
+++ b/drivers/staging/speakup/main.c
@@ -2317,7 +2317,6 @@ static int __init speakup_init(void)
 {
 	int i;
 	long err = 0;
-	struct st_spk_t *first_console;
 	struct vc_data *vc = vc_cons[fg_console].d;
 	struct var_t *var;
 
@@ -2342,15 +2341,6 @@ static int __init speakup_init(void)
 	if (err)
 		goto error_virtkeyboard;
 
-	first_console = kzalloc(sizeof(*first_console), GFP_KERNEL);
-	if (!first_console) {
-		err = -ENOMEM;
-		goto error_alloc;
-	}
-
-	speakup_console[vc->vc_num] = first_console;
-	speakup_date(vc);
-
 	for (i = 0; i < MAX_NR_CONSOLES; i++)
 		if (vc_cons[i].d) {
 			err = speakup_allocate(vc_cons[i].d);
@@ -2412,7 +2402,6 @@ static int __init speakup_init(void)
 	for (i = 0; i < MAX_NR_CONSOLES; i++)
 		kfree(speakup_console[i]);
 
-error_alloc:
 	speakup_remove_virtual_keyboard();
 
 error_virtkeyboard:
-- 
2.10.2

[PATCH staging/speakup v3 3/3] use speakup_allocate as per required context

[Pranay Kr. Srivastava]

speakup_allocate used GFP_ATOMIC for allocations
even while during initialization due to it's use
in notifier call.

Pass GFP_ flags as well to speakup_allocate depending
on the context it is called in.

Signed-off-by: Pranay Kr. Srivastava <pranjas@gmail.com>
---
 drivers/staging/speakup/main.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/staging/speakup/main.c b/drivers/staging/speakup/main.c
index ca817ca..ede842e 100644
--- a/drivers/staging/speakup/main.c
+++ b/drivers/staging/speakup/main.c
@@ -1327,14 +1327,14 @@ static int edit_bits(struct vc_data *vc, u_char type, u_char ch, u_short key)
 }
 
 /* Allocation concurrency is protected by the console semaphore */
-static int speakup_allocate(struct vc_data *vc)
+static int speakup_allocate(struct vc_data *vc, gfp_t gfp_flags)
 {
 	int vc_num;
 
 	vc_num = vc->vc_num;
 	if (speakup_console[vc_num] == NULL) {
 		speakup_console[vc_num] = kzalloc(sizeof(*speakup_console[0]),
-						  GFP_ATOMIC);
+						  gfp_flags);
 		if (speakup_console[vc_num] == NULL)
 			return -ENOMEM;
 		speakup_date(vc);
@@ -2257,7 +2257,7 @@ static int vt_notifier_call(struct notifier_block *nb,
 	switch (code) {
 	case VT_ALLOCATE:
 		if (vc->vc_mode == KD_TEXT)
-			speakup_allocate(vc);
+			speakup_allocate(vc, GFP_ATOMIC);
 		break;
 	case VT_DEALLOCATE:
 		speakup_deallocate(vc);
@@ -2343,7 +2343,7 @@ static int __init speakup_init(void)
 
 	for (i = 0; i < MAX_NR_CONSOLES; i++)
 		if (vc_cons[i].d) {
-			err = speakup_allocate(vc_cons[i].d);
+			err = speakup_allocate(vc_cons[i].d, GFP_KERNEL);
 			if (err)
 				goto error_kobjects;
 		}
-- 
2.10.2

Re: [PATCH staging/speakup v3 3/3] use speakup_allocate as per required context

[Greg KH]

On Tue, Mar 21, 2017 at 12:40:24PM +0530, Pranay Kr. Srivastava wrote:
> speakup_allocate used GFP_ATOMIC for allocations
> even while during initialization due to it's use
> in notifier call.
> 
> Pass GFP_ flags as well to speakup_allocate depending
> on the context it is called in.
> 
> Signed-off-by: Pranay Kr. Srivastava <pranjas@gmail.com>
> ---
>  drivers/staging/speakup/main.c | 8 ++++----
>  1 file changed, 4 insertions(+), 4 deletions(-)

This patch didn't apply to my staging-testing branch, but the 2 others
did, odd.  Can you rebase it and resend?

thanks,

greg k-h

[[RESEND]PATCH staging/speakup v3 3/3] use speakup_allocate as per required context

[Pranay Kr. Srivastava]

speakup_allocate used GFP_ATOMIC for allocations
even while during initialization due to it's use
in notifier call.

Pass GFP_ flags as well to speakup_allocate depending
on the context it is called in.

Signed-off-by: Pranay Kr. Srivastava <pranjas@gmail.com>
---
 drivers/staging/speakup/main.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/staging/speakup/main.c b/drivers/staging/speakup/main.c
index 2db3f06..b811c86 100644
--- a/drivers/staging/speakup/main.c
+++ b/drivers/staging/speakup/main.c
@@ -1341,14 +1341,14 @@ static int edit_bits(struct vc_data *vc, u_char type, u_char ch, u_short key)
 }
 
 /* Allocation concurrency is protected by the console semaphore */
-static int speakup_allocate(struct vc_data *vc)
+static int speakup_allocate(struct vc_data *vc, gfp_t gfp_flags)
 {
 	int vc_num;
 
 	vc_num = vc->vc_num;
 	if (speakup_console[vc_num] == NULL) {
 		speakup_console[vc_num] = kzalloc(sizeof(*speakup_console[0]),
-						  GFP_ATOMIC);
+						  gfp_flags);
 		if (!speakup_console[vc_num])
 			return -ENOMEM;
 		speakup_date(vc);
@@ -2277,7 +2277,7 @@ static int vt_notifier_call(struct notifier_block *nb,
 	switch (code) {
 	case VT_ALLOCATE:
 		if (vc->vc_mode == KD_TEXT)
-			speakup_allocate(vc);
+			speakup_allocate(vc, GFP_ATOMIC);
 		break;
 	case VT_DEALLOCATE:
 		speakup_deallocate(vc);
@@ -2362,7 +2362,7 @@ static int __init speakup_init(void)
 
 	for (i = 0; i < MAX_NR_CONSOLES; i++)
 		if (vc_cons[i].d) {
-			err = speakup_allocate(vc_cons[i].d);
+			err = speakup_allocate(vc_cons[i].d, GFP_KERNEL);
 			if (err)
 				goto error_kobjects;
 		}
-- 
2.10.2

Re: [[RESEND]PATCH staging/speakup v3 3/3] use speakup_allocate as per required context

[Greg KH]

On Fri, Mar 24, 2017 at 02:07:11PM +0530, Pranay Kr. Srivastava wrote:
> speakup_allocate used GFP_ATOMIC for allocations
> even while during initialization due to it's use
> in notifier call.

Is that a problem?

> Pass GFP_ flags as well to speakup_allocate depending
> on the context it is called in.

At init, we should be fine to use GFP_ATOMIC, so is this change really
needed?

thanks,

greg k-h

Re: [[RESEND]PATCH staging/speakup v3 3/3] use speakup_allocate as per required context

[Pranay Srivastava]

On Fri, Mar 24, 2017 at 2:13 PM, Greg KH <greg@kroah.com> wrote:
> On Fri, Mar 24, 2017 at 02:07:11PM +0530, Pranay Kr. Srivastava wrote:
>> speakup_allocate used GFP_ATOMIC for allocations
>> even while during initialization due to it's use
>> in notifier call.
>
> Is that a problem?

No that's the way it should be. I was just trying to say that allocation
should be context based[?]. If we can be lenient then that's better[?]

>
>> Pass GFP_ flags as well to speakup_allocate depending
>> on the context it is called in.
>
> At init, we should be fine to use GFP_ATOMIC, so is this change really
> needed?
>
> thanks,
>
> greg k-h



-- 
        ---P.K.S