Re: Kbuild change breaks the ppc64 build
Isn't the extra space there because you've included it in the definition of "test"? The attempt at introducing indentation introduces the extra space character. Defining test without the internal indentation should produce the results you are looking for. -- Michal Ostrowski <[EMAIL PROTECTED]> On Thu, 2007-02-08 at 04:00 -0800, David Miller wrote: > For some reason $(call ...) invocations add spaces. I tried > another test case: > > define test > $(shell echo -n) > endef > > VAR:=$(call test) > > all: > @echo "\'$(VAR)\'" > > And this always prints: > > ' TEST' > > even with GNU Make version 3.81 > > If I put a $(strip ...) around the define, the space is > still there. If I put the $(strip ...) around VAR's > $(call), the space goes away. - 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/
BUG? [PATCH] driver core: Add the ability to bind drivers to devices from userspace
If a driver's probe function returns -ENXIO or -ENODEV, driver_probe_device() will translate that to return 0 (comments argue it is not an error). Consequently driver_bind() will return 0 resulting in the write system-call that initiated all of this in returning 0 as well. If one uses "echo" to write to a "bind" attribute, echo will continuously call write() trying to write to the attribute and always get 0 as a result and thus find itself in a loop trying to do the write. Perhaps the translation of -ENZIO to -ENODEV to success in driver_probe_device() is not approriate here? -- Michal Ostrowski pgpz384goyOYk.pgp Description: PGP signature
[PATCH] rocket.c: Fix ldisc ref count handling
If bailing out because there is nothing to receive in rp_do_receive(), tty_ldisc_deref is not called. Failure to do so increases the ref count and causes release_dev() to hang since it can't get the ref count to 0. --- Signed-off-by: Michal Ostrowski <[EMAIL PROTECTED]> drivers/char/rocket.c |3 ++- 1 files changed, 2 insertions(+), 1 deletions(-) diff --git a/drivers/char/rocket.c b/drivers/char/rocket.c --- a/drivers/char/rocket.c +++ b/drivers/char/rocket.c @@ -355,7 +355,7 @@ static void rp_do_receive(struct r_port ToRecv = space; if (ToRecv <= 0) - return; + goto done; /* * if status indicates there are errored characters in the @@ -437,6 +437,7 @@ static void rp_do_receive(struct r_port } /* Push the data up to the tty layer */ ld->receive_buf(tty, tty->flip.char_buf, tty->flip.flag_buf, count); + done: tty_ldisc_deref(ld); } pgpj30YGKPzqF.pgp Description: PGP signature
Re: [PATCH] PPPOE can kfree SKB twice (was Re: kernel panic problem. (smp, iptables?))
Alexey replied to my last post with some valuable comments and in
response I have a new patch (that goes on top of David Miller's patch
from yesterday).
The approach here is that in case we don't have room in the skb for
PPPoE headers, we create a new one (skb2) and copy the entire thing.
If we do have space, we clone it. We always give dev_queue_xmit the
copy/clone pointer and let it free it. We then kfree_skb the original
skb depending on the return value of dev_queue_xmit (to conform to the
expectations of ppp_generic).
Michal Ostrowski
[EMAIL PROTECTED]
--- drivers/net/pppoe.c~Wed Jul 18 10:24:25 2001
+++ drivers/net/pppoe.c Thu Jul 19 14:49:36 2001
@@ -5,7 +5,7 @@
* PPPoE --- PPP over Ethernet (RFC 2516)
*
*
- * Version:0.6.7
+ * Version:0.6.8
*
* 030700 : Fixed connect logic to allow for disconnect.
* 270700 :Fixed potential SMP problems; we must protect against
@@ -25,8 +25,12 @@
* locked. (DaveM)
* Ignore return value of dev_queue_xmit in __pppoe_xmit
* or else we may kfree an SKB twice. (DaveM)
+ * 190701 :When doing copies of skb's in __pppoe_xmit, always delete
+ * the original skb that was passed in on success, never on
+ * failure. Delete the copy of the skb on failure to avoid
+ * a memory leak.
*
- * Author: Michal Ostrowski <[EMAIL PROTECTED]>
+ * Author: Michal Ostrowski <[EMAIL PROTECTED]>
* Contributors:
* Arnaldo Carvalho de Melo <[EMAIL PROTECTED]>
* David S. Miller ([EMAIL PROTECTED])
@@ -837,6 +841,7 @@
return error;
}
+
/
*
* xmit function for internal use.
@@ -849,6 +854,7 @@
struct pppoe_hdr *ph;
int headroom = skb_headroom(skb);
int data_len = skb->len;
+ struct sk_buff *skb2;
if (sk->dead || !(sk->state & PPPOX_CONNECTED))
goto abort;
@@ -864,7 +870,6 @@
/* Copy the skb if there is no space for the header. */
if (headroom < (sizeof(struct pppoe_hdr) + dev->hard_header_len)) {
- struct sk_buff *skb2;
skb2 = dev_alloc_skb(32+skb->len +
sizeof(struct pppoe_hdr) +
@@ -876,30 +881,36 @@
skb_reserve(skb2, dev->hard_header_len + sizeof(struct pppoe_hdr));
memcpy(skb_put(skb2, skb->len), skb->data, skb->len);
- skb_unlink(skb);
- kfree_skb(skb);
- skb = skb2;
+ } else {
+ /* Make a clone so as to not disturb the original skb,
+* give dev_queue_xmit something it can free.
+*/
+ skb2 = skb_clone(skb, GFP_ATOMIC);
}
- /* We may not return error beyond this point. Potentially this
-* is a new SKB and in such a case we've already freed the
-* original SKB. So if we were to return error, our caller would
-* double free that original SKB. -DaveM
-*/
-
- ph = (struct pppoe_hdr *) skb_push(skb, sizeof(struct pppoe_hdr));
+ ph = (struct pppoe_hdr *) skb_push(skb2, sizeof(struct pppoe_hdr));
memcpy(ph, &hdr, sizeof(struct pppoe_hdr));
- skb->protocol = __constant_htons(ETH_P_PPP_SES);
+ skb2->protocol = __constant_htons(ETH_P_PPP_SES);
- skb->nh.raw = skb->data;
+ skb2->nh.raw = skb2->data;
- skb->dev = dev;
+ skb2->dev = dev;
- dev->hard_header(skb, dev, ETH_P_PPP_SES,
+ dev->hard_header(skb2, dev, ETH_P_PPP_SES,
sk->protinfo.pppox->pppoe_pa.remote,
NULL, data_len);
- dev_queue_xmit(skb);
+ /* We're transmitting skb2, and assuming that dev_queue_xmit
+* will free it. The generic ppp layer however, is expecting
+* that we give back the skb in case of failure,
+* but free it in case of success.
+*/
+
+ if (dev_queue_xmit(skb2)<0) {
+ goto abort;
+ }
+
+ kfree_skb(skb);
return 1;
abort:
@@ -1049,7 +1060,6 @@
int err = register_pppox_proto(PX_PROTO_OE, &pppoe_proto);
if (err == 0) {
- printk(KERN_INFO "Registered PPPoE v0.6.5\n");
dev_add_pack(&pppoes_ptype);
dev_add_pack(&pppoed_ptype);
--- drivers/net/pppox.c~Tue Feb 13 16:15:05 2001
+++ drivers/net/pppox.c Wed Jul 18 10:27:25 2001
@@ -148,10 +148,6 @@
err = sock_register(&pppox_proto_family);
- if (err == 0) {
- printk(KERN_INFO "Registered PPPoX v0.5\n");
- }
-
return err;
}
-
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: [PATCH] PPPOE can kfree SKB twice (was Re: kernel panic problem. (smp, iptables?))
[EMAIL PROTECTED] writes:
> Hello!
>
> SOme short comment on the patch:
>
>
> > - dev_queue_xmit(skb);
> > + /* The skb we are to transmit may be a copy (see above). If
> > +* this fails, then the caller is responsible for the original
> > +* skb, otherwise we must free it. Also if this fails we must
> > +* free the copy that we made.
> > +*/
> > +
> > + if (dev_queue_xmit(skb)<0) {
>
> dev_queue_xmit _frees_ frame, not depending on return value.
> Return value is not a criterium to assume anything.
>
My mistake. It seemed perfectly reasonable at 6:00 am. :-)
However, could we not have dev_queue_xmit behave as such (not free
frame on failure)? That is, could we extend dev_queue_xmit to tell it
(optionally) that we want the skb back in case of failure?
dev_queue_xmit unconditionally frees the skb in any failure mode,
which is I would venture to say that we could do this.
The reason why I'm proposing this is that ppp_generic.c assumes that
the skb is still available after a transmission failure via pppoe. To
support the semantics of dev_queue_xmit and ppp_generic we would have
to always copy skb's inside pppoe_xmit. Then, if dev_queue_xmit fails
the original is deleted.
In the common case dev_queue_xmit will not fail, and so in that case
I'd like to have to avoid making a copy of the skb.
Michal Ostrowski
[EMAIL PROTECTED]
-
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/
Scheduling in interrupt BUG. [Patch]
Marcell GAL writes:
> Hi Guys,
>
> Once upon a time on my
> x86 UP box, UP kernel 2.4.4, (64M ram, 260M swap)
> http://home.sch.bme.hu/~cell/.config
> I hit a reproducable "Scheduling in interrupt" BUG.
> Also reproduced with 128M ram and low memory pressure
> (first I suspected it is related to swapping)
> Running lots of pppd version 2.4.0 (pppoe) sessions almost at the same
> time.
> (before the crash the pppoe sessions work fine)
> It crashed after 89 sessions, 473 another time.. (depending
> on the phase of Jupiter moons I guess .. I still have to verify this),
> usually much before memory is exhausted (30k mem/pppd process).
> To do this you have to patch ppp_generic.c
> http://x-dsl.hu/~cell/ppp_generic_hash/, because
> otherwise we hit 'NULL ptr in all_ppp_units list'
> BUG much _more likely_ than this 'sched.c line 709 thingy'..
>
> EIP: c010faa4<= sched.c schedule(), line 709:
> which is ~ printk("Scheduling in interrupt");BUG();
>From what I've seen, you have a call chain consisting of:
__release_sock -> pppoe_backlog_rcv -> __lock_sock
This is going to be bad, because when __release_sock calls
sk->backlog_rcv, lock.users is still non-zero and thus the lock_sock
operation will block (leading to deadlock). This problem is fixed
with the patch that I've added below.
You're seeing the "Scheduling in interrupt" message because the
combined effect of the various spin_lock/unlock calls in release_sock
and __release_sock at the time of the call to sk->backlog_rcv is to
increase the local bh count.
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?
Michal Ostrowski
[EMAIL PROTECTED]
--- drivers/net/pppoe.c~Tue Mar 6 22:44:35 2001
+++ drivers/net/pppoe.c Mon May 14 08:24:06 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 :pppoe_backlog_rcv must call bh_lock_sock, not lock_sock.
*
* Author: Michal Ostrowski <[EMAIL PROTECTED]>
* Contributors:
@@ -384,9 +385,9 @@
***/
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;
}
-
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/
pppoe and pppox circular dependencies in test9
Meelis Roos writes: > 2.4.0-test9 > modules.dep reports that pppox needs pppoe and pppoe needs pppox. > modprobe pppo(e|x) segfaults (out of memory???). > A fix has been submitted. If you need a quick fix, do not compile as a module, or remove line 158 of pppox.c. Michal Ostrowski [EMAIL PROTECTED] - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] Please read the FAQ at http://www.tux.org/lkml/
