* Re: [RFC v2 1/2] PCI-Express Non-Transparent Bridge Support
From: Bjorn Helgaas @ 2012-07-31 17:03 UTC (permalink / raw)
To: chetan loke; +Cc: Jon Mason, linux-kernel, netdev, linux-pci, Dave Jiang
In-Reply-To: <CAAsGZS5kOyQ5FQ7Jg3HxOmmPNYS+F6XcB-_3tm=rcgDV7wAvRQ@mail.gmail.com>
On Tue, Jul 31, 2012 at 10:02 AM, chetan loke <loke.chetan@gmail.com> wrote:
> On Tue, Jul 31, 2012 at 9:45 AM, Bjorn Helgaas <bhelgaas@google.com> wrote:
>> On Mon, Jul 30, 2012 at 12:15 PM, Jon Mason <jon.mason@intel.com> wrote:
>>>
>>> I've tried to make it all generic enough that non-Intel NTBs should plug in with
>>> minimal changes to ntb_hw.c. If their design is too divergent, then a slight
>>> redesign of ntb_hw.c might be necessary. But from what I've seen of other
>>> designs on the internet, they appear to be extremely similar. The transport and
>>> client drivers were written with the hardware abstracted away as much as
>>> possible to prevent the need to modify it for different hardware. If there is
>>> anything which is Intel hardware specific, I'd be happy to change it to make it
>>> more generic.
>>
>> That makes sense from a technical point of view, but I think it's
>> going to cause maintenance issues. For example, assume PLX NTB
>> support is added. Will PLX be happy about having to convince you to
>> accept changes? Will Intel be happy about having to release a new
>
> Do you mean convince Intel? Well, if we think of ntb as the class
> driver then the onus is on the community to vet the changes and NOT
> intel.
No, I said convince "you," meaning *Jon*. Intel doesn't apply
patches. "The community" doesn't apply patches. Jon applies patches.
This has nothing to do with the fact that Jon is employed by Intel.
The point is that when you have multiple organizations involved, they
have different goals, markets, customers, and schedules. If one
module contains both Intel-specific and PLX-specific things, that's a
place where these organizational differences may cause issues,
regardless of who is applying the patches.
Obviously it's not a problem now, and maybe it will never be. But I
think there's a possibility. Since I have no direct interest in any
of these devices, I'm only raising that possibility, not trying to
force any particular direction.
^ permalink raw reply
* Re: [RFC v2 1/2] PCI-Express Non-Transparent Bridge Support
From: Jon Mason @ 2012-07-31 17:10 UTC (permalink / raw)
To: Bjorn Helgaas; +Cc: linux-kernel, netdev, linux-pci, Dave Jiang
In-Reply-To: <CAErSpo6gjMf4C3RMn5YLNg5JtJ8CjHvTw1iqsKPa6YuRqPe90g@mail.gmail.com>
On Tue, Jul 31, 2012 at 07:45:29AM -0600, Bjorn Helgaas wrote:
> On Mon, Jul 30, 2012 at 12:15 PM, Jon Mason <jon.mason@intel.com> wrote:
> > On Mon, Jul 30, 2012 at 10:50:13AM -0600, Bjorn Helgaas wrote:
> >> On Sun, Jul 29, 2012 at 6:26 PM, Jon Mason <jon.mason@intel.com> wrote:
> >> > A PCI-Express non-transparent bridge (NTB) is a point-to-point PCIe bus
> >> > connecting 2 systems, providing electrical isolation between the two subsystems.
> >> > A non-transparent bridge is functionally similar to a transparent bridge except
> >> > that both sides of the bridge have their own independent address domains. The
> >> > host on one side of the bridge will not have the visibility of the complete
> >> > memory or I/O space on the other side of the bridge. To communicate across the
> >> > non-transparent bridge, each NTB endpoint has one (or more) apertures exposed to
> >> > the local system. Writes to these apertures are mirrored to memory on the
> >> > remote system. Communications can also occur through the use of doorbell
> >> > registers that initiate interrupts to the alternate domain, and scratch-pad
> >> > registers accessible from both sides.
> >> >
> >> > The NTB device driver is needed to configure these memory windows, doorbell, and
> >> > scratch-pad registers as well as use them in such a way as they can be turned
> >> > into a viable communication channel to the remote system. ntb_hw.[ch]
> >> > determines the usage model (NTB to NTB or NTB to Root Port) and abstracts away
> >> > the underlying hardware to provide access and a common interface to the doorbell
> >> > registers, scratch pads, and memory windows. These hardware interfaces are
> >> > exported so that other, non-mainlined kernel drivers can access these.
> >> > ntb_transport.[ch] also uses the exported interfaces in ntb_hw.[ch] to setup a
> >> > communication channel(s) and provide a reliable way of transferring data from
> >> > one side to the other, which it then exports so that "client" drivers can access
> >> > them. These client drivers are used to provide a standard kernel interface
> >> > (i.e., Ethernet device) to NTB, such that Linux can transfer data from one
> >> > system to the other in a standard way.
> >> >
> >> > Signed-off-by: Jon Mason <jon.mason@intel.com>
> >> > ---
> >> > MAINTAINERS | 6 +
> >> > drivers/Kconfig | 2 +
> >> > drivers/Makefile | 1 +
> >> > drivers/ntb/Kconfig | 13 +
> >> > drivers/ntb/Makefile | 3 +
> >> > drivers/ntb/ntb_hw.c | 1178 ++++++++++++++++++++++++++++++++++++
> >> > drivers/ntb/ntb_hw.h | 206 +++++++
> >> > drivers/ntb/ntb_regs.h | 150 +++++
> >> > drivers/ntb/ntb_transport.c | 1387 +++++++++++++++++++++++++++++++++++++++++++
> >> > include/linux/ntb.h | 92 +++
> >>
> >> Where will drivers for non-Intel NTBs fit in this hierarchy? It seems
> >> a bit presumptuous to claim the generic "ntb" names just for Intel
> >> devices.
> >
> > I've tried to make it all generic enough that non-Intel NTBs should plug in with
> > minimal changes to ntb_hw.c. If their design is too divergent, then a slight
> > redesign of ntb_hw.c might be necessary. But from what I've seen of other
> > designs on the internet, they appear to be extremely similar. The transport and
> > client drivers were written with the hardware abstracted away as much as
> > possible to prevent the need to modify it for different hardware. If there is
> > anything which is Intel hardware specific, I'd be happy to change it to make it
> > more generic.
>
> That makes sense from a technical point of view, but I think it's
> going to cause maintenance issues. For example, assume PLX NTB
> support is added. Will PLX be happy about having to convince you to
> accept changes? Will Intel be happy about having to release a new
> driver for their hardware just to incorporate a PLX bug fix? Will
> users of PLX hardware accept a new driver release that only benefits
> Intel users?
Firstly, I don't believe this is an issue. The amount of hardware
specific code is small (<150 loc), and the design of the PLX hardware
appears to be the same as Intel (from what I've read on the internet).
Assuming Intel is me, I am always happy to do whatever I can to help
PLX or any other NTB vendor. I am willing to make internal releases
(which I am assuming is the issue you are referring to) for any change
to the driver.
If necessary, refactoring the driver shouldn't be a significant issue.
However, I would prefer to do it if/when another NTB driver wants to
be added. PLX (and those who are using their NTB hardware) hasn't
pushed an NTB driver yet, and may never (as their design docs on the
web are from '05). Why complicate the driver for them if they never
come?
Thanks,
Jon
^ permalink raw reply
* Re: [RFC v2 1/2] PCI-Express Non-Transparent Bridge Support
From: Jon Mason @ 2012-07-31 17:27 UTC (permalink / raw)
To: chetan loke; +Cc: Bjorn Helgaas, linux-kernel, netdev, linux-pci, Dave Jiang
In-Reply-To: <CAAsGZS5kOyQ5FQ7Jg3HxOmmPNYS+F6XcB-_3tm=rcgDV7wAvRQ@mail.gmail.com>
On Tue, Jul 31, 2012 at 12:02:20PM -0400, chetan loke wrote:
> On Tue, Jul 31, 2012 at 9:45 AM, Bjorn Helgaas <bhelgaas@google.com> wrote:
> > On Mon, Jul 30, 2012 at 12:15 PM, Jon Mason <jon.mason@intel.com> wrote:
> >>
> >> I've tried to make it all generic enough that non-Intel NTBs should plug in with
> >> minimal changes to ntb_hw.c. If their design is too divergent, then a slight
> >> redesign of ntb_hw.c might be necessary. But from what I've seen of other
> >> designs on the internet, they appear to be extremely similar. The transport and
> >> client drivers were written with the hardware abstracted away as much as
> >> possible to prevent the need to modify it for different hardware. If there is
> >> anything which is Intel hardware specific, I'd be happy to change it to make it
> >> more generic.
> >
> > That makes sense from a technical point of view, but I think it's
> > going to cause maintenance issues. For example, assume PLX NTB
> > support is added. Will PLX be happy about having to convince you to
> > accept changes? Will Intel be happy about having to release a new
>
> Do you mean convince Intel? Well, if we think of ntb as the class
> driver then the onus is on the community to vet the changes and NOT
> intel.
> And since this is the first NTB part for which the support is
> introduced the class driver design could be a moving target. As
> someone else mentioned, the async/sync tx-dma is another hook that
> could change the class driver's design.
>
>
> > driver for their hardware just to incorporate a PLX bug fix? Will
> > users of PLX hardware accept a new driver release that only benefits
> > Intel users?
>
> May be till the class driver is stable, the client(intel/PLX) drivers
> might have to be modified. This is a cue for other vendors to
> chime-in and review this design?
> Just thinking if this could sit in staging for some time(so that
> others get a chance to review/suggest changes as well) and then get
> promoted out of staging.
I don't see the benefit of having the driver in staging. Any vendors
who would notice the ntb driver in staging would be sitting on these
mailing lists and hopefully have planety of comments on the design.
Stashing the driver in staging while waiting for these comments (which
may never come) doesn't seem the best course of action.
Thanks,
Jon
>
> Chetan Loke
^ permalink raw reply
* Re: [PATCH 0/7] Deconstruct struct fib_result
From: Alexander Duyck @ 2012-07-31 17:27 UTC (permalink / raw)
To: David Miller; +Cc: eric.dumazet, netdev
In-Reply-To: <20120727.211814.1628871775065221675.davem@davemloft.net>
On Fri, Jul 27, 2012 at 9:18 PM, David Miller <davem@davemloft.net> wrote:
>
> This patch set tries to move towards reducing struct fib_result down
> to it's absolute minimum.
>
> The eventual idea is to make it so that fib_lookup() simply
> returns a "struct fib_nh *" and pointer encoded errnos, instead
> of writing into a complicated structure as the return value on
> the stack as is done now.
>
> Signed-off-by: David S. Miller <davem@davemloft.net>
I tested these patches most of yesterday and I haven't seen any
issues. Then again I haven't seen much of an improvement either.
Tested-by: Alexander Duyck <alexander.h.duyck@intel.com>
^ permalink raw reply
* inquiry
From: roboth roli company @ 2012-07-31 9:31 UTC (permalink / raw)
i am robothroli, Purchase manager from roli Merchant Ltd. We are
Import/export Company based in taiwan. We are interested in purchasing
your product and I would like to make an inquiry. Please inform me on:
Sample availability and price
Minimum order quantity
FOB Prices
Sincerely
Purchase Manager
robothroli
^ permalink raw reply
* Re: CPU: 0 Not tainted (3.1.9+ #1) when ifconfig rose0 down
From: folkert @ 2012-07-31 17:56 UTC (permalink / raw)
To: Bernard Pidoux; +Cc: linux-hams, Linux Netdev List
In-Reply-To: <5017E786.5000102@free.fr>
> I observe systematically a kernel panic when I try to shutdown rose0
> device using ifconfig rose0 down
That's funny: I always get a kernel panic(!) when doing an ifconfig
rose0 up!
Folkert van Heusden
--
Nagios user? Check out CoffeeSaint - the versatile Nagios status
viewer! http://www.vanheusden.com/java/CoffeeSaint/
----------------------------------------------------------------------
Phone: +31-6-41278122, PGP-key: 1F28D8AE, www.vanheusden.com
^ permalink raw reply
* Re: CPU: 0 Not tainted (3.1.9+ #1) when ifconfig rose0 down
From: folkert @ 2012-07-31 17:58 UTC (permalink / raw)
To: Bernard Pidoux; +Cc: linux-hams, Linux Netdev List
In-Reply-To: <5017F96B.1060701@free.fr>
> Here is a complementary observation.
> Trying to remove rose module with rmmod rose did not create any
> kernel panic.
A while ago I wrote a script that at random removed and inserted modules
into the system and that gave surprising results: not only crashing
system, also corrupt filesystems etc. Filed a bug-report and I was told
that removing a module might work but there's no guarantee.
Folkert van Heusden
--
Feeling generous? -> http://www.vanheusden.com/wishlist.php
----------------------------------------------------------------------
Phone: +31-6-41278122, PGP-key: 1F28D8AE, www.vanheusden.com
^ permalink raw reply
* Re: [RFC v2 1/2] PCI-Express Non-Transparent Bridge Support
From: chetan loke @ 2012-07-31 18:02 UTC (permalink / raw)
To: Jon Mason; +Cc: Bjorn Helgaas, linux-kernel, netdev, linux-pci, Dave Jiang
In-Reply-To: <20120731172709.GB14080@jonmason-lab>
On Tue, Jul 31, 2012 at 1:27 PM, Jon Mason <jon.mason@intel.com> wrote:
>
> I don't see the benefit of having the driver in staging. Any vendors
> who would notice the ntb driver in staging would be sitting on these
> mailing lists and hopefully have planety of comments on the design.
> Stashing the driver in staging while waiting for these comments (which
> may never come) doesn't seem the best course of action.
>
I thought that since others are talking about it then may be there is
some WIP code for foo-NTB. Seems like that's not the case. So no need
to stage.
Correct me if I'm wrong but wouldn't apps just open a socket and route
data via ntb_vir_eth_dev? So I don't see an ABI breakage issue and
hence nothing would prevent us from changing the kernel parts(for
accommodating some foo-NTB part) in future.
It may not be a bad idea to prefix intel-specific(if any)
ntb_structs/variables/logic with the 'intc'(Intel ticker or pick your
string) keyword.
Chetan Loke
^ permalink raw reply
* Re: [PATCH v2 3/6] mv643xx.c: Add basic device tree support.
From: Arnd Bergmann @ 2012-07-31 18:23 UTC (permalink / raw)
To: Ian Molton; +Cc: linux-arm-kernel, andrew, thomas.petazzoni, ben.dooks, netdev
In-Reply-To: <1343749529-17571-4-git-send-email-ian.molton@codethink.co.uk>
On Tuesday 31 July 2012, Ian Molton wrote:
> @@ -33,6 +34,10 @@ struct of_dev_auxdata kirkwood_auxdata_lookup[] __initdata = {
> OF_DEV_AUXDATA("marvell,orion-wdt", 0xf1020300, "orion_wdt", NULL),
> OF_DEV_AUXDATA("marvell,orion-sata", 0xf1080000, "sata_mv.0", NULL),
> OF_DEV_AUXDATA("marvell,orion-nand", 0xf4000000, "orion_nand", NULL),
> + OF_DEV_AUXDATA("marvell,mv643xx", 0xf1072000, MV643XX_ETH_NAME ".0",
> + NULL),
> + OF_DEV_AUXDATA("marvell,mv643xx", 0xf1076000, MV643XX_ETH_NAME ".1",
> + NULL),
> {},
> };
Please don't do string concatenation like this, it just makes it harder to grep for the
strings.
> @@ -2654,15 +2677,22 @@ static int mv643xx_eth_shared_probe(struct platform_device *pdev)
> /*
> * Check whether the error interrupt is hooked up.
> */
> - res = platform_get_resource(pdev, IORESOURCE_IRQ, 0);
> - if (res != NULL) {
> + if (pdev->dev.of_node) {
> + irq = irq_of_parse_and_map(pdev->dev.of_node, 0);
> + } else {
> + res = platform_get_resource(pdev, IORESOURCE_IRQ, 0);
> + if (res)
> + irq = res->start;
> + }
> +
Why is this necessary? In theory, the old code should be the same as the new one,
unless something goes wrong in the domain registration.
Arnd
^ permalink raw reply
* wlcore: might_sleep operation in interrupt handler
From: Denis Yefremov @ 2012-07-31 18:28 UTC (permalink / raw)
To: Luciano Coelho
Cc: John W. Linville, Eliad Peller, Arik Nemtsov, Eyal Shapira,
linux-wireless, netdev, linux-kernel, ldv-project
Interrupt handler wlcore_irq in wlcore/main.c file
invokes mutex_lock that is might sleep operation.
Found by Linux Driver Verification project (linuxtesting.org)
within my participation in the GSoC 2012 program.
drivers/net/wireless/ti/wlcore/main.c
static irqreturn_t wlcore_irq(int irq, void *cookie)
{
int ret;
unsigned long flags;
struct wl1271 *wl = cookie;
/* TX might be handled here, avoid redundant work */
set_bit(WL1271_FLAG_TX_PENDING, &wl->flags);
cancel_work_sync(&wl->tx_work);
mutex_lock(&wl->mutex);
...
}
int __devinit wlcore_probe(struct wl1271 *wl, struct platform_device *pdev)
{
...
ret = request_threaded_irq(wl->irq, wl12xx_hardirq, wlcore_irq,
irqflags,
pdev->name, wl);
...
}
^ permalink raw reply
* cpu usage on blocking network call
From: Robert Engels @ 2012-07-31 19:14 UTC (permalink / raw)
To: netdev
I have a strange problem, and was hoping the developers could provide
some insight.
I have tested with ubuntu 12.04, with the 3.2.0-23 lowlatency kernel.
I also tested with a 3.0.31 vanilla realtime kernel on a different
machine.
I open a blocking socket (and connect to a remote client), and start
two threads, a reader and a writer.
I pump LOTS of data down the writer, but do not receive any data on
the reader (by design).
The reader thread is reporting some CPU usage (not a lot, about 10% of
the writer cpu). I determine this using "top", and other profiling
tools.
I also run strace on the thread, and it shows that it is blocked in
read() call, and that is doesn't return.
Why/How is the reader thread consuming CPU? Poorly written driver?
Kernel bug? Or is this expected behavior or the kernel?
I am more than willing to spend more time tracking it down in the
kernel, but I'd like some direction.
--
Robert Engels
OptionsCity Software Inc.
Chicago, IL
^ permalink raw reply
* Re: [PATCH v2 3/6] mv643xx.c: Add basic device tree support.
From: Andrew Lunn @ 2012-07-31 19:24 UTC (permalink / raw)
To: Arnd Bergmann
Cc: Ian Molton, linux-arm-kernel, andrew, thomas.petazzoni, ben.dooks,
netdev
In-Reply-To: <201207311823.54857.arnd@arndb.de>
On Tue, Jul 31, 2012 at 06:23:54PM +0000, Arnd Bergmann wrote:
> On Tuesday 31 July 2012, Ian Molton wrote:
> > @@ -33,6 +34,10 @@ struct of_dev_auxdata kirkwood_auxdata_lookup[] __initdata = {
> > OF_DEV_AUXDATA("marvell,orion-wdt", 0xf1020300, "orion_wdt", NULL),
> > OF_DEV_AUXDATA("marvell,orion-sata", 0xf1080000, "sata_mv.0", NULL),
> > OF_DEV_AUXDATA("marvell,orion-nand", 0xf4000000, "orion_nand", NULL),
> > + OF_DEV_AUXDATA("marvell,mv643xx", 0xf1072000, MV643XX_ETH_NAME ".0",
> > + NULL),
> > + OF_DEV_AUXDATA("marvell,mv643xx", 0xf1076000, MV643XX_ETH_NAME ".1",
> > + NULL),
> > {},
> > };
>
> Please don't do string concatenation like this, it just makes it harder to grep for the
> strings.
Hi Arnd
This pattern is used in other places, e.g. when creating the
clocks. The macro MV643XX_ETH_NAME is also used when creating the
platform data, and in the driver itself.
Are you suggested we replace this with "mv643xx_eth.0"? Or is adding
the macro
#define MV643XX_ETH_NAME0 "mv643xx_eth.0"
in include/linux/mv643xx_eth.h O.K, since grep will find it, and
induce the grep'er to perform a second grep on the macro?
Andrew
^ permalink raw reply
* Re: [PATCH v2] ipv4: percpu nh_rth_output cache
From: Alexander Duyck @ 2012-07-31 20:10 UTC (permalink / raw)
To: Eric Dumazet; +Cc: David Miller, netdev
In-Reply-To: <1343749530.21269.336.camel@edumazet-glaptop>
On 07/31/2012 08:45 AM, Eric Dumazet wrote:
> From: Eric Dumazet <edumazet@google.com>
>
> Input path is mostly run under RCU and doesnt touch dst refcnt
>
> But output path on forwarding or UDP workloads hits
> badly dst refcount, and we have lot of false sharing, for example
> in ipv4_mtu() when reading rt->rt_pmtu
>
> Using a percpu cache for nh_rth_output gives a nice performance
> increase at a small cost.
>
> 24 udpflood test on my 24 cpu machine (dummy0 output device)
> (each process sends 1.000.000 udp frames, 24 processes are started)
>
> before : 5.24 s
> after : 2.06 s
> For reference, time on linux-3.5 : 6.60 s
>
> Signed-off-by: Eric Dumazet <edumazet@google.com>
> ---
> v2: use __this_cpu_ptr() and slighly better annotations to avoid
> ugly casts
>
> On top on previous "ipv4: Restore old dst_free() behavior" patch
>
> We probably can remove all paddings in struct dst_entry
>
I've done some quick testing and it looks like it has little to no
effect on routing performance in my system, but for UDP workloads it is
making a huge difference. I just ran a simple test with 16 sessions of
netperf all sending UDP small packets. Without your patch it runs at
just over 2.7Mpps, with your patch it is runs at over 10.5Mpps.
Tested-by: Alexander Duyck <alexander.h.duyck@intel.com>
^ permalink raw reply
* Re: wlcore: might_sleep operation in interrupt handler
From: Johannes Berg @ 2012-07-31 20:14 UTC (permalink / raw)
To: Denis Yefremov
Cc: Luciano Coelho, John W. Linville, Eliad Peller, Arik Nemtsov,
Eyal Shapira, linux-wireless, netdev, linux-kernel, ldv-project
In-Reply-To: <CADKZ3FKDw78ac9JPXJuw0s7Mqg1RrVfonbjXZ6zHmtwF2i5HoQ@mail.gmail.com>
On Tue, 2012-07-31 at 22:28 +0400, Denis Yefremov wrote:
> Interrupt handler wlcore_irq in wlcore/main.c file
> invokes mutex_lock that is might sleep operation.
> int __devinit wlcore_probe(struct wl1271 *wl, struct platform_device *pdev)
> {
> ...
> ret = request_threaded_irq(wl->irq, wl12xx_hardirq, wlcore_irq,
> irqflags,
> pdev->name, wl);
Yes, umm, why do you think they're using a *threaded* IRQ? :-)
johannes
^ permalink raw reply
* Re: [PATCH v2 3/6] mv643xx.c: Add basic device tree support.
From: Arnd Bergmann @ 2012-07-31 20:18 UTC (permalink / raw)
To: Andrew Lunn
Cc: Ian Molton, linux-arm-kernel, thomas.petazzoni, ben.dooks, netdev
In-Reply-To: <20120731192458.GE28852@lunn.ch>
On Tuesday 31 July 2012, Andrew Lunn wrote:
> This pattern is used in other places, e.g. when creating the
> clocks. The macro MV643XX_ETH_NAME is also used when creating the
> platform data, and in the driver itself.
>
> Are you suggested we replace this with "mv643xx_eth.0"? Or is adding
> the macro
>
> #define MV643XX_ETH_NAME0 "mv643xx_eth.0"
>
> in include/linux/mv643xx_eth.h O.K, since grep will find it, and
> induce the grep'er to perform a second grep on the macro?
I meant the former. Just remove the macro entirely, or at least
don't add new users.
Arnd
^ permalink raw reply
* Re: [PATCH] igb: correct hardware type (i210/i211) check in igb_loopback_test()
From: Jesper Juhl @ 2012-07-31 20:23 UTC (permalink / raw)
To: Wyborny, Carolyn, David S. Miller
Cc: e1000-devel@lists.sourceforge.net, netdev@vger.kernel.org,
Rick Jones, Allan, Bruce W, linux-kernel@vger.kernel.org,
Brandeburg, Jesse, Ronciak, John
In-Reply-To: <9BBC4E0CF881AA4299206E2E1412B62630D64753@ORSMSX102.amr.corp.intel.com>
On Wed, 25 Jul 2012, Wyborny, Carolyn wrote:
>
> > -----Original Message-----
> > From: Jesper Juhl [mailto:jj@chaosbits.net]
> > Sent: Wednesday, July 25, 2012 12:06 PM
> > To: linux-kernel@vger.kernel.org
> > Cc: netdev@vger.kernel.org; e1000-devel@lists.sourceforge.net; Wyborny, Carolyn; Pieper, Jeffrey E; Kirsher, Jeffrey T; Rick Jones; Ronciak, John; Brandeburg, Jesse; Allan, Bruce W; Skidmore, Donald C; Rose, Gregory V; Waskiewicz Jr, Peter P; Duyck, Alexander H; David S. Miller
> > Subject: [PATCH] igb: correct hardware type (i210/i211) check in igb_loopback_test()
> >
> > In the original code
> > ...
> > if ((adapter->hw.mac.type == e1000_i210)
> > || (adapter->hw.mac.type == e1000_i210)) { ...
> > the second check of 'adapter->hw.mac.type' is pointless since it tests for the exact same value as the first.
> >
> > After reading through a few other parts of the driver I believe that the second check was actually intended to check for 'e1000_i211'
> > rather than 'e1000_i210', but I admit that I'm not certain so someone with more knowledge about this driver should ACK the patch before it gets merged.
> >
> > Unfortunately I have no hardware to actually test this on, so it is compile tested only.
> >
> > Signed-off-by: Jesper Juhl <jj@chaosbits.net>
> > ---
> > drivers/net/ethernet/intel/igb/igb_ethtool.c | 2 +-
> > 1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/drivers/net/ethernet/intel/igb/igb_ethtool.c b/drivers/net/ethernet/intel/igb/igb_ethtool.c
> > index a19c84c..ad489b7 100644
> > --- a/drivers/net/ethernet/intel/igb/igb_ethtool.c
> > +++ b/drivers/net/ethernet/intel/igb/igb_ethtool.c
> > @@ -1783,7 +1783,7 @@ static int igb_loopback_test(struct igb_adapter *adapter, u64 *data)
> > goto out;
> > }
> > if ((adapter->hw.mac.type == e1000_i210)
> > - || (adapter->hw.mac.type == e1000_i210)) {
> > + || (adapter->hw.mac.type == e1000_i211)) {
> > dev_err(&adapter->pdev->dev,
> > "Loopback test not supported "
> > "on this part at this time.\n");
> > --
> > 1.7.11.3
> >
>
> ACK.
>
> Good catch. Thanks Jesper!
>
> Carolyn
>
> Carolyn Wyborny
> Linux Development
> LAN Access Division
> Intel Corporation
>
Thank you for the ack Carolyn.
David: will you take this in the networking tree?
--
Jesper Juhl <jj@chaosbits.net> http://www.chaosbits.net/
Don't top-post http://www.catb.org/jargon/html/T/top-post.html
Plain text mails only, please.
------------------------------------------------------------------------------
Live Security Virtual Conference
Exclusive live event will cover all the ways today's security and
threat landscape has changed and how IT managers can respond. Discussions
will include endpoint security, mobile security and the latest in malware
threats. http://www.accelacomm.com/jaw/sfrnl04242012/114/50122263/
_______________________________________________
E1000-devel mailing list
E1000-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/e1000-devel
To learn more about Intel® Ethernet, visit http://communities.intel.com/community/wired
^ permalink raw reply
* Re: wlcore: might_sleep operation in interrupt handler
From: Denis Yefremov @ 2012-07-31 20:48 UTC (permalink / raw)
To: Johannes Berg
Cc: Luciano Coelho, John W. Linville, Eliad Peller, Arik Nemtsov,
Eyal Shapira, linux-wireless-u79uwXL29TY76Z2rM5mHXA,
netdev-u79uwXL29TY76Z2rM5mHXA,
linux-kernel-u79uwXL29TY76Z2rM5mHXA,
ldv-project-ufN2psIa012HXe+LvDLADg
In-Reply-To: <1343765698.4474.10.camel-8upI4CBIZJIJvtFkdXX2HixXY32XiHfO@public.gmane.org>
More precise (IRQF_ONESHOT flag may be used):
int __devinit wlcore_probe(struct wl1271 *wl, struct platform_device *pdev)
{
...
if (wl->platform_quirks & WL12XX_PLATFORM_QUIRK_EDGE_IRQ)
irqflags = IRQF_TRIGGER_RISING;
else
irqflags = IRQF_TRIGGER_HIGH | IRQF_ONESHOT;
ret = request_threaded_irq(wl->irq, wl12xx_hardirq, wlcore_irq,
irqflags,
pdev->name, wl);
2012/8/1 Johannes Berg <johannes-cdvu00un1VgdHxzADdlk8Q@public.gmane.org>:
> On Tue, 2012-07-31 at 22:28 +0400, Denis Yefremov wrote:
>> Interrupt handler wlcore_irq in wlcore/main.c file
>> invokes mutex_lock that is might sleep operation.
>
>> int __devinit wlcore_probe(struct wl1271 *wl, struct platform_device *pdev)
>> {
>> ...
>> ret = request_threaded_irq(wl->irq, wl12xx_hardirq, wlcore_irq,
>> irqflags,
>> pdev->name, wl);
>
> Yes, umm, why do you think they're using a *threaded* IRQ? :-)
>
> johannes
>
--
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
* Re: [PATCH iproute2] ss: Report MSS from internal TCP information
From: Stephen Hemminger @ 2012-07-31 21:07 UTC (permalink / raw)
To: Ben Hutchings; +Cc: netdev
In-Reply-To: <1343677867.2667.33.camel@bwh-desktop.uk.solarflarecom.com>
On Mon, 30 Jul 2012 20:51:07 +0100
Ben Hutchings <bhutchings@solarflare.com> wrote:
> Signed-off-by: Ben Hutchings <bhutchings@solarflare.com>
> ---
> misc/ss.c | 2 ++
> 1 files changed, 2 insertions(+), 0 deletions(-)
>
> diff --git a/misc/ss.c b/misc/ss.c
> index cf529ef..8ad830b 100644
> --- a/misc/ss.c
> +++ b/misc/ss.c
> @@ -1392,6 +1392,8 @@ static void tcp_show_info(const struct nlmsghdr *nlh, struct inet_diag_msg *r)
> (double)info->tcpi_rttvar/1000);
> if (info->tcpi_ato)
> printf(" ato:%g", (double)info->tcpi_ato/1000);
> + if (info->tcpi_snd_mss)
> + printf(" mss:%d", info->tcpi_snd_mss);
> if (info->tcpi_snd_cwnd != 2)
> printf(" cwnd:%d", info->tcpi_snd_cwnd);
> if (info->tcpi_snd_ssthresh < 0xFFFF)
Applied
^ permalink raw reply
* Re: [PATCH] igb: correct hardware type (i210/i211) check in igb_loopback_test()
From: Jeff Kirsher @ 2012-07-31 21:12 UTC (permalink / raw)
To: Jesper Juhl
Cc: e1000-devel@lists.sourceforge.net, netdev@vger.kernel.org,
Rick Jones, Allan, Bruce W, linux-kernel@vger.kernel.org,
Brandeburg, Jesse, Ronciak, John, David S. Miller
In-Reply-To: <alpine.LNX.2.00.1207312219080.7100@swampdragon.chaosbits.net>
[-- Attachment #1.1: Type: text/plain, Size: 2778 bytes --]
On Tue, 2012-07-31 at 22:23 +0200, Jesper Juhl wrote:
> On Wed, 25 Jul 2012, Wyborny, Carolyn wrote:
>
> >
> > > -----Original Message-----
> > > From: Jesper Juhl [mailto:jj@chaosbits.net]
> > > Sent: Wednesday, July 25, 2012 12:06 PM
> > > To: linux-kernel@vger.kernel.org
> > > Cc: netdev@vger.kernel.org; e1000-devel@lists.sourceforge.net;
> Wyborny, Carolyn; Pieper, Jeffrey E; Kirsher, Jeffrey T; Rick Jones;
> Ronciak, John; Brandeburg, Jesse; Allan, Bruce W; Skidmore, Donald C;
> Rose, Gregory V; Waskiewicz Jr, Peter P; Duyck, Alexander H; David S.
> Miller
> > > Subject: [PATCH] igb: correct hardware type (i210/i211) check in
> igb_loopback_test()
> > >
> > > In the original code
> > > ...
> > > if ((adapter->hw.mac.type == e1000_i210)
> > > || (adapter->hw.mac.type == e1000_i210)) { ...
> > > the second check of 'adapter->hw.mac.type' is pointless since it
> tests for the exact same value as the first.
> > >
> > > After reading through a few other parts of the driver I believe
> that the second check was actually intended to check for 'e1000_i211'
> > > rather than 'e1000_i210', but I admit that I'm not certain so
> someone with more knowledge about this driver should ACK the patch
> before it gets merged.
> > >
> > > Unfortunately I have no hardware to actually test this on, so it
> is compile tested only.
> > >
> > > Signed-off-by: Jesper Juhl <jj@chaosbits.net>
> > > ---
> > > drivers/net/ethernet/intel/igb/igb_ethtool.c | 2 +-
> > > 1 file changed, 1 insertion(+), 1 deletion(-)
> > >
> > > diff --git a/drivers/net/ethernet/intel/igb/igb_ethtool.c
> b/drivers/net/ethernet/intel/igb/igb_ethtool.c
> > > index a19c84c..ad489b7 100644
> > > --- a/drivers/net/ethernet/intel/igb/igb_ethtool.c
> > > +++ b/drivers/net/ethernet/intel/igb/igb_ethtool.c
> > > @@ -1783,7 +1783,7 @@ static int igb_loopback_test(struct
> igb_adapter *adapter, u64 *data)
> > > goto out;
> > > }
> > > if ((adapter->hw.mac.type == e1000_i210)
> > > - || (adapter->hw.mac.type == e1000_i210)) {
> > > + || (adapter->hw.mac.type == e1000_i211)) {
> > > dev_err(&adapter->pdev->dev,
> > > "Loopback test not supported "
> > > "on this part at this time.\n");
> > > --
> > > 1.7.11.3
> > >
> >
> > ACK.
> >
> > Good catch. Thanks Jesper!
> >
> > Carolyn
> >
> > Carolyn Wyborny
> > Linux Development
> > LAN Access Division
> > Intel Corporation
> >
> Thank you for the ack Carolyn.
>
> David: will you take this in the networking tree?
I have a few igb patches already, so I will send a pull request for Dave
with this patch added to the igb patches I already have. Ok?
[-- Attachment #1.2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 836 bytes --]
[-- Attachment #2: Type: text/plain, Size: 395 bytes --]
------------------------------------------------------------------------------
Live Security Virtual Conference
Exclusive live event will cover all the ways today's security and
threat landscape has changed and how IT managers can respond. Discussions
will include endpoint security, mobile security and the latest in malware
threats. http://www.accelacomm.com/jaw/sfrnl04242012/114/50122263/
[-- Attachment #3: Type: text/plain, Size: 257 bytes --]
_______________________________________________
E1000-devel mailing list
E1000-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/e1000-devel
To learn more about Intel® Ethernet, visit http://communities.intel.com/community/wired
^ permalink raw reply
* Re: [PATCH] igb: correct hardware type (i210/i211) check in igb_loopback_test()
From: Jesper Juhl @ 2012-07-31 21:19 UTC (permalink / raw)
To: Jeff Kirsher
Cc: e1000-devel@lists.sourceforge.net, netdev@vger.kernel.org,
Rick Jones, Allan, Bruce W, linux-kernel@vger.kernel.org,
Brandeburg, Jesse, Ronciak, John, David S. Miller
In-Reply-To: <1343769121.2230.28.camel@jtkirshe-mobl>
On Tue, 31 Jul 2012, Jeff Kirsher wrote:
> On Tue, 2012-07-31 at 22:23 +0200, Jesper Juhl wrote:
> > On Wed, 25 Jul 2012, Wyborny, Carolyn wrote:
> >
> > >
> > > > -----Original Message-----
> > > > From: Jesper Juhl [mailto:jj@chaosbits.net]
> > > > Sent: Wednesday, July 25, 2012 12:06 PM
> > > > To: linux-kernel@vger.kernel.org
> > > > Cc: netdev@vger.kernel.org; e1000-devel@lists.sourceforge.net;
> > Wyborny, Carolyn; Pieper, Jeffrey E; Kirsher, Jeffrey T; Rick Jones;
> > Ronciak, John; Brandeburg, Jesse; Allan, Bruce W; Skidmore, Donald C;
> > Rose, Gregory V; Waskiewicz Jr, Peter P; Duyck, Alexander H; David S.
> > Miller
> > > > Subject: [PATCH] igb: correct hardware type (i210/i211) check in
> > igb_loopback_test()
> > > >
> > > > In the original code
> > > > ...
> > > > if ((adapter->hw.mac.type == e1000_i210)
> > > > || (adapter->hw.mac.type == e1000_i210)) { ...
> > > > the second check of 'adapter->hw.mac.type' is pointless since it
> > tests for the exact same value as the first.
> > > >
> > > > After reading through a few other parts of the driver I believe
> > that the second check was actually intended to check for 'e1000_i211'
> > > > rather than 'e1000_i210', but I admit that I'm not certain so
> > someone with more knowledge about this driver should ACK the patch
> > before it gets merged.
> > > >
> > > > Unfortunately I have no hardware to actually test this on, so it
> > is compile tested only.
> > > >
> > > > Signed-off-by: Jesper Juhl <jj@chaosbits.net>
> > > > ---
> > > > drivers/net/ethernet/intel/igb/igb_ethtool.c | 2 +-
> > > > 1 file changed, 1 insertion(+), 1 deletion(-)
> > > >
> > > > diff --git a/drivers/net/ethernet/intel/igb/igb_ethtool.c
> > b/drivers/net/ethernet/intel/igb/igb_ethtool.c
> > > > index a19c84c..ad489b7 100644
> > > > --- a/drivers/net/ethernet/intel/igb/igb_ethtool.c
> > > > +++ b/drivers/net/ethernet/intel/igb/igb_ethtool.c
> > > > @@ -1783,7 +1783,7 @@ static int igb_loopback_test(struct
> > igb_adapter *adapter, u64 *data)
> > > > goto out;
> > > > }
> > > > if ((adapter->hw.mac.type == e1000_i210)
> > > > - || (adapter->hw.mac.type == e1000_i210)) {
> > > > + || (adapter->hw.mac.type == e1000_i211)) {
> > > > dev_err(&adapter->pdev->dev,
> > > > "Loopback test not supported "
> > > > "on this part at this time.\n");
> > > > --
> > > > 1.7.11.3
> > > >
> > >
> > > ACK.
> > >
> > > Good catch. Thanks Jesper!
> > >
> > > Carolyn
> > >
> > > Carolyn Wyborny
> > > Linux Development
> > > LAN Access Division
> > > Intel Corporation
> > >
> > Thank you for the ack Carolyn.
> >
> > David: will you take this in the networking tree?
>
> I have a few igb patches already, so I will send a pull request for Dave
> with this patch added to the igb patches I already have. Ok?
>
That's perfectly fine with me.
Perhaps you could consider also picking up the other one I sent a few
minutes after this one ?
Subject: [PATCH] igb: don't break user visible strings over multiple lines in igb_ethtool.c
Message-ID: <alpine.LNX.2.00.1207252115321.11754@swampdragon.chaosbits.net>
--
Jesper Juhl <jj@chaosbits.net> http://www.chaosbits.net/
Don't top-post http://www.catb.org/jargon/html/T/top-post.html
Plain text mails only, please.
------------------------------------------------------------------------------
Live Security Virtual Conference
Exclusive live event will cover all the ways today's security and
threat landscape has changed and how IT managers can respond. Discussions
will include endpoint security, mobile security and the latest in malware
threats. http://www.accelacomm.com/jaw/sfrnl04242012/114/50122263/
_______________________________________________
E1000-devel mailing list
E1000-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/e1000-devel
To learn more about Intel® Ethernet, visit http://communities.intel.com/community/wired
^ permalink raw reply
* Re: [PATCH] igb: don't break user visible strings over multiple lines in igb_ethtool.c
From: Jeff Kirsher @ 2012-07-31 21:30 UTC (permalink / raw)
To: Jesper Juhl
Cc: linux-kernel, netdev, e1000-devel, Carolyn Wyborny, Jeff Pieper,
Rick Jones, Jesse Brandeburg, Bruce Allan, Don Skidmore,
Greg Rose, Peter P Waskiewicz Jr, Alex Duyck, David S. Miller
In-Reply-To: <alpine.LNX.2.00.1207252115321.11754@swampdragon.chaosbits.net>
[-- Attachment #1: Type: text/plain, Size: 421 bytes --]
On Wed, 2012-07-25 at 21:19 +0200, Jesper Juhl wrote:
> Even when they go beyond 80 characters, user visible strings should be
> on one line to make them easy to grep for.
>
> Signed-off-by: Jesper Juhl <jj@chaosbits.net>
> ---
> drivers/net/ethernet/intel/igb/igb_ethtool.c | 23
> +++++++++++------------
> 1 file changed, 11 insertions(+), 12 deletions(-)
Applied to my queue of patches, thanks Jesper.
[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 836 bytes --]
^ permalink raw reply
* Re: [PATCH] igb: correct hardware type (i210/i211) check in igb_loopback_test()
From: Jeff Kirsher @ 2012-07-31 21:30 UTC (permalink / raw)
To: Jesper Juhl
Cc: e1000-devel@lists.sourceforge.net, netdev@vger.kernel.org,
Rick Jones, Allan, Bruce W, linux-kernel@vger.kernel.org,
Brandeburg, Jesse, Ronciak, John, David S. Miller
In-Reply-To: <alpine.LNX.2.00.1207312315310.7100@swampdragon.chaosbits.net>
[-- Attachment #1.1: Type: text/plain, Size: 3506 bytes --]
On Tue, 2012-07-31 at 23:19 +0200, Jesper Juhl wrote:
> On Tue, 31 Jul 2012, Jeff Kirsher wrote:
>
> > On Tue, 2012-07-31 at 22:23 +0200, Jesper Juhl wrote:
> > > On Wed, 25 Jul 2012, Wyborny, Carolyn wrote:
> > >
> > > >
> > > > > -----Original Message-----
> > > > > From: Jesper Juhl [mailto:jj@chaosbits.net]
> > > > > Sent: Wednesday, July 25, 2012 12:06 PM
> > > > > To: linux-kernel@vger.kernel.org
> > > > > Cc: netdev@vger.kernel.org; e1000-devel@lists.sourceforge.net;
> > > Wyborny, Carolyn; Pieper, Jeffrey E; Kirsher, Jeffrey T; Rick Jones;
> > > Ronciak, John; Brandeburg, Jesse; Allan, Bruce W; Skidmore, Donald C;
> > > Rose, Gregory V; Waskiewicz Jr, Peter P; Duyck, Alexander H; David S.
> > > Miller
> > > > > Subject: [PATCH] igb: correct hardware type (i210/i211) check in
> > > igb_loopback_test()
> > > > >
> > > > > In the original code
> > > > > ...
> > > > > if ((adapter->hw.mac.type == e1000_i210)
> > > > > || (adapter->hw.mac.type == e1000_i210)) { ...
> > > > > the second check of 'adapter->hw.mac.type' is pointless since it
> > > tests for the exact same value as the first.
> > > > >
> > > > > After reading through a few other parts of the driver I believe
> > > that the second check was actually intended to check for 'e1000_i211'
> > > > > rather than 'e1000_i210', but I admit that I'm not certain so
> > > someone with more knowledge about this driver should ACK the patch
> > > before it gets merged.
> > > > >
> > > > > Unfortunately I have no hardware to actually test this on, so it
> > > is compile tested only.
> > > > >
> > > > > Signed-off-by: Jesper Juhl <jj@chaosbits.net>
> > > > > ---
> > > > > drivers/net/ethernet/intel/igb/igb_ethtool.c | 2 +-
> > > > > 1 file changed, 1 insertion(+), 1 deletion(-)
> > > > >
> > > > > diff --git a/drivers/net/ethernet/intel/igb/igb_ethtool.c
> > > b/drivers/net/ethernet/intel/igb/igb_ethtool.c
> > > > > index a19c84c..ad489b7 100644
> > > > > --- a/drivers/net/ethernet/intel/igb/igb_ethtool.c
> > > > > +++ b/drivers/net/ethernet/intel/igb/igb_ethtool.c
> > > > > @@ -1783,7 +1783,7 @@ static int igb_loopback_test(struct
> > > igb_adapter *adapter, u64 *data)
> > > > > goto out;
> > > > > }
> > > > > if ((adapter->hw.mac.type == e1000_i210)
> > > > > - || (adapter->hw.mac.type == e1000_i210)) {
> > > > > + || (adapter->hw.mac.type == e1000_i211)) {
> > > > > dev_err(&adapter->pdev->dev,
> > > > > "Loopback test not supported "
> > > > > "on this part at this time.\n");
> > > > > --
> > > > > 1.7.11.3
> > > > >
> > > >
> > > > ACK.
> > > >
> > > > Good catch. Thanks Jesper!
> > > >
> > > > Carolyn
> > > >
> > > > Carolyn Wyborny
> > > > Linux Development
> > > > LAN Access Division
> > > > Intel Corporation
> > > >
> > > Thank you for the ack Carolyn.
> > >
> > > David: will you take this in the networking tree?
> >
> > I have a few igb patches already, so I will send a pull request for Dave
> > with this patch added to the igb patches I already have. Ok?
> >
> That's perfectly fine with me.
>
> Perhaps you could consider also picking up the other one I sent a few
> minutes after this one ?
> Subject: [PATCH] igb: don't break user visible strings over multiple lines in igb_ethtool.c
> Message-ID: <alpine.LNX.2.00.1207252115321.11754@swampdragon.chaosbits.net>
Done.
[-- Attachment #1.2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 836 bytes --]
[-- Attachment #2: Type: text/plain, Size: 395 bytes --]
------------------------------------------------------------------------------
Live Security Virtual Conference
Exclusive live event will cover all the ways today's security and
threat landscape has changed and how IT managers can respond. Discussions
will include endpoint security, mobile security and the latest in malware
threats. http://www.accelacomm.com/jaw/sfrnl04242012/114/50122263/
[-- Attachment #3: Type: text/plain, Size: 257 bytes --]
_______________________________________________
E1000-devel mailing list
E1000-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/e1000-devel
To learn more about Intel® Ethernet, visit http://communities.intel.com/community/wired
^ permalink raw reply
* [PATCH] bnx2x: fix mem leak when command is unknown
From: Jesper Juhl @ 2012-07-31 21:39 UTC (permalink / raw)
To: Eilon Greenstein; +Cc: netdev, linux-kernel
In bnx2x_mcast_enqueue_cmd() we'll leak the memory allocated to
'new_cmd' if we hit the deafault case of the 'switch (cmd)'.
Add a 'kfree(new_cmd)' to that case to avoid the leak.
Signed-off-by: Jesper Juhl <jj@chaosbits.net>
---
drivers/net/ethernet/broadcom/bnx2x/bnx2x_sp.c | 1 +
1 file changed, 1 insertion(+)
note: due to lack of hardware this patch is compile tested only.
diff --git a/drivers/net/ethernet/broadcom/bnx2x/bnx2x_sp.c b/drivers/net/ethernet/broadcom/bnx2x/bnx2x_sp.c
index 734fd87..62f754b 100644
--- a/drivers/net/ethernet/broadcom/bnx2x/bnx2x_sp.c
+++ b/drivers/net/ethernet/broadcom/bnx2x/bnx2x_sp.c
@@ -2485,6 +2485,7 @@ static int bnx2x_mcast_enqueue_cmd(struct bnx2x *bp,
break;
default:
+ kfree(new_cmd);
BNX2X_ERR("Unknown command: %d\n", cmd);
return -EINVAL;
}
--
1.7.11.3
--
Jesper Juhl <jj@chaosbits.net> http://www.chaosbits.net/
Don't top-post http://www.catb.org/jargon/html/T/top-post.html
Plain text mails only, please.
^ permalink raw reply related
* Re: [PATCH v2] ipv4: Restore old dst_free() behavior.
From: David Miller @ 2012-07-31 21:42 UTC (permalink / raw)
To: eric.dumazet; +Cc: netdev
In-Reply-To: <1343732903.21269.190.camel@edumazet-glaptop>
From: Eric Dumazet <eric.dumazet@gmail.com>
Date: Tue, 31 Jul 2012 13:08:23 +0200
> [PATCH v2] ipv4: Restore old dst_free() behavior
>
> commit 404e0a8b6a55 (net: ipv4: fix RCU races on dst refcounts) tried
> to solve a race but added a problem at device/fib dismantle time :
>
> We really want to call dst_free() as soon as possible, even if sockets
> still have dst in their cache.
> dst_release() calls in free_fib_info_rcu() are not welcomed.
>
> Root of the problem was that now we also cache output routes (in
> nh_rth_output), we must use call_rcu() instead of call_rcu_bh() in
> rt_free(), because output route lookups are done in process context.
>
> Based on feedback and initial patch from David Miller (adding another
> call_rcu_bh() call in fib, but it appears it was not the right fix)
>
> I left the inet_sk_rx_dst_set() helper and added __rcu attributes
> to nh_rth_output and nh_rth_input to better document what is going on in
> this code.
>
> Signed-off-by: Eric Dumazet <edumazet@google.com>
Applied, thanks Eric.
^ permalink raw reply
* Re: [PATCH v2] ipv4: percpu nh_rth_output cache
From: David Miller @ 2012-07-31 21:43 UTC (permalink / raw)
To: eric.dumazet; +Cc: netdev, alexander.h.duyck
In-Reply-To: <1343749530.21269.336.camel@edumazet-glaptop>
From: Eric Dumazet <eric.dumazet@gmail.com>
Date: Tue, 31 Jul 2012 17:45:30 +0200
> From: Eric Dumazet <edumazet@google.com>
>
> Input path is mostly run under RCU and doesnt touch dst refcnt
>
> But output path on forwarding or UDP workloads hits
> badly dst refcount, and we have lot of false sharing, for example
> in ipv4_mtu() when reading rt->rt_pmtu
>
> Using a percpu cache for nh_rth_output gives a nice performance
> increase at a small cost.
>
> 24 udpflood test on my 24 cpu machine (dummy0 output device)
> (each process sends 1.000.000 udp frames, 24 processes are started)
>
> before : 5.24 s
> after : 2.06 s
> For reference, time on linux-3.5 : 6.60 s
>
> Signed-off-by: Eric Dumazet <edumazet@google.com>
Applied.
However I think fib_create_info() should fail if it cannot alloc_percpu()
successfully, instead of having funny logic in rt_cache_route().
^ 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