* Re: pull request: wireless-next-2.6 2009-10-28
From: Johannes Berg @ 2009-11-02 9:23 UTC (permalink / raw)
To: Ingo Molnar
Cc: John W. Linville, Jarek Poplawski, Bartlomiej Zolnierkiewicz,
Pekka Enberg, David Miller, linux-wireless, netdev, linux-kernel
In-Reply-To: <20091102091038.GA9044@elte.hu>
[-- Attachment #1: Type: text/plain, Size: 663 bytes --]
On Mon, 2009-11-02 at 10:10 +0100, Ingo Molnar wrote:
> So i have read the thread you and Bartlomiej referenced:
>
> http://lkml.org/lkml/2009/10/17/81
>
> ... and my understanding of that discussion is very different from
> yours. Here is my annotated history of the beginnings of that
> discussion:
[snip]
You shouldn't ignore all previous interaction between Bart and us --
which wasn't pretty: http://thread.gmane.org/gmane.linux.kernel/901892
Of course we were biased when he came around with that petty code
duplication argument, since it seemed to support only his agenda of
working only with the staging drivers.
johannes
[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 801 bytes --]
^ permalink raw reply
* Re: pull request: wireless-next-2.6 2009-10-28
From: Ingo Molnar @ 2009-11-02 9:10 UTC (permalink / raw)
To: John W. Linville
Cc: Jarek Poplawski, Johannes Berg, Bartlomiej Zolnierkiewicz,
Pekka Enberg, David Miller, linux-wireless, netdev, linux-kernel
In-Reply-To: <20091030150223.GA2586@tuxdriver.com>
* John W. Linville <linville@tuxdriver.com> wrote:
> On Fri, Oct 30, 2009 at 11:06:16AM +0000, Jarek Poplawski wrote:
>
> > There are various ways to disagree, and ignoring by John questions
> > from a merited developer both in this referenced lkml and current
> > threads looks at least strange (if not offensive) as well.
>
> Did you read the thread for which Bartlomiej provided a link earlier?
> There were ten responses (only three of them from him) in that thread.
> His comments were not ignored, they were rejected.
>
> Ever since Bartlomiej decided to tear himself away from
> drivers/staging, he has been nothing but negative -- petty, whining,
> indignat, whatever. Just what has he done to merit any special
> consideration here? Why should he have any sort of veto over rt2x00?
I got curious, as my past experience with Bartlomiej is that he is a
factual, reliable, knowledgable upstream driver developer interested in
difficult pieces of code others are reluctant to touch, for whom it is
rather atypical to get 'petty, whining, indignant'.
So i have read the thread you and Bartlomiej referenced:
http://lkml.org/lkml/2009/10/17/81
... and my understanding of that discussion is very different from
yours. Here is my annotated history of the beginnings of that
discussion:
Bartlomiej (in <200910171654.03344.bzolnier@gmail.com>) started his
review of the driver with:
| First let me say that I'm very happy to see this patch finally being
| submitted and I appreciate the effort..
|
| (I'll give it a spin on Eee 901 w/ 2.6.32-rc5 sometime later..)
Very friendly and constructive. Pretty much the Bartlomiej i have known
for years.
Then he continues with his technical observations:
| Now to the less happy part..
|
| I also used the opportunity to take a closer look at this driver and
| it seems that it needlessly adds around 2 KLOC to kernel by
| duplicating the common content of rt2800usb.h to rt2800pci.h instead
| of moving it to the shared header (like it is done in the staging
| crap drivers):
|
| [...]
|
| All in all, the total amount of the kernel code needed for
| implementing rt2800pci functionality should 1-2 KLOC instead of the
| current 5 KLOC.
Looks like a valid technical point that should be replied to in ernest.
Johannes Berg's first reply (<1255792104.3434.2.camel@johannes.local>)
ignored Bartlomiej's friendly approach and launched a combative,
emotion-laden, unconstructive (and technically inapposite) attack:
| Tell me you're kidding -- comparing 2k duplicated LOC with a driver
| that ships its own wifi stack?
Bartlomiej's reply (<1255792104.3434.2.camel@johannes.local>) ignored
the attack (gracefully) and replied to the technical portion only:
| > Tell me you're kidding -- comparing 2k duplicated LOC with a driver
| > that ships its own wifi stack?
|
| Why would I be?
|
| 1) The patch is submitted to kernel _proper_ not kernel staging so I
| see no excuse for duplicating 2-4 KLOC and it should be fixed.
|
| 2) The fact that the some staging driver consists in 90% of crap
| doesn't mean that it doesn't have some good design ideas.. (i.e.
| abstracting chipset registers access in a discussed case)
To which technical point Johannes elected not to reply. (Effectively
conceding Bartlomiej's point as per lkml discussion rules.)
[ There are similar patterns in other threads of this discussion - the
reply in (<200910181859.22413.IvDoorn@gmail.com>) and followups
were similarly dismissive (while not as combative as Johannes's reply)
- with an often offensive tone against Bartlomiej. ]
Bartlomiej followed up with his test results in another message in
<200910172318.56929.bzolnier@gmail.com>. Corroborated by Luis Correia in
<efe7343f0910180240o223ac346j3dd7c45c7460ec41@mail.gmail.com>. Both
messages were factual, constructive and friendly.
Neither failure report was replied to in that thread and remains ignored
up to today, 15 days down the line.
Alas, the portion of the story that is visible in that discussion on
lkml contradicts your claim almost 180 degrees. The person being
attacked there was Bartlomiej and i simply dont see where you got the
conclusion from that he was 'petty, whining, indignant'.
Now look at the aftermath from Bartlomiej's perspective: this
non-working driver with arguably unresponsive, unfriendly maintainers
got pulled twice (first by you and then by David), and it is now on the
unstoppable path upstream. By omission he's been forced to raise these
issues at every hop that pulls this piece of code - and it was not his
choice to be exposed to such a spiral of a workflow.
I can understand David trusting your judgement and not wanting to get
involved in the fine details, but having read the surrounding discussion
i dont understand your interpretation of the events, and i dont
understand on what basis you launched your very serious accusation, that
he is being 'petty, whining, indignant'. Every reply from him in that
thread is the exact opposite of that. Care to elaborate?
Thanks,
Ingo
^ permalink raw reply
* Re: kerneloops.org report for the week of October 31 2009
From: Andy Whitcroft @ 2009-11-02 8:31 UTC (permalink / raw)
To: Rafael J. Wysocki
Cc: Arjan van de Ven, linux-kernel, torvalds, linux-acpi, tytso,
netdev, Eric Paris, akpm, Matt Domsch
In-Reply-To: <200911010020.20284.rjw@sisk.pl>
> This should be fixed by commit 04bf7539c08d64184736cdc5e4ad617eda77eb0f
> (PM: Make warning in suspend_test_finish() less likely to happen), at least on some
> machines that report it.
That commit has been pulled back to the first 9.10 updates kernel to
reduce the noise.
-apw
^ permalink raw reply
* Re: HTB accuracy on 10GbE
From: Badalian Vyacheslav @ 2009-11-02 8:17 UTC (permalink / raw)
To: Ryousei Takano; +Cc: shemminger, Linux Netdev List, takano-ryousei
In-Reply-To: <b30d1c3b0911012322v5d553fd6va56575fcf5e3daf8@mail.gmail.com>
Hello.
Also we planed convert 5-10 servers witch 1gigabit connection to one BIG server witch 10g (traffic rate in peak about 6gigabit) network card (Intel multiqueue).
Can test any patches for fix any problems in 10 gigabit connections!
Thanks!
JSC BIG Telecom
> Hi Stephen and all,
>
> I have observed a HTB accuracy problem on the Linux kernel 2.6.30 and
> the Myri-10G 10 GbE NIC.
> HTB can control the transmission rate at Gigabit speed, however it can
> not work well at 10 Gigabit speed.
>
> I asked Stephen this problem at Japan Linux Symposium. He mentioned a
> HTB bug related to the timer granularity.
> I want to know what is happen, and what should be do for fixing it.
>
> Any comments and suggestions will be welcome.
>
> For more detail, please see the following page:
> http://code.google.com/p/pspacer/wiki/HTBon10GbE
>
> Best regards,
> Ryousei
> --
> 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: RFC: netdev: allow ethtool physical id to drop rtnl_lock
From: David Miller @ 2009-11-02 8:17 UTC (permalink / raw)
To: mchan; +Cc: shemminger, netdev
In-Reply-To: <1257007462.9706.9.camel@HP1>
From: "Michael Chan" <mchan@broadcom.com>
Date: Sat, 31 Oct 2009 09:44:22 -0700
>
> On Fri, 2009-10-30 at 10:42 -0700, Stephen Hemminger wrote:
>> The ethtool operation to blink LED can take an indeterminately long time,
>> blocking out other operations (via rtnl_lock). This patch is an attempt
>> to work around the problem.
>>
>> It does need more discussion, because it will mean that drivers that formerly
>> were protected from changes during blink aren't. For example, user could
>> start device blinking, and then plug in cable causing change netlink event
>> to change state or pull cable and have device come down.
>
> Yeah, the biggest concern is shutting down the device while it is still
> blinking. During shutdown, some devices are brought to low power state
> and the chip will no longer respond to I/Os to blink the LEDs. On some
> systems, this can cause bus hang or NMI.
Right, and for this reason we'll either need find some way to stop
the LED blinking when the device is brought down.
We can deal with this in a way such that we'll never need to bug
the drivers again if we want to mess with the implementation again.
Create a "netif_phys_id_loop_iter()" that, along with a netdev
pointer, takes a "u32 data" which is whatever was passed in to
ethtool_ops->id().
The drivers then structure their loops like:
while (1) {
blink_it_baby();
data = netif_phys_id_loop_iter(dev, data);
if (!data)
break;
}
Next, we take that:
if (data == 0)
data = UINT_MAX / 2;
That every driver seems to do, and stick it in the ethtool op dispatch
in net/core/ethtool.c so it doesn't need to be duplicated (and
potentially forgotten) in every implementation.
Finally, in netif_phys_id_loop_iter() we put something like:
u32 netif_phys_id_loop_iter(struct netdev *dev, u32 data)
{
if (dev->reg_state == NETREG_UNREGISTERING)
return 0;
if (msleep_interruptible(500))
return 0;
return data - 2;
}
Then, unregister somehow blocks on the ->phys_id() hitting that
NETREG_UNREGISTERING check and returning.
Anyways, you get the idea.
^ permalink raw reply
* Re: [RFC] bridge: check address size
From: David Miller @ 2009-11-02 8:07 UTC (permalink / raw)
To: shemminger; +Cc: nneul, netdev
In-Reply-To: <20091029152408.6c6cc29f@nehalam>
From: Stephen Hemminger <shemminger@vyatta.com>
Date: Thu, 29 Oct 2009 15:24:08 -0700
> - if (dev->flags & IFF_LOOPBACK || dev->type != ARPHRD_ETHER)
> + /* Don't allow bridging non ethernet like devices */
> + if (dev->flags & IFF_LOOPBACK
> + || dev->type != ARPHRD_ETHER
> + || dev->addr_len != ETH_ALEN)
Please format this as:
> + if (dev->flags & IFF_LOOPBACK ||
> + dev->type != ARPHRD_ETHER ||
> + dev->addr_len != ETH_ALEN)
What you're doing in the patch follows the GNU coding standards, no
the kernel ones. :-)
^ permalink raw reply
* Re: [RFC, PATCH] net: suspicious test in dev_change_name()
From: David Miller @ 2009-11-02 8:05 UTC (permalink / raw)
To: jarkao2; +Cc: eric.dumazet, netdev
In-Reply-To: <4AEB7BB1.9000901@gmail.com>
From: Jarek Poplawski <jarkao2@gmail.com>
Date: Sat, 31 Oct 2009 00:50:09 +0100
> I don't think so: err stores the previous ret meaning rollback and
> is checked for this later. But somebody forgot err can store previous
> (positive) value here, so IMHO you're right: there is a bug in this
> place ;-)
Just not the one Eric is specifically fixing :-)
^ permalink raw reply
* Re: [net-next PATCH 3/4] qlge: Reduce debug print output.
From: David Miller @ 2009-11-02 8:03 UTC (permalink / raw)
To: joe; +Cc: ron.mercer, netdev
In-Reply-To: <1256942686.1917.66.camel@Joe-Laptop.home>
From: Joe Perches <joe@perches.com>
Date: Fri, 30 Oct 2009 15:44:46 -0700
> On Fri, 2009-10-30 at 15:13 -0700, Ron Mercer wrote:
>> Signed-off-by: Ron Mercer <ron.mercer@qlogic.com>
> []
>> diff --git a/drivers/net/qlge/qlge.h b/drivers/net/qlge/qlge.h
>> index b9f65e0..502c3af 100644
>> --- a/drivers/net/qlge/qlge.h
>> +++ b/drivers/net/qlge/qlge.h
>> @@ -27,6 +27,18 @@
>> dev_printk(KERN_##klevel, &((qdev)->pdev->dev), \
>> "%s: " fmt, __func__, ##args); \
>> } while (0)
>> +#if 0
>> +#define QPRINTK_DBG(qdev, nlevel, klevel, fmt, args...) \
>> + do { \
>> + if (!((qdev)->msg_enable & NETIF_MSG_##nlevel)) \
>> + ; \
>> + else \
>> + dev_printk(KERN_##klevel, &((qdev)->pdev->dev), \
>> + "%s: " fmt, __func__, ##args); \
>> + } while (0)
>> +#else
>> +#define QPRINTK_DBG(qdev, nlevel, klevel, fmt, args...)
>> +#endif
>
> This uses an inverted test and it doesn't verify the args to
> dev_printk when not #defined.
>
> How about:
I also don't like this kind of change for another reason.
The message levels are pointless if you're going to adhere to them
or not based upon some CPP define.
Either do it, or don't. Like every other driver does.
If for some reason the default is problematic, adjust the default that
you pass to netif_msg_init() or, alternatively, adjust what level the
debugging messages are assigned to.
We have all of this wonderful, full, infrastructure for message
levelling. And you can change the setting either at module load time
or via ethtool. The default is also up to you as well.
And you're going to stick a "#if 0" CPP control in there? :-/
No way, I absolutely won't accept this kind of change, there is no
need for it. There is more than enough dynamic flexibility, both at
run-time via module option and ethtool message level selections, and
at compile time via the default you can choose hoever you like.
^ permalink raw reply
* Re: [PATCH net-next-2.6] pppoe: RCU locking in get_item_by_addr()
From: David Miller @ 2009-11-02 7:57 UTC (permalink / raw)
To: eric.dumazet; +Cc: netdev
In-Reply-To: <4AEE735A.4020601@gmail.com>
From: Eric Dumazet <eric.dumazet@gmail.com>
Date: Mon, 02 Nov 2009 06:51:22 +0100
>
> Note : this patch depends on dev_get_by_name_rcu(), not yet comitted.
Yep, I just tossed that one ine.
> [PATCH net-next-2.6] pppoe: RCU locking in get_item_by_addr()
>
> Use dev_get_by_name_rcu() instead of dev_get_by_name(),
> to avoid touching device refcount in hotpath.
>
> Signed-off-by: Eric Dumazet <eric.dumazet@gmail.com>
And now this one too, applied, thanks!
^ permalink raw reply
* Re: [PATCH net-next-2.6] ifb: RCU locking avoids touching dev refcount
From: David Miller @ 2009-11-02 7:57 UTC (permalink / raw)
To: eric.dumazet; +Cc: netdev
In-Reply-To: <4AEE71EC.7040208@gmail.com>
From: Eric Dumazet <eric.dumazet@gmail.com>
Date: Mon, 02 Nov 2009 06:45:16 +0100
> Avoids touching dev refcount in hotpath
>
> Signed-off-by: Eric Dumazet <eric.dumazet@gmail.com>
Applied.
^ permalink raw reply
* Re: [PATCH net-next-2.6] net: RCU locking for simple ioctl()
From: David Miller @ 2009-11-02 7:57 UTC (permalink / raw)
To: eric.dumazet; +Cc: netdev
In-Reply-To: <4AEE7131.601@gmail.com>
From: Eric Dumazet <eric.dumazet@gmail.com>
Date: Mon, 02 Nov 2009 06:42:09 +0100
> All ioctls() implemented by dev_ifsioc_locked() :
> SIOCGIFFLAGS, SIOCGIFMETRIC, SIOCGIFMTU, SIOCGIFHWADDR,
> SIOCGIFSLAVE, SIOCGIFMAP, SIOCGIFINDEX & SIOCGIFTXQLEN
> can use RCU lock instead of dev_base_lock rwlock
>
> Signed-off-by: Eric Dumazet <eric.dumazet@gmail.com>
Applied.
^ permalink raw reply
* Re: [PATCH net-next-2.6] icmp: icmp_send() can avoid a dev_put()
From: David Miller @ 2009-11-02 7:57 UTC (permalink / raw)
To: eric.dumazet; +Cc: netdev
In-Reply-To: <4AEE6E97.5070809@gmail.com>
From: Eric Dumazet <eric.dumazet@gmail.com>
Date: Mon, 02 Nov 2009 06:31:03 +0100
> We can avoid touching device refcount in icmp_send(),
> using dev_get_by_index_rcu()
>
> Signed-off-by: Eric Dumazet <eric.dumazet@gmail.com>
Applied.
^ permalink raw reply
* Re: [PPATCH net-next-2.6] ipv4: inetdev_by_index() switch to RCU
From: David Miller @ 2009-11-02 7:56 UTC (permalink / raw)
To: eric.dumazet; +Cc: netdev
In-Reply-To: <4AEE6CB8.1000106@gmail.com>
From: Eric Dumazet <eric.dumazet@gmail.com>
Date: Mon, 02 Nov 2009 06:23:04 +0100
> Use dev_get_by_index_rcu() instead of __dev_get_by_index() and
> dev_base_lock rwlock
>
> Signed-off-by: Eric Dumazet <eric.dumazet@gmail.com>
Man, you've been busy.... :-)
Applied, thanks!
^ permalink raw reply
* Re: [PATCH net-next-2.6] veth: Fix unregister_netdevice_queue for veth
From: David Miller @ 2009-11-02 7:56 UTC (permalink / raw)
To: eric.dumazet; +Cc: ebiederm, netdev, xemul
In-Reply-To: <4AEC0653.8050109@gmail.com>
From: Eric Dumazet <eric.dumazet@gmail.com>
Date: Sat, 31 Oct 2009 10:41:39 +0100
> Eric W. Biederman a écrit :
>> I tested the recent unregister many changes and got a weird,
>> nasty and seemingly unrelasted kernel oops. Changing
>> unregister_netdevice_queue to use list_move_tail fixes
>> the problem for me.
>>
>> ip link add type veth
>> rmmod veth
>>
>> ls /sys/class/net/
>> showed one of the veth devices still present.
>>
>> A subsequent ip link oopsed the box.
>>
>> Signed-off-by: "Eric W. Biederman" <ebiederm@xmission.com>
...
> Acked-by: Eric Dumazet <eric.dumazet@gmail.com>
Applied, thanks everyone.
^ permalink raw reply
* Re: [net-next-2.6] net: Introduce dev_get_by_name_rcu()
From: David Miller @ 2009-11-02 7:56 UTC (permalink / raw)
To: eric.dumazet; +Cc: netdev
Applied to net-next-2.6, thanks Eric.
^ permalink raw reply
* Re: [PATCH]NET:KS8695: add API for get rx interrupt bit
From: David Miller @ 2009-11-02 7:55 UTC (permalink / raw)
To: figo1802; +Cc: netdev
In-Reply-To: <1256907911.2148.87.camel@myhost>
From: "Figo.zhang" <figo1802@gmail.com>
Date: Fri, 30 Oct 2009 21:05:11 +0800
> 1. Add API Add k8695_get_rx_enable_bit() for get Rx interrupt
> enable/status bit.
> 2. add some comment or document about some functions and variables.
> 3. update driver version to "1.02"
> 4. add napi_enable() and napi_disable() in open/close file method.
>
> Signed-off-by: Figo.zhang <figo1802@gmail.com>
Patch applied to net-next-2.6, thanks.
^ permalink raw reply
* HTB accuracy on 10GbE
From: Ryousei Takano @ 2009-11-02 7:22 UTC (permalink / raw)
To: shemminger; +Cc: Linux Netdev List, takano-ryousei
Hi Stephen and all,
I have observed a HTB accuracy problem on the Linux kernel 2.6.30 and
the Myri-10G 10 GbE NIC.
HTB can control the transmission rate at Gigabit speed, however it can
not work well at 10 Gigabit speed.
I asked Stephen this problem at Japan Linux Symposium. He mentioned a
HTB bug related to the timer granularity.
I want to know what is happen, and what should be do for fixing it.
Any comments and suggestions will be welcome.
For more detail, please see the following page:
http://code.google.com/p/pspacer/wiki/HTBon10GbE
Best regards,
Ryousei
^ permalink raw reply
* Re: [PATCH] r8169: remove firmware for RTL8169D PHY
From: David Miller @ 2009-11-02 6:47 UTC (permalink / raw)
To: ben; +Cc: romieu, edward_hsu, netdev
In-Reply-To: <1257115687.3136.337.camel@localhost>
From: Ben Hutchings <ben@decadent.org.uk>
Date: Sun, 01 Nov 2009 22:48:07 +0000
> The recently added support for RTL8169D chips included some machine
> code without accompanying source code. Replace this with use of the
> firmware loader.
>
> Compile-tested only.
>
> Signed-off-by: Ben Hutchings <ben@decadent.org.uk>
> ---
> Here's my attempt at the driver change. I leave it to you to work out
> licencing of the firmware blob.
I'm not even looking at this patch until you do some research
and substantiate your claims about what this thing is doing.
^ permalink raw reply
* Re: [PATCH] r8169: partial support and phy init for the 8168d
From: David Miller @ 2009-11-02 6:46 UTC (permalink / raw)
To: ben; +Cc: romieu, netdev, edward_hsu
In-Reply-To: <1257092683.3136.325.camel@localhost>
From: Ben Hutchings <ben@decadent.org.uk>
Date: Sun, 01 Nov 2009 16:24:43 +0000
> I believe this is patching machine code in the PHY. And we do not have
> source for that code, so it cannot possibly be distributed under GPL.
You don't know if it's machine code or some data values that
are used to control the PHY's execution.
In fact I would really be surprised if they had some cpu interpeting
code in the 8168d PHY.
You did do some research about that before making such accusations
right? :-)
^ permalink raw reply
* [PATCH net-next-2.6] pppoe: RCU locking in get_item_by_addr()
From: Eric Dumazet @ 2009-11-02 5:51 UTC (permalink / raw)
To: David S. Miller; +Cc: Linux Netdev List
Note : this patch depends on dev_get_by_name_rcu(), not yet comitted.
Thanks
[PATCH net-next-2.6] pppoe: RCU locking in get_item_by_addr()
Use dev_get_by_name_rcu() instead of dev_get_by_name(),
to avoid touching device refcount in hotpath.
Signed-off-by: Eric Dumazet <eric.dumazet@gmail.com>
---
drivers/net/pppoe.c | 19 +++++++++----------
1 files changed, 9 insertions(+), 10 deletions(-)
diff --git a/drivers/net/pppoe.c b/drivers/net/pppoe.c
index 2559991..60c8d23 100644
--- a/drivers/net/pppoe.c
+++ b/drivers/net/pppoe.c
@@ -250,20 +250,19 @@ static inline struct pppox_sock *get_item_by_addr(struct net *net,
{
struct net_device *dev;
struct pppoe_net *pn;
- struct pppox_sock *pppox_sock;
+ struct pppox_sock *pppox_sock = NULL;
int ifindex;
- dev = dev_get_by_name(net, sp->sa_addr.pppoe.dev);
- if (!dev)
- return NULL;
-
- ifindex = dev->ifindex;
- pn = net_generic(net, pppoe_net_id);
- pppox_sock = get_item(pn, sp->sa_addr.pppoe.sid,
+ rcu_read_lock();
+ dev = dev_get_by_name_rcu(net, sp->sa_addr.pppoe.dev);
+ if (dev) {
+ ifindex = dev->ifindex;
+ pn = net_generic(net, pppoe_net_id);
+ pppox_sock = get_item(pn, sp->sa_addr.pppoe.sid,
sp->sa_addr.pppoe.remote, ifindex);
- dev_put(dev);
-
+ }
+ rcu_read_unlock();
return pppox_sock;
}
^ permalink raw reply related
* [PATCH net-next-2.6] ifb: RCU locking avoids touching dev refcount
From: Eric Dumazet @ 2009-11-02 5:45 UTC (permalink / raw)
To: David S. Miller, Linux Netdev List
Avoids touching dev refcount in hotpath
Signed-off-by: Eric Dumazet <eric.dumazet@gmail.com>
---
drivers/net/ifb.c | 6 ++++--
1 files changed, 4 insertions(+), 2 deletions(-)
diff --git a/drivers/net/ifb.c b/drivers/net/ifb.c
index 030913f..69c2566 100644
--- a/drivers/net/ifb.c
+++ b/drivers/net/ifb.c
@@ -98,13 +98,15 @@ static void ri_tasklet(unsigned long dev)
stats->tx_packets++;
stats->tx_bytes +=skb->len;
- skb->dev = dev_get_by_index(&init_net, skb->iif);
+ rcu_read_lock();
+ skb->dev = dev_get_by_index_rcu(&init_net, skb->iif);
if (!skb->dev) {
+ rcu_read_unlock();
dev_kfree_skb(skb);
stats->tx_dropped++;
break;
}
- dev_put(skb->dev);
+ rcu_read_unlock();
skb->iif = _dev->ifindex;
if (from & AT_EGRESS) {
^ permalink raw reply related
* [PATCH net-next-2.6] net: RCU locking for simple ioctl()
From: Eric Dumazet @ 2009-11-02 5:42 UTC (permalink / raw)
To: David S. Miller; +Cc: Linux Netdev List
All ioctls() implemented by dev_ifsioc_locked() :
SIOCGIFFLAGS, SIOCGIFMETRIC, SIOCGIFMTU, SIOCGIFHWADDR,
SIOCGIFSLAVE, SIOCGIFMAP, SIOCGIFINDEX & SIOCGIFTXQLEN
can use RCU lock instead of dev_base_lock rwlock
Signed-off-by: Eric Dumazet <eric.dumazet@gmail.com>
---
net/core/dev.c | 8 ++++----
1 files changed, 4 insertions(+), 4 deletions(-)
diff --git a/net/core/dev.c b/net/core/dev.c
index f54d8b8..c0f27ad 100644
--- a/net/core/dev.c
+++ b/net/core/dev.c
@@ -4315,12 +4315,12 @@ int dev_set_mac_address(struct net_device *dev, struct sockaddr *sa)
EXPORT_SYMBOL(dev_set_mac_address);
/*
- * Perform the SIOCxIFxxx calls, inside read_lock(dev_base_lock)
+ * Perform the SIOCxIFxxx calls, inside rcu_read_lock()
*/
static int dev_ifsioc_locked(struct net *net, struct ifreq *ifr, unsigned int cmd)
{
int err;
- struct net_device *dev = __dev_get_by_name(net, ifr->ifr_name);
+ struct net_device *dev = dev_get_by_name_rcu(net, ifr->ifr_name);
if (!dev)
return -ENODEV;
@@ -4552,9 +4552,9 @@ int dev_ioctl(struct net *net, unsigned int cmd, void __user *arg)
case SIOCGIFINDEX:
case SIOCGIFTXQLEN:
dev_load(net, ifr.ifr_name);
- read_lock(&dev_base_lock);
+ rcu_read_lock();
ret = dev_ifsioc_locked(net, &ifr, cmd);
- read_unlock(&dev_base_lock);
+ rcu_read_unlock();
if (!ret) {
if (colon)
*colon = ':';
^ permalink raw reply related
* [PATCH net-next-2.6] icmp: icmp_send() can avoid a dev_put()
From: Eric Dumazet @ 2009-11-02 5:31 UTC (permalink / raw)
To: David S. Miller; +Cc: Linux Netdev List
We can avoid touching device refcount in icmp_send(),
using dev_get_by_index_rcu()
Signed-off-by: Eric Dumazet <eric.dumazet@gmail.com>
---
diff --git a/net/ipv4/icmp.c b/net/ipv4/icmp.c
index 84adb57..fe11f60 100644
--- a/net/ipv4/icmp.c
+++ b/net/ipv4/icmp.c
@@ -501,15 +501,16 @@ void icmp_send(struct sk_buff *skb_in, int type, int code, __be32 info)
if (!(rt->rt_flags & RTCF_LOCAL)) {
struct net_device *dev = NULL;
+ rcu_read_lock();
if (rt->fl.iif &&
net->ipv4.sysctl_icmp_errors_use_inbound_ifaddr)
- dev = dev_get_by_index(net, rt->fl.iif);
+ dev = dev_get_by_index_rcu(net, rt->fl.iif);
- if (dev) {
+ if (dev)
saddr = inet_select_addr(dev, 0, RT_SCOPE_LINK);
- dev_put(dev);
- } else
+ else
saddr = 0;
+ rcu_read_unlock();
}
tos = icmp_pointers[type].error ? ((iph->tos & IPTOS_TOS_MASK) |
^ permalink raw reply related
* [PPATCH net-next-2.6] ipv4: inetdev_by_index() switch to RCU
From: Eric Dumazet @ 2009-11-02 5:23 UTC (permalink / raw)
To: David S. Miller; +Cc: Linux Netdev List
Use dev_get_by_index_rcu() instead of __dev_get_by_index() and
dev_base_lock rwlock
Signed-off-by: Eric Dumazet <eric.dumazet@gmail.com>
---
diff --git a/net/ipv4/devinet.c b/net/ipv4/devinet.c
index 5df2f6a..ccccaae 100644
--- a/net/ipv4/devinet.c
+++ b/net/ipv4/devinet.c
@@ -405,11 +405,12 @@ struct in_device *inetdev_by_index(struct net *net, int ifindex)
{
struct net_device *dev;
struct in_device *in_dev = NULL;
- read_lock(&dev_base_lock);
- dev = __dev_get_by_index(net, ifindex);
+
+ rcu_read_lock();
+ dev = dev_get_by_index_rcu(net, ifindex);
if (dev)
in_dev = in_dev_get(dev);
- read_unlock(&dev_base_lock);
+ rcu_read_unlock();
return in_dev;
}
^ permalink raw reply related
* Re: [PATCH] e1000: the power down when running ifdown command
From: Naohiro Ooiwa @ 2009-11-02 1:28 UTC (permalink / raw)
To: Stephen Hemminger
Cc: jeffrey.t.kirsher, jesse.brandeburg, peter.p.waskiewicz.jr,
john.ronciak, davem, Andrew Morton, netdev, svaidy, e1000-devel
In-Reply-To: <20091031105838.0d4b59a2@nehalam>
Stephen Hemminger wrote:
> On Sat, 31 Oct 2009 18:39:52 +0900
> Naohiro Ooiwa <nooiwa@miraclelinux.com> wrote:
>
>> Hi All
>>
>> I resend my patch.
>> Sorry, my previous mail lacked an explanation.
>>
>>
>> The e1000 driver doesn't let the power down when running ifdown command.
>> So, I set to the D3hot state of a PCI device at the end of e1000_close().
>>
>> With this modification, e1000 driver reduces power by ifdown.
>> It's about 6 watts when I measured a total power of one server machine
>> in my environment.
>>
>> I tested this patch. The result is good enough to me.
>>
>> Could you please check my patch ?
>> If I should have other considerations, please tell me.
>>
Hi Stephen
Thank you so much for your reply.
> Does this work with Wake On Lan?
Yes, it works WOL.
But I worry that my test is enough.
They are following:
- simple data transmission after ifdown;ifup.
- enable wol, ifup network device, system shutdown, and make sure wol work.
- enable wol, ifdown network device, system shutdown, and make sure wol work.
- while [ 0 ] ; do ifdown eth0 ; ifup eth0 ; done
- while [ 0 ] ; do modprobe e1000 ; rmmod e1000 ; done
> @@ -1265,6 +1287,7 @@ static int e1000_open(struct net_device *netdev)
> goto err_setup_rx;
>
> e1000_power_up_phy(adapter);
> + e1000_reset(adapter);
>
> adapter->mng_vlan_id = E1000_MNG_VLAN_NONE;
> if ((hw->mng_cookie.status &
This code fix problem that e1000 driver doesn't work to auto-negotiation
once in a while.
Maybe, the cause is that set state to D0 just before it.
I found it by repeat of ifup and ifdown.
If you find out other points and any necessary tests from my patch,
please tell me. I will make sure them.
Thanks you.
Naohiro Ooiwa
^ 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