Re: Scheduling in interrupt BUG. [Patch]
On Mon, May 14, 2001 at 08:32:33AM -0400, Michal Ostrowski wrote: > Having looked at the code for locking sockets I am concerned that the > locking procedures for tcp may be wrong. __release_sock releases the > socket spinlock before calling sk->backlog_rcv() (== tcp_v4_do_rcv), > however the comments at the top of tcp_v4_do_rcv() assert that the > socket's spinlock is held (which is definitely not the case). > > Anybody care to comment on this? Looks ok for me. The user socket lock (lock.users>0) is held while __release_sock runs, which is also sufficient to protect it as all new packets will go into backlog. The spinlock comment only applies to bottom halves. -Andi - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: Scheduling in interrupt BUG. [Patch]
Marcell Gal writes:
> Hi,
>
> This patch solved the problem. Should be ready for inclusion in 2.4.
> No more 'Scheduling in interrupt' under those conditions.
> Thanx for the thoughts, solution and the amazing speed.
> You guys are doing a really great job!
>
Alexey pointed out a much nicer/correct/elegant/efficient
solution to this problem and I think that that's the way to go.
New patch below.
Michal Ostrowski
[EMAIL PROTECTED]
--- drivers/net/pppoe.c~Tue Mar 6 22:44:35 2001
+++ drivers/net/pppoe.c Mon May 14 14:10:49 2001
@@ -5,7 +5,7 @@
* PPPoE --- PPP over Ethernet (RFC 2516)
*
*
- * Version:0.6.5
+ * Version:0.6.6
*
* 030700 : Fixed connect logic to allow for disconnect.
* 270700 :Fixed potential SMP problems; we must protect against
@@ -19,6 +19,7 @@
* 051000 :Initialization cleanup.
* 00 :Fix recvmsg.
* 050101 :Fix PADT procesing.
+ * 140501 :Use pppoe_rcv_core to handle all backlog. (Alexey)
*
* Author: Michal Ostrowski <[EMAIL PROTECTED]>
* Contributors:
@@ -376,22 +377,6 @@
return ret;
}
-
-/
- *
- * Receive wrapper called in process context.
- *
- ***/
-int pppoe_backlog_rcv(struct sock *sk, struct sk_buff *skb)
-{
- lock_sock(sk);
- pppoe_rcv_core(sk, skb);
- release_sock(sk);
- return 0;
-}
-
-
-
/
*
* Receive a PPPoE Discovery frame.
@@ -481,7 +466,7 @@
sk->protocol = PX_PROTO_OE;
sk->family = PF_PPPOX;
- sk->backlog_rcv = pppoe_backlog_rcv;
+ sk->backlog_rcv = pppoe_rcv_core;
sk->next = NULL;
sk->pprev = NULL;
sk->state = PPPOX_NONE;
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: Scheduling in interrupt BUG. [Patch]
Hi,
This patch solved the problem. Should be ready for inclusion in 2.4.
No more 'Scheduling in interrupt' under those conditions.
Thanx for the thoughts, solution and the amazing speed.
You guys are doing a really great job!
I hope we can get the earlier mentioned NULL ptr in all_ppp_units list
straight
soon. (I have a simple workaround - the mentioned hash, that even improves
speed,
but I a real fix would be more satisfaction. The relevant part of
ppp_generic.c
is so simple that it's really strange it is not correct.. ).
thanx:
Cell
Michal Ostrowski wrote:
> Anybody care to comment on this?
> [EMAIL PROTECTED]
--- linuxold/drivers/net/pppoe.cMon May 14 22:06:44 2001
+++ linux/drivers/net/pppoe.c Mon May 14 22:11:25 2001
@@ -4,9 +4,9 @@
* PPPoX --- Generic PPP encapsulation socket family
* PPPoE --- PPP over Ethernet (RFC 2516)
*
*
- * Version:0.6.5
+ * Version:0.6.6
*
* 030700 : Fixed connect logic to allow for disconnect.
* 270700 :Fixed potential SMP problems; we must protect against
* simultaneous invocation of ppp_input
@@ -18,8 +18,9 @@
* in pppoe_release.
* 051000 :Initialization cleanup.
* 00 :Fix recvmsg.
* 050101 :Fix PADT procesing.
+ * 140501 :pppoe_backlog_rcv must call bh_lock_sock, not lock_sock.
*
* Author: Michal Ostrowski <[EMAIL PROTECTED]>
* Contributors:
* Arnaldo Carvalho de Melo <[EMAIL PROTECTED]>
@@ -383,11 +384,11 @@
*
***/
int pppoe_backlog_rcv(struct sock *sk, struct sk_buff *skb)
{
- lock_sock(sk);
+ bh_lock_sock(sk);
pppoe_rcv_core(sk, skb);
- release_sock(sk);
+ bh_unlock_sock(sk);
return 0;
}
--
You'll never see all the places, or read all the books, but fortunately,
they're not all recommended.
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: Scheduling in interrupt BUG. [Patch]
Marcell Gal writes:
Hi,
This patch solved the problem. Should be ready for inclusion in 2.4.
No more 'Scheduling in interrupt' under those conditions.
Thanx for the thoughts, solution and the amazing speed.
You guys are doing a really great job!
Alexey pointed out a much nicer/correct/elegant/efficient
solution to this problem and I think that that's the way to go.
New patch below.
Michal Ostrowski
[EMAIL PROTECTED]
--- drivers/net/pppoe.c~Tue Mar 6 22:44:35 2001
+++ drivers/net/pppoe.c Mon May 14 14:10:49 2001
@@ -5,7 +5,7 @@
* PPPoE --- PPP over Ethernet (RFC 2516)
*
*
- * Version:0.6.5
+ * Version:0.6.6
*
* 030700 : Fixed connect logic to allow for disconnect.
* 270700 :Fixed potential SMP problems; we must protect against
@@ -19,6 +19,7 @@
* 051000 :Initialization cleanup.
* 00 :Fix recvmsg.
* 050101 :Fix PADT procesing.
+ * 140501 :Use pppoe_rcv_core to handle all backlog. (Alexey)
*
* Author: Michal Ostrowski [EMAIL PROTECTED]
* Contributors:
@@ -376,22 +377,6 @@
return ret;
}
-
-/
- *
- * Receive wrapper called in process context.
- *
- ***/
-int pppoe_backlog_rcv(struct sock *sk, struct sk_buff *skb)
-{
- lock_sock(sk);
- pppoe_rcv_core(sk, skb);
- release_sock(sk);
- return 0;
-}
-
-
-
/
*
* Receive a PPPoE Discovery frame.
@@ -481,7 +466,7 @@
sk-protocol = PX_PROTO_OE;
sk-family = PF_PPPOX;
- sk-backlog_rcv = pppoe_backlog_rcv;
+ sk-backlog_rcv = pppoe_rcv_core;
sk-next = NULL;
sk-pprev = NULL;
sk-state = PPPOX_NONE;
-
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: Scheduling in interrupt BUG. [Patch]
On Mon, May 14, 2001 at 08:32:33AM -0400, Michal Ostrowski wrote: Having looked at the code for locking sockets I am concerned that the locking procedures for tcp may be wrong. __release_sock releases the socket spinlock before calling sk-backlog_rcv() (== tcp_v4_do_rcv), however the comments at the top of tcp_v4_do_rcv() assert that the socket's spinlock is held (which is definitely not the case). Anybody care to comment on this? Looks ok for me. The user socket lock (lock.users0) is held while __release_sock runs, which is also sufficient to protect it as all new packets will go into backlog. The spinlock comment only applies to bottom halves. -Andi - To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
