[PATCH-rt 1/1] Fix spinlock issue in net/core/sock.c

[PATCH-rt 1/1] Fix spinlock issue in net/core/sock.c

[Vivek Satpute]

Kernel panic's and reboot while doing network operations such ifconfig
and ping on MIPS architecture after RT-patches applied.

In case of CONFIG_PREEMPT_RT, after releasing the lock with
"spin_unlock", context switch might occur before enabling the bottom
half with local_bh_enable and this causes the kernel to panic.
The issue is resolved by releasing the lock afer acquiring the mutex
using spin_unlock_bh.

Tested the fix on MIPS and X86 architecture.

Signed-off-by: Vivek Satpute <vivek@linsyssoft.com>
---
net/core/sock.c |    8 ++++++++
1 file changed, 8 insertions(+)

Index: linux-2.6.28.4/net/core/sock.c
===================================================================
--- linux-2.6.28.4.orig/net/core/sock.c
+++ linux-2.6.28.4/net/core/sock.c
@@ -1752,12 +1752,20 @@ void lock_sock_nested(struct sock *sk, i
 	if (sk->sk_lock.owned)
 		__lock_sock(sk);
 	sk->sk_lock.owned = 1;
+
+#ifndef CONFIG_PREEMPT_RT
 	spin_unlock(&sk->sk_lock.slock);
+#endif
 	/*
 	 * The sk_lock has mutex_lock() semantics here:
 	 */
 	mutex_acquire(&sk->sk_lock.dep_map, subclass, 0, _RET_IP_);
+
+#ifdef CONFIG_PREEMPT_RT
+	spin_unlock_bh(&sk->sk_lock.slock);
+#else
 	local_bh_enable();
+#endif
 }
 
 EXPORT_SYMBOL(lock_sock_nested);




Thanks and Regards,
Vivek Satpute
System Software Engineer
LinSysSoft Technologies, Pune

Re: [PATCH-rt 1/1] Fix spinlock issue in net/core/sock.c

[Jan Blunck]

On Thu, Jun 25, 2009 at 2:07 PM, Vivek Satpute<vivek@linsyssoft.com> wrote:
> Kernel panic's and reboot while doing network operations such ifconfig
> and ping on MIPS architecture after RT-patches applied.
>
> In case of CONFIG_PREEMPT_RT, after releasing the lock with
> "spin_unlock", context switch might occur before enabling the bottom
> half with local_bh_enable and this causes the kernel to panic.
> The issue is resolved by releasing the lock afer acquiring the mutex
> using spin_unlock_bh.
>
> Tested the fix on MIPS and X86 architecture.
>
> Signed-off-by: Vivek Satpute <vivek@linsyssoft.com>
> ---
> net/core/sock.c |    8 ++++++++
> 1 file changed, 8 insertions(+)
>
> Index: linux-2.6.28.4/net/core/sock.c
> ===================================================================
> --- linux-2.6.28.4.orig/net/core/sock.c
> +++ linux-2.6.28.4/net/core/sock.c
> @@ -1752,12 +1752,20 @@ void lock_sock_nested(struct sock *sk, i
>        if (sk->sk_lock.owned)
>                __lock_sock(sk);
>        sk->sk_lock.owned = 1;
> +
> +#ifndef CONFIG_PREEMPT_RT
>        spin_unlock(&sk->sk_lock.slock);
> +#endif
>        /*
>         * The sk_lock has mutex_lock() semantics here:
>         */
>        mutex_acquire(&sk->sk_lock.dep_map, subclass, 0, _RET_IP_);
> +
> +#ifdef CONFIG_PREEMPT_RT
> +       spin_unlock_bh(&sk->sk_lock.slock);
> +#else
>        local_bh_enable();
> +#endif
>  }
>

I don't think you should move the unlock after the mutex_acquire(). I
guess that this will lead to false positive lockdep warnings.

Anyway, I wonder why using spin_unlock_bh() is fixing the problem that
you are having. Do you have more context about the problem or maybe an
Oops or so?

Thanks,
Jan

>  EXPORT_SYMBOL(lock_sock_nested);
>
>
>
>
> Thanks and Regards,
> Vivek Satpute
> System Software Engineer
> LinSysSoft Technologies, Pune
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-rt-users" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>
--
To unsubscribe from this list: send the line "unsubscribe linux-rt-users" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH-rt 1/1] Fix spinlock issue in net/core/sock.c

[Vivek Satpute]

Hi Jan,

(Sending mail again as first mail to mailing-list got bounced due to
Rich Text Format)

