* Re: [PATCH 07/18] isdn: avm: call disable_pci_device() if pci_probe() failed
From: David Miller @ 2010-08-08 5:34 UTC (permalink / raw)
To: segooon; +Cc: kernel-janitors, isdn, adobriyan, tilman, netdev
In-Reply-To: <1281124303-13537-1-git-send-email-segooon@gmail.com>
From: Kulikov Vasiliy <segooon@gmail.com>
Date: Fri, 6 Aug 2010 23:51:41 +0400
> Driver should call disable_pci_device() if it returns from pci_probe()
"pci_disable_device()"
> with error. Also it must not be called if request_region() fails as
> it means that somebody uses device resources and rules the device.
This interface, frankly, sucks.
If this is what we want then pci_enable_device() and
pci_disable_device() should maintain a reference count,
so that device drivers need not be mindful of what
arbitrary special circumstances they should make the
disable call or not. They can make it unconditionally
and the PCI layer makes sure the device doesn't get
disabled unintentionally.
This is software interface design 101.
The current requirements are beyond unreasonable and
should be fixed before we apply patches like this.
I'm tossing all of these patches, sorry.
^ permalink raw reply
* Re: [PATCH] net: disable preemption before call smp_processor_id()
From: David Miller @ 2010-08-08 5:25 UTC (permalink / raw)
To: paulmck; +Cc: xiaosuo, therbert, netdev
In-Reply-To: <20100808045751.GF19600@linux.vnet.ibm.com>
From: "Paul E. McKenney" <paulmck@linux.vnet.ibm.com>
Date: Sat, 7 Aug 2010 21:57:51 -0700
> If I cover preemptible RCU's semantics, a first cut comes out like this:
>
> * In non-preemptible RCU implementations (TREE_RCU and TINY_RCU), it
> * is illegal to block while in an RCU read-side critical section. In
> * preemptible RCU implementations (TREE_PREEMPT_RCU and TINY_PREEMPT_RCU)
> * in CONFIG_PREEMPT kernel builds, RCU read-side critical sections may
> * be preempted, but explicit blocking is illegal. Finally, in preemptible
> * RCU implementations in real-time (CONFIG_PREEMPT_RT) kernel builds,
> * RCU read-side critical sections may be preempted and they may also
> * block, but only when acquiring spinlocks that are subject to priority
> * inheritance.
>
> Does that seem reasonable?
Sounds good to me.
^ permalink raw reply
* Re: [PATCH] net: disable preemption before call smp_processor_id()
From: Paul E. McKenney @ 2010-08-08 4:57 UTC (permalink / raw)
To: David Miller; +Cc: xiaosuo, therbert, netdev
In-Reply-To: <20100807.203700.186284252.davem@davemloft.net>
On Sat, Aug 07, 2010 at 08:37:00PM -0700, David Miller wrote:
> From: Changli Gao <xiaosuo@gmail.com>
> Date: Sat, 7 Aug 2010 17:26:02 +0800
>
> > Although netif_rx() isn't expected to be called in process context with
> > preemption enabled, it'd better handle this case. And this is why get_cpu()
> > is used in the non-RPS #ifdef branch. If tree RCU is selected,
> > rcu_read_lock() won't disable preemption, so preempt_disable() should be
> > called explictly.
> >
> > Signed-off-by: Changli Gao <xiaosuo@gmail.com>
>
> Applied, thanks Changli.
>
> Paul, perhaps the comment above rcu_read_lock()'s definition should
> be updated in rcupdate.h? It says blocking is not allowed inside
> of a read side critical section, but obviously with tree-rcu that
> is not the case.
>
> Either we should add a mention of tree-rcu's semantics or just remote
> this part of the comment altogether.
Good point, that last sentence is quite obsolete. It also survived a
recent cleanup. :-/
If I cover preemptible RCU's semantics, a first cut comes out like this:
* In non-preemptible RCU implementations (TREE_RCU and TINY_RCU), it
* is illegal to block while in an RCU read-side critical section. In
* preemptible RCU implementations (TREE_PREEMPT_RCU and TINY_PREEMPT_RCU)
* in CONFIG_PREEMPT kernel builds, RCU read-side critical sections may
* be preempted, but explicit blocking is illegal. Finally, in preemptible
* RCU implementations in real-time (CONFIG_PREEMPT_RT) kernel builds,
* RCU read-side critical sections may be preempted and they may also
* block, but only when acquiring spinlocks that are subject to priority
* inheritance.
Does that seem reasonable?
Thanx, Paul
^ permalink raw reply
* Re: [PATCH] net: disable preemption before call smp_processor_id()
From: David Miller @ 2010-08-08 3:37 UTC (permalink / raw)
To: xiaosuo; +Cc: paulmck, therbert, netdev
In-Reply-To: <1281173162-10587-1-git-send-email-xiaosuo@gmail.com>
From: Changli Gao <xiaosuo@gmail.com>
Date: Sat, 7 Aug 2010 17:26:02 +0800
> Although netif_rx() isn't expected to be called in process context with
> preemption enabled, it'd better handle this case. And this is why get_cpu()
> is used in the non-RPS #ifdef branch. If tree RCU is selected,
> rcu_read_lock() won't disable preemption, so preempt_disable() should be
> called explictly.
>
> Signed-off-by: Changli Gao <xiaosuo@gmail.com>
Applied, thanks Changli.
Paul, perhaps the comment above rcu_read_lock()'s definition should
be updated in rcupdate.h? It says blocking is not allowed inside
of a read side critical section, but obviously with tree-rcu that
is not the case.
Either we should add a mention of tree-rcu's semantics or just remote
this part of the comment altogether.
^ permalink raw reply
* Re: [PATCH] tcp: no md5sig option size check bug
From: David Miller @ 2010-08-08 3:24 UTC (permalink / raw)
To: dp
Cc: kuznet, jmorris, kaber, yoshfuji, pekkas, gilad, yony, ori,
ilpo.jarvinen, netdev, linux-kernel
In-Reply-To: <AANLkTimHMAwi158ycRh_nRYPYUTSkww-psdFai-LsTsb@mail.gmail.com>
From: Dmitry Popov <dp@highloadlab.com>
Date: Sat, 7 Aug 2010 23:17:52 +0400
> From: Dmitry Popov <dp@highloadlab.com>
>
> tcp_parse_md5sig_option doesn't check md5sig option (TCPOPT_MD5SIG)
> length, but tcp_v[46]_inbound_md5_hash assume that it's at least 16
> bytes long.
>
> Signed-off-by: Dmitry Popov <dp@highloadlab.com>
I'll apply this, but the memcmp() we do against this pointer is always
safe because there's at least skb_shared_info()'s worth of valid
memory past skb->data guarenteed at all times which is much larger
than 16 bytes.
So at worst we'd access garbage, but never past a valid piece of
allocated memory.
Thanks.
^ permalink raw reply
* [PATCH] tcp: no md5sig option size check bug
From: Dmitry Popov @ 2010-08-07 19:17 UTC (permalink / raw)
To: David S. Miller, Alexey Kuznetsov, James Morris, Patrick McHardy,
Hideaki YOSHIFUJI
Cc: Pekka Savola (ipv6), Gilad Ben-Yossef, Yony Amit, Ori Finkelman,
Ilpo Järvinen, netdev, linux-kernel
From: Dmitry Popov <dp@highloadlab.com>
tcp_parse_md5sig_option doesn't check md5sig option (TCPOPT_MD5SIG)
length, but tcp_v[46]_inbound_md5_hash assume that it's at least 16
bytes long.
Signed-off-by: Dmitry Popov <dp@highloadlab.com>
---
net/ipv4/tcp_input.c | 2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/net/ipv4/tcp_input.c b/net/ipv4/tcp_input.c
index 3c426cb..e663b78 100644
--- a/net/ipv4/tcp_input.c
+++ b/net/ipv4/tcp_input.c
@@ -3930,7 +3930,7 @@ u8 *tcp_parse_md5sig_option(struct tcphdr *th)
if (opsize < 2 || opsize > length)
return NULL;
if (opcode == TCPOPT_MD5SIG)
- return ptr;
+ return opsize == TCPOLEN_MD5SIG ? ptr : NULL;
}
ptr += opsize - 2;
length -= opsize;
^ permalink raw reply related
* Re: [RFC PATCH] platform: Faciliatate the creation of pseduo-platform busses
From: Grant Likely @ 2010-08-07 17:28 UTC (permalink / raw)
To: Greg KH
Cc: Patrick Pannuto, Patrick Pannuto, linux-kernel@vger.kernel.org,
linux-arm-msm@vger.kernel.org, linux-omap@vger.kernel.org,
damm@opensource.se, lethal@linux-sh.org, rjw@sisk.pl,
dtor@mail.ru, eric.y.miao@gmail.com, netdev@vger.kernel.org,
Kevin Hilman
In-Reply-To: <AANLkTimHbcF+fz=aBMLdhLo2s-rhfNvaPCtqE8nYJ8aT@mail.gmail.com>
On Sat, Aug 7, 2010 at 12:35 AM, Grant Likely <grant.likely@secretlab.ca> wrote:
> On Fri, Aug 6, 2010 at 5:46 PM, Greg KH <gregkh@suse.de> wrote:
>> That would be nice, but take your "standard" PC today:
>> > ls /sys/devices/platform/
>> Fixed MDIO bus.0 i8042 pcspkr power serial8250 uevent vesafb.0
>>
>> There are tty devices below the serial port, which is nice to see, but
>> the others? I don't know what type of bus they would be on, do you?
[...]
> I wouldn't have any problem modifying those specific drivers to
> register under something like /sys/devices/legacy, but I don't really
> think it is in any way necessary.
Or for that matter, make those drivers explicitly use
/sys/devices/platform so that I don't cause churn on PCs. :-) I'd
like to be rid of it as default behaviour for embedded though.
g.
^ permalink raw reply
* Re: [PATCH 1/42] drivers/net/wan: Adjust confusing if indentation
From: Krzysztof Halasa @ 2010-08-07 12:37 UTC (permalink / raw)
To: Julia Lawall; +Cc: netdev, linux-kernel, kernel-janitors
In-Reply-To: <Pine.LNX.4.64.1008052216400.31692@ask.diku.dk>
Julia Lawall <julia@diku.dk> writes:
> drivers/net/wan/c101.c | 2 +-
> drivers/net/wan/n2.c | 2 +-
> drivers/net/wan/pci200syn.c | 2 +-
Fully Acked-by: Krzysztof Hałasa <khc@pm.waw.pl>
Thanks.
--
Krzysztof Halasa
^ permalink raw reply
* [PATCH] net: disable preemption before call smp_processor_id()
From: Changli Gao @ 2010-08-07 9:26 UTC (permalink / raw)
To: David S. Miller; +Cc: Paul E. McKenney, Tom Herbert, netdev, Changli Gao
Although netif_rx() isn't expected to be called in process context with
preemption enabled, it'd better handle this case. And this is why get_cpu()
is used in the non-RPS #ifdef branch. If tree RCU is selected,
rcu_read_lock() won't disable preemption, so preempt_disable() should be
called explictly.
Signed-off-by: Changli Gao <xiaosuo@gmail.com>
---
net/core/dev.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/net/core/dev.c b/net/core/dev.c
index 2b50896..1ae6543 100644
--- a/net/core/dev.c
+++ b/net/core/dev.c
@@ -2517,6 +2517,7 @@ int netif_rx(struct sk_buff *skb)
struct rps_dev_flow voidflow, *rflow = &voidflow;
int cpu;
+ preempt_disable();
rcu_read_lock();
cpu = get_rps_cpu(skb->dev, skb, &rflow);
@@ -2526,6 +2527,7 @@ int netif_rx(struct sk_buff *skb)
ret = enqueue_to_backlog(skb, cpu, &rflow->last_qtail);
rcu_read_unlock();
+ preempt_enable();
}
#else
{
^ permalink raw reply related
* Re: [PATCH] platform: Facilitate the creation of pseduo-platform busses
From: Grant Likely @ 2010-08-07 6:53 UTC (permalink / raw)
To: Patrick Pannuto
Cc: Kevin Hilman, linux-kernel@vger.kernel.org,
linux-arm-msm@vger.kernel.org, linux-omap@vger.kernel.org,
damm@opensource.se, lethal@linux-sh.org, rjw@sisk.pl,
eric.y.miao@gmail.com, netdev@vger.kernel.org, Greg Kroah-Hartman,
alan, zt.tmzt, magnus.damm
In-Reply-To: <4C5B646C.1030308@codeaurora.org>
On Thu, Aug 5, 2010 at 7:25 PM, Patrick Pannuto <ppannuto@codeaurora.org> wrote:
> On 08/05/2010 04:16 PM, Grant Likely wrote:
>> The relevant question before going down this path is, "Is the
>> omap/sh/other-soc behaviour something fundamentally different from the
>> platform bus? Or is it something complementary that would be better
>> handled with a notifier or some orthogonal method of adding new
>> behaviour?"
>>
>> I don't have a problem with multiple platform_bus instances using the
>> same code (I did suggest it after all), but I do worry about muddying
>> the Linux device model or making it overly complex. Binding single
>> drivers to multiple device types could be messy.
>>
>> Cheers,
>> g.
>
> From your other email, the distinction of /bus_types/ versus /physical
> or logical buses/ was very valuable (thanks). I am becoming less
> convinced that I need this infrastructure or the ability to create
> pseudo-platform buses. Let me outline some thoughts below, which I
> think at least solves my problems ( ;) ), and if some of the OMAP folks
> and Alan could chime in as to whether they can apply something similar,
> or if they still have need of creating actual new bus types?
>
>
> As we are exploring all of this, let me put a concrete (ish) scenario
> out there to discuss:
>
> SUB_BUS1---DEVICE1
> /
> CPU ---
> \
> SUB_BUS2---DEVICE2
>
> DEVICE1 and DEVICE2 are the same device (say, usb host controller, or
> whatever), and they should be driven by the same driver. However,
> physically they are attached to different buses.
>
> SUB_BUS1 and SUB_BUS2 are *different* and let's say require a different
> suspend method. If I understand correctly, the right way to build the
> topology would be:
>
> struct device_type SUB_BUS1_type = {
> .name = "sub-bus1-type",
> .pm = &sub_bus1_pm_ops;
> };
>
> struct platform_device SUB_BUS1 = {
> .init_name = "sub-bus1",
> .dev.type = &sub_bus1_type,
> };
>
> struct platform_device DEVICE1 = {
> .name = "device1",
> .dev.parent = &SUB_BUS1.dev,
> };
>
> platform_device_register(&SUB_BUS1);
> platform_device_register(&DEVICE1);
>
> [analogous for *2]
Not quite. The device_type is intended to collect similar, but
different things (ie, all keyboards, regardless of how they are
attached to the system). Trying to capture the device-specific
behavour really belongs in the specific device driver attached to the
SUB_BUS* device. In fact, the way you've constructed your example,
SUB_BUS1 and SUB_BUS2 don't really need to be platform_devices at all;
you could have used a plain struct device because in this example you
aren't binding them to a device driver).
That being said, because each bus *does* have custom behaviour, it
probably does make sense to either bind each to a device driver, or to
do something to each child device that changes the PM behaviour. I
haven't dug into the problem domain deep enough to give you absolute
answers, but the devres approach looks promising, as does creating a
derived platform_bus_type (although I'm not fond of sharing device
drivers between multiple bus_types). Another option might be giving
the parent struct device some runtime PM operations that it performs
per-child. I'm just throwing out ideas here though.
> which would yield:
>
> /sys/bus/platform/devices/
> |
> |-SUB_BUS1
> | |
> | |-DEVICE1
> |
> |-SUB_BUS2
> | |
> | |-DEVICE2
You're looking in the wrong place. Look in /sys/devices instead of
/sys/bus... (see comments below)
> Which is the correct topology, and logically provides all the correct
> interfaces. The only thing that becomes confusing now, is that SUB_BUS*
> is *actually* a physically different bus, yet it's not in /sys/bus;
> although I suppose this is actually how the world works presently (you
> don't see an individual entry in /sys/bus for each usb host controller,
> which is technically defining a sub-bus...)
/sys/bus is for bus_types, not bus instances. All USB devices will be
listed in /sys/bus/usb/devices regardless of the bus instance that
they attached to.
However, look carefully at the files in /sys/bus/*/devices. Notice
that they are all symlinks? Notice that the all of them point to
directories in /sys/devices? /sys/bus tells you which devices are
registered against each bus type, but /sys/devices is the actual
structure. Always look in /sys/devices when you want to know the
topology of the system.
Cheers,
g.
^ permalink raw reply
* Re: [RFC PATCH] platform: Faciliatate the creation of pseduo-platform busses
From: Grant Likely @ 2010-08-07 6:35 UTC (permalink / raw)
To: Greg KH
Cc: Patrick Pannuto, Patrick Pannuto, linux-kernel@vger.kernel.org,
linux-arm-msm@vger.kernel.org, linux-omap@vger.kernel.org,
damm@opensource.se, lethal@linux-sh.org, rjw@sisk.pl,
dtor@mail.ru, eric.y.miao@gmail.com, netdev@vger.kernel.org,
Kevin Hilman
In-Reply-To: <20100806234635.GC16793@suse.de>
On Fri, Aug 6, 2010 at 5:46 PM, Greg KH <gregkh@suse.de> wrote:
> On Fri, Aug 06, 2010 at 09:12:27AM -0600, Grant Likely wrote:
>> On Fri, Aug 6, 2010 at 8:27 AM, Greg KH <gregkh@suse.de> wrote:
>> > On Thu, Aug 05, 2010 at 04:59:35PM -0600, Grant Likely wrote:
>> >> (On that point Greg, what is the reason for even having the
>> >> /sys/devices/platform/ parent? Why not just let the platform devices
>> >> sit at the root of the device tree? In the OF case (granted, I'm
>> >> biased) all of the platform_device registrations reflect the actual
>> >> device hierarchy expressed in the device tree data.)
>> >
>> > If we sat them at the "root", there would be a bunch of them there. I
>> > don't know, we could drop the parent, I guess whoever created the
>> > platform device oh so long ago, decided that it would look nicer to be
>> > in this type of structure.
>>
>> Personally I'd rather see a meaningful structure used here. Maybe
>> having them all in the root will encourage people to find realistic
>> parents for their platform devices. :-)
>
> That would be nice, but take your "standard" PC today:
> > ls /sys/devices/platform/
> Fixed MDIO bus.0 i8042 pcspkr power serial8250 uevent vesafb.0
>
> There are tty devices below the serial port, which is nice to see, but
> the others? I don't know what type of bus they would be on, do you?
Does it matter? On my PC I count 7 devices. On my servers, I see the
same. I don't personally don't see any disadvantage to having them at
the root.
However, looking at them:
"Fixed MDIO bus" is actually a complete hack that I'm going to try and
get rid of.
i8042 is keyboard/mouse that probably lives on the southbridge. I
imagine hanging off the PCI bus in the ISA IO range.
ditto pcspkr
ditto serial8250
ditto vesafb
I wouldn't have any problem modifying those specific drivers to
register under something like /sys/devices/legacy, but I don't really
think it is in any way necessary.
I suppose one *could* try to figure out the actual PCI topology that
leads to these devices, but that gets into trying to guess what the
BIOS set up to decide where to register those things. Probably a lot
of error-prone work for absolutely no benefit. :-)
However, on the embedded side I'm seeing a lot of devices show up
under /sys/devices/platform. Embedded is generally different. We
*know* what the bus topology is. However, it seems to me that many
developers are under the mistaken assumption that platform devices
must live in /sys/devices/platform, so they don't bother reflecting
the topology in the device model and it seems to be affecting how
embedded PM is being approached (my opinion). Removing the "struct
device platform_bus" may reduce the confusion.
>> Why don't I float a patch to remove this and see if anybody freaks
>> out. Should I wrap it with a CONFIG_ so that it can be configurable
>> for a release or to, or just make it unconditional?
>
> If you can figure out a structure for the desktop/server machines, sure,
> I say just always enable it :)
I've got a patch in my tree now. I'll play with it a bit before
posting it for RFC.
g.
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" 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/6] bna: Brocade 10Gb Ethernet device driver
From: Debashis Dutt @ 2010-08-07 3:23 UTC (permalink / raw)
To: Stephen Hemminger; +Cc: netdev@vger.kernel.org
Hi Stephen,
Thanks a lot for your comments.
> + if (likely
> + (wis > BNA_QE_FREE_CNT(tcb, tcb->q_depth) ||
> + vectors > BNA_QE_FREE_CNT(unmap_q, unmap_q->q_depth))) {
> + BNAD_UPDATE_CTR(bnad, netif_queue_stop);
> + return NETDEV_TX_BUSY;
>The transmit routine should check for available space after
> queueing to device, so you can avoid having to return
>TX_BUSY.
However your above comment is not very clear to me.
Our Tx routine already cleans up the Tx buffers at the end, through a tasklet.
Cleaning of Tx buffers happens
1) In the sending context at the end of the Tx routine.
2) In the IRQ context when we get a Tx completion interrupt.
Only thing that we could have done, is do this check again at the end of the Tx routine
and called netif_stop_queue() if required, so that the stack stops sending. Even then I am
not sure that we could avoid the current check and returning TX_BUSY.
It would be nice if you clarify, so that I understand this better.
Thanks
--Debashis
Linux LL Driver Team.
-----Original Message-----
From: Stephen Hemminger [mailto:shemminger@vyatta.com]
Sent: Wednesday, August 04, 2010 10:09 AM
To: Rasesh Mody
Cc: netdev@vger.kernel.org; Debashis Dutt; Jing Huang
Subject: Re: [PATCH 1/6] bna: Brocade 10Gb Ethernet device driver
On Tue, 3 Aug 2010 22:15:36 -0700
Rasesh Mody <rmody@brocade.com> wrote:
> From: Rasesh Mody <rmody@brocade.com>
>
> This is patch 1/6 which contains linux driver source for
> Brocade's BR1010/BR1020 10Gb CEE capable ethernet adapter.
> Source is based against net-next-2.6.
>
> We wish this patch to be considered for inclusion in net-next-2.6
>
> Signed-off-by: Rasesh Mody <rmody@brocade.com>
> ---
> bnad.c | 3326 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++
> bnad.h | 474 ++++++++
> bnad_ethtool.c | 1269 +++++++++++++++++++++
> 3 files changed, 5069 insertions(+)
>
> diff -ruP net-next-2.6.35-rc1-orig/drivers/net/bna/bnad.c net-next-2.6.35-rc1-mod/drivers/net/bna/bnad.c
> --- net-next-2.6.35-rc1-orig/drivers/net/bna/bnad.c 1969-12-31 16:00:00.000000000 -0800
> +++ net-next-2.6.35-rc1-mod/drivers/net/bna/bnad.c 2010-08-02 17:19:19.447239000 -0700
> @@ -0,0 +1,3326 @@
> +/*
> + * Linux network driver for Brocade Converged Network Adapter.
> + *
> + * This program is free software; you can redistribute it and/or modify it
> + * under the terms of the GNU General Public License (GPL) Version 2 as
> + * published by the Free Software Foundation
> + *
> + * This program is distributed in the hope that it will be useful, but
> + * WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
> + * General Public License for more details.
> + */
> +/*
> + * Copyright (c) 2005-2009 Brocade Communications Systems, Inc.
> + * All rights reserved
> + * www.brocade.com
> + */
> +#include <linux/netdevice.h>
> +#include <linux/skbuff.h>
> +#include <linux/etherdevice.h>
> +#include <linux/in.h>
> +#include <linux/ethtool.h>
> +#include <linux/if_vlan.h>
> +#include <linux/if_ether.h>
> +#include <linux/ip.h>
> +
> +#include "bnad.h"
> +#include "bna.h"
> +#include "cna.h"
> +
> +DEFINE_MUTEX(bnad_fwimg_mutex);
> +
> +/*
> + * Module params
> + */
> +static uint bnad_msix_disable;
> +module_param(bnad_msix_disable, uint, 0444);
> +MODULE_PARM_DESC(bnad_msix_disable, "Disable MSIX mode");
> +
> +static uint bnad_ioc_auto_recover = 1;
> +module_param(bnad_ioc_auto_recover, uint, 0444);
> +MODULE_PARM_DESC(bnad_ioc_auto_recover, "Enable / Disable auto recovery");
> +
> +/*
> + * Global variables
> + */
> +u32 bna_id;
> +u32 bnad_rxqs_per_cq = 2;
> +
> +DECLARE_MUTEX(bnad_list_sem);
> +LIST_HEAD(bnad_list);
> +
> +const u8 bnad_bcast_addr[] = {0xff, 0xff, 0xff, 0xff, 0xff, 0xff};
Surprised this isn't defined somewhere else.
> +
> +/*
> + * Local MACROS
> + */
> +#define BNAD_TX_UNMAPQ_DEPTH (bnad->txq_depth * 2)
> +
> +#define BNAD_RX_UNMAPQ_DEPTH (bnad->rxq_depth)
> +
> +#define BNAD_GET_MBOX_IRQ(_bnad) \
> + (((_bnad)->cfg_flags & BNAD_CF_MSIX) ? \
> + ((_bnad)->msix_table[(_bnad)->msix_num - 1].vector) : \
> + ((_bnad)->pcidev->irq))
> +
> +#define BNAD_FILL_UNMAPQ_MEM_REQ(_res_info, _num, _depth) \
> +do { \
> + (_res_info)->res_type = BNA_RES_T_MEM; \
> + (_res_info)->res_u.mem_info.mem_type = BNA_MEM_T_KVA; \
> + (_res_info)->res_u.mem_info.num = (_num); \
> + (_res_info)->res_u.mem_info.len = \
> + sizeof(struct bnad_unmap_q) + \
> + (sizeof(struct bnad_skb_unmap) * ((_depth) - 1)); \
> +} while (0)
> +
> +void
> +bnad_add_to_list(struct bnad *bnad)
> +{
> + down(&bnad_list_sem);
> + list_add_tail(&bnad->list_entry, &bnad_list);
> + bna_id++;
> + up(&bnad_list_sem);
> +}
Why do you need to list semaphore? Isn't RTNL mutex held
when this is done. If you have to have own exclusion use
a mutex for this.
> +void
> +bnad_remove_from_list(struct bnad *bnad)
> +{
> + down(&bnad_list_sem);
> + list_del(&bnad->list_entry);
> + up(&bnad_list_sem);
> +}
> +
> +const struct pci_device_id bnad_pci_id_table[] = {
> + {
> + PCI_DEVICE(PCI_VENDOR_ID_BROCADE,
> + PCI_DEVICE_ID_BROCADE_CT),
> + .class = PCI_CLASS_NETWORK_ETHERNET << 8,
> + .class_mask = 0xffff00
> + }, {0, }
> +};
Why is this not static?
> +/* TX */
> +/* bnad_start_xmit : Netdev entry point for Transmit */
> +/* Called under lock held by net_device */
> +
> +netdev_tx_t
> +bnad_start_xmit(struct sk_buff *skb, struct net_device *netdev)
Should also be static...
> +{
> + struct bnad *bnad = netdev_priv(netdev);
> +
> + u16 txq_prod, vlan_tag = 0;
> + u32 unmap_prod, wis, wis_used, wi_range;
> + u32 vectors, vect_id, i, acked;
> + u32 tx_id;
> + int err;
> +
> + struct bnad_tx_info *tx_info;
> + struct bna_tcb *tcb;
> + struct bnad_unmap_q *unmap_q;
> + dma_addr_t dma_addr;
> + struct bna_txq_entry *txqent;
> + bna_txq_wi_ctrl_flag_t flags;
> +
> + if (unlikely
> + (skb->len <= ETH_HLEN || skb->len > BFI_TX_MAX_DATA_PER_PKT)) {
> + dev_kfree_skb(skb);
> + return NETDEV_TX_OK;
> + }
> +
> + /*
> + * Takes care of the Tx that is scheduled between clearing the flag
> + * and the netif_stop_queue() call.
> + */
> + if (unlikely(!test_bit(BNAD_RF_TX_STARTED, &bnad->run_flags))) {
> + dev_kfree_skb(skb);
> + return NETDEV_TX_OK;
> + }
> +
> + tx_id = BNAD_GET_TX_ID(skb);
> +
> + tx_info = &bnad->tx_info[tx_id];
> + tcb = tx_info->tcb[tx_id];
> + unmap_q = tcb->unmap_q;
> +
> + vectors = 1 + skb_shinfo(skb)->nr_frags;
> + if (vectors > BFI_TX_MAX_VECTORS_PER_PKT) {
> + dev_kfree_skb(skb);
> + return NETDEV_TX_OK;
> + }
> + wis = BNA_TXQ_WI_NEEDED(vectors); /* 4 vectors per work item */
> + acked = 0;
> + if (unlikely
> + (wis > BNA_QE_FREE_CNT(tcb, tcb->q_depth) ||
> + vectors > BNA_QE_FREE_CNT(unmap_q, unmap_q->q_depth))) {
> + if ((u16) (*tcb->hw_consumer_index) !=
> + tcb->consumer_index &&
> + !test_and_set_bit(BNAD_TXQ_FREE_SENT, &tcb->flags)) {
> + acked = bnad_free_txbufs(bnad, tcb);
> + bna_ib_ack(tcb->i_dbell, acked);
> + smp_mb__before_clear_bit();
> + clear_bit(BNAD_TXQ_FREE_SENT, &tcb->flags);
> + } else {
> + netif_stop_queue(netdev);
> + BNAD_UPDATE_CTR(bnad, netif_queue_stop);
> + }
> +
> + smp_mb();
> + /*
> + * Check again to deal with race condition between
> + * netif_stop_queue here, and netif_wake_queue in
> + * interrupt handler which is not inside netif tx lock.
> + */
> + if (likely
> + (wis > BNA_QE_FREE_CNT(tcb, tcb->q_depth) ||
> + vectors > BNA_QE_FREE_CNT(unmap_q, unmap_q->q_depth))) {
> + BNAD_UPDATE_CTR(bnad, netif_queue_stop);
> + return NETDEV_TX_BUSY;
The transmit routine should check for available space after
queueing to device, so you can avoid having to return
TX_BUSY.
^ permalink raw reply
* Re: [RFC PATCH] platform: Faciliatate the creation of pseduo-platform busses
From: Greg KH @ 2010-08-06 23:46 UTC (permalink / raw)
To: Grant Likely
Cc: Patrick Pannuto, Patrick Pannuto, linux-kernel@vger.kernel.org,
linux-arm-msm@vger.kernel.org, linux-omap@vger.kernel.org,
damm@opensource.se, lethal@linux-sh.org, rjw@sisk.pl,
dtor@mail.ru, eric.y.miao@gmail.com, netdev@vger.kernel.org,
Kevin Hilman
In-Reply-To: <AANLkTi=VbVed09=u-Ea0L8c_CWZ_CLLjWH9ZjD2dyg6Y@mail.gmail.com>
On Fri, Aug 06, 2010 at 09:12:27AM -0600, Grant Likely wrote:
> On Fri, Aug 6, 2010 at 8:27 AM, Greg KH <gregkh@suse.de> wrote:
> > On Thu, Aug 05, 2010 at 04:59:35PM -0600, Grant Likely wrote:
> >> (On that point Greg, what is the reason for even having the
> >> /sys/devices/platform/ parent? Why not just let the platform devices
> >> sit at the root of the device tree? In the OF case (granted, I'm
> >> biased) all of the platform_device registrations reflect the actual
> >> device hierarchy expressed in the device tree data.)
> >
> > If we sat them at the "root", there would be a bunch of them there. I
> > don't know, we could drop the parent, I guess whoever created the
> > platform device oh so long ago, decided that it would look nicer to be
> > in this type of structure.
>
> Personally I'd rather see a meaningful structure used here. Maybe
> having them all in the root will encourage people to find realistic
> parents for their platform devices. :-)
That would be nice, but take your "standard" PC today:
> ls /sys/devices/platform/
Fixed MDIO bus.0 i8042 pcspkr power serial8250 uevent vesafb.0
There are tty devices below the serial port, which is nice to see, but
the others? I don't know what type of bus they would be on, do you?
> Why don't I float a patch to remove this and see if anybody freaks
> out. Should I wrap it with a CONFIG_ so that it can be configurable
> for a release or to, or just make it unconditional?
If you can figure out a structure for the desktop/server machines, sure,
I say just always enable it :)
thanks,
greg k-h
^ permalink raw reply
* Re: [PATCH] sfq: add dummy bind/unbind handles
From: Jarek Poplawski @ 2010-08-06 23:17 UTC (permalink / raw)
To: Stephen Hemminger; +Cc: David Miller, netdev, Franchoze Eric
In-Reply-To: <20100806152313.43abc30b@nehalam>
Stephen Hemminger wrote, On 07.08.2010 00:23:
> Applying a filter to an SFQ qdisc would cause null dereference
> in tcf_bind_filter because although SFQ is classful it didn't
> have all the necessary equipment.
>
> Better alternative to changing tcf_bind API is to just fix
> SFQ. This should go to net-2.6 and stable.
>
Hmm... FYI, actually I've sent already a similar patch to the
original bug report thread (except .unbind_tcf method which
doesn't matter for fixing this bug, so should be rather
implemented in a separate patch, if needed at all in this
case).
Jarek P.
>
> Signed-off-by: Stephen Hemminger <shemminger@vyatta.com>
>
> --- a/net/sched/sch_sfq.c 2010-08-06 15:07:26.552820159 -0700
> +++ b/net/sched/sch_sfq.c 2010-08-06 15:14:24.458287452 -0700
> @@ -502,6 +502,10 @@ static unsigned long sfq_get(struct Qdis
> return 0;
> }
>
> +static void sfq_put(struct Qdisc *q, unsigned long cl)
> +{
> +}
> +
> static struct tcf_proto **sfq_find_tcf(struct Qdisc *sch, unsigned long cl)
> {
> struct sfq_sched_data *q = qdisc_priv(sch);
> @@ -511,6 +515,12 @@ static struct tcf_proto **sfq_find_tcf(s
> return &q->filter_list;
> }
>
> +static unsigned long sfq_bind_tcf(struct Qdisc *sch, unsigned long parent,
> + u32 cl)
> +{
> + return 0;
> +}
> +
> static int sfq_dump_class(struct Qdisc *sch, unsigned long cl,
> struct sk_buff *skb, struct tcmsg *tcm)
> {
> @@ -556,6 +566,8 @@ static void sfq_walk(struct Qdisc *sch,
> static const struct Qdisc_class_ops sfq_class_ops = {
> .get = sfq_get,
> .tcf_chain = sfq_find_tcf,
> + .bind_tcf = sfq_bind_tcf,
> + .unbind_tcf = sfq_put,
> .dump = sfq_dump_class,
> .dump_stats = sfq_dump_class_stats,
>
^ permalink raw reply
* [PATCH] sfq: add dummy bind/unbind handles
From: Stephen Hemminger @ 2010-08-06 22:23 UTC (permalink / raw)
To: Stephen Hemminger; +Cc: Jarek Poplawski, David Miller, netdev, Franchoze Eric
In-Reply-To: <20100806145818.66d389eb@nehalam>
Applying a filter to an SFQ qdisc would cause null dereference
in tcf_bind_filter because although SFQ is classful it didn't
have all the necessary equipment.
Better alternative to changing tcf_bind API is to just fix
SFQ. This should go to net-2.6 and stable.
Signed-off-by: Stephen Hemminger <shemminger@vyatta.com>
--- a/net/sched/sch_sfq.c 2010-08-06 15:07:26.552820159 -0700
+++ b/net/sched/sch_sfq.c 2010-08-06 15:14:24.458287452 -0700
@@ -502,6 +502,10 @@ static unsigned long sfq_get(struct Qdis
return 0;
}
+static void sfq_put(struct Qdisc *q, unsigned long cl)
+{
+}
+
static struct tcf_proto **sfq_find_tcf(struct Qdisc *sch, unsigned long cl)
{
struct sfq_sched_data *q = qdisc_priv(sch);
@@ -511,6 +515,12 @@ static struct tcf_proto **sfq_find_tcf(s
return &q->filter_list;
}
+static unsigned long sfq_bind_tcf(struct Qdisc *sch, unsigned long parent,
+ u32 cl)
+{
+ return 0;
+}
+
static int sfq_dump_class(struct Qdisc *sch, unsigned long cl,
struct sk_buff *skb, struct tcmsg *tcm)
{
@@ -556,6 +566,8 @@ static void sfq_walk(struct Qdisc *sch,
static const struct Qdisc_class_ops sfq_class_ops = {
.get = sfq_get,
.tcf_chain = sfq_find_tcf,
+ .bind_tcf = sfq_bind_tcf,
+ .unbind_tcf = sfq_put,
.dump = sfq_dump_class,
.dump_stats = sfq_dump_class_stats,
^ permalink raw reply
* Re: [PATCH] net: convert to rcu_dereference_index_check()
From: Paul E. McKenney @ 2010-08-06 22:52 UTC (permalink / raw)
To: Herbert Xu; +Cc: netdev, linux-kernel, davem, arnd
In-Reply-To: <20100806221520.GA31949@gondor.apana.org.au>
On Sat, Aug 07, 2010 at 06:15:20AM +0800, Herbert Xu wrote:
> On Fri, Aug 06, 2010 at 02:26:07PM -0700, Paul E. McKenney wrote:
> > Hello, Herbert!
> >
> > At long last, the following is the final patch blocking acceptance
> > of Arnd's sparse annotations for RCU. It was itself blocked behind
> > the definition of rcu_dereference_index_check(), which made it to
> > mainline only just today.
> >
> > Would you be willing to ack this patch so that I can push it to -tip,
> > along with Arnd's work and a bunch of other stuff?
>
> Sure,
>
> Acked-by: Herbert Xu <herbert@gondor.apana.org.au>
Thank you, Herbert!!!
Thanx, Paul
^ permalink raw reply
* Re: [PATCH 1/9] net classifier: dont allow filters on semi-classful qdisc
From: Jarek Poplawski @ 2010-08-06 22:26 UTC (permalink / raw)
To: Stephen Hemminger; +Cc: David Miller, netdev
In-Reply-To: <20100806145818.66d389eb@nehalam>
Stephen Hemminger wrote, On 06.08.2010 23:58:
> On Fri, 06 Aug 2010 23:24:47 +0200
> Jarek Poplawski <jarkao2@gmail.com> wrote:
>
>> Stephen Hemminger wrote, On -10.01.-28163 20:59:
>>
>>> There are several qdisc which only support a single class (sfq, mq, tbf)
>>> and the kernel would dereference a null pointer (bind_tcf), if a user
>>> attempted to apply a filter one of these classes.
>>
>>
>> mq and tbf can't have this issue because they don't have
>> .tcf_chain class method. sfq should support it on purpose
>> after this patch:
>>
>> http://git.kernel.org/?p=linux/kernel/git/torvalds/linux-2.6.git;a=commitdiff;h=7d2681a6ff4f9ab5e48d02550b4c6338f1638998
>> and needs tiny fix only.
>
> Probably best to fix both ways. Fix sfq to allow filters to
> be chained, and fix API to prevent refuse to allow qdisc to
> register with tcf_chain && !bind_tcf
>
Yes, but your patch needs a different changelog and I'm not sure
it's necessary for stable (there should be no other such cases
for now).
Jarek P.
^ permalink raw reply
* Re: [PATCH] net: convert to rcu_dereference_index_check()
From: Herbert Xu @ 2010-08-06 22:15 UTC (permalink / raw)
To: Paul E. McKenney; +Cc: netdev, linux-kernel, davem, arnd
In-Reply-To: <20100806212607.GA9410@linux.vnet.ibm.com>
On Fri, Aug 06, 2010 at 02:26:07PM -0700, Paul E. McKenney wrote:
> Hello, Herbert!
>
> At long last, the following is the final patch blocking acceptance
> of Arnd's sparse annotations for RCU. It was itself blocked behind
> the definition of rcu_dereference_index_check(), which made it to
> mainline only just today.
>
> Would you be willing to ack this patch so that I can push it to -tip,
> along with Arnd's work and a bunch of other stuff?
Sure,
Acked-by: Herbert Xu <herbert@gondor.apana.org.au>
Thanks,
--
Email: Herbert Xu <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt
^ permalink raw reply
* Re: [PATCH 1/9] net classifier: dont allow filters on semi-classful qdisc
From: Stephen Hemminger @ 2010-08-06 21:58 UTC (permalink / raw)
To: Jarek Poplawski; +Cc: David Miller, netdev
In-Reply-To: <4C5C7D9F.4040303@gmail.com>
On Fri, 06 Aug 2010 23:24:47 +0200
Jarek Poplawski <jarkao2@gmail.com> wrote:
> Stephen Hemminger wrote, On -10.01.-28163 20:59:
>
> > There are several qdisc which only support a single class (sfq, mq, tbf)
> > and the kernel would dereference a null pointer (bind_tcf), if a user
> > attempted to apply a filter one of these classes.
>
>
> mq and tbf can't have this issue because they don't have
> .tcf_chain class method. sfq should support it on purpose
> after this patch:
>
> http://git.kernel.org/?p=linux/kernel/git/torvalds/linux-2.6.git;a=commitdiff;h=7d2681a6ff4f9ab5e48d02550b4c6338f1638998
> and needs tiny fix only.
Probably best to fix both ways. Fix sfq to allow filters to
be chained, and fix API to prevent refuse to allow qdisc to
register with tcf_chain && !bind_tcf
^ permalink raw reply
* Re: [PATCH] net: convert to rcu_dereference_index_check()
From: Paul E. McKenney @ 2010-08-06 21:54 UTC (permalink / raw)
To: David Miller; +Cc: herbert, netdev, linux-kernel, arnd
In-Reply-To: <20100806.144901.184847372.davem@davemloft.net>
On Fri, Aug 06, 2010 at 02:49:01PM -0700, David Miller wrote:
> From: "Paul E. McKenney" <paulmck@linux.vnet.ibm.com>
> Date: Fri, 6 Aug 2010 14:26:07 -0700
>
> > At long last, the following is the final patch blocking acceptance
> > of Arnd's sparse annotations for RCU. It was itself blocked behind
> > the definition of rcu_dereference_index_check(), which made it to
> > mainline only just today.
> >
> > Would you be willing to ack this patch so that I can push it to -tip,
> > along with Arnd's work and a bunch of other stuff?
> ...
> > net: convert to rcu_dereference_index_check()
> >
> > The task_cls_classid() function applies rcu_dereference() to integers,
> > which does not work with the shiny new sparse-based checking in
> > rcu_dereference(). This commit therefore moves to the new RCU API
> > rcu_dereference_index_check().
> >
> > Signed-off-by: Paul E. McKenney <paulmck@linux.vnet.ibm.com>
> > Reviewed-by: Josh Triplett <josh@joshtriplett.org>
>
> You should of course wait for Herbert's ACK too, but for
> the record:
>
> Acked-by: David S. Miller <davem@davemloft.net>
>
> :-)
Thank you, David!!! ;-)
Thanx, Paul
^ permalink raw reply
* Re: [PATCH] net: convert to rcu_dereference_index_check()
From: David Miller @ 2010-08-06 21:49 UTC (permalink / raw)
To: paulmck; +Cc: herbert, netdev, linux-kernel, arnd
In-Reply-To: <20100806212607.GA9410@linux.vnet.ibm.com>
From: "Paul E. McKenney" <paulmck@linux.vnet.ibm.com>
Date: Fri, 6 Aug 2010 14:26:07 -0700
> At long last, the following is the final patch blocking acceptance
> of Arnd's sparse annotations for RCU. It was itself blocked behind
> the definition of rcu_dereference_index_check(), which made it to
> mainline only just today.
>
> Would you be willing to ack this patch so that I can push it to -tip,
> along with Arnd's work and a bunch of other stuff?
...
> net: convert to rcu_dereference_index_check()
>
> The task_cls_classid() function applies rcu_dereference() to integers,
> which does not work with the shiny new sparse-based checking in
> rcu_dereference(). This commit therefore moves to the new RCU API
> rcu_dereference_index_check().
>
> Signed-off-by: Paul E. McKenney <paulmck@linux.vnet.ibm.com>
> Reviewed-by: Josh Triplett <josh@joshtriplett.org>
You should of course wait for Herbert's ACK too, but for
the record:
Acked-by: David S. Miller <davem@davemloft.net>
:-)
^ permalink raw reply
* [PATCH] net: convert to rcu_dereference_index_check()
From: Paul E. McKenney @ 2010-08-06 21:26 UTC (permalink / raw)
To: herbert; +Cc: netdev, linux-kernel, davem, arnd
Hello, Herbert!
At long last, the following is the final patch blocking acceptance
of Arnd's sparse annotations for RCU. It was itself blocked behind
the definition of rcu_dereference_index_check(), which made it to
mainline only just today.
Would you be willing to ack this patch so that I can push it to -tip,
along with Arnd's work and a bunch of other stuff?
Thanx, Paul
------------------------------------------------------------------------
commit 34ea15566530f8e4d0c1571cc8c3554fd91d5809
Author: Paul E. McKenney <paulmck@linux.vnet.ibm.com>
Date: Mon Jun 14 17:06:21 2010 -0700
net: convert to rcu_dereference_index_check()
The task_cls_classid() function applies rcu_dereference() to integers,
which does not work with the shiny new sparse-based checking in
rcu_dereference(). This commit therefore moves to the new RCU API
rcu_dereference_index_check().
Signed-off-by: Paul E. McKenney <paulmck@linux.vnet.ibm.com>
Reviewed-by: Josh Triplett <josh@joshtriplett.org>
cls_cgroup.h | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/include/net/cls_cgroup.h b/include/net/cls_cgroup.h
index 726cc35..dd1fdb8 100644
--- a/include/net/cls_cgroup.h
+++ b/include/net/cls_cgroup.h
@@ -45,7 +45,8 @@ static inline u32 task_cls_classid(struct task_struct *p)
return 0;
rcu_read_lock();
- id = rcu_dereference(net_cls_subsys_id);
+ id = rcu_dereference_index_check(net_cls_subsys_id,
+ rcu_read_lock_held());
if (id >= 0)
classid = container_of(task_subsys_state(p, id),
struct cgroup_cls_state, css)->classid;
^ permalink raw reply related
* Re: [PATCH 1/9] net classifier: dont allow filters on semi-classful qdisc
From: Jarek Poplawski @ 2010-08-06 21:24 UTC (permalink / raw)
To: Stephen Hemminger; +Cc: David Miller, netdev
In-Reply-To: <20100806193558.580890552@vyatta.com>
Stephen Hemminger wrote, On -10.01.-28163 20:59:
> There are several qdisc which only support a single class (sfq, mq, tbf)
> and the kernel would dereference a null pointer (bind_tcf), if a user
> attempted to apply a filter one of these classes.
mq and tbf can't have this issue because they don't have
.tcf_chain class method. sfq should support it on purpose
after this patch:
http://git.kernel.org/?p=linux/kernel/git/torvalds/linux-2.6.git;a=commitdiff;h=7d2681a6ff4f9ab5e48d02550b4c6338f1638998
and needs tiny fix only.
Jarek P.
>
> This patch changes the tcf_bind_filter to return an error in
> these cases.
>
> Signed-off-by: Stephen Hemminger <shemminger@vyatta.com>
>
>
> ---
> This needs to go in net-2.6 and stable.
>
> include/net/pkt_cls.h | 12 +++++++++---
> net/sched/cls_basic.c | 4 +++-
> net/sched/cls_fw.c | 6 ++++--
> net/sched/cls_route.c | 4 +++-
> net/sched/cls_tcindex.c | 4 +++-
> net/sched/cls_u32.c | 4 +++-
> 6 files changed, 25 insertions(+), 9 deletions(-)
>
> --- a/include/net/pkt_cls.h 2010-08-06 11:51:18.903581556 -0700
> +++ b/include/net/pkt_cls.h 2010-08-06 12:20:02.072241508 -0700
> @@ -40,15 +40,21 @@ cls_set_class(struct tcf_proto *tp, unsi
> return old_cl;
> }
>
> -static inline void
> +static inline int
> tcf_bind_filter(struct tcf_proto *tp, struct tcf_result *r, unsigned long base)
> {
> + const struct Qdisc_class_ops *cops = tp->q->ops->cl_ops;
> unsigned long cl;
>
> - cl = tp->q->ops->cl_ops->bind_tcf(tp->q, base, r->classid);
> + if (!cops->bind_tcf)
> + return -EINVAL;
> +
> + cl = cops->bind_tcf(tp->q, base, r->classid);
> cl = cls_set_class(tp, &r->class, cl);
> if (cl)
> - tp->q->ops->cl_ops->unbind_tcf(tp->q, cl);
> + cops->unbind_tcf(tp->q, cl);
> +
> + return 0;
> }
>
> static inline void
> --- a/net/sched/cls_basic.c 2010-08-06 11:51:18.923582342 -0700
> +++ b/net/sched/cls_basic.c 2010-08-06 11:55:13.292553190 -0700
> @@ -153,7 +153,9 @@ static inline int basic_set_parms(struct
>
> if (tb[TCA_BASIC_CLASSID]) {
> f->res.classid = nla_get_u32(tb[TCA_BASIC_CLASSID]);
> - tcf_bind_filter(tp, &f->res, base);
> + err = tcf_bind_filter(tp, &f->res, base);
> + if (err)
> + goto errout;
> }
>
> tcf_exts_change(tp, &f->exts, &e);
> --- a/net/sched/cls_fw.c 2010-08-06 11:51:18.943583126 -0700
> +++ b/net/sched/cls_fw.c 2010-08-06 11:55:39.085476144 -0700
> @@ -206,10 +206,11 @@ fw_change_attrs(struct tcf_proto *tp, st
> if (err < 0)
> return err;
>
> - err = -EINVAL;
> if (tb[TCA_FW_CLASSID]) {
> f->res.classid = nla_get_u32(tb[TCA_FW_CLASSID]);
> - tcf_bind_filter(tp, &f->res, base);
> + err = tcf_bind_filter(tp, &f->res, base);
> + if (err)
> + goto errout;
> }
>
> #ifdef CONFIG_NET_CLS_IND
> @@ -220,6 +221,7 @@ fw_change_attrs(struct tcf_proto *tp, st
> }
> #endif /* CONFIG_NET_CLS_IND */
>
> + err = -EINVAL;
> if (tb[TCA_FW_MASK]) {
> mask = nla_get_u32(tb[TCA_FW_MASK]);
> if (mask != head->mask)
> --- a/net/sched/cls_route.c 2010-08-06 11:51:18.959583757 -0700
> +++ b/net/sched/cls_route.c 2010-08-06 11:55:50.077870498 -0700
> @@ -412,7 +412,9 @@ static int route4_set_parms(struct tcf_p
>
> if (tb[TCA_ROUTE4_CLASSID]) {
> f->res.classid = nla_get_u32(tb[TCA_ROUTE4_CLASSID]);
> - tcf_bind_filter(tp, &f->res, base);
> + err = tcf_bind_filter(tp, &f->res, base);
> + if (err)
> + goto errout;
> }
>
> tcf_exts_change(tp, &f->exts, &e);
> --- a/net/sched/cls_tcindex.c 2010-08-06 11:51:18.999585326 -0700
> +++ b/net/sched/cls_tcindex.c 2010-08-06 11:56:01.486283847 -0700
> @@ -295,7 +295,9 @@ tcindex_set_parms(struct tcf_proto *tp,
>
> if (tb[TCA_TCINDEX_CLASSID]) {
> cr.res.classid = nla_get_u32(tb[TCA_TCINDEX_CLASSID]);
> - tcf_bind_filter(tp, &cr.res, base);
> + err = tcf_bind_filter(tp, &cr.res, base);
> + if (err)
> + goto errout;
> }
>
> tcf_exts_change(tp, &cr.exts, &e);
> --- a/net/sched/cls_u32.c 2010-08-06 11:51:19.019586112 -0700
> +++ b/net/sched/cls_u32.c 2010-08-06 11:56:12.390678703 -0700
> @@ -528,7 +528,9 @@ static int u32_set_parms(struct tcf_prot
> }
> if (tb[TCA_U32_CLASSID]) {
> n->res.classid = nla_get_u32(tb[TCA_U32_CLASSID]);
> - tcf_bind_filter(tp, &n->res, base);
> + err = tcf_bind_filter(tp, &n->res, base);
> + if (err)
> + goto errout;
> }
>
> #ifdef CONFIG_NET_CLS_IND
>
>
> --
> 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: pull request: wireless-2.6 2010-08-06
From: David Miller @ 2010-08-06 20:46 UTC (permalink / raw)
To: linville-2XuSBdqkA4R54TAoqtyWWQ
Cc: linux-wireless-u79uwXL29TY76Z2rM5mHXA,
netdev-u79uwXL29TY76Z2rM5mHXA,
linux-kernel-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <20100806191443.GB2753-2XuSBdqkA4R54TAoqtyWWQ@public.gmane.org>
From: "John W. Linville" <linville-2XuSBdqkA4R54TAoqtyWWQ@public.gmane.org>
Date: Fri, 6 Aug 2010 15:14:43 -0400
> Here is the first round of fixes intended for 2.6.36. It is bigger than
> I would prefer, but I hope you will still find it acceptable, especially
> as we are still so early in the cycle.
Pulled, thanks John.
As you know I'd prefer if things settle down from this point
forward, thanks :-)
--
To unsubscribe from this list: send the line "unsubscribe linux-wireless" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* [PATCH 10/18] isdn: hisax: call disable_pci_device() if pci_probe() failed
From: Kulikov Vasiliy @ 2010-08-06 20:02 UTC (permalink / raw)
To: kernel-janitors
Cc: Karsten Keil, Jesse Barnes, Tilman Schmidt, Tejun Heo, netdev,
linux-kernel
Driver should call disable_pci_device() if it returns from pci_probe()
with error.
Signed-off-by: Kulikov Vasiliy <segooon@gmail.com>
---
drivers/isdn/hisax/avm_pci.c | 2 ++
drivers/isdn/hisax/elsa.c | 5 +++++
drivers/isdn/hisax/nj_u.c | 2 ++
drivers/isdn/hisax/sedlbauer.c | 3 +++
drivers/isdn/hisax/w6692.c | 6 +++++-
5 files changed, 17 insertions(+), 1 deletions(-)
diff --git a/drivers/isdn/hisax/avm_pci.c b/drivers/isdn/hisax/avm_pci.c
index fcf4ed1..c503d7f 100644
--- a/drivers/isdn/hisax/avm_pci.c
+++ b/drivers/isdn/hisax/avm_pci.c
@@ -845,12 +845,14 @@ static int __devinit avm_pci_setup(struct IsdnCardState *cs)
cs->irq = dev_avm->irq;
if (!cs->irq) {
printk(KERN_ERR "FritzPCI: No IRQ for PCI card found\n");
+ pci_disable_device(dev_avm);
return(0);
}
cs->hw.avm.cfg_reg = pci_resource_start(dev_avm, 1);
if (!cs->hw.avm.cfg_reg) {
printk(KERN_ERR "FritzPCI: No IO-Adr for PCI card found\n");
+ pci_disable_device(dev_avm);
return(0);
}
diff --git a/drivers/isdn/hisax/elsa.c b/drivers/isdn/hisax/elsa.c
index 5d9d338..452429a 100644
--- a/drivers/isdn/hisax/elsa.c
+++ b/drivers/isdn/hisax/elsa.c
@@ -1034,12 +1034,14 @@ static int __devinit
setup_elsa_pci(struct IsdnCard *card)
{
struct IsdnCardState *cs = card->cs;
+ struct pci_dev *pdev;
cs->subtyp = 0;
if ((dev_qs1000 = hisax_find_pci_device(PCI_VENDOR_ID_ELSA,
PCI_DEVICE_ID_ELSA_MICROLINK, dev_qs1000))) {
if (pci_enable_device(dev_qs1000))
return(0);
+ pdev = dev_qs1000;
cs->subtyp = ELSA_QS1000PCI;
cs->irq = dev_qs1000->irq;
cs->hw.elsa.cfg = pci_resource_start(dev_qs1000, 1);
@@ -1048,6 +1050,7 @@ setup_elsa_pci(struct IsdnCard *card)
PCI_DEVICE_ID_ELSA_QS3000, dev_qs3000))) {
if (pci_enable_device(dev_qs3000))
return(0);
+ pdev = dev_qs3000;
cs->subtyp = ELSA_QS3000PCI;
cs->irq = dev_qs3000->irq;
cs->hw.elsa.cfg = pci_resource_start(dev_qs3000, 1);
@@ -1058,11 +1061,13 @@ setup_elsa_pci(struct IsdnCard *card)
}
if (!cs->irq) {
printk(KERN_WARNING "Elsa: No IRQ for PCI card found\n");
+ pci_disable_device(pdev);
return(0);
}
if (!(cs->hw.elsa.base && cs->hw.elsa.cfg)) {
printk(KERN_WARNING "Elsa: No IO-Adr for PCI card found\n");
+ pci_disable_device(pdev);
return(0);
}
if ((cs->hw.elsa.cfg & 0xff) || (cs->hw.elsa.base & 0xf)) {
diff --git a/drivers/isdn/hisax/nj_u.c b/drivers/isdn/hisax/nj_u.c
index 095e974..bc5afbe 100644
--- a/drivers/isdn/hisax/nj_u.c
+++ b/drivers/isdn/hisax/nj_u.c
@@ -137,11 +137,13 @@ static int __devinit nju_pci_probe(struct pci_dev *dev_netjet,
cs->irq = dev_netjet->irq;
if (!cs->irq) {
printk(KERN_WARNING "NETspider-U: No IRQ for PCI card found\n");
+ pci_disable_device(dev_netjet);
return(0);
}
cs->hw.njet.base = pci_resource_start(dev_netjet, 0);
if (!cs->hw.njet.base) {
printk(KERN_WARNING "NETspider-U: No IO-Adr for PCI card found\n");
+ pci_disable_device(dev_netjet);
return(0);
}
diff --git a/drivers/isdn/hisax/sedlbauer.c b/drivers/isdn/hisax/sedlbauer.c
index 69dfc8d..dc9864b 100644
--- a/drivers/isdn/hisax/sedlbauer.c
+++ b/drivers/isdn/hisax/sedlbauer.c
@@ -614,6 +614,7 @@ setup_sedlbauer_pci(struct IsdnCard *card)
cs->irq = dev_sedl->irq;
if (!cs->irq) {
printk(KERN_WARNING "Sedlbauer: No IRQ for PCI card found\n");
+ pci_disable_device(dev_sedl);
return(0);
}
cs->hw.sedl.cfg_reg = pci_resource_start(dev_sedl, 0);
@@ -631,6 +632,7 @@ setup_sedlbauer_pci(struct IsdnCard *card)
cs->hw.sedl.cfg_reg);
if (sub_id != PCI_SUB_ID_SEDLBAUER) {
printk(KERN_ERR "Sedlbauer: unknown sub id %#x\n", sub_id);
+ pci_disable_device(dev_sedl);
return(0);
}
if (sub_vendor_id == PCI_SUBVENDOR_SPEEDFAX_PYRAMID) {
@@ -648,6 +650,7 @@ setup_sedlbauer_pci(struct IsdnCard *card)
} else {
printk(KERN_ERR "Sedlbauer: unknown sub vendor id %#x\n",
sub_vendor_id);
+ pci_disable_device(dev_sedl);
return(0);
}
diff --git a/drivers/isdn/hisax/w6692.c b/drivers/isdn/hisax/w6692.c
index e2cfb6f..4221cab 100644
--- a/drivers/isdn/hisax/w6692.c
+++ b/drivers/isdn/hisax/w6692.c
@@ -1012,8 +1012,10 @@ setup_w6692(struct IsdnCard *card)
id_list[id_idx].device_id,
dev_w6692);
if (dev_w6692) {
- if (pci_enable_device(dev_w6692))
+ if (pci_enable_device(dev_w6692)) {
+ dev_w6692 = NULL;
continue;
+ }
cs->subtyp = id_idx;
break;
}
@@ -1040,10 +1042,12 @@ setup_w6692(struct IsdnCard *card)
cs->irq = pci_irq;
if (!cs->irq) {
printk(KERN_WARNING "W6692: No IRQ for PCI card found\n");
+ pci_disable_device(dev_w6692);
return (0);
}
if (!pci_ioaddr) {
printk(KERN_WARNING "W6692: NO I/O Base Address found\n");
+ pci_disable_device(dev_w6692);
return (0);
}
cs->hw.w6692.iobase = pci_ioaddr;
--
1.7.0.4
^ permalink raw reply related
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