* linux-next: build failure after merge of the final tree (net tree related)
From: Stephen Rothwell @ 2010-03-18 4:23 UTC (permalink / raw)
To: David Miller, netdev; +Cc: linux-next, linux-kernel, Tom Herbert
[-- Attachment #1: Type: text/plain, Size: 633 bytes --]
Hi Dave,
After merging the final tree, today's linux-next build (powerpc
ppc44x_defconfig) failed like this:
net/core/dev.c: In function 'net_rps_action':
net/core/dev.c:3183: error: implicit declaration of function '__smp_call_function_single'
Caused by commit 0a9627f2649a02bea165cfd529d7bcb625c2fcad ("rps: Receive
Packet Steering") from the net tree. This config does not have
CONFIG_SMP set. __smp_call_function_single is only available when
CONFIG_SMP is set.
I have reverted that commit for today.
--
Cheers,
Stephen Rothwell sfr@canb.auug.org.au
http://www.canb.auug.org.au/~sfr/
[-- Attachment #2: Type: application/pgp-signature, Size: 198 bytes --]
^ permalink raw reply
* Re: [PATCH v7] rps: Receive Packet Steering
From: Changli Gao @ 2010-03-18 2:14 UTC (permalink / raw)
To: Tom Herbert; +Cc: Eric Dumazet, David Miller, netdev
In-Reply-To: <4BA16AB8.3090800@google.com>
On Thu, Mar 18, 2010 at 7:50 AM, Tom Herbert <therbert@google.com> wrote:
>
>>>
>>> # echo "0 1 0 1 0 1 1 1 1 1" >/sys/class/net/eth0/queues/rx-0/rps_map
>>> # cat /sys/class/net/eth0/queues/rx-0/rps_cpus
>>> 3
>>> # cat /sys/class/net/eth0/queues/rx-0/rps_map
>>> 0 1 0 1 0 1 1 1 1 1
>>> # echo 3 >/sys/class/net/eth0/queues/rx-0/rps_cpus
>>> # cat /sys/class/net/eth0/queues/rx-0/rps_map
>>> 0 1
>>
>> Alternatively, the rps_map could be specified explicitly, which will
>> allow weighting. For example "0 0 0 0 2 10 10 10" would select CPUs
>> 0, 2, 10 for the map with weights four, one, and three respectively.
>> This would go back to have sysfs files with multiple values in them,
>> so it might not be the right interface.
>
> Here is a patch for this...
>
> Allow specification of CPUs in rps to be done with a vector instead of a bit map. This allows relative weighting of CPUs in the map by repeating ones to give higher weight.
>
> For example "echo 0 0 0 3 4 4 4 4 > /sys/class/net/eth0/queues/rx-0/rps_cpus"
>
> assigns CPUs 0, 3, and 4 to the RPS mask with relative weights 3, 1, and 4 respectively.
>
If the weight of CPU0 is 100, I have to repeat 0 100 times. How about
using the * to simplify the weight.
The above examples will be "echo 0*3 3 4*4 >
/sys/class/net/eth0/queues/rx-0/rps_cpus"
--
Regards，
Changli Gao(xiaosuo@gmail.com)
^ permalink raw reply
* Re: [Patch] netpoll: warn when there are spaces in parameters
From: Cong Wang @ 2010-03-18 2:02 UTC (permalink / raw)
To: Frans Pop; +Cc: linux-kernel, netdev, davem
In-Reply-To: <201003171158.49549.elendil@planet.nl>
Frans Pop wrote:
> Amerigo Wang wrote:
>> + if (*cur == ' ' || *cur == '\t')
>> + printk(KERN_INFO "%s: warning: white spaces"
>> + "are not allowed.\n", np->name);
>>
>
> The term "white spaces" not correct. Please change the message to
> "whitespace is not allowed".
>
> Also, it's normally not necessary to close kernel messages with a period;
> most kernel messages do not have a closing period. Messages are not
> sentences and the periods only help increase the kernel size.
>
Ok, I will update it.
Thanks.
^ permalink raw reply
* Re: [PATCH] [V2] Add non-Virtex5 support for LL TEMAC driver
From: Edgar E. Iglesias @ 2010-03-18 0:46 UTC (permalink / raw)
To: Michal Simek
Cc: John Linn, netdev, linuxppc-dev, grant.likely, jwboyer,
john.williams, John Tyner
In-Reply-To: <4B9E46B5.1060508@petalogix.com>
On Mon, Mar 15, 2010 at 03:39:49PM +0100, Michal Simek wrote:
> John Linn wrote:
> >> -----Original Message-----
> >> From: Michal Simek [mailto:michal.simek@petalogix.com]
> >> Sent: Monday, March 15, 2010 2:40 AM
> >> To: John Linn
> >> Cc: netdev@vger.kernel.org; linuxppc-dev@ozlabs.org;
> > grant.likely@secretlab.ca;
> >> jwboyer@linux.vnet.ibm.com; john.williams@petalogix.com; John Tyner
> >> Subject: Re: [PATCH] [V2] Add non-Virtex5 support for LL TEMAC driver
> >>
> >> John Linn wrote:
> >>> This patch adds support for using the LL TEMAC Ethernet driver on
> >>> non-Virtex 5 platforms by adding support for accessing the Soft DMA
> >>> registers as if they were memory mapped instead of solely through
> > the
> >>> DCR's (available on the Virtex 5).
> >>>
> >>> The patch also updates the driver so that it runs on the MicroBlaze.
> >>> The changes were tested on the PowerPC 440, PowerPC 405, and the
> >>> MicroBlaze platforms.
> >> Which git-tree have you tested on? (Of course microblaze)
> >
> > It was tested on the Xilinx tree for MicroBlaze which is based on the
> > mainline and the Petalogix tree as DMA was needed. I tried to build
> > against the mainline head but got errors with the DMA routines. I guess
> > it's possible that it was a configuration issue there as I didn't dig
> > real deep.
>
> New dma api is in for-linus branch.
> I tested it on that version and I am seeing some weird things. :-(
> Access to bad area. I will try your tree.
>
> The second thing which I see is in ll_temac_recv function.
> On the following line is read a packet length which could be 0-16k.
> length = cur_p->app4 & 0x3FFF;
>
> But allocated skb has max size XTE_MAX_JUMBO_FRAME_SIZE + XTE_ALIGN.
>
> What happen if driver get packet greater than 9kB?
> I got it (I don't know how) but skb_put has one checking mechanism which
> will cal skb_over_panic which caused panic.
> That's why I think that will be good always to check that length is less
> than XTE_MAX_JUMBO_FRAME_SIZE + XTE_ALIGN.
>
> What do you think?
I agree. IIRC the LLTEMAC when configured to accept jumboframes will
accepts packets up to 16K and the driver should handle that without
raising panics.
I remeber beeing a bit surprised about that when hacking the QEMU model.
Not sure if I remember correctly though.
Cheers
^ permalink raw reply
* Re: ixgbe_clean_tx_irq: tx hang 1 detected, resetting adapter (2.6.32.8)
From: Jesse Brandeburg @ 2010-03-18 0:44 UTC (permalink / raw)
To: Jesper Krogh; +Cc: linux-nfs@vger.kernel.org, netdev@vger.kernel.org
In-Reply-To: <4B82CF94.5070903@krogh.cc>
On Mon, 2010-02-22 at 10:40 -0800, Jesper Krogh wrote:
> Jesse Brandeburg wrote:
> > On Sun, Feb 14, 2010 at 8:29 AM, Jesper Krogh <jesper@krogh.cc> wrote:
> >> Hi List.
> >>
> >> I have tried to get a dual bond of 2 x 10G NICs using the
> >> Intel Corporation 82598EB 10-Gigabit AT2 Server Adapter (rev 01)
> >> going. As first it looked like it "just worked" but when tried to fill
> >> the links with data one of the NIC's (eth7) hang and did a reset of
> >> itself, so all data was pushed through the other NIC in the bond (eth8)
> >>
> >> Full dmesg below, but I think the important part is this:
> >>
> >> [ 2162.745354] ixgbe: eth7: ixgbe_check_tx_hang: Detected Tx Unit Hang
> >> [ 2162.745356] Tx Queue <4>
> >> [ 2162.745356] TDH, TDT <e1>, <cc>
> >> [ 2162.745357] next_to_use <cc>
> >> [ 2162.745358] next_to_clean <e1>
> >> [ 2162.745359] tx_buffer_info[next_to_clean]
> >> [ 2162.745359] time_stamp <1000713d3>
> >> [ 2162.745360] jiffies <10007152e>
> >> [ 2163.162478] ixgbe: eth7: ixgbe_clean_tx_irq: tx hang 1 detected,
> >> resetting adapter
> >> [ 2163.357333] bonding: bond0: link status definitely down for interface
> >> eth7, disabling it
> >> [ 2168.670342] ixgbe: eth7 NIC Link is Up 10 Gbps, Flow Control: None
> >
> > Hi Jesper, my first thought was flow control, but I can see you have it off.
>
> I didnt change it, so its default.
>
> > Can we get some more details on the hardware and bios version?
>
> Sun X4600, 128GB, 16 cores..
>
> > What
> > about some dmidecode output. I'm checking here if we have any
> > hardware like this.
>
> http://shrek.krogh.cc/~jesper/dmidecode.txt
>
>
> > are you running ubuntu 9.10 or something else?
>
> We're on Ubuntu 8.04 but we basically "only" use a filesystem, kernel
> with a well-performing NFS-server and some NIC that work well :-)
>
> > Wow, thats a monster machine, 8 nodes, 128GB ram. Can we get a full
> > lspci -vvv output, as well as ethtool -e eth7 and eth8
>
> http://shrek.krogh.cc/~jesper/lspci.txt
>
> http://shrek.krogh.cc/~jesper/ethtool-eth7.txt
> http://shrek.krogh.cc/~jesper/ethtool-eth8.txt
>
> > 32 has ixgbe with a known issue of multiple mappings on transmit
> > possibly causing some problems, could it be that you're running into
> > this? can you apply commit e5a43549f7a58509a91b299a51337d386697b92c
> > and see if it fixes your issue?
>
> I'll do that the next time I can push a reboot through.
>
including full email for context, did this solve your issue?
--
Jesse Brandeburg
This email sent via Evolution, powered by Linux
^ permalink raw reply
* Re: [PATCH] r8169: Fix rtl8169_rx_interrupt()
From: Francois Romieu @ 2010-03-17 23:55 UTC (permalink / raw)
To: Eric Dumazet; +Cc: Sergey Senozhatsky, Oleg Nesterov, David Miller, netdev
In-Reply-To: <1268811437.2932.66.camel@edumazet-laptop>
On Wed, Mar 17, 2010 at 08:37:17AM +0100, Eric Dumazet wrote:
[...]
> I suspect lot of work is needed on this driver to make it working, but I
> dont have a machine with said adapter.
This one should help too if Sergey owns a (MSI) 8168.
diff --git a/drivers/net/r8169.c b/drivers/net/r8169.c
index dfc3573..721e7f3 100644
--- a/drivers/net/r8169.c
+++ b/drivers/net/r8169.c
@@ -4532,21 +4532,39 @@ static int rtl8169_rx_interrupt(struct net_device *dev,
return count;
}
+static void rtl_napi_cond_schedule(struct rtl8169_private *tp, u16 status)
+{
+ if (status & tp->napi_event) {
+ void __iomem *ioaddr = tp->mmio_addr;
+
+ RTL_W16(IntrMask, tp->intr_event & ~tp->napi_event);
+ mmiowb();
+ napi_schedule(&tp->napi);
+ }
+}
+
static irqreturn_t rtl8169_interrupt(int irq, void *dev_instance)
{
struct net_device *dev = dev_instance;
struct rtl8169_private *tp = netdev_priv(dev);
void __iomem *ioaddr = tp->mmio_addr;
int handled = 0;
- int status;
+ u16 status;
/* loop handling interrupts until we have no new ones or
* we hit a invalid/hotplug case.
*/
status = RTL_R16(IntrStatus);
while (status && status != 0xffff) {
+ u16 acked;
+
handled = 1;
+ acked = (status & RxFIFOOver) ? (status | RxOverflow) : status;
+ acked &= ~tp->napi_event;
+
+ RTL_W16(IntrStatus, acked);
+
/* Handle all of the error cases first. These will reset
* the chip, so just exit the loop.
*/
@@ -4557,7 +4575,7 @@ static irqreturn_t rtl8169_interrupt(int irq, void *dev_instance)
/* Work around for rx fifo overflow */
if (unlikely(status & RxFIFOOver) &&
- (tp->mac_version == RTL_GIGA_MAC_VER_11)) {
+ (tp->mac_version == RTL_GIGA_MAC_VER_11)) {
netif_stop_queue(dev);
rtl8169_tx_timeout(dev);
break;
@@ -4571,30 +4589,9 @@ static irqreturn_t rtl8169_interrupt(int irq, void *dev_instance)
if (status & LinkChg)
rtl8169_check_link_status(dev, tp, ioaddr);
- /* We need to see the lastest version of tp->intr_mask to
- * avoid ignoring an MSI interrupt and having to wait for
- * another event which may never come.
- */
- smp_rmb();
- if (status & tp->intr_mask & tp->napi_event) {
- RTL_W16(IntrMask, tp->intr_event & ~tp->napi_event);
- tp->intr_mask = ~tp->napi_event;
-
- if (likely(napi_schedule_prep(&tp->napi)))
- __napi_schedule(&tp->napi);
- else
- netif_info(tp, intr, dev,
- "interrupt %04x in poll\n", status);
- }
+ rtl_napi_cond_schedule(tp, status);
- /* We only get a new MSI interrupt when all active irq
- * sources on the chip have been acknowledged. So, ack
- * everything we've seen and check if new sources have become
- * active to avoid blocking all interrupts from the chip.
- */
- RTL_W16(IntrStatus,
- (status & RxFIFOOver) ? (status | RxOverflow) : status);
- status = RTL_R16(IntrStatus);
+ break;
}
return IRQ_RETVAL(handled);
@@ -4607,22 +4604,19 @@ static int rtl8169_poll(struct napi_struct *napi, int budget)
void __iomem *ioaddr = tp->mmio_addr;
int work_done;
+
+ RTL_W16(IntrStatus, tp->napi_event);
+
work_done = rtl8169_rx_interrupt(dev, tp, ioaddr, (u32) budget);
rtl8169_tx_interrupt(dev, tp, ioaddr);
if (work_done < budget) {
napi_complete(napi);
- /* We need for force the visibility of tp->intr_mask
- * for other CPUs, as we can loose an MSI interrupt
- * and potentially wait for a retransmit timeout if we don't.
- * The posted write to IntrMask is safe, as it will
- * eventually make it to the chip and we won't loose anything
- * until it does.
- */
- tp->intr_mask = 0xffff;
- smp_wmb();
RTL_W16(IntrMask, tp->intr_event);
+ mmiowb();
+
+ rtl_napi_cond_schedule(tp, RTL_R16(IntrStatus));
}
return work_done;
^ permalink raw reply related
* Re: [PATCH v7] rps: Receive Packet Steering
From: Tom Herbert @ 2010-03-17 23:50 UTC (permalink / raw)
To: Eric Dumazet; +Cc: Changli Gao, David Miller, netdev
In-Reply-To: <65634d661003170801x1042a6am563c9d937ba672a4@mail.gmail.com>
>>
>> # echo "0 1 0 1 0 1 1 1 1 1" >/sys/class/net/eth0/queues/rx-0/rps_map
>> # cat /sys/class/net/eth0/queues/rx-0/rps_cpus
>> 3
>> # cat /sys/class/net/eth0/queues/rx-0/rps_map
>> 0 1 0 1 0 1 1 1 1 1
>> # echo 3 >/sys/class/net/eth0/queues/rx-0/rps_cpus
>> # cat /sys/class/net/eth0/queues/rx-0/rps_map
>> 0 1
>
> Alternatively, the rps_map could be specified explicitly, which will
> allow weighting. For example "0 0 0 0 2 10 10 10" would select CPUs
> 0, 2, 10 for the map with weights four, one, and three respectively.
> This would go back to have sysfs files with multiple values in them,
> so it might not be the right interface.
Here is a patch for this...
Allow specification of CPUs in rps to be done with a vector instead of a bit map. This allows relative weighting of CPUs in the map by repeating ones to give higher weight.
For example "echo 0 0 0 3 4 4 4 4 > /sys/class/net/eth0/queues/rx-0/rps_cpus"
assigns CPUs 0, 3, and 4 to the RPS mask with relative weights 3, 1, and 4 respectively.
diff --git a/net/core/net-sysfs.c b/net/core/net-sysfs.c
index 7a46343..41956a5 100644
--- a/net/core/net-sysfs.c
+++ b/net/core/net-sysfs.c
@@ -17,6 +17,7 @@
#include <linux/rtnetlink.h>
#include <linux/wireless.h>
#include <net/wext.h>
+#include <linux/ctype.h>
#include "net-sysfs.h"
@@ -514,30 +515,20 @@ static ssize_t show_rps_map(struct netdev_rx_queue *queue,
struct rx_queue_attribute *attribute, char *buf)
{
struct rps_map *map;
- cpumask_var_t mask;
size_t len = 0;
int i;
- if (!zalloc_cpumask_var(&mask, GFP_KERNEL))
- return -ENOMEM;
-
rcu_read_lock();
+
map = rcu_dereference(queue->rps_map);
if (map)
for (i = 0; i < map->len; i++)
- cpumask_set_cpu(map->cpus[i], mask);
+ len += snprintf(buf + len, PAGE_SIZE - len, "%u%s",
+ map->cpus[i], i + 1 < map->len ? " " : "\n");
- len += cpumask_scnprintf(buf + len, PAGE_SIZE, mask);
- if (PAGE_SIZE - len < 3) {
- rcu_read_unlock();
- free_cpumask_var(mask);
- return -EINVAL;
- }
rcu_read_unlock();
- free_cpumask_var(mask);
- len += sprintf(buf + len, "\n");
- return len;
+ return len < PAGE_SIZE ? len : -EINVAL;
}
static void rps_map_release(struct rcu_head *rcu)
@@ -552,41 +543,50 @@ ssize_t store_rps_map(struct netdev_rx_queue *queue,
const char *buf, size_t len)
{
struct rps_map *old_map, *map;
- cpumask_var_t mask;
- int err, cpu, i;
+ int i, count = 0;
+ unsigned int val;
static DEFINE_SPINLOCK(rps_map_lock);
+ char *tbuf;
if (!capable(CAP_NET_ADMIN))
return -EPERM;
- if (!alloc_cpumask_var(&mask, GFP_KERNEL))
- return -ENOMEM;
+ /* Validate and count the number of CPUs in the input list. */
+ tbuf = (char *)buf;
+ while (tbuf < buf + len) {
+ char *rbuf;
- err = bitmap_parse(buf, len, cpumask_bits(mask), nr_cpumask_bits);
- if (err) {
- free_cpumask_var(mask);
- return err;
- }
+ if (isspace(*tbuf)) {
+ tbuf++;
+ continue;
+ }
- map = kzalloc(max_t(unsigned,
- RPS_MAP_SIZE(cpumask_weight(mask)), L1_CACHE_BYTES),
- GFP_KERNEL);
- if (!map) {
- free_cpumask_var(mask);
- return -ENOMEM;
- }
+ val = simple_strtoul(tbuf, &rbuf, 0);
- i = 0;
- for_each_cpu_and(cpu, mask, cpu_online_mask)
- map->cpus[i++] = cpu;
+ if ((tbuf == rbuf) || (val >= num_possible_cpus()))
+ return -EINVAL;
- if (i)
- map->len = i;
- else {
- kfree(map);
- map = NULL;
+ tbuf = rbuf;
+ count++;
}
+ if (count) {
+ map = kzalloc(max_t(unsigned, RPS_MAP_SIZE(count),
+ L1_CACHE_BYTES), GFP_KERNEL);
+ if (!map)
+ return -ENOMEM;
+
+ tbuf = (char *)buf;
+ for (i = 0; i < count; i++) {
+ while (isspace(*tbuf))
+ tbuf++;
+ map->cpus[i] = simple_strtoul(tbuf, &tbuf, 0);
+ }
+ map->len = count;
+ } else
+ map = NULL;
+
+
spin_lock(&rps_map_lock);
old_map = queue->rps_map;
rcu_assign_pointer(queue->rps_map, map);
@@ -595,7 +595,6 @@ ssize_t store_rps_map(struct netdev_rx_queue *queue,
if (old_map)
call_rcu(&old_map->rcu, rps_map_release);
- free_cpumask_var(mask);
return len;
}
^ permalink raw reply related
* Re: e1000e: Fix build with CONFIG_PM disabled.
From: Rafael J. Wysocki @ 2010-03-17 23:51 UTC (permalink / raw)
To: Jesse Brandeburg
Cc: David Miller, Tantilov, Emil S, netdev@vger.kernel.org,
Allan, Bruce W, Kirsher, Jeffrey T
In-Reply-To: <1268869036.19191.92.camel@jbrandeb-mobl3.amr.corp.intel.com>
On Thursday 18 March 2010, Jesse Brandeburg wrote:
> On Wed, 2010-03-17 at 15:39 -0700, Rafael J. Wysocki wrote:
> > On Wednesday 17 March 2010, David Miller wrote:
> > > From: "Tantilov, Emil S" <emil.s.tantilov@intel.com>
> > > Date: Wed, 17 Mar 2010 11:57:59 -0600
> > >
> > > > But then the driver build fails again when CONFIG_PM_RUNTIME is not set.
> > > >
> > > > drivers/net/e1000e/netdev.c: In function `e1000_runtime_resume`:
> > > > drivers/net/e1000e/netdev.c:4807: error: `struct dev_pm_info` has no member named `runtime_auto`
> > >
> > > Ugh, can someone send me a fix for that?
> >
> > Appended, on top of your patch. It only moves the code around and adds
> > #ifdefs.
>
> I know this is throwing a wrench in things, but won't this series of
> changes break a clean bisect (due to the compile errors?)
Well, it will, but reverting them doesn't actually help fix bisectability.
You'd have to "git reset --hard" to the commit preceding them and then apply
the combo patch.
Rafael
^ permalink raw reply
* Re: e1000e: Fix build with CONFIG_PM disabled.
From: Jesse Brandeburg @ 2010-03-17 23:37 UTC (permalink / raw)
To: Rafael J. Wysocki
Cc: David Miller, Tantilov, Emil S, netdev@vger.kernel.org,
Allan, Bruce W, Kirsher, Jeffrey T
In-Reply-To: <201003172339.15494.rjw@sisk.pl>
On Wed, 2010-03-17 at 15:39 -0700, Rafael J. Wysocki wrote:
> On Wednesday 17 March 2010, David Miller wrote:
> > From: "Tantilov, Emil S" <emil.s.tantilov@intel.com>
> > Date: Wed, 17 Mar 2010 11:57:59 -0600
> >
> > > But then the driver build fails again when CONFIG_PM_RUNTIME is not set.
> > >
> > > drivers/net/e1000e/netdev.c: In function `e1000_runtime_resume`:
> > > drivers/net/e1000e/netdev.c:4807: error: `struct dev_pm_info` has no member named `runtime_auto`
> >
> > Ugh, can someone send me a fix for that?
>
> Appended, on top of your patch. It only moves the code around and adds
> #ifdefs.
I know this is throwing a wrench in things, but won't this series of
changes break a clean bisect (due to the compile errors?)
so, revert
e50208a080a8abdfa6c85865362a1cf329e9fe31
and
23606cf5d1192c2b17912cb2ef6e62f9b11de133
and this is a mashup of the three:
---
e1000e: Add basic runtime PM support (rev 5)
From: Rafael J. Wysocki <rjw@sisk.pl>
these two commits should be reverted, and this patch applied in their
place.
this patch applies based on a tree with the below two commits reverted,
and should include the ifdefs necessary to compile with/without
PM/PM_RUNTIME support.
I compile tested it very briefly by compiling without CONFIG_PM, with
CONFIG_PM only and WITH CONFIG_PM_RUNTIME
commit e50208a080a8abdfa6c85865362a1cf329e9fe31
Author: David S. Miller <davem@davemloft.net>
Date: Tue Mar 16 23:36:24 2010 -0700
e1000e: Fix build with CONFIG_PM disabled.
Signed-off-by: David S. Miller <davem@davemloft.net>
:100644 100644 79b33c5... b96532a... M drivers/net/e1000e/netdev.c
commit 23606cf5d1192c2b17912cb2ef6e62f9b11de133
Author: Rafael J. Wysocki <rjw@sisk.pl>
Date: Sun Mar 14 14:35:17 2010 +0000
e1000e / PCI / PM: Add basic runtime PM support (rev. 4)
Use the PCI runtime power management framework to add basic PCI
runtime PM support to the e1000e driver. Namely, make the driver
suspend the device when the link is off and set it up for generating
a wakeup event after the link has been detected again. [This
feature is disabled until the user space enables it with the help of
the /sys/devices/.../power/contol device attribute.]
Based on a patch from Matthew Garrett.
Signed-off-by: Rafael J. Wysocki <rjw@sisk.pl>
Signed-off-by: David S. Miller <davem@davemloft.net>
:100644 100644 c2ec095... 8da190b... M drivers/net/e1000e/e1000.h
:100644 100644 88d54d3... 79b33c5... M drivers/net/e1000e/netdev.c
Signed-off-by: Jesse Brandeburg <jesse.brandeburg@intel.com>
---
drivers/net/e1000e/e1000.h | 5 +
drivers/net/e1000e/netdev.c | 182
++++++++++++++++++++++++++++++++++---------
2 files changed, 149 insertions(+), 38 deletions(-)
diff --git a/drivers/net/e1000e/e1000.h b/drivers/net/e1000e/e1000.h
index c2ec095..8da190b 100644
--- a/drivers/net/e1000e/e1000.h
+++ b/drivers/net/e1000e/e1000.h
@@ -158,6 +158,9 @@ struct e1000_info;
#define HV_M_STATUS_SPEED_1000 0x0200
#define HV_M_STATUS_LINK_UP 0x0040
+/* Time to wait before putting the device into D3 if there's no link
(in ms). */
+#define LINK_TIMEOUT 100
+
enum e1000_boards {
board_82571,
board_82572,
@@ -370,6 +373,8 @@ struct e1000_adapter {
struct work_struct update_phy_task;
struct work_struct led_blink_task;
struct work_struct print_hang_task;
+
+ bool idle_check;
};
struct e1000_info {
diff --git a/drivers/net/e1000e/netdev.c b/drivers/net/e1000e/netdev.c
index 88d54d3..06ba46a 100644
--- a/drivers/net/e1000e/netdev.c
+++ b/drivers/net/e1000e/netdev.c
@@ -44,6 +44,7 @@
#include <linux/cpu.h>
#include <linux/smp.h>
#include <linux/pm_qos_params.h>
+#include <linux/pm_runtime.h>
#include <linux/aer.h>
#include "e1000.h"
@@ -3083,12 +3084,15 @@ static int e1000_open(struct net_device *netdev)
{
struct e1000_adapter *adapter = netdev_priv(netdev);
struct e1000_hw *hw = &adapter->hw;
+ struct pci_dev *pdev = adapter->pdev;
int err;
/* disallow open during test */
if (test_bit(__E1000_TESTING, &adapter->state))
return -EBUSY;
+ pm_runtime_get_sync(&pdev->dev);
+
netif_carrier_off(netdev);
/* allocate transmit descriptors */
@@ -3149,6 +3153,9 @@ static int e1000_open(struct net_device *netdev)
netif_start_queue(netdev);
+ adapter->idle_check = true;
+ pm_runtime_put(&pdev->dev);
+
/* fire a link status change interrupt to start the watchdog */
ew32(ICS, E1000_ICS_LSC);
@@ -3162,6 +3169,7 @@ err_setup_rx:
e1000e_free_tx_resources(adapter);
err_setup_tx:
e1000e_reset(adapter);
+ pm_runtime_put_sync(&pdev->dev);
return err;
}
@@ -3180,11 +3188,17 @@ err_setup_tx:
static int e1000_close(struct net_device *netdev)
{
struct e1000_adapter *adapter = netdev_priv(netdev);
+ struct pci_dev *pdev = adapter->pdev;
WARN_ON(test_bit(__E1000_RESETTING, &adapter->state));
- e1000e_down(adapter);
+
+ pm_runtime_get_sync(&pdev->dev);
+
+ if (!test_bit(__E1000_DOWN, &adapter->state)) {
+ e1000e_down(adapter);
+ e1000_free_irq(adapter);
+ }
e1000_power_down_phy(adapter);
- e1000_free_irq(adapter);
e1000e_free_tx_resources(adapter);
e1000e_free_rx_resources(adapter);
@@ -3206,6 +3220,8 @@ static int e1000_close(struct net_device *netdev)
if (adapter->flags & FLAG_HAS_AMT)
e1000_release_hw_control(adapter);
+ pm_runtime_put_sync(&pdev->dev);
+
return 0;
}
/**
@@ -3550,6 +3566,9 @@ static void e1000_watchdog_task(struct work_struct
*work)
link = e1000e_has_link(adapter);
if ((netif_carrier_ok(netdev)) && link) {
+ /* Cancel scheduled suspend requests. */
+ pm_runtime_resume(netdev->dev.parent);
+
e1000e_enable_receives(adapter);
goto link_up;
}
@@ -3561,6 +3580,10 @@ static void e1000_watchdog_task(struct
work_struct *work)
if (link) {
if (!netif_carrier_ok(netdev)) {
bool txb2b = 1;
+
+ /* Cancel scheduled suspend requests. */
+ pm_runtime_resume(netdev->dev.parent);
+
/* update snapshot of PHY registers on LSC */
e1000_phy_read_status(adapter);
mac->ops.get_link_up_info(&adapter->hw,
@@ -3676,6 +3699,9 @@ static void e1000_watchdog_task(struct work_struct
*work)
if (adapter->flags & FLAG_RX_NEEDS_RESTART)
schedule_work(&adapter->reset_task);
+ else
+ pm_schedule_suspend(netdev->dev.parent,
+ LINK_TIMEOUT);
}
}
@@ -4473,13 +4499,15 @@ out:
return retval;
}
-static int __e1000_shutdown(struct pci_dev *pdev, bool *enable_wake)
+static int __e1000_shutdown(struct pci_dev *pdev, bool *enable_wake,
+ bool runtime)
{
struct net_device *netdev = pci_get_drvdata(pdev);
struct e1000_adapter *adapter = netdev_priv(netdev);
struct e1000_hw *hw = &adapter->hw;
u32 ctrl, ctrl_ext, rctl, status;
- u32 wufc = adapter->wol;
+ /* Runtime suspend should only enable wakeup for link changes */
+ u32 wufc = runtime ? E1000_WUFC_LNKC : adapter->wol;
int retval = 0;
netif_device_detach(netdev);
@@ -4636,43 +4664,21 @@ static void e1000e_disable_l1aspm(struct pci_dev
*pdev)
}
}
-#ifdef CONFIG_PM
-static int e1000_suspend(struct pci_dev *pdev, pm_message_t state)
+#ifdef CONFIG_PM_OPS
+static bool e1000e_pm_ready(struct e1000_adapter *adapter)
{
- int retval;
- bool wake;
-
- retval = __e1000_shutdown(pdev, &wake);
- if (!retval)
- e1000_complete_shutdown(pdev, true, wake);
-
- return retval;
+ return !!adapter->tx_ring->buffer_info;
}
-static int e1000_resume(struct pci_dev *pdev)
+static int __e1000_resume(struct pci_dev *pdev)
{
struct net_device *netdev = pci_get_drvdata(pdev);
struct e1000_adapter *adapter = netdev_priv(netdev);
struct e1000_hw *hw = &adapter->hw;
u32 err;
- pci_set_power_state(pdev, PCI_D0);
- pci_restore_state(pdev);
- pci_save_state(pdev);
e1000e_disable_l1aspm(pdev);
- err = pci_enable_device_mem(pdev);
- if (err) {
- dev_err(&pdev->dev,
- "Cannot enable PCI device from suspend\n");
- return err;
- }
-
- pci_set_master(pdev);
-
- pci_enable_wake(pdev, PCI_D3hot, 0);
- pci_enable_wake(pdev, PCI_D3cold, 0);
-
e1000e_set_interrupt_capability(adapter);
if (netif_running(netdev)) {
err = e1000_request_irq(adapter);
@@ -4730,13 +4736,88 @@ static int e1000_resume(struct pci_dev *pdev)
return 0;
}
-#endif
+
+#ifdef CONFIG_PM_SLEEP
+static int e1000_suspend(struct device *dev)
+{
+ struct pci_dev *pdev = to_pci_dev(dev);
+ int retval;
+ bool wake;
+
+ retval = __e1000_shutdown(pdev, &wake, false);
+ if (!retval)
+ e1000_complete_shutdown(pdev, true, wake);
+
+ return retval;
+}
+
+static int e1000_resume(struct device *dev)
+{
+ struct pci_dev *pdev = to_pci_dev(dev);
+ struct net_device *netdev = pci_get_drvdata(pdev);
+ struct e1000_adapter *adapter = netdev_priv(netdev);
+
+ if (e1000e_pm_ready(adapter))
+ adapter->idle_check = true;
+
+ return __e1000_resume(pdev);
+}
+#endif /* CONFIG_PM_SLEEP */
+
+#ifdef CONFIG_PM_RUNTIME
+static int e1000_runtime_suspend(struct device *dev)
+{
+ struct pci_dev *pdev = to_pci_dev(dev);
+ struct net_device *netdev = pci_get_drvdata(pdev);
+ struct e1000_adapter *adapter = netdev_priv(netdev);
+
+ if (e1000e_pm_ready(adapter)) {
+ bool wake;
+
+ __e1000_shutdown(pdev, &wake, true);
+ }
+
+ return 0;
+}
+
+static int e1000_idle(struct device *dev)
+{
+ struct pci_dev *pdev = to_pci_dev(dev);
+ struct net_device *netdev = pci_get_drvdata(pdev);
+ struct e1000_adapter *adapter = netdev_priv(netdev);
+
+ if (!e1000e_pm_ready(adapter))
+ return 0;
+
+ if (adapter->idle_check) {
+ adapter->idle_check = false;
+ if (!e1000e_has_link(adapter))
+ pm_schedule_suspend(dev, MSEC_PER_SEC);
+ }
+
+ return -EBUSY;
+}
+
+static int e1000_runtime_resume(struct device *dev)
+{
+ struct pci_dev *pdev = to_pci_dev(dev);
+ struct net_device *netdev = pci_get_drvdata(pdev);
+ struct e1000_adapter *adapter = netdev_priv(netdev);
+
+ if (!e1000e_pm_ready(adapter))
+ return 0;
+
+ adapter->idle_check = !dev->power.runtime_auto;
+ return __e1000_resume(pdev);
+}
+#endif /* CONFIG_PM_RUNTIME */
+#endif /* CONFIG_PM_OPS */
static void e1000_shutdown(struct pci_dev *pdev)
{
bool wake = false;
- __e1000_shutdown(pdev, &wake);
+ __e1000_shutdown(pdev, &wake, false);
if (system_state == SYSTEM_POWER_OFF)
e1000_complete_shutdown(pdev, false, wake);
@@ -4809,8 +4890,8 @@ static pci_ers_result_t e1000_io_slot_reset(struct
pci_dev *pdev)
result = PCI_ERS_RESULT_DISCONNECT;
} else {
pci_set_master(pdev);
+ pdev->state_saved = true;
pci_restore_state(pdev);
- pci_save_state(pdev);
pci_enable_wake(pdev, PCI_D3hot, 0);
pci_enable_wake(pdev, PCI_D3cold, 0);
@@ -5217,6 +5298,12 @@ static int __devinit e1000_probe(struct pci_dev
*pdev,
e1000_print_device_info(adapter);
+ if (pci_dev_run_wake(pdev)) {
+ pm_runtime_set_active(&pdev->dev);
+ pm_runtime_enable(&pdev->dev);
+ }
+ pm_schedule_suspend(&pdev->dev, MSEC_PER_SEC);
+
return 0;
err_register:
@@ -5259,12 +5346,16 @@ static void __devexit e1000_remove(struct
pci_dev *pdev)
{
struct net_device *netdev = pci_get_drvdata(pdev);
struct e1000_adapter *adapter = netdev_priv(netdev);
+ bool down = test_bit(__E1000_DOWN, &adapter->state);
+
+ pm_runtime_get_sync(&pdev->dev);
/*
* flush_scheduled work may reschedule our watchdog task, so
* explicitly disable watchdog tasks from being rescheduled
*/
- set_bit(__E1000_DOWN, &adapter->state);
+ if (!down)
+ set_bit(__E1000_DOWN, &adapter->state);
del_timer_sync(&adapter->watchdog_timer);
del_timer_sync(&adapter->phy_info_timer);
@@ -5278,8 +5369,17 @@ static void __devexit e1000_remove(struct pci_dev
*pdev)
if (!(netdev->flags & IFF_UP))
e1000_power_down_phy(adapter);
+ /* Don't lie to e1000_close() down the road. */
+ if (!down)
+ clear_bit(__E1000_DOWN, &adapter->state);
unregister_netdev(netdev);
+ if (pci_dev_run_wake(pdev)) {
+ pm_runtime_disable(&pdev->dev);
+ pm_runtime_set_suspended(&pdev->dev);
+ }
+ pm_runtime_put_noidle(&pdev->dev);
+
/*
* Release control of h/w to f/w. If f/w is AMT enabled, this
* would have already happened in close and is redundant.
@@ -5379,16 +5479,22 @@ static DEFINE_PCI_DEVICE_TABLE(e1000_pci_tbl) =
{
};
MODULE_DEVICE_TABLE(pci, e1000_pci_tbl);
+#ifdef CONFIG_PM_OPS
+static const struct dev_pm_ops e1000_pm_ops = {
+ SET_SYSTEM_SLEEP_PM_OPS(e1000_suspend, e1000_resume)
+ SET_RUNTIME_PM_OPS(e1000_runtime_suspend,
+ e1000_runtime_resume, e1000_idle)
+};
+#endif
+
/* PCI Device API Driver */
static struct pci_driver e1000_driver = {
.name = e1000e_driver_name,
.id_table = e1000_pci_tbl,
.probe = e1000_probe,
.remove = __devexit_p(e1000_remove),
-#ifdef CONFIG_PM
- /* Power Management Hooks */
- .suspend = e1000_suspend,
- .resume = e1000_resume,
+#ifdef CONFIG_PM_OPS
+ .driver.pm = &e1000_pm_ops,
#endif
.shutdown = e1000_shutdown,
.err_handler = &e1000_err_handler
--
Jesse Brandeburg
This email sent via Evolution, powered by Linux
^ permalink raw reply related
* Re: e1000e: Fix build with CONFIG_PM disabled.
From: Rafael J. Wysocki @ 2010-03-17 22:39 UTC (permalink / raw)
To: David Miller
Cc: emil.s.tantilov, netdev, bruce.w.allan, jesse.brandeburg,
jeffrey.t.kirsher
In-Reply-To: <20100317.111717.85209898.davem@davemloft.net>
On Wednesday 17 March 2010, David Miller wrote:
> From: "Tantilov, Emil S" <emil.s.tantilov@intel.com>
> Date: Wed, 17 Mar 2010 11:57:59 -0600
>
> > But then the driver build fails again when CONFIG_PM_RUNTIME is not set.
> >
> > drivers/net/e1000e/netdev.c: In function `e1000_runtime_resume`:
> > drivers/net/e1000e/netdev.c:4807: error: `struct dev_pm_info` has no member named `runtime_auto`
>
> Ugh, can someone send me a fix for that?
Appended, on top of your patch. It only moves the code around and adds
#ifdefs.
Thanks,
Rafael
---
From: Rafael J. Wysocki <rjw@sisk.pl>
Subject: Net / e1000e: Fix build issue introduced by runtime PM patch
The recent PCI runtime PM patch broke build for CONFIG_PM_RUNTIME
and CONFIG_PM_SLEEP undefined. Fix that by moving the PM callbacks
under suitable #ifdefs.
Signed-off-by: Rafael J. Wysocki <rjw@sisk.pl>
---
drivers/net/e1000e/netdev.c | 116 +++++++++++++++++++++-----------------------
1 file changed, 57 insertions(+), 59 deletions(-)
Index: linux-2.6/drivers/net/e1000e/netdev.c
===================================================================
--- linux-2.6.orig/drivers/net/e1000e/netdev.c
+++ linux-2.6/drivers/net/e1000e/netdev.c
@@ -4664,58 +4664,12 @@ static void e1000e_disable_l1aspm(struct
}
}
-#ifdef CONFIG_PM
+#ifdef CONFIG_PM_OPS
static bool e1000e_pm_ready(struct e1000_adapter *adapter)
{
return !!adapter->tx_ring->buffer_info;
}
-static int e1000_idle(struct device *dev)
-{
- struct pci_dev *pdev = to_pci_dev(dev);
- struct net_device *netdev = pci_get_drvdata(pdev);
- struct e1000_adapter *adapter = netdev_priv(netdev);
-
- if (!e1000e_pm_ready(adapter))
- return 0;
-
- if (adapter->idle_check) {
- adapter->idle_check = false;
- if (!e1000e_has_link(adapter))
- pm_schedule_suspend(dev, MSEC_PER_SEC);
- }
-
- return -EBUSY;
-}
-
-static int e1000_suspend(struct device *dev)
-{
- struct pci_dev *pdev = to_pci_dev(dev);
- int retval;
- bool wake;
-
- retval = __e1000_shutdown(pdev, &wake, false);
- if (!retval)
- e1000_complete_shutdown(pdev, true, wake);
-
- return retval;
-}
-
-static int e1000_runtime_suspend(struct device *dev)
-{
- struct pci_dev *pdev = to_pci_dev(dev);
- struct net_device *netdev = pci_get_drvdata(pdev);
- struct e1000_adapter *adapter = netdev_priv(netdev);
-
- if (e1000e_pm_ready(adapter)) {
- bool wake;
-
- __e1000_shutdown(pdev, &wake, true);
- }
-
- return 0;
-}
-
static int __e1000_resume(struct pci_dev *pdev)
{
struct net_device *netdev = pci_get_drvdata(pdev);
@@ -4783,6 +4737,20 @@ static int __e1000_resume(struct pci_dev
return 0;
}
+#ifdef CONFIG_PM_SLEEP
+static int e1000_suspend(struct device *dev)
+{
+ struct pci_dev *pdev = to_pci_dev(dev);
+ int retval;
+ bool wake;
+
+ retval = __e1000_shutdown(pdev, &wake, false);
+ if (!retval)
+ e1000_complete_shutdown(pdev, true, wake);
+
+ return retval;
+}
+
static int e1000_resume(struct device *dev)
{
struct pci_dev *pdev = to_pci_dev(dev);
@@ -4794,6 +4762,41 @@ static int e1000_resume(struct device *d
return __e1000_resume(pdev);
}
+#endif /* CONFIG_PM_SLEEP */
+
+#ifdef CONFIG_PM_RUNTIME
+static int e1000_runtime_suspend(struct device *dev)
+{
+ struct pci_dev *pdev = to_pci_dev(dev);
+ struct net_device *netdev = pci_get_drvdata(pdev);
+ struct e1000_adapter *adapter = netdev_priv(netdev);
+
+ if (e1000e_pm_ready(adapter)) {
+ bool wake;
+
+ __e1000_shutdown(pdev, &wake, true);
+ }
+
+ return 0;
+}
+
+static int e1000_idle(struct device *dev)
+{
+ struct pci_dev *pdev = to_pci_dev(dev);
+ struct net_device *netdev = pci_get_drvdata(pdev);
+ struct e1000_adapter *adapter = netdev_priv(netdev);
+
+ if (!e1000e_pm_ready(adapter))
+ return 0;
+
+ if (adapter->idle_check) {
+ adapter->idle_check = false;
+ if (!e1000e_has_link(adapter))
+ pm_schedule_suspend(dev, MSEC_PER_SEC);
+ }
+
+ return -EBUSY;
+}
static int e1000_runtime_resume(struct device *dev)
{
@@ -4807,7 +4810,8 @@ static int e1000_runtime_resume(struct d
adapter->idle_check = !dev->power.runtime_auto;
return __e1000_resume(pdev);
}
-#endif
+#endif /* CONFIG_PM_RUNTIME */
+#endif /* CONFIG_PM_OPS */
static void e1000_shutdown(struct pci_dev *pdev)
{
@@ -5475,17 +5479,11 @@ static DEFINE_PCI_DEVICE_TABLE(e1000_pci
};
MODULE_DEVICE_TABLE(pci, e1000_pci_tbl);
-#ifdef CONFIG_PM
+#ifdef CONFIG_PM_OPS
static const struct dev_pm_ops e1000_pm_ops = {
- .suspend = e1000_suspend,
- .resume = e1000_resume,
- .freeze = e1000_suspend,
- .thaw = e1000_resume,
- .poweroff = e1000_suspend,
- .restore = e1000_resume,
- .runtime_suspend = e1000_runtime_suspend,
- .runtime_resume = e1000_runtime_resume,
- .runtime_idle = e1000_idle,
+ SET_SYSTEM_SLEEP_PM_OPS(e1000_suspend, e1000_resume)
+ SET_RUNTIME_PM_OPS(e1000_runtime_suspend,
+ e1000_runtime_resume, e1000_idle)
};
#endif
@@ -5495,7 +5493,7 @@ static struct pci_driver e1000_driver =
.id_table = e1000_pci_tbl,
.probe = e1000_probe,
.remove = __devexit_p(e1000_remove),
-#ifdef CONFIG_PM
+#ifdef CONFIG_PM_OPS
.driver.pm = &e1000_pm_ops,
#endif
.shutdown = e1000_shutdown,
^ permalink raw reply
* Re: [PATCH RFC] tun: add ioctl to modify vnet header size
From: Michael S. Tsirkin @ 2010-03-17 22:11 UTC (permalink / raw)
To: David Stevens
Cc: David S. Miller, David Woodhouse, Herbert Xu, linux-kernel,
netdev, netdev-owner, Paul Moore, sri
In-Reply-To: <OFB24B6EEE.B788B01F-ON882576E9.00789167-882576E9.0079198A@us.ibm.com>
On Wed, Mar 17, 2010 at 03:02:44PM -0700, David Stevens wrote:
> netdev-owner@vger.kernel.org wrote on 03/17/2010 02:35:04 PM:
>
> > On Wed, Mar 17, 2010 at 02:10:11PM -0700, David Stevens wrote:
> > > Shouldn't we enforce a maximum too? Esp. if overflow/underflow
> > > will break any of the checks when it's used.
> > >
> > > +-DLS
> >
> > So the maximum is MAX_INT :)
> > I don't think it can break any checks that aren't
> > already broken - what do you have in mind?
>
> I was thinking more like a page. At least, it'd be better
> to fail when trying to set it large than failing allocations
> later. As a header, it really ought to be small.
> But if it works, or fails gracefully, at 2^31-1 on 32-bit
> machines, negative values, etc, then it's ok. Just a suggestion.
>
> +-DLS
All this does is set how much of the buffer to skip, this option does
not allocate any memory. So if you set it to a value > length that you
passed in, you get -EINVAL. Anything else should work. Negative values
are checked for and return -EINVAL when you try to set it. At least,
all that's by design - pls take a look at the code and if you see any
issues, speak up please.
I agree we don't really need to support very large values here,
it just seemed less work.
--
MST
^ permalink raw reply
* Re: [PATCH] drivers/net/wimax/i2400m/fw.c fix possible double free
From: Inaky Perez-Gonzalez @ 2010-03-17 22:10 UTC (permalink / raw)
To: Darren Jenkins
Cc: David Miller, linux-wimax, kernel-janitors@vger.kernel.org,
Kao, Cindy H, Brandewie, Dirk J, wimax@linuxwimax.org,
netdev@vger.kernel.org, Linux Kernel Mailing List
In-Reply-To: <1268829638.10618.28.camel@ICE-BOX>
On Wed, 2010-03-17 at 05:40 -0700, Darren Jenkins wrote:
> On Wed, Mar 17, 2010 at 8:14 AM, David Miller <davem@davemloft.net>
> wrote:
>
> > Therefore the krealloc() failure handling in this driver should NULL
> > out i2400m->fw_hdrs and that will fix the double kfree problem as well
> > as trap any stray references.
>
> Yes that is a much better Idea. Thanks for the advice.
> It also fixes the i2400m_barker_db problem that I didn't notice before.
>
>
> Fix double free on krealloc() failure by zeroing pointer
If krealloc() fails to aallocate a new pointer, the old block is
unmodified, so by doing this you are leaking a buffer allocation.
I think this should be solved at the site where i2400m_zrealloc_2x() is
called, with a
if (result < 0) {
kfree(i2400m->fw_hdrs);
i2400m->fw_hdrs = NULL;
goto error_zrealloc;
}
or any other better fix. I am hesitant of having zrealloc_2x free the
original pointer because it breaks the traditional semantics that come
along being called 'realloc' (realloc if successful, keep the original
if not).
Am I missing anything?
> coverity CID: 13455
>
> Signed-off-by: Darren Jenkins <darrenrjenkins@gmail.com>
> ---
> drivers/net/wimax/i2400m/fw.c | 5 +++--
> 1 files changed, 3 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/net/wimax/i2400m/fw.c b/drivers/net/wimax/i2400m/fw.c
> index 25c24f0..9f3b594 100644
> --- a/drivers/net/wimax/i2400m/fw.c
> +++ b/drivers/net/wimax/i2400m/fw.c
> @@ -232,8 +232,9 @@ int i2400m_zrealloc_2x(void **ptr, size_t *_count, size_t el_size,
> *_count = new_count;
> *ptr = nptr;
> return 0;
> - } else
> - return -ENOMEM;
> + }
> + *ptr = NULL;
> + return -ENOMEM;
> }
>
>
^ permalink raw reply
* Re: [PATCH RFC] tun: add ioctl to modify vnet header size
From: David Stevens @ 2010-03-17 22:02 UTC (permalink / raw)
To: Michael S. Tsirkin
Cc: David S. Miller, David Woodhouse, Herbert Xu, linux-kernel,
netdev, netdev-owner, Paul Moore, sri
In-Reply-To: <20100317213504.GA7433@redhat.com>
netdev-owner@vger.kernel.org wrote on 03/17/2010 02:35:04 PM:
> On Wed, Mar 17, 2010 at 02:10:11PM -0700, David Stevens wrote:
> > Shouldn't we enforce a maximum too? Esp. if overflow/underflow
> > will break any of the checks when it's used.
> >
> > +-DLS
>
> So the maximum is MAX_INT :)
> I don't think it can break any checks that aren't
> already broken - what do you have in mind?
I was thinking more like a page. At least, it'd be better
to fail when trying to set it large than failing allocations
later. As a header, it really ought to be small.
But if it works, or fails gracefully, at 2^31-1 on 32-bit
machines, negative values, etc, then it's ok. Just a suggestion.
+-DLS
^ permalink raw reply
* Re: [PATCH] gigaset: fix build failure
From: Randy Dunlap @ 2010-03-17 21:35 UTC (permalink / raw)
To: Tilman Schmidt
Cc: Karsten Keil, David Miller, Stephen Rothwell, linux-next,
Hansjoerg Lipp, isdn4linux, i4ldeveloper, netdev, linux-kernel,
stable
In-Reply-To: <20100317211231.569B140118@xenon.ts.pxnet.com>
On 03/17/10 10:37, Tilman Schmidt wrote:
> Update the dummy LL interface to the LL interface change
> introduced by commit daab433c03c15fd642c71c94eb51bdd3f32602c8.
> This fixes the build failure occurring after that commit when
> enabling ISDN_DRV_GIGASET but neither ISDN_I4L nor ISDN_CAPI.
>
> Impact: bugfix
> Signed-off-by: Tilman Schmidt <tilman@imap.cc>
Acked-by: Randy Dunlap <randy.dunlap@oracle.com>
Thanks.
> ---
> drivers/isdn/gigaset/dummyll.c | 14 +++++++++++---
> 1 files changed, 11 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/isdn/gigaset/dummyll.c b/drivers/isdn/gigaset/dummyll.c
> index 5b27c99..bd0b1ea 100644
> --- a/drivers/isdn/gigaset/dummyll.c
> +++ b/drivers/isdn/gigaset/dummyll.c
> @@ -57,12 +57,20 @@ void gigaset_isdn_stop(struct cardstate *cs)
> {
> }
>
> -int gigaset_isdn_register(struct cardstate *cs, const char *isdnid)
> +int gigaset_isdn_regdev(struct cardstate *cs, const char *isdnid)
> {
> - pr_info("no ISDN subsystem interface\n");
> return 1;
> }
>
> -void gigaset_isdn_unregister(struct cardstate *cs)
> +void gigaset_isdn_unregdev(struct cardstate *cs)
> +{
> +}
> +
> +void gigaset_isdn_regdrv(void)
> +{
> + pr_info("no ISDN subsystem interface\n");
> +}
> +
> +void gigaset_isdn_unregdrv(void)
> {
> }
--
~Randy
^ permalink raw reply
* Re: [PATCH RFC] tun: add ioctl to modify vnet header size
From: Michael S. Tsirkin @ 2010-03-17 21:35 UTC (permalink / raw)
To: David Stevens
Cc: David S. Miller, David Woodhouse, Herbert Xu, linux-kernel,
netdev, Paul Moore, sri
In-Reply-To: <OF526018DA.E6922CE9-ON882576E9.0073CB01-882576E9.007449E0@us.ibm.com>
On Wed, Mar 17, 2010 at 02:10:11PM -0700, David Stevens wrote:
> Shouldn't we enforce a maximum too? Esp. if overflow/underflow
> will break any of the checks when it's used.
>
> +-DLS
So the maximum is MAX_INT :)
I don't think it can break any checks that aren't
already broken - what do you have in mind?
--
MST
^ permalink raw reply
* Re: [PATCH] gigaset: fix build failure
From: David Miller @ 2010-03-17 21:22 UTC (permalink / raw)
To: tilman
Cc: isdn, randy.dunlap, sfr, linux-next, hjlipp, isdn4linux,
i4ldeveloper, netdev, linux-kernel, stable
In-Reply-To: <20100317211231.569B140118@xenon.ts.pxnet.com>
From: Tilman Schmidt <tilman@imap.cc>
Date: Wed, 17 Mar 2010 18:37:05 +0100
> Update the dummy LL interface to the LL interface change
> introduced by commit daab433c03c15fd642c71c94eb51bdd3f32602c8.
> This fixes the build failure occurring after that commit when
> enabling ISDN_DRV_GIGASET but neither ISDN_I4L nor ISDN_CAPI.
>
> Impact: bugfix
> Signed-off-by: Tilman Schmidt <tilman@imap.cc>
Applied, thanks.
^ permalink raw reply
* Re: [PATCH RFC] tun: add ioctl to modify vnet header size
From: David Stevens @ 2010-03-17 21:10 UTC (permalink / raw)
To: Michael S. Tsirkin
Cc: David S. Miller, David Woodhouse, Herbert Xu, linux-kernel,
Michael S. Tsirkin, netdev, Paul Moore, sri
In-Reply-To: <20100317154501.GA5244@redhat.com>
Shouldn't we enforce a maximum too? Esp. if overflow/underflow
will break any of the checks when it's used.
+-DLS
^ permalink raw reply
* Re: [Regression] r8169: enable 64-bit DMA by default for PCI Express devices (v2)
From: Francois Romieu @ 2010-03-17 20:52 UTC (permalink / raw)
To: J�rn Engel
Cc: Robert Hancock, David Miller, torvalds, linux-kernel, netdev
In-Reply-To: <20100316083501.GA3489@Dublin.logfs.org>
On Tue, Mar 16, 2010 at 09:35:01AM +0100, J�rn Engel wrote:
[...]
> I just noticed lspci is also unhappy.
>
> With patch in:
> Bikini:~# lspci -vv > lspci
> pcilib: sysfs_read_vpd: read failed: Connection timed out.
>
> With patch reverted:
> Bikini:~# lspci -vv > lspci2
> Bikini:~# diff -u lspci*
> --- lspci 2010-03-16 09:03:02.000000000 +0100
> +++ lspci2 2010-03-16 09:09:36.000000000 +0100
> @@ -246,7 +246,7 @@
> Vector table: BAR=4 offset=00000000
> PBA: BAR=4 offset=00000800
> Capabilities: [cc] Vital Product Data
> - Not readable
> + Unknown small resource type 00, will not decode more.
Neither looks good.
Did lspci output look different with an older kernel ?
--
Ueimor
^ permalink raw reply
* Re: e1000e: Fix build with CONFIG_PM disabled.
From: Rafael J. Wysocki @ 2010-03-17 20:34 UTC (permalink / raw)
To: David Miller
Cc: emil.s.tantilov, netdev, bruce.w.allan, jesse.brandeburg,
jeffrey.t.kirsher
In-Reply-To: <20100317.111717.85209898.davem@davemloft.net>
On Wednesday 17 March 2010, David Miller wrote:
> From: "Tantilov, Emil S" <emil.s.tantilov@intel.com>
> Date: Wed, 17 Mar 2010 11:57:59 -0600
>
> > But then the driver build fails again when CONFIG_PM_RUNTIME is not set.
> >
> > drivers/net/e1000e/netdev.c: In function `e1000_runtime_resume`:
> > drivers/net/e1000e/netdev.c:4807: error: `struct dev_pm_info` has no member named `runtime_auto`
>
> Ugh, can someone send me a fix for that?
I will in a while.
^ permalink raw reply
* Re: [PATCH] [V2] Add non-Virtex5 support for LL TEMAC driver
From: Grant Likely @ 2010-03-17 20:02 UTC (permalink / raw)
To: John Linn
Cc: netdev, linuxppc-dev, jwboyer, john.williams, michal.simek,
John Tyner, David Miller
In-Reply-To: <bda26c8d-00b2-411d-b4b7-ad6954b9049e@SG2EHSMHS005.ehs.local>
On Fri, Mar 12, 2010 at 7:05 PM, John Linn <john.linn@xilinx.com> wrote:
> This patch adds support for using the LL TEMAC Ethernet driver on
> non-Virtex 5 platforms by adding support for accessing the Soft DMA
> registers as if they were memory mapped instead of solely through the
> DCR's (available on the Virtex 5).
>
> The patch also updates the driver so that it runs on the MicroBlaze.
> The changes were tested on the PowerPC 440, PowerPC 405, and the
> MicroBlaze platforms.
>
> Signed-off-by: John Tyner <jtyner@cs.ucr.edu>
> Signed-off-by: John Linn <john.linn@xilinx.com>
> ---
I've not booted this, but it looks right, and it compiles fine. The
issues that Michal raised need to be delt with too, but they are
preexisting bugs unrelated to this change which you should fix up in a
separate patch.
Acked-by: Grant Likely <grant.likely@secretlab.ca>
>
> V2 - Incorporated comments from Grant and added more logic to allow the driver
> to work on MicroBlaze.
>
> drivers/net/Kconfig | 1 -
> drivers/net/ll_temac.h | 17 +++++-
> drivers/net/ll_temac_main.c | 124 ++++++++++++++++++++++++++++++++++---------
> 3 files changed, 113 insertions(+), 29 deletions(-)
>
> diff --git a/drivers/net/Kconfig b/drivers/net/Kconfig
> index 9b6efe1..5402105 100644
> --- a/drivers/net/Kconfig
> +++ b/drivers/net/Kconfig
> @@ -2443,7 +2443,6 @@ config MV643XX_ETH
> config XILINX_LL_TEMAC
> tristate "Xilinx LL TEMAC (LocalLink Tri-mode Ethernet MAC) driver"
> select PHYLIB
> - depends on PPC_DCR_NATIVE
> help
> This driver supports the Xilinx 10/100/1000 LocalLink TEMAC
> core used in Xilinx Spartan and Virtex FPGAs
> diff --git a/drivers/net/ll_temac.h b/drivers/net/ll_temac.h
> index 1af66a1..915aa34 100644
> --- a/drivers/net/ll_temac.h
> +++ b/drivers/net/ll_temac.h
> @@ -5,8 +5,11 @@
> #include <linux/netdevice.h>
> #include <linux/of.h>
> #include <linux/spinlock.h>
> +
> +#ifdef CONFIG_PPC_DCR
> #include <asm/dcr.h>
> #include <asm/dcr-regs.h>
> +#endif
>
> /* packet size info */
> #define XTE_HDR_SIZE 14 /* size of Ethernet header */
> @@ -290,8 +293,12 @@ This option defaults to enabled (set) */
>
> #define TX_CONTROL_CALC_CSUM_MASK 1
>
> +/* Align the IP data in the packet on word boundaries as MicroBlaze
> + * needs it.
> + */
> +
> #define XTE_ALIGN 32
> -#define BUFFER_ALIGN(adr) ((XTE_ALIGN - ((u32) adr)) % XTE_ALIGN)
> +#define BUFFER_ALIGN(adr) ((34 - ((u32) adr)) % XTE_ALIGN)
>
> #define MULTICAST_CAM_TABLE_NUM 4
>
> @@ -335,9 +342,15 @@ struct temac_local {
> struct mii_bus *mii_bus; /* MII bus reference */
> int mdio_irqs[PHY_MAX_ADDR]; /* IRQs table for MDIO bus */
>
> - /* IO registers and IRQs */
> + /* IO registers, dma functions and IRQs */
> void __iomem *regs;
> + void __iomem *sdma_regs;
> +#ifdef CONFIG_PPC_DCR
> dcr_host_t sdma_dcrs;
> +#endif
> + u32 (*dma_in)(struct temac_local *, int);
> + void (*dma_out)(struct temac_local *, int, u32);
> +
> int tx_irq;
> int rx_irq;
> int emac_num;
> diff --git a/drivers/net/ll_temac_main.c b/drivers/net/ll_temac_main.c
> index a18e348..9aedf9b 100644
> --- a/drivers/net/ll_temac_main.c
> +++ b/drivers/net/ll_temac_main.c
> @@ -20,9 +20,6 @@
> * or rx, so this should be okay.
> *
> * TODO:
> - * - Fix driver to work on more than just Virtex5. Right now the driver
> - * assumes that the locallink DMA registers are accessed via DCR
> - * instructions.
> * - Factor out locallink DMA code into separate driver
> * - Fix multicast assignment.
> * - Fix support for hardware checksumming.
> @@ -115,17 +112,86 @@ void temac_indirect_out32(struct temac_local *lp, int reg, u32 value)
> temac_iow(lp, XTE_CTL0_OFFSET, CNTLREG_WRITE_ENABLE_MASK | reg);
> }
>
> +/**
> + * temac_dma_in32 - Memory mapped DMA read, this function expects a
> + * register input that is based on DCR word addresses which
> + * are then converted to memory mapped byte addresses
> + */
> static u32 temac_dma_in32(struct temac_local *lp, int reg)
> {
> - return dcr_read(lp->sdma_dcrs, reg);
> + return in_be32((u32 *)(lp->sdma_regs + (reg << 2)));
> }
>
> +/**
> + * temac_dma_out32 - Memory mapped DMA read, this function expects a
> + * register input that is based on DCR word addresses which
> + * are then converted to memory mapped byte addresses
> + */
> static void temac_dma_out32(struct temac_local *lp, int reg, u32 value)
> {
> + out_be32((u32 *)(lp->sdma_regs + (reg << 2)), value);
> +}
> +
> +/* DMA register access functions can be DCR based or memory mapped.
> + * The PowerPC 440 is DCR based, the PowerPC 405 and MicroBlaze are both
> + * memory mapped.
> + */
> +#ifdef CONFIG_PPC_DCR
> +
> +/**
> + * temac_dma_dcr_in32 - DCR based DMA read
> + */
> +static u32 temac_dma_dcr_in(struct temac_local *lp, int reg)
> +{
> + return dcr_read(lp->sdma_dcrs, reg);
> +}
> +
> +/**
> + * temac_dma_dcr_out32 - DCR based DMA write
> + */
> +static void temac_dma_dcr_out(struct temac_local *lp, int reg, u32 value)
> +{
> dcr_write(lp->sdma_dcrs, reg, value);
> }
>
> /**
> + * temac_dcr_setup - If the DMA is DCR based, then setup the address and
> + * I/O functions
> + */
> +static int temac_dcr_setup(struct temac_local *lp, struct of_device *op,
> + struct device_node *np)
> +{
> + unsigned int dcrs;
> +
> + /* setup the dcr address mapping if it's in the device tree */
> +
> + dcrs = dcr_resource_start(np, 0);
> + if (dcrs != 0) {
> + lp->sdma_dcrs = dcr_map(np, dcrs, dcr_resource_len(np, 0));
> + lp->dma_in = temac_dma_dcr_in;
> + lp->dma_out = temac_dma_dcr_out;
> + dev_dbg(&op->dev, "DCR base: %x\n", dcrs);
> + return 0;
> + }
> + /* no DCR in the device tree, indicate a failure */
> + return -1;
> +}
> +
> +#else
> +
> +/*
> + * temac_dcr_setup - This is a stub for when DCR is not supported,
> + * such as with MicroBlaze
> + */
> +static int temac_dcr_setup(struct temac_local *lp, struct of_device *op,
> + struct device_node *np)
> +{
> + return -1;
> +}
> +
> +#endif
> +
> +/**
> * temac_dma_bd_init - Setup buffer descriptor rings
> */
> static int temac_dma_bd_init(struct net_device *ndev)
> @@ -172,23 +238,23 @@ static int temac_dma_bd_init(struct net_device *ndev)
> lp->rx_bd_v[i].app0 = STS_CTRL_APP0_IRQONEND;
> }
>
> - temac_dma_out32(lp, TX_CHNL_CTRL, 0x10220400 |
> + lp->dma_out(lp, TX_CHNL_CTRL, 0x10220400 |
> CHNL_CTRL_IRQ_EN |
> CHNL_CTRL_IRQ_DLY_EN |
> CHNL_CTRL_IRQ_COAL_EN);
> /* 0x10220483 */
> /* 0x00100483 */
> - temac_dma_out32(lp, RX_CHNL_CTRL, 0xff010000 |
> + lp->dma_out(lp, RX_CHNL_CTRL, 0xff010000 |
> CHNL_CTRL_IRQ_EN |
> CHNL_CTRL_IRQ_DLY_EN |
> CHNL_CTRL_IRQ_COAL_EN |
> CHNL_CTRL_IRQ_IOE);
> /* 0xff010283 */
>
> - temac_dma_out32(lp, RX_CURDESC_PTR, lp->rx_bd_p);
> - temac_dma_out32(lp, RX_TAILDESC_PTR,
> + lp->dma_out(lp, RX_CURDESC_PTR, lp->rx_bd_p);
> + lp->dma_out(lp, RX_TAILDESC_PTR,
> lp->rx_bd_p + (sizeof(*lp->rx_bd_v) * (RX_BD_NUM - 1)));
> - temac_dma_out32(lp, TX_CURDESC_PTR, lp->tx_bd_p);
> + lp->dma_out(lp, TX_CURDESC_PTR, lp->tx_bd_p);
>
> return 0;
> }
> @@ -426,9 +492,9 @@ static void temac_device_reset(struct net_device *ndev)
> temac_indirect_out32(lp, XTE_RXC1_OFFSET, val & ~XTE_RXC1_RXEN_MASK);
>
> /* Reset Local Link (DMA) */
> - temac_dma_out32(lp, DMA_CONTROL_REG, DMA_CONTROL_RST);
> + lp->dma_out(lp, DMA_CONTROL_REG, DMA_CONTROL_RST);
> timeout = 1000;
> - while (temac_dma_in32(lp, DMA_CONTROL_REG) & DMA_CONTROL_RST) {
> + while (lp->dma_in(lp, DMA_CONTROL_REG) & DMA_CONTROL_RST) {
> udelay(1);
> if (--timeout == 0) {
> dev_err(&ndev->dev,
> @@ -436,7 +502,7 @@ static void temac_device_reset(struct net_device *ndev)
> break;
> }
> }
> - temac_dma_out32(lp, DMA_CONTROL_REG, DMA_TAIL_ENABLE);
> + lp->dma_out(lp, DMA_CONTROL_REG, DMA_TAIL_ENABLE);
>
> temac_dma_bd_init(ndev);
>
> @@ -597,7 +663,7 @@ static int temac_start_xmit(struct sk_buff *skb, struct net_device *ndev)
> lp->tx_bd_tail = 0;
>
> /* Kick off the transfer */
> - temac_dma_out32(lp, TX_TAILDESC_PTR, tail_p); /* DMA start */
> + lp->dma_out(lp, TX_TAILDESC_PTR, tail_p); /* DMA start */
>
> return NETDEV_TX_OK;
> }
> @@ -663,7 +729,7 @@ static void ll_temac_recv(struct net_device *ndev)
> cur_p = &lp->rx_bd_v[lp->rx_bd_ci];
> bdstat = cur_p->app0;
> }
> - temac_dma_out32(lp, RX_TAILDESC_PTR, tail_p);
> + lp->dma_out(lp, RX_TAILDESC_PTR, tail_p);
>
> spin_unlock_irqrestore(&lp->rx_lock, flags);
> }
> @@ -674,8 +740,8 @@ static irqreturn_t ll_temac_tx_irq(int irq, void *_ndev)
> struct temac_local *lp = netdev_priv(ndev);
> unsigned int status;
>
> - status = temac_dma_in32(lp, TX_IRQ_REG);
> - temac_dma_out32(lp, TX_IRQ_REG, status);
> + status = lp->dma_in(lp, TX_IRQ_REG);
> + lp->dma_out(lp, TX_IRQ_REG, status);
>
> if (status & (IRQ_COAL | IRQ_DLY))
> temac_start_xmit_done(lp->ndev);
> @@ -692,8 +758,8 @@ static irqreturn_t ll_temac_rx_irq(int irq, void *_ndev)
> unsigned int status;
>
> /* Read and clear the status registers */
> - status = temac_dma_in32(lp, RX_IRQ_REG);
> - temac_dma_out32(lp, RX_IRQ_REG, status);
> + status = lp->dma_in(lp, RX_IRQ_REG);
> + lp->dma_out(lp, RX_IRQ_REG, status);
>
> if (status & (IRQ_COAL | IRQ_DLY))
> ll_temac_recv(lp->ndev);
> @@ -794,7 +860,7 @@ static ssize_t temac_show_llink_regs(struct device *dev,
> int i, len = 0;
>
> for (i = 0; i < 0x11; i++)
> - len += sprintf(buf + len, "%.8x%s", temac_dma_in32(lp, i),
> + len += sprintf(buf + len, "%.8x%s", lp->dma_in(lp, i),
> (i % 8) == 7 ? "\n" : " ");
> len += sprintf(buf + len, "\n");
>
> @@ -820,7 +886,6 @@ temac_of_probe(struct of_device *op, const struct of_device_id *match)
> struct net_device *ndev;
> const void *addr;
> int size, rc = 0;
> - unsigned int dcrs;
>
> /* Init network device structure */
> ndev = alloc_etherdev(sizeof(*lp));
> @@ -870,13 +935,20 @@ temac_of_probe(struct of_device *op, const struct of_device_id *match)
> goto nodev;
> }
>
> - dcrs = dcr_resource_start(np, 0);
> - if (dcrs == 0) {
> - dev_err(&op->dev, "could not get DMA register address\n");
> - goto nodev;
> + /* Setup the DMA register accesses, could be DCR or memory mapped */
> + if (temac_dcr_setup(lp, op, np)) {
> +
> + /* no DCR in the device tree, try non-DCR */
> + lp->sdma_regs = of_iomap(np, 0);
> + if (lp->sdma_regs) {
> + lp->dma_in = temac_dma_in32;
> + lp->dma_out = temac_dma_out32;
> + dev_dbg(&op->dev, "MEM base: %p\n", lp->sdma_regs);
> + } else {
> + dev_err(&op->dev, "unable to map DMA registers\n");
> + goto nodev;
> + }
> }
> - lp->sdma_dcrs = dcr_map(np, dcrs, dcr_resource_len(np, 0));
> - dev_dbg(&op->dev, "DCR base: %x\n", dcrs);
>
> lp->rx_irq = irq_of_parse_and_map(np, 0);
> lp->tx_irq = irq_of_parse_and_map(np, 1);
> --
> 1.6.2.1
>
>
>
> This email and any attachments are intended for the sole use of the named recipient(s) and contain(s) confidential information that may be proprietary, privileged or copyrighted under applicable law. If you are not the intended recipient, do not read, copy, or forward this email message or any attachments. Delete this email message and any attachments immediately.
>
>
>
--
Grant Likely, B.Sc., P.Eng.
Secret Lab Technologies Ltd.
^ permalink raw reply
* Re: [PATCH 1/1] 3c59x: Acquire vortex lock instead of disabling irq
From: David Miller @ 2010-03-17 18:47 UTC (permalink / raw)
To: chase.douglas; +Cc: netdev
In-Reply-To: <40ec3ea41003171144s6acbf2fdl4434f2147b8244a0@mail.gmail.com>
From: Chase Douglas <chase.douglas@canonical.com>
Date: Wed, 17 Mar 2010 14:44:30 -0400
> On Wed, Mar 17, 2010 at 2:27 PM, David Miller <davem@davemloft.net> wrote:
>> From: Chase Douglas <chase.douglas@canonical.com>
>> Date: Wed, 17 Mar 2010 14:20:45 -0400
>>
>>> If that's the case, what's the solution? It's not safe to call
>>> disable_irq* in softirq context anymore.
>>
>> That's a huge problem, because such a restriction has broken
>> several drivers. 3c59x is not the only one which uses this
>> technique for this reason. The 8390 one does too.
>
> So basically the answer right now is: it's broken and needs to be
> reworked, and a switch to disabling irqs is deemed inadequate. Is that
> accurate?
Yep. The bug is whatever caused disable_irq*() to stop working
where these drivers have been using it for what feels like
a century :-)
^ permalink raw reply
* Re: [PATCH 1/1] 3c59x: Acquire vortex lock instead of disabling irq
From: Chase Douglas @ 2010-03-17 18:44 UTC (permalink / raw)
To: David Miller; +Cc: netdev
In-Reply-To: <20100317.112706.143765408.davem@davemloft.net>
On Wed, Mar 17, 2010 at 2:27 PM, David Miller <davem@davemloft.net> wrote:
> From: Chase Douglas <chase.douglas@canonical.com>
> Date: Wed, 17 Mar 2010 14:20:45 -0400
>
>> If that's the case, what's the solution? It's not safe to call
>> disable_irq* in softirq context anymore.
>
> That's a huge problem, because such a restriction has broken
> several drivers. 3c59x is not the only one which uses this
> technique for this reason. The 8390 one does too.
So basically the answer right now is: it's broken and needs to be
reworked, and a switch to disabling irqs is deemed inadequate. Is that
accurate?
If that's a fair statement of the current drivers, then I can go back
and inform end users of this when they hit the bug.
-- Chase
^ permalink raw reply
* Re: [PATCH] vhost: fix error handling in vring ioctls
From: Laurent Chavey @ 2010-03-17 18:40 UTC (permalink / raw)
To: Michael S. Tsirkin; +Cc: Jiri Slaby, kvm, virtualization, netdev, linux-kernel
In-Reply-To: <97949e3e1003171054q3bee27d5yce0d92ae33e5b5ca@mail.gmail.com>
Acked-by: Laurent Chavey <chavey@google.com>
On Wed, Mar 17, 2010 at 10:54 AM, Laurent Chavey <chavey@google.com> wrote:
> Acked-by: chavey@google.com
>
>
> On Wed, Mar 17, 2010 at 7:42 AM, Michael S. Tsirkin <mst@redhat.com> wrote:
>> Stanse found a locking problem in vhost_set_vring:
>> several returns from VHOST_SET_VRING_KICK, VHOST_SET_VRING_CALL,
>> VHOST_SET_VRING_ERR with the vq->mutex held.
>> Fix these up.
>>
>> Reported-by: Jiri Slaby <jirislaby@gmail.com>
>> Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
>> ---
>> drivers/vhost/vhost.c | 18 ++++++++++++------
>> 1 files changed, 12 insertions(+), 6 deletions(-)
>>
>> diff --git a/drivers/vhost/vhost.c b/drivers/vhost/vhost.c
>> index 7cd55e0..7bd7a1e 100644
>> --- a/drivers/vhost/vhost.c
>> +++ b/drivers/vhost/vhost.c
>> @@ -476,8 +476,10 @@ static long vhost_set_vring(struct vhost_dev *d, int ioctl, void __user *argp)
>> if (r < 0)
>> break;
>> eventfp = f.fd == -1 ? NULL : eventfd_fget(f.fd);
>> - if (IS_ERR(eventfp))
>> - return PTR_ERR(eventfp);
>> + if (IS_ERR(eventfp)) {
>> + r = PTR_ERR(eventfp);
>> + break;
>> + }
>> if (eventfp != vq->kick) {
>> pollstop = filep = vq->kick;
>> pollstart = vq->kick = eventfp;
>> @@ -489,8 +491,10 @@ static long vhost_set_vring(struct vhost_dev *d, int ioctl, void __user *argp)
>> if (r < 0)
>> break;
>> eventfp = f.fd == -1 ? NULL : eventfd_fget(f.fd);
>> - if (IS_ERR(eventfp))
>> - return PTR_ERR(eventfp);
>> + if (IS_ERR(eventfp)) {
>> + r = PTR_ERR(eventfp);
>> + break;
>> + }
>> if (eventfp != vq->call) {
>> filep = vq->call;
>> ctx = vq->call_ctx;
>> @@ -505,8 +509,10 @@ static long vhost_set_vring(struct vhost_dev *d, int ioctl, void __user *argp)
>> if (r < 0)
>> break;
>> eventfp = f.fd == -1 ? NULL : eventfd_fget(f.fd);
>> - if (IS_ERR(eventfp))
>> - return PTR_ERR(eventfp);
>> + if (IS_ERR(eventfp)) {
>> + r = PTR_ERR(eventfp);
>> + break;
>> + }
>> if (eventfp != vq->error) {
>> filep = vq->error;
>> vq->error = eventfp;
>> --
>> 1.7.0.18.g0d53a5
>> --
>> 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 1/1] 3c59x: Acquire vortex lock instead of disabling irq
From: David Miller @ 2010-03-17 18:27 UTC (permalink / raw)
To: chase.douglas; +Cc: netdev
In-Reply-To: <40ec3ea41003171120x26d2e8b4q6b59c992ab28e454@mail.gmail.com>
From: Chase Douglas <chase.douglas@canonical.com>
Date: Wed, 17 Mar 2010 14:20:45 -0400
> If that's the case, what's the solution? It's not safe to call
> disable_irq* in softirq context anymore.
That's a huge problem, because such a restriction has broken
several drivers. 3c59x is not the only one which uses this
technique for this reason. The 8390 one does too.
^ permalink raw reply
* Re: [PATCH 1/1] 3c59x: Acquire vortex lock instead of disabling irq
From: Chase Douglas @ 2010-03-17 18:20 UTC (permalink / raw)
To: David Miller; +Cc: netdev
In-Reply-To: <20100317.110320.15445884.davem@davemloft.net>
On Wed, Mar 17, 2010 at 2:03 PM, David Miller <davem@davemloft.net> wrote:
> From: Chase Douglas <chase.douglas@canonical.com>
> Date: Wed, 17 Mar 2010 10:33:16 -0400
>
>> Last year, threaded IRQ handlers were introduced to the mainline kernel.
>> This change requires the disable_irq function to sleep if any IRQ
>> handler threads for a given IRQ line are running.
>>
>> Back in 2006, while working on the -rt patch set that had threaded IRQ
>> handlers, the vortex_timer function was causing scheduling bugs because
>> it is run in softirq context and called disable_irq. This patch was the
>> best fix determined at the time, and still exists in the .33 -rt
>> patchset. Now that threaded IRQ handlers are present in the mainline
>> kernel we need to apply the patch there as well.
>>
>> http://lkml.org/lkml/2006/5/12/178
>>
>> BugLink: http://bugs.launchpad.net/bugs/533335
>>
>> Signed-off-by: Chase Douglas <chase.douglas@canonical.com>
>
> This code is very much intentionally using disable_irq*().
>
> The operation being performed here is extremely expensive,
> and during that time if we have cpu interrupts disabled
> serial devices will drop characters etc.
If that's the case, what's the solution? It's not safe to call
disable_irq* in softirq context anymore. With the patch we have a
stable driver that may cause some serial devices to drop characters.
Without the patch we have an unstable driver that can lock up. To me
it seems the latter is preferable to the former, especially when the
lockup is occurs somewhat frequently.
-- Chase
^ 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