* [PATCH] atm: he: use module_pci_driver to simplify the code
From: Wei Yongjun @ 2012-10-10 13:00 UTC (permalink / raw)
To: chas; +Cc: yongjun_wei, linux-atm-general, netdev
From: Wei Yongjun <yongjun_wei@trendmicro.com.cn>
Use the module_pci_driver() macro to make the code simpler
by eliminating module_init and module_exit calls.
dpatch engine is used to auto generate this patch.
(https://github.com/weiyj/dpatch)
Signed-off-by: Wei Yongjun <yongjun_wei@trendmicro.com.cn>
---
drivers/atm/he.c | 13 +------------
1 file changed, 1 insertion(+), 12 deletions(-)
diff --git a/drivers/atm/he.c b/drivers/atm/he.c
index b182c2f..e184339 100644
--- a/drivers/atm/he.c
+++ b/drivers/atm/he.c
@@ -2883,15 +2883,4 @@ static struct pci_driver he_driver = {
.id_table = he_pci_tbl,
};
-static int __init he_init(void)
-{
- return pci_register_driver(&he_driver);
-}
-
-static void __exit he_cleanup(void)
-{
- pci_unregister_driver(&he_driver);
-}
-
-module_init(he_init);
-module_exit(he_cleanup);
+module_pci_driver(he_driver);
^ permalink raw reply related
* [RFC PATCH] net/core: support runtime PM on net_device
From: Ming Lei @ 2012-10-10 12:58 UTC (permalink / raw)
To: David S. Miller, Rafael J. Wysocki
Cc: Oliver Neukum, Alan Stern, netdev, linux-pm, Ming Lei
In ioctl path on net_device, the physical deivce is often
touched, but the physical device may have been put into runtime
suspend state already, so cause some utilitis(ifconfig, ethtool,
...) to return failure in this situation.
This patch enables runtime PM on net_device and mark it as
no_callbacks, and resumes the net_device if physical device
is to be accessed, then suspends it after completion of the
access.
This patch fixes the problem above.
Signed-off-by: Ming Lei <ming.lei@canonical.com>
---
net/core/dev.c | 83 +++++++++++++++++++++++++++++++-------------------
net/core/ethtool.c | 9 ++++--
net/core/net-sysfs.c | 4 +++
3 files changed, 63 insertions(+), 33 deletions(-)
diff --git a/net/core/dev.c b/net/core/dev.c
index de2bad7..d46d4ed 100644
--- a/net/core/dev.c
+++ b/net/core/dev.c
@@ -135,6 +135,7 @@
#include <linux/net_tstamp.h>
#include <linux/static_key.h>
#include <net/flow_keys.h>
+#include <linux/pm_runtime.h>
#include "net-sysfs.h"
@@ -4966,39 +4967,24 @@ static int dev_ifsioc_locked(struct net *net, struct ifreq *ifr, unsigned int cm
/*
* Perform the SIOCxIFxxx calls, inside rtnl_lock()
*/
-static int dev_ifsioc(struct net *net, struct ifreq *ifr, unsigned int cmd)
+static int __dev_ifsioc(struct net *net, struct ifreq *ifr, unsigned int cmd)
{
int err;
struct net_device *dev = __dev_get_by_name(net, ifr->ifr_name);
const struct net_device_ops *ops;
- if (!dev)
- return -ENODEV;
-
ops = dev->netdev_ops;
switch (cmd) {
case SIOCSIFFLAGS: /* Set interface flags */
return dev_change_flags(dev, ifr->ifr_flags);
- case SIOCSIFMETRIC: /* Set the metric on the interface
- (currently unused) */
- return -EOPNOTSUPP;
-
case SIOCSIFMTU: /* Set the MTU of a device */
return dev_set_mtu(dev, ifr->ifr_mtu);
case SIOCSIFHWADDR:
return dev_set_mac_address(dev, &ifr->ifr_hwaddr);
- case SIOCSIFHWBROADCAST:
- if (ifr->ifr_hwaddr.sa_family != dev->type)
- return -EINVAL;
- memcpy(dev->broadcast, ifr->ifr_hwaddr.sa_data,
- min(sizeof ifr->ifr_hwaddr.sa_data, (size_t) dev->addr_len));
- call_netdevice_notifiers(NETDEV_CHANGEADDR, dev);
- return 0;
-
case SIOCSIFMAP:
if (ops->ndo_set_config) {
if (!netif_device_present(dev))
@@ -5022,21 +5008,6 @@ static int dev_ifsioc(struct net *net, struct ifreq *ifr, unsigned int cmd)
if (!netif_device_present(dev))
return -ENODEV;
return dev_mc_del_global(dev, ifr->ifr_hwaddr.sa_data);
-
- case SIOCSIFTXQLEN:
- if (ifr->ifr_qlen < 0)
- return -EINVAL;
- dev->tx_queue_len = ifr->ifr_qlen;
- return 0;
-
- case SIOCSIFNAME:
- ifr->ifr_newname[IFNAMSIZ-1] = '\0';
- return dev_change_name(dev, ifr->ifr_newname);
-
- case SIOCSHWTSTAMP:
- err = net_hwtstamp_validate(ifr);
- if (err)
- return err;
/* fall through */
/*
@@ -5072,6 +5043,56 @@ static int dev_ifsioc(struct net *net, struct ifreq *ifr, unsigned int cmd)
return err;
}
+static int dev_ifsioc(struct net *net, struct ifreq *ifr, unsigned int cmd)
+{
+ int err;
+ struct net_device *dev = __dev_get_by_name(net, ifr->ifr_name);
+ const struct net_device_ops *ops;
+
+ if (!dev)
+ return -ENODEV;
+
+ ops = dev->netdev_ops;
+
+ switch (cmd) {
+ case SIOCSIFMETRIC: /* Set the metric on the interface
+ (currently unused) */
+ return -EOPNOTSUPP;
+
+ case SIOCSIFHWBROADCAST:
+ if (ifr->ifr_hwaddr.sa_family != dev->type)
+ return -EINVAL;
+ memcpy(dev->broadcast, ifr->ifr_hwaddr.sa_data,
+ min(sizeof ifr->ifr_hwaddr.sa_data, (size_t) dev->addr_len));
+ call_netdevice_notifiers(NETDEV_CHANGEADDR, dev);
+ return 0;
+
+ case SIOCSIFTXQLEN:
+ if (ifr->ifr_qlen < 0)
+ return -EINVAL;
+ dev->tx_queue_len = ifr->ifr_qlen;
+ return 0;
+
+ case SIOCSIFNAME:
+ ifr->ifr_newname[IFNAMSIZ-1] = '\0';
+ return dev_change_name(dev, ifr->ifr_newname);
+
+ case SIOCSHWTSTAMP:
+ err = net_hwtstamp_validate(ifr);
+ if (err)
+ return err;
+ }
+
+ if (pm_runtime_get_sync(&dev->dev) < 0)
+ return -ENODEV;
+
+ err = __dev_ifsioc(net, ifr, cmd);
+
+ pm_runtime_put(&dev->dev);
+
+ return err;
+}
+
/*
* This function handles all "interface"-type I/O control requests. The actual
* 'doing' part of this is dev_ifsioc above.
diff --git a/net/core/ethtool.c b/net/core/ethtool.c
index 4d64cc2..2dc43da 100644
--- a/net/core/ethtool.c
+++ b/net/core/ethtool.c
@@ -25,6 +25,7 @@
#include <linux/slab.h>
#include <linux/rtnetlink.h>
#include <linux/sched.h>
+#include <linux/pm_runtime.h>
/*
* Some useful ethtool_ops methods that're device independent.
@@ -1464,10 +1465,13 @@ int dev_ethtool(struct net *net, struct ifreq *ifr)
return -EPERM;
}
+ if ((ret = pm_runtime_get_sync(&dev->dev)) < 0)
+ return -ENODEV;
+
if (dev->ethtool_ops->begin) {
rc = dev->ethtool_ops->begin(dev);
if (rc < 0)
- return rc;
+ goto exit;
}
old_features = dev->features;
@@ -1648,6 +1652,7 @@ int dev_ethtool(struct net *net, struct ifreq *ifr)
if (old_features != dev->features)
netdev_features_change(dev);
-
+exit:
+ pm_runtime_put(&dev->dev);
return rc;
}
diff --git a/net/core/net-sysfs.c b/net/core/net-sysfs.c
index bcf02f6..c9adb89 100644
--- a/net/core/net-sysfs.c
+++ b/net/core/net-sysfs.c
@@ -23,6 +23,7 @@
#include <linux/export.h>
#include <linux/jiffies.h>
#include <net/wext.h>
+#include <linux/pm_runtime.h>
#include "net-sysfs.h"
@@ -1415,6 +1416,9 @@ int netdev_register_kobject(struct net_device *net)
if (error)
return error;
+ pm_runtime_no_callbacks(dev);
+ pm_runtime_enable(dev);
+
error = register_queue_kobjects(net);
if (error) {
device_del(dev);
--
1.7.9.5
^ permalink raw reply related
* Fwd: [Bug 1063038] Re: Broadcom BCM57780 Cannot Connect at Gigabit Speed using tg3 module.
From: Justin Chudgar @ 2012-10-10 12:24 UTC (permalink / raw)
To: netdev; +Cc: 1063038
In-Reply-To: <20121009143846.2196.28573.malone@chaenomeles.canonical.com>
[-- Attachment #1: Type: text/plain, Size: 7343 bytes --]
I've got a device with a Broadcom 57780 ethernet NIC. It uses the tg3.ko
driver module. The problem is that this NIC cannot connect at gigabit
(1000MBps) speed. In all cases connection works well at 100MBps. I
reported this to Ubuntu as noted below and was directed here.
[https://bugs.launchpad.net/ubuntu/+source/linux/+bug/1063038] Following
are the tests I've completed:
- Cables tested with cable tester
- Cables tested to work at gigabit speed with Realtek and Intel gigabit
NICs.
- All unmanaged switch ports tested at gigabit speed with other NICs.
- Tried to connect with NetworkManager enabled and disabled.
- Tried both dhcp and static connections.
- Tried to renegotiate connection with `ethtool -r` both with TSO
enabled and disabled.
- Tried 3.2.0-23-generic 3.5.0-15-generic3.5.0-17-generic
3.6.0-999-generic kernels from main, xorg-edgers and mainline repos.
- Used 'tg3_debug=1' kernel option in grub with the following output:
[ 0.000000] Command line: BOOT_IMAGE=/vmlinuz-3.5.0-17-generic
root=UUID=eccabacd-efb2-45b4-b7a6-2f530d50afd3 ro rootflags=subvol=@
tg3_debug=1 i915_enable_rc6=7 i915_enable_fbc=1 lvds_downclock=1
[ 0.000000] Kernel command line:
BOOT_IMAGE=/vmlinuz-3.5.0-17-generic
root=UUID=eccabacd-efb2-45b4-b7a6-2f530d50afd3 ro rootflags=subvol=@
tg3_debug=1 i915_enable_rc6=7 i915_enable_fbc=1 lvds_downclock=1
[ 5.963136] tg3.c:v3.123 (March 21, 2012)
[ 5.995781] tg3 mdio bus: probed
[ 6.005583] tg3 0000:01:00.0: >eth0: Tigon3 [partno(BCM57780) rev
57780001] (PCI Express) MAC address dc:0e:a1:ac:38:d5
[ 6.005687] tg3 0000:01:00.0: >eth0: attached PHY driver [Broadcom
BCM57780] (mii_bus:phy_addr=100:01)
[ 6.005790] tg3 0000:01:00.0: >eth0: RXcsums[1] LinkChgREG[0]
MIirq[0] ASF[0] TSOcap[1]
[ 6.005887] tg3 0000:01:00.0: >eth0: dma_rwctrl[76180000]
dma_mask[64-bit]
[ 294.987653] tg3 0000:01:00.0: >irq 44 for MSI/MSI-X
[ 295.893224] tg3 0000:01:00.0: >eth0: Link is down
[ 309.887879] tg3 0000:01:00.0: >eth0: Link is up at 100 Mbps, full
duplex
[ 309.887886] tg3 0000:01:00.0: >eth0: Flow control is on for TX and
on for RX
Other distros' forums suggest loading broadcom.ko first; however, this
module is no present. Any suggestions would be much appreciated.
lspci -vv:
01:00.0 Ethernet controller: Broadcom Corporation NetLink BCM57780
Gigabit Ethernet PCIe (rev 01)
Subsystem: Acer Incorporated [ALI] Device 0601
Control: I/O- Mem+ BusMaster+ SpecCycle- MemWINV- VGASnoop-
ParErr- Stepping- SERR- FastB2B- DisINTx+
Status: Cap+ 66MHz- UDF- FastB2B- ParErr- DEVSEL=fast >TAbort-
<TAbort- <MAbort- >SERR- <PERR- INTx-
Latency: 0, Cache Line Size: 64 bytes
Interrupt: pin A routed to IRQ 44
Region 0: Memory at d3400000 (64-bit, non-prefetchable) [size=64K]
Capabilities: [48] Power Management version 3
Flags: PMEClk- DSI- D1- D2- AuxCurrent=0mA
PME(D0-,D1-,D2-,D3hot+,D3cold+)
Status: D0 NoSoftRst+ PME-Enable- DSel=0 DScale=1 PME-
Capabilities: [60] Vendor Specific Information: Len=6c <?>
Capabilities: [50] MSI: Enable+ Count=1/1 Maskable- 64bit+
Address: 00000000fee0100c Data: 4189
Capabilities: [cc] Express (v2) Endpoint, MSI 00
DevCap: MaxPayload 128 bytes, PhantFunc 0, Latency L0s
<4us, L1 unlimited
ExtTag+ AttnBtn- AttnInd- PwrInd- RBE+ FLReset-
DevCtl: Report errors: Correctable- Non-Fatal- Fatal-
Unsupported-
RlxdOrd- ExtTag- PhantFunc- AuxPwr- NoSnoop-
MaxPayload 128 bytes, MaxReadReq 512 bytes
DevSta: CorrErr+ UncorrErr- FatalErr- UnsuppReq- AuxPwr+
TransPend+
LnkCap: Port #0, Speed 2.5GT/s, Width x1, ASPM L0s L1,
Latency L0 <1us, L1 <32us
ClockPM+ Surprise- LLActRep- BwNot-
LnkCtl: ASPM L0s L1 Enabled; RCB 64 bytes Disabled-
Retrain- CommClk+
ExtSynch- ClockPM+ AutWidDis- BWInt- AutBWInt-
LnkSta: Speed 2.5GT/s, Width x1, TrErr- Train- SlotClk+
DLActive- BWMgmt- ABWMgmt-
DevCap2: Completion Timeout: Range ABCD, TimeoutDis+
DevCtl2: Completion Timeout: 50us to 50ms, TimeoutDis-
LnkCtl2: Target Link Speed: 2.5GT/s, EnterCompliance-
SpeedDis-, Selectable De-emphasis: -6dB
Transmit Margin: Normal Operating Range,
EnterModifiedCompliance- ComplianceSOS-
Compliance De-emphasis: -6dB
LnkSta2: Current De-emphasis Level: -6dB
Capabilities: [100 v1] Advanced Error Reporting
UESta: DLP- SDES- TLP- FCP- CmpltTO- CmpltAbrt-
UnxCmplt- RxOF- MalfTLP- ECRC- UnsupReq- ACSViol-
UEMsk: DLP- SDES- TLP- FCP- CmpltTO- CmpltAbrt-
UnxCmplt- RxOF- MalfTLP- ECRC- UnsupReq- ACSViol-
UESvrt: DLP+ SDES+ TLP- FCP+ CmpltTO- CmpltAbrt-
UnxCmplt- RxOF+ MalfTLP+ ECRC- UnsupReq- ACSViol-
CESta: RxErr+ BadTLP- BadDLLP- Rollover- Timeout-
NonFatalErr-
CEMsk: RxErr- BadTLP- BadDLLP- Rollover- Timeout-
NonFatalErr+
AERCap: First Error Pointer: 00, GenCap+ CGenEn- ChkCap+
ChkEn-
Capabilities: [13c v1] Virtual Channel
Caps: LPEVC=0 RefClk=100ns PATEntryBits=1
Arb: Fixed- WRR32- WRR64- WRR128-
Ctrl: ArbSelect=Fixed
Status: InProgress-
VC0: Caps: PATOffset=00 MaxTimeSlots=1 RejSnoopTrans-
Arb: Fixed- WRR32- WRR64- WRR128- TWRR128- WRR256-
Ctrl: Enable+ ID=0 ArbSelect=Fixed TC/VC=ff
Status: NegoPending- InProgress-
Capabilities: [160 v1] Device Serial Number dc-0e-a1-ff-fe-ac-38-d5
Capabilities: [16c v1] Power Budgeting <?>
Kernel driver in use: tg3
Kernel modules: tg3
---
Justin Chudgar | Weed, CA 96094 | 530 921 0738 | http://www.justinzane.com/
-------- Original Message --------
Subject: [Bug 1063038] Re: Broadcom BCM57780 Cannot Connect at Gigabit
Speed using tg3 module.
Date: Tue, 09 Oct 2012 14:38:45 -0000
From: Joseph Salisbury <joseph.salisbury@canonical.com>
Reply-To: Bug 1063038 <1063038@bugs.launchpad.net>
To: justin@justinzane.com
This issue appears to be an upstream bug, since you tested the latest
upstream kernel. Would it be possible for you to open an upstream bug
report[0]? That will allow the upstream Developers to examine the
issue, and may provide a quicker resolution to the bug.
Please follow the instructions on the wiki page[0]. The first step is
to email the appropriate mailing list. If no response is received, then
a bug may be opened on bugzilla.kernel.org.
[0] https://wiki.ubuntu.com/Bugs/Upstream/kernel
** Changed in: linux (Ubuntu)
Status: Confirmed => Triaged
** Changed in: linux (Ubuntu)
Importance: Undecided => Medium
** Tags added: kernel-bug-exists-upstream
--
You received this bug notification because you are subscribed to the bug
report.
https://bugs.launchpad.net/bugs/1063038
Title:
Broadcom BCM57780 Cannot Connect at Gigabit Speed using tg3 module.
To manage notifications about this bug go to:
https://bugs.launchpad.net/ubuntu/+source/linux/+bug/1063038/+subscriptions
[-- Attachment #2: justin.vcf --]
[-- Type: text/x-vcard, Size: 150 bytes --]
begin:vcard
fn:Justin Chudgar
n:Chudgar;Justin
email;internet:justin@justinzane.com
tel;cell:530-921-0738
x-mozilla-html:FALSE
version:2.1
end:vcard
^ permalink raw reply
* Re: [Xen-devel] compound skb frag pages appearing in start_xmit
From: Ian Campbell @ 2012-10-10 12:29 UTC (permalink / raw)
To: Sander Eikelenboom
Cc: Eric Dumazet, netdev@vger.kernel.org, Eric Dumazet,
Konrad Rzeszutek Wilk, xen-devel
In-Reply-To: <1622789731.20121010142428@eikelenboom.it>
On Wed, 2012-10-10 at 13:24 +0100, Sander Eikelenboom wrote:
> Wednesday, October 10, 2012, 12:13:04 PM, you wrote:
>
> > On Tue, 2012-10-09 at 15:40 +0100, Ian Campbell wrote:
> >> On Tue, 2012-10-09 at 15:27 +0100, Eric Dumazet wrote:
> >> > On Tue, 2012-10-09 at 15:17 +0100, Ian Campbell wrote:
> >> >
> >> > > Does the higher order pages effectively reduce the number of frags which
> >> > > are in use? e.g if MAX_SKB_FRAGS is 16, then for order-0 pages you could
> >> > > have 64K worth of frag data.
> >> > >
> >> > > If we switch to order-3 pages everywhere then can the skb contain 512K
> >> > > of data, or does the effective maximum number of frags in an skb reduce
> >> > > to 2?
> >> >
> >> > effective number of frags reduce to 2 or 3
> >> >
> >> > (We still limit GSO packets to ~63536 bytes)
> >>
> >> Great! Then I think the fix is more/less trivial...
>
> > The following seems to work for me.
>
> But it doesn't seem to work for me ... dmesg attached.
> [ 191.777994] ------------[ cut here ]------------
> [ 191.784245] kernel BUG at drivers/net/xen-netback/netback.c:481!
Looks like that BUG_ON is a little aggressive. It'll trigger if the data
happens to end on a frame boundary. Hopefully this will fix it for you:
diff --git a/drivers/net/xen-netback/netback.c b/drivers/net/xen-netback/netback.c
index d747e30..f2d6b78 100644
--- a/drivers/net/xen-netback/netback.c
+++ b/drivers/net/xen-netback/netback.c
@@ -477,7 +477,7 @@ static void netbk_gop_frag_copy(struct xenvif *vif, struct sk_buff *skb,
size -= bytes;
/* Next frame */
- if (offset == PAGE_SIZE) {
+ if (offset == PAGE_SIZE && size) {
BUG_ON(!PageCompound(page));
page++;
offset = 0;
^ permalink raw reply related
* Accounting Unit
From: JAMES SCHERMAN @ 2012-10-10 12:20 UTC (permalink / raw)
[-- Attachment #1: Type: text/plain, Size: 0 bytes --]
[-- Attachment #2: KINDLY READ.docx --]
[-- Type: application/vnd.openxmlformats-officedocument.wordprocessingml.document, Size: 13172 bytes --]
^ permalink raw reply
* Re: compound skb frag pages appearing in start_xmit
From: Sander Eikelenboom @ 2012-10-10 12:24 UTC (permalink / raw)
To: Ian Campbell
Cc: netdev@vger.kernel.org, Eric Dumazet, xen-devel, Eric Dumazet,
Konrad Rzeszutek Wilk
In-Reply-To: <1349863984.10070.26.camel@zakaz.uk.xensource.com>
Wednesday, October 10, 2012, 12:13:04 PM, you wrote:
> On Tue, 2012-10-09 at 15:40 +0100, Ian Campbell wrote:
>> On Tue, 2012-10-09 at 15:27 +0100, Eric Dumazet wrote:
>> > On Tue, 2012-10-09 at 15:17 +0100, Ian Campbell wrote:
>> >
>> > > Does the higher order pages effectively reduce the number of frags which
>> > > are in use? e.g if MAX_SKB_FRAGS is 16, then for order-0 pages you could
>> > > have 64K worth of frag data.
>> > >
>> > > If we switch to order-3 pages everywhere then can the skb contain 512K
>> > > of data, or does the effective maximum number of frags in an skb reduce
>> > > to 2?
>> >
>> > effective number of frags reduce to 2 or 3
>> >
>> > (We still limit GSO packets to ~63536 bytes)
>>
>> Great! Then I think the fix is more/less trivial...
> The following seems to work for me.
But it doesn't seem to work for me ... dmesg attached.
I don't know if the "mcelog:4359 map pfn expected mapping type write-back for [mem 0x0009f000-0x000a0fff], got uncached-minus"
is related, is shows up right after the nics get initialized ?
netback still fails with:
[ 191.777994] ------------[ cut here ]------------
[ 191.784245] kernel BUG at drivers/net/xen-netback/netback.c:481!
[ 191.790423] invalid opcode: 0000 [#1] PREEMPT SMP
[ 191.796462] Modules linked in:
[ 191.802315] CPU 1
[ 191.802367] Pid: 1177, comm: netback/1 Tainted: G W 3.6.0pre-rc1-20121010 #1 MSI MS-7640/890FXA-GD70 (MS-7640)
[ 191.814043] RIP: e030:[<ffffffff8146de61>] [<ffffffff8146de61>] netbk_gop_frag_copy+0x3f1/0x400
[ 191.820171] RSP: e02b:ffff880037c6bb98 EFLAGS: 00010246
[ 191.826271] RAX: 0000000000000244 RBX: ffffc90010827f98 RCX: ffff880031ed9880
[ 191.832450] RDX: 00000000000000a8 RSI: ffff880037c6bd24 RDI: ffffea0000b03f80
[ 191.838581] RBP: ffff880037c6bc28 R08: ffff8800319f8100 R09: 0000000000001000
[ 191.844739] R10: 0000000000000000 R11: 0000000000000132 R12: 00000000000000a8
[ 191.850785] R13: ffff880037c6bcd8 R14: 0000000000001000 R15: ffffc9001082cf70
[ 191.856741] FS: 00007f9f3c944700(0000) GS:ffff88003f840000(0000) knlGS:0000000000000000
[ 191.862841] CS: e033 DS: 0000 ES: 0000 CR0: 000000008005003b
[ 191.868901] CR2: 0000000001337ca0 CR3: 0000000032cec000 CR4: 0000000000000660
[ 191.875053] DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000
[ 191.881175] DR3: 0000000000000000 DR6: 00000000ffff0ff0 DR7: 0000000000000400
[ 191.887247] Process netback/1 (pid: 1177, threadinfo ffff880037c6a000, task ffff880039984140)
[ 191.893325] Stack:
[ 191.899328] ffff880037c6bd24 00000000000000a8 ffff8800319f8100 ffff880031ed9880
[ 191.905534] ffffc90000000000 0000000000001000 0000000000000000 0000000000000000
[ 191.911742] ffff880000000000 ffffffff817459f3 ffffc90010823420 ffffea0000b03f80
[ 191.917898] Call Trace:
[ 191.923939] [<ffffffff817459f3>] ? _raw_spin_unlock_irqrestore+0x53/0xa0
[ 191.930141] [<ffffffff8146e1cb>] xen_netbk_rx_action+0x30b/0x830
[ 191.936543] [<ffffffff810ad22d>] ? trace_hardirqs_on+0xd/0x10
[ 191.942942] [<ffffffff8146f6da>] xen_netbk_kthread+0xba/0xa90
[ 191.949147] [<ffffffff81095b06>] ? try_to_wake_up+0x1b6/0x310
[ 191.955250] [<ffffffff81086b40>] ? wake_up_bit+0x40/0x40
[ 191.961421] [<ffffffff8146f620>] ? xen_netbk_tx_build_gops+0xa70/0xa70
[ 191.967660] [<ffffffff810864d6>] kthread+0xd6/0xe0
[ 191.973834] [<ffffffff81086400>] ? __init_kthread_worker+0x70/0x70
[ 191.979953] [<ffffffff8174677c>] ret_from_fork+0x7c/0x90
[ 191.986107] [<ffffffff81086400>] ? __init_kthread_worker+0x70/0x70
[ 191.992174] Code: b8 b3 00 00 48 8d 8c f1 60 01 00 00 48 3b 14 01 0f 85 72 fc ff ff e9 7a fc ff ff 0f 0b eb fe 0f 0b eb fe 0f 0b eb fe 0f 0b eb fe <0f> 0b eb fe 66 66 2e 0f 1f 84 00 00 00 00 00 55 48 89 e5 48 83
[ 192.005230] RIP [<ffffffff8146de61>] netbk_gop_frag_copy+0x3f1/0x400
[ 192.011786] RSP <ffff880037c6bb98>
[ 192.018402] ---[ end trace c51ab5e2c2c918fc ]---
--
Sander
> I haven't tackled netfront yet.
> 8<--------------------------------------------------------------
> From 551e42e3dd203f2eb97cb082985013bb33b8f020 Mon Sep 17 00:00:00 2001
> From: Ian Campbell <ian.campbell@citrix.com>
> Date: Tue, 9 Oct 2012 15:51:20 +0100
> Subject: [PATCH] xen: netback: handle compound page fragments on transmit.
> An SKB paged fragment can consist of a compound page with order > 0.
> However the netchannel protocol deals only in PAGE_SIZE frames.
> Handle this in netbk_gop_frag_copy and xen_netbk_count_skb_slots by
> iterating over the frames which make up the page.
> Signed-off-by: Ian Campbell <ian.campbell@citrix.com>
> Cc: Eric Dumazet <eric.dumazet@gmail.com>
> Cc: Konrad Rzeszutek Wilk <konrad@kernel.org>
> Cc: Sander Eikelenboom <linux@eikelenboom.it>
> ---
> drivers/net/xen-netback/netback.c | 40 ++++++++++++++++++++++++++++++++----
> 1 files changed, 35 insertions(+), 5 deletions(-)
> diff --git a/drivers/net/xen-netback/netback.c b/drivers/net/xen-netback/netback.c
> index 4ebfcf3..d747e30 100644
> --- a/drivers/net/xen-netback/netback.c
> +++ b/drivers/net/xen-netback/netback.c
> @@ -335,21 +335,35 @@ unsigned int xen_netbk_count_skb_slots(struct xenvif *vif, struct sk_buff *skb)
>
> for (i = 0; i < skb_shinfo(skb)->nr_frags; i++) {
> unsigned long size = skb_frag_size(&skb_shinfo(skb)->frags[i]);
> + unsigned long offset = skb_shinfo(skb)->frags[i].page_offset;
> unsigned long bytes;
> +
> + offset &= ~PAGE_MASK;
> +
> while (size > 0) {
> + BUG_ON(offset >= PAGE_SIZE);
> BUG_ON(copy_off > MAX_BUFFER_OFFSET);
>
> - if (start_new_rx_buffer(copy_off, size, 0)) {
> + bytes = PAGE_SIZE - offset;
> +
> + if (bytes > size)
> + bytes = size;
> +
> + if (start_new_rx_buffer(copy_off, bytes, 0)) {
> count++;
> copy_off = 0;
> }
>
> - bytes = size;
> if (copy_off + bytes > MAX_BUFFER_OFFSET)
> bytes = MAX_BUFFER_OFFSET - copy_off;
>
> copy_off += bytes;
> +
> + offset += bytes;
> size -= bytes;
> +
> + if (offset == PAGE_SIZE)
> + offset = 0;
> }
> }
> return count;
> @@ -403,14 +417,24 @@ static void netbk_gop_frag_copy(struct xenvif *vif, struct sk_buff *skb,
> unsigned long bytes;
>
> /* Data must not cross a page boundary. */
> - BUG_ON(size + offset > PAGE_SIZE);
> + BUG_ON(size + offset > PAGE_SIZE<<compound_order(page));
>
> meta = npo->meta + npo->meta_prod - 1;
>
> + /* Skip unused frames from start of page */
> + page += offset >> PAGE_SHIFT;
> + offset &= ~PAGE_MASK;
> +
> while (size > 0) {
> + BUG_ON(offset >= PAGE_SIZE);
> BUG_ON(npo->copy_off > MAX_BUFFER_OFFSET);
>
> - if (start_new_rx_buffer(npo->copy_off, size, *head)) {
> + bytes = PAGE_SIZE - offset;
> +
> + if (bytes > size)
> + bytes = size;
> +
> + if (start_new_rx_buffer(npo->copy_off, bytes, *head)) {
> /*
> * Netfront requires there to be some data in the head
> * buffer.
> @@ -420,7 +444,6 @@ static void netbk_gop_frag_copy(struct xenvif *vif, struct sk_buff *skb,
> meta = get_next_rx_buffer(vif, npo);
> }
>
> - bytes = size;
> if (npo->copy_off + bytes > MAX_BUFFER_OFFSET)
> bytes = MAX_BUFFER_OFFSET - npo->copy_off;
>
> @@ -453,6 +476,13 @@ static void netbk_gop_frag_copy(struct xenvif *vif, struct sk_buff *skb,
> offset += bytes;
> size -= bytes;
>
> + /* Next frame */
> + if (offset == PAGE_SIZE) {
> + BUG_ON(!PageCompound(page));
> + page++;
> + offset = 0;
> + }
> +
> /* Leave a gap for the GSO descriptor. */
> if (*head && skb_shinfo(skb)->gso_size && !vif->gso_prefix)
> vif->rx.req_cons++;
^ permalink raw reply
* Re: [patch] isdn: fix a wrapping bug in isdn_ppp_ioctl()
From: Andreas Schwab @ 2012-10-10 12:05 UTC (permalink / raw)
To: Joe Perches
Cc: Dan Carpenter, Karsten Keil, David S. Miller, Masanari Iida,
netdev, kernel-janitors
In-Reply-To: <1349864358.2386.27.camel@joe-AO722>
Joe Perches <joe@perches.com> writes:
> @@ -589,16 +589,15 @@ isdn_ppp_ioctl(int min, struct file *file, unsigned int cmd, unsigned long arg)
> break;
> case PPPIOCGCOMPRESSORS:
> {
> - unsigned long protos[8] = {0,};
> + DECLARE_BITMAP(protos, BITS_PER_LONG * 8) = { 0, };
> struct isdn_ppp_compressor *ipc = ipc_head;
> +
> while (ipc) {
> - j = ipc->num / (sizeof(long) * 8);
> - i = ipc->num % (sizeof(long) * 8);
> - if (j < 8)
> - protos[j] |= (0x1 << i);
> + if (ipc->num < BITS_PER_LONG * 8)
> + set_bit(ipc->num, protos);
> ipc = ipc->next;
> }
> - if ((r = set_arg(argp, protos, 8 * sizeof(long))))
> + if ((r = set_arg(argp, protos, sizeof(protos))))
This changes the bit endianess. Since protos is exported to user space
it is an ABI change.
Andreas.
--
Andreas Schwab, schwab@linux-m68k.org
GPG Key fingerprint = 58CA 54C7 6D53 942B 1756 01D3 44D5 214B 8276 4ED5
"And now for something completely different."
^ permalink raw reply
* Re: [PATCH 01/12] usbnet: introduce usbnet 3 command helpers
From: Ming Lei @ 2012-10-10 11:45 UTC (permalink / raw)
To: David Laight
Cc: Oliver Neukum, David S. Miller, Greg Kroah-Hartman, netdev,
linux-usb
In-Reply-To: <AE90C24D6B3A694183C094C60CF0A2F6026B702F@saturn3.aculab.com>
On Wed, Oct 10, 2012 at 7:25 PM, David Laight <David.Laight@aculab.com> wrote:
>
> What about the error handler/sleep/resume code calling into the
> memory allocator to indicate that all allocates be GFP_NOIO until
> it calls back to indicate that the restricted path is complete.
> Might be a per-cpu count?
IMO, it might be a per-task variable, but unfortunately no such
mechanism is provided by current kernel.
Thanks,
--
Ming Lei
^ permalink raw reply
* Re: [PATCH 01/12] usbnet: introduce usbnet 3 command helpers
From: Oliver Neukum @ 2012-10-10 11:39 UTC (permalink / raw)
To: David Laight
Cc: Ming Lei, David S. Miller, Greg Kroah-Hartman,
netdev-u79uwXL29TY76Z2rM5mHXA, linux-usb-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <AE90C24D6B3A694183C094C60CF0A2F6026B702F-CgBM+Bx2aUAnGFn1LkZF6NBPR1lH4CV8@public.gmane.org>
On Wednesday 10 October 2012 12:25:58 David Laight wrote:
> > On Wed, Oct 10, 2012 at 6:08 PM, Oliver Neukum <oneukum-l3A5Bk7waGM@public.gmane.org> wrote:
> >
> > > A reset always applies to the whole device. Resets are used in error
> > > handling of block devices (storage and uas). If you reset a device,
> > > pre_reset() and post_reset() of all interfaces need to be called. So they
> > > are part of the SCSI error handler. SCSI error handlers can allocate memory
> > > only with GFP_NOIO (or GFP_ATOMIC) because any IO for paging
> > > can cause the SCSI layer to wait for the error handling to finish. The error
> > > handling can only finish when pre/post_reset() have finished. Catch-22
> >
> > IMO, it is not practical to obey the rule for drivers, because driver may
> > call many other kernel component API which may allocate memory
> > via GFP_KERNEL in the path easily.
>
> What about the error handler/sleep/resume code calling into the
> memory allocator to indicate that all allocates be GFP_NOIO until
> it calls back to indicate that the restricted path is complete.
This seems to be a very complex scheme.
> Might be a per-cpu count?
No. The handlers may sleep and switch CPUs.
Regards
Oliver
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" 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 01/12] usbnet: introduce usbnet 3 command helpers
From: David Laight @ 2012-10-10 11:25 UTC (permalink / raw)
To: Ming Lei, Oliver Neukum
Cc: David S. Miller, Greg Kroah-Hartman,
netdev-u79uwXL29TY76Z2rM5mHXA, linux-usb-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <CACVXFVPDg89y7LyKLA0YUN7oA2rGfptfHLZhJrqBjTVPjsGdNg-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
> On Wed, Oct 10, 2012 at 6:08 PM, Oliver Neukum <oneukum-l3A5Bk7waGM@public.gmane.org> wrote:
>
> > A reset always applies to the whole device. Resets are used in error
> > handling of block devices (storage and uas). If you reset a device,
> > pre_reset() and post_reset() of all interfaces need to be called. So they
> > are part of the SCSI error handler. SCSI error handlers can allocate memory
> > only with GFP_NOIO (or GFP_ATOMIC) because any IO for paging
> > can cause the SCSI layer to wait for the error handling to finish. The error
> > handling can only finish when pre/post_reset() have finished. Catch-22
>
> IMO, it is not practical to obey the rule for drivers, because driver may
> call many other kernel component API which may allocate memory
> via GFP_KERNEL in the path easily.
What about the error handler/sleep/resume code calling into the
memory allocator to indicate that all allocates be GFP_NOIO until
it calls back to indicate that the restricted path is complete.
Might be a per-cpu count?
David
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" 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] isdn: fix a wrapping bug in isdn_ppp_ioctl()
From: Joe Perches @ 2012-10-10 11:15 UTC (permalink / raw)
To: David Laight
Cc: Dan Carpenter, Karsten Keil, David S. Miller, Masanari Iida,
netdev, kernel-janitors
In-Reply-To: <AE90C24D6B3A694183C094C60CF0A2F6026B702D@saturn3.aculab.com>
On Wed, 2012-10-10 at 11:42 +0100, David Laight wrote:
> > - unsigned long protos[8] = {0,};
> > + DECLARE_BITMAP(protos, BITS_PER_LONG * 8) = { 0, };
> ...
> > - if ((r = set_arg(argp, protos, 8 * sizeof(long))))
> > + if ((r = set_arg(argp, protos, sizeof(protos))))
>
> That change makes a big assumption about the implementation
> of DECLARE_BITMAP().
> Unless it is guaranteed to be implemented as 'unsigned long[]'
> then you've changed what the code might do.
Possible, but it's hard to imagine it changing.
The = { 0, } should probably be bitmap_zero
^ permalink raw reply
* [PATCH] bridge: Pull ip header into skb->data before looking into ip header.
From: sarveshwar.bandi @ 2012-10-10 11:15 UTC (permalink / raw)
To: davem; +Cc: netdev, Sarveshwar Bandi
From: Sarveshwar Bandi <sarveshwar.bandi@emulex.com>
If lower layer driver leaves the ip header in the skb fragment, it needs to
be first pulled into skb->data before inspecting ip header length or ip version
number.
Signed-off-by: Sarveshwar Bandi <sarveshwar.bandi@emulex.com>
---
net/bridge/br_netfilter.c | 3 +++
1 file changed, 3 insertions(+)
diff --git a/net/bridge/br_netfilter.c b/net/bridge/br_netfilter.c
index 68e8f36..fe43bc7 100644
--- a/net/bridge/br_netfilter.c
+++ b/net/bridge/br_netfilter.c
@@ -265,6 +265,9 @@ static int br_parse_ip_options(struct sk_buff *skb)
struct net_device *dev = skb->dev;
u32 len;
+ if (!pskb_may_pull(skb, sizeof(struct iphdr)))
+ goto inhdr_error;
+
iph = ip_hdr(skb);
opt = &(IPCB(skb)->opt);
--
1.7.9.5
^ permalink raw reply related
* Re: [PATCH 01/12] usbnet: introduce usbnet 3 command helpers
From: Ming Lei @ 2012-10-10 11:02 UTC (permalink / raw)
To: Oliver Neukum; +Cc: David S. Miller, Greg Kroah-Hartman, netdev, linux-usb
In-Reply-To: <1691674.ASueyuVmUn@linux-lqwf.site>
On Wed, Oct 10, 2012 at 6:08 PM, Oliver Neukum <oneukum@suse.de> wrote:
>
> A reset always applies to the whole device. Resets are used in error
> handling of block devices (storage and uas). If you reset a device,
> pre_reset() and post_reset() of all interfaces need to be called. So they
> are part of the SCSI error handler. SCSI error handlers can allocate memory
> only with GFP_NOIO (or GFP_ATOMIC) because any IO for paging
> can cause the SCSI layer to wait for the error handling to finish. The error
> handling can only finish when pre/post_reset() have finished. Catch-22
IMO, it is not practical to obey the rule for drivers, because driver may
call many other kernel component API which may allocate memory
via GFP_KERNEL in the path easily.
Same with runtime PM case.
>
> So any control messages in block error handling need to use GFP_NOIO.
> If you look at the control message helpers in usbcore you will find a lot
> of GFP_NOIO. That is the reason.
If one driver has no .pre_reset or .post_reset, usb_reset_device() will
try to unbind&bind the interface, so you mean these usb drivers have
to use GFP_NOIO to allocate memory in its probe() and disconnect()?
Unfortunately, that is not true, and no way to make sure all memory
allocations in the path via GFP_NOIO, IMO.
Also, only very few drivers have implemented .pre_reset and .post_reset.
Thanks,
--
Ming Lei
^ permalink raw reply
* RE: [patch] isdn: fix a wrapping bug in isdn_ppp_ioctl()
From: David Laight @ 2012-10-10 10:42 UTC (permalink / raw)
To: Joe Perches, Dan Carpenter
Cc: Karsten Keil, David S. Miller, Masanari Iida, netdev,
kernel-janitors
In-Reply-To: <1349864358.2386.27.camel@joe-AO722>
> - unsigned long protos[8] = {0,};
> + DECLARE_BITMAP(protos, BITS_PER_LONG * 8) = { 0, };
...
> - if ((r = set_arg(argp, protos, 8 * sizeof(long))))
> + if ((r = set_arg(argp, protos, sizeof(protos))))
That change makes a big assumption about the implementation
of DECLARE_BITMAP().
Unless it is guaranteed to be implemented as 'unsigned long[]'
then you've changed what the code might do.
David
^ permalink raw reply
* Re: [patch] isdn: fix a wrapping bug in isdn_ppp_ioctl()
From: Joe Perches @ 2012-10-10 10:41 UTC (permalink / raw)
To: Dan Carpenter
Cc: Karsten Keil, David S. Miller, Masanari Iida, netdev,
kernel-janitors
In-Reply-To: <1349864358.2386.27.camel@joe-AO722>
On Wed, 2012-10-10 at 03:19 -0700, Joe Perches wrote:
> diff --git a/drivers/isdn/i4l/isdn_ppp.c b/drivers/isdn/i4l/isdn_ppp.c
[]
> struct isdn_ppp_comp_data data;
> @@ -589,16 +589,15 @@ isdn_ppp_ioctl(int min, struct file *file, unsigned int cmd, unsigned long arg)
> break;
> case PPPIOCGCOMPRESSORS:
> {
> - unsigned long protos[8] = {0,};
> + DECLARE_BITMAP(protos, BITS_PER_LONG * 8) = { 0, };
s/BITS_PER_LONG/sizeof(long)/duh...
^ permalink raw reply
* Re: [patch] isdn: fix a wrapping bug in isdn_ppp_ioctl()
From: Joe Perches @ 2012-10-10 10:19 UTC (permalink / raw)
To: Dan Carpenter
Cc: Karsten Keil, David S. Miller, Masanari Iida, netdev,
kernel-janitors
In-Reply-To: <20121010093816.GA3669@elgon.mountain>
On Wed, 2012-10-10 at 12:42 +0300, Dan Carpenter wrote:
> "protos" is an array of unsigned longs and "i" is the number of bits in
> an unsigned long so we need to use 1UL as well to prevent the shift
> from wrapping around.
>
> Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>
>
> diff --git a/drivers/isdn/i4l/isdn_ppp.c b/drivers/isdn/i4l/isdn_ppp.c
> index a1e7601..69b5b58 100644
> --- a/drivers/isdn/i4l/isdn_ppp.c
> +++ b/drivers/isdn/i4l/isdn_ppp.c
> @@ -595,7 +595,7 @@ isdn_ppp_ioctl(int min, struct file *file, unsigned int cmd, unsigned long arg)
> j = ipc->num / (sizeof(long) * 8);
> i = ipc->num % (sizeof(long) * 8);
> if (j < 8)
> - protos[j] |= (0x1 << i);
> + protos[j] |= (1UL << i);
This looks like a bitmap and probably it should use
DECLARE_BITMAP and set_bit.
Also, it looks like the set_arg size is wrong and is
a stack leak.
Maybe:
drivers/isdn/i4l/isdn_ppp.c | 13 ++++++-------
1 file changed, 6 insertions(+), 7 deletions(-)
diff --git a/drivers/isdn/i4l/isdn_ppp.c b/drivers/isdn/i4l/isdn_ppp.c
index a1e7601..6438d0c 100644
--- a/drivers/isdn/i4l/isdn_ppp.c
+++ b/drivers/isdn/i4l/isdn_ppp.c
@@ -471,7 +471,7 @@ int
isdn_ppp_ioctl(int min, struct file *file, unsigned int cmd, unsigned long arg)
{
unsigned long val;
- int r, i, j;
+ int r;
struct ippp_struct *is;
isdn_net_local *lp;
struct isdn_ppp_comp_data data;
@@ -589,16 +589,15 @@ isdn_ppp_ioctl(int min, struct file *file, unsigned int cmd, unsigned long arg)
break;
case PPPIOCGCOMPRESSORS:
{
- unsigned long protos[8] = {0,};
+ DECLARE_BITMAP(protos, BITS_PER_LONG * 8) = { 0, };
struct isdn_ppp_compressor *ipc = ipc_head;
+
while (ipc) {
- j = ipc->num / (sizeof(long) * 8);
- i = ipc->num % (sizeof(long) * 8);
- if (j < 8)
- protos[j] |= (0x1 << i);
+ if (ipc->num < BITS_PER_LONG * 8)
+ set_bit(ipc->num, protos);
ipc = ipc->next;
}
- if ((r = set_arg(argp, protos, 8 * sizeof(long))))
+ if ((r = set_arg(argp, protos, sizeof(protos))))
return r;
}
break;
^ permalink raw reply related
* Re: [Xen-devel] compound skb frag pages appearing in start_xmit
From: Ian Campbell @ 2012-10-10 10:13 UTC (permalink / raw)
To: Eric Dumazet
Cc: netdev@vger.kernel.org, Eric Dumazet, Sander Eikelenboom,
Konrad Rzeszutek Wilk, xen-devel
In-Reply-To: <1349793630.21847.208.camel@zakaz.uk.xensource.com>
On Tue, 2012-10-09 at 15:40 +0100, Ian Campbell wrote:
> On Tue, 2012-10-09 at 15:27 +0100, Eric Dumazet wrote:
> > On Tue, 2012-10-09 at 15:17 +0100, Ian Campbell wrote:
> >
> > > Does the higher order pages effectively reduce the number of frags which
> > > are in use? e.g if MAX_SKB_FRAGS is 16, then for order-0 pages you could
> > > have 64K worth of frag data.
> > >
> > > If we switch to order-3 pages everywhere then can the skb contain 512K
> > > of data, or does the effective maximum number of frags in an skb reduce
> > > to 2?
> >
> > effective number of frags reduce to 2 or 3
> >
> > (We still limit GSO packets to ~63536 bytes)
>
> Great! Then I think the fix is more/less trivial...
The following seems to work for me.
I haven't tackled netfront yet.
8<--------------------------------------------------------------
>From 551e42e3dd203f2eb97cb082985013bb33b8f020 Mon Sep 17 00:00:00 2001
From: Ian Campbell <ian.campbell@citrix.com>
Date: Tue, 9 Oct 2012 15:51:20 +0100
Subject: [PATCH] xen: netback: handle compound page fragments on transmit.
An SKB paged fragment can consist of a compound page with order > 0.
However the netchannel protocol deals only in PAGE_SIZE frames.
Handle this in netbk_gop_frag_copy and xen_netbk_count_skb_slots by
iterating over the frames which make up the page.
Signed-off-by: Ian Campbell <ian.campbell@citrix.com>
Cc: Eric Dumazet <eric.dumazet@gmail.com>
Cc: Konrad Rzeszutek Wilk <konrad@kernel.org>
Cc: Sander Eikelenboom <linux@eikelenboom.it>
---
drivers/net/xen-netback/netback.c | 40 ++++++++++++++++++++++++++++++++----
1 files changed, 35 insertions(+), 5 deletions(-)
diff --git a/drivers/net/xen-netback/netback.c b/drivers/net/xen-netback/netback.c
index 4ebfcf3..d747e30 100644
--- a/drivers/net/xen-netback/netback.c
+++ b/drivers/net/xen-netback/netback.c
@@ -335,21 +335,35 @@ unsigned int xen_netbk_count_skb_slots(struct xenvif *vif, struct sk_buff *skb)
for (i = 0; i < skb_shinfo(skb)->nr_frags; i++) {
unsigned long size = skb_frag_size(&skb_shinfo(skb)->frags[i]);
+ unsigned long offset = skb_shinfo(skb)->frags[i].page_offset;
unsigned long bytes;
+
+ offset &= ~PAGE_MASK;
+
while (size > 0) {
+ BUG_ON(offset >= PAGE_SIZE);
BUG_ON(copy_off > MAX_BUFFER_OFFSET);
- if (start_new_rx_buffer(copy_off, size, 0)) {
+ bytes = PAGE_SIZE - offset;
+
+ if (bytes > size)
+ bytes = size;
+
+ if (start_new_rx_buffer(copy_off, bytes, 0)) {
count++;
copy_off = 0;
}
- bytes = size;
if (copy_off + bytes > MAX_BUFFER_OFFSET)
bytes = MAX_BUFFER_OFFSET - copy_off;
copy_off += bytes;
+
+ offset += bytes;
size -= bytes;
+
+ if (offset == PAGE_SIZE)
+ offset = 0;
}
}
return count;
@@ -403,14 +417,24 @@ static void netbk_gop_frag_copy(struct xenvif *vif, struct sk_buff *skb,
unsigned long bytes;
/* Data must not cross a page boundary. */
- BUG_ON(size + offset > PAGE_SIZE);
+ BUG_ON(size + offset > PAGE_SIZE<<compound_order(page));
meta = npo->meta + npo->meta_prod - 1;
+ /* Skip unused frames from start of page */
+ page += offset >> PAGE_SHIFT;
+ offset &= ~PAGE_MASK;
+
while (size > 0) {
+ BUG_ON(offset >= PAGE_SIZE);
BUG_ON(npo->copy_off > MAX_BUFFER_OFFSET);
- if (start_new_rx_buffer(npo->copy_off, size, *head)) {
+ bytes = PAGE_SIZE - offset;
+
+ if (bytes > size)
+ bytes = size;
+
+ if (start_new_rx_buffer(npo->copy_off, bytes, *head)) {
/*
* Netfront requires there to be some data in the head
* buffer.
@@ -420,7 +444,6 @@ static void netbk_gop_frag_copy(struct xenvif *vif, struct sk_buff *skb,
meta = get_next_rx_buffer(vif, npo);
}
- bytes = size;
if (npo->copy_off + bytes > MAX_BUFFER_OFFSET)
bytes = MAX_BUFFER_OFFSET - npo->copy_off;
@@ -453,6 +476,13 @@ static void netbk_gop_frag_copy(struct xenvif *vif, struct sk_buff *skb,
offset += bytes;
size -= bytes;
+ /* Next frame */
+ if (offset == PAGE_SIZE) {
+ BUG_ON(!PageCompound(page));
+ page++;
+ offset = 0;
+ }
+
/* Leave a gap for the GSO descriptor. */
if (*head && skb_shinfo(skb)->gso_size && !vif->gso_prefix)
vif->rx.req_cons++;
--
1.7.2.5
^ permalink raw reply related
* Re: [PATCH 01/12] usbnet: introduce usbnet 3 command helpers
From: Oliver Neukum @ 2012-10-10 10:08 UTC (permalink / raw)
To: Ming Lei; +Cc: David S. Miller, Greg Kroah-Hartman, netdev, linux-usb
In-Reply-To: <CACVXFVMq8-t2o5=M=3xCQEiPFXjvH14g3NtRgOTMYr542d5zXw@mail.gmail.com>
On Wednesday 10 October 2012 17:48:54 Ming Lei wrote:
> On Wed, Oct 10, 2012 at 4:39 PM, Oliver Neukum <oneukum@suse.de> wrote:
> > On Wednesday 10 October 2012 16:17:25 Ming Lei wrote:
> >> On Wed, Oct 10, 2012 at 1:51 PM, Oliver Neukum <oneukum@suse.de> wrote:
> >
> >> > We need to use GFP_NOIO in situations the helper cannot know about.
> >> > Please add a gfp_t parameter. Then the caller will solve that.
> >>
> >> Considered that most of drivers call the helpers in different context, I think
> >> it is better to switch the gpf_t flag runtime inside helpers, like below:
> >>
> >> if (dev->power.runtime_status == RPM_RESUMING)
> >> gfp = GFP_NOIO;
> >> else
> >> gfp = GFP_KERNEL;
> >
> > You are admirably persistent ;-)
>
> I am only trying to solve the problem more generally, :-)
The most generic solution is passing the parameter.
> > If you extended the check to RPM_SUSPENDING it might work,
> > but still the problem with error handling exists.
>
> Could you describe the error handling case in a bit detail so
> that callers of these helpers can know when GFP_KERNEL
> is to be passed and when GFP_NOIO is taken if the gfp
> patamerer has to be added?
A reset always applies to the whole device. Resets are used in error
handling of block devices (storage and uas). If you reset a device,
pre_reset() and post_reset() of all interfaces need to be called. So they
are part of the SCSI error handler. SCSI error handlers can allocate memory
only with GFP_NOIO (or GFP_ATOMIC) because any IO for paging
can cause the SCSI layer to wait for the error handling to finish. The error
handling can only finish when pre/post_reset() have finished. Catch-22
So any control messages in block error handling need to use GFP_NOIO.
If you look at the control message helpers in usbcore you will find a lot
of GFP_NOIO. That is the reason.
Regards
Oliver
^ permalink raw reply
* Re: [PATCH] of/mdio: Staticise !CONFIG_OF stubs
From: Srinivas KANDAGATLA @ 2012-10-10 9:56 UTC (permalink / raw)
To: Mark Brown; +Cc: David S. Miller, Fengguang Wu, netdev
In-Reply-To: <1349843618-11209-1-git-send-email-broonie@opensource.wolfsonmicro.com>
On 10/10/12 05:33, Mark Brown wrote:
> The !CONFIG_OF stubs aren't static so if multiple files include the
> header with this configuration then the linker will see multiple
> definitions of the stubs.
>
> Reported-by: Fengguang Wu <fengguang.wu@intel.com>
> Signed-off-by: Mark Brown <broonie@opensource.wolfsonmicro.com>
> ---
> include/linux/of_mdio.h | 10 +++++-----
> 1 file changed, 5 insertions(+), 5 deletions(-)
Acked-by: Srinivas Kandagatla <srinivas.kandagatla@st.com>
^ permalink raw reply
* Re: [PATCH 01/12] usbnet: introduce usbnet 3 command helpers
From: Ming Lei @ 2012-10-10 9:48 UTC (permalink / raw)
To: Oliver Neukum
Cc: David S. Miller, Greg Kroah-Hartman,
netdev-u79uwXL29TY76Z2rM5mHXA, linux-usb-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <1631246.gHVDWoZpLi-ugxBuEnWX9yG/4A2pS7c2Q@public.gmane.org>
On Wed, Oct 10, 2012 at 4:39 PM, Oliver Neukum <oneukum-l3A5Bk7waGM@public.gmane.org> wrote:
> On Wednesday 10 October 2012 16:17:25 Ming Lei wrote:
>> On Wed, Oct 10, 2012 at 1:51 PM, Oliver Neukum <oneukum-l3A5Bk7waGM@public.gmane.org> wrote:
>
>> > We need to use GFP_NOIO in situations the helper cannot know about.
>> > Please add a gfp_t parameter. Then the caller will solve that.
>>
>> Considered that most of drivers call the helpers in different context, I think
>> it is better to switch the gpf_t flag runtime inside helpers, like below:
>>
>> if (dev->power.runtime_status == RPM_RESUMING)
>> gfp = GFP_NOIO;
>> else
>> gfp = GFP_KERNEL;
>
> You are admirably persistent ;-)
I am only trying to solve the problem more generally, :-)
> If you extended the check to RPM_SUSPENDING it might work,
> but still the problem with error handling exists.
Could you describe the error handling case in a bit detail so
that callers of these helpers can know when GFP_KERNEL
is to be passed and when GFP_NOIO is taken if the gfp
patamerer has to be added?
Thanks,
--
Ming Lei
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* [patch] isdn: fix a wrapping bug in isdn_ppp_ioctl()
From: Dan Carpenter @ 2012-10-10 9:42 UTC (permalink / raw)
To: Karsten Keil; +Cc: David S. Miller, Masanari Iida, netdev, kernel-janitors
"protos" is an array of unsigned longs and "i" is the number of bits in
an unsigned long so we need to use 1UL as well to prevent the shift
from wrapping around.
Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>
diff --git a/drivers/isdn/i4l/isdn_ppp.c b/drivers/isdn/i4l/isdn_ppp.c
index a1e7601..69b5b58 100644
--- a/drivers/isdn/i4l/isdn_ppp.c
+++ b/drivers/isdn/i4l/isdn_ppp.c
@@ -595,7 +595,7 @@ isdn_ppp_ioctl(int min, struct file *file, unsigned int cmd, unsigned long arg)
j = ipc->num / (sizeof(long) * 8);
i = ipc->num % (sizeof(long) * 8);
if (j < 8)
- protos[j] |= (0x1 << i);
+ protos[j] |= (1UL << i);
ipc = ipc->next;
}
if ((r = set_arg(argp, protos, 8 * sizeof(long))))
^ permalink raw reply related
* Re: [PATCH 01/12] usbnet: introduce usbnet 3 command helpers
From: Oliver Neukum @ 2012-10-10 8:39 UTC (permalink / raw)
To: Ming Lei
Cc: David S. Miller, Greg Kroah-Hartman,
netdev-u79uwXL29TY76Z2rM5mHXA, linux-usb-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <CACVXFVM7wPLXy0JL7QDnCaZFidwucTFf3t_38DuwukxWtOESHQ-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
On Wednesday 10 October 2012 16:17:25 Ming Lei wrote:
> On Wed, Oct 10, 2012 at 1:51 PM, Oliver Neukum <oneukum-l3A5Bk7waGM@public.gmane.org> wrote:
> > We need to use GFP_NOIO in situations the helper cannot know about.
> > Please add a gfp_t parameter. Then the caller will solve that.
>
> Considered that most of drivers call the helpers in different context, I think
> it is better to switch the gpf_t flag runtime inside helpers, like below:
>
> if (dev->power.runtime_status == RPM_RESUMING)
> gfp = GFP_NOIO;
> else
> gfp = GFP_KERNEL;
You are admirably persistent ;-)
If you extended the check to RPM_SUSPENDING it might work,
but still the problem with error handling exists.
Regards
Oliver
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" 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 01/12] usbnet: introduce usbnet 3 command helpers
From: Oliver Neukum @ 2012-10-10 8:24 UTC (permalink / raw)
To: Ming Lei
Cc: David S. Miller, Greg Kroah-Hartman,
netdev-u79uwXL29TY76Z2rM5mHXA, linux-usb-u79uwXL29TY76Z2rM5mHXA,
rjw-KKrjLPT3xs0
In-Reply-To: <CACVXFVM7CrxXPYzr+dfWhbbmbF+3sXq4C1q2OauvP6x_jebbYQ-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
On Wednesday 10 October 2012 13:56:16 Ming Lei wrote:
> On Wed, Oct 10, 2012 at 11:19 AM, Ming Lei <ming.lei-Z7WLFzj8eWMS+FvcfC7Uqw@public.gmane.org> wrote:
> > On Tue, Oct 9, 2012 at 4:47 PM, Oliver Neukum <oneukum-l3A5Bk7waGM@public.gmane.org> wrote:
> >>
> >> Using GFP_KERNEL you preclude using those in resume() and error handling.
> >> Please pass a gfp_t parameter.
> >
> > IMO, it is not a big deal because generally only several bytes are to be
> > allocated inside these helpers.
>
> Also pm_restrict_gfp_mask()/pm_restore_gfp_mask() have been introduced
> to address the problem for 2 years, looks the gfp_t isn't needed, doesn't it?
No, absolutely not. Introducing them was a mistake and is hiding errors.
Those helpers solve the problem only for the case of _system_ suspend/resume.
However the runtime case has the same problem. So in addition to not solving
the problem, we now have two code paths.
Frankly, those functions should be removed.
Secondly, in this case a similar deadlock exists with error handling.
Again take a device with network and storage functions (a.k.a. cell phone).
The storage function does a reset. And the deadlock happens like this:
reset storage -> pre_reset() -> physical reset -> post_reset() -> net interface
does a control message -> kmalloc(..., GFP_KERNEL) -> VM layer decide
to page out -> IO to storage function -> SCSI layer waits for error handler --> DEADLOCK
Believe me, you won't find a fancy solution for this. Just pass the gfp_t.
Regards
Oliver
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" 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 1/6] vxlan: minor output refactoring
From: David Laight @ 2012-10-10 8:08 UTC (permalink / raw)
To: Joe Perches, Stephen Hemminger; +Cc: David Miller, netdev
In-Reply-To: <1349807275.2386.14.camel@joe-AO722>
> -----Original Message-----
> From: netdev-owner@vger.kernel.org [mailto:netdev-owner@vger.kernel.org] On Behalf Of Joe Perches
> Sent: 09 October 2012 19:28
> To: Stephen Hemminger
> Cc: David Miller; netdev@vger.kernel.org
> Subject: Re: [PATCH 1/6] vxlan: minor output refactoring
>
> On Tue, 2012-10-09 at 10:56 -0700, Stephen Hemminger wrote:
> > +static __be32 vxlan_find_dst(struct vxlan_dev *vxlan, struct sk_buff *skb)
> > +{
> > + const struct ethhdr *eth = (struct ethhdr *) skb->data;
> > + struct vxlan_fdb *f;
> > +
> > + if (is_multicast_ether_addr(eth->h_dest) ||
> > + (f = vxlan_find_mac(vxlan, eth->h_dest)) == NULL)
> > + return vxlan->gaddr;
> > + else
> > + return f->remote_ip;
> > +}
>
> Bikeshedding:
>
> This might be simpler to read with a few more lines like:
>
> static __be32 vxlan_find_dst(struct vxlan_dev *vxlan, struct sk_buff *skb)
> {
> const struct ethhdr *eth = (const struct ethhdr *)skb->data;
> struct vxlan_fdb *f;
>
> if (is_multicast_ether_addr(eth->h_dest))
> return vxlan->gaddr;
>
> f = vxlan_find_mac(vxlan, eth->h_dest);
> if (!f)
> return vxlan->gaddr;
>
> return f->remote_ip;
> }
or, painting it green:
static __be32 vxlan_find_dst(struct vxlan_dev *vxlan, struct sk_buff *skb)
{
const struct ethhdr *eth = (struct ethhdr *)skb->data;
struct vxlan_fdb *f;
if (!is_multicast_ether_addr(eth->h_dest) &&
(f = vxlan_find_mac(vxlan, eth->h_dest)) != NULL)
return f->remote_ip;
return vxlan->gaddr;
}
David
^ permalink raw reply
* Re: [PATCH 01/12] usbnet: introduce usbnet 3 command helpers
From: Ming Lei @ 2012-10-10 8:17 UTC (permalink / raw)
To: Oliver Neukum
Cc: David S. Miller, Greg Kroah-Hartman,
netdev-u79uwXL29TY76Z2rM5mHXA, linux-usb-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <4085386.s0fOKMaRDP-ugxBuEnWX9yG/4A2pS7c2Q@public.gmane.org>
On Wed, Oct 10, 2012 at 1:51 PM, Oliver Neukum <oneukum-l3A5Bk7waGM@public.gmane.org> wrote:
>
> No, the problem is autoresume.
>
> Suppose we have a device with two interface. Interface A be usbnet; interface B
> something you page on. Now consider that you can only resume both interfaces
> and this is (and needs to be) done synchronously.
>
> Now we can have this code path:
>
> autoresume of device -> resume() -> kmalloc(..., GFP_KERNEL) ->
> VM layer decides to start paging out -> IO to interface B -> autoresume of device
> --> DEADLOCK
OK, thanks for your detailed explanation.
> We need to use GFP_NOIO in situations the helper cannot know about.
> Please add a gfp_t parameter. Then the caller will solve that.
Considered that most of drivers call the helpers in different context, I think
it is better to switch the gpf_t flag runtime inside helpers, like below:
if (dev->power.runtime_status == RPM_RESUMING)
gfp = GFP_NOIO;
else
gfp = GFP_KERNEL;
Thanks,
--
Ming Lei
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" 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
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