Re: Longstanding networking / SMP issue? (duplextest)

[Christoph Hellwig <hch@infradead.org>]

On Sun, Feb 23, 2003 at 01:12:17AM -0800, David S. Miller wrote:
> +static struct socket *__icmp_socket[NR_CPUS];
> +#define icmp_socket	__icmp_socket[smp_processor_id()]

This should be per-cpu data

> -static __inline__ int icmp_xmit_lock(void)
> +static __inline__ void icmp_xmit_lock(void)
>  {
> -	int ret;
>  	local_bh_disable();
> -	ret = icmp_xmit_lock_bh();
> -	if (ret)
> -		local_bh_enable();
> -	return ret;
> -}
>  
> -static void icmp_xmit_unlock_bh(void)
> -{
> -	icmp_xmit_holder = -1;
> -	spin_unlock(&icmp_socket->sk->lock.slock);
> +	if (!spin_trylock(&icmp_socket->sk->lock.slock))
> +		BUG();

unlikely()?

> -static __inline__ void icmp_xmit_unlock(void)
> +static void icmp_xmit_unlock(void)
>  {
> -	icmp_xmit_unlock_bh();
> +	spin_unlock(&icmp_socket->sk->lock.slock);
>  	local_bh_enable();

spin_unlock_bh

> +	icmp_xmit_lock();

Hmm, and I guess the code would be much more readable if you used
the spin_lock call directly.  The impliclit icmp_socket doesn't
really help readability either.