Patchwork [1/1] netstamp_needed shouldn't be jump_label_key

login
register
mail settings
Submitter Igor Maravić
Date Nov. 29, 2011, 9:48 a.m.
Message ID <88caa73892721f74b81e72c1ff1a0be3.squirrel@kondor.etf.bg.ac.rs>
Download mbox | patch
Permalink /patch/128268/
State Changes Requested
Delegated to: David Miller
Headers show

Comments

Igor Maravić - Nov. 29, 2011, 9:48 a.m.
> Its better to have coverage of the test, even on machines with no
> HAVE_JUMP_LABEL.
>
> So move the test before the
> #ifdef HAVE_JUMP_LABEL
>

Here is it.

PS. Tested the patch - it works.

[PATCH 1/1] Warning if_interrupt is true

Move warning on the top of function net_enable_timestamp,
so we would be also warn if we are going to use jump_label_dec in interrupt

Signed-off-by: Igor Maravic <igorm@etf.rs>

:100644 100644 45eab03... d358088... M	net/core/dev.c
Eric Dumazet - Nov. 29, 2011, 10:27 a.m.
Le mardi 29 novembre 2011 à 10:48 +0100, "Igor Maravić" a écrit :

> 
> PS. Tested the patch - it works.
> 
> [PATCH 1/1] Warning if_interrupt is true

Few remarks, since you start to send official patches ;)

- Since its not a serie of several patches, you dont need the 1/1
- Since tree is net-next, not net, you should say it to David
- We need a better title, with subsystem: tag, for example

Here, it can be "net:"

[PATCH net-next] net: check net_enable_timestamp() is not called in irq context

> 
> Move warning on the top of function net_enable_timestamp,
> so we would be also warn if we are going to use jump_label_dec in interrupt
> 
> Signed-off-by: Igor Maravic <igorm@etf.rs>
> 
> :100644 100644 45eab03... d358088... M	net/core/dev.c
> 

Missing "---" separator

Please carefully read Documentation/SubmittingPatches

> diff --git a/net/core/dev.c b/net/core/dev.c
> index 45eab03..d358088 100644
> --- a/net/core/dev.c
> +++ b/net/core/dev.c
> @@ -1451,16 +1451,18 @@ static atomic_t netstamp_needed_deferred;
> 
>  void net_enable_timestamp(void)
>  {
> +	WARN_ON(in_interrupt());
>  #ifdef HAVE_JUMP_LABEL
> -	int deferred = atomic_xchg(&netstamp_needed_deferred, 0);
> -
> -	if (deferred) {
> -		while (--deferred)
> -			jump_label_dec(&netstamp_needed);
> -		return;
> +	{
> +		int deferred = atomic_xchg(&netstamp_needed_deferred, 0);
> +
> +		if (deferred) {
> +			while (--deferred)
> +				jump_label_dec(&netstamp_needed);
> +			return;
> +		}
>  	}
>  #endif
> -	WARN_ON(in_interrupt());
>  	jump_label_inc(&netstamp_needed);
>  }
>  EXPORT_SYMBOL(net_enable_timestamp);


--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Igor Maravić - Nov. 29, 2011, 10:32 a.m.
Sorry,
just send another without using your remarks...
Thaks
Will listen to you in future.
BR
Igor

2011/11/29 Eric Dumazet <eric.dumazet@gmail.com>:
> Le mardi 29 novembre 2011 à 10:48 +0100, "Igor Maravić" a écrit :
>
>>
>> PS. Tested the patch - it works.
>>
>> [PATCH 1/1] Warning if_interrupt is true
>
> Few remarks, since you start to send official patches ;)
>
> - Since its not a serie of several patches, you dont need the 1/1
> - Since tree is net-next, not net, you should say it to David
> - We need a better title, with subsystem: tag, for example
>
> Here, it can be "net:"
>
> [PATCH net-next] net: check net_enable_timestamp() is not called in irq context
>
>>
>> Move warning on the top of function net_enable_timestamp,
>> so we would be also warn if we are going to use jump_label_dec in interrupt
>>
>> Signed-off-by: Igor Maravic <igorm@etf.rs>
>>
>> :100644 100644 45eab03... d358088... M        net/core/dev.c
>>
>
> Missing "---" separator
>
> Please carefully read Documentation/SubmittingPatches
>
>> diff --git a/net/core/dev.c b/net/core/dev.c
>> index 45eab03..d358088 100644
>> --- a/net/core/dev.c
>> +++ b/net/core/dev.c
>> @@ -1451,16 +1451,18 @@ static atomic_t netstamp_needed_deferred;
>>
>>  void net_enable_timestamp(void)
>>  {
>> +     WARN_ON(in_interrupt());
>>  #ifdef HAVE_JUMP_LABEL
>> -     int deferred = atomic_xchg(&netstamp_needed_deferred, 0);
>> -
>> -     if (deferred) {
>> -             while (--deferred)
>> -                     jump_label_dec(&netstamp_needed);
>> -             return;
>> +     {
>> +             int deferred = atomic_xchg(&netstamp_needed_deferred, 0);
>> +
>> +             if (deferred) {
>> +                     while (--deferred)
>> +                             jump_label_dec(&netstamp_needed);
>> +                     return;
>> +             }
>>       }
>>  #endif
>> -     WARN_ON(in_interrupt());
>>       jump_label_inc(&netstamp_needed);
>>  }
>>  EXPORT_SYMBOL(net_enable_timestamp);
>
>
>
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Patch

diff --git a/net/core/dev.c b/net/core/dev.c
index 45eab03..d358088 100644
--- a/net/core/dev.c
+++ b/net/core/dev.c
@@ -1451,16 +1451,18 @@  static atomic_t netstamp_needed_deferred;

 void net_enable_timestamp(void)
 {
+	WARN_ON(in_interrupt());
 #ifdef HAVE_JUMP_LABEL
-	int deferred = atomic_xchg(&netstamp_needed_deferred, 0);
-
-	if (deferred) {
-		while (--deferred)
-			jump_label_dec(&netstamp_needed);
-		return;
+	{
+		int deferred = atomic_xchg(&netstamp_needed_deferred, 0);
+
+		if (deferred) {
+			while (--deferred)
+				jump_label_dec(&netstamp_needed);
+			return;
+		}
 	}
 #endif
-	WARN_ON(in_interrupt());
 	jump_label_inc(&netstamp_needed);
 }
 EXPORT_SYMBOL(net_enable_timestamp);