Re: [PATCH v3] um: net: replace GFP_KERNEL with GFP_ATOMIC when spinlock is held

[Joe Perches <joe@perches.com>]

On Thu, 2015-10-29 at 14:46 +0530, Saurabh Sengar wrote:
> replace GFP_KERNEL with GFP_ATOMIC while spinlock is held,
> as code while holding a spinlock should be atomic.
> GFP_KERNEL may sleep and can cause deadlock,
> where as GFP_ATOMIC may fail but certainly avoids deadlock
> 
> Signed-off-by: Saurabh Sengar <saurabh.truth@gmail.com>
> ---
> v3: removed the atomic variable, as per Richard comment

Trivia: You could remove the gfp_mask variables too
	and just use GFP_KERNEL and GFP_ATOMIC directly.

> diff --git a/arch/um/drivers/net_kern.c b/arch/um/drivers/net_kern.c
[]
> @@ -600,20 +600,22 @@ void register_transport(struct transport *new)
>  	void *init;
>  	char *mac = NULL;
>  	int match;
> +	int gfp_mask;
>  
>  	spin_lock(&transports_lock);
>  	BUG_ON(!list_empty(&new->list));
>  	list_add(&new->list, &transports);
>  	spin_unlock(&transports_lock);
>  
> +	gfp_mask = GFP_KERNEL;
>  	list_for_each_safe(ele, next, &eth_cmd_line) {
>  		eth = list_entry(ele, struct eth_init, list);
>  		match = check_transport(new, eth->init, eth->index, &init,
> -					&mac);
> +					&mac, gfp_mask);
>  		if (!match)
>  			continue;
>  		else if (init != NULL) {
> -			eth_configure(eth->index, init, mac, new);
> +			eth_configure(eth->index, init, mac, new, gfp_mask);
>  			kfree(init);
>  		}
>  		list_del(&eth->list);
> @@ -627,14 +629,17 @@ static int eth_setup_common(char *str, int index)
>  	void *init;
>  	char *mac = NULL;
>  	int found = 0;
> +	int gfp_mask;
>  
>  	spin_lock(&transports_lock);
> +	gfp_mask = GFP_ATOMIC;
>  	list_for_each(ele, &transports) {
>  		transport = list_entry(ele, struct transport, list);
> -	        if (!check_transport(transport, str, index, &init, &mac))
> +		if (!check_transport(transport, str, index, &init,
> +							&mac, gfp_mask))
>  			continue;
>  		if (init != NULL) {
> -			eth_configure(index, init, mac, transport);
> +			eth_configure(index, init, mac, transport, gfp_mask);
>  			kfree(init);
>  		}
>  		found = 1;