Re: [PATCH 1/1] netstamp_needed shouldn't be jump_label_key

[Eric Dumazet <eric.dumazet@gmail.com>]

Le mardi 29 novembre 2011 à 10:03 +0100, "Igor Maravić" a écrit :
> >
> > Could you test following patch ?
> >
> 
> I've just put it to compile. I'l let you know soon as it finishes compiling.
> 
> I have one proposition for your patch. To put WARN_ON before in ifdef endif,
> because we don't needed if we don't have HAVE_JUMP_LABEL, and also to check it
> before using jump_label_dec.
> 
> Also I have few questions :)
> 
> First - why can't we use spin_lock on jump_label?
> 
> I know that You said that is because we are using mutex_lock in
> arch_jump_label_transform() or text_poke_smp(), but I don't see why couldn't
> we use mutex_lock inside spin_lock.
> 


Try it, you'll see how bad it is.

> To me using spin_lock_irqsave and spin_lock_irqrestore does sound like as most logical solution.
> With that lock, we would be sure that our mutex_lock in arch_jump_label_transform() or text_poke_smp()
> isn't going to be interrupted.
> Please correct if I'm wrong.
> 
> Also I saw that you put calling of net_enable_timestamp outside of spin_lock_bh
> Why?
> 

Because its the same problem. We want to be allowed to sleep.

> BR
> Igor
> 
> [PATCH 1/1] jump_label warning if_interrupt
> 
> Move warning on the top of function net_enable_timestamp,
> so we would be also warn if we are going to jump_label_dec in interrupt
> 
> Signed-off-by: Igor Maravic <igorm@etf.rs>
> 
> :100644 100644 45eab03... ef23cf7... M	net/core/dev.c
> 
> diff --git a/net/core/dev.c b/net/core/dev.c
> index 45eab03..ef23cf7 100644
> --- a/net/core/dev.c
> +++ b/net/core/dev.c
> @@ -1453,6 +1453,7 @@ void net_enable_timestamp(void)
>  {
>  #ifdef HAVE_JUMP_LABEL
>  	int deferred = atomic_xchg(&netstamp_needed_deferred, 0);
> +	WARN_ON(in_interrupt());
> 
>  	if (deferred) {
>  		while (--deferred)
> @@ -1460,7 +1461,6 @@ void net_enable_timestamp(void)
>  		return;
>  	}
>  #endif
> -	WARN_ON(in_interrupt());
>  	jump_label_inc(&netstamp_needed);
>  }
>  EXPORT_SYMBOL(net_enable_timestamp);

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