On Mon, Jun 29, 2009 at 5:27 PM, Jan Blunck <jblunck@gmail.com> wrote:
>
> On Thu, Jun 25, 2009 at 2:07 PM, Vivek Satpute<vivek@linsyssoft.com> wrote:
> > Kernel panic's and reboot while doing network operations such ifconfig
> > and ping on MIPS architecture after RT-patches applied.
> >
> > In case of CONFIG_PREEMPT_RT, after releasing the lock with
> > "spin_unlock", context switch might occur before enabling the bottom
> > half with local_bh_enable and this causes the kernel to panic.
> > The issue is resolved by releasing the lock afer acquiring the mutex
> > using spin_unlock_bh.
> >
> > Tested the fix on MIPS and X86 architecture.
> >
> > Signed-off-by: Vivek Satpute <vivek@linsyssoft.com>
> > ---
> > net/core/sock.c |    8 ++++++++
> > 1 file changed, 8 insertions(+)
> >
> > Index: linux-2.6.28.4/net/core/sock.c
> > ===================================================================
> > --- linux-2.6.28.4.orig/net/core/sock.c
> > +++ linux-2.6.28.4/net/core/sock.c
> > @@ -1752,12 +1752,20 @@ void lock_sock_nested(struct sock *sk, i
> >        if (sk->sk_lock.owned)
> >                __lock_sock(sk);
> >        sk->sk_lock.owned = 1;
> > +
> > +#ifndef CONFIG_PREEMPT_RT
> >        spin_unlock(&sk->sk_lock.slock);
> > +#endif
> >        /*
> >         * The sk_lock has mutex_lock() semantics here:
> >         */
> >        mutex_acquire(&sk->sk_lock.dep_map, subclass, 0, _RET_IP_);
> > +
> > +#ifdef CONFIG_PREEMPT_RT
> > +       spin_unlock_bh(&sk->sk_lock.slock);
> > +#else
> >        local_bh_enable();
> > +#endif
> >  }
> >
>
> I don't think you should move the unlock after the mutex_acquire(). I
> guess that this will lead to false positive lockdep warnings.
>
> Anyway, I wonder why using spin_unlock_bh() is fixing the problem that
> you are having. Do you have more context about the problem or maybe an
> Oops or so?

On firing command "ifconfig", I get following messages:

--------------------------------<snip>--------------------------------
# ifconfig
Kernel panic - not syncing: Aiee, killing interrupt handler!
Rebooting in 1 seconds..
--------------------------------<snip>--------------------------------


If kernel compiled with Lock Debugging options then it gives following
call trace:

--------------------------------<snip>--------------------------------
Call Trace:
[<80111768>] dump_stack+0x8/0x34
[<8012e9a0>] warn_on_slowpath+0x60/0x88
[<80135b48>] local_bh_enable+0x40/0x11c
[<8035d05c>] lock_sock_nested+0xf8/0x11c
[<803bbba8>] inet_bind+0x100/0x210
[<803d7888>] xs_bind4+0x70/0x158
[<803d9ac0>] xs_udp_connect_worker4+0x120/0x1a4
[<801440f8>] run_workqueue+0x1d0/0x264
[<801450ac>] worker_thread+0x7c/0xec
[<80148f70>] kthread+0x58/0x94
[<8010cc2c>] kernel_thread_helper+0x10/0x18
--------------------------------<snip>--------------------------------


>
> Thanks,
> Jan
>
> >  EXPORT_SYMBOL(lock_sock_nested);
> >
> >
> >
> >
> > Thanks and Regards,
> > Vivek Satpute
> > System Software Engineer
> > LinSysSoft Technologies, Pune
> >
> > --
> > To unsubscribe from this list: send the line "unsubscribe linux-rt-users" in
> > the body of a message to majordomo@vger.kernel.org
> > More majordomo info at  http://vger.kernel.org/majordomo-info.html
> >

Thanks and Regards,
Vivek Satpute
System Software Engineer
LinSysSoft Technologies, Pune
--
To unsubscribe from this list: send the line "unsubscribe linux-rt-users" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH-rt 1/1] Fix spinlock issue in net/core/sock.c

[Vivek Satpute]

Hi,

I want to keep this thread alive. Has anyone encountered/tested this
issue ?

Thanks and Regards,
Vivek Satpute
System Software Engineer
LinSysSoft Technologies, Pune


On Thu, 2009-06-25 at 17:37 +0530, Vivek Satpute wrote:
> Kernel panic's and reboot while doing network operations such ifconfig
> and ping on MIPS architecture after RT-patches applied.
> 
> In case of CONFIG_PREEMPT_RT, after releasing the lock with
> "spin_unlock", context switch might occur before enabling the bottom
> half with local_bh_enable and this causes the kernel to panic.
> The issue is resolved by releasing the lock afer acquiring the mutex
> using spin_unlock_bh.
> 
> Tested the fix on MIPS and X86 architecture.
> 
> Signed-off-by: Vivek Satpute <vivek@linsyssoft.com>
> ---
> net/core/sock.c |    8 ++++++++
> 1 file changed, 8 insertions(+)
> 
> Index: linux-2.6.28.4/net/core/sock.c
> ===================================================================
> --- linux-2.6.28.4.orig/net/core/sock.c
> +++ linux-2.6.28.4/net/core/sock.c
> @@ -1752,12 +1752,20 @@ void lock_sock_nested(struct sock *sk, i
>  	if (sk->sk_lock.owned)
>  		__lock_sock(sk);
>  	sk->sk_lock.owned = 1;
> +
> +#ifndef CONFIG_PREEMPT_RT
>  	spin_unlock(&sk->sk_lock.slock);
> +#endif
>  	/*
>  	 * The sk_lock has mutex_lock() semantics here:
>  	 */
>  	mutex_acquire(&sk->sk_lock.dep_map, subclass, 0, _RET_IP_);
> +
> +#ifdef CONFIG_PREEMPT_RT
> +	spin_unlock_bh(&sk->sk_lock.slock);
> +#else
>  	local_bh_enable();
> +#endif
>  }
>  
>  EXPORT_SYMBOL(lock_sock_nested);
> 
> 
> 
> 
> Thanks and Regards,
> Vivek Satpute
> System Software Engineer
> LinSysSoft Technologies, Pune
>