* Re: [PATCH] net: Fix sock freeing before sock_init_data() with __sk_free()
From: Jarek Poplawski @ 2009-08-31 6:36 UTC (permalink / raw)
To: Eric Dumazet; +Cc: David Miller, netdev
In-Reply-To: <4A9B6D23.9090505@gmail.com>
On Mon, Aug 31, 2009 at 08:26:43AM +0200, Eric Dumazet wrote:
> Jarek Poplawski a écrit :
> > After recent changes sk_free() frees socks conditionally and depends
> > on sk_wmem_alloc beeing set e.g. in sock_init_data(). But in some
> > cases sk_free() is called earlier, usually after other alloc errors.
> > This patch fixes it by exporting and using __sk_free() directly.
...
> Very nice catch Jarek, but dont you think it would be cleaner to make sure
> we can call sk_free() right after sk_alloc() instead, and not exporting
> __sk_free() ?
>
> ie initialize wmem_alloc in sk_alloc() instead of initializing it in
> sock_init_data() ?
>
Most probably it should be better. But I meant this fix for -net and
didn't wan't to break too much... So, if you're sure it's OK feel free
to send your version. (Or it could be changed like this in the -next.)
Jarek P.
^ permalink raw reply
* Re: [PATCH] net: Fix sock freeing before sock_init_data() with __sk_free()
From: Eric Dumazet @ 2009-08-31 6:50 UTC (permalink / raw)
To: Jarek Poplawski; +Cc: David Miller, netdev
In-Reply-To: <20090831063648.GB5005@ff.dom.local>
Jarek Poplawski a écrit :
> On Mon, Aug 31, 2009 at 08:26:43AM +0200, Eric Dumazet wrote:
>> Jarek Poplawski a écrit :
>>> After recent changes sk_free() frees socks conditionally and depends
>>> on sk_wmem_alloc beeing set e.g. in sock_init_data(). But in some
>>> cases sk_free() is called earlier, usually after other alloc errors.
>>> This patch fixes it by exporting and using __sk_free() directly.
> ...
>> Very nice catch Jarek, but dont you think it would be cleaner to make sure
>> we can call sk_free() right after sk_alloc() instead, and not exporting
>> __sk_free() ?
>>
>> ie initialize wmem_alloc in sk_alloc() instead of initializing it in
>> sock_init_data() ?
>>
>
> Most probably it should be better. But I meant this fix for -net and
> didn't wan't to break too much... So, if you're sure it's OK feel free
> to send your version. (Or it could be changed like this in the -next.)
Well, patch is yours, not mine, and I am confident it is OK.
We should check that no sk_alloc() user did a blind memset() or something
strange like that, before calling sock_init_data() or sk_free()
diff --git a/net/core/sock.c b/net/core/sock.c
index bbb25be..7633422 100644
--- a/net/core/sock.c
+++ b/net/core/sock.c
@@ -1025,6 +1025,7 @@ struct sock *sk_alloc(struct net *net, int family, gfp_t priority,
sk->sk_prot = sk->sk_prot_creator = prot;
sock_lock_init(sk);
sock_net_set(sk, get_net(net));
+ atomic_set(&sk->sk_wmem_alloc, 1);
}
return sk;
@@ -1872,7 +1873,6 @@ void sock_init_data(struct socket *sock, struct sock *sk)
*/
smp_wmb();
atomic_set(&sk->sk_refcnt, 1);
- atomic_set(&sk->sk_wmem_alloc, 1);
atomic_set(&sk->sk_drops, 0);
}
EXPORT_SYMBOL(sock_init_data);
^ permalink raw reply related
* Re: [PATCH] net: Fix sock freeing before sock_init_data() with __sk_free()
From: Jarek Poplawski @ 2009-08-31 7:07 UTC (permalink / raw)
To: Eric Dumazet; +Cc: David Miller, netdev
In-Reply-To: <4A9B72B1.2040908@gmail.com>
On Mon, Aug 31, 2009 at 08:50:25AM +0200, Eric Dumazet wrote:
> Jarek Poplawski a écrit :
> > On Mon, Aug 31, 2009 at 08:26:43AM +0200, Eric Dumazet wrote:
> >> Jarek Poplawski a écrit :
> >>> After recent changes sk_free() frees socks conditionally and depends
> >>> on sk_wmem_alloc beeing set e.g. in sock_init_data(). But in some
> >>> cases sk_free() is called earlier, usually after other alloc errors.
> >>> This patch fixes it by exporting and using __sk_free() directly.
> > ...
> >> Very nice catch Jarek, but dont you think it would be cleaner to make sure
> >> we can call sk_free() right after sk_alloc() instead, and not exporting
> >> __sk_free() ?
> >>
> >> ie initialize wmem_alloc in sk_alloc() instead of initializing it in
> >> sock_init_data() ?
> >>
> >
> > Most probably it should be better. But I meant this fix for -net and
> > didn't wan't to break too much... So, if you're sure it's OK feel free
> > to send your version. (Or it could be changed like this in the -next.)
>
> Well, patch is yours, not mine, and I am confident it is OK.
Well, it's from you, and I guess you'll sign off too, but if you
think so...
Signed-off-by: Jarek Poplawski <jarkao2@gmail.com>
Thanks,
Jarek P.
>
> We should check that no sk_alloc() user did a blind memset() or something
> strange like that, before calling sock_init_data() or sk_free()
>
> diff --git a/net/core/sock.c b/net/core/sock.c
> index bbb25be..7633422 100644
> --- a/net/core/sock.c
> +++ b/net/core/sock.c
> @@ -1025,6 +1025,7 @@ struct sock *sk_alloc(struct net *net, int family, gfp_t priority,
> sk->sk_prot = sk->sk_prot_creator = prot;
> sock_lock_init(sk);
> sock_net_set(sk, get_net(net));
> + atomic_set(&sk->sk_wmem_alloc, 1);
> }
>
> return sk;
> @@ -1872,7 +1873,6 @@ void sock_init_data(struct socket *sock, struct sock *sk)
> */
> smp_wmb();
> atomic_set(&sk->sk_refcnt, 1);
> - atomic_set(&sk->sk_wmem_alloc, 1);
> atomic_set(&sk->sk_drops, 0);
> }
> EXPORT_SYMBOL(sock_init_data);
>
^ permalink raw reply
* Re: [PATCH] net: Fix sock freeing before sock_init_data() with __sk_free()
From: Eric Dumazet @ 2009-08-31 7:18 UTC (permalink / raw)
To: Jarek Poplawski; +Cc: David Miller, netdev
In-Reply-To: <20090831070733.GC5005@ff.dom.local>
Jarek Poplawski a écrit :
> On Mon, Aug 31, 2009 at 08:50:25AM +0200, Eric Dumazet wrote:
>> Jarek Poplawski a écrit :
>>> On Mon, Aug 31, 2009 at 08:26:43AM +0200, Eric Dumazet wrote:
>>>> Jarek Poplawski a écrit :
>>>>> After recent changes sk_free() frees socks conditionally and depends
>>>>> on sk_wmem_alloc beeing set e.g. in sock_init_data(). But in some
>>>>> cases sk_free() is called earlier, usually after other alloc errors.
>>>>> This patch fixes it by exporting and using __sk_free() directly.
>>> ...
>>>> Very nice catch Jarek, but dont you think it would be cleaner to make sure
>>>> we can call sk_free() right after sk_alloc() instead, and not exporting
>>>> __sk_free() ?
>>>>
>>>> ie initialize wmem_alloc in sk_alloc() instead of initializing it in
>>>> sock_init_data() ?
>>>>
>>> Most probably it should be better. But I meant this fix for -net and
>>> didn't wan't to break too much... So, if you're sure it's OK feel free
>>> to send your version. (Or it could be changed like this in the -next.)
>> Well, patch is yours, not mine, and I am confident it is OK.
>
> Well, it's from you, and I guess you'll sign off too, but if you
> think so...
>
> Signed-off-by: Jarek Poplawski <jarkao2@gmail.com>
>
> Thanks,
> Jarek P.
>
Give me a few hours to review sk_alloc() call sites, test patch and officially submit it.
Thanks
^ permalink raw reply
* RE: [PATCH] net-next:can: add TI CAN (HECC) driver
From: Gole, Anant @ 2009-08-31 7:22 UTC (permalink / raw)
To: Wolfgang Grandegger; +Cc: Linux Netdev List, Socketcan-core@lists.berlios.de
In-Reply-To: <4A98D36E.9070903@grandegger.com>
>-----Original Message-----
>From: Wolfgang Grandegger [mailto:wg@grandegger.com]
>Sent: Saturday, August 29, 2009 12:36 PM
>To: Gole, Anant
>Cc: Linux Netdev List; Socketcan-core@lists.berlios.de
>Subject: Re: [PATCH] net-next:can: add TI CAN (HECC) driver
>
>> TI HECC (High End CAN Controller) module is found on many TI devices. It
>has
>> 32 harwdare mailboxes with full implementation of CAN protocol version
>2.0B
>> and bus speeds up to 1Mbps. The module specifications are available at TI
>web
>> <http://www.ti.com>.
>>
>> This driver is tested on OMAP3517 EVM. Suspend/Resume not tested as yet.
>
>Nice driver, even using the NAPI interface. First some general comments.
>Please remove debugging code mainly useful for development, e.g. the
>CONFIG_DEBUG_FS block, many dev_info calls and special statistics
>counters. Also use dev_dbg for the remaining debug messages useful for
>the normal user, like state changes. More comments inline.
Ack on most comments you gave - some good bugs uncovered with this review. Thanks. I am answering those which I have some comment on.
[snip]
>
>> +#define HECC_MODULE_VERSION "0.2"
>
>A version number is will usually not maintained. May drivers have it but
>it's never changed.
I understand but unless this is a strong objection I would like to keep it as it would be nice to update it when any major change happens.
[snip]
>> +
>> +/* CAN Bittiming constants as per HECC specs */
>> +static struct can_bittiming_const ti_hecc_bittiming_const = {
>> + .name = DRV_NAME,
>> + .tseg1_min = 1,
>> + .tseg1_max = 16,
>> + .tseg2_min = 1,
>
>Please check if tseg2_min is a valid value. Usually it's larger.
>
Yes I did. This controller allows tseg2_min = 1
[snip]
>
>> + struct napi_struct napi;
>> + struct net_device *ndev;
>> + struct clk *clk;
>> + void __iomem *base;
>> + unsigned int scc_ram_offset;
>> + unsigned int hecc_ram_offset;
>> + unsigned int mbox_offset;
>> + unsigned int int_line;
>> + DECLARE_BITMAP(tx_free_mbx, TI_HECC_MAX_TX_MBOX);
>> + spinlock_t tx_lock;
>
>Please document the spinlock tx_lock. What is it used for.
It is to protect the tx_free_mbx bitmap above - will document.
[snip]
>Right, automatic bus-off recovery should be disabled. To make that clear:
>
> hecc_clear_bit(priv, HECC_CANMC, HECC_CANMC_ABO);
Since CANMC register is set to 0 and ABO bit is part of it I did not make this statement explicitly (it will be redundant). I will update the comment above to state that if it changes in future then enable auto bus off with that statement
[snip]
>> +
>> +static int ti_hecc_do_set_mode(struct net_device *ndev, enum can_mode
>mode)
>> +{
>> + struct ti_hecc_priv *priv = netdev_priv(ndev);
>> + int ret = 0;
>> +
>> + switch (mode) {
>> + case CAN_MODE_SLEEP:
>> + dev_info(priv->ndev->dev.parent, "device going to
>sleep\n");
>> + if (netif_running(ndev)) {
>> + netif_stop_queue(ndev);
>> + netif_device_detach(ndev);
>> + /* Put HECC in low power mode */
>> + hecc_set_bit(priv, HECC_CANMC, HECC_CANMC_PDR);
>> + }
>> + priv->can.state = CAN_STATE_SLEEPING;
>> + break;
>
>Has sleeping been tested? Actually, we do not have an interface yet to
>enter sleep mode. Please remove. If there is a need for it, we should
>work togehter to refine the interface and to provide a solution.
>
No I have not yet tested sleep/suspend/resume. Will remove based upon response to CAN_MODE_STOP comment below
>> + case CAN_MODE_STOP:
>> + dev_info(priv->ndev->dev.parent, "device stopping\n");
>> + ti_hecc_stop(ndev);
>> + break;
>
>Only CAN_MODE_START is used by the CAN devicde interface for bus-off
>recovery.
>
I saw other drivers (like mscan) also have the code for sleep/stop. Agreed that I have not tested it as you mentioned that the CAN infrastructure does not call it right now. Do you want me to remove it?
[snip]
>> +
>> + }
>> + set_bit(mbxno, priv->tx_free_mbx);
>> + spin_unlock_irqrestore(&priv->tx_lock, flags);
>
>Hm, I wonder how the driver ensures that packages go out in order. How
>does the CAN hardware schedule pending TX message objects? Vladislav
>posted a test programs recently to check message ordering. See:
>
>https://lists.berlios.de/pipermail/socketcan-core/2009-August/002871.html
TI HECC hardware has mailbox priority field and higher mailbox number will transmit if two mailboxes have same priority level. But the code does not use that feature as yet. Let me think of how I can ensure out of order issue.
[snip]
>> +
>> + pending_pkts = hecc_read(priv, HECC_CANRMP);
>> + while (pending_pkts && (num_pkts < quota)) {
>> + mbxno = find_first_bit(&pending_pkts,
>HECC_MAX_MAILBOXES);
>
>Here I also wonder if the messages are handled in the correct order.
No I have not taken care of that. Let me look into the pkt handling to see how I can do it based upon hardware capability.
[snip]
>
>> +
>> + ti_hecc_error(ndev, int_status, hecc_read(priv, HECC_CANES));
>
>Hm, you create an error frame for each interrupt!? What do you see with:
>
> # candump any,0:0,#FFFFFFFF
>
Good catch. I did not observe unnecessary error frames via candump (or I really missed them). But I will correct this anyway.
>> +
>> + /* Handle Abort acknowledge interrupt */
>> + if (int_status & HECC_CANGIF_AAIF) {
>> + ack = hecc_read(priv, HECC_CANAA);
>> + while (ack) {
>> + mbxno = find_first_bit(&ack, HECC_MAX_MAILBOXES);
>> + if (mbxno == HECC_MAX_MAILBOXES) {
>> + break;
>> + } else {
>> + clear_bit(mbxno, &ack);
>> + /* release echo pkt & update counters */
>> + hecc_set_bit(priv, HECC_CANAA, (1 <<
>mbxno));
>> + hecc_clear_bit(priv, HECC_CANME, (1 <<
>mbxno));
>> + /* FIXME: since net-next tree's dev.h
>does not
>> + * include can_free_echo_skb() doing
>equivalent
>> + * of can_free_echo_skb(ndev, mbxno);
>> + */
>> + if (priv->can.echo_skb[mbxno]) {
>> + kfree_skb(priv-
>>can.echo_skb[mbxno]);
>> + priv->can.echo_skb[mbxno] = NULL;
>> + }
>> + if (netif_queue_stopped(ndev))
>> + netif_wake_queue(ndev);
>> + spin_lock_irqsave(&priv->tx_lock, flags);
>> + clear_bit(mbxno, priv->tx_free_mbx);
>> + spin_unlock_irqrestore(&priv->tx_lock,
>flags);
>> + }
>> + }
>> + }
>
>Can that interrupt happen? I have not found any code aborting messages.
It can happen only if a tx mailbox is told to abort. This interrupt is an ack for the same. I am not using the tx abort feature so will remove this.
[snip]
>> + }
>> +
>> + /* Open common can device */
>> + err = open_candev(ndev);
>> + if (err) {
>> + dev_err(ndev->dev.parent, "open_candev() failed %08X\n",
>err);
>
>free_irq?
Good catch. Will fix it.
>
>> + return err;
>> + }
>> +
>
>Thanks for your contribution.
>
>Wolfgang.
>
Appreciate the review. I will send v2 of patch. Not sure still if the original patch reached the socketcan mailing list due to size.
--
Anant
^ permalink raw reply
* Re: [PATCH] net: Fix sock freeing before sock_init_data() with __sk_free()
From: Jarek Poplawski @ 2009-08-31 7:25 UTC (permalink / raw)
To: Eric Dumazet; +Cc: David Miller, netdev
In-Reply-To: <4A9B7956.40507@gmail.com>
On Mon, Aug 31, 2009 at 09:18:46AM +0200, Eric Dumazet wrote:
> Jarek Poplawski a écrit :
> > On Mon, Aug 31, 2009 at 08:50:25AM +0200, Eric Dumazet wrote:
...
> >> Well, patch is yours, not mine, and I am confident it is OK.
> >
> > Well, it's from you, and I guess you'll sign off too, but if you
> > think so...
> >
> > Signed-off-by: Jarek Poplawski <jarkao2@gmail.com>
> >
> > Thanks,
> > Jarek P.
> >
>
> Give me a few hours to review sk_alloc() call sites, test patch and officially submit it.
>
> Thanks
Hmm... but you're "confident it is OK"?! (You know, I could've risked
my life or something... ;-)
Jarek P.
^ permalink raw reply
* [patch, v2] IPVS: Add handling of incoming ICMPV6 messages
From: Simon Horman @ 2009-08-31 7:59 UTC (permalink / raw)
To: lvs-devel, netdev; +Cc: David Miller, Julius Volz, Rob Gallagher
From: Julius Volz <julius.volz@gmail.com>
IPVS: Add handling of incoming ICMPV6 messages
Add handling of incoming ICMPv6 messages.
This follows the handling of IPv4 ICMP messages.
Amongst ther things this problem allows IPVS to behave sensibly
when an ICMPV6_PKT_TOOBIG message is received:
This message is received when a realserver sends a packet >PMTU to the
client. The hop on this path with insufficient MTU will generate an
ICMPv6 Packet Too Big message back to the VIP. The LVS server receives
this message, but the call to the function handling this has been
missing. Thus, IPVS fails to forward the message to the real server,
which then does not adjust the path MTU. This patch adds the missing
call to ip_vs_in_icmp_v6() in ip_vs_in() to handle this situation.
Thanks to Rob Gallagher from HEAnet for reporting this issue and for
testing this patch in production (with direct routing mode).
[horms@verge.net.au: tweaked changelog]
Signed-off-by: Julius Volz <julius.volz@gmail.com>
Tested-by: Rob Gallagher <robert.gallagher@heanet.ie>
Signed-off-by: Simon Horman <horms@verge.net.au>
---
net/netfilter/ipvs/ip_vs_core.c | 23 +++++++++++++++++------
1 files changed, 17 insertions(+), 6 deletions(-)
Changelog
---------
* Fri, 24 Jul 2009 12:47:16 +1000
Initial post to netdev
* Mon, 31 Aug 2009 17:53:40 +1000
Tweaked commit message as per feedback
diff --git a/net/netfilter/ipvs/ip_vs_core.c b/net/netfilter/ipvs/ip_vs_core.c
index 8dddb17..5750800 100644
--- a/net/netfilter/ipvs/ip_vs_core.c
+++ b/net/netfilter/ipvs/ip_vs_core.c
@@ -1274,13 +1274,24 @@ ip_vs_in(unsigned int hooknum, struct sk_buff *skb,
return NF_ACCEPT;
}
- if (unlikely(iph.protocol == IPPROTO_ICMP)) {
- int related, verdict = ip_vs_in_icmp(skb, &related, hooknum);
+#ifdef CONFIG_IP_VS_IPV6
+ if (af == AF_INET6) {
+ if (unlikely(iph.protocol == IPPROTO_ICMPV6)) {
+ int related, verdict = ip_vs_in_icmp_v6(skb, &related, hooknum);
- if (related)
- return verdict;
- ip_vs_fill_iphdr(af, skb_network_header(skb), &iph);
- }
+ if (related)
+ return verdict;
+ ip_vs_fill_iphdr(af, skb_network_header(skb), &iph);
+ }
+ } else
+#endif
+ if (unlikely(iph.protocol == IPPROTO_ICMP)) {
+ int related, verdict = ip_vs_in_icmp(skb, &related, hooknum);
+
+ if (related)
+ return verdict;
+ ip_vs_fill_iphdr(af, skb_network_header(skb), &iph);
+ }
/* Protocol supported? */
pp = ip_vs_proto_get(iph.protocol);
--
To unsubscribe from this list: send the line "unsubscribe lvs-devel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply related
* Re: [PATCH] Re: iproute2 / tbf with large burst seems broken again
From: Denys Fedoryschenko @ 2009-08-31 8:03 UTC (permalink / raw)
To: David Miller; +Cc: jarkao2, netdev
In-Reply-To: <20090830.223239.185830289.davem@davemloft.net>
On Monday 31 August 2009 08:32:39 David Miller wrote:
> From: Jarek Poplawski <jarkao2@gmail.com>
> Date: Mon, 31 Aug 2009 05:30:27 +0000
>
> > Actually, I think now, after 2.6.31, this patch might be a bit too
> > late. If people hit this problem they probably should've fixed it
> > until -next already, so let's forget about this patch until there are
> > more such reports.
>
> Ok.
>
> > Btw, I guess, Denys could submit some warnings into iproute's code
> > and/or documentation, as he proposed earlier.
>
> Yeah sounds like a good idea.
Sorry, just was busy with network overload on ramadan.
I will test now.
^ permalink raw reply
* Re: [PATCH] Re: iproute2 / tbf with large burst seems broken again
From: Denys Fedoryschenko @ 2009-08-31 8:18 UTC (permalink / raw)
To: David Miller; +Cc: jarkao2, netdev
In-Reply-To: <20090830.223239.185830289.davem@davemloft.net>
On Monday 31 August 2009 08:32:39 David Miller wrote:
> From: Jarek Poplawski <jarkao2@gmail.com>
> Date: Mon, 31 Aug 2009 05:30:27 +0000
>
> > Actually, I think now, after 2.6.31, this patch might be a bit too
> > late. If people hit this problem they probably should've fixed it
> > until -next already, so let's forget about this patch until there are
> > more such reports.
>
> Ok.
>
> > Btw, I guess, Denys could submit some warnings into iproute's code
> > and/or documentation, as he proposed earlier.
>
> Yeah sounds like a good idea.
2.6.31-rc8 i got dmesg flooded. First message appearing even without patch, i
will make sure now what is a reason of that (i have HTB classes on ifb0, and
TBF on pppX).
[ 155.199923] Attempt to kill tasklet from interrupt
[ 158.425096] Attempt to kill tasklet from interrupt
[ 158.425176] Attempt to kill tasklet from interrupt
[ 162.549083] Attempt to kill tasklet from interrupt
[ 162.549159] Attempt to kill tasklet from interrupt
[ 162.601892] Attempt to kill tasklet from interrupt
[ 162.601968] Attempt to kill tasklet from interrupt
[ 183.719070] Attempt to kill tasklet from interrupt
[ 183.719146] Attempt to kill tasklet from interrupt
[ 192.147105] Attempt to kill tasklet from interrupt
[ 192.147181] Attempt to kill tasklet from interrupt
[ 192.604718] tbf: tokens overflow; fix limits.
[ 193.085459] tbf: tokens overflow; fix limits.
[ 193.102169] tbf: tokens overflow; fix limits.
[ 194.100395] tbf: tokens overflow; fix limits.
[ 196.010286] tbf: tokens overflow; fix limits.
[ 196.014330] tbf: tokens overflow; fix limits.
[ 196.100723] tbf: tokens overflow; fix limits.
[ 196.104619] tbf: tokens overflow; fix limits.
[ 196.998374] tbf: tokens overflow; fix limits.
[ 197.854766] tbf: tokens overflow; fix limits.
[ 198.676258] tbf: tokens overflow; fix limits.
[ 198.676564] tbf: tokens overflow; fix limits.
[ 198.984620] tbf: tokens overflow; fix limits.
[ 199.599671] tbf: tokens overflow; fix limits.
[ 200.101338] tbf: tokens overflow; fix limits.
^ permalink raw reply
* [PATCH] au1000_eth: possible NULL dereference of aup->mii_bus->irq in au1000_probe()
From: Roel Kluin @ 2009-08-31 8:40 UTC (permalink / raw)
To: netdev, Andrew Morton, David S. Miller
aup->mii_bus->irq allocation may fail, prevent a dereference of NULL.
Signed-off-by: Roel Kluin <roel.kluin@gmail.com>
---
diff --git a/drivers/net/au1000_eth.c b/drivers/net/au1000_eth.c
index d3c734f..02e4be0 100644
--- a/drivers/net/au1000_eth.c
+++ b/drivers/net/au1000_eth.c
@@ -1157,6 +1157,9 @@ static struct net_device * au1000_probe(int port_num)
aup->mii_bus->name = "au1000_eth_mii";
snprintf(aup->mii_bus->id, MII_BUS_ID_SIZE, "%x", aup->mac_id);
aup->mii_bus->irq = kmalloc(sizeof(int)*PHY_MAX_ADDR, GFP_KERNEL);
+ if (aup->mii_bus->irq == NULL)
+ goto err_out;
+
for(i = 0; i < PHY_MAX_ADDR; ++i)
aup->mii_bus->irq[i] = PHY_POLL;
^ permalink raw reply related
* Re: [PATCH] Re: iproute2 / tbf with large burst seems broken again
From: David Miller @ 2009-08-31 8:37 UTC (permalink / raw)
To: denys; +Cc: jarkao2, netdev
In-Reply-To: <200908311118.43024.denys@visp.net.lb>
From: Denys Fedoryschenko <denys@visp.net.lb>
Date: Mon, 31 Aug 2009 11:18:42 +0300
> 2.6.31-rc8 i got dmesg flooded. First message appearing even without patch, i
> will make sure now what is a reason of that (i have HTB classes on ifb0, and
> TBF on pppX).
>
> [ 155.199923] Attempt to kill tasklet from interrupt
We know the reasons for this message, it's an HTB regression and we're
working on it.
^ permalink raw reply
* Re: [PATCH] Re: iproute2 / tbf with large burst seems broken again
From: Jarek Poplawski @ 2009-08-31 8:49 UTC (permalink / raw)
To: Denys Fedoryschenko; +Cc: David Miller, netdev
In-Reply-To: <200908311118.43024.denys@visp.net.lb>
On Mon, Aug 31, 2009 at 11:18:42AM +0300, Denys Fedoryschenko wrote:
> On Monday 31 August 2009 08:32:39 David Miller wrote:
> > From: Jarek Poplawski <jarkao2@gmail.com>
> > Date: Mon, 31 Aug 2009 05:30:27 +0000
> >
> > > Actually, I think now, after 2.6.31, this patch might be a bit too
> > > late. If people hit this problem they probably should've fixed it
> > > until -next already, so let's forget about this patch until there are
> > > more such reports.
> >
> > Ok.
> >
> > > Btw, I guess, Denys could submit some warnings into iproute's code
> > > and/or documentation, as he proposed earlier.
> >
> > Yeah sounds like a good idea.
>
> 2.6.31-rc8 i got dmesg flooded. First message appearing even without patch, i
> will make sure now what is a reason of that (i have HTB classes on ifb0, and
> TBF on pppX).
>
> [ 155.199923] Attempt to kill tasklet from interrupt
> [ 158.425096] Attempt to kill tasklet from interrupt
> [ 158.425176] Attempt to kill tasklet from interrupt
> [ 162.549083] Attempt to kill tasklet from interrupt
> [ 162.549159] Attempt to kill tasklet from interrupt
> [ 162.601892] Attempt to kill tasklet from interrupt
> [ 162.601968] Attempt to kill tasklet from interrupt
> [ 183.719070] Attempt to kill tasklet from interrupt
> [ 183.719146] Attempt to kill tasklet from interrupt
> [ 192.147105] Attempt to kill tasklet from interrupt
> [ 192.147181] Attempt to kill tasklet from interrupt
> [ 192.604718] tbf: tokens overflow; fix limits.
> [ 193.085459] tbf: tokens overflow; fix limits.
> [ 193.102169] tbf: tokens overflow; fix limits.
> [ 194.100395] tbf: tokens overflow; fix limits.
> [ 196.010286] tbf: tokens overflow; fix limits.
> [ 196.014330] tbf: tokens overflow; fix limits.
> [ 196.100723] tbf: tokens overflow; fix limits.
> [ 196.104619] tbf: tokens overflow; fix limits.
> [ 196.998374] tbf: tokens overflow; fix limits.
> [ 197.854766] tbf: tokens overflow; fix limits.
> [ 198.676258] tbf: tokens overflow; fix limits.
> [ 198.676564] tbf: tokens overflow; fix limits.
> [ 198.984620] tbf: tokens overflow; fix limits.
> [ 199.599671] tbf: tokens overflow; fix limits.
> [ 200.101338] tbf: tokens overflow; fix limits.
I guess it's with a very large (wrt. rate) tbf buffer setting?
Btw, of course testing it would be appreciated, but as I wrote
earlier we can probably wait with this patch for more reports.
Jarek P.
^ permalink raw reply
* Re: [PATCH resend] tracing/events: convert NAPI's tracepoint via TRACE_EVENT
From: Ingo Molnar @ 2009-08-31 8:49 UTC (permalink / raw)
To: Xiao Guangrong
Cc: David Miller, Neil Horman, Steven Rostedt, Frederic Weisbecker,
Wei Yongjun, Netdev, LKML
In-Reply-To: <4A9B6AA7.5020508@cn.fujitsu.com>
* Xiao Guangrong <xiaoguangrong@cn.fujitsu.com> wrote:
> - Convert NAPI's tracepoint via TRACE_EVENT macro, the output information
> like below:
I think as long as it does not touch tracing infrastructure (which
your patches dont do in their current form) this should be
done/merged via the networking tree.
[ There might be some small collisions in define_trace.h (because
these tracepoints move from legacy to new-style TRACE_EVENT()
form) but that's OK. ]
Ingo
^ permalink raw reply
* Re: [PATCH] Re: iproute2 / tbf with large burst seems broken again
From: Denys Fedoryschenko @ 2009-08-31 8:51 UTC (permalink / raw)
To: David Miller; +Cc: jarkao2, netdev
In-Reply-To: <20090831.013738.127727614.davem@davemloft.net>
On Monday 31 August 2009 11:37:38 David Miller wrote:
> From: Denys Fedoryschenko <denys@visp.net.lb>
> Date: Mon, 31 Aug 2009 11:18:42 +0300
>
> > 2.6.31-rc8 i got dmesg flooded. First message appearing even without
> > patch, i will make sure now what is a reason of that (i have HTB classes
> > on ifb0, and TBF on pppX).
> >
> > [ 155.199923] Attempt to kill tasklet from interrupt
>
> We know the reasons for this message, it's an HTB regression and we're
> working on it.
Well, those TBF warning seems works fine, i even found one missing shaper that
was overflowing. Thanks to Jarek.
For now no false positives. Message is appearing only when real overflow
happen.
^ permalink raw reply
* Re: [PATCH] Re: iproute2 / tbf with large burst seems broken again
From: Jarek Poplawski @ 2009-08-31 8:58 UTC (permalink / raw)
To: David Miller; +Cc: denys, netdev
In-Reply-To: <20090831.013738.127727614.davem@davemloft.net>
On Mon, Aug 31, 2009 at 01:37:38AM -0700, David Miller wrote:
> From: Denys Fedoryschenko <denys@visp.net.lb>
> Date: Mon, 31 Aug 2009 11:18:42 +0300
>
> > 2.6.31-rc8 i got dmesg flooded. First message appearing even without patch, i
> > will make sure now what is a reason of that (i have HTB classes on ifb0, and
> > TBF on pppX).
> >
> > [ 155.199923] Attempt to kill tasklet from interrupt
>
> We know the reasons for this message, it's an HTB regression and we're
> working on it.
There is a few more than HTB. Btw, IMHO, we don't risk much (if at
all) if we revert this qdisc_watchdog change. There is still CBQ
problem, where we could probably reconsider Thomas Gleixner's first
attempt with additional locking and irq disabling as a temporary
solution?
Jarek P.
^ permalink raw reply
* Re: [PATCH] net-next:can: add TI CAN (HECC) driver
From: Wolfgang Grandegger @ 2009-08-31 8:59 UTC (permalink / raw)
To: Gole, Anant; +Cc: Socketcan-core@lists.berlios.de, Linux Netdev List
In-Reply-To: <2A3DCF3DA181AD40BDE86A3150B27B6B02F6211427@dbde02.ent.ti.com>
Gole, Anant wrote:
>> -----Original Message-----
>> From: Wolfgang Grandegger [mailto:wg@grandegger.com]
>> Sent: Saturday, August 29, 2009 12:36 PM
>> To: Gole, Anant
>> Cc: Linux Netdev List; Socketcan-core@lists.berlios.de
>> Subject: Re: [PATCH] net-next:can: add TI CAN (HECC) driver
>>
>>> TI HECC (High End CAN Controller) module is found on many TI devices. It
>> has
>>> 32 harwdare mailboxes with full implementation of CAN protocol version
>> 2.0B
>>> and bus speeds up to 1Mbps. The module specifications are available at TI
>> web
>>> <http://www.ti.com>.
>>>
>>> This driver is tested on OMAP3517 EVM. Suspend/Resume not tested as yet.
>> Nice driver, even using the NAPI interface. First some general comments.
>> Please remove debugging code mainly useful for development, e.g. the
>> CONFIG_DEBUG_FS block, many dev_info calls and special statistics
>> counters. Also use dev_dbg for the remaining debug messages useful for
>> the normal user, like state changes. More comments inline.
>
> Ack on most comments you gave - some good bugs uncovered with this review. Thanks. I am answering those which I have some comment on.
>
> [snip]
>
>>> +#define HECC_MODULE_VERSION "0.2"
>> A version number is will usually not maintained. May drivers have it but
>> it's never changed.
>
> I understand but unless this is a strong objection I would like to keep it as it would be nice to update it when any major change happens.
It's *not* a strong objection but I personally believe that git is doing
a much better job.
> [snip]
>
>>> +
>>> +/* CAN Bittiming constants as per HECC specs */
>>> +static struct can_bittiming_const ti_hecc_bittiming_const = {
>>> + .name = DRV_NAME,
>>> + .tseg1_min = 1,
>>> + .tseg1_max = 16,
>>> + .tseg2_min = 1,
>> Please check if tseg2_min is a valid value. Usually it's larger.
>>
> Yes I did. This controller allows tseg2_min = 1
OK.
>
> [snip]
>>> + struct napi_struct napi;
>>> + struct net_device *ndev;
>>> + struct clk *clk;
>>> + void __iomem *base;
>>> + unsigned int scc_ram_offset;
>>> + unsigned int hecc_ram_offset;
>>> + unsigned int mbox_offset;
>>> + unsigned int int_line;
>>> + DECLARE_BITMAP(tx_free_mbx, TI_HECC_MAX_TX_MBOX);
>>> + spinlock_t tx_lock;
>> Please document the spinlock tx_lock. What is it used for.
>
> It is to protect the tx_free_mbx bitmap above - will document.
I know, but for kernel inclusion it should be documented.
> [snip]
>
>> Right, automatic bus-off recovery should be disabled. To make that clear:
>>
>> hecc_clear_bit(priv, HECC_CANMC, HECC_CANMC_ABO);
>
> Since CANMC register is set to 0 and ABO bit is part of it I did not make this statement explicitly (it will be redundant). I will update the comment above to state that if it changes in future then enable auto bus off with that statement
OK.
> [snip]
>
>>> +
>>> +static int ti_hecc_do_set_mode(struct net_device *ndev, enum can_mode
>> mode)
>>> +{
>>> + struct ti_hecc_priv *priv = netdev_priv(ndev);
>>> + int ret = 0;
>>> +
>>> + switch (mode) {
>>> + case CAN_MODE_SLEEP:
>>> + dev_info(priv->ndev->dev.parent, "device going to
>> sleep\n");
>>> + if (netif_running(ndev)) {
>>> + netif_stop_queue(ndev);
>>> + netif_device_detach(ndev);
>>> + /* Put HECC in low power mode */
>>> + hecc_set_bit(priv, HECC_CANMC, HECC_CANMC_PDR);
>>> + }
>>> + priv->can.state = CAN_STATE_SLEEPING;
>>> + break;
>> Has sleeping been tested? Actually, we do not have an interface yet to
>> enter sleep mode. Please remove. If there is a need for it, we should
>> work togehter to refine the interface and to provide a solution.
>>
> No I have not yet tested sleep/suspend/resume. Will remove based upon response to CAN_MODE_STOP comment below
>
>>> + case CAN_MODE_STOP:
>>> + dev_info(priv->ndev->dev.parent, "device stopping\n");
>>> + ti_hecc_stop(ndev);
>>> + break;
>> Only CAN_MODE_START is used by the CAN devicde interface for bus-off
>> recovery.
>>
> I saw other drivers (like mscan) also have the code for sleep/stop. Agreed that I have not tested it as you mentioned that the CAN infrastructure does not call it right now. Do you want me to remove it?
Yes, for kernel inclusion it should be removed. We are more relaxed to
keep that "preliminary" and "un-tested" code in the BerliOS SVN
repository. Note that the MSCAN driver is not yet mainline
> [snip]
>
>>> +
>>> + }
>>> + set_bit(mbxno, priv->tx_free_mbx);
>>> + spin_unlock_irqrestore(&priv->tx_lock, flags);
>> Hm, I wonder how the driver ensures that packages go out in order. How
>> does the CAN hardware schedule pending TX message objects? Vladislav
>> posted a test programs recently to check message ordering. See:
>>
>> https://lists.berlios.de/pipermail/socketcan-core/2009-August/002871.html
>
> TI HECC hardware has mailbox priority field and higher mailbox number will transmit if two mailboxes have same priority level. But the code does not use that feature as yet. Let me think of how I can ensure out of order issue.
Have a look to the AT91 driver, it seems to be quite similar.
> [snip]
>>> +
>>> + pending_pkts = hecc_read(priv, HECC_CANRMP);
>>> + while (pending_pkts && (num_pkts < quota)) {
>>> + mbxno = find_first_bit(&pending_pkts,
>> HECC_MAX_MAILBOXES);
>>
>> Here I also wonder if the messages are handled in the correct order.
>
> No I have not taken care of that. Let me look into the pkt handling to see how I can do it based upon hardware capability.
>
> [snip]
>
>>> +
>>> + ti_hecc_error(ndev, int_status, hecc_read(priv, HECC_CANES));
>> Hm, you create an error frame for each interrupt!? What do you see with:
>>
>> # candump any,0:0,#FFFFFFFF
>>
> Good catch. I did not observe unnecessary error frames via candump (or I really missed them). But I will correct this anyway.
I see. That's because the error code of can_id is 0 and the mask does
not trigger.
>>> +
>>> + /* Handle Abort acknowledge interrupt */
>>> + if (int_status & HECC_CANGIF_AAIF) {
>>> + ack = hecc_read(priv, HECC_CANAA);
>>> + while (ack) {
>>> + mbxno = find_first_bit(&ack, HECC_MAX_MAILBOXES);
>>> + if (mbxno == HECC_MAX_MAILBOXES) {
>>> + break;
>>> + } else {
>>> + clear_bit(mbxno, &ack);
>>> + /* release echo pkt & update counters */
>>> + hecc_set_bit(priv, HECC_CANAA, (1 <<
>> mbxno));
>>> + hecc_clear_bit(priv, HECC_CANME, (1 <<
>> mbxno));
>>> + /* FIXME: since net-next tree's dev.h
>> does not
>>> + * include can_free_echo_skb() doing
>> equivalent
>>> + * of can_free_echo_skb(ndev, mbxno);
>>> + */
>>> + if (priv->can.echo_skb[mbxno]) {
>>> + kfree_skb(priv-
>>> can.echo_skb[mbxno]);
>>> + priv->can.echo_skb[mbxno] = NULL;
>>> + }
>>> + if (netif_queue_stopped(ndev))
>>> + netif_wake_queue(ndev);
>>> + spin_lock_irqsave(&priv->tx_lock, flags);
>>> + clear_bit(mbxno, priv->tx_free_mbx);
>>> + spin_unlock_irqrestore(&priv->tx_lock,
>> flags);
>>> + }
>>> + }
>>> + }
>> Can that interrupt happen? I have not found any code aborting messages.
>
> It can happen only if a tx mailbox is told to abort. This interrupt is an ack for the same. I am not using the tx abort feature so will remove this.
>
> [snip]
>>> + }
>>> +
>>> + /* Open common can device */
>>> + err = open_candev(ndev);
>>> + if (err) {
>>> + dev_err(ndev->dev.parent, "open_candev() failed %08X\n",
>> err);
>>
>> free_irq?
>
> Good catch. Will fix it.
>
>>> + return err;
>>> + }
>>> +
>> Thanks for your contribution.
>>
>> Wolfgang.
>>
>
> Appreciate the review. I will send v2 of patch. Not sure still if the original patch reached the socketcan mailing list due to size.
v2 will fit but I'm wondering who is moderating the ML. Oliver?
Wolfgang.
^ permalink raw reply
* Re: [PATCH] Re: iproute2 / tbf with large burst seems broken again
From: Jarek Poplawski @ 2009-08-31 9:05 UTC (permalink / raw)
To: Denys Fedoryschenko; +Cc: David Miller, netdev
In-Reply-To: <200908311151.22607.denys@visp.net.lb>
On Mon, Aug 31, 2009 at 11:51:22AM +0300, Denys Fedoryschenko wrote:
> On Monday 31 August 2009 11:37:38 David Miller wrote:
> > From: Denys Fedoryschenko <denys@visp.net.lb>
> > Date: Mon, 31 Aug 2009 11:18:42 +0300
> >
> > > 2.6.31-rc8 i got dmesg flooded. First message appearing even without
> > > patch, i will make sure now what is a reason of that (i have HTB classes
> > > on ifb0, and TBF on pppX).
> > >
> > > [ 155.199923] Attempt to kill tasklet from interrupt
> >
> > We know the reasons for this message, it's an HTB regression and we're
> > working on it.
>
> Well, those TBF warning seems works fine, i even found one missing shaper that
> was overflowing. Thanks to Jarek.
> For now no false positives. Message is appearing only when real overflow
> happen.
I'm glad it helps. On the other hand, I doubt you'll have such configs
until 2.6.32 where this patch was intended. (It's a pity you didn't
report it a bit earlier.) So, let David decide...
Jarek P.
^ permalink raw reply
* [PATCH] net: sk_free() should be allowed right after sk_alloc()
From: Eric Dumazet @ 2009-08-31 9:15 UTC (permalink / raw)
To: Jarek Poplawski, David Miller; +Cc: netdev
In-Reply-To: <4A9B7956.40507@gmail.com>
From: Jarek Poplawski <jarkao2@gmail.com>
After commit 2b85a34e911bf483c27cfdd124aeb1605145dc80
(net: No more expensive sock_hold()/sock_put() on each tx)
sk_free() frees socks conditionally and depends
on sk_wmem_alloc beeing set e.g. in sock_init_data(). But in some
cases sk_free() is called earlier, usually after other alloc errors.
Fix is to move sk_wmem_alloc initialization from sock_init_data()
to sk_alloc() itself.
Signed-off-by: Jarek Poplawski <jarkao2@gmail.com>
Signed-off-by: Eric Dumazet <eric.dumazet@gmail.com>
---
diff --git a/net/core/sock.c b/net/core/sock.c
index bbb25be..7633422 100644
--- a/net/core/sock.c
+++ b/net/core/sock.c
@@ -1025,6 +1025,7 @@ struct sock *sk_alloc(struct net *net, int family, gfp_t priority,
sk->sk_prot = sk->sk_prot_creator = prot;
sock_lock_init(sk);
sock_net_set(sk, get_net(net));
+ atomic_set(&sk->sk_wmem_alloc, 1);
}
return sk;
@@ -1872,7 +1873,6 @@ void sock_init_data(struct socket *sock, struct sock *sk)
*/
smp_wmb();
atomic_set(&sk->sk_refcnt, 1);
- atomic_set(&sk->sk_wmem_alloc, 1);
atomic_set(&sk->sk_drops, 0);
}
EXPORT_SYMBOL(sock_init_data);
^ permalink raw reply related
* Re: [PATCH] net: sk_free() should be allowed right after sk_alloc()
From: Jarek Poplawski @ 2009-08-31 9:30 UTC (permalink / raw)
To: Eric Dumazet; +Cc: David Miller, netdev
In-Reply-To: <4A9B94B8.4050808@gmail.com>
On Mon, Aug 31, 2009 at 11:15:36AM +0200, Eric Dumazet wrote:
> From: Jarek Poplawski <jarkao2@gmail.com>
>
> After commit 2b85a34e911bf483c27cfdd124aeb1605145dc80
> (net: No more expensive sock_hold()/sock_put() on each tx)
> sk_free() frees socks conditionally and depends
> on sk_wmem_alloc beeing set e.g. in sock_init_data(). But in some
Very nice, but I hope David could fix btw. my "beeing" misspelling.
Thanks everyone,
Jarek P.
> cases sk_free() is called earlier, usually after other alloc errors.
>
> Fix is to move sk_wmem_alloc initialization from sock_init_data()
> to sk_alloc() itself.
>
> Signed-off-by: Jarek Poplawski <jarkao2@gmail.com>
> Signed-off-by: Eric Dumazet <eric.dumazet@gmail.com>
> ---
> diff --git a/net/core/sock.c b/net/core/sock.c
> index bbb25be..7633422 100644
> --- a/net/core/sock.c
> +++ b/net/core/sock.c
> @@ -1025,6 +1025,7 @@ struct sock *sk_alloc(struct net *net, int family, gfp_t priority,
> sk->sk_prot = sk->sk_prot_creator = prot;
> sock_lock_init(sk);
> sock_net_set(sk, get_net(net));
> + atomic_set(&sk->sk_wmem_alloc, 1);
> }
>
> return sk;
> @@ -1872,7 +1873,6 @@ void sock_init_data(struct socket *sock, struct sock *sk)
> */
> smp_wmb();
> atomic_set(&sk->sk_refcnt, 1);
> - atomic_set(&sk->sk_wmem_alloc, 1);
> atomic_set(&sk->sk_drops, 0);
> }
> EXPORT_SYMBOL(sock_init_data);
>
^ permalink raw reply
* Re: [PATCH 1/5] ucc_geth: Fix NULL pointer dereference in uec_get_ethtool_stats()
From: Benjamin Herrenschmidt @ 2009-08-31 9:35 UTC (permalink / raw)
To: David Miller; +Cc: scottwood, netdev, timur, linuxppc-dev
In-Reply-To: <20090830.215224.203201252.davem@davemloft.net>
On Sun, 2009-08-30 at 21:52 -0700, David Miller wrote:
> All 5 patches applied to net-next-2.6
>
> I would have liked to have seen at least one powerpc ACK for
> patch #2 but these were posted more than a week ago, the
> patch looks pretty reasonable, and we can't wait forever for
> stuff like this.
The qe stuff ? Well, that's entirely Kumar's domain, but Anton
patches tend to be of good quality so I wouldn't have too much
second thoughts here.
Cheers,
Ben.
^ permalink raw reply
* Re: [PATCH] sky2: Don't try to turn led off in sky2_down()
From: Rene Mayrhofer @ 2009-08-31 9:48 UTC (permalink / raw)
To: Mike McCormack; +Cc: Stephen Hemminger, netdev, Richard Leitner
In-Reply-To: <4A9928C9.7000907@ring3k.org>
Hi Mika and Stephen,
Am Samstag, 29. August 2009 15:10:33 schrieb Mike McCormack:
> There are a few problems with the following line of code in sky2_down()
>
> sky2_write16(hw, B0_Y2LED, LED_STAT_OFF);
>
> * It doesn't specify which port's LED to turn off.
> * We don't turn send LED_STAT_ON on in sky2_up()
> * B0_LED is 0x0006 in the vendor driver, but 0x0005 in sky2.
> B0_LED is right next to B0_POWER_CTRL, so this is possibly
> accounts for the device being accidently powered down as
> reported by Rene Mayrhofer.
>
> Signed-off-by: Mike McCormack <mikem@ring3k.org>
I tried the current net-next version with this patch, and a network restart
still leads to a system reset. Is this patch aimed at net-next or the vanilla
2.6.30.5 version?
I assume the updated 2 patches would not make any difference, right?
best regards,
Rene
--
-------------------------------------------------
Gibraltar firewall http://www.gibraltar.at/
^ permalink raw reply
* Re: [PATCH] sky2: Don't try to turn led off in sky2_down()
From: Rene Mayrhofer @ 2009-08-31 9:58 UTC (permalink / raw)
To: Mike McCormack; +Cc: Stephen Hemminger, netdev, Richard Leitner
In-Reply-To: <4A9928C9.7000907@ring3k.org>
Am Samstag, 29. August 2009 15:10:33 schrieb Mike McCormack:
> There are a few problems with the following line of code in sky2_down()
>
> sky2_write16(hw, B0_Y2LED, LED_STAT_OFF);
>
> * It doesn't specify which port's LED to turn off.
> * We don't turn send LED_STAT_ON on in sky2_up()
> * B0_LED is 0x0006 in the vendor driver, but 0x0005 in sky2.
> B0_LED is right next to B0_POWER_CTRL, so this is possibly
> accounts for the device being accidently powered down as
> reported by Rene Mayrhofer.
With this patch applied to the net-next version from Friday, I now got this:
[~]# /etc/init.d/networking restart
Reconfiguring network interfaces...[ 403.000092] sky2 0000:03:00.0: error
interrupt status=0xffffffff
[ 403.006905] sky2 0000:03:00.0: PCI hardware error (0xffff)
[ 403.013095] sky2 0000:03:00.0: PCI Express error (0xffffffff)
[ 403.019586] sky2 dmz: ram data read parity error
[ 403.024814] sky2 dmz: ram data write parity error
[ 403.030130] sky2 dmz: MAC parity error
[ 403.034392] sky2 dmz: RX parity error
[ 403.038560] sky2 dmz: TCP segmentation error
[ 403.043531] BUG: unable to handle kernel NULL pointer dereference at
0000038d
[ 403.047392] IP: [<f8078ce0>] sky2_mac_intr+0x30/0xc1 [sky2]
[ 403.047392] *pde = 00000000
[ 403.047392] Thread overran stack, or stack corrupted
[ 403.047392] Oops: 0000 [#1] PREEMPT SMP
[ 403.047392] last sysfs file:
/sys/devices/system/cpu/cpu0/cpufreq/scaling_setspeed
[ 403.047392] Modules linked in: xt_multiport cpufreq_userspace ip6t_REJECT
xt_DSCP xt_length xt_mark xt_dscp xt_MARK xt_IMQ xt_CONNMARK xt_comment
xt_policy ip6t_LOG xt_tcpudp ip6table_mangle iptable_mangle ip6table_filter
ip6_tables sit tunnel4 8021q garp stp llc ipt_LOG xt_limit xt_state
iptable_nat iptable_filter ip_tables x_tables dm_mod lm90 led_class p4_clockmod
speedstep_lib freq_table tun imq nf_nat_ftp nf_nat nf_conntrack_ftp
nf_conntrack_ipv6 nf_conntrack_ipv4 nf_conntrack nf_defrag_ipv4 ipv6 evdev
iTCO_wdt parport_pc parport i2c_i801 i2c_core serio_raw rng_core button
processor intel_agp pcspkr squashfs loop aufs exportfs nls_utf8 nls_cp437
ide_generic sd_mod ata_generic pata_acpi ata_piix ide_pci_generic ide_core
skge sky2 thermal fan thermal_sys
[ 403.047392]
[ 403.047392] Pid: 0, comm: swapper Not tainted (2.6.30.5 #20)
[ 403.047392] EIP: 0060:[<f8078ce0>] EFLAGS: 00010286 CPU: 0
[ 403.047392] EIP is at sky2_mac_intr+0x30/0xc1 [sky2]
[ 403.047392] EAX: f8098f88 EBX: 00000001 ECX: 00000008 EDX: 000000ff
[ 403.047392] ESI: 00000000 EDI: f68f5080 EBP: c1c8de3c ESP: c1c8de24
[ 403.191036] DS: 0068 ES: 0068 FS: 00d8 GS: 00e0 SS: 0068
[ 403.191036] Process swapper (pid: 0, ti=c1c8c000 task=c1c3f1d4
task.ti=c1c8c000)
[ 403.202984] Stack:
[ 403.202984] 00000080 ff8f5080 4aa8b206 f70e54c0 ffffffff ffffffff c1c8deb0 f807c33b
[ 403.202984] 00000000 c1c8dea4 00000040 f68f5088 c184526c 00000000 f68f5080
ffffffff
[ 403.202984] 00000001 f7224400 c2523140 00006497 00000001 00000000 4aa8b206
c25229c8
[ 403.202984] Call Trace:
[ 403.202984] [<f807c33b>] ? sky2_poll+0x1d2/0xa07 [sky2]
[ 403.202984] [<c184526c>] ? insert_work+0xa5/0xbf
[ 403.202984] [<c1af8f7f>] ? _spin_unlock_irq+0x2f/0x42
[ 403.202984] [<c1a6ebc5>] ? net_rx_action+0x9e/0x1ae
[ 403.202984] [<c1838e5e>] ? __do_softirq+0xb2/0x188
[ 403.202984] [<c1838f73>] ? do_softirq+0x3f/0x5c
[ 403.202984] [<c18390fd>] ? irq_exit+0x37/0x80
[ 403.202984] [<c18146fa>] ? smp_apic_timer_interrupt+0x7c/0x9b
[ 403.202984] [<c1803a31>] ? apic_timer_interrupt+0x31/0x40
[ 403.202984] [<c1840068>] ? complete_signal+0x97/0x1ac
[ 403.202984] [<c180a262>] ? mwait_idle+0xad/0x116
[ 403.202984] [<c1801e2f>] ? cpu_idle+0x96/0xc3
[ 403.202984] [<c1ae6629>] ? rest_init+0x75/0x88
[ 403.202984] [<c102aa10>] ? start_kernel+0x32d/0x343
[ 403.202984] [<c102a072>] ? _sinittext+0x72/0x88
[ 403.323244] Code: c7 56 53 89 d3 83 ec 0c 65 a1 14 00 00 00 89 45 f0 31 c0
8b 74 97 3c c1 e2 07 89 d0 05 08 0f 00 00 89 55 e8 03 07 8a 10 88 55 ef <f6>
86 8d 03 00 00 02 74 12 0f b6 c2 50 56 68 74 e3 07 f8 e8 e2
[ 403.323244] EIP: [<f8078ce0>] sky2_mac_intr+0x30/0xc1 [sky2] SS:ESP
0068:c1c8de24
[ 403.323244] CR2: 000000000000038d
[ 403.365775] ---[ end trace 7f4d83bf1130e18d ]---
[ 403.370999] Kernel panic - not syncing: Fatal exception in interrupt
[ 403.378154] Pid: 0, comm: swapper Tainted: G D 2.6.30.5 #20
[ 403.385185] Call Trace:
[ 403.388006] [<c1af5bf7>] ? printk+0x1d/0x30
[ 403.392861] [<c1af5b35>] panic+0x53/0xf8
[ 403.397404] [<c1806787>] oops_end+0x9f/0xbf
[ 403.402234] [<c181d7ef>] no_context+0x15d/0x178
[ 403.407443] [<c181db53>] __bad_area_nosemaphore+0x349/0x362
[ 403.413797] [<c197afed>] ? vsnprintf+0x2de/0x332
[ 403.419104] [<c197ada0>] ? vsnprintf+0x91/0x332
[ 403.424312] [<c1af8fc3>] ? _spin_unlock_irqrestore+0x31/0x44
[ 403.430773] [<c1af8fc3>] ? _spin_unlock_irqrestore+0x31/0x44
[ 403.437280] [<c18340f7>] ? release_console_sem+0x18b/0x1c9
[ 403.443583] [<c181db89>] bad_area_nosemaphore+0x1d/0x34
[ 403.449590] [<c181de56>] do_page_fault+0x10d/0x24b
[ 403.455119] [<c181dd49>] ? do_page_fault+0x0/0x24b
[ 403.460654] [<c1af945d>] error_code+0x7d/0x90
[ 403.465759] [<c1970068>] ? blk_alloc_devt+0x59/0xb9
[ 403.471421] [<f8078ce0>] ? sky2_mac_intr+0x30/0xc1 [sky2]
[ 403.477684] [<f807c33b>] sky2_poll+0x1d2/0xa07 [sky2]
[ 403.483502] [<c184526c>] ? insert_work+0xa5/0xbf
[ 403.488843] [<c1af8f7f>] ? _spin_unlock_irq+0x2f/0x42
[ 403.494664] [<c1a6ebc5>] net_rx_action+0x9e/0x1ae
[ 403.500102] [<c1838e5e>] __do_softirq+0xb2/0x188
Message from[ 403.505490] [<c1838f73>] do_softirq+0x3f/0x5c
syslogd@gibralt[ 403.511881] [<c18390fd>] irq_exit+0x37/0x80
ar3-esys-master [ 403.518089] [<c18146fa>]
smp_apic_timer_interrupt+0x7c/0x9b
at Aug 31 11:56:[ 403.525829] [<c1803a31>] apic_timer_interrupt+0x31/0x40
58 ...
kernel[ 403.533177] [<c1840068>] ? complete_signal+0x97/0x1ac
:[ 403.047392] [ 403.540336] [<c180a262>] ? mwait_idle+0xad/0x116
Oops: 0000 [#1] [ 403.547010] [<c1801e2f>] cpu_idle+0x96/0xc3
PREEMPT SMP
[ 403.553229] [<c1ae6629>] rest_init+0x75/0x88
Message from [ 403.559515] [<c102aa10>] start_kernel+0x32d/0x343
syslogd@gibralta[ 403.566324] [<c102a072>] _sinittext+0x72/0x88
r3-esys-master a[ 403.572752] Rebooting in 30 seconds..
Potentially the last system reset was similar but just didn't print the stack
trace for some reason (that is, on the previous reboot with the very same
sky2.ko loaded, the system just hard-locked and rebooted itself after 30s
without printing anything to the serial console).
best regards,
Rene
--
-------------------------------------------------
Gibraltar firewall http://www.gibraltar.at/
^ permalink raw reply
* Re: [PATCH] net-next:can: add TI CAN (HECC) driver
From: Oliver Hartkopp @ 2009-08-31 10:29 UTC (permalink / raw)
To: Wolfgang Grandegger, Jan Kiszka
Cc: Gole, Anant, Socketcan-core@lists.berlios.de, Linux Netdev List
In-Reply-To: <4A9B90FD.3000301@grandegger.com>
Wolfgang Grandegger wrote:
> Gole, Anant wrote:
>>>
>> Appreciate the review. I will send v2 of patch. Not sure still if the original patch reached the socketcan mailing list due to size.
>
> v2 will fit but I'm wondering who is moderating the ML. Oliver?
>
No. SocketCAN Core is Jan Kiszka.
Jan, can you increase the message size in the SocketCAN-core-ML settings to 150kB?
Regards,
Oliver
^ permalink raw reply
* Re: [PATCH resend] drop_monitor: fix trace_napi_poll_hit()
From: Neil Horman @ 2009-08-31 11:12 UTC (permalink / raw)
To: Eric Dumazet; +Cc: Xiao Guangrong, David Miller, Wei Yongjun, Netdev, LKML
In-Reply-To: <4A9B6E57.7090703@gmail.com>
On Mon, Aug 31, 2009 at 08:31:51AM +0200, Eric Dumazet wrote:
> Xiao Guangrong a écrit :
> > The net_dev of backlog napi is NULL, like below:
> >
> > __get_cpu_var(softnet_data).backlog.dev == NULL
> >
> > So, we should check it in napi tracepoint's probe function
> >
> > Acked-by: Neil Horman <nhorman@tuxdriver.com>
> > Signed-off-by: Xiao Guangrong <xiaoguangrong@cn.fujitsu.com>
> > ---
> > net/core/drop_monitor.c | 3 ++-
> > 1 files changed, 2 insertions(+), 1 deletions(-)
> >
> > diff --git a/net/core/drop_monitor.c b/net/core/drop_monitor.c
> > index 9d66fa9..d311202 100644
> > --- a/net/core/drop_monitor.c
> > +++ b/net/core/drop_monitor.c
> > @@ -182,7 +182,8 @@ static void trace_napi_poll_hit(struct napi_struct *napi)
> > /*
> > * Ratelimit our check time to dm_hw_check_delta jiffies
> > */
> > - if (!time_after(jiffies, napi->dev->last_rx + dm_hw_check_delta))
> > + if (!napi->dev ||
> > + !time_after(jiffies, napi->dev->last_rx + dm_hw_check_delta))
> > return;
> >
> > rcu_read_lock();
>
>
> This reminds me dev->last_rx is not anymore updated, unless special conditions
> are met.
>
I still see a large number of drivers that update dev->last_rx, although its
not all as I look through the list, so something definately seems amiss.
If its not going to be consistently updated, why are still carrying that field
in dev? Are we just waiting on someone to do the janitorial work to remove it?
If so, I can, and I'll fix up the drop monitor in the process, to use a private
timestamp.
Neil
> Test done in trace_napi_poll_hit() is probably not good, even with a non null napi->dev
> --
> To unsubscribe from this list: send the line "unsubscribe netdev" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
>
^ permalink raw reply
* Re: [PATCH] net-next:can: add TI CAN (HECC) driver
From: Jan Kiszka @ 2009-08-31 10:43 UTC (permalink / raw)
To: Oliver Hartkopp
Cc: Wolfgang Grandegger, Gole, Anant, Socketcan-core@lists.berlios.de,
Linux Netdev List
In-Reply-To: <4A9BA611.80505@hartkopp.net>
Oliver Hartkopp wrote:
> Wolfgang Grandegger wrote:
>> Gole, Anant wrote:
>
>
>>> Appreciate the review. I will send v2 of patch. Not sure still if the original patch reached the socketcan mailing list due to size.
>> v2 will fit but I'm wondering who is moderating the ML. Oliver?
>>
>
> No. SocketCAN Core is Jan Kiszka.
>
> Jan, can you increase the message size in the SocketCAN-core-ML settings to 150kB?
Done.
Jan
--
Siemens AG, Corporate Technology, CT SE 2
Corporate Competence Center Embedded Linux
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox