* Re: [PATCH -next] net/m68k: Include <linux/interrupt.h> where needed
From: David Miller @ 2011-06-11 21:58 UTC (permalink / raw)
To: geert; +Cc: adobriyan, netdev, linux-next, linux-kernel
In-Reply-To: <1307792335-9926-1-git-send-email-geert@linux-m68k.org>
From: Geert Uytterhoeven <geert@linux-m68k.org>
Date: Sat, 11 Jun 2011 13:38:55 +0200
...
> Introduced by commit a6b7a407865a ("net: remove interrupt.h inclusion from
> netdevice.h").
>
> Include <linux/interrupt.h> in the individual drivers to fix the build.
>
> Signed-off-by: Geert Uytterhoeven <geert@linux-m68k.org>
Applied, thanks!
^ permalink raw reply
* Re: [PATCH] net/usb: Add Samsung Kalmia driver for Samsung GT-B3730
From: Marius Kotsbak @ 2011-06-11 21:57 UTC (permalink / raw)
To: Greg KH; +Cc: davem, netdev, linux-usb, Marius B. Kotsbak
In-Reply-To: <20110611200625.GB14582@kroah.com>
On 11. juni 2011 22:06, Greg KH wrote:
>>>> > >> + * This program is distributed in the hope that it will be useful,
>>>> > >> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
>>>> > >> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
>>>> > >> + * GNU General Public License for more details.
>>>> > >> + *
>>>> > >> + * You should have received a copy of the GNU General Public License
>>>> > >> + * along with this program; if not, write to the Free Software
>>>> > >> + * Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA
>>> > > These two paragraphs are not needed, and unless you want to track the
>>> > > FSF's office changes for the next 40+ years, I'd really suggest not
>>> > > putting it in there at all.
>>> > >
>> >
>> > Same here, I have reused what I found in other modules. Do you have an
>> > example of a module with the recommended header?
> Just take out these two paragraphs and you should be fine, and be sure
> that you really mean "any later" for your license and that you didn't
> copy any code from a non "any later" file.
>
Japp, that is fine for me. Fixed the other stuff and sent updated patch
now. How is the possibility of getting a stable backported version of
this one accepted (to Ubuntu LTS/Natty kernel versions)?
--
Marius
^ permalink raw reply
* [PATCH] net/usb: Add Samsung Kalmia driver for Samsung GT-B3730
From: Marius B. Kotsbak @ 2011-06-11 21:55 UTC (permalink / raw)
To: davem, netdev; +Cc: linux-usb, Marius B. Kotsbak
In-Reply-To: <20110611200625.GB14582@kroah.com>
Introducing driver for the network port of Samsung Kalmia based USB LTE modems.
It has also an ACM interface that previous patches associates with the "option"
module. To access those interfaces, the modem must first be switched from modem
mode using a tool like usb_modeswitch.
As the proprietary protocol has been discovered by watching the MS Windows driver
behavior, there might be errors in the protocol handling, but stable and fast
connection has been established for hours with Norwegian operator NetCom that
distributes this modem with their LTE/4G subscription.
More and updated information about how to use this driver is available here:
http://www.draisberghof.de/usb_modeswitch/bb/viewtopic.php?t=465
https://github.com/mkotsbak/Samsung-GT-B3730-linux-driver
Signed-off-by: Marius B. Kotsbak <marius@kotsbak.com>
---
drivers/net/usb/Kconfig | 10 ++
drivers/net/usb/Makefile | 1 +
drivers/net/usb/kalmia.c | 384 ++++++++++++++++++++++++++++++++++++++++++++++
3 files changed, 395 insertions(+), 0 deletions(-)
create mode 100644 drivers/net/usb/kalmia.c
diff --git a/drivers/net/usb/Kconfig b/drivers/net/usb/Kconfig
index 9d4f911..b6e4efc 100644
--- a/drivers/net/usb/Kconfig
+++ b/drivers/net/usb/Kconfig
@@ -385,6 +385,16 @@ config USB_NET_CX82310_ETH
router with USB ethernet port. This driver is for routers only,
it will not work with ADSL modems (use cxacru driver instead).
+config CONFIG_USB_NET_KALMIA
+ tristate "Samsung Kalmia based LTE USB modem"
+ depends on USB_USBNET
+ help
+ Choose this option if you have a Samsung Kalmia based USB modem
+ as Samsung GT-B3730.
+
+ To compile this driver as a module, choose M here: the
+ module will be called kalmia.
+
config USB_HSO
tristate "Option USB High Speed Mobile Devices"
depends on USB && RFKILL
diff --git a/drivers/net/usb/Makefile b/drivers/net/usb/Makefile
index c7ec8a5..c203fa2 100644
--- a/drivers/net/usb/Makefile
+++ b/drivers/net/usb/Makefile
@@ -23,6 +23,7 @@ obj-$(CONFIG_USB_NET_MCS7830) += mcs7830.o
obj-$(CONFIG_USB_USBNET) += usbnet.o
obj-$(CONFIG_USB_NET_INT51X1) += int51x1.o
obj-$(CONFIG_USB_CDC_PHONET) += cdc-phonet.o
+obj-$(CONFIG_USB_NET_KALMIA) += kalmia.o
obj-$(CONFIG_USB_IPHETH) += ipheth.o
obj-$(CONFIG_USB_SIERRA_NET) += sierra_net.o
obj-$(CONFIG_USB_NET_CX82310_ETH) += cx82310_eth.o
diff --git a/drivers/net/usb/kalmia.c b/drivers/net/usb/kalmia.c
new file mode 100644
index 0000000..d965fb1
--- /dev/null
+++ b/drivers/net/usb/kalmia.c
@@ -0,0 +1,384 @@
+/*
+ * USB network interface driver for Samsung Kalmia based LTE USB modem like the
+ * Samsung GT-B3730 and GT-B3710.
+ *
+ * Copyright (C) 2011 Marius Bjoernstad Kotsbak <marius@kotsbak.com>
+ *
+ * Sponsored by Quicklink Video Distribution Services Ltd.
+ *
+ * Based on the cdc_eem module.
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ */
+
+#include <linux/module.h>
+#include <linux/init.h>
+#include <linux/netdevice.h>
+#include <linux/etherdevice.h>
+#include <linux/ctype.h>
+#include <linux/ethtool.h>
+#include <linux/workqueue.h>
+#include <linux/mii.h>
+#include <linux/usb.h>
+#include <linux/crc32.h>
+#include <linux/usb/cdc.h>
+#include <linux/usb/usbnet.h>
+#include <linux/gfp.h>
+
+/*
+ * The Samsung Kalmia based LTE USB modems have a CDC ACM port for modem control
+ * handled by the "option" module and an ethernet data port handled by this
+ * module.
+ *
+ * The stick must first be switched into modem mode by usb_modeswitch
+ * or similar tool. Then the modem gets sent two initialization packets by
+ * this module, which gives the MAC address of the device. User space can then
+ * connect the modem using AT commands through the ACM port and then use
+ * DHCP on the network interface exposed by this module. Network packets are
+ * sent to and from the modem in a proprietary format discovered after watching
+ * the behavior of the windows driver for the modem.
+ *
+ * More information about the use of the modem is available in usb_modeswitch
+ * forum and the project page:
+ *
+ * http://www.draisberghof.de/usb_modeswitch/bb/viewtopic.php?t=465
+ * https://github.com/mkotsbak/Samsung-GT-B3730-linux-driver
+ */
+
+/* #define DEBUG */
+/* #define VERBOSE */
+
+#define KALMIA_HEADER_LENGTH 6
+#define KALMIA_ALIGN_SIZE 4
+#define KALMIA_USB_TIMEOUT 10000
+
+/*-------------------------------------------------------------------------*/
+
+static int
+kalmia_send_init_packet(struct usbnet *dev, u8 *init_msg, u8 init_msg_len,
+ u8 *buffer, u8 expected_len)
+{
+ int act_len;
+ int status;
+
+ netdev_dbg(dev->net, "Sending init packet");
+
+ status = usb_bulk_msg(dev->udev, usb_sndbulkpipe(dev->udev, 0x02),
+ init_msg, init_msg_len, &act_len, KALMIA_USB_TIMEOUT);
+ if (status != 0) {
+ netdev_err(dev->net,
+ "Error sending init packet. Status %i, length %i\n",
+ status, act_len);
+ return status;
+ }
+ else if (act_len != init_msg_len) {
+ netdev_err(dev->net,
+ "Did not send all of init packet. Bytes sent: %i",
+ act_len);
+ }
+ else {
+ netdev_dbg(dev->net, "Successfully sent init packet.");
+ }
+
+ status = usb_bulk_msg(dev->udev, usb_rcvbulkpipe(dev->udev, 0x81),
+ buffer, expected_len, &act_len, KALMIA_USB_TIMEOUT);
+
+ if (status != 0)
+ netdev_err(dev->net,
+ "Error receiving init result. Status %i, length %i\n",
+ status, act_len);
+ else if (act_len != expected_len)
+ netdev_err(dev->net, "Unexpected init result length: %i\n",
+ act_len);
+
+ return status;
+}
+
+static int
+kalmia_init_and_get_ethernet_addr(struct usbnet *dev, u8 *ethernet_addr)
+{
+ char init_msg_1[] =
+ { 0x57, 0x50, 0x04, 0x00, 0x00, 0x00, 0x00, 0x20, 0x00, 0x00,
+ 0x00, 0x00 };
+ char init_msg_2[] =
+ { 0x57, 0x50, 0x04, 0x00, 0x00, 0x00, 0x00, 0x02, 0x00, 0xf4,
+ 0x00, 0x00 };
+ char receive_buf[28];
+ int status;
+
+ status = kalmia_send_init_packet(dev, init_msg_1, sizeof(init_msg_1)
+ / sizeof(init_msg_1[0]), receive_buf, 24);
+ if (status != 0)
+ return status;
+
+ status = kalmia_send_init_packet(dev, init_msg_2, sizeof(init_msg_2)
+ / sizeof(init_msg_2[0]), receive_buf, 28);
+ if (status != 0)
+ return status;
+
+ memcpy(ethernet_addr, receive_buf + 10, ETH_ALEN);
+
+ return status;
+}
+
+static int
+kalmia_bind(struct usbnet *dev, struct usb_interface *intf)
+{
+ u8 status;
+ u8 ethernet_addr[ETH_ALEN];
+
+ /* Don't bind to AT command interface */
+ if (intf->cur_altsetting->desc.bInterfaceClass != USB_CLASS_VENDOR_SPEC)
+ return -EINVAL;
+
+ dev->in = usb_rcvbulkpipe(dev->udev, 0x81 & USB_ENDPOINT_NUMBER_MASK);
+ dev->out = usb_sndbulkpipe(dev->udev, 0x02 & USB_ENDPOINT_NUMBER_MASK);
+ dev->status = NULL;
+
+ dev->net->hard_header_len += KALMIA_HEADER_LENGTH;
+ dev->hard_mtu = 1400;
+ dev->rx_urb_size = dev->hard_mtu * 10; // Found as optimal after testing
+
+ status = kalmia_init_and_get_ethernet_addr(dev, ethernet_addr);
+
+ if (status < 0) {
+ usb_set_intfdata(intf, NULL);
+ usb_driver_release_interface(driver_of(intf), intf);
+ return status;
+ }
+
+ memcpy(dev->net->dev_addr, ethernet_addr, ETH_ALEN);
+ memcpy(dev->net->perm_addr, ethernet_addr, ETH_ALEN);
+
+ return status;
+}
+
+static struct sk_buff *
+kalmia_tx_fixup(struct usbnet *dev, struct sk_buff *skb, gfp_t flags)
+{
+ struct sk_buff *skb2 = NULL;
+ u16 content_len;
+ unsigned char *header_start;
+ unsigned char ether_type_1, ether_type_2;
+ u8 remainder, padlen = 0;
+
+ if (!skb_cloned(skb)) {
+ int headroom = skb_headroom(skb);
+ int tailroom = skb_tailroom(skb);
+
+ if ((tailroom >= KALMIA_ALIGN_SIZE) && (headroom
+ >= KALMIA_HEADER_LENGTH))
+ goto done;
+
+ if ((headroom + tailroom) > (KALMIA_HEADER_LENGTH
+ + KALMIA_ALIGN_SIZE)) {
+ skb->data = memmove(skb->head + KALMIA_HEADER_LENGTH,
+ skb->data, skb->len);
+ skb_set_tail_pointer(skb, skb->len);
+ goto done;
+ }
+ }
+
+ skb2 = skb_copy_expand(skb, KALMIA_HEADER_LENGTH,
+ KALMIA_ALIGN_SIZE, flags);
+ if (!skb2)
+ return NULL;
+
+ dev_kfree_skb_any(skb);
+ skb = skb2;
+
+ done: header_start = skb_push(skb, KALMIA_HEADER_LENGTH);
+ ether_type_1 = header_start[KALMIA_HEADER_LENGTH + 12];
+ ether_type_2 = header_start[KALMIA_HEADER_LENGTH + 13];
+
+ netdev_dbg(dev->net, "Sending etherType: %02x%02x", ether_type_1,
+ ether_type_2);
+
+ /* According to empiric data for data packages */
+ header_start[0] = 0x57;
+ header_start[1] = 0x44;
+ content_len = skb->len - KALMIA_HEADER_LENGTH;
+ header_start[2] = (content_len & 0xff); /* low byte */
+ header_start[3] = (content_len >> 8); /* high byte */
+
+ header_start[4] = ether_type_1;
+ header_start[5] = ether_type_2;
+
+ /* Align to 4 bytes by padding with zeros */
+ remainder = skb->len % KALMIA_ALIGN_SIZE;
+ if (remainder > 0) {
+ padlen = KALMIA_ALIGN_SIZE - remainder;
+ memset(skb_put(skb, padlen), 0, padlen);
+ }
+
+ netdev_dbg(
+ dev->net,
+ "Sending package with length %i and padding %i. Header: %02x:%02x:%02x:%02x:%02x:%02x.",
+ content_len, padlen, header_start[0], header_start[1],
+ header_start[2], header_start[3], header_start[4],
+ header_start[5]);
+
+ return skb;
+}
+
+static int
+kalmia_rx_fixup(struct usbnet *dev, struct sk_buff *skb)
+{
+ /*
+ * Our task here is to strip off framing, leaving skb with one
+ * data frame for the usbnet framework code to process.
+ */
+ const u8 HEADER_END_OF_USB_PACKET[] =
+ { 0x57, 0x5a, 0x00, 0x00, 0x08, 0x00 };
+ const u8 EXPECTED_UNKNOWN_HEADER_1[] =
+ { 0x57, 0x43, 0x1e, 0x00, 0x15, 0x02 };
+ const u8 EXPECTED_UNKNOWN_HEADER_2[] =
+ { 0x57, 0x50, 0x0e, 0x00, 0x00, 0x00 };
+ u8 i = 0;
+
+ /* incomplete header? */
+ if (skb->len < KALMIA_HEADER_LENGTH)
+ return 0;
+
+ do {
+ struct sk_buff *skb2 = NULL;
+ u8 *header_start;
+ u16 usb_packet_length, ether_packet_length;
+ int is_last;
+
+ header_start = skb->data;
+
+ if (unlikely(header_start[0] != 0x57 || header_start[1] != 0x44)) {
+ if (!memcmp(header_start, EXPECTED_UNKNOWN_HEADER_1,
+ sizeof(EXPECTED_UNKNOWN_HEADER_1)) || !memcmp(
+ header_start, EXPECTED_UNKNOWN_HEADER_2,
+ sizeof(EXPECTED_UNKNOWN_HEADER_2))) {
+ netdev_dbg(
+ dev->net,
+ "Received expected unknown frame header: %02x:%02x:%02x:%02x:%02x:%02x. Package length: %i\n",
+ header_start[0], header_start[1],
+ header_start[2], header_start[3],
+ header_start[4], header_start[5],
+ skb->len - KALMIA_HEADER_LENGTH);
+ }
+ else {
+ netdev_err(
+ dev->net,
+ "Received unknown frame header: %02x:%02x:%02x:%02x:%02x:%02x. Package length: %i\n",
+ header_start[0], header_start[1],
+ header_start[2], header_start[3],
+ header_start[4], header_start[5],
+ skb->len - KALMIA_HEADER_LENGTH);
+ return 0;
+ }
+ }
+ else
+ netdev_dbg(
+ dev->net,
+ "Received header: %02x:%02x:%02x:%02x:%02x:%02x. Package length: %i\n",
+ header_start[0], header_start[1], header_start[2],
+ header_start[3], header_start[4], header_start[5],
+ skb->len - KALMIA_HEADER_LENGTH);
+
+ /* subtract start header and end header */
+ usb_packet_length = skb->len - (2 * KALMIA_HEADER_LENGTH);
+ ether_packet_length = header_start[2] + (header_start[3] << 8);
+ skb_pull(skb, KALMIA_HEADER_LENGTH);
+
+ /* Some small packets misses end marker */
+ if (usb_packet_length < ether_packet_length) {
+ ether_packet_length = usb_packet_length
+ + KALMIA_HEADER_LENGTH;
+ is_last = true;
+ }
+ else {
+ netdev_dbg(dev->net, "Correct package length #%i", i
+ + 1);
+
+ is_last = (memcmp(skb->data + ether_packet_length,
+ HEADER_END_OF_USB_PACKET,
+ sizeof(HEADER_END_OF_USB_PACKET)) == 0);
+ if (!is_last) {
+ header_start = skb->data + ether_packet_length;
+ netdev_dbg(
+ dev->net,
+ "End header: %02x:%02x:%02x:%02x:%02x:%02x. Package length: %i\n",
+ header_start[0], header_start[1],
+ header_start[2], header_start[3],
+ header_start[4], header_start[5],
+ skb->len - KALMIA_HEADER_LENGTH);
+ }
+ }
+
+ if (is_last) {
+ skb2 = skb;
+ }
+ else {
+ skb2 = skb_clone(skb, GFP_ATOMIC);
+ if (unlikely(!skb2))
+ return 0;
+ }
+
+ skb_trim(skb2, ether_packet_length);
+
+ if (is_last) {
+ return 1;
+ }
+ else {
+ usbnet_skb_return(dev, skb2);
+ skb_pull(skb, ether_packet_length);
+ }
+
+ i++;
+ }
+ while (skb->len);
+
+ return 1;
+}
+
+static const struct driver_info kalmia_info = {
+ .description = "Samsung Kalmia LTE USB dongle",
+ .flags = FLAG_WWAN,
+ .bind = kalmia_bind,
+ .rx_fixup = kalmia_rx_fixup,
+ .tx_fixup = kalmia_tx_fixup
+};
+
+/*-------------------------------------------------------------------------*/
+
+static const struct usb_device_id products[] = {
+ /* The unswitched USB ID, to get the module auto loaded: */
+ { USB_DEVICE(0x04e8, 0x689a) },
+ /* The stick swithed into modem (by e.g. usb_modeswitch): */
+ { USB_DEVICE(0x04e8, 0x6889),
+ .driver_info = (unsigned long) &kalmia_info, },
+ { /* EMPTY == end of list */} };
+MODULE_DEVICE_TABLE( usb, products);
+
+static struct usb_driver kalmia_driver = {
+ .name = "kalmia",
+ .id_table = products,
+ .probe = usbnet_probe,
+ .disconnect = usbnet_disconnect,
+ .suspend = usbnet_suspend,
+ .resume = usbnet_resume
+};
+
+static int __init kalmia_init(void)
+{
+ return usb_register(&kalmia_driver);
+}
+module_init( kalmia_init);
+
+static void __exit kalmia_exit(void)
+{
+ usb_deregister(&kalmia_driver);
+}
+module_exit( kalmia_exit);
+
+MODULE_AUTHOR("Marius Bjoernstad Kotsbak <marius@kotsbak.com>");
+MODULE_DESCRIPTION("Samsung Kalmia USB network driver");
+MODULE_LICENSE("GPL");
--
1.7.4.1
^ permalink raw reply related
* Re: [PATCH] net/usb: Add Samsung Kalmia driver for Samsung GT-B3730
From: Greg KH @ 2011-06-11 20:06 UTC (permalink / raw)
To: Marius Kotsbak; +Cc: davem, netdev, linux-usb, Marius B. Kotsbak
In-Reply-To: <4DF2B5E4.8070303@gmail.com>
On Sat, Jun 11, 2011 at 02:25:08AM +0200, Marius Kotsbak wrote:
> On 11. juni 2011 01:55, Greg KH wrote:
> > On Sat, Jun 11, 2011 at 01:34:30AM +0200, Marius B. Kotsbak wrote:
> >> Introducing driver for the network port of Samsung Kalmia based USB LTE modems.
> >> It has also an ACM interface that previous patches associates with the "option"
> >> module. To access those interfaces, the modem must first be switched from modem
> >> mode using a tool like usb_modeswitch.
> > Do we need to blacklist this device from the option driver?
>
> It is already done, in commits:
>
> 80f9df3e0093ad9f1eeefd2ff7fd27daaa518d25
> 15b2f3204a5c878c32939094775fb7349f707263
Ah, nevermind then :)
> >> + * This program is distributed in the hope that it will be useful,
> >> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> >> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
> >> + * GNU General Public License for more details.
> >> + *
> >> + * You should have received a copy of the GNU General Public License
> >> + * along with this program; if not, write to the Free Software
> >> + * Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA
> > These two paragraphs are not needed, and unless you want to track the
> > FSF's office changes for the next 40+ years, I'd really suggest not
> > putting it in there at all.
> >
>
> Same here, I have reused what I found in other modules. Do you have an
> example of a module with the recommended header?
Just take out these two paragraphs and you should be fine, and be sure
that you really mean "any later" for your license and that you didn't
copy any code from a non "any later" file.
thanks,
greg k-h
^ permalink raw reply
* Question about LRO/GRO and TCP acknowledgements
From: Joris van Rantwijk @ 2011-06-11 19:59 UTC (permalink / raw)
To: netdev
Hi,
I'm trying to understand how Linux produces TCP acknowledgements
for segments received via LRO/GRO.
As far as I can see, the network driver uses GRO to collect several
received packets into one big super skb, which is then handled
during just one call to tcp_v4_rcv(). This will eventually result
in the sending of at most one ACK packet for the entire GRO packet.
Conventional wisdom (RFC 5681) says that a receiver should send at
least one ACK for every two data segments received. The sending TCP
needs these ACKs to update its congestion window (e.g. slow start).
It seems to me that the current implementation in Linux may send
just one ACK for a large number of received segments. This would
be a deviation from the standard. As a result the congestion
window of the sender would grow much slower than intended.
Maybe I misunderstand something in the network code (likely).
Could someone please explain me how this ACK issue is handled?
Thanks,
Joris van Rantwijk.
^ permalink raw reply
* Re: [PATCH] drivers/ssb/driver_chipcommon_pmu.c: uninitilized warning
From: Michael Büsch @ 2011-06-11 18:50 UTC (permalink / raw)
To: Connor Hansen; +Cc: mb, netdev, linux-kernel
In-Reply-To: <1307816085-20062-1-git-send-email-cmdkhh@gmail.com>
On Sat, 11 Jun 2011 11:14:45 -0700
Connor Hansen <cmdkhh@gmail.com> wrote:
> warning message
> drivers/ssb/driver_chipcommon_pmu.c: In function ssb_pmu_resources_init
> drivers/ssb/driver_chipcommon_pmu.c:420:15: warning: updown_tab_size may
> be used uninitilized in this function.
>
> updown_tab_size and depend_tab_size may not be set in the bus->chip_id
> switch statement, so set to 0 by default to avoid using uninitialized
> stack space.
We wouldn't be using uninitialized stack space or uninitialized variables,
without this patch. However, for the sake of shutting up the compiler, ACK.
> Signed-off-by: Connor Hansen <cmdkhh@gmail.com>
> ---
> drivers/ssb/driver_chipcommon_pmu.c | 4 ++--
> 1 files changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/ssb/driver_chipcommon_pmu.c b/drivers/ssb/driver_chipcommon_pmu.c
> index 305ade7..a7aef47 100644
> --- a/drivers/ssb/driver_chipcommon_pmu.c
> +++ b/drivers/ssb/driver_chipcommon_pmu.c
> @@ -417,9 +417,9 @@ static void ssb_pmu_resources_init(struct ssb_chipcommon *cc)
> u32 min_msk = 0, max_msk = 0;
> unsigned int i;
> const struct pmu_res_updown_tab_entry *updown_tab = NULL;
> - unsigned int updown_tab_size;
> + unsigned int updown_tab_size = 0;
> const struct pmu_res_depend_tab_entry *depend_tab = NULL;
> - unsigned int depend_tab_size;
> + unsigned int depend_tab_size = 0;
>
> switch (bus->chip_id) {
> case 0x4312:
^ permalink raw reply
* [PATCH v3 5/5] net: ep93xx_eth: fix DMA API violations
From: Mika Westerberg @ 2011-06-11 18:39 UTC (permalink / raw)
To: netdev; +Cc: linux-arm-kernel, hsweeten, rmallon, davem, ynezz,
Mika Westerberg
In-Reply-To: <a908e30a30247a07b2352002742f291d1df0eb18.1307816881.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>
Acked-by: Russell King <rmk+kernel@arm.linux.org.uk>
Acked-by: H Hartley Sweeten <hsweeten@visionengravers.com>
Tested-by: Petr Stetiar <ynezz@true.cz>
---
drivers/net/arm/ep93xx_eth.c | 18 +++++++++++++-----
1 files changed, 13 insertions(+), 5 deletions(-)
diff --git a/drivers/net/arm/ep93xx_eth.c b/drivers/net/arm/ep93xx_eth.c
index bef3811..0b46b8e 100644
--- a/drivers/net/arm/ep93xx_eth.c
+++ b/drivers/net/arm/ep93xx_eth.c
@@ -283,10 +283,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(dev->dev.parent, ep->descs->rdesc[entry].buf_addr,
+ dma_sync_single_for_cpu(dev->dev.parent, rxd->buf_addr,
length, DMA_FROM_DEVICE);
skb_copy_to_linear_data(skb, ep->rx_buf[entry], length);
+ dma_sync_single_for_device(dev->dev.parent,
+ rxd->buf_addr, length,
+ DMA_FROM_DEVICE);
skb_put(skb, length);
skb->protocol = eth_type_trans(skb, dev);
@@ -348,6 +352,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)) {
@@ -359,11 +364,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(dev->dev.parent, txd->buf_addr, skb->len,
+ DMA_TO_DEVICE);
skb_copy_and_csum_dev(skb, ep->tx_buf[entry]);
- dma_sync_single_for_cpu(dev->dev.parent, ep->descs->tdesc[entry].buf_addr,
- skb->len, DMA_TO_DEVICE);
+ dma_sync_single_for_device(dev->dev.parent, 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 v3 4/5] net: ep93xx_eth: drop GFP_DMA from call to dma_alloc_coherent()
From: Mika Westerberg @ 2011-06-11 18:39 UTC (permalink / raw)
To: netdev; +Cc: linux-arm-kernel, hsweeten, rmallon, davem, ynezz,
Mika Westerberg
In-Reply-To: <a908e30a30247a07b2352002742f291d1df0eb18.1307816881.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>
Acked-by: Russell King <rmk+kernel@arm.linux.org.uk>
Acked-by: H Hartley Sweeten <hsweeten@visionengravers.com>
Tested-by: Petr Stetiar <ynezz@true.cz>
---
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 97bf6b1..bef3811 100644
--- a/drivers/net/arm/ep93xx_eth.c
+++ b/drivers/net/arm/ep93xx_eth.c
@@ -492,7 +492,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 v3 3/5] net: ep93xx_eth: allocate buffers using kmalloc()
From: Mika Westerberg @ 2011-06-11 18:39 UTC (permalink / raw)
To: netdev; +Cc: linux-arm-kernel, hsweeten, rmallon, davem, ynezz,
Mika Westerberg
In-Reply-To: <a908e30a30247a07b2352002742f291d1df0eb18.1307816881.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>
Acked-by: Russell King <rmk+kernel@arm.linux.org.uk>
Acked-by: H Hartley Sweeten <hsweeten@visionengravers.com>
Tested-by: Petr Stetiar <ynezz@true.cz>
---
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 f65dfb6..97bf6b1 100644
--- a/drivers/net/arm/ep93xx_eth.c
+++ b/drivers/net/arm/ep93xx_eth.c
@@ -460,36 +460,32 @@ static void ep93xx_free_buffers(struct ep93xx_priv *ep)
struct device *dev = ep->dev->dev.parent;
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->dev->dev.parent;
@@ -500,48 +496,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 v3 2/5] net: ep93xx_eth: pass struct device to DMA API functions
From: Mika Westerberg @ 2011-06-11 18:39 UTC (permalink / raw)
To: netdev; +Cc: linux-arm-kernel, hsweeten, rmallon, davem, ynezz,
Mika Westerberg
In-Reply-To: <a908e30a30247a07b2352002742f291d1df0eb18.1307816881.git.mika.westerberg@iki.fi>
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>
---
I modified this patch according to Hartley's comments; There is no separate
dma_dev pointer in ep93xx_priv structure but we use SET_NETDEV_DEV() to store
pointer to the platform device which we use as the device for DMA API.
Because this patch is different than the original one, I dropped all the acks
and tested-by's.
drivers/net/arm/ep93xx_eth.c | 23 +++++++++++++----------
1 files changed, 13 insertions(+), 10 deletions(-)
diff --git a/drivers/net/arm/ep93xx_eth.c b/drivers/net/arm/ep93xx_eth.c
index 5a77001..f65dfb6 100644
--- a/drivers/net/arm/ep93xx_eth.c
+++ b/drivers/net/arm/ep93xx_eth.c
@@ -284,7 +284,7 @@ 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(dev->dev.parent, 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 +362,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(dev->dev.parent, ep->descs->tdesc[entry].buf_addr,
skb->len, DMA_TO_DEVICE);
dev_kfree_skb(skb);
@@ -457,6 +457,7 @@ static irqreturn_t ep93xx_irq(int irq, void *dev_id)
static void ep93xx_free_buffers(struct ep93xx_priv *ep)
{
+ struct device *dev = ep->dev->dev.parent;
int i;
for (i = 0; i < RX_QUEUE_ENTRIES; i += 2) {
@@ -464,7 +465,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 +476,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 +492,10 @@ static void ep93xx_free_buffers(struct ep93xx_priv *ep)
*/
static int ep93xx_alloc_buffers(struct ep93xx_priv *ep)
{
+ struct device *dev = ep->dev->dev.parent;
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 +508,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 +531,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 +831,7 @@ static int ep93xx_eth_probe(struct platform_device *pdev)
}
ep = netdev_priv(dev);
ep->dev = dev;
+ SET_NETDEV_DEV(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 v3 1/5] ep93xx: set DMA masks for the ep93xx_eth
From: Mika Westerberg @ 2011-06-11 18:39 UTC (permalink / raw)
To: netdev; +Cc: linux-arm-kernel, hsweeten, rmallon, davem, ynezz,
Mika Westerberg
Since the driver uses the DMA API, we should pass it valid DMA masks.
Signed-off-by: Mika Westerberg <mika.westerberg@iki.fi>
Acked-by: Russell King <rmk+kernel@arm.linux.org.uk>
Acked-by: H Hartley Sweeten <hsweeten@visionengravers.com>
Tested-by: Petr Stetiar <ynezz@true.cz>
---
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
* [PATCH] drivers/ssb/driver_chipcommon_pmu.c: uninitilized warning
From: Connor Hansen @ 2011-06-11 18:14 UTC (permalink / raw)
To: mb; +Cc: netdev, linux-kernel, Connor Hansen
warning message
drivers/ssb/driver_chipcommon_pmu.c: In function ssb_pmu_resources_init
drivers/ssb/driver_chipcommon_pmu.c:420:15: warning: updown_tab_size may
be used uninitilized in this function.
updown_tab_size and depend_tab_size may not be set in the bus->chip_id
switch statement, so set to 0 by default to avoid using uninitialized
stack space.
Signed-off-by: Connor Hansen <cmdkhh@gmail.com>
---
drivers/ssb/driver_chipcommon_pmu.c | 4 ++--
1 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/ssb/driver_chipcommon_pmu.c b/drivers/ssb/driver_chipcommon_pmu.c
index 305ade7..a7aef47 100644
--- a/drivers/ssb/driver_chipcommon_pmu.c
+++ b/drivers/ssb/driver_chipcommon_pmu.c
@@ -417,9 +417,9 @@ static void ssb_pmu_resources_init(struct ssb_chipcommon *cc)
u32 min_msk = 0, max_msk = 0;
unsigned int i;
const struct pmu_res_updown_tab_entry *updown_tab = NULL;
- unsigned int updown_tab_size;
+ unsigned int updown_tab_size = 0;
const struct pmu_res_depend_tab_entry *depend_tab = NULL;
- unsigned int depend_tab_size;
+ unsigned int depend_tab_size = 0;
switch (bus->chip_id) {
case 0x4312:
--
1.7.4.4
^ permalink raw reply related
* [PATCH] ISDN, hfcsusb: Don't leak in hfcsusb_ph_info()
From: Jesper Juhl @ 2011-06-11 16:36 UTC (permalink / raw)
To: Karsten Keil
Cc: David S. Miller, netdev, linux-kernel, Peter Sprenger,
Martin Bachem
We leak the memory allocated to 'phi' when the variable goes out of scope
in hfcsusb_ph_info().
Signed-off-by: Jesper Juhl <jj@chaosbits.net>
---
hfcsusb.c | 1 +
1 file changed, 1 insertion(+)
I have no way to really test this patch, so it is only compile tested.
diff --git a/drivers/isdn/hardware/mISDN/hfcsusb.c b/drivers/isdn/hardware/mISDN/hfcsusb.c
index 3ccbff1..71a8eb6 100644
--- a/drivers/isdn/hardware/mISDN/hfcsusb.c
+++ b/drivers/isdn/hardware/mISDN/hfcsusb.c
@@ -283,6 +283,7 @@ hfcsusb_ph_info(struct hfcsusb *hw)
_queue_data(&dch->dev.D, MPH_INFORMATION_IND, MISDN_ID_ANY,
sizeof(struct ph_info_dch) + dch->dev.nrbchan *
sizeof(struct ph_info_ch), phi, GFP_ATOMIC);
+ kfree(phi);
}
/*
--
Jesper Juhl <jj@chaosbits.net> http://www.chaosbits.net/
Don't top-post http://www.catb.org/jargon/html/T/top-post.html
Plain text mails only, please.
^ permalink raw reply related
* [patch] netpoll: call dev_put() on error in netpoll_setup()
From: Dan Carpenter @ 2011-06-11 15:50 UTC (permalink / raw)
To: WANG Cong
Cc: Herbert Xu, Neil Horman, David S. Miller, Eric Dumazet,
open list:NETWORKING [GENERAL], kernel-janitors
There is a dev_put(ndev) missing on an error path. This was
introduced in 0c1ad04aecb "netpoll: prevent netpoll setup on slave
devices".
Signed-off-by: Dan Carpenter <error27@gmail.com>
---
This is a static checker bug, and it's possible I've misunderstood
something.
diff --git a/net/core/netpoll.c b/net/core/netpoll.c
index 42ea4b0..18d9cbd 100644
--- a/net/core/netpoll.c
+++ b/net/core/netpoll.c
@@ -795,7 +795,8 @@ int netpoll_setup(struct netpoll *np)
if (ndev->master) {
printk(KERN_ERR "%s: %s is a slave device, aborting.\n",
np->name, np->dev_name);
- return -EBUSY;
+ err = -EBUSY;
+ goto put;
}
if (!netif_running(ndev)) {
^ permalink raw reply related
* bnx2 udp multicast packet loss and jtter
From: Kieran Kunhya @ 2011-06-11 14:32 UTC (permalink / raw)
To: netdev
Hello,
I get some bad packet loss and high jitter on a BCM5709. This is on
kernel 2.6.38-8 - unfortunately using a newer kernel isn't an option
right now.
The bnx2 driver is version v2.0.23b.
I've tried a lot of kenel types such as -lowlatency, -ck and vanilla.
The thread doing the udp output is at realtime priority and the same
thing happens with different applications.
Example of jitter
http://dl.dropbox.com/u/2701213/jitter.png
Is there anything I can do to further diagnose this problem?
Regards,
Kieran Kunhya
^ permalink raw reply
* Re: linux-next: build failure after merge of the net tree
From: Geert Uytterhoeven @ 2011-06-11 11:39 UTC (permalink / raw)
To: Stephen Rothwell
Cc: David Miller, netdev, linux-next, linux-kernel, Greg KH,
Alexey Dobriyan
In-Reply-To: <20110611142648.bf71b026.sfr@canb.auug.org.au>
On Sat, Jun 11, 2011 at 06:26, Stephen Rothwell <sfr@canb.auug.org.au> wrote:
> On Fri, 10 Jun 2011 22:28:03 +0200 Geert Uytterhoeven <geert@linux-m68k.org> wrote:
>> My first bisection between two successive versions of linux-next failed, but
>
> Bisecting between successive linux-next versions is bound not to work.
> Bisection really only works between a commit and one of ist ancestors.
Yeah, I should have known that...
>> the second one between linus and linux-next just finished and points
>> to that commit.
>
> That is a much better bet.
Patch has been sent.
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
^ permalink raw reply
* [PATCH -next] net/m68k: Include <linux/interrupt.h> where needed
From: Geert Uytterhoeven @ 2011-06-11 11:38 UTC (permalink / raw)
To: Alexey Dobriyan, David Miller
Cc: netdev, linux-next, linux-kernel, Geert Uytterhoeven
arch/m68k/emu/nfeth.c: In function ‘nfeth_init’:
arch/m68k/emu/nfeth.c:243: error: implicit declaration of function ‘request_irq’
arch/m68k/emu/nfeth.c:243: error: ‘IRQF_SHARED’ undeclared (first use in this function)
arch/m68k/emu/nfeth.c:243: error: (Each undeclared identifier is reported only once
arch/m68k/emu/nfeth.c:243: error: for each function it appears in.)
arch/m68k/emu/nfeth.c: In function ‘nfeth_cleanup’:
arch/m68k/emu/nfeth.c:266: error: implicit declaration of function ‘free_irq’
drivers/net/apne.c: In function ‘apne_probe’:
drivers/net/apne.c:189: error: implicit declaration of function ‘free_irq’
drivers/net/apne.c: In function ‘apne_probe1’:
drivers/net/apne.c:317: error: implicit declaration of function ‘request_irq’
drivers/net/apne.c:317: error: ‘IRQF_SHARED’ undeclared (first use in this function)
drivers/net/apne.c:317: error: (Each undeclared identifier is reported only once
drivers/net/apne.c:317: error: for each function it appears in.)
Introduced by commit a6b7a407865a ("net: remove interrupt.h inclusion from
netdevice.h").
Include <linux/interrupt.h> in the individual drivers to fix the build.
Signed-off-by: Geert Uytterhoeven <geert@linux-m68k.org>
---
arch/m68k/emu/nfeth.c | 1 +
drivers/net/apne.c | 1 +
2 files changed, 2 insertions(+), 0 deletions(-)
diff --git a/arch/m68k/emu/nfeth.c b/arch/m68k/emu/nfeth.c
index 8b6e201..3c55312 100644
--- a/arch/m68k/emu/nfeth.c
+++ b/arch/m68k/emu/nfeth.c
@@ -16,6 +16,7 @@
#include <linux/netdevice.h>
#include <linux/etherdevice.h>
+#include <linux/interrupt.h>
#include <linux/module.h>
#include <asm/natfeat.h>
#include <asm/virtconvert.h>
diff --git a/drivers/net/apne.c b/drivers/net/apne.c
index 2fe60f1..5477373 100644
--- a/drivers/net/apne.c
+++ b/drivers/net/apne.c
@@ -36,6 +36,7 @@
#include <linux/delay.h>
#include <linux/netdevice.h>
#include <linux/etherdevice.h>
+#include <linux/interrupt.h>
#include <linux/jiffies.h>
#include <asm/system.h>
--
1.7.0.4
^ permalink raw reply related
* Dear Friend
From: Liu Yan @ 2011-06-11 10:53 UTC (permalink / raw)
Dear Friend,
I am Mr. Liu Yan a transfer supervisor in Bank of China Ltd. I have a business plan for you.
On June 5, 2002, a client made a numbered time (Fixed) Deposit in my branch and upon maturity, I sent a routine notification to his forwarding address but got no reply and later discovered from his contract employers that he died from an auto-mobile accident.
On further investigation, I found out that he died without making a WILL and he did not declare any next of kin or relations in all his official documents with us.
The money is still sitting in my Bank and the interest is being rolled over with the principal sum at the end of each year. No one will ever come forward to claim it.
Consequently, my proposal is that I will like you as a foreigner to stand in as the next of kin to this client so we can share the money before it falls into the hands of some corrupt government officials.
There is no risk at all as my position as the credit officer guarantees the successful execution of this transaction.
If you are interested, please reply immediately to my private email box, ly@ostatmig.com
Regards,
Liu Yan
^ permalink raw reply
* [smatch] re: bonding:delete lacp_fast from ad_bond_info
From: Dan Carpenter @ 2011-06-11 8:12 UTC (permalink / raw)
To: panweiping3; +Cc: open list:BONDING DRIVER
There was a dereference before a check added in 56d00c677de0a
"bonding:delete lacp_fast from ad_bond_info"
drivers/net/bonding/bond_3ad.c +1907 bond_3ad_bind_slave(7)
warn: variable dereferenced before check 'bond'
1900 int bond_3ad_bind_slave(struct slave *slave)
1901 {
1902 struct bonding *bond = bond_get_bond_by_slave(slave);
1903 int lacp_fast = bond->params.lacp_fast;
^^^^^^^^^^^^
dereference.
1904 struct port *port;
1905 struct aggregator *aggregator;
1906
1907 if (bond == NULL) {
^^^^^^^^^^^^
check.
1908 pr_err("%s: The slave %s is not attached to its bond\n",
1909 slave->dev->master->name, slave->dev->name);
1910 return -1;
1911 }
regards,
dan carpenter
^ permalink raw reply
* Re: [PATCH net-next-2.6] inetpeer: lower false sharing effect
From: Andi Kleen @ 2011-06-11 7:09 UTC (permalink / raw)
To: Changli Gao; +Cc: Eric Dumazet, Tim Chen, David Miller, netdev, andi
In-Reply-To: <BANLkTim4ikjYJjdW8AWjmPi8hx-a0nh=Ug@mail.gmail.com>
On Sat, Jun 11, 2011 at 02:17:24PM +0800, Changli Gao wrote:
> On Sat, Jun 11, 2011 at 12:54 PM, Eric Dumazet <eric.dumazet@gmail.com> wrote:
> > Le samedi 11 juin 2011 à 08:54 +0800, Changli Gao a écrit :
> >
> >
> > Whole point of the exercice is to prepare ground for routing cache
> > removal :)
> >
> > If you want a server being hit by millions of clients around the world,
> > routing cache is a real pain because of memory needs.
> >
>
> Yes. I know the routing cache removal is our goal. But for his
> scenario, there aren't so many routing cache entries, so routing cache
> may be a better option currently. However, if he just wants to
The routing cache has terrible cache line bouncing in the reference
count too.
-andi
^ permalink raw reply
* Re: [PATCH net-next-2.6] inetpeer: lower false sharing effect
From: Changli Gao @ 2011-06-11 6:17 UTC (permalink / raw)
To: Eric Dumazet; +Cc: Tim Chen, David Miller, netdev, andi
In-Reply-To: <1307768052.2872.50.camel@edumazet-laptop>
On Sat, Jun 11, 2011 at 12:54 PM, Eric Dumazet <eric.dumazet@gmail.com> wrote:
> Le samedi 11 juin 2011 à 08:54 +0800, Changli Gao a écrit :
>
>
> Whole point of the exercice is to prepare ground for routing cache
> removal :)
>
> If you want a server being hit by millions of clients around the world,
> routing cache is a real pain because of memory needs.
>
Yes. I know the routing cache removal is our goal. But for his
scenario, there aren't so many routing cache entries, so routing cache
may be a better option currently. However, if he just wants to
evaluate the effect of the routing cache removal, it is another thing.
:)
--
Regards,
Changli Gao(xiaosuo@gmail.com)
^ permalink raw reply
* [PATCH net-next-2.6] snmp: reduce percpu needs by 50%
From: Eric Dumazet @ 2011-06-11 5:45 UTC (permalink / raw)
To: David Miller
Cc: netdev, Andi Kleen, linux-kernel, Benjamin Herrenschmidt,
Ingo Molnar, Tejun Heo, Christoph Lameter, linux-arch
In-Reply-To: <20110608.235113.2177191884416780789.davem@davemloft.net>
SNMP mibs use two percpu arrays, one used in BH context, another in USER
context. With increasing number of cpus in machines, and fact that ipv6
uses per network device ipstats_mib, this is consuming a lot of memory
if many network devices are registered.
commit be281e554e2a (ipv6: reduce per device ICMP mib sizes) shrinked
percpu needs for ipv6, but we can reduce memory use a bit more.
With recent percpu infrastructure (irqsafe_cpu_inc() ...), we no longer
need this BH/USER separation since we can update counters in a single
x86 instruction, regardless of the BH/USER context.
Other arches than x86 might need to disable irq in their
irqsafe_cpu_inc() implementation : If this happens to be a problem, we
can make SNMP_ARRAY_SZ arch dependent, but a previous poll
( https://lkml.org/lkml/2011/3/17/174 ) to arch maintainers did not
raise strong opposition.
Only on 32bit arches, we need to disable BH for 64bit counters updates
done from USER context (currently used for IP MIB)
This also reduces vmlinux size :
1) x86_64 build
$ size vmlinux.before vmlinux.after
text data bss dec hex filename
7853650 1293772 1896448 11043870 a8841e vmlinux.before
7850578 1293772 1896448 11040798 a8781e vmlinux.after
2) i386 build
$ size vmlinux.before vmlinux.afterpatch
text data bss dec hex filename
6039335 635076 3670016 10344427 9dd7eb vmlinux.before
6037342 635076 3670016 10342434 9dd022 vmlinux.afterpatch
Signed-off-by: Eric Dumazet <eric.dumazet@gmail.com>
CC: Andi Kleen <andi@firstfloor.org>
CC: Ingo Molnar <mingo@elte.hu>
CC: Tejun Heo <tj@kernel.org>
CC: Christoph Lameter <cl@linux-foundation.org>
CC: Benjamin Herrenschmidt <benh@kernel.crashing.org
CC: linux-arch@vger.kernel.org
---
include/net/snmp.h | 85 ++++++++++++++++---------------------------
net/ipv4/af_inet.c | 52 +++++++++++---------------
2 files changed, 55 insertions(+), 82 deletions(-)
diff --git a/include/net/snmp.h b/include/net/snmp.h
index 479083a..8f0f9ac 100644
--- a/include/net/snmp.h
+++ b/include/net/snmp.h
@@ -116,57 +116,51 @@ struct linux_xfrm_mib {
unsigned long mibs[LINUX_MIB_XFRMMAX];
};
-/*
- * FIXME: On x86 and some other CPUs the split into user and softirq parts
- * is not needed because addl $1,memory is atomic against interrupts (but
- * atomic_inc would be overkill because of the lock cycles). Wants new
- * nonlocked_atomic_inc() primitives -AK
- */
+#define SNMP_ARRAY_SZ 1
+
#define DEFINE_SNMP_STAT(type, name) \
- __typeof__(type) __percpu *name[2]
+ __typeof__(type) __percpu *name[SNMP_ARRAY_SZ]
#define DEFINE_SNMP_STAT_ATOMIC(type, name) \
__typeof__(type) *name
#define DECLARE_SNMP_STAT(type, name) \
- extern __typeof__(type) __percpu *name[2]
-
-#define SNMP_STAT_BHPTR(name) (name[0])
-#define SNMP_STAT_USRPTR(name) (name[1])
+ extern __typeof__(type) __percpu *name[SNMP_ARRAY_SZ]
#define SNMP_INC_STATS_BH(mib, field) \
__this_cpu_inc(mib[0]->mibs[field])
+
#define SNMP_INC_STATS_USER(mib, field) \
- this_cpu_inc(mib[1]->mibs[field])
+ irqsafe_cpu_inc(mib[0]->mibs[field])
+
#define SNMP_INC_STATS_ATOMIC_LONG(mib, field) \
atomic_long_inc(&mib->mibs[field])
+
#define SNMP_INC_STATS(mib, field) \
- this_cpu_inc(mib[!in_softirq()]->mibs[field])
+ irqsafe_cpu_inc(mib[0]->mibs[field])
+
#define SNMP_DEC_STATS(mib, field) \
- this_cpu_dec(mib[!in_softirq()]->mibs[field])
+ irqsafe_cpu_dec(mib[0]->mibs[field])
+
#define SNMP_ADD_STATS_BH(mib, field, addend) \
__this_cpu_add(mib[0]->mibs[field], addend)
+
#define SNMP_ADD_STATS_USER(mib, field, addend) \
- this_cpu_add(mib[1]->mibs[field], addend)
+ irqsafe_cpu_add(mib[0]->mibs[field], addend)
+
#define SNMP_ADD_STATS(mib, field, addend) \
- this_cpu_add(mib[!in_softirq()]->mibs[field], addend)
+ irqsafe_cpu_add(mib[0]->mibs[field], addend)
/*
* Use "__typeof__(*mib[0]) *ptr" instead of "__typeof__(mib[0]) ptr"
* to make @ptr a non-percpu pointer.
*/
#define SNMP_UPD_PO_STATS(mib, basefield, addend) \
do { \
- __typeof__(*mib[0]) *ptr; \
- preempt_disable(); \
- ptr = this_cpu_ptr((mib)[!in_softirq()]); \
- ptr->mibs[basefield##PKTS]++; \
- ptr->mibs[basefield##OCTETS] += addend;\
- preempt_enable(); \
+ irqsafe_cpu_inc(mib[0]->mibs[basefield##PKTS]); \
+ irqsafe_cpu_add(mib[0]->mibs[basefield##OCTETS], addend); \
} while (0)
#define SNMP_UPD_PO_STATS_BH(mib, basefield, addend) \
do { \
- __typeof__(*mib[0]) *ptr = \
- __this_cpu_ptr((mib)[0]); \
- ptr->mibs[basefield##PKTS]++; \
- ptr->mibs[basefield##OCTETS] += addend;\
+ __this_cpu_inc(mib[0]->mibs[basefield##PKTS]); \
+ __this_cpu_add(mib[0]->mibs[basefield##OCTETS], addend); \
} while (0)
@@ -179,40 +173,20 @@ struct linux_xfrm_mib {
ptr->mibs[field] += addend; \
u64_stats_update_end(&ptr->syncp); \
} while (0)
+
#define SNMP_ADD_STATS64_USER(mib, field, addend) \
do { \
- __typeof__(*mib[0]) *ptr; \
- preempt_disable(); \
- ptr = __this_cpu_ptr((mib)[1]); \
- u64_stats_update_begin(&ptr->syncp); \
- ptr->mibs[field] += addend; \
- u64_stats_update_end(&ptr->syncp); \
- preempt_enable(); \
+ local_bh_disable(); \
+ SNMP_ADD_STATS64_BH(mib, field, addend); \
+ local_bh_enable(); \
} while (0)
+
#define SNMP_ADD_STATS64(mib, field, addend) \
- do { \
- __typeof__(*mib[0]) *ptr; \
- preempt_disable(); \
- ptr = __this_cpu_ptr((mib)[!in_softirq()]); \
- u64_stats_update_begin(&ptr->syncp); \
- ptr->mibs[field] += addend; \
- u64_stats_update_end(&ptr->syncp); \
- preempt_enable(); \
- } while (0)
+ SNMP_ADD_STATS64_USER(mib, field, addend)
+
#define SNMP_INC_STATS64_BH(mib, field) SNMP_ADD_STATS64_BH(mib, field, 1)
#define SNMP_INC_STATS64_USER(mib, field) SNMP_ADD_STATS64_USER(mib, field, 1)
#define SNMP_INC_STATS64(mib, field) SNMP_ADD_STATS64(mib, field, 1)
-#define SNMP_UPD_PO_STATS64(mib, basefield, addend) \
- do { \
- __typeof__(*mib[0]) *ptr; \
- preempt_disable(); \
- ptr = __this_cpu_ptr((mib)[!in_softirq()]); \
- u64_stats_update_begin(&ptr->syncp); \
- ptr->mibs[basefield##PKTS]++; \
- ptr->mibs[basefield##OCTETS] += addend; \
- u64_stats_update_end(&ptr->syncp); \
- preempt_enable(); \
- } while (0)
#define SNMP_UPD_PO_STATS64_BH(mib, basefield, addend) \
do { \
__typeof__(*mib[0]) *ptr; \
@@ -222,6 +196,12 @@ struct linux_xfrm_mib {
ptr->mibs[basefield##OCTETS] += addend; \
u64_stats_update_end(&ptr->syncp); \
} while (0)
+#define SNMP_UPD_PO_STATS64(mib, basefield, addend) \
+ do { \
+ local_bh_disable(); \
+ SNMP_UPD_PO_STATS64_BH(mib, basefield, addend); \
+ local_bh_enable(); \
+ } while (0)
#else
#define SNMP_INC_STATS64_BH(mib, field) SNMP_INC_STATS_BH(mib, field)
#define SNMP_INC_STATS64_USER(mib, field) SNMP_INC_STATS_USER(mib, field)
diff --git a/net/ipv4/af_inet.c b/net/ipv4/af_inet.c
index 9c19260..260ccf0 100644
--- a/net/ipv4/af_inet.c
+++ b/net/ipv4/af_inet.c
@@ -1437,11 +1437,11 @@ EXPORT_SYMBOL_GPL(inet_ctl_sock_create);
unsigned long snmp_fold_field(void __percpu *mib[], int offt)
{
unsigned long res = 0;
- int i;
+ int i, j;
for_each_possible_cpu(i) {
- res += *(((unsigned long *) per_cpu_ptr(mib[0], i)) + offt);
- res += *(((unsigned long *) per_cpu_ptr(mib[1], i)) + offt);
+ for (j = 0; j < SNMP_ARRAY_SZ; j++)
+ res += *(((unsigned long *) per_cpu_ptr(mib[j], i)) + offt);
}
return res;
}
@@ -1455,28 +1455,19 @@ u64 snmp_fold_field64(void __percpu *mib[], int offt, size_t syncp_offset)
int cpu;
for_each_possible_cpu(cpu) {
- void *bhptr, *userptr;
+ void *bhptr;
struct u64_stats_sync *syncp;
- u64 v_bh, v_user;
+ u64 v;
unsigned int start;
- /* first mib used by softirq context, we must use _bh() accessors */
- bhptr = per_cpu_ptr(SNMP_STAT_BHPTR(mib), cpu);
+ bhptr = per_cpu_ptr(mib[0], cpu);
syncp = (struct u64_stats_sync *)(bhptr + syncp_offset);
do {
start = u64_stats_fetch_begin_bh(syncp);
- v_bh = *(((u64 *) bhptr) + offt);
+ v = *(((u64 *) bhptr) + offt);
} while (u64_stats_fetch_retry_bh(syncp, start));
- /* second mib used in USER context */
- userptr = per_cpu_ptr(SNMP_STAT_USRPTR(mib), cpu);
- syncp = (struct u64_stats_sync *)(userptr + syncp_offset);
- do {
- start = u64_stats_fetch_begin(syncp);
- v_user = *(((u64 *) userptr) + offt);
- } while (u64_stats_fetch_retry(syncp, start));
-
- res += v_bh + v_user;
+ res += v;
}
return res;
}
@@ -1488,25 +1479,28 @@ int snmp_mib_init(void __percpu *ptr[2], size_t mibsize, size_t align)
BUG_ON(ptr == NULL);
ptr[0] = __alloc_percpu(mibsize, align);
if (!ptr[0])
- goto err0;
+ return -ENOMEM;
+#if SNMP_ARRAY_SZ == 2
ptr[1] = __alloc_percpu(mibsize, align);
- if (!ptr[1])
- goto err1;
+ if (!ptr[1]) {
+ free_percpu(ptr[0]);
+ ptr[0] = NULL;
+ return -ENOMEM;
+ }
+#endif
return 0;
-err1:
- free_percpu(ptr[0]);
- ptr[0] = NULL;
-err0:
- return -ENOMEM;
}
EXPORT_SYMBOL_GPL(snmp_mib_init);
-void snmp_mib_free(void __percpu *ptr[2])
+void snmp_mib_free(void __percpu *ptr[SNMP_ARRAY_SZ])
{
+ int i;
+
BUG_ON(ptr == NULL);
- free_percpu(ptr[0]);
- free_percpu(ptr[1]);
- ptr[0] = ptr[1] = NULL;
+ for (i = 0; i < SNMP_ARRAY_SZ; i++) {
+ free_percpu(ptr[i]);
+ ptr[i] = NULL;
+ }
}
EXPORT_SYMBOL_GPL(snmp_mib_free);
^ permalink raw reply related
* Re: [net-next 13/13] ixgbe: use per NUMA node lock for FCoE DDP
From: Eric Dumazet @ 2011-06-11 5:42 UTC (permalink / raw)
To: Jeff Kirsher; +Cc: davem, Vasu Dev, netdev, gospo
In-Reply-To: <1307769482.2872.62.camel@edumazet-laptop>
Le samedi 11 juin 2011 à 07:18 +0200, Eric Dumazet a écrit :
> > /**
> > diff --git a/drivers/net/ixgbe/ixgbe_fcoe.h b/drivers/net/ixgbe/ixgbe_fcoe.h
> > index d876e7a..8618892 100644
> > --- a/drivers/net/ixgbe/ixgbe_fcoe.h
> > +++ b/drivers/net/ixgbe/ixgbe_fcoe.h
> > @@ -69,6 +69,7 @@ struct ixgbe_fcoe {
> > struct pci_pool **pool;
> > atomic_t refcnt;
> > spinlock_t lock;
> > + struct spinlock **node_lock;
>
> Wont this read_mostly pointer sits in often modified cache line ?
>
> > struct ixgbe_fcoe_ddp ddp[IXGBE_FCOE_DDP_MAX];
> > unsigned char *extra_ddp_buffer;
> > dma_addr_t extra_ddp_buffer_dma;
>
This patch seems overkill to me, have you tried the more simple way I
did in commit 79640a4ca6955e3ebdb7038508fa7a0cd7fa5527
(net: add additional lock to qdisc to increase throughput )
(remember you must place ->busylock in a separate cache line, to not
slow down the two cpus that have access to ->lock)
struct ixgbe_fcoe could probably be more carefuly reordered to lower
false sharing
I kindly ask you guys provide actual perf numbers between
1) before any patch
2) After your multilevel per numanode locks
3) A more simple way (my suggestion of adding a single 'busylock')
Thanks
^ permalink raw reply
* Re: [net-next 13/13] ixgbe: use per NUMA node lock for FCoE DDP
From: Eric Dumazet @ 2011-06-11 5:18 UTC (permalink / raw)
To: Jeff Kirsher; +Cc: davem, Vasu Dev, netdev, gospo
In-Reply-To: <1307761341-5267-14-git-send-email-jeffrey.t.kirsher@intel.com>
Le vendredi 10 juin 2011 à 20:02 -0700, Jeff Kirsher a écrit :
> From: Vasu Dev <vasu.dev@intel.com>
>
> Adds per NUMA node lock to have CPU pass thru its NUMA lock
> first before contending for global DDP fcoe->lock to setup
> DDP lock, this is to reduce contentions across NUMA nodes.
>
> Allocates and initialize per NUMA node lock in added
> ixgbe_fcoe_lock_init and then have current CPU's numa_node_id
> based NUMA node lock acquired before taking global fcoe->lock.
>
> The node lock is allocated from its NUMA node using kzalloc_node.
>
> Signed-off-by: Vasu Dev <vasu.dev@intel.com>
> Tested-by: Ross Brattain <ross.b.brattain@intel.com>
> Signed-off-by: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
> ---
> drivers/net/ixgbe/ixgbe_fcoe.c | 50 ++++++++++++++++++++++++++++++++++++++-
> drivers/net/ixgbe/ixgbe_fcoe.h | 1 +
> 2 files changed, 49 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/net/ixgbe/ixgbe_fcoe.c b/drivers/net/ixgbe/ixgbe_fcoe.c
> index f5f39ed..aadff4f 100644
> --- a/drivers/net/ixgbe/ixgbe_fcoe.c
> +++ b/drivers/net/ixgbe/ixgbe_fcoe.c
> @@ -109,6 +109,7 @@ int ixgbe_fcoe_ddp_put(struct net_device *netdev, u16 xid)
> len = ddp->len;
> /* if there an error, force to invalidate ddp context */
> if (ddp->err) {
> + spin_lock(fcoe->node_lock[numa_node_id()]);
> spin_lock_bh(&fcoe->lock);
> IXGBE_WRITE_REG(&adapter->hw, IXGBE_FCFLT, 0);
> IXGBE_WRITE_REG(&adapter->hw, IXGBE_FCFLTRW,
> @@ -122,6 +123,7 @@ int ixgbe_fcoe_ddp_put(struct net_device *netdev, u16 xid)
> (xid | IXGBE_FCDMARW_RE));
> fcbuff = IXGBE_READ_REG(&adapter->hw, IXGBE_FCBUFF);
> spin_unlock_bh(&fcoe->lock);
> + spin_unlock(fcoe->node_lock[numa_node_id()]);
> if (fcbuff & IXGBE_FCBUFF_VALID)
> udelay(100);
> }
> @@ -294,6 +296,7 @@ static int ixgbe_fcoe_ddp_setup(struct net_device *netdev, u16 xid,
>
> /* program DMA context */
> hw = &adapter->hw;
> + spin_lock(fcoe->node_lock[numa_node_id()]);
> spin_lock_bh(&fcoe->lock);
>
> /* turn on last frame indication for target mode as FCP_RSPtarget is
> @@ -315,6 +318,7 @@ static int ixgbe_fcoe_ddp_setup(struct net_device *netdev, u16 xid,
> IXGBE_WRITE_REG(hw, IXGBE_FCFLTRW, fcfltrw);
>
> spin_unlock_bh(&fcoe->lock);
> + spin_unlock(fcoe->node_lock[numa_node_id()]);
>
> return 1;
>
> @@ -634,6 +638,42 @@ static void ixgbe_fcoe_ddp_pools_alloc(struct ixgbe_adapter *adapter)
> }
> }
>
> +static void ixgbe_fcoe_locks_free(struct ixgbe_fcoe *fcoe)
> +{
> + int node;
> +
> + if (!fcoe->node_lock)
> + return;
> +
> + for_each_node_with_cpus(node)
> + kfree(fcoe->node_lock[node]);
> +
> + kfree(fcoe->node_lock);
> + fcoe->node_lock = NULL;
> +}
> +
> +static void ixgbe_fcoe_lock_init(struct ixgbe_fcoe *fcoe)
> +{
> + int node;
> + spinlock_t *node_lock;
> +
> + fcoe->node_lock = kzalloc(sizeof(node_lock) * num_possible_nodes(),
> + GFP_KERNEL);
Hmm...
1) Think of what happens if some machine has 3 possible nodes : 0, 2, 3
-> You should use nr_node_ids instead of num_possible_nodes()
2) Make sure this block cant have false sharing : Allocate at least a
full cache line : On a typical 2 node machine, you currently allocate
16bytes of memory, and this small block could share a contended cache
line.
> + if (!fcoe->node_lock)
> + return;
> +
> + for_each_node_with_cpus(node) {
> + node_lock = kzalloc_node(sizeof(*node_lock) , GFP_KERNEL, node);
> + if (!node_lock) {
> + ixgbe_fcoe_locks_free(fcoe);
> + return;
> + }
> + spin_lock_init(node_lock);
> + fcoe->node_lock[node] = node_lock;
> + }
> + spin_lock_init(&fcoe->lock);
> +}
> +
...
>
> /**
> diff --git a/drivers/net/ixgbe/ixgbe_fcoe.h b/drivers/net/ixgbe/ixgbe_fcoe.h
> index d876e7a..8618892 100644
> --- a/drivers/net/ixgbe/ixgbe_fcoe.h
> +++ b/drivers/net/ixgbe/ixgbe_fcoe.h
> @@ -69,6 +69,7 @@ struct ixgbe_fcoe {
> struct pci_pool **pool;
> atomic_t refcnt;
> spinlock_t lock;
> + struct spinlock **node_lock;
Wont this read_mostly pointer sits in often modified cache line ?
> struct ixgbe_fcoe_ddp ddp[IXGBE_FCOE_DDP_MAX];
> unsigned char *extra_ddp_buffer;
> dma_addr_t extra_ddp_buffer_dma;
^ permalink raw reply
* Re: [PATCH net-next-2.6] inetpeer: lower false sharing effect
From: Eric Dumazet @ 2011-06-11 4:54 UTC (permalink / raw)
To: Changli Gao; +Cc: Tim Chen, David Miller, netdev, andi
In-Reply-To: <BANLkTikU_ug=zUdfOZAX6eJ+BD9ZZvDrJA@mail.gmail.com>
Le samedi 11 juin 2011 à 08:54 +0800, Changli Gao a écrit :
> Did you disable routing cache when profiling? If so, enable it and try again.
>
Whole point of the exercice is to prepare ground for routing cache
removal :)
If you want a server being hit by millions of clients around the world,
routing cache is a real pain because of memory needs.
^ 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