* [PATCH] Au1000 eth : fix ioctl handling
From: Florian Fainelli @ 2007-07-24 9:15 UTC (permalink / raw)
To: netdev
[-- Attachment #1.1: Type: text/plain, Size: 167 bytes --]
Hi all,
This patch fixes the handling of unsupported ioctls with the au1000_eth
driver.
Signed-off-by: Florian Fainelli <florian.fainelli@telecomint.eu>
--
[-- Attachment #1.2: au1000_eth_ioctl.patch --]
[-- Type: text/plain, Size: 818 bytes --]
diff --git a/drivers/net/au1000_eth.c b/drivers/net/au1000_eth.c
index c27cfce..99a1c61 100644
--- a/drivers/net/au1000_eth.c
+++ b/drivers/net/au1000_eth.c
@@ -1316,12 +1316,20 @@ static void set_rx_mode(struct net_device *dev)
static int au1000_ioctl(struct net_device *dev, struct ifreq *rq, int cmd)
{
struct au1000_private *aup = (struct au1000_private *)dev->priv;
+ struct mii_ioctl_data *data = if_mii(rq);
+ int rc = -EOPNOTSUPP;
if (!netif_running(dev)) return -EINVAL;
if (!aup->phy_dev) return -EINVAL; // PHY not controllable
- return phy_mii_ioctl(aup->phy_dev, if_mii(rq), cmd);
+ switch (cmd) {
+ default:
+ rc = phy_mii_ioctl(aup->phy_dev, data, cmd);
+ break;
+ }
+
+ return rc;
}
static struct net_device_stats *au1000_get_stats(struct net_device *dev)
[-- Attachment #2: This is a digitally signed message part. --]
[-- Type: application/pgp-signature, Size: 189 bytes --]
^ permalink raw reply related
* [GENETLINK]: Fix race in genl_unregister_mc_groups()
From: Thomas Graf @ 2007-07-24 9:45 UTC (permalink / raw)
To: davem; +Cc: johannes, netdev
family->mcast_groups is protected by genl_lock so it must
be held while accessing the list in genl_unregister_mc_groups().
Requires adding a non-locking variant of genl_unregister_mc_group().
Signed-off-by: Thomas Graf <tgraf@suug.ch>
Index: net-2.6/net/netlink/genetlink.c
===================================================================
--- net-2.6.orig/net/netlink/genetlink.c 2007-07-23 22:08:04.000000000 +0200
+++ net-2.6/net/netlink/genetlink.c 2007-07-23 22:09:08.000000000 +0200
@@ -200,6 +200,18 @@ int genl_register_mc_group(struct genl_f
}
EXPORT_SYMBOL(genl_register_mc_group);
+static void __genl_unregister_mc_group(struct genl_family *family,
+ struct genl_multicast_group *grp)
+{
+ BUG_ON(grp->family != family);
+ netlink_clear_multicast_users(genl_sock, grp->id);
+ clear_bit(grp->id, mc_groups);
+ list_del(&grp->list);
+ genl_ctrl_event(CTRL_CMD_DELMCAST_GRP, grp);
+ grp->id = 0;
+ grp->family = NULL;
+}
+
/**
* genl_unregister_mc_group - unregister a multicast group
*
@@ -217,14 +229,8 @@ EXPORT_SYMBOL(genl_register_mc_group);
void genl_unregister_mc_group(struct genl_family *family,
struct genl_multicast_group *grp)
{
- BUG_ON(grp->family != family);
genl_lock();
- netlink_clear_multicast_users(genl_sock, grp->id);
- clear_bit(grp->id, mc_groups);
- list_del(&grp->list);
- genl_ctrl_event(CTRL_CMD_DELMCAST_GRP, grp);
- grp->id = 0;
- grp->family = NULL;
+ genl_unregister_mc_group(family, grp);
genl_unlock();
}
@@ -232,8 +238,10 @@ static void genl_unregister_mc_groups(st
{
struct genl_multicast_group *grp, *tmp;
+ genl_lock();
list_for_each_entry_safe(grp, tmp, &family->mcast_groups, list)
- genl_unregister_mc_group(family, grp);
+ __genl_unregister_mc_group(family, grp);
+ genl_unlock();
}
/**
^ permalink raw reply
* [GENETLINK]: Fix adjustment of number of multicast groups
From: Thomas Graf @ 2007-07-24 9:45 UTC (permalink / raw)
To: davem; +Cc: johannes, netdev
The current calculation of the maximum number of genetlink
multicast groups seems odd, fix it.
Signed-off-by: Thomas Graf <tgraf@suug.ch>
Index: net-2.6/net/netlink/genetlink.c
===================================================================
--- net-2.6.orig/net/netlink/genetlink.c 2007-07-23 22:03:02.000000000 +0200
+++ net-2.6/net/netlink/genetlink.c 2007-07-23 22:05:12.000000000 +0200
@@ -184,7 +184,7 @@ int genl_register_mc_group(struct genl_f
}
err = netlink_change_ngroups(genl_sock,
- sizeof(unsigned long) * NETLINK_GENERIC);
+ mc_groups_longs * BITS_PER_LONG);
if (err)
goto out;
^ permalink raw reply
* [GENETLINK]: Correctly report errors while registering a multicast group
From: Thomas Graf @ 2007-07-24 9:44 UTC (permalink / raw)
To: davem; +Cc: johannes, netdev
Signed-off-by: Thomas Graf <tgraf@suug.ch>
Index: net-2.6/net/netlink/genetlink.c
===================================================================
--- net-2.6.orig/net/netlink/genetlink.c 2007-07-23 21:54:35.000000000 +0200
+++ net-2.6/net/netlink/genetlink.c 2007-07-23 21:54:54.000000000 +0200
@@ -196,7 +196,7 @@ int genl_register_mc_group(struct genl_f
genl_ctrl_event(CTRL_CMD_NEWMCAST_GRP, grp);
out:
genl_unlock();
- return 0;
+ return err;
}
EXPORT_SYMBOL(genl_register_mc_group);
^ permalink raw reply
* Re: 2.6.20->2.6.21 - networking dies after random time
From: Ingo Molnar @ 2007-07-24 9:42 UTC (permalink / raw)
To: Marcin ??lusarz
Cc: Jarek Poplawski, Jean-Baptiste Vignaud, linux-kernel, shemminger,
linux-net, netdev, Thomas Gleixner, Andrew Morton, Linus Torvalds
In-Reply-To: <20070724080534.GC18740@elte.hu>
* Ingo Molnar <mingo@elte.hu> wrote:
> thanks for tracking it down! Could you try the patch below (ontop an
> otherwise unmodified kernel)? This tests the theory whether the
> problem is related to the disable_irq_nosync() call in the ne2k
> driver's xmit path. Does this solve the hangs too?
please try the patch below instead.
Ingo
Index: linux/kernel/irq/chip.c
===================================================================
--- linux.orig/kernel/irq/chip.c
+++ linux/kernel/irq/chip.c
@@ -231,7 +231,7 @@ static void default_enable(unsigned int
/*
* default disable function
*/
-static void default_disable(unsigned int irq)
+void default_disable(unsigned int irq)
{
}
Index: linux/kernel/irq/internals.h
===================================================================
--- linux.orig/kernel/irq/internals.h
+++ linux/kernel/irq/internals.h
@@ -10,6 +10,8 @@ extern void irq_chip_set_defaults(struct
/* Set default handler: */
extern void compat_irq_chip_set_default_handler(struct irq_desc *desc);
+extern void default_disable(unsigned int irq);
+
#ifdef CONFIG_PROC_FS
extern void register_irq_proc(unsigned int irq);
extern void register_handler_proc(unsigned int irq, struct irqaction *action);
Index: linux/kernel/irq/manage.c
===================================================================
--- linux.orig/kernel/irq/manage.c
+++ linux/kernel/irq/manage.c
@@ -102,7 +102,19 @@ void disable_irq_nosync(unsigned int irq
spin_lock_irqsave(&desc->lock, flags);
if (!desc->depth++) {
desc->status |= IRQ_DISABLED;
- desc->chip->disable(irq);
+ /*
+ * the _nosync variant of irq-disable suggests that the
+ * caller is not worried about concurrency but about the
+ * ordering of the irq flow itself. (such as hardware
+ * getting confused about certain, normally valid irq
+ * handling sequences.) So if the default disable handler
+ * is in place then try the more conservative masking
+ * instead:
+ */
+ if (desc->chip->disable == default_disable && desc->chip->mask)
+ desc->chip->mask(irq);
+ else
+ desc->chip->disable(irq);
}
spin_unlock_irqrestore(&desc->lock, flags);
}
^ permalink raw reply
* Re: 2.6.23-rc1 sky2 boot crash in sky2_mac_intr
From: Florian Lohoff @ 2007-07-24 9:40 UTC (permalink / raw)
To: Stephen Hemminger; +Cc: linux-kernel, netdev
In-Reply-To: <20070724095008.0eb154fe@oldman.hamilton.local>
[-- Attachment #1: Type: text/plain, Size: 1785 bytes --]
On Tue, Jul 24, 2007 at 09:50:08AM +0100, Stephen Hemminger wrote:
> The problem is related to power management. The PHY has a number of PCI configuration
> registers for power control, and the function of these changes based on the version and
> revision of the chip. The driver does work on older versions of the EC-U, in
> Fujitsu laptop's, it is just the new rev that is broken.
>
> The driver should probably fail smarter (by not loading) if the PHY isn't powered
> up correctly, but that doesn't help your problem.
>
> The vendor has provided me with documentation on many versions
> of the chip, but I don't have doc's on the lastest revision differences of the EC Ultra,
> so a proper solution is not easily available. The best method for resolving this would
> be to first try the vendor driver version of sk98lin and see if that fixes it. If so,
> then it is easy to change sky2, to match the phy setup in the vendor driver.
> Another possibility is to look for places in sky2 driver where there are places
> that compare version/revision.
>
> The most likely bits that need to change are in PCI registers: 0x80, 0x84 and 0x88
> You could also load the windows driver and dump PCI config space (with lspci from
> cygwin), and see what the settings are there.
>
> I am away from my office for a month, and therefore away from any sky2
> hardware for testing.
I'll try the above and keep you posted. The crash itself seems to be a
2.6.23-rc1 regression though. I never experienced this with 2.6.22-rc5
which i was running before.
Flo
--
Florian Lohoff flo@rfc822.org +49-171-2280134
Those who would give up a little freedom to get a little
security shall soon have neither - Benjamin Franklin
[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 189 bytes --]
^ permalink raw reply
* Re: [GENETLINK]: Question: global lock (genl_mutex) possible refinement?
From: Thomas Graf @ 2007-07-24 9:35 UTC (permalink / raw)
To: Richard MUSIL; +Cc: Patrick McHardy, netdev
In-Reply-To: <46A4DB34.4040502@st.com>
* Richard MUSIL <richard.musil@st.com> 2007-07-23 18:45
> I have been giving it a second thought and came up with something more
> complex. The idea is to have locking granularity at the level of
> individual families.
I agree in general, it would make up a better solution.
However, your initial patch allows operations and families to be
unregistered while message of the same family are being processed
which must not be allowed.
Please provide a new overall patch which is not based on your
initial patch so I can review your idea properly.
^ permalink raw reply
* Re: [PATCH 4/4] Initialize and fill IPv6 route age
From: Varun Chandramohan @ 2007-07-24 9:19 UTC (permalink / raw)
To: Stephen Hemminger; +Cc: netdev, sri, dlstevens, varuncha
In-Reply-To: <20070724082646.4cbd1ede@oldman>
Stephen Hemminger wrote:
> On Tue, 24 Jul 2007 09:50:57 +0530
> Varun Chandramohan <varunc@linux.vnet.ibm.com> wrote:
>
>
>> Stephen Hemminger wrote:
>>
>>> On Mon, 23 Jul 2007 10:13:18 +0530
>>> Varun Chandramohan <varunc@linux.vnet.ibm.com> wrote:
>>>
>>>
>>>
>>>> The age field of the ipv6 route structures are initilized with the current timeval at the time of route creation. When the route dump is called the route age value stored in the structure is subtracted from the present timeval and the difference is passed on as the route age.
>>>>
>>>> Signed-off-by: Varun Chandramohan <varunc@linux.vnet.ibm.com>
>>>> ---
>>>> include/net/ip6_fib.h | 1 +
>>>> include/net/ip6_route.h | 3 +++
>>>> net/ipv6/addrconf.c | 5 +++++
>>>> net/ipv6/route.c | 23 +++++++++++++++++++----
>>>> 4 files changed, 28 insertions(+), 4 deletions(-)
>>>>
>>>> diff --git a/include/net/ip6_fib.h b/include/net/ip6_fib.h
>>>> index c48ea87..e30a1cf 100644
>>>> --- a/include/net/ip6_fib.h
>>>> +++ b/include/net/ip6_fib.h
>>>> @@ -98,6 +98,7 @@ struct rt6_info
>>>>
>>>> u32 rt6i_flags;
>>>> u32 rt6i_metric;
>>>> + time_t rt6i_age;
>>>> atomic_t rt6i_ref;
>>>> struct fib6_table *rt6i_table;
>>>>
>>>> diff --git a/include/net/ip6_route.h b/include/net/ip6_route.h
>>>> index 5456fdd..fc9716c 100644
>>>> --- a/include/net/ip6_route.h
>>>> +++ b/include/net/ip6_route.h
>>>> @@ -36,6 +36,9 @@ struct route_info {
>>>> #define RT6_LOOKUP_F_REACHABLE 0x2
>>>> #define RT6_LOOKUP_F_HAS_SADDR 0x4
>>>>
>>>> +#define RT6_SET_ROUTE_INFO 0x0
>>>> +#define RT6_GET_ROUTE_INFO 0x1
>>>> +
>>>> extern struct rt6_info ip6_null_entry;
>>>>
>>>> #ifdef CONFIG_IPV6_MULTIPLE_TABLES
>>>> diff --git a/net/ipv6/addrconf.c b/net/ipv6/addrconf.c
>>>> index 5a5f8bd..715c766 100644
>>>> --- a/net/ipv6/addrconf.c
>>>> +++ b/net/ipv6/addrconf.c
>>>> @@ -4187,6 +4187,7 @@ EXPORT_SYMBOL(unregister_inet6addr_notif
>>>>
>>>> int __init addrconf_init(void)
>>>> {
>>>> + struct timeval tv;
>>>> int err = 0;
>>>>
>>>> /* The addrconf netdev notifier requires that loopback_dev
>>>> @@ -4214,10 +4215,14 @@ int __init addrconf_init(void)
>>>> if (err)
>>>> return err;
>>>>
>>>> + do_gettimeofday(&tv);
>>>>
>>>>
>>> Better to use ktime_t or timespec in new code.
>>>
>>>
>> You are saying not to use timeval as its going to be removed sometime in
>> future? If not, may i know why should we use timespec or ktime?
>> I need only seconds granularity so i was wondering if that matters.
>>
>>
>
> Timeval isn't going away, but ktime is easier for basic maths.
> If all you need is seconds resolution, just use jiffies. Gettimeofday
>
Thanks stephen for the review. As far as using jiffies (which is my
original implementation) i got a comment saying that on 32bit platform
jiffies wrap every 49 days. That's the reason for the do_gettimeofday()
implementation. However i understand that such calls require expensive
h/w accesses. Considering the fact that jiffies wrap around rather
quickly, can you suggest any other way of doing it?
> (and ktime_get) both require expensive hardware accesses on
> some platforms.
>
> For the specific case IPV6 route ageing, this isn't a big deal it is just
> that people tend to copy code.
> -
> 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: Stalled connection (need help to debug)
From: Jarek Poplawski @ 2007-07-24 9:02 UTC (permalink / raw)
To: Oleg Verych; +Cc: netdev
In-Reply-To: <slrnf9sp11.anq.for.gmane@flower.upol.cz>
On 18-07-2007 20:50, Oleg Verych wrote:
>> Hallo.
>>
>> I have a very strange problem.
> []
>> Any advise on how to debug this will be very appreciated. Thanks.
>
> Thanks...
You're quite welcome...
Cheers,
Jarek P.
PS: Oleg, no offence, but are you sure this was the right problem
and the right list?
^ permalink raw reply
* Re: 2.6.23-rc1 sky2 boot crash in sky2_mac_intr
From: Stephen Hemminger @ 2007-07-24 8:50 UTC (permalink / raw)
To: Florian Lohoff; +Cc: linux-kernel, netdev
In-Reply-To: <20070724082205.GA19480@paradigm.rfc822.org>
On Tue, 24 Jul 2007 10:22:05 +0200
Florian Lohoff <flo@rfc822.org> wrote:
>
> Hi,
> i am seeing irregular crashes on boot in the sky2_mac_intr. This is an
> Fujitsu Siemens Lifebook E8110 with a Core Duo. Currently i suspect some
> strange BIOS issues as the issues i see with the sky2 aka parity errors
> etc i also see sometimes with the integrated ipw3945 which complains
> about firmware errors. It seems the BIOS randomly fails to initialize
> all the hardware. To reproduce this crash and catch it on the serial
> console it took me around 12 boots. The machine is stable once correctly
> booted and i work most of the day on it.
>
> [ 46.479939] ACPI: PCI Interrupt 0000:02:00.0[A] -> GSI 16 (level, low) -> IRQ 19
> [ 46.568569] sky2 0000:02:00.0: v1.16 addr 0xf0000000 irq 19 Yukon-EC Ultra (0xb4) rev 2
> [ 46.664555] sky2 eth0: addr 00:17:42:13:45:8c
>
> [ 61.958741] sky2 eth1: enabling interface
> [ 62.010834] sky2 eth1: phy write timeout
The problem is related to power management. The PHY has a number of PCI configuration
registers for power control, and the function of these changes based on the version and
revision of the chip. The driver does work on older versions of the EC-U, in
Fujitsu laptop's, it is just the new rev that is broken.
The driver should probably fail smarter (by not loading) if the PHY isn't powered
up correctly, but that doesn't help your problem.
The vendor has provided me with documentation on many versions
of the chip, but I don't have doc's on the lastest revision differences of the EC Ultra,
so a proper solution is not easily available. The best method for resolving this would
be to first try the vendor driver version of sk98lin and see if that fixes it. If so,
then it is easy to change sky2, to match the phy setup in the vendor driver.
Another possibility is to look for places in sky2 driver where there are places
that compare version/revision.
The most likely bits that need to change are in PCI registers: 0x80, 0x84 and 0x88
You could also load the windows driver and dump PCI config space (with lspci from
cygwin), and see what the settings are there.
I am away from my office for a month, and therefore away from any sky2
hardware for testing.
^ permalink raw reply
* Re: 2.6.23-rc1: BUG_ON in kmap_atomic_prot()
From: Andrew Morton @ 2007-07-24 8:34 UTC (permalink / raw)
To: Jens Axboe
Cc: Alexey Dobriyan, Linus Torvalds, linux-kernel, netdev,
mark.fasheh, dan.j.williams, Nelson, Shannon
In-Reply-To: <20070724082207.GN3287@kernel.dk>
On Tue, 24 Jul 2007 10:22:07 +0200 Jens Axboe <jens.axboe@oracle.com> wrote:
> On Tue, Jul 24 2007, Jens Axboe wrote:
> > On Mon, Jul 23 2007, Andrew Morton wrote:
> > > I worked out that the crash I saw was in
> > >
> > > BUG_ON(!pte_none(*(kmap_pte-idx)));
> > >
> > > in the read of kmap_pte[idx]. Which would be weird as the caller is using
> > > a literal KM_USER0.
> > >
> > > So maybe I goofed, and that BUG_ON is triggering (it scrolled off, and I am
> > > unable to reproduce it now).
> > >
> > > If that BUG_ON _is_ triggering then it might indicate that someone is doing
> > > a __GFP_HIGHMEM|__GFP_ZERO allocation while holding KM_USER0.
> >
> > Or doing double kunmaps, or doing a kunmap_atomic() on the page, not the
> > address. I've seen both of those end up triggering that BUG_ON() in a
> > later kmap.
> >
> > Looking over the 2.6.22..2.6.23-rc1 diff, I found one such error in
> > ocfs2 at least. But you are probably not using that, so I'll keep
> > looking...
>
> What about the new async crypto stuff? I've been looking, but is it
> guarenteed that async_memcpy() runs in process context with interrupts
> enabled always? If not, there's a km type bug there.
I think Shannon maintains that now.
> In general, I think the highmem stuff could do with more safety checks:
>
> - People ALWAYS get the atomic unmaps wrong, passing in the page instead
> of the address. I've seen tons of these. And since kunmap_atomic()
> takes a void pointer, nobody notices until it goes boom.
yeah, it's a real trap. For a while I had a patch which converted
kmap_atomic() to return a char*, and kunmap_atomic() to take a char*, so
misuse got compile warnings. But it was a pig to maintain so I tossed it.
It'd be somewhat easier to do now we've converted a lot of callers to
clear_user_highpage() and similar.
> - People easily get the km type wrong - they use KM_USERx in interrupt
> context, or one of the irq variants without disabling interrupts.
>
> If we could just catch these two types of bugs, we've got a lot of these
> problems covered.
Here's the -mm debug patch:
diff -puN arch/i386/mm/highmem.c~kmap_atomic-debugging arch/i386/mm/highmem.c
--- a/arch/i386/mm/highmem.c~kmap_atomic-debugging
+++ a/arch/i386/mm/highmem.c
@@ -30,7 +30,44 @@ void *kmap_atomic(struct page *page, enu
{
enum fixed_addresses idx;
unsigned long vaddr;
+ static unsigned warn_count = 10;
+ if (unlikely(warn_count == 0))
+ goto skip;
+
+ if (unlikely(in_interrupt())) {
+ if (in_irq()) {
+ if (type != KM_IRQ0 && type != KM_IRQ1 &&
+ type != KM_BIO_SRC_IRQ && type != KM_BIO_DST_IRQ &&
+ type != KM_BOUNCE_READ) {
+ WARN_ON(1);
+ warn_count--;
+ }
+ } else if (!irqs_disabled()) { /* softirq */
+ if (type != KM_IRQ0 && type != KM_IRQ1 &&
+ type != KM_SOFTIRQ0 && type != KM_SOFTIRQ1 &&
+ type != KM_SKB_SUNRPC_DATA &&
+ type != KM_SKB_DATA_SOFTIRQ &&
+ type != KM_BOUNCE_READ) {
+ WARN_ON(1);
+ warn_count--;
+ }
+ }
+ }
+
+ if (type == KM_IRQ0 || type == KM_IRQ1 || type == KM_BOUNCE_READ ||
+ type == KM_BIO_SRC_IRQ || type == KM_BIO_DST_IRQ) {
+ if (!irqs_disabled()) {
+ WARN_ON(1);
+ warn_count--;
+ }
+ } else if (type == KM_SOFTIRQ0 || type == KM_SOFTIRQ1) {
+ if (irq_count() == 0 && !irqs_disabled()) {
+ WARN_ON(1);
+ warn_count--;
+ }
+ }
+skip:
/* even !CONFIG_PREEMPT needs this, for in_atomic in do_page_fault */
pagefault_disable();
_
^ permalink raw reply
* Re: 2.6.23-rc1: BUG_ON in kmap_atomic_prot()
From: Jens Axboe @ 2007-07-24 8:22 UTC (permalink / raw)
To: Andrew Morton
Cc: Alexey Dobriyan, Linus Torvalds, linux-kernel, netdev,
mark.fasheh, dan.j.williams
In-Reply-To: <20070724081750.GM3287@kernel.dk>
On Tue, Jul 24 2007, Jens Axboe wrote:
> On Mon, Jul 23 2007, Andrew Morton wrote:
> > I worked out that the crash I saw was in
> >
> > BUG_ON(!pte_none(*(kmap_pte-idx)));
> >
> > in the read of kmap_pte[idx]. Which would be weird as the caller is using
> > a literal KM_USER0.
> >
> > So maybe I goofed, and that BUG_ON is triggering (it scrolled off, and I am
> > unable to reproduce it now).
> >
> > If that BUG_ON _is_ triggering then it might indicate that someone is doing
> > a __GFP_HIGHMEM|__GFP_ZERO allocation while holding KM_USER0.
>
> Or doing double kunmaps, or doing a kunmap_atomic() on the page, not the
> address. I've seen both of those end up triggering that BUG_ON() in a
> later kmap.
>
> Looking over the 2.6.22..2.6.23-rc1 diff, I found one such error in
> ocfs2 at least. But you are probably not using that, so I'll keep
> looking...
What about the new async crypto stuff? I've been looking, but is it
guarenteed that async_memcpy() runs in process context with interrupts
enabled always? If not, there's a km type bug there.
In general, I think the highmem stuff could do with more safety checks:
- People ALWAYS get the atomic unmaps wrong, passing in the page instead
of the address. I've seen tons of these. And since kunmap_atomic()
takes a void pointer, nobody notices until it goes boom.
- People easily get the km type wrong - they use KM_USERx in interrupt
context, or one of the irq variants without disabling interrupts.
If we could just catch these two types of bugs, we've got a lot of these
problems covered.
--
Jens Axboe
^ permalink raw reply
* [PATCH] possible deadlock in tulip driver
From: Denis V. Lunev @ 2007-07-24 7:49 UTC (permalink / raw)
To: netdev, davem, val; +Cc: tulip-users, devel
Calling flush_scheduled_work() may deadlock if called under rtnl_lock
(from dev->stop) as linkwatch_event() may be on the workqueue and it will try
to get the rtnl_lock
Signed-off-by: Denis V. Lunev <den@openvz.org>
---
tulip_core.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
--- ./drivers/net/tulip/tulip_core.c.tulip 2007-07-16 12:54:29.000000000 +0400
+++ ./drivers/net/tulip/tulip_core.c 2007-07-23 19:06:24.000000000 +0400
@@ -726,8 +726,6 @@ static void tulip_down (struct net_devic
void __iomem *ioaddr = tp->base_addr;
unsigned long flags;
- flush_scheduled_work();
-
del_timer_sync (&tp->timer);
#ifdef CONFIG_TULIP_NAPI
del_timer_sync (&tp->oom_timer);
@@ -1788,6 +1786,8 @@ static void __devexit tulip_remove_one (
if (!dev)
return;
+ flush_scheduled_work();
+
tp = netdev_priv(dev);
unregister_netdev(dev);
pci_free_consistent (pdev,
^ permalink raw reply
* Re: 2.6.23-rc1: BUG_ON in kmap_atomic_prot()
From: Jens Axboe @ 2007-07-24 8:17 UTC (permalink / raw)
To: Andrew Morton
Cc: Alexey Dobriyan, Linus Torvalds, linux-kernel, netdev,
mark.fasheh
In-Reply-To: <20070723152712.02ded067.akpm@linux-foundation.org>
On Mon, Jul 23 2007, Andrew Morton wrote:
> I worked out that the crash I saw was in
>
> BUG_ON(!pte_none(*(kmap_pte-idx)));
>
> in the read of kmap_pte[idx]. Which would be weird as the caller is using
> a literal KM_USER0.
>
> So maybe I goofed, and that BUG_ON is triggering (it scrolled off, and I am
> unable to reproduce it now).
>
> If that BUG_ON _is_ triggering then it might indicate that someone is doing
> a __GFP_HIGHMEM|__GFP_ZERO allocation while holding KM_USER0.
Or doing double kunmaps, or doing a kunmap_atomic() on the page, not the
address. I've seen both of those end up triggering that BUG_ON() in a
later kmap.
Looking over the 2.6.22..2.6.23-rc1 diff, I found one such error in
ocfs2 at least. But you are probably not using that, so I'll keep
looking...
---
[PATCH] ocfs2: bad kunmap_atomic()
kunmap_atomic() takes the virtual address, not the mapped page as
argument.
Signed-off-by: Jens Axboe <jens.axboe@oracle.com>
diff --git a/fs/ocfs2/file.c b/fs/ocfs2/file.c
index 5727cd1..c4034f6 100644
--- a/fs/ocfs2/file.c
+++ b/fs/ocfs2/file.c
@@ -2153,7 +2153,7 @@ static int ocfs2_splice_write_actor(struct pipe_inode_info *pipe,
src = buf->ops->map(pipe, buf, 1);
dst = kmap_atomic(page, KM_USER1);
memcpy(dst + offset, src + buf->offset, count);
- kunmap_atomic(page, KM_USER1);
+ kunmap_atomic(dst, KM_USER1);
buf->ops->unmap(pipe, buf, src);
copied = ocfs2_write_end(file, file->f_mapping, sd->pos, count, count,
--
Jens Axboe
^ permalink raw reply related
* Re: 2.6.20->2.6.21 - networking dies after random time
From: Ingo Molnar @ 2007-07-24 8:05 UTC (permalink / raw)
To: Marcin Ślusarz
Cc: Jarek Poplawski, Jean-Baptiste Vignaud, linux-kernel, shemminger,
linux-net, netdev, Thomas Gleixner, Andrew Morton, Linus Torvalds
In-Reply-To: <4bacf17f0707222244p664e7a6ap850b3357a57d73c@mail.gmail.com>
* Marcin Ślusarz <marcin.slusarz@gmail.com> wrote:
> Ok, I've bisected this problem and found that this patch broke my NIC:
>
> 76d2160147f43f982dfe881404cfde9fd0a9da21 is first bad commit
> commit 76d2160147f43f982dfe881404cfde9fd0a9da21
> Author: Ingo Molnar <mingo@elte.hu>
> Date: Fri Feb 16 01:28:24 2007 -0800
>
> [PATCH] genirq: do not mask interrupts by default
thanks for tracking it down! Could you try the patch below (ontop an
otherwise unmodified kernel)? This tests the theory whether the problem
is related to the disable_irq_nosync() call in the ne2k driver's xmit
path. Does this solve the hangs too?
Ingo
Index: linux/kernel/irq/manage.c
===================================================================
--- linux.orig/kernel/irq/manage.c
+++ linux/kernel/irq/manage.c
@@ -102,7 +102,19 @@ void disable_irq_nosync(unsigned int irq
spin_lock_irqsave(&desc->lock, flags);
if (!desc->depth++) {
desc->status |= IRQ_DISABLED;
- desc->chip->disable(irq);
+ /*
+ * the _nosync variant of irq-disable suggests that the
+ * caller is not worried about concurrency but about the
+ * ordering of the irq flow itself. (such as hardware
+ * getting confused about certain, normally valid irq
+ * handling sequences.) So if the default disable handler
+ * is in place then try the more conservative masking
+ * instead:
+ */
+ if (desc->chip->disable == default_disable && desc->chip->mask)
+ desc->chip->mask(irq);
+ else
+ desc->chip->disable(irq);
}
spin_unlock_irqrestore(&desc->lock, flags);
}
^ permalink raw reply
* Re: [PATCH RFX]: napi_struct V3
From: David Miller @ 2007-07-24 7:35 UTC (permalink / raw)
To: romieu; +Cc: netdev, shemminger, rusty, jgarzik
In-Reply-To: <20070724071201.GA7134@electric-eye.fr.zoreil.com>
From: Francois Romieu <romieu@fr.zoreil.com>
Date: Tue, 24 Jul 2007 09:12:01 +0200
> David Miller <davem@davemloft.net> :
> [...]
> > I also didn't play with turning off NAPI in kconfig where drivers
> > allow that, can we just get rid of that crap already? :-/
>
> Would it be accepted for 2.6.23 or must it be considered post-2.6.23 ?
The 2.6.23 merge window closed already, so this is of course
2.6.24 material.
^ permalink raw reply
* Re: [PATCH RFX]: napi_struct V3
From: Jeff Garzik @ 2007-07-24 7:33 UTC (permalink / raw)
To: Francois Romieu; +Cc: David Miller, netdev, shemminger, rusty
In-Reply-To: <20070724071201.GA7134@electric-eye.fr.zoreil.com>
Francois Romieu wrote:
> David Miller <davem@davemloft.net> :
> [...]
>> I also didn't play with turning off NAPI in kconfig where drivers
>> allow that, can we just get rid of that crap already? :-/
>
> Would it be accepted for 2.6.23 or must it be considered post-2.6.23 ?
The merge window is closed, so not 2.6.23.
The cases where Kconfig allows selection of NAPI-or-not is generally
either (a) the author considers one path new and relatively unproven or
(b) the author has not bothered to figure out which is the best choice,
and instead just offers both rather than doing the legwork to find out.
Either way, individual cases must be examined and it is not necessarily
just a cleanup.
Jeff
^ permalink raw reply
* Re: tg3 issues
From: patric @ 2007-07-24 7:33 UTC (permalink / raw)
To: Michael Chan; +Cc: netdev, mcarlson
In-Reply-To: <1185226496.7922.33.camel@dell>
Michael Chan wrote:
>> Last update on this.
>>
>> "udelay( 100 + net_random % 300 )" seems to work much better and i have
>> not had a single problem getting the link up within 10 seconds of a cold
>> or warm-boot, and most often the link comes up directly without any sort
>> of delay instead like before when it could hang for 30 seconds before
>> getting a link, if you even got a link.
>>
>>
>
> We'll have to do some testing to see if we can find a better solution.
> Adding up to 400 usec of busy wait is not ideal. Are you connecting two
> 5701 fiber cards directly to each other in your setup?
>
>
>
Hi,
Yea, it's and ugly hack, but it's a workaround that at least works for me.
Have done some additional tests and to me it seems that the driver just
needs to wait a bit longer to detect the link + some random time to get
around the issues it might have when chasing the other systems state.
Don't have the numbers in front of me, but i did some tests to get the
'optimum' delay to wait and it seems like the larger the wait time (even
up to around 40-50ms!) causes the autoneg to go much smoother and
faster. And remember that the link can be reported up, but no traffic
can be passed via the link before the autoneg is complete and both cards
reports back that flowcontrol is enabled so you need to verify the link
by trying to send some data and not just look at the link-status.
Hope my conclusions might help, or atleast point you in the right direction.
I have 2
01:08.0 Ethernet controller: Broadcom Corporation NetXtreme BCM5701
Gigabit Ethernet (rev 15) ( 14e4:1645 )
connected via a single FC cable.
And the two systems are one single-core and one dual-core AMD system.
Regards,
Patric
^ permalink raw reply
* Re: [PATCH 4/4] Initialize and fill IPv6 route age
From: Stephen Hemminger @ 2007-07-24 7:26 UTC (permalink / raw)
To: Varun Chandramohan; +Cc: netdev, sri, dlstevens, varuncha
In-Reply-To: <46A57E29.9040404@linux.vnet.ibm.com>
On Tue, 24 Jul 2007 09:50:57 +0530
Varun Chandramohan <varunc@linux.vnet.ibm.com> wrote:
> Stephen Hemminger wrote:
> > On Mon, 23 Jul 2007 10:13:18 +0530
> > Varun Chandramohan <varunc@linux.vnet.ibm.com> wrote:
> >
> >
> >> The age field of the ipv6 route structures are initilized with the current timeval at the time of route creation. When the route dump is called the route age value stored in the structure is subtracted from the present timeval and the difference is passed on as the route age.
> >>
> >> Signed-off-by: Varun Chandramohan <varunc@linux.vnet.ibm.com>
> >> ---
> >> include/net/ip6_fib.h | 1 +
> >> include/net/ip6_route.h | 3 +++
> >> net/ipv6/addrconf.c | 5 +++++
> >> net/ipv6/route.c | 23 +++++++++++++++++++----
> >> 4 files changed, 28 insertions(+), 4 deletions(-)
> >>
> >> diff --git a/include/net/ip6_fib.h b/include/net/ip6_fib.h
> >> index c48ea87..e30a1cf 100644
> >> --- a/include/net/ip6_fib.h
> >> +++ b/include/net/ip6_fib.h
> >> @@ -98,6 +98,7 @@ struct rt6_info
> >>
> >> u32 rt6i_flags;
> >> u32 rt6i_metric;
> >> + time_t rt6i_age;
> >> atomic_t rt6i_ref;
> >> struct fib6_table *rt6i_table;
> >>
> >> diff --git a/include/net/ip6_route.h b/include/net/ip6_route.h
> >> index 5456fdd..fc9716c 100644
> >> --- a/include/net/ip6_route.h
> >> +++ b/include/net/ip6_route.h
> >> @@ -36,6 +36,9 @@ struct route_info {
> >> #define RT6_LOOKUP_F_REACHABLE 0x2
> >> #define RT6_LOOKUP_F_HAS_SADDR 0x4
> >>
> >> +#define RT6_SET_ROUTE_INFO 0x0
> >> +#define RT6_GET_ROUTE_INFO 0x1
> >> +
> >> extern struct rt6_info ip6_null_entry;
> >>
> >> #ifdef CONFIG_IPV6_MULTIPLE_TABLES
> >> diff --git a/net/ipv6/addrconf.c b/net/ipv6/addrconf.c
> >> index 5a5f8bd..715c766 100644
> >> --- a/net/ipv6/addrconf.c
> >> +++ b/net/ipv6/addrconf.c
> >> @@ -4187,6 +4187,7 @@ EXPORT_SYMBOL(unregister_inet6addr_notif
> >>
> >> int __init addrconf_init(void)
> >> {
> >> + struct timeval tv;
> >> int err = 0;
> >>
> >> /* The addrconf netdev notifier requires that loopback_dev
> >> @@ -4214,10 +4215,14 @@ int __init addrconf_init(void)
> >> if (err)
> >> return err;
> >>
> >> + do_gettimeofday(&tv);
> >>
> >
> > Better to use ktime_t or timespec in new code.
> >
> You are saying not to use timeval as its going to be removed sometime in
> future? If not, may i know why should we use timespec or ktime?
> I need only seconds granularity so i was wondering if that matters.
>
Timeval isn't going away, but ktime is easier for basic maths.
If all you need is seconds resolution, just use jiffies. Gettimeofday
(and ktime_get) both require expensive hardware accesses on
some platforms.
For the specific case IPV6 route ageing, this isn't a big deal it is just
that people tend to copy code.
^ permalink raw reply
* Re: 2.6.20->2.6.21 - networking dies after random time
From: Jarek Poplawski @ 2007-07-24 7:18 UTC (permalink / raw)
To: Ingo Molnar
Cc: Marcin Ślusarz, Jean-Baptiste Vignaud, linux-kernel,
shemminger, linux-net, netdev, Ingo Molnar, Thomas Gleixner,
Andrew Morton, Linus Torvalds
In-Reply-To: <4bacf17f0707222244p664e7a6ap850b3357a57d73c@mail.gmail.com>
On Mon, Jul 23, 2007 at 07:44:58AM +0200, Marcin Ślusarz wrote:
> Ok, I've bisected this problem and found that this patch broke my NIC:
>
> 76d2160147f43f982dfe881404cfde9fd0a9da21 is first bad commit
> commit 76d2160147f43f982dfe881404cfde9fd0a9da21
> Author: Ingo Molnar <mingo@elte.hu>
> Date: Fri Feb 16 01:28:24 2007 -0800
>
> [PATCH] genirq: do not mask interrupts by default
>
> Never mask interrupts immediately upon request. Disabling interrupts in
> high-performance codepaths is rare, and on the other hand this change
> could
> recover lost edges (or even other types of lost interrupts) by
> conservatively
> only masking interrupts after they happen. (NOTE: with this change the
> highlevel irq-disable code still soft-disables this IRQ line - and
> if such an
> interrupt happens then the IRQ flow handler keeps the IRQ masked.)
>
> Mark i8529A controllers as 'never loses an edge'.
>
> Signed-off-by: Ingo Molnar <mingo@elte.hu>
> Cc: Thomas Gleixner <tglx@linutronix.de>
> Signed-off-by: Andrew Morton <akpm@linux-foundation.org>
> Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org>
So, it seems nobody (except the users) cares...
BTW, maybe there should be created something like "Network Cards
Producers Made Rich on Unnecessary Changed Cards Linux Foundation"?:
On Fri, Jun 29, 2007 at 10:50:20AM +0200, Jean-Baptiste Vignaud wrote:
...
> 2) changed the 3com cards
> i replaced by two cards,
> 01:06.0 Ethernet controller: VIA Technologies, Inc. VT6102 [Rhine-II] (rev 42)
> 01:07.0 Ethernet controller: Realtek Semiconductor Co., Ltd. RTL-8029(AS)
>
> reinstalled and stressed the network (small download from a laptop) and :
>
> Jun 29 09:34:10 loki kernel: NETDEV WATCHDOG: eth0: transmit timed out
> Jun 29 09:34:51 loki last message repeated 14 times
> Jun 29 09:35:18 loki last message repeated 8 times
...Of course, no response of any "serious" developer for this as well.
BTW #2: I wonder how true is this (after above-mentioned patch):
From include/linux/irq.h:
> /**
> * struct irq_chip - hardware interrupt chip descriptor
...
> * @disable: disable the interrupt (defaults to chip->mask if NULL)
Regards,
Jarek P.
^ permalink raw reply
* Re: [PATCH RFX]: napi_struct V3
From: Francois Romieu @ 2007-07-24 7:12 UTC (permalink / raw)
To: David Miller; +Cc: netdev, shemminger, rusty, jgarzik
In-Reply-To: <20070723.210748.63127793.davem@davemloft.net>
David Miller <davem@davemloft.net> :
[...]
> I also didn't play with turning off NAPI in kconfig where drivers
> allow that, can we just get rid of that crap already? :-/
Would it be accepted for 2.6.23 or must it be considered post-2.6.23 ?
--
Ueimor
^ permalink raw reply
* Re: specifying scopid's for link-local IPv6 addrs
From: Bill Fink @ 2007-07-24 7:01 UTC (permalink / raw)
To: Rick Jones; +Cc: Linux Network Development list
In-Reply-To: <46A4FAFF.8010809@hp.com>
On Mon, 23 Jul 2007, Rick Jones wrote:
> Folks -
>
> People running netperf have reported that they have trouble with IPv6 under
> Linux. Specifically, wereas the use of link-local IPv6 addresses "just works"
> in netperf under a number of "other OSes" they do not under Linux. I'm
> ass-u-me-ing 2.6 here, but not sure exactly which ones - I've seen it on a
> 2.6.18-based RHEL5.
>
> Some poking about and conversation has suggested that one has to set a
> sin6_scope_id in the sockaddr_in6. This needs to be an index of one of the
> interfaces in the system, which I presume means walking some additional structures.
>
> Is this a requirement which might be expected to remain in the future, or is it
> something which might just go away? That will have an effect on netperf future
> development.
>
> thanks,
>
> rick jones
Rick,
I don't see any way around this. For example, on one of my test
systems, I have the following link local routes:
chance% netstat -A inet6 -rn | grep fe80::/64
fe80::/64 :: U 256 0 0 eth0
fe80::/64 :: U 256 0 0 eth2
fe80::/64 :: U 256 0 0 eth3
fe80::/64 :: U 256 0 0 eth4
fe80::/64 :: U 256 0 0 eth5
fe80::/64 :: U 256 0 0 eth6
So if I want to run a link local test to fe80::202:b3ff:fed4:cd1,
the system has no way to choose which is the correct interface to
use for the test, and will give an error if the interface isn't
specified. Here's an example of this with nuttcp:
chance% nuttcp -P5100 fe80::202:b3ff:fed4:cd1
nuttcp-t: Info: attempting to switch to deprecated "classic" mode
nuttcp-t: Info: will use less reliable transmitter side statistics
nuttcp-t: v5.5.5: Error: connect: Invalid argument
errno=22
You must explicitly specify the desired interface. For example,
on my test system, the correct interface is eth6 which is interface 8
(lo eth0 eth1 eth2 ... eth5 eth6). Here is an example nuttcp test
specifying interface 8:
chance% nuttcp -P5100 fe80::202:b3ff:fed4:cd1%8
1178.5809 MB / 10.02 sec = 986.2728 Mbps 12 %TX 15 %RX
nuttcp uses getaddrinfo() which parses the "%<ifindex>" field,
and then copies the sin6_scope_id from the res structure to the
server's sockaddr_in6 structure before initiating the connect().
-Bill
^ permalink raw reply
* Re: [PATCH 2/4] Add new timeval_to_sec function
From: Varun Chandramohan @ 2007-07-24 6:40 UTC (permalink / raw)
To: Oliver Hartkopp
Cc: Patrick McHardy, netdev, sri, dlstevens, varuncha,
Thomas Gleixner
In-Reply-To: <46A59AAF.6090605@hartkopp.net>
Oliver Hartkopp wrote:
> Varun Chandramohan wrote:
>
>> Patrick McHardy wrote:
>>
>>
>>> Varun Chandramohan wrote:
>>>
>>>
>>>
>>>> /**
>>>> + * timeval_to_sec - Convert timeval to seconds
>>>> + * @tv: pointer to the timeval variable to be converted
>>>> + *
>>>> + * Returns the seconds representation of timeval parameter.
>>>> + */
>>>> +static inline time_t timeval_to_sec(const struct timeval *tv)
>>>> +{
>>>> + return (tv->tv_sec + (tv->tv_usec + 500000)/1000000);
>>>> +}
>>>>
>>>>
>>>>
>>> I don't think you should round down timeout values.
>>>
>>>
>>>
>> Can you elaborate on that? As per the RFC of MIB ,we need only seconds
>> granularity. Taking that as the case i dont understand why round down
>> should not be done?
>>
>>
>
> When you like to create any timeout based on your calculated value, you
> might run into the problem that your calculated value is set to _zero_
> even if there was "some time" before the conversion. This might probably
> not what you indented to get.
>
> So what about rounding up with
>
> return (tv->tv_sec + (tv->tv_usec + 999999)/1000000);
>
> ???
>
>
This can done. Is this what you were ref to me, Patrick?
> Btw. isn't here already any solution based on ktime conversions?
>
>
AFAIK there isint any conversion function to secs. Correct me if iam wrong.
But we can have a function or macro to do this conversion.
> Regards,
> Oliver
>
>
>
Regards,
Varun
> -
> 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 2/4] Add new timeval_to_sec function
From: Oliver Hartkopp @ 2007-07-24 6:22 UTC (permalink / raw)
To: Varun Chandramohan
Cc: Patrick McHardy, netdev, sri, dlstevens, varuncha,
Thomas Gleixner
In-Reply-To: <46A57CF3.6040505@linux.vnet.ibm.com>
Varun Chandramohan wrote:
> Patrick McHardy wrote:
>
>> Varun Chandramohan wrote:
>>
>>
>>> /**
>>> + * timeval_to_sec - Convert timeval to seconds
>>> + * @tv: pointer to the timeval variable to be converted
>>> + *
>>> + * Returns the seconds representation of timeval parameter.
>>> + */
>>> +static inline time_t timeval_to_sec(const struct timeval *tv)
>>> +{
>>> + return (tv->tv_sec + (tv->tv_usec + 500000)/1000000);
>>> +}
>>>
>>>
>> I don't think you should round down timeout values.
>>
>>
> Can you elaborate on that? As per the RFC of MIB ,we need only seconds
> granularity. Taking that as the case i dont understand why round down
> should not be done?
>
When you like to create any timeout based on your calculated value, you
might run into the problem that your calculated value is set to _zero_
even if there was "some time" before the conversion. This might probably
not what you indented to get.
So what about rounding up with
return (tv->tv_sec + (tv->tv_usec + 999999)/1000000);
???
Btw. isn't here already any solution based on ktime conversions?
Regards,
Oliver
^ permalink raw reply
* Re: [PATCH RFX]: napi_struct V3
From: Rusty Russell @ 2007-07-24 6:21 UTC (permalink / raw)
To: David Miller; +Cc: netdev, shemminger, jgarzik, hadi
In-Reply-To: <20070723.224707.30179585.davem@davemloft.net>
On Mon, 2007-07-23 at 22:47 -0700, David Miller wrote:
> I don't think it's wise to implement this over and over again in each
> driver, since we already know at least a handfull of drivers will use
> this.
Yep. Alternative is a napi_struct_with_restart, but I don't think it's
worth the few-byte savings per queue.
> Any objections?
On the contrary, this looks good.
Thanks!
Rusty.
^ 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