* Re: [PATCH RFC 3/3] virtio_net: limit xmit polling
From: Rusty Russell @ 2011-06-02 3:54 UTC (permalink / raw)
To: Michael S. Tsirkin, linux-kernel-u79uwXL29TY76Z2rM5mHXA
Cc: Krishna Kumar, Carsten Otte, lguest-uLR06cmDAlY/bJ5BZ2RsiQ,
Shirley Ma, kvm-u79uwXL29TY76Z2rM5mHXA,
linux-s390-u79uwXL29TY76Z2rM5mHXA, netdev-u79uwXL29TY76Z2rM5mHXA,
habanero-23VcF4HTsmIX0ybBhKVfKdBPR1lH4CV8, Heiko Carstens,
linux-kernel-u79uwXL29TY76Z2rM5mHXA,
virtualization-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA,
steved-r/Jw6+rmf7HQT0dZR+AlfA, Christian Borntraeger,
Tom Lendacky, Martin Schwidefsky, linux390-tA70FqPdS9bQT0dZR+AlfA
In-Reply-To: <1ec8eec325839ecf2eac9930a230361e7956047c.1306921434.git.mst-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
On Wed, 1 Jun 2011 12:50:03 +0300, "Michael S. Tsirkin" <mst-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org> wrote:
> Current code might introduce a lot of latency variation
> if there are many pending bufs at the time we
> attempt to transmit a new one. This is bad for
> real-time applications and can't be good for TCP either.
>
> Free up just enough to both clean up all buffers
> eventually and to be able to xmit the next packet.
OK, I found this quite confusing to read.
> - while ((skb = virtqueue_get_buf(vi->svq, &len)) != NULL) {
> + while ((r = virtqueue_min_capacity(vi->svq) < MAX_SKB_FRAGS + 2) ||
> + min_skbs-- > 0) {
> + skb = virtqueue_get_buf(vi->svq, &len);
> + if (unlikely(!skb))
> + break;
> pr_debug("Sent skb %p\n", skb);
> vi->dev->stats.tx_bytes += skb->len;
> vi->dev->stats.tx_packets++;
> dev_kfree_skb_any(skb);
> }
> + return r;
> }
Gah... what a horrible loop.
Basically, this patch makes hard-to-read code worse, and we should try
to make it better.
Currently, xmit *can* fail when an xmit interrupt wakes the queue, but
the packet(s) xmitted didn't free up enough space for the new packet.
With indirect buffers this only happens if we hit OOM (and thus go to
direct buffers).
We could solve this by only waking the queue in skb_xmit_done if the
capacity is >= 2 + MAX_SKB_FRAGS. But can we do it without a race?
If not, then I'd really prefer to see this, because I think it's clearer:
// Try to free 2 buffers for every 1 xmit, to stay ahead.
free_old_buffers(2)
if (!add_buf()) {
// Screw latency, free them all.
free_old_buffers(UINT_MAX)
// OK, this can happen if we are using direct buffers,
// and the xmit interrupt woke us but the packets
// xmitted were smaller than this one. Rare though.
if (!add_buf())
Whinge and stop queue, maybe loop.
}
if (capacity < 2 + MAX_SKB_FRAGS) {
// We don't have enough for the next packet? Try
// freeing more.
free_old_buffers(UINT_MAX);
if (capacity < 2 + MAX_SKB_FRAGS) {
Stop queue, maybe loop.
}
The current code makes my head hurt :(
Thoughts?
Rusty.
^ permalink raw reply
* Re: [PATCH] vlan: Fix the b0rked ingress VLAN_FLAG_REORDER_HDR check.
From: David Miller @ 2011-06-02 3:59 UTC (permalink / raw)
To: ebiederm
Cc: shemminger, greearb, nicolas.2p.debian, jpirko, xiaosuo, netdev,
kaber, fubar, eric.dumazet, andy, jesse
In-Reply-To: <m1oc2spmyt.fsf@fess.ebiederm.org>
From: ebiederm@xmission.com (Eric W. Biederman)
Date: Tue, 24 May 2011 00:38:34 -0700
> Would you be happier if the pkt_type check was moved earlier in the
> function like say:
Yes, I like this best because it's fully consistent and it fixes
the bug.
^ permalink raw reply
* Re: Bad behaviour when unintentionally mixing ipv4 and ipv6 addresses
From: David Miller @ 2011-06-02 4:03 UTC (permalink / raw)
To: meissner; +Cc: netdev, max
In-Reply-To: <20110525155918.GA27869@suse.de>
From: Marcus Meissner <meissner@suse.de>
Date: Wed, 25 May 2011 17:59:18 +0200
> @@ -465,6 +465,9 @@ int inet_bind(struct socket *sock, struct sockaddr *uaddr, int addr_len)
> if (addr_len < sizeof(struct sockaddr_in))
> goto out;
>
> + if (addr->sin_family != AF_INET)
> + goto out;
> +
> chk_addr_ret = inet_addr_type(sock_net(sk), addr->sin_addr.s_addr);
Since we haven't been validating the sin_family field for 18+ years, the
chance to break some applications is very real.
But I think it's more important to fix this (and force any broken apps
to set sin_family correctly). So I will apply this, thanks.
^ permalink raw reply
* Re: [PATCH] vlan: fix typo in vlan_dev_hard_start_xmit()
From: David Miller @ 2011-06-02 4:09 UTC (permalink / raw)
To: eric.dumazet; +Cc: yjwei, netdev
In-Reply-To: <1306940827.2890.0.camel@edumazet-laptop>
From: Eric Dumazet <eric.dumazet@gmail.com>
Date: Wed, 01 Jun 2011 17:07:07 +0200
> Le mercredi 01 juin 2011 à 16:53 +0800, Wei Yongjun a écrit :
>> commit 4af429d29b341bb1735f04c2fb960178ed5d52e7 (vlan: lockless
>> transmit path) have a typo in vlan_dev_hard_start_xmit(), using
>> u64_stats_update_begin() to end the stat update, it should be
>> u64_stats_update_end().
>>
>> Signed-off-by: Wei Yongjun <yjwei@cn.fujitsu.com>
>
> Good catch, Thanks !
>
> Acked-by: Eric Dumazet <eric.dumazet@gmail.com>
Applied and queued up for -stable.
Thanks!
^ permalink raw reply
* Re: [PATCH] usbnet/cdc_ncm: add missing .reset_resume hook
From: David Miller @ 2011-06-02 4:11 UTC (permalink / raw)
To: metze; +Cc: oliver, gregkh, linux-usb, netdev, linux-kernel
In-Reply-To: <1306929701-22861-2-git-send-email-metze@samba.org>
From: Stefan Metzmacher <metze@samba.org>
Date: Wed, 1 Jun 2011 14:01:41 +0200
> This avoids messages like this after suspend:
>
> cdc_ncm 2-1.4:1.6: no reset_resume for driver cdc_ncm?
> cdc_ncm 2-1.4:1.7: no reset_resume for driver cdc_ncm?
> cdc_ncm 2-1.4:1.6: usb0: unregister 'cdc_ncm' usb-0000:00:1d.0-1.4, CDC NCM
>
> This is important for the Ericsson F5521gw GSM/UMTS modem.
> Otherwise modemmanager looses the fact that the cdc_ncm and cdc_acm devices
> belong together.
>
> The cdc_ether module does the same.
>
> Signed-off-by: Stefan Metzmacher <metze@samba.org>
Applied and queued up for -stable, thanks.
^ permalink raw reply
* Re: [PATCH] caif: Fix race when conditionally taking rtnl lock
From: David Miller @ 2011-06-02 4:14 UTC (permalink / raw)
To: sjur.brandeland; +Cc: netdev
In-Reply-To: <1306925737-1782-1-git-send-email-sjur.brandeland@stericsson.com>
From: Sjur Brændeland <sjur.brandeland@stericsson.com>
Date: Wed, 1 Jun 2011 12:55:37 +0200
> Take the RTNL lock unconditionally when calling dev_close.
> Taking the lock conditionally may cause race conditions.
>
> Signed-off-by: Sjur Brændeland <sjur.brandeland@stericsson.com>
Applied, thanks.
^ permalink raw reply
* Re: [PATCHv3] caif: Add CAIF HSI Link layer driver
From: David Miller @ 2011-06-02 4:17 UTC (permalink / raw)
To: sjur.brandeland; +Cc: netdev, Dmitry.Tarnyagin
In-Reply-To: <1306934958-5666-1-git-send-email-sjur.brandeland@stericsson.com>
From: Sjur Brændeland <sjur.brandeland@stericsson.com>
Date: Wed, 1 Jun 2011 15:29:18 +0200
> From: Dmitry.Tarnyagin <Dmitry.Tarnyagin@stericsson.com>
>
> This patch introduces the CAIF HSI Protocol Driver for the
> CAIF Link Layer.
>
> This driver implements a platform driver to accommodate for a
> platform specific HSI devices. A general platform driver is not
> possible as there are no HSI side Kernel API defined.
>
> Signed-off-by: Sjur Brændeland <sjur.brandeland@stericsson.com>
Applied to net-next-2.6
^ permalink raw reply
* Re: [PATCH v3] af-packet: Add flag to distinguish VID 0 from no-vlan.
From: David Miller @ 2011-06-02 4:18 UTC (permalink / raw)
To: greearb; +Cc: netdev
In-Reply-To: <1306946950-29360-1-git-send-email-greearb@candelatech.com>
From: greearb@candelatech.com
Date: Wed, 1 Jun 2011 09:49:10 -0700
> From: Ben Greear <greearb@candelatech.com>
>
> Currently, user-space cannot determine if a 0 tcp_vlan_tci
> means there is no VLAN tag or the VLAN ID was zero.
>
> Add flag to make this explicit. User-space can check for
> TP_STATUS_VLAN_VALID || tp_vlan_tci > 0, which will be backwards
> compatible. Older could would have just checked for tp_vlan_tci,
> so it will work no worse than before.
>
> Signed-off-by: Ben Greear <greearb@candelatech.com>
Applied.
^ permalink raw reply
* Re: [PATCH 7/10] drivers/net/davinci_emac.c: add missing clk_put
From: David Miller @ 2011-06-02 4:20 UTC (permalink / raw)
To: khilman; +Cc: julia, kernel-janitors, cyril, srk, weil, netdev, linux-kernel
In-Reply-To: <1306957764.26236.37.camel@vence>
From: Kevin Hilman <khilman@ti.com>
Date: Wed, 01 Jun 2011 12:49:24 -0700
> On Wed, 2011-06-01 at 19:10 +0200, Julia Lawall wrote:
>> From: Julia Lawall <julia@diku.dk>
>>
>> Go to existing error handling code at the end of the function that calls
>> clk_put.
>>
>> A simplified version of the semantic match that finds this problem is as
>> follows: (http://coccinelle.lip6.fr/)
>>
>> // <smpl>
>> @r exists@
>> expression e1,e2;
>> statement S;
>> @@
>>
>> e1 = clk_get@p1(...);
>> ... when != e1 = e2
>> when != clk_put(e1)
>> when any
>> if (...) { ... when != clk_put(e1)
>> when != if (...) { ... clk_put(e1) ... }
>> * return@p3 ...;
>> } else S
>> // </smpl>
>>
>> Signed-off-by: Julia Lawall <julia@diku.dk>
>
> Acked-by: Kevin Hilman <khilman@ti.com>
Applied, thanks.
^ permalink raw reply
* Re: [PATCH] net: dm9000: Get the chip in a known good state before enabling interrupts
From: David Miller @ 2011-06-02 4:22 UTC (permalink / raw)
To: broonie; +Cc: ben-linux, netdev
In-Reply-To: <1306959489-28865-1-git-send-email-broonie@opensource.wolfsonmicro.com>
From: Mark Brown <broonie@opensource.wolfsonmicro.com>
Date: Wed, 1 Jun 2011 21:18:09 +0100
> Currently the DM9000 driver requests the primary interrupt before it
> resets the chip and puts it into a known good state. This means that if
> the chip is asserting interrupt for some reason we can end up with a
> screaming IRQ that the interrupt handler is unable to deal with. Avoid
> this by only requesting the interrupt after we've reset the chip so we
> know what state it's in.
>
> This started manifesting itself on one of my boards in the past month or
> so, I suspect as a result of some core infrastructure changes removing
> some form of mitigation against bad behaviour here, even when things boot
> it seems that the new code brings the interface up more quickly.
>
> Signed-off-by: Mark Brown <broonie@opensource.wolfsonmicro.com>
Applied, thanks.
^ permalink raw reply
* Re: [PATCH 4/10] drivers/net/can/flexcan.c: add missing clk_put
From: Julia Lawall @ 2011-06-02 5:48 UTC (permalink / raw)
To: David Miller
Cc: netdev-u79uwXL29TY76Z2rM5mHXA,
kernel-janitors-u79uwXL29TY76Z2rM5mHXA,
linux-kernel-u79uwXL29TY76Z2rM5mHXA,
socketcan-core-0fE9KPoRgkgATYTw5x5z8w, joe-6d6DIl74uiNBDgjK7y7TUQ,
wg-5Yr1BZd7O62+XT7JhA+gdA, wharms-fPG8STNUNVg
In-Reply-To: <20110601.131136.391831410750786951.davem-fT/PcQaiUtIeIZ0/mPfg9Q@public.gmane.org>
From: Julia Lawall <julia-dAYI7NvHqcQ@public.gmane.org>
The failed_get label is used after the call to clk_get has succeeded, so it
should be moved up above the call to clk_put.
The failed_req labels doesn't do anything different than failed_get, so
delete it.
A simplified version of the semantic match that finds this problem is as
follows: (http://coccinelle.lip6.fr/)
// <smpl>
@r exists@
expression e1,e2;
statement S;
@@
e1 = clk_get@p1(...);
... when != e1 = e2
when != clk_put(e1)
when any
if (...) { ... when != clk_put(e1)
when != if (...) { ... clk_put(e1) ... }
* return@p3 ...;
} else S
// </smpl>
Signed-off-by: Julia Lawall <julia-dAYI7NvHqcQ@public.gmane.org>
---
drivers/net/can/flexcan.c | 5 ++---
1 file changed, 2 insertions(+), 3 deletions(-)
diff --git a/drivers/net/can/flexcan.c b/drivers/net/can/flexcan.c
index d499056..1767811 100644
--- a/drivers/net/can/flexcan.c
+++ b/drivers/net/can/flexcan.c
@@ -923,7 +923,7 @@ static int __devinit flexcan_probe(struct platform_device *pdev)
mem_size = resource_size(mem);
if (!request_mem_region(mem->start, mem_size, pdev->name)) {
err = -EBUSY;
- goto failed_req;
+ goto failed_get;
}
base = ioremap(mem->start, mem_size);
@@ -977,9 +977,8 @@ static int __devinit flexcan_probe(struct platform_device *pdev)
iounmap(base);
failed_map:
release_mem_region(mem->start, mem_size);
- failed_req:
- clk_put(clk);
failed_get:
+ clk_put(clk);
failed_clock:
return err;
}
^ permalink raw reply related
* Re: [PATCH 4/10] drivers/net/can/flexcan.c: add missing clk_put
From: David Miller @ 2011-06-02 7:10 UTC (permalink / raw)
To: julia-dAYI7NvHqcQ
Cc: netdev-u79uwXL29TY76Z2rM5mHXA,
kernel-janitors-u79uwXL29TY76Z2rM5mHXA,
linux-kernel-u79uwXL29TY76Z2rM5mHXA,
socketcan-core-0fE9KPoRgkgATYTw5x5z8w, joe-6d6DIl74uiNBDgjK7y7TUQ,
wg-5Yr1BZd7O62+XT7JhA+gdA, wharms-fPG8STNUNVg
In-Reply-To: <Pine.LNX.4.64.1106020748020.22932-QfmoRoYWmW9knbxzx/v8hQ@public.gmane.org>
From: Julia Lawall <julia-dAYI7NvHqcQ@public.gmane.org>
Date: Thu, 2 Jun 2011 07:48:50 +0200 (CEST)
> From: Julia Lawall <julia-dAYI7NvHqcQ@public.gmane.org>
>
> The failed_get label is used after the call to clk_get has succeeded, so it
> should be moved up above the call to clk_put.
>
> The failed_req labels doesn't do anything different than failed_get, so
> delete it.
>
> A simplified version of the semantic match that finds this problem is as
> follows: (http://coccinelle.lip6.fr/)
...
> Signed-off-by: Julia Lawall <julia-dAYI7NvHqcQ@public.gmane.org>
Applied.
^ permalink raw reply
* Re: [PATCH 1/6 v9] sctp: Add ADD/DEL ASCONF handling at the receiver.
From: Wei Yongjun @ 2011-06-02 7:18 UTC (permalink / raw)
To: Michio Honda; +Cc: netdev, David Miller
In-Reply-To: <506FB213-6C73-4F00-95B2-E20976F2064E@sfc.wide.ad.jp>
> >From fb3b20d29f89c1570d44df7fdc20fe0ed12a71f0 Mon Sep 17 00:00:00 2001
> From: Michio Honda <micchie@sfc.wide.ad.jp>
> Date: Tue, 26 Apr 2011 17:37:02 +0900
> Subject: [PATCH 1/6 v9] sctp: Add ADD/DEL ASCONF handling at the receiver.
>
> This patch fixes the problem that the original code cannot delete
> the remote address where the corresponding transport is currently
> directed, even when the ASCONF is sent from the other address (this
> situation happens when the single-homed sender transmits ASCONF
> with ADD and DEL.)
>
> Signed-off-by: Michio Honda <micchie@sfc.wide.ad.jp>
> Signed-off-by: YOSHIFUJI Hideaki <yoshfuji@linux-ipv6.org>
>
Acked-by: Wei Yongjun <yjwei@cn.fujitsu.com>
^ permalink raw reply
* Re: [PATCH 2/6 v9] sctp: Allow regular C expression in 4th argument for SCTP_DEBUG_PRINTK_IPADDR macro.
From: Wei Yongjun @ 2011-06-02 7:18 UTC (permalink / raw)
To: Michio Honda; +Cc: netdev, David Miller
In-Reply-To: <9FF73D2B-4FBE-4BE6-9E84-3B7A4E0CA622@sfc.wide.ad.jp>
> >From 48390815413c0dac1b39868dd7e874b351848322 Mon Sep 17 00:00:00 2001
> From: YOSHIFUJI Hideaki <yoshfuji@linux-ipv6.org>
> Date: Tue, 26 Apr 2011 19:23:24 +0900
> Subject: [PATCH 2/6 v9] sctp: Allow regular C expression in 4th argument for SCTP_DEBUG_PRINTK_IPADDR macro.
>
> Signed-off-by: YOSHIFUJI Hideaki <yoshfuji@linux-ipv6.org>
> ---
>
Acked-by: Wei Yongjun <yjwei@cn.fujitsu.com>
^ permalink raw reply
* Re: [PATCH 3/6 v9] sctp: Add Auto-ASCONF support (core).
From: Wei Yongjun @ 2011-06-02 7:19 UTC (permalink / raw)
To: Michio Honda; +Cc: netdev, David Miller
In-Reply-To: <0750E26B-3F0C-4280-AC8D-DECFDA5C48E1@sfc.wide.ad.jp>
> >From fb6a7bb57810aff5ea542a24119b857a6843ac88 Mon Sep 17 00:00:00 2001
> From: Michio Honda <micchie@sfc.wide.ad.jp>
> Date: Tue, 26 Apr 2011 19:32:51 +0900
> Subject: [PATCH 3/6 v9] sctp: Add Auto-ASCONF support (core).
>
> SCTP reconfigure the IP addresses in the association by using
> ASCONF chunks as mentioned in RFC5061. For example, we can
> start to use the newly configured IP address in the existing
> association. This patch implements automatic ASCONF operation
> in the SCTP stack with address events in the host computer,
> which is called auto_asconf.
>
> Signed-off-by: Michio Honda <micchie@sfc.wide.ad.jp>
> Signed-off-by: YOSHIFUJI Hideaki <yoshfuji@linux-ipv6.org>
> ---
>
Acked-by: Wei Yongjun <yjwei@cn.fujitsu.com>
^ permalink raw reply
* Re: [PATCH 4/6 v9] sctp: Add sysctl support for Auto-ASCONF.
From: Wei Yongjun @ 2011-06-02 7:19 UTC (permalink / raw)
To: Michio Honda; +Cc: netdev, David Miller
In-Reply-To: <B723DBE5-24B2-4F5D-873A-4AB68F7B032C@sfc.wide.ad.jp>
> >From 6d5abc7fa954ffa8808f4dd78636984c0cd57ce8 Mon Sep 17 00:00:00 2001
> From: Michio Honda <micchie@sfc.wide.ad.jp>
> Date: Tue, 26 Apr 2011 17:36:05 +0900
> Subject: [PATCH 4/6 v9] sctp: Add sysctl support for Auto-ASCONF.
>
> This patch allows the system administrator to change default
> Auto-ASCONF on/off behavior via an sysctl value.
>
> Signed-off-by: Michio Honda <micchie@sfc.wide.ad.jp>
> Signed-off-by: YOSHIFUJI Hideaki <yoshfuji@linux-ipv6.org>
> ---
> net/sctp/sysctl.c | 7 +++++++
> 1 files changed, 7 insertions(+), 0 deletions(-)
>
>
Acked-by: Wei Yongjun <yjwei@cn.fujitsu.com>
^ permalink raw reply
* Re: [PATCH 5/6 v9] sctp: Add socket option operation for Auto-ASCONF.
From: Wei Yongjun @ 2011-06-02 7:20 UTC (permalink / raw)
To: Michio Honda; +Cc: netdev, David Miller
In-Reply-To: <590D86AF-4768-4536-88AA-6EB8E5FAF118@sfc.wide.ad.jp>
> >From cd8da98334d825bc3eff55b7b5b6537283e850ef Mon Sep 17 00:00:00 2001
> From: Michio Honda <micchie@sfc.wide.ad.jp>
> Date: Tue, 26 Apr 2011 20:16:31 +0900
> Subject: [PATCH 5/6 v9] sctp: Add socket option operation for Auto-ASCONF.
>
> This patch allows the application to operate Auto-ASCONF on/off
> behavior via setsockopt() and getsockopt().
>
> Signed-off-by: Michio Honda <micchie@sfc.wide.ad.jp>
> Signed-off-by: YOSHIFUJI Hideaki <yoshfuji@linux-ipv6.org>
> ---
>
Acked-by: Wei Yongjun <yjwei@cn.fujitsu.com>
^ permalink raw reply
* Re: [PATCH 6/6 v9] sctp: Add ASCONF operation on the single-homed host
From: Wei Yongjun @ 2011-06-02 7:20 UTC (permalink / raw)
To: Michio Honda; +Cc: netdev, David Miller
In-Reply-To: <99BEF1C4-C638-4007-A538-E43B1463F564@sfc.wide.ad.jp>
> >From 907306fafe1ebdde2ad42aefbc6e45821b448cbf Mon Sep 17 00:00:00 2001
> From: Michio Honda <micchie@sfc.wide.ad.jp>
> Date: Tue, 26 Apr 2011 20:19:36 +0900
> Subject: [PATCH 6/6 v9] sctp: Add ASCONF operation on the single-homed host
>
> In this case, the SCTP association transmits an ASCONF packet
> including addition of the new IP address and deletion of the old
> address. This patch implements this functionality.
> In this case, the ASCONF chunk is added to the beginning of the
> queue, because the other chunks cannot be transmitted in this state.
>
> Signed-off-by: Michio Honda <micchie@sfc.wide.ad.jp>
> Signed-off-by: YOSHIFUJI Hideaki <yoshfuji@linux-ipv6.org>
> ---
>
Acked-by: Wei Yongjun <yjwei@cn.fujitsu.com>
^ permalink raw reply
* Re: [PATCH 0/6 v9] sctp: Auto-ASCONF patch series
From: David Miller @ 2011-06-02 9:06 UTC (permalink / raw)
To: micchie; +Cc: netdev, yjwei
In-Reply-To: <4C8C57AE-E872-46F5-806D-DD2699F424C6@sfc.wide.ad.jp>
From: Michio Honda <micchie@sfc.wide.ad.jp>
Date: Thu, 2 Jun 2011 12:00:49 +0900
> Series of 6 patches to support auto_asconf and the other related functionalities that auto_asconf relies on
All applied, thank you.
^ permalink raw reply
* [PATCH 1/5] net: ep93xx_eth: pass struct device to DMA API functions
From: Mika Westerberg @ 2011-06-02 9:38 UTC (permalink / raw)
To: netdev; +Cc: linux-arm-kernel, kernel, hsweeten, ryan, Mika Westerberg
We shouldn't use NULL for any DMA API functions, unless we are dealing with
ISA or EISA device. So pass correct struct dev pointer to these functions.
Signed-off-by: Mika Westerberg <mika.westerberg@iki.fi>
---
drivers/net/arm/ep93xx_eth.c | 26 ++++++++++++++++----------
1 files changed, 16 insertions(+), 10 deletions(-)
diff --git a/drivers/net/arm/ep93xx_eth.c b/drivers/net/arm/ep93xx_eth.c
index 5a77001..8779d3b 100644
--- a/drivers/net/arm/ep93xx_eth.c
+++ b/drivers/net/arm/ep93xx_eth.c
@@ -159,6 +159,8 @@ struct ep93xx_priv
void __iomem *base_addr;
int irq;
+ struct device *dma_dev;
+
struct ep93xx_descs *descs;
dma_addr_t descs_dma_addr;
@@ -284,7 +286,8 @@ static int ep93xx_rx(struct net_device *dev, int processed, int budget)
skb = dev_alloc_skb(length + 2);
if (likely(skb != NULL)) {
skb_reserve(skb, 2);
- dma_sync_single_for_cpu(NULL, ep->descs->rdesc[entry].buf_addr,
+ dma_sync_single_for_cpu(ep->dma_dev,
+ ep->descs->rdesc[entry].buf_addr,
length, DMA_FROM_DEVICE);
skb_copy_to_linear_data(skb, ep->rx_buf[entry], length);
skb_put(skb, length);
@@ -362,7 +365,7 @@ static int ep93xx_xmit(struct sk_buff *skb, struct net_device *dev)
ep->descs->tdesc[entry].tdesc1 =
TDESC1_EOF | (entry << 16) | (skb->len & 0xfff);
skb_copy_and_csum_dev(skb, ep->tx_buf[entry]);
- dma_sync_single_for_cpu(NULL, ep->descs->tdesc[entry].buf_addr,
+ dma_sync_single_for_cpu(ep->dma_dev, ep->descs->tdesc[entry].buf_addr,
skb->len, DMA_TO_DEVICE);
dev_kfree_skb(skb);
@@ -457,6 +460,7 @@ static irqreturn_t ep93xx_irq(int irq, void *dev_id)
static void ep93xx_free_buffers(struct ep93xx_priv *ep)
{
+ struct device *dev = ep->dma_dev;
int i;
for (i = 0; i < RX_QUEUE_ENTRIES; i += 2) {
@@ -464,7 +468,7 @@ static void ep93xx_free_buffers(struct ep93xx_priv *ep)
d = ep->descs->rdesc[i].buf_addr;
if (d)
- dma_unmap_single(NULL, d, PAGE_SIZE, DMA_FROM_DEVICE);
+ dma_unmap_single(dev, d, PAGE_SIZE, DMA_FROM_DEVICE);
if (ep->rx_buf[i] != NULL)
free_page((unsigned long)ep->rx_buf[i]);
@@ -475,13 +479,13 @@ static void ep93xx_free_buffers(struct ep93xx_priv *ep)
d = ep->descs->tdesc[i].buf_addr;
if (d)
- dma_unmap_single(NULL, d, PAGE_SIZE, DMA_TO_DEVICE);
+ dma_unmap_single(dev, d, PAGE_SIZE, DMA_TO_DEVICE);
if (ep->tx_buf[i] != NULL)
free_page((unsigned long)ep->tx_buf[i]);
}
- dma_free_coherent(NULL, sizeof(struct ep93xx_descs), ep->descs,
+ dma_free_coherent(dev, sizeof(struct ep93xx_descs), ep->descs,
ep->descs_dma_addr);
}
@@ -491,9 +495,10 @@ static void ep93xx_free_buffers(struct ep93xx_priv *ep)
*/
static int ep93xx_alloc_buffers(struct ep93xx_priv *ep)
{
+ struct device *dev = ep->dma_dev;
int i;
- ep->descs = dma_alloc_coherent(NULL, sizeof(struct ep93xx_descs),
+ ep->descs = dma_alloc_coherent(dev, sizeof(struct ep93xx_descs),
&ep->descs_dma_addr, GFP_KERNEL | GFP_DMA);
if (ep->descs == NULL)
return 1;
@@ -506,8 +511,8 @@ static int ep93xx_alloc_buffers(struct ep93xx_priv *ep)
if (page == NULL)
goto err;
- d = dma_map_single(NULL, page, PAGE_SIZE, DMA_FROM_DEVICE);
- if (dma_mapping_error(NULL, d)) {
+ d = dma_map_single(dev, page, PAGE_SIZE, DMA_FROM_DEVICE);
+ if (dma_mapping_error(dev, d)) {
free_page((unsigned long)page);
goto err;
}
@@ -529,8 +534,8 @@ static int ep93xx_alloc_buffers(struct ep93xx_priv *ep)
if (page == NULL)
goto err;
- d = dma_map_single(NULL, page, PAGE_SIZE, DMA_TO_DEVICE);
- if (dma_mapping_error(NULL, d)) {
+ d = dma_map_single(dev, page, PAGE_SIZE, DMA_TO_DEVICE);
+ if (dma_mapping_error(dev, d)) {
free_page((unsigned long)page);
goto err;
}
@@ -829,6 +834,7 @@ static int ep93xx_eth_probe(struct platform_device *pdev)
}
ep = netdev_priv(dev);
ep->dev = dev;
+ ep->dma_dev = &pdev->dev;
netif_napi_add(dev, &ep->napi, ep93xx_poll, 64);
platform_set_drvdata(pdev, dev);
--
1.7.4.4
^ permalink raw reply related
* [PATCH 2/5] net: ep93xx_eth: allocate buffers using kmalloc()
From: Mika Westerberg @ 2011-06-02 9:38 UTC (permalink / raw)
To: netdev; +Cc: linux-arm-kernel, kernel, hsweeten, ryan, Mika Westerberg
In-Reply-To: <89df467f9811b4f869513de3ada90ce7de74c6d1.1307006502.git.mika.westerberg@iki.fi>
We can use simply kmalloc() to allocate the buffers. This also simplifies the
code and allows us to perform DMA sync operations more easily.
Memory is allocated with only GFP_KERNEL since there are no DMA allocation
restrictions on this platform.
Signed-off-by: Mika Westerberg <mika.westerberg@iki.fi>
---
drivers/net/arm/ep93xx_eth.c | 51 ++++++++++++++++-------------------------
1 files changed, 20 insertions(+), 31 deletions(-)
diff --git a/drivers/net/arm/ep93xx_eth.c b/drivers/net/arm/ep93xx_eth.c
index 8779d3b..0c9df11 100644
--- a/drivers/net/arm/ep93xx_eth.c
+++ b/drivers/net/arm/ep93xx_eth.c
@@ -463,36 +463,32 @@ static void ep93xx_free_buffers(struct ep93xx_priv *ep)
struct device *dev = ep->dma_dev;
int i;
- for (i = 0; i < RX_QUEUE_ENTRIES; i += 2) {
+ for (i = 0; i < RX_QUEUE_ENTRIES; i++) {
dma_addr_t d;
d = ep->descs->rdesc[i].buf_addr;
if (d)
- dma_unmap_single(dev, d, PAGE_SIZE, DMA_FROM_DEVICE);
+ dma_unmap_single(dev, d, PKT_BUF_SIZE, DMA_FROM_DEVICE);
if (ep->rx_buf[i] != NULL)
- free_page((unsigned long)ep->rx_buf[i]);
+ kfree(ep->rx_buf[i]);
}
- for (i = 0; i < TX_QUEUE_ENTRIES; i += 2) {
+ for (i = 0; i < TX_QUEUE_ENTRIES; i++) {
dma_addr_t d;
d = ep->descs->tdesc[i].buf_addr;
if (d)
- dma_unmap_single(dev, d, PAGE_SIZE, DMA_TO_DEVICE);
+ dma_unmap_single(dev, d, PKT_BUF_SIZE, DMA_TO_DEVICE);
if (ep->tx_buf[i] != NULL)
- free_page((unsigned long)ep->tx_buf[i]);
+ kfree(ep->tx_buf[i]);
}
dma_free_coherent(dev, sizeof(struct ep93xx_descs), ep->descs,
ep->descs_dma_addr);
}
-/*
- * The hardware enforces a sub-2K maximum packet size, so we put
- * two buffers on every hardware page.
- */
static int ep93xx_alloc_buffers(struct ep93xx_priv *ep)
{
struct device *dev = ep->dma_dev;
@@ -503,48 +499,41 @@ static int ep93xx_alloc_buffers(struct ep93xx_priv *ep)
if (ep->descs == NULL)
return 1;
- for (i = 0; i < RX_QUEUE_ENTRIES; i += 2) {
- void *page;
+ for (i = 0; i < RX_QUEUE_ENTRIES; i++) {
+ void *buf;
dma_addr_t d;
- page = (void *)__get_free_page(GFP_KERNEL | GFP_DMA);
- if (page == NULL)
+ buf = kmalloc(PKT_BUF_SIZE, GFP_KERNEL);
+ if (buf == NULL)
goto err;
- d = dma_map_single(dev, page, PAGE_SIZE, DMA_FROM_DEVICE);
+ d = dma_map_single(dev, buf, PKT_BUF_SIZE, DMA_FROM_DEVICE);
if (dma_mapping_error(dev, d)) {
- free_page((unsigned long)page);
+ kfree(buf);
goto err;
}
- ep->rx_buf[i] = page;
+ ep->rx_buf[i] = buf;
ep->descs->rdesc[i].buf_addr = d;
ep->descs->rdesc[i].rdesc1 = (i << 16) | PKT_BUF_SIZE;
-
- ep->rx_buf[i + 1] = page + PKT_BUF_SIZE;
- ep->descs->rdesc[i + 1].buf_addr = d + PKT_BUF_SIZE;
- ep->descs->rdesc[i + 1].rdesc1 = ((i + 1) << 16) | PKT_BUF_SIZE;
}
- for (i = 0; i < TX_QUEUE_ENTRIES; i += 2) {
- void *page;
+ for (i = 0; i < TX_QUEUE_ENTRIES; i++) {
+ void *buf;
dma_addr_t d;
- page = (void *)__get_free_page(GFP_KERNEL | GFP_DMA);
- if (page == NULL)
+ buf = kmalloc(PKT_BUF_SIZE, GFP_KERNEL);
+ if (buf == NULL)
goto err;
- d = dma_map_single(dev, page, PAGE_SIZE, DMA_TO_DEVICE);
+ d = dma_map_single(dev, buf, PKT_BUF_SIZE, DMA_TO_DEVICE);
if (dma_mapping_error(dev, d)) {
- free_page((unsigned long)page);
+ kfree(buf);
goto err;
}
- ep->tx_buf[i] = page;
+ ep->tx_buf[i] = buf;
ep->descs->tdesc[i].buf_addr = d;
-
- ep->tx_buf[i + 1] = page + PKT_BUF_SIZE;
- ep->descs->tdesc[i + 1].buf_addr = d + PKT_BUF_SIZE;
}
return 0;
--
1.7.4.4
^ permalink raw reply related
* [PATCH 3/5] net: ep93xx_eth: drop GFP_DMA from call to dma_alloc_coherent()
From: Mika Westerberg @ 2011-06-02 9:38 UTC (permalink / raw)
To: netdev; +Cc: linux-arm-kernel, kernel, hsweeten, ryan, Mika Westerberg
In-Reply-To: <89df467f9811b4f869513de3ada90ce7de74c6d1.1307006502.git.mika.westerberg@iki.fi>
Commit a197b59ae6e8 (mm: fail GFP_DMA allocations when ZONE_DMA is not
configured) made page allocator to return NULL if GFP_DMA is set but
CONFIG_ZONE_DMA is disabled.
This causes ep93xx_eth to fail:
WARNING: at mm/page_alloc.c:2251 __alloc_pages_nodemask+0x11c/0x638()
Modules linked in:
[<c0035498>] (unwind_backtrace+0x0/0xf4) from [<c0043da4>] (warn_slowpath_common+0x48/0x60)
[<c0043da4>] (warn_slowpath_common+0x48/0x60) from [<c0043dd8>] (warn_slowpath_null+0x1c/0x24)
[<c0043dd8>] (warn_slowpath_null+0x1c/0x24) from [<c0083b6c>] (__alloc_pages_nodemask+0x11c/0x638)
[<c0083b6c>] (__alloc_pages_nodemask+0x11c/0x638) from [<c00366fc>] (__dma_alloc+0x8c/0x3ec)
[<c00366fc>] (__dma_alloc+0x8c/0x3ec) from [<c0036adc>] (dma_alloc_coherent+0x54/0x60)
[<c0036adc>] (dma_alloc_coherent+0x54/0x60) from [<c0227808>] (ep93xx_open+0x20/0x864)
[<c0227808>] (ep93xx_open+0x20/0x864) from [<c0283144>] (__dev_open+0xb8/0x108)
[<c0283144>] (__dev_open+0xb8/0x108) from [<c0280528>] (__dev_change_flags+0x70/0x128)
[<c0280528>] (__dev_change_flags+0x70/0x128) from [<c0283054>] (dev_change_flags+0x10/0x48)
[<c0283054>] (dev_change_flags+0x10/0x48) from [<c001a720>] (ip_auto_config+0x190/0xf68)
[<c001a720>] (ip_auto_config+0x190/0xf68) from [<c00233b0>] (do_one_initcall+0x34/0x18c)
[<c00233b0>] (do_one_initcall+0x34/0x18c) from [<c0008400>] (kernel_init+0x94/0x134)
[<c0008400>] (kernel_init+0x94/0x134) from [<c0030858>] (kernel_thread_exit+0x0/0x8)
Since there is no restrictions for DMA on ep93xx, we can fix this by just
removing the GFP_DMA flag from the call.
Signed-off-by: Mika Westerberg <mika.westerberg@iki.fi>
---
drivers/net/arm/ep93xx_eth.c | 2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/drivers/net/arm/ep93xx_eth.c b/drivers/net/arm/ep93xx_eth.c
index 0c9df11..56b51a1 100644
--- a/drivers/net/arm/ep93xx_eth.c
+++ b/drivers/net/arm/ep93xx_eth.c
@@ -495,7 +495,7 @@ static int ep93xx_alloc_buffers(struct ep93xx_priv *ep)
int i;
ep->descs = dma_alloc_coherent(dev, sizeof(struct ep93xx_descs),
- &ep->descs_dma_addr, GFP_KERNEL | GFP_DMA);
+ &ep->descs_dma_addr, GFP_KERNEL);
if (ep->descs == NULL)
return 1;
--
1.7.4.4
^ permalink raw reply related
* [PATCH 4/5] net: ep93xx_eth: fix DMA API violations
From: Mika Westerberg @ 2011-06-02 9:38 UTC (permalink / raw)
To: netdev; +Cc: linux-arm-kernel, kernel, hsweeten, ryan, Mika Westerberg
In-Reply-To: <89df467f9811b4f869513de3ada90ce7de74c6d1.1307006502.git.mika.westerberg@iki.fi>
Russell King said:
>
> So, to summarize what its doing:
>
> 1. It allocates buffers for rx and tx.
> 2. It maps them with dma_map_single().
> This transfers ownership of the buffer to the DMA device.
> 3. In ep93xx_xmit,
> 3a. It copies the data into the buffer with skb_copy_and_csum_dev()
> This violates the DMA buffer ownership rules - the CPU should
> not be writing to this buffer while it is (in principle) owned
> by the DMA device.
> 3b. It then calls dma_sync_single_for_cpu() for the buffer.
> This transfers ownership of the buffer to the CPU, which surely
> is the wrong direction.
> 4. In ep93xx_rx,
> 4a. It calls dma_sync_single_for_cpu() for the buffer.
> This at least transfers the DMA buffer ownership to the CPU
> before the CPU reads the buffer
> 4b. It then uses skb_copy_to_linear_data() to copy the data out.
> At no point does it transfer ownership back to the DMA device.
> 5. When the driver is removed, it dma_unmap_single()'s the buffer.
> This transfers ownership of the buffer to the CPU.
> 6. It frees the buffer.
>
> While it may work on ep93xx, it's not respecting the DMA API rules,
> and with DMA debugging enabled it will probably encounter quite a few
> warnings.
This patch fixes these violations.
Signed-off-by: Mika Westerberg <mika.westerberg@iki.fi>
---
drivers/net/arm/ep93xx_eth.c | 19 +++++++++++++------
1 files changed, 13 insertions(+), 6 deletions(-)
diff --git a/drivers/net/arm/ep93xx_eth.c b/drivers/net/arm/ep93xx_eth.c
index 56b51a1..c2a7847 100644
--- a/drivers/net/arm/ep93xx_eth.c
+++ b/drivers/net/arm/ep93xx_eth.c
@@ -285,11 +285,14 @@ static int ep93xx_rx(struct net_device *dev, int processed, int budget)
skb = dev_alloc_skb(length + 2);
if (likely(skb != NULL)) {
+ struct ep93xx_rdesc *rxd = &ep->descs->rdesc[entry];
skb_reserve(skb, 2);
- dma_sync_single_for_cpu(ep->dma_dev,
- ep->descs->rdesc[entry].buf_addr,
+
+ dma_sync_single_for_cpu(ep->dma_dev, rxd->buf_addr,
length, DMA_FROM_DEVICE);
skb_copy_to_linear_data(skb, ep->rx_buf[entry], length);
+ dma_sync_single_for_device(ep->dma_dev, rxd->buf_addr,
+ length, DMA_FROM_DEVICE);
skb_put(skb, length);
skb->protocol = eth_type_trans(skb, dev);
@@ -351,6 +354,7 @@ poll_some_more:
static int ep93xx_xmit(struct sk_buff *skb, struct net_device *dev)
{
struct ep93xx_priv *ep = netdev_priv(dev);
+ struct ep93xx_tdesc *txd;
int entry;
if (unlikely(skb->len > MAX_PKT_SIZE)) {
@@ -362,11 +366,14 @@ static int ep93xx_xmit(struct sk_buff *skb, struct net_device *dev)
entry = ep->tx_pointer;
ep->tx_pointer = (ep->tx_pointer + 1) & (TX_QUEUE_ENTRIES - 1);
- ep->descs->tdesc[entry].tdesc1 =
- TDESC1_EOF | (entry << 16) | (skb->len & 0xfff);
+ txd = &ep->descs->tdesc[entry];
+
+ txd->tdesc1 = TDESC1_EOF | (entry << 16) | (skb->len & 0xfff);
+ dma_sync_single_for_cpu(ep->dma_dev, txd->buf_addr, skb->len,
+ DMA_TO_DEVICE);
skb_copy_and_csum_dev(skb, ep->tx_buf[entry]);
- dma_sync_single_for_cpu(ep->dma_dev, ep->descs->tdesc[entry].buf_addr,
- skb->len, DMA_TO_DEVICE);
+ dma_sync_single_for_device(ep->dma_dev, txd->buf_addr, skb->len,
+ DMA_TO_DEVICE);
dev_kfree_skb(skb);
spin_lock_irq(&ep->tx_pending_lock);
--
1.7.4.4
^ permalink raw reply related
* [PATCH 5/5] ep93xx: set DMA masks for the ep93xx_eth
From: Mika Westerberg @ 2011-06-02 9:38 UTC (permalink / raw)
To: netdev; +Cc: linux-arm-kernel, kernel, hsweeten, ryan, Mika Westerberg
In-Reply-To: <89df467f9811b4f869513de3ada90ce7de74c6d1.1307006502.git.mika.westerberg@iki.fi>
As the driver is now passing platform device to the DMA mapping functions,
we should give it valid DMA masks.
Signed-off-by: Mika Westerberg <mika.westerberg@iki.fi>
---
arch/arm/mach-ep93xx/core.c | 6 +++++-
1 files changed, 5 insertions(+), 1 deletions(-)
diff --git a/arch/arm/mach-ep93xx/core.c b/arch/arm/mach-ep93xx/core.c
index 8207954..1d4b65f 100644
--- a/arch/arm/mach-ep93xx/core.c
+++ b/arch/arm/mach-ep93xx/core.c
@@ -402,11 +402,15 @@ static struct resource ep93xx_eth_resource[] = {
}
};
+static u64 ep93xx_eth_dma_mask = DMA_BIT_MASK(32);
+
static struct platform_device ep93xx_eth_device = {
.name = "ep93xx-eth",
.id = -1,
.dev = {
- .platform_data = &ep93xx_eth_data,
+ .platform_data = &ep93xx_eth_data,
+ .coherent_dma_mask = DMA_BIT_MASK(32),
+ .dma_mask = &ep93xx_eth_dma_mask,
},
.num_resources = ARRAY_SIZE(ep93xx_eth_resource),
.resource = ep93xx_eth_resource,
--
1.7.4.4
^ permalink raw reply related
* Re: [PATCH 1/5] net: ep93xx_eth: pass struct device to DMA API functions
From: Russell King - ARM Linux @ 2011-06-02 9:52 UTC (permalink / raw)
To: Mika Westerberg; +Cc: netdev, hsweeten, ryan, kernel, linux-arm-kernel
In-Reply-To: <89df467f9811b4f869513de3ada90ce7de74c6d1.1307006502.git.mika.westerberg@iki.fi>
On Thu, Jun 02, 2011 at 12:38:13PM +0300, Mika Westerberg wrote:
> We shouldn't use NULL for any DMA API functions, unless we are dealing with
> ISA or EISA device. So pass correct struct dev pointer to these functions.
Thanks for doing this - it certainly makes things more correct, even
if the original problem has been resolved by the troublesome GFP_DMA
commit having been reverted. As such I think this patch series should
still be applied.
I think the series should be re-ordered so that patch 5 is first however,
as you can do that with or without the remainder of the patches.
For the entire series:
Acked-by: Russell King <rmk+kernel@arm.linux.org.uk>
^ 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