Netdev List
 help / color / mirror / Atom feed
* [PATCH v2 03/11] hso: fix memory leak when device disconnects
From: Olivier Sobrie @ 2015-01-30 12:21 UTC (permalink / raw)
  To: Jan Dumon, Dan Williams; +Cc: linux-kernel, linux-usb, netdev, Olivier Sobrie
In-Reply-To: <1422620523-15021-3-git-send-email-olivier@sobrie.be>

In the disconnect path, tx_buffer should freed like tx_data to avoid
a memory leak when the device disconnects.

Signed-off-by: Olivier Sobrie <olivier@sobrie.be>
---
 drivers/net/usb/hso.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/net/usb/hso.c b/drivers/net/usb/hso.c
index 3a6c630..470ef9e 100644
--- a/drivers/net/usb/hso.c
+++ b/drivers/net/usb/hso.c
@@ -2253,6 +2253,7 @@ static void hso_serial_common_free(struct hso_serial *serial)
 
 	/* unlink and free TX URB */
 	usb_free_urb(serial->tx_urb);
+	kfree(serial->tx_buffer);
 	kfree(serial->tx_data);
 	tty_port_destroy(&serial->port);
 }
-- 
2.2.0

^ permalink raw reply related

* [PATCH v2 02/11] hso: fix crash when device disappears while serial port is open
From: Olivier Sobrie @ 2015-01-30 12:21 UTC (permalink / raw)
  To: Jan Dumon, Dan Williams; +Cc: linux-kernel, linux-usb, netdev, Olivier Sobrie
In-Reply-To: <1422620523-15021-2-git-send-email-olivier@sobrie.be>

When the device disappear, the function hso_disconnect() is called to
perform cleanup. In the cleanup function, hso_free_interface() calls
tty_port_tty_hangup() in view of scheduling a work to hang up the tty if
needed. If the port was not open then hso_serial_ref_free() is called
directly to cleanup everything. Otherwise, hso_serial_ref_free() is called
when the last fd associated to the port is closed.

For each open port, tty_release() will call the close method,
hso_serial_close(), which drops the last kref and call
hso_serial_ref_free() which unregisters, destroys the tty port
and finally frees the structure in which the tty_port structure
is included. Later, in tty_release(), more precisely when release_tty()
is called, the tty_port previously freed is accessed to cancel
the tty buf workqueue and it leads to a crash.

In view of avoiding this crash, we add a cleanup method that is called
at the end of the hangup process and we drop the last kref in this
function when all the ports have been closed, when tty_port is no
more needed and when it is safe to free the structure containing the
tty_port structure.

Signed-off-by: Olivier Sobrie <olivier@sobrie.be>
---
 drivers/net/usb/hso.c | 17 +++++++++++++----
 1 file changed, 13 insertions(+), 4 deletions(-)

diff --git a/drivers/net/usb/hso.c b/drivers/net/usb/hso.c
index cb0bcc1..3a6c630 100644
--- a/drivers/net/usb/hso.c
+++ b/drivers/net/usb/hso.c
@@ -1270,7 +1270,6 @@ static int hso_serial_open(struct tty_struct *tty, struct file *filp)
 		goto err_out;
 
 	D1("Opening %d", serial->minor);
-	kref_get(&serial->parent->ref);
 
 	/* setup */
 	tty->driver_data = serial;
@@ -1289,7 +1288,8 @@ static int hso_serial_open(struct tty_struct *tty, struct file *filp)
 		if (result) {
 			hso_stop_serial_device(serial->parent);
 			serial->port.count--;
-			kref_put(&serial->parent->ref, hso_serial_ref_free);
+		} else {
+			kref_get(&serial->parent->ref);
 		}
 	} else {
 		D1("Port was already open");
@@ -1339,8 +1339,6 @@ static void hso_serial_close(struct tty_struct *tty, struct file *filp)
 		usb_autopm_put_interface(serial->parent->interface);
 
 	mutex_unlock(&serial->parent->mutex);
-
-	kref_put(&serial->parent->ref, hso_serial_ref_free);
 }
 
 /* close the requested serial port */
@@ -1391,6 +1389,16 @@ static int hso_serial_write_room(struct tty_struct *tty)
 	return room;
 }
 
+static void hso_serial_cleanup(struct tty_struct *tty)
+{
+	struct hso_serial *serial = tty->driver_data;
+
+	if (!serial)
+		return;
+
+	kref_put(&serial->parent->ref, hso_serial_ref_free);
+}
+
 /* setup the term */
 static void hso_serial_set_termios(struct tty_struct *tty, struct ktermios *old)
 {
@@ -3215,6 +3223,7 @@ static const struct tty_operations hso_serial_ops = {
 	.close = hso_serial_close,
 	.write = hso_serial_write,
 	.write_room = hso_serial_write_room,
+	.cleanup = hso_serial_cleanup,
 	.ioctl = hso_serial_ioctl,
 	.set_termios = hso_serial_set_termios,
 	.chars_in_buffer = hso_serial_chars_in_buffer,
-- 
2.2.0

^ permalink raw reply related

* [PATCH v2 01/11] hso: remove useless header file timer.h
From: Olivier Sobrie @ 2015-01-30 12:21 UTC (permalink / raw)
  To: Jan Dumon, Dan Williams; +Cc: linux-kernel, linux-usb, netdev, Olivier Sobrie
In-Reply-To: <1422620523-15021-1-git-send-email-olivier@sobrie.be>

No timer related function is used in this driver.

Signed-off-by: Olivier Sobrie <olivier@sobrie.be>
---
 drivers/net/usb/hso.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/drivers/net/usb/hso.c b/drivers/net/usb/hso.c
index babda7d..cb0bcc1 100644
--- a/drivers/net/usb/hso.c
+++ b/drivers/net/usb/hso.c
@@ -58,7 +58,6 @@
 #include <linux/module.h>
 #include <linux/ethtool.h>
 #include <linux/usb.h>
-#include <linux/timer.h>
 #include <linux/tty.h>
 #include <linux/tty_driver.h>
 #include <linux/tty_flip.h>
-- 
2.2.0

^ permalink raw reply related

* [PATCH v2 00/11] hso: fix some problems in the disconnect path
From: Olivier Sobrie @ 2015-01-30 12:21 UTC (permalink / raw)
  To: Jan Dumon, Dan Williams; +Cc: linux-kernel, linux-usb, netdev, Olivier Sobrie
In-Reply-To: <1421756978-4093-1-git-send-email-olivier@sobrie.be>

These patches attempt to fix some problems I observed when the hso
device is disconnected.
Several patches of this serie are fixing crashes or memleaks when a
hso device is disconnected.
This serie of patches is based on v3.18.

changes in v2:
 - Last patch of the serie dropped since another patch fix the issue.
   See http://marc.info/?l=linux-usb&m=142186699418489 for more info.

 - Added an extra patch avoiding name conflicts for the rfkill interface.

Olivier Sobrie (11):
  hso: remove useless header file timer.h
  hso: fix crash when device disappears while serial port is open
  hso: fix memory leak when device disconnects
  hso: fix memory leak in hso_create_rfkill()
  hso: fix small indentation error
  hso: rename hso_dev into serial in hso_free_interface()
  hso: replace reset_device work by usb_queue_reset_device()
  hso: move tty_unregister outside hso_serial_common_free()
  hso: update serial_table in usb disconnect method
  hso: add missing cancel_work_sync in disconnect()
  hso: fix rfkill name conflicts

 drivers/net/usb/hso.c | 91 ++++++++++++++++++++++-----------------------------
 1 file changed, 40 insertions(+), 51 deletions(-)

-- 
2.2.0

^ permalink raw reply

* [PATCH net] qlge: Fix qlge_update_hw_vlan_features to handle if interface is down
From: Marcelo Ricardo Leitner @ 2015-01-30 11:56 UTC (permalink / raw)
  To: netdev; +Cc: harish.patil, cdupuis

Currently qlge_update_hw_vlan_features() will always first put the
interface down, then update features and then bring it up again. But it
is possible to hit this code while the adapter is down and this causes a
non-paired call to napi_disable(), which will get stuck.

This patch fixes it by skipping these down/up actions if the interface
is already down.

Fixes: a45adbe8d352 ("qlge: Enhance nested VLAN (Q-in-Q) handling.")
Cc: Harish Patil <harish.patil@qlogic.com>
Signed-off-by: Marcelo Ricardo Leitner <mleitner@redhat.com>
---
 drivers/net/ethernet/qlogic/qlge/qlge_main.c | 26 ++++++++++++++++----------
 1 file changed, 16 insertions(+), 10 deletions(-)

diff --git a/drivers/net/ethernet/qlogic/qlge/qlge_main.c b/drivers/net/ethernet/qlogic/qlge/qlge_main.c
index dc0058f9037001b56eb816f0b3d3765089c919cd..8011ef3e7707f783f4caf9f1c16f3d7be3410c4f 100644
--- a/drivers/net/ethernet/qlogic/qlge/qlge_main.c
+++ b/drivers/net/ethernet/qlogic/qlge/qlge_main.c
@@ -2351,23 +2351,29 @@ static int qlge_update_hw_vlan_features(struct net_device *ndev,
 {
 	struct ql_adapter *qdev = netdev_priv(ndev);
 	int status = 0;
+	bool need_restart = netif_running(ndev);
 
-	status = ql_adapter_down(qdev);
-	if (status) {
-		netif_err(qdev, link, qdev->ndev,
-			  "Failed to bring down the adapter\n");
-		return status;
+	if (need_restart) {
+		status = ql_adapter_down(qdev);
+		if (status) {
+			netif_err(qdev, link, qdev->ndev,
+				  "Failed to bring down the adapter\n");
+			return status;
+		}
 	}
 
 	/* update the features with resent change */
 	ndev->features = features;
 
-	status = ql_adapter_up(qdev);
-	if (status) {
-		netif_err(qdev, link, qdev->ndev,
-			  "Failed to bring up the adapter\n");
-		return status;
+	if (need_restart) {
+		status = ql_adapter_up(qdev);
+		if (status) {
+			netif_err(qdev, link, qdev->ndev,
+				  "Failed to bring up the adapter\n");
+			return status;
+		}
 	}
+
 	return status;
 }
 
-- 
1.9.3

^ permalink raw reply related

* Re: [PATCH v2 2/7] phy: miphy365x: Pass sysconfig register offsets via syscfg dt property.
From: Maxime Coquelin @ 2015-01-30 11:48 UTC (permalink / raw)
  To: Kishon Vijay Abraham I, Peter Griffin,
	linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA,
	srinivas.kandagatla-Re5JQEeQqe8AvxtiuMwx3w,
	patrice.chotard-qxv4g6HH51o, peppe.cavallaro-qxv4g6HH51o,
	arnd-r2nGTMty4D4
  Cc: lee.jones-QSEj5FYQhm4dnm+yROfE0A,
	devicetree-u79uwXL29TY76Z2rM5mHXA, netdev-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <54CB661A.7050706-l0cyMroinI0@public.gmane.org>


On 01/30/2015 12:08 PM, Kishon Vijay Abraham I wrote:
> Hi,
>
> On Friday 30 January 2015 04:18 PM, Maxime Coquelin wrote:
>> Hi Kishon,
>>
>> On 01/30/2015 11:35 AM, Kishon Vijay Abraham I wrote:
>>> Hi,
>>>
>>> On Wednesday 07 January 2015 08:34 PM, Peter Griffin wrote:
>>>> Based on Arnds review comments here https://lkml.org/lkml/2014/11/13/161,
>>>> update the miphy365 phy driver to access sysconfig register offsets via
>>>> syscfg dt property.
>>>>
>>>> This is because the reg property should not be mixing address spaces
>>>> like it does currently for miphy365. This change then also aligns us
>>>> to how other platforms such as keystone and bcm7445 pass there syscon
>>>> offsets via DT.
>>>>
>>>> This patch breaks DT compatibility, but this platform is considered WIP,
>>>> and is only used by a few developers who are upstreaming support for it.
>>>> This change has been done as a single atomic commit to ensure it is
>>>> bisectable.
>>> I'm dropping this from my tree since I didn't get Ack from
>>> "arch/arm/boot/dts/stih416.dtsi" Maintainer.
>> Sorry, on cover letter, I replied the series looked good to me.
>> So you can add:
>>
>> Acked-by: Maxime Coquelin <maxime.coquelin-qxv4g6HH51o@public.gmane.org>
>>
>> And even:
>>
>> Tested-by: Maxime Coquelin <maxime.coquelin-qxv4g6HH51o@public.gmane.org>
> Thanks. I'll merge them now.
> I'd assume there won't be any conflicts when it gets merged to linus tree.
Thanks Kishon,

I have merged this patch with the last DT pull-request I sent for v3.20, 
and didn't had any conflicts.
So I'm confident there won't be any issue.

Br,
Maxime
>
> Cheers
> Kishon
>
>> Kind regards,
>> Maxime
>>
>>> Thanks
>>> Kishon
>>>> Signed-off-by: Peter Griffin <peter.griffin-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
>>>> Reviewed-by: Arnd Bergmann <arnd-r2nGTMty4D4@public.gmane.org>
>>>> ---
>>>>    .../devicetree/bindings/phy/phy-miphy365x.txt      | 15 +++++------
>>>>    arch/arm/boot/dts/stih416.dtsi                     | 10 ++++----
>>>>    drivers/phy/phy-miphy365x.c                        | 29
>>>> ++++++++--------------
>>>>    3 files changed, 23 insertions(+), 31 deletions(-)
>>>>
>>>> diff --git a/Documentation/devicetree/bindings/phy/phy-miphy365x.txt
>>>> b/Documentation/devicetree/bindings/phy/phy-miphy365x.txt
>>>> index 42c8808..9802d5d 100644
>>>> --- a/Documentation/devicetree/bindings/phy/phy-miphy365x.txt
>>>> +++ b/Documentation/devicetree/bindings/phy/phy-miphy365x.txt
>>>> @@ -6,8 +6,10 @@ for SATA and PCIe.
>>>>      Required properties (controller (parent) node):
>>>>    - compatible    : Should be "st,miphy365x-phy"
>>>> -- st,syscfg     : Should be a phandle of the system configuration register
>>>> group
>>>> -          which contain the SATA, PCIe mode setting bits
>>>> +- st,syscfg     : Phandle / integer array property. Phandle of sysconfig group
>>>> +          containing the miphy registers and integer array should contain
>>>> +          an entry for each port sub-node, specifying the control
>>>> +          register offset inside the sysconfig group.
>>>>      Required nodes    :  A sub-node is required for each channel the controller
>>>>               provides. Address range information including the usual
>>>> @@ -26,7 +28,6 @@ Required properties (port (child) node):
>>>>              registers filled in "reg":
>>>>                - sata:   For SATA devices
>>>>                - pcie:   For PCIe devices
>>>> -            - syscfg: To specify the syscfg based config register
>>>>      Optional properties (port (child) node):
>>>>    - st,sata-gen         :    Generation of locally attached SATA IP.
>>>> Expected values
>>>> @@ -39,20 +40,20 @@ Example:
>>>>          miphy365x_phy: miphy365x@fe382000 {
>>>>            compatible      = "st,miphy365x-phy";
>>>> -        st,syscfg      = <&syscfg_rear>;
>>>> +        st,syscfg      = <&syscfg_rear 0x824 0x828>;
>>>>            #address-cells    = <1>;
>>>>            #size-cells    = <1>;
>>>>            ranges;
>>>>              phy_port0: port@fe382000 {
>>>> -            reg = <0xfe382000 0x100>, <0xfe394000 0x100>, <0x824 0x4>;
>>>> -            reg-names = "sata", "pcie", "syscfg";
>>>> +            reg = <0xfe382000 0x100>, <0xfe394000 0x100>;
>>>> +            reg-names = "sata", "pcie";
>>>>                #phy-cells = <1>;
>>>>                st,sata-gen = <3>;
>>>>            };
>>>>              phy_port1: port@fe38a000 {
>>>> -            reg = <0xfe38a000 0x100>, <0xfe804000 0x100>, <0x828 0x4>;;
>>>> +            reg = <0xfe38a000 0x100>, <0xfe804000 0x100>;;
>>>>                reg-names = "sata", "pcie", "syscfg";
>>>>                #phy-cells = <1>;
>>>>                st,pcie-tx-pol-inv;
>>>> diff --git a/arch/arm/boot/dts/stih416.dtsi b/arch/arm/boot/dts/stih416.dtsi
>>>> index fad9073..85afe01 100644
>>>> --- a/arch/arm/boot/dts/stih416.dtsi
>>>> +++ b/arch/arm/boot/dts/stih416.dtsi
>>>> @@ -283,21 +283,21 @@
>>>>              miphy365x_phy: phy@fe382000 {
>>>>                compatible      = "st,miphy365x-phy";
>>>> -            st,syscfg      = <&syscfg_rear>;
>>>> +            st,syscfg    = <&syscfg_rear 0x824 0x828>;
>>>>                #address-cells    = <1>;
>>>>                #size-cells    = <1>;
>>>>                ranges;
>>>>                  phy_port0: port@fe382000 {
>>>>                    #phy-cells = <1>;
>>>> -                reg = <0xfe382000 0x100>, <0xfe394000 0x100>, <0x824 0x4>;
>>>> -                reg-names = "sata", "pcie", "syscfg";
>>>> +                reg = <0xfe382000 0x100>, <0xfe394000 0x100>;
>>>> +                reg-names = "sata", "pcie";
>>>>                };
>>>>                  phy_port1: port@fe38a000 {
>>>>                    #phy-cells = <1>;
>>>> -                reg = <0xfe38a000 0x100>, <0xfe804000 0x100>, <0x828 0x4>;
>>>> -                reg-names = "sata", "pcie", "syscfg";
>>>> +                reg = <0xfe38a000 0x100>, <0xfe804000 0x100>;
>>>> +                reg-names = "sata", "pcie";
>>>>                };
>>>>            };
>>>>    diff --git a/drivers/phy/phy-miphy365x.c b/drivers/phy/phy-miphy365x.c
>>>> index 6ab43a8..6c80154 100644
>>>> --- a/drivers/phy/phy-miphy365x.c
>>>> +++ b/drivers/phy/phy-miphy365x.c
>>>> @@ -141,7 +141,7 @@ struct miphy365x_phy {
>>>>        bool pcie_tx_pol_inv;
>>>>        bool sata_tx_pol_inv;
>>>>        u32 sata_gen;
>>>> -    u64 ctrlreg;
>>>> +    u32 ctrlreg;
>>>>        u8 type;
>>>>    };
>>>>    @@ -179,7 +179,7 @@ static int miphy365x_set_path(struct miphy365x_phy
>>>> *miphy_phy,
>>>>        bool sata = (miphy_phy->type == MIPHY_TYPE_SATA);
>>>>          return regmap_update_bits(miphy_dev->regmap,
>>>> -                  (unsigned int)miphy_phy->ctrlreg,
>>>> +                  miphy_phy->ctrlreg,
>>>>                      SYSCFG_SELECT_SATA_MASK,
>>>>                      sata << SYSCFG_SELECT_SATA_POS);
>>>>    }
>>>> @@ -445,7 +445,6 @@ int miphy365x_get_addr(struct device *dev, struct
>>>> miphy365x_phy *miphy_phy,
>>>>    {
>>>>        struct device_node *phynode = miphy_phy->phy->dev.of_node;
>>>>        const char *name;
>>>> -    const __be32 *taddr;
>>>>        int type = miphy_phy->type;
>>>>        int ret;
>>>>    @@ -455,22 +454,6 @@ int miphy365x_get_addr(struct device *dev, struct
>>>> miphy365x_phy *miphy_phy,
>>>>            return ret;
>>>>        }
>>>>    -    if (!strncmp(name, "syscfg", 6)) {
>>>> -        taddr = of_get_address(phynode, index, NULL, NULL);
>>>> -        if (!taddr) {
>>>> -            dev_err(dev, "failed to fetch syscfg address\n");
>>>> -            return -EINVAL;
>>>> -        }
>>>> -
>>>> -        miphy_phy->ctrlreg = of_translate_address(phynode, taddr);
>>>> -        if (miphy_phy->ctrlreg == OF_BAD_ADDR) {
>>>> -            dev_err(dev, "failed to translate syscfg address\n");
>>>> -            return -EINVAL;
>>>> -        }
>>>> -
>>>> -        return 0;
>>>> -    }
>>>> -
>>>>        if (!((!strncmp(name, "sata", 4) && type == MIPHY_TYPE_SATA) ||
>>>>              (!strncmp(name, "pcie", 4) && type == MIPHY_TYPE_PCIE)))
>>>>            return 0;
>>>> @@ -606,7 +589,15 @@ static int miphy365x_probe(struct platform_device *pdev)
>>>>                return ret;
>>>>              phy_set_drvdata(phy, miphy_dev->phys[port]);
>>>> +
>>>>            port++;
>>>> +        /* sysconfig offsets are indexed from 1 */
>>>> +        ret = of_property_read_u32_index(np, "st,syscfg", port,
>>>> +                    &miphy_phy->ctrlreg);
>>>> +        if (ret) {
>>>> +            dev_err(&pdev->dev, "No sysconfig offset found\n");
>>>> +            return ret;
>>>> +        }
>>>>        }
>>>>          provider = devm_of_phy_provider_register(&pdev->dev, miphy365x_xlate);
>>>>

--
To unsubscribe from this list: send the line "unsubscribe devicetree" 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 3/3] ixgbe: Add new ndo to allow VF multicast promiscuous mode
From: Hiroshi Shimamoto @ 2015-01-30 11:38 UTC (permalink / raw)
  To: Alexander Duyck, Skidmore, Donald C, Bjørn Mork
  Cc: e1000-devel@lists.sourceforge.net, netdev@vger.kernel.org,
	Choi, Sy Jong, linux-kernel@vger.kernel.org, David Laight,
	Hayato Momma

From: Hiroshi Shimamoto <h-shimamoto@ct.jp.nec.com>

Implements the new netdev op to allow VF multicast promiscuous mode.

The administrator can allow to VF multicast promiscuous mode for only
trusted VM. After allowing multicast promiscuous mode from the host,
we can use over 30 IPv6 addresses on VM.
 # ./ip link set dev eth0 vf 1 mc_promisc on

When disallowing multicast promiscuous mode, we can only use 30 IPv6 addresses.
 # ./ip link set dev eth0 vf 1 mc_promisc off

Signed-off-by: Hiroshi Shimamoto <h-shimamoto@ct.jp.nec.com>
Reviewed-by: Hayato Momma <h-momma@ce.jp.nec.com>
CC: Choi, Sy Jong <sy.jong.choi@intel.com>
---
 drivers/net/ethernet/intel/ixgbe/ixgbe.h       |  1 +
 drivers/net/ethernet/intel/ixgbe/ixgbe_main.c  |  7 ++++++
 drivers/net/ethernet/intel/ixgbe/ixgbe_sriov.c | 35 ++++++++++++++++++++++++--
 drivers/net/ethernet/intel/ixgbe/ixgbe_sriov.h |  1 +
 4 files changed, 42 insertions(+), 2 deletions(-)

diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe.h b/drivers/net/ethernet/intel/ixgbe/ixgbe.h
index bf3333e..33fde2e 100644
--- a/drivers/net/ethernet/intel/ixgbe/ixgbe.h
+++ b/drivers/net/ethernet/intel/ixgbe/ixgbe.h
@@ -150,6 +150,7 @@ struct vf_data_storage {
 	u16 tx_rate;
 	u16 vlan_count;
 	u8 spoofchk_enabled;
+	u8 mc_promisc_allowed;
 	unsigned int vf_api;
 };
 
diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
index 2ed2c7d..34924f7 100644
--- a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
+++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
@@ -3569,6 +3569,12 @@ static void ixgbe_configure_virtualization(struct ixgbe_adapter *adapter)
 		if (!adapter->vfinfo[i].spoofchk_enabled)
 			ixgbe_ndo_set_vf_spoofchk(adapter->netdev, i, false);
 	}
+
+	/* Reconfigure multicast promiscuous mode */
+	for (i = 0; i < adapter->num_vfs; i++) {
+		ixgbe_ndo_set_vf_mc_promisc(adapter->netdev, i,
+					adapter->vfinfo[i].mc_promisc_allowed);
+	}
 }
 
 static void ixgbe_set_rx_buffer_len(struct ixgbe_adapter *adapter)
@@ -7955,6 +7961,7 @@ static const struct net_device_ops ixgbe_netdev_ops = {
 	.ndo_set_vf_vlan	= ixgbe_ndo_set_vf_vlan,
 	.ndo_set_vf_rate	= ixgbe_ndo_set_vf_bw,
 	.ndo_set_vf_spoofchk	= ixgbe_ndo_set_vf_spoofchk,
+	.ndo_set_vf_mc_promisc	= ixgbe_ndo_set_vf_mc_promisc,
 	.ndo_get_vf_config	= ixgbe_ndo_get_vf_config,
 	.ndo_get_stats64	= ixgbe_get_stats64,
 #ifdef CONFIG_IXGBE_DCB
diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_sriov.c b/drivers/net/ethernet/intel/ixgbe/ixgbe_sriov.c
index c19b7b8..9f39a26 100644
--- a/drivers/net/ethernet/intel/ixgbe/ixgbe_sriov.c
+++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_sriov.c
@@ -111,8 +111,11 @@ static int __ixgbe_enable_sriov(struct ixgbe_adapter *adapter)
 		for (i = 0; i < adapter->num_vfs; i++) {
 			/* Enable spoof checking for all VFs */
 			adapter->vfinfo[i].spoofchk_enabled = true;
-			/* Turn multicast promiscuous mode off for all VFs */
+			/*
+			 * Disallow VF multicast promiscuous capability
+			 * and turn it off for all VFs */
 			adapter->vfinfo[i].mc_promisc = false;
+			adapter->vfinfo[i].mc_promisc_allowed = false;
 		}
 		return 0;
 	}
@@ -1019,7 +1022,7 @@ static int ixgbe_set_vf_mc_promisc(struct ixgbe_adapter *adapter,
 
 	adapter->vfinfo[vf].mc_promisc = enable;
 
-	if (enable)
+	if (enable && adapter->vfinfo[vf].mc_promisc_allowed)
 		return ixgbe_enable_vf_mc_promisc(adapter, vf);
 	else
 		return ixgbe_disable_vf_mc_promisc(adapter, vf);
@@ -1415,6 +1418,32 @@ int ixgbe_ndo_set_vf_spoofchk(struct net_device *netdev, int vf, bool setting)
 	return 0;
 }
 
+int ixgbe_ndo_set_vf_mc_promisc(struct net_device *netdev, int vf, bool setting)
+{
+	struct ixgbe_adapter *adapter = netdev_priv(netdev);
+	struct ixgbe_hw *hw = &adapter->hw;
+	u32 vmolr;
+
+	if (vf >= adapter->num_vfs)
+		return -EINVAL;
+
+	/* nothing to do */
+	if (adapter->vfinfo[vf].mc_promisc_allowed == setting)
+		return 0;
+
+	adapter->vfinfo[vf].mc_promisc_allowed = setting;
+
+	/* if VF requests multicast promiscuous */
+	if (adapter->vfinfo[vf].mc_promisc) {
+		if (setting)
+			ixgbe_enable_vf_mc_promisc(adapter, vf);
+		else
+			ixgbe_disable_vf_mc_promisc(adapter, vf);
+	}
+
+	return 0;
+}
+
 int ixgbe_ndo_get_vf_config(struct net_device *netdev,
 			    int vf, struct ifla_vf_info *ivi)
 {
@@ -1428,5 +1457,7 @@ int ixgbe_ndo_get_vf_config(struct net_device *netdev,
 	ivi->vlan = adapter->vfinfo[vf].pf_vlan;
 	ivi->qos = adapter->vfinfo[vf].pf_qos;
 	ivi->spoofchk = adapter->vfinfo[vf].spoofchk_enabled;
+	ivi->mc_promisc = adapter->vfinfo[vf].mc_promisc_allowed;
+
 	return 0;
 }
diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_sriov.h b/drivers/net/ethernet/intel/ixgbe/ixgbe_sriov.h
index 32c26d5..0ab98e6 100644
--- a/drivers/net/ethernet/intel/ixgbe/ixgbe_sriov.h
+++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_sriov.h
@@ -47,6 +47,7 @@ int ixgbe_ndo_set_vf_vlan(struct net_device *netdev, int queue, u16 vlan,
 int ixgbe_ndo_set_vf_bw(struct net_device *netdev, int vf, int min_tx_rate,
 			int max_tx_rate);
 int ixgbe_ndo_set_vf_spoofchk(struct net_device *netdev, int vf, bool setting);
+int ixgbe_ndo_set_vf_mc_promisc(struct net_device *netdev, int vf, bool setting);
 int ixgbe_ndo_get_vf_config(struct net_device *netdev,
 			    int vf, struct ifla_vf_info *ivi);
 void ixgbe_check_vf_rate_limit(struct ixgbe_adapter *adapter);
-- 
1.9.0

------------------------------------------------------------------------------
Dive into the World of Parallel Programming. The Go Parallel Website,
sponsored by Intel and developed in partnership with Slashdot Media, is your
hub for all things parallel software development, from weekly thought
leadership blogs to news, videos, case studies, tutorials and more. Take a
look and join the conversation now. http://goparallel.sourceforge.net/
_______________________________________________
E1000-devel mailing list
E1000-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/e1000-devel
To learn more about Intel&#174; Ethernet, visit http://communities.intel.com/community/wired

^ permalink raw reply related

* [PATCH 2/3] if_link: Add VF multicast promiscuous control
From: Hiroshi Shimamoto @ 2015-01-30 11:37 UTC (permalink / raw)
  To: Alexander Duyck, Skidmore, Donald C, Bjørn Mork
  Cc: e1000-devel@lists.sourceforge.net, netdev@vger.kernel.org,
	Choi, Sy Jong, linux-kernel@vger.kernel.org, David Laight,
	Hayato Momma

From: Hiroshi Shimamoto <h-shimamoto@ct.jp.nec.com>

Add netlink directives and ndo entry to allow VF multicast promiscuous mode.

The administrator wants to allow dedicatedly multicast promiscuous per VF.

Signed-off-by: Hiroshi Shimamoto <h-shimamoto@ct.jp.nec.com>
Reviewed-by: Hayato Momma <h-momma@ce.jp.nec.com>
CC: Choi, Sy Jong <sy.jong.choi@intel.com>
---
 include/linux/if_link.h      |  1 +
 include/linux/netdevice.h    |  3 +++
 include/uapi/linux/if_link.h |  6 ++++++
 net/core/rtnetlink.c         | 18 ++++++++++++++++--
 4 files changed, 26 insertions(+), 2 deletions(-)

diff --git a/include/linux/if_link.h b/include/linux/if_link.h
index 119130e..bc29ddf 100644
--- a/include/linux/if_link.h
+++ b/include/linux/if_link.h
@@ -14,5 +14,6 @@ struct ifla_vf_info {
 	__u32 linkstate;
 	__u32 min_tx_rate;
 	__u32 max_tx_rate;
+	__u32 mc_promisc;
 };
 #endif /* _LINUX_IF_LINK_H */
diff --git a/include/linux/netdevice.h b/include/linux/netdevice.h
index 52fd8e8..12e88a7 100644
--- a/include/linux/netdevice.h
+++ b/include/linux/netdevice.h
@@ -868,6 +868,7 @@ typedef u16 (*select_queue_fallback_t)(struct net_device *dev,
  * int (*ndo_set_vf_rate)(struct net_device *dev, int vf, int min_tx_rate,
  *			  int max_tx_rate);
  * int (*ndo_set_vf_spoofchk)(struct net_device *dev, int vf, bool setting);
+ * int (*ndo_set_vf_mc_promisc)(struct net_device *dev, int vf, bool setting);
  * int (*ndo_get_vf_config)(struct net_device *dev,
  *			    int vf, struct ifla_vf_info *ivf);
  * int (*ndo_set_vf_link_state)(struct net_device *dev, int vf, int link_state);
@@ -1084,6 +1085,8 @@ struct net_device_ops {
 						   int max_tx_rate);
 	int			(*ndo_set_vf_spoofchk)(struct net_device *dev,
 						       int vf, bool setting);
+	int			(*ndo_set_vf_mc_promisc)(struct net_device *dev,
+							 int vf, bool setting);
 	int			(*ndo_get_vf_config)(struct net_device *dev,
 						     int vf,
 						     struct ifla_vf_info *ivf);
diff --git a/include/uapi/linux/if_link.h b/include/uapi/linux/if_link.h
index f7d0d2d..a476aea 100644
--- a/include/uapi/linux/if_link.h
+++ b/include/uapi/linux/if_link.h
@@ -454,6 +454,7 @@ enum {
 	IFLA_VF_SPOOFCHK,	/* Spoof Checking on/off switch */
 	IFLA_VF_LINK_STATE,	/* link state enable/disable/auto switch */
 	IFLA_VF_RATE,		/* Min and Max TX Bandwidth Allocation */
+	IFLA_VF_MC_PROMISC,	/* Multicast Promiscuous allow/disallow */
 	__IFLA_VF_MAX,
 };
 
@@ -498,6 +499,11 @@ struct ifla_vf_link_state {
 	__u32 link_state;
 };
 
+struct ifla_vf_mc_promisc {
+	__u32 vf;
+	__u32 setting;
+};
+
 /* VF ports management section
  *
  *	Nested layout of set/get msg is:
diff --git a/net/core/rtnetlink.c b/net/core/rtnetlink.c
index 9cf6fe9..5992245 100644
--- a/net/core/rtnetlink.c
+++ b/net/core/rtnetlink.c
@@ -807,7 +807,8 @@ static inline int rtnl_vfinfo_size(const struct net_device *dev,
 			 nla_total_size(sizeof(struct ifla_vf_vlan)) +
 			 nla_total_size(sizeof(struct ifla_vf_spoofchk)) +
 			 nla_total_size(sizeof(struct ifla_vf_rate)) +
-			 nla_total_size(sizeof(struct ifla_vf_link_state)));
+			 nla_total_size(sizeof(struct ifla_vf_link_state)) +
+			 nla_total_size(sizeof(struct ifla_vf_mc_promisc)));
 		return size;
 	} else
 		return 0;
@@ -1099,6 +1100,7 @@ static int rtnl_fill_ifinfo(struct sk_buff *skb, struct net_device *dev,
 			struct ifla_vf_tx_rate vf_tx_rate;
 			struct ifla_vf_spoofchk vf_spoofchk;
 			struct ifla_vf_link_state vf_linkstate;
+			struct ifla_vf_mc_promisc vf_mc_promisc;
 
 			/*
 			 * Not all SR-IOV capable drivers support the
@@ -1107,6 +1109,7 @@ static int rtnl_fill_ifinfo(struct sk_buff *skb, struct net_device *dev,
 			 * report anything.
 			 */
 			ivi.spoofchk = -1;
+			ivi.mc_promisc = -1;
 			memset(ivi.mac, 0, sizeof(ivi.mac));
 			/* The default value for VF link state is "auto"
 			 * IFLA_VF_LINK_STATE_AUTO which equals zero
@@ -1119,7 +1122,8 @@ static int rtnl_fill_ifinfo(struct sk_buff *skb, struct net_device *dev,
 				vf_rate.vf =
 				vf_tx_rate.vf =
 				vf_spoofchk.vf =
-				vf_linkstate.vf = ivi.vf;
+				vf_linkstate.vf =
+				vf_mc_promisc.vf = ivi.vf;
 
 			memcpy(vf_mac.mac, ivi.mac, sizeof(ivi.mac));
 			vf_vlan.vlan = ivi.vlan;
@@ -1128,6 +1132,7 @@ static int rtnl_fill_ifinfo(struct sk_buff *skb, struct net_device *dev,
 			vf_rate.min_tx_rate = ivi.min_tx_rate;
 			vf_rate.max_tx_rate = ivi.max_tx_rate;
 			vf_spoofchk.setting = ivi.spoofchk;
+			vf_mc_promisc.setting = ivi.mc_promisc;
 			vf_linkstate.link_state = ivi.linkstate;
 			vf = nla_nest_start(skb, IFLA_VF_INFO);
 			if (!vf) {
@@ -1461,6 +1466,15 @@ static int do_setvfinfo(struct net_device *dev, struct nlattr *attr)
 								 ivl->link_state);
 			break;
 		}
+		case IFLA_VF_MC_PROMISC: {
+			struct ifla_vf_mc_promisc *ivm;
+			ivm = nla_data(vf);
+			err = -EOPNOTSUPP;
+			if (ops->ndo_set_vf_mc_promisc)
+				err = ops->ndo_set_vf_mc_promisc(dev, ivm->vf,
+								 ivm->setting);
+			break;
+		}
 		default:
 			err = -EINVAL;
 			break;
-- 
1.9.0

------------------------------------------------------------------------------
Dive into the World of Parallel Programming. The Go Parallel Website,
sponsored by Intel and developed in partnership with Slashdot Media, is your
hub for all things parallel software development, from weekly thought
leadership blogs to news, videos, case studies, tutorials and more. Take a
look and join the conversation now. http://goparallel.sourceforge.net/
_______________________________________________
E1000-devel mailing list
E1000-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/e1000-devel
To learn more about Intel&#174; Ethernet, visit http://communities.intel.com/community/wired

^ permalink raw reply related

* [PATCH 1/3] ixgbe, ixgbevf: Add new mbox API to enable MC promiscuous mode
From: Hiroshi Shimamoto @ 2015-01-30 11:37 UTC (permalink / raw)
  To: Alexander Duyck, Skidmore, Donald C, Bjørn Mork
  Cc: e1000-devel@lists.sourceforge.net, netdev@vger.kernel.org,
	Choi, Sy Jong, linux-kernel@vger.kernel.org, David Laight,
	Hayato Momma

From: Hiroshi Shimamoto <h-shimamoto@ct.jp.nec.com>

The limitation of the number of multicast address for VF is not enough
for the large scale server with SR-IOV feature.
IPv6 requires the multicast MAC address for each IP address to handle
the Neighbor Solicitation message.
We couldn't assign over 30 IPv6 addresses to a single VF interface.

The easy way to solve this is enabling multicast promiscuous mode.
It is good to have a functionality to enable multicast promiscuous mode
for each VF from VF driver.

This patch introduces the new mbox API, IXGBE_VF_SET_MC_PROMISC, to
enable/disable multicast promiscuous mode in VF. If multicast promiscuous
mode is enabled the VF can receive all multicast packets.

With this patch, the ixgbevf driver automatically enable multicast
promiscuous mode when the number of multicast addresses is over than 30
if possible.

This also bump the API version up to 1.2 to check whether the API,
IXGBE_VF_SET_MC_PROMISC is available.

Signed-off-by: Hiroshi Shimamoto <h-shimamoto@ct.jp.nec.com>
Reviewed-by: Hayato Momma <h-momma@ce.jp.nec.com>
CC: Choi, Sy Jong <sy.jong.choi@intel.com>
---
 drivers/net/ethernet/intel/ixgbe/ixgbe.h          |  1 +
 drivers/net/ethernet/intel/ixgbe/ixgbe_mbx.h      |  4 +
 drivers/net/ethernet/intel/ixgbe/ixgbe_sriov.c    | 89 ++++++++++++++++++++++-
 drivers/net/ethernet/intel/ixgbevf/ixgbevf_main.c | 13 +++-
 drivers/net/ethernet/intel/ixgbevf/mbx.h          |  4 +
 drivers/net/ethernet/intel/ixgbevf/vf.c           | 29 +++++++-
 drivers/net/ethernet/intel/ixgbevf/vf.h           |  1 +
 7 files changed, 137 insertions(+), 4 deletions(-)

diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe.h b/drivers/net/ethernet/intel/ixgbe/ixgbe.h
index b6137be..bf3333e 100644
--- a/drivers/net/ethernet/intel/ixgbe/ixgbe.h
+++ b/drivers/net/ethernet/intel/ixgbe/ixgbe.h
@@ -144,6 +144,7 @@ struct vf_data_storage {
 	u16 vlans_enabled;
 	bool clear_to_send;
 	bool pf_set_mac;
+	bool mc_promisc;
 	u16 pf_vlan; /* When set, guest VLAN config not allowed. */
 	u16 pf_qos;
 	u16 tx_rate;
diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_mbx.h b/drivers/net/ethernet/intel/ixgbe/ixgbe_mbx.h
index a5cb755..2963557 100644
--- a/drivers/net/ethernet/intel/ixgbe/ixgbe_mbx.h
+++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_mbx.h
@@ -73,6 +73,7 @@ enum ixgbe_pfvf_api_rev {
 	ixgbe_mbox_api_10,	/* API version 1.0, linux/freebsd VF driver */
 	ixgbe_mbox_api_20,	/* API version 2.0, solaris Phase1 VF driver */
 	ixgbe_mbox_api_11,	/* API version 1.1, linux/freebsd VF driver */
+	ixgbe_mbox_api_12,	/* API version 1.2, linux/freebsd VF driver */
 	/* This value should always be last */
 	ixgbe_mbox_api_unknown,	/* indicates that API version is not known */
 };
@@ -91,6 +92,9 @@ enum ixgbe_pfvf_api_rev {
 /* mailbox API, version 1.1 VF requests */
 #define IXGBE_VF_GET_QUEUES	0x09 /* get queue configuration */
 
+/* mailbox API, version 1.2 VF requests */
+#define IXGBE_VF_SET_MC_PROMISC	0x0a /* VF requests PF to set MC promiscuous */
+
 /* GET_QUEUES return data indices within the mailbox */
 #define IXGBE_VF_TX_QUEUES	1	/* number of Tx queues supported */
 #define IXGBE_VF_RX_QUEUES	2	/* number of Rx queues supported */
diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_sriov.c b/drivers/net/ethernet/intel/ixgbe/ixgbe_sriov.c
index c76ba90..c19b7b8 100644
--- a/drivers/net/ethernet/intel/ixgbe/ixgbe_sriov.c
+++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_sriov.c
@@ -108,9 +108,12 @@ static int __ixgbe_enable_sriov(struct ixgbe_adapter *adapter)
 		adapter->flags2 &= ~(IXGBE_FLAG2_RSC_CAPABLE |
 				     IXGBE_FLAG2_RSC_ENABLED);
 
-		/* enable spoof checking for all VFs */
-		for (i = 0; i < adapter->num_vfs; i++)
+		for (i = 0; i < adapter->num_vfs; i++) {
+			/* Enable spoof checking for all VFs */
 			adapter->vfinfo[i].spoofchk_enabled = true;
+			/* Turn multicast promiscuous mode off for all VFs */
+			adapter->vfinfo[i].mc_promisc = false;
+		}
 		return 0;
 	}
 
@@ -311,6 +314,40 @@ int ixgbe_pci_sriov_configure(struct pci_dev *dev, int num_vfs)
 		return ixgbe_pci_sriov_enable(dev, num_vfs);
 }
 
+static int ixgbe_enable_vf_mc_promisc(struct ixgbe_adapter *adapter, u32 vf)
+{
+	struct ixgbe_hw *hw;
+	u32 vmolr;
+
+	hw = &adapter->hw;
+	vmolr = IXGBE_READ_REG(hw, IXGBE_VMOLR(vf));
+
+	e_info(drv, "VF %u: enabling multicast promiscuous\n", vf);
+
+	vmolr |= IXGBE_VMOLR_MPE;
+
+	IXGBE_WRITE_REG(hw, IXGBE_VMOLR(vf), vmolr);
+
+	return 0;
+}
+
+static int ixgbe_disable_vf_mc_promisc(struct ixgbe_adapter *adapter, u32 vf)
+{
+	struct ixgbe_hw *hw;
+	u32 vmolr;
+
+	hw = &adapter->hw;
+	vmolr = IXGBE_READ_REG(hw, IXGBE_VMOLR(vf));
+
+	e_info(drv, "VF %u: disabling multicast promiscuous\n", vf);
+
+	vmolr &= ~IXGBE_VMOLR_MPE;
+
+	IXGBE_WRITE_REG(hw, IXGBE_VMOLR(vf), vmolr);
+
+	return 0;
+}
+
 static int ixgbe_set_vf_multicasts(struct ixgbe_adapter *adapter,
 				   u32 *msgbuf, u32 vf)
 {
@@ -325,6 +362,12 @@ static int ixgbe_set_vf_multicasts(struct ixgbe_adapter *adapter,
 	u32 mta_reg;
 	u32 vmolr = IXGBE_READ_REG(hw, IXGBE_VMOLR(vf));
 
+	/* Disable multicast promiscuous first */
+	if (adapter->vfinfo[vf].mc_promisc) {
+		ixgbe_disable_vf_mc_promisc(adapter, vf);
+		adapter->vfinfo[vf].mc_promisc = false;
+	}
+
 	/* only so many hash values supported */
 	entries = min(entries, IXGBE_MAX_VF_MC_ENTRIES);
 
@@ -427,6 +470,7 @@ static s32 ixgbe_set_vf_lpe(struct ixgbe_adapter *adapter, u32 *msgbuf, u32 vf)
 #endif /* CONFIG_FCOE */
 		switch (adapter->vfinfo[vf].vf_api) {
 		case ixgbe_mbox_api_11:
+		case ixgbe_mbox_api_12:
 			/*
 			 * Version 1.1 supports jumbo frames on VFs if PF has
 			 * jumbo frames enabled which means legacy VFs are
@@ -710,6 +754,12 @@ static int ixgbe_vf_reset_msg(struct ixgbe_adapter *adapter, u32 vf)
 		IXGBE_WRITE_REG(hw, IXGBE_PVFTDWBALn(q_per_pool, vf, i), 0);
 	}
 
+	/* Disable multicast promiscuous at reset */
+	if (adapter->vfinfo[vf].mc_promisc) {
+		ixgbe_disable_vf_mc_promisc(adapter, vf);
+		adapter->vfinfo[vf].mc_promisc = false;
+	}
+
 	/* reply to reset with ack and vf mac address */
 	msgbuf[0] = IXGBE_VF_RESET;
 	if (!is_zero_ether_addr(vf_mac)) {
@@ -894,6 +944,12 @@ static int ixgbe_negotiate_vf_api(struct ixgbe_adapter *adapter,
 	switch (api) {
 	case ixgbe_mbox_api_10:
 	case ixgbe_mbox_api_11:
+	case ixgbe_mbox_api_12:
+		e_info(drv, "VF %d requested api_version %s\n", vf,
+			(api == ixgbe_mbox_api_12) ? "ixgbe_mbox_api_12" :
+			(api == ixgbe_mbox_api_11) ? "ixgbe_mbox_api_11" :
+			(api == ixgbe_mbox_api_10) ? "ixgbe_mbox_api_10" :
+			"unknown");
 		adapter->vfinfo[vf].vf_api = api;
 		return 0;
 	default:
@@ -917,6 +973,7 @@ static int ixgbe_get_vf_queues(struct ixgbe_adapter *adapter,
 	switch (adapter->vfinfo[vf].vf_api) {
 	case ixgbe_mbox_api_20:
 	case ixgbe_mbox_api_11:
+	case ixgbe_mbox_api_12:
 		break;
 	default:
 		return -1;
@@ -944,6 +1001,31 @@ static int ixgbe_get_vf_queues(struct ixgbe_adapter *adapter,
 	return 0;
 }
 
+static int ixgbe_set_vf_mc_promisc(struct ixgbe_adapter *adapter,
+				   u32 *msgbuf, u32 vf)
+{
+	bool enable = !!msgbuf[1];	/* msgbuf contains the flag to enable */
+
+	switch (adapter->vfinfo[vf].vf_api) {
+	case ixgbe_mbox_api_12:
+		break;
+	default:
+		return -1;
+	}
+
+	/* nothing to do */
+	if (adapter->vfinfo[vf].mc_promisc == enable)
+		return 0;
+
+	adapter->vfinfo[vf].mc_promisc = enable;
+
+	if (enable)
+		return ixgbe_enable_vf_mc_promisc(adapter, vf);
+	else
+		return ixgbe_disable_vf_mc_promisc(adapter, vf);
+}
+
+
 static int ixgbe_rcv_msg_from_vf(struct ixgbe_adapter *adapter, u32 vf)
 {
 	u32 mbx_size = IXGBE_VFMAILBOX_SIZE;
@@ -1000,6 +1082,9 @@ static int ixgbe_rcv_msg_from_vf(struct ixgbe_adapter *adapter, u32 vf)
 	case IXGBE_VF_GET_QUEUES:
 		retval = ixgbe_get_vf_queues(adapter, msgbuf, vf);
 		break;
+	case IXGBE_VF_SET_MC_PROMISC:
+		retval = ixgbe_set_vf_mc_promisc(adapter, msgbuf, vf);
+		break;
 	default:
 		e_err(drv, "Unhandled Msg %8.8x\n", msgbuf[0]);
 		retval = IXGBE_ERR_MBX;
diff --git a/drivers/net/ethernet/intel/ixgbevf/ixgbevf_main.c b/drivers/net/ethernet/intel/ixgbevf/ixgbevf_main.c
index 62a0d8e..0403e1d 100644
--- a/drivers/net/ethernet/intel/ixgbevf/ixgbevf_main.c
+++ b/drivers/net/ethernet/intel/ixgbevf/ixgbevf_main.c
@@ -1880,7 +1880,8 @@ static void ixgbevf_init_last_counter_stats(struct ixgbevf_adapter *adapter)
 static void ixgbevf_negotiate_api(struct ixgbevf_adapter *adapter)
 {
 	struct ixgbe_hw *hw = &adapter->hw;
-	int api[] = { ixgbe_mbox_api_11,
+	int api[] = { ixgbe_mbox_api_12,
+		      ixgbe_mbox_api_11,
 		      ixgbe_mbox_api_10,
 		      ixgbe_mbox_api_unknown };
 	int err = 0, idx = 0;
@@ -1894,6 +1895,12 @@ static void ixgbevf_negotiate_api(struct ixgbevf_adapter *adapter)
 		idx++;
 	}
 
+	dev_info(&adapter->pdev->dev, "mbox api_version = %s\n",
+		(hw->api_version == ixgbe_mbox_api_12) ? "ixgbe_mbox_api_12" :
+		(hw->api_version == ixgbe_mbox_api_11) ? "ixgbe_mbox_api_11" :
+		(hw->api_version == ixgbe_mbox_api_10) ? "ixgbe_mbox_api_10" :
+		"unknown");
+
 	spin_unlock_bh(&adapter->mbx_lock);
 }
 
@@ -2072,6 +2079,9 @@ void ixgbevf_down(struct ixgbevf_adapter *adapter)
 
 	netif_carrier_off(netdev);
 
+	/* drop multicast promiscuous mode flag */
+	adapter->hw.mac.mc_promisc = false;
+
 	if (!pci_channel_offline(adapter->pdev))
 		ixgbevf_reset(adapter);
 
@@ -3525,6 +3535,7 @@ static int ixgbevf_change_mtu(struct net_device *netdev, int new_mtu)
 
 	switch (adapter->hw.api_version) {
 	case ixgbe_mbox_api_11:
+	case ixgbe_mbox_api_12:
 		max_possible_frame = IXGBE_MAX_JUMBO_FRAME_SIZE;
 		break;
 	default:
diff --git a/drivers/net/ethernet/intel/ixgbevf/mbx.h b/drivers/net/ethernet/intel/ixgbevf/mbx.h
index 0bc3005..62ef0d8 100644
--- a/drivers/net/ethernet/intel/ixgbevf/mbx.h
+++ b/drivers/net/ethernet/intel/ixgbevf/mbx.h
@@ -86,6 +86,7 @@ enum ixgbe_pfvf_api_rev {
 	ixgbe_mbox_api_10,	/* API version 1.0, linux/freebsd VF driver */
 	ixgbe_mbox_api_20,	/* API version 2.0, solaris Phase1 VF driver */
 	ixgbe_mbox_api_11,	/* API version 1.1, linux/freebsd VF driver */
+	ixgbe_mbox_api_12,	/* API version 1.2, linux/freebsd VF driver */
 	/* This value should always be last */
 	ixgbe_mbox_api_unknown,	/* indicates that API version is not known */
 };
@@ -104,6 +105,9 @@ enum ixgbe_pfvf_api_rev {
 /* mailbox API, version 1.1 VF requests */
 #define IXGBE_VF_GET_QUEUE	0x09 /* get queue configuration */
 
+/* mailbox API, version 1.2 VF requests */
+#define IXGBE_VF_SET_MC_PROMISC	0x0a /* VF requests PF to set MC promiscuous */
+
 /* GET_QUEUES return data indices within the mailbox */
 #define IXGBE_VF_TX_QUEUES	1	/* number of Tx queues supported */
 #define IXGBE_VF_RX_QUEUES	2	/* number of Rx queues supported */
diff --git a/drivers/net/ethernet/intel/ixgbevf/vf.c b/drivers/net/ethernet/intel/ixgbevf/vf.c
index cdb53be..dfc87b0 100644
--- a/drivers/net/ethernet/intel/ixgbevf/vf.c
+++ b/drivers/net/ethernet/intel/ixgbevf/vf.c
@@ -120,6 +120,9 @@ static s32 ixgbevf_reset_hw_vf(struct ixgbe_hw *hw)
 	memcpy(hw->mac.perm_addr, addr, ETH_ALEN);
 	hw->mac.mc_filter_type = msgbuf[IXGBE_VF_MC_TYPE_WORD];
 
+	/* after reset, MC promiscuous mode is disabled */
+	hw->mac.mc_promisc = false;
+
 	return 0;
 }
 
@@ -327,8 +330,29 @@ static s32 ixgbevf_update_mc_addr_list_vf(struct ixgbe_hw *hw,
 	 */
 
 	cnt = netdev_mc_count(netdev);
-	if (cnt > 30)
+	if (cnt > 30) {
+		/*
+		 * If the API has the capability to handle MC promiscuous
+		 * mode, turn it on.
+		 */
+		if (hw->api_version == ixgbe_mbox_api_12) {
+			if (!hw->mac.mc_promisc) {
+				struct ixgbevf_adapter *adapter = hw->back;
+
+				dev_info(&adapter->pdev->dev, "Request MC PROMISC\n");
+
+				/* enabling multicast promiscuous */
+				msgbuf[0] = IXGBE_VF_SET_MC_PROMISC;
+				msgbuf[1] = 1;
+				ixgbevf_write_msg_read_ack(hw, msgbuf, 2);
+
+				hw->mac.mc_promisc = true;
+			}
+
+			return 0;
+		}
 		cnt = 30;
+	}
 	msgbuf[0] = IXGBE_VF_SET_MULTICAST;
 	msgbuf[0] |= cnt << IXGBE_VT_MSGINFO_SHIFT;
 
@@ -344,6 +368,8 @@ static s32 ixgbevf_update_mc_addr_list_vf(struct ixgbe_hw *hw,
 
 	ixgbevf_write_msg_read_ack(hw, msgbuf, IXGBE_VFMAILBOX_SIZE);
 
+	hw->mac.mc_promisc = false;
+
 	return 0;
 }
 
@@ -545,6 +571,7 @@ int ixgbevf_get_queues(struct ixgbe_hw *hw, unsigned int *num_tcs,
 	/* do nothing if API doesn't support ixgbevf_get_queues */
 	switch (hw->api_version) {
 	case ixgbe_mbox_api_11:
+	case ixgbe_mbox_api_12:
 		break;
 	default:
 		return 0;
diff --git a/drivers/net/ethernet/intel/ixgbevf/vf.h b/drivers/net/ethernet/intel/ixgbevf/vf.h
index 5b17242..97790db 100644
--- a/drivers/net/ethernet/intel/ixgbevf/vf.h
+++ b/drivers/net/ethernet/intel/ixgbevf/vf.h
@@ -87,6 +87,7 @@ struct ixgbe_mac_info {
 	enum ixgbe_mac_type type;
 
 	s32  mc_filter_type;
+	bool mc_promisc;
 
 	bool get_link_status;
 	u32  max_tx_queues;
-- 
1.9.0

------------------------------------------------------------------------------
Dive into the World of Parallel Programming. The Go Parallel Website,
sponsored by Intel and developed in partnership with Slashdot Media, is your
hub for all things parallel software development, from weekly thought
leadership blogs to news, videos, case studies, tutorials and more. Take a
look and join the conversation now. http://goparallel.sourceforge.net/
_______________________________________________
E1000-devel mailing list
E1000-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/e1000-devel
To learn more about Intel&#174; Ethernet, visit http://communities.intel.com/community/wired

^ permalink raw reply related

* Re: [PATCH 9/9] netfilter: nf_tables: add support for dynamic set updates
From: Herbert Xu @ 2015-01-30 11:29 UTC (permalink / raw)
  To: Patrick McHardy
  Cc: tgraf, davem, David.Laight, ying.xue, paulmck, netdev,
	netfilter-devel
In-Reply-To: <20150130101802.GA19073@gondor.apana.org.au>

On Fri, Jan 30, 2015 at 09:18:02PM +1100, Herbert Xu wrote:
> 
> OK so it looks like you really do need a totally lockless walk.
> So I'll reshape my iterators to do that.

OK here is a completely untested patch that implements a totally
lockless walk with restart-on-resize support.

diff --git a/include/linux/rhashtable.h b/include/linux/rhashtable.h
index 6d7e840..a0e5b08 100644
--- a/include/linux/rhashtable.h
+++ b/include/linux/rhashtable.h
@@ -18,6 +18,7 @@
 #ifndef _LINUX_RHASHTABLE_H
 #define _LINUX_RHASHTABLE_H
 
+#include <linux/compiler.h>
 #include <linux/list_nulls.h>
 #include <linux/workqueue.h>
 #include <linux/mutex.h>
@@ -110,6 +111,7 @@ struct rhashtable_params {
  * @p: Configuration parameters
  * @run_work: Deferred worker to expand/shrink asynchronously
  * @mutex: Mutex to protect current/future table swapping
+ * @walkers: List of active walkers
  * @being_destroyed: True if table is set up for destruction
  */
 struct rhashtable {
@@ -120,9 +122,36 @@ struct rhashtable {
 	struct rhashtable_params	p;
 	struct work_struct		run_work;
 	struct mutex                    mutex;
+	struct list_head		walkers;
 	bool                            being_destroyed;
 };
 
+/**
+ * struct rhashtable_walker - Hash table walker
+ * @list: List entry on list of walkers
+ * @resize: Resize event occured
+ */
+struct rhashtable_walker {
+	struct list_head list;
+	bool resize;
+};
+
+/**
+ * struct rhashtable_iter - Hash table iterator, fits into netlink cb
+ * @ht: Table to iterate through
+ * @p: Current pointer
+ * @walker: Associated rhashtable walker
+ * @slot: Current slot
+ * @skip: Number of entries to skip in slot
+ */
+struct rhashtable_iter {
+	struct rhashtable *ht;
+	struct rhash_head *p;
+	struct rhashtable_walker *walker;
+	unsigned int slot;
+	unsigned int skip;
+};
+
 static inline unsigned long rht_marker(const struct rhashtable *ht, u32 hash)
 {
 	return NULLS_MARKER(ht->p.nulls_base + hash);
@@ -178,6 +207,12 @@ bool rhashtable_lookup_compare_insert(struct rhashtable *ht,
 				      bool (*compare)(void *, void *),
 				      void *arg);
 
+int rhashtable_walk_init(struct rhashtable *ht, struct rhashtable_iter *iter);
+void rhashtable_walk_exit(struct rhashtable_iter *iter);
+int rhashtable_walk_start(struct rhashtable_iter *iter) __acquires(RCU);
+void *rhashtable_walk_next(struct rhashtable_iter *iter);
+void rhashtable_walk_stop(struct rhashtable_iter *iter) __releases(RCU);
+
 void rhashtable_destroy(struct rhashtable *ht);
 
 #define rht_dereference(p, ht) \
diff --git a/lib/rhashtable.c b/lib/rhashtable.c
index 71c6aa1..a3e9e5c 100644
--- a/lib/rhashtable.c
+++ b/lib/rhashtable.c
@@ -485,11 +485,15 @@ static void rht_deferred_worker(struct work_struct *work)
 {
 	struct rhashtable *ht;
 	struct bucket_table *tbl;
+	struct rhashtable_walker *walker;
 
 	ht = container_of(work, struct rhashtable, run_work);
 	mutex_lock(&ht->mutex);
 	tbl = rht_dereference(ht->tbl, ht);
 
+	list_for_each_entry(walker, &ht->walkers, list)
+		walker->resize = true;
+
 	if (ht->p.grow_decision && ht->p.grow_decision(ht, tbl->size))
 		rhashtable_expand(ht);
 	else if (ht->p.shrink_decision && ht->p.shrink_decision(ht, tbl->size))
@@ -813,6 +817,165 @@ exit:
 }
 EXPORT_SYMBOL_GPL(rhashtable_lookup_compare_insert);
 
+/**
+ * rhashtable_walk_init - Initialise an iterator
+ * @ht:		Table to walk over
+ * @iter:	Hash table Iterator
+ *
+ * This function prepares a hash table walk.
+ *
+ * Note that if you restart a walk after rhashtable_walk_stop you
+ * may see the same object twice.  Also, you may miss objects if
+ * there are removals in between rhashtable_walk_stop and the next
+ * call to rhashtable_walk_start.
+ *
+ * For a completely stable walk you should construct your own data
+ * structure outside the hash table.
+ *
+ * This function may sleep so you must not call it from interrupt
+ * context or with spin locks held.
+ *
+ * You must call rhashtable_walk_exit if this function returns
+ * successfully.
+ */
+int rhashtable_walk_init(struct rhashtable *ht, struct rhashtable_iter *iter)
+{
+	int err;
+
+	iter->ht = ht;
+	iter->p = NULL;
+	iter->slot = 0;
+	iter->skip = 0;
+
+	iter->walk = kmalloc(sizeof(*iter->walk), GFP_KERNEL);
+	if (!iter->walk)
+		return -ENOMEM;
+
+	mutex_lock(&ht->mutex);
+	list_add(&iter->walker->list, &ht->walkers);
+	mutex_unlock(&ht->mutex);
+
+	return 0;
+}
+EXPORT_SYMBOL_GPL(rhashtable_walk_init);
+
+/**
+ * rhashtable_walk_exit - Free an iterator
+ * @iter:	Hash table Iterator
+ *
+ * This function frees resources allocated by rhashtable_walk_init.
+ */
+void rhashtable_walk_exit(struct rhashtable_iter *iter)
+{
+	mutex_lock(&ht->mutex);
+	list_del(&iter->walker->list);
+	mutex_unlock(&ht->mutex);
+	kfree(iter->walker);
+}
+EXPORT_SYMBOL_GPL(rhashtable_walk_exit);
+
+/**
+ * rhashtable_walk_start - Start a hash table walk
+ * @iter:	Hash table iterator
+ *
+ * Start a hash table walk.  Note that we take the RCU lock in all
+ * cases including when we return an error.  So you must always call
+ * rhashtable_walk_stop to clean up.
+ *
+ * Returns zero if successful.
+ *
+ * Returns -EAGAIN if resize event occured.  Note that the iterator
+ * will rewind back to the beginning and you may use it immediately
+ * by calling rhashtable_walk_next.
+ */
+int rhashtable_walk_start(struct rhashtable_iter *iter)
+{
+	rcu_read_lock();
+
+	if (iter->walker->resize) {
+		iter->slot = 0;
+		iter->skip = 0;
+		iter->walker->resize = false;
+		return -EAGAIN;
+	}
+
+	return 0;
+}
+
+/**
+ * rhashtable_walk_next - Return the next object and advance the iterator
+ * @iter:	Hash table iterator
+ *
+ * Note that you must call rhashtable_walk_stop when you are finished
+ * with the walk.
+ *
+ * Returns the next object or NULL when the end of the table is reached.
+ *
+ * Returns -EAGAIN if resize event occured.  Note that the iterator
+ * will rewind back to the beginning and you may continue to use it.
+ */
+void *rhashtable_walk_next(struct rhashtable_iter *iter)
+{
+	const struct bucket_table *tbl;
+	struct rhashtable *ht = iter->ht;
+	struct rhash_head *p = iter->p;
+	void *obj = NULL;
+
+	tbl = rht_dereference_rcu(ht->tbl, ht);
+
+	if (p) {
+		p = rht_dereference_bucket_rcu(p->next, tbl, iter->slot);
+		goto next;
+	}
+
+	for (; iter->slot < tbl->size; iter->slot++) {
+		int skip = iter->skip;
+
+		rht_for_each_rcu(p, tbl, iter->slot) {
+			if (!skip)
+				break;
+			skip--;
+		}
+
+next:
+		if (!rht_is_a_nulls(p)) {
+			iter->skip++;
+			iter->p = p;
+			obj = rht_obj(ht, p);
+			goto out;
+		}
+
+		iter->skip = 0;
+	}
+
+	iter->p = NULL;
+
+out:
+	if (iter->walker->resize) {
+		iter->p = NULL;
+		iter->slot = 0;
+		iter->skip = 0;
+		iter->walker->resize = false;
+		return ERR_PTR(-EAGAIN);
+	}
+
+	return obj;
+}
+EXPORT_SYMBOL_GPL(rhashtable_walk_next);
+
+/**
+ * rhashtable_walk_stop - Finish a hash table walk
+ * @iter:	Hash table iterator
+ *
+ * Finish a hash table walk.
+ */
+void rhashtable_walk_stop(struct rhashtable_iter *iter)
+{
+	rcu_read_unlock();
+	iter->p = NULL;
+}
+EXPORT_SYMBOL_GPL(rhashtable_walk_stop);
+
 static size_t rounded_hashtable_size(struct rhashtable_params *params)
 {
 	return max(roundup_pow_of_two(params->nelem_hint * 4 / 3),

Cheers,
-- 
Email: Herbert Xu <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

^ permalink raw reply related

* Re: [PATCH v2 2/7] phy: miphy365x: Pass sysconfig register offsets via syscfg dt property.
From: Kishon Vijay Abraham I @ 2015-01-30 11:08 UTC (permalink / raw)
  To: Maxime Coquelin, Peter Griffin, linux-arm-kernel, linux-kernel,
	srinivas.kandagatla, patrice.chotard, peppe.cavallaro, arnd
  Cc: lee.jones, devicetree, netdev
In-Reply-To: <54CB618E.60808@st.com>

Hi,

On Friday 30 January 2015 04:18 PM, Maxime Coquelin wrote:
> Hi Kishon,
> 
> On 01/30/2015 11:35 AM, Kishon Vijay Abraham I wrote:
>> Hi,
>>
>> On Wednesday 07 January 2015 08:34 PM, Peter Griffin wrote:
>>> Based on Arnds review comments here https://lkml.org/lkml/2014/11/13/161,
>>> update the miphy365 phy driver to access sysconfig register offsets via
>>> syscfg dt property.
>>>
>>> This is because the reg property should not be mixing address spaces
>>> like it does currently for miphy365. This change then also aligns us
>>> to how other platforms such as keystone and bcm7445 pass there syscon
>>> offsets via DT.
>>>
>>> This patch breaks DT compatibility, but this platform is considered WIP,
>>> and is only used by a few developers who are upstreaming support for it.
>>> This change has been done as a single atomic commit to ensure it is
>>> bisectable.
>> I'm dropping this from my tree since I didn't get Ack from
>> "arch/arm/boot/dts/stih416.dtsi" Maintainer.
> Sorry, on cover letter, I replied the series looked good to me.
> So you can add:
> 
> Acked-by: Maxime Coquelin <maxime.coquelin@st.com>
> 
> And even:
> 
> Tested-by: Maxime Coquelin <maxime.coquelin@st.com>

Thanks. I'll merge them now.
I'd assume there won't be any conflicts when it gets merged to linus tree.

Cheers
Kishon

> 
> Kind regards,
> Maxime
> 
>>
>> Thanks
>> Kishon
>>> Signed-off-by: Peter Griffin <peter.griffin@linaro.org>
>>> Reviewed-by: Arnd Bergmann <arnd@arndb.de>
>>> ---
>>>   .../devicetree/bindings/phy/phy-miphy365x.txt      | 15 +++++------
>>>   arch/arm/boot/dts/stih416.dtsi                     | 10 ++++----
>>>   drivers/phy/phy-miphy365x.c                        | 29
>>> ++++++++--------------
>>>   3 files changed, 23 insertions(+), 31 deletions(-)
>>>
>>> diff --git a/Documentation/devicetree/bindings/phy/phy-miphy365x.txt
>>> b/Documentation/devicetree/bindings/phy/phy-miphy365x.txt
>>> index 42c8808..9802d5d 100644
>>> --- a/Documentation/devicetree/bindings/phy/phy-miphy365x.txt
>>> +++ b/Documentation/devicetree/bindings/phy/phy-miphy365x.txt
>>> @@ -6,8 +6,10 @@ for SATA and PCIe.
>>>     Required properties (controller (parent) node):
>>>   - compatible    : Should be "st,miphy365x-phy"
>>> -- st,syscfg     : Should be a phandle of the system configuration register
>>> group
>>> -          which contain the SATA, PCIe mode setting bits
>>> +- st,syscfg     : Phandle / integer array property. Phandle of sysconfig group
>>> +          containing the miphy registers and integer array should contain
>>> +          an entry for each port sub-node, specifying the control
>>> +          register offset inside the sysconfig group.
>>>     Required nodes    :  A sub-node is required for each channel the controller
>>>              provides. Address range information including the usual
>>> @@ -26,7 +28,6 @@ Required properties (port (child) node):
>>>             registers filled in "reg":
>>>               - sata:   For SATA devices
>>>               - pcie:   For PCIe devices
>>> -            - syscfg: To specify the syscfg based config register
>>>     Optional properties (port (child) node):
>>>   - st,sata-gen         :    Generation of locally attached SATA IP.
>>> Expected values
>>> @@ -39,20 +40,20 @@ Example:
>>>         miphy365x_phy: miphy365x@fe382000 {
>>>           compatible      = "st,miphy365x-phy";
>>> -        st,syscfg      = <&syscfg_rear>;
>>> +        st,syscfg      = <&syscfg_rear 0x824 0x828>;
>>>           #address-cells    = <1>;
>>>           #size-cells    = <1>;
>>>           ranges;
>>>             phy_port0: port@fe382000 {
>>> -            reg = <0xfe382000 0x100>, <0xfe394000 0x100>, <0x824 0x4>;
>>> -            reg-names = "sata", "pcie", "syscfg";
>>> +            reg = <0xfe382000 0x100>, <0xfe394000 0x100>;
>>> +            reg-names = "sata", "pcie";
>>>               #phy-cells = <1>;
>>>               st,sata-gen = <3>;
>>>           };
>>>             phy_port1: port@fe38a000 {
>>> -            reg = <0xfe38a000 0x100>, <0xfe804000 0x100>, <0x828 0x4>;;
>>> +            reg = <0xfe38a000 0x100>, <0xfe804000 0x100>;;
>>>               reg-names = "sata", "pcie", "syscfg";
>>>               #phy-cells = <1>;
>>>               st,pcie-tx-pol-inv;
>>> diff --git a/arch/arm/boot/dts/stih416.dtsi b/arch/arm/boot/dts/stih416.dtsi
>>> index fad9073..85afe01 100644
>>> --- a/arch/arm/boot/dts/stih416.dtsi
>>> +++ b/arch/arm/boot/dts/stih416.dtsi
>>> @@ -283,21 +283,21 @@
>>>             miphy365x_phy: phy@fe382000 {
>>>               compatible      = "st,miphy365x-phy";
>>> -            st,syscfg      = <&syscfg_rear>;
>>> +            st,syscfg    = <&syscfg_rear 0x824 0x828>;
>>>               #address-cells    = <1>;
>>>               #size-cells    = <1>;
>>>               ranges;
>>>                 phy_port0: port@fe382000 {
>>>                   #phy-cells = <1>;
>>> -                reg = <0xfe382000 0x100>, <0xfe394000 0x100>, <0x824 0x4>;
>>> -                reg-names = "sata", "pcie", "syscfg";
>>> +                reg = <0xfe382000 0x100>, <0xfe394000 0x100>;
>>> +                reg-names = "sata", "pcie";
>>>               };
>>>                 phy_port1: port@fe38a000 {
>>>                   #phy-cells = <1>;
>>> -                reg = <0xfe38a000 0x100>, <0xfe804000 0x100>, <0x828 0x4>;
>>> -                reg-names = "sata", "pcie", "syscfg";
>>> +                reg = <0xfe38a000 0x100>, <0xfe804000 0x100>;
>>> +                reg-names = "sata", "pcie";
>>>               };
>>>           };
>>>   diff --git a/drivers/phy/phy-miphy365x.c b/drivers/phy/phy-miphy365x.c
>>> index 6ab43a8..6c80154 100644
>>> --- a/drivers/phy/phy-miphy365x.c
>>> +++ b/drivers/phy/phy-miphy365x.c
>>> @@ -141,7 +141,7 @@ struct miphy365x_phy {
>>>       bool pcie_tx_pol_inv;
>>>       bool sata_tx_pol_inv;
>>>       u32 sata_gen;
>>> -    u64 ctrlreg;
>>> +    u32 ctrlreg;
>>>       u8 type;
>>>   };
>>>   @@ -179,7 +179,7 @@ static int miphy365x_set_path(struct miphy365x_phy
>>> *miphy_phy,
>>>       bool sata = (miphy_phy->type == MIPHY_TYPE_SATA);
>>>         return regmap_update_bits(miphy_dev->regmap,
>>> -                  (unsigned int)miphy_phy->ctrlreg,
>>> +                  miphy_phy->ctrlreg,
>>>                     SYSCFG_SELECT_SATA_MASK,
>>>                     sata << SYSCFG_SELECT_SATA_POS);
>>>   }
>>> @@ -445,7 +445,6 @@ int miphy365x_get_addr(struct device *dev, struct
>>> miphy365x_phy *miphy_phy,
>>>   {
>>>       struct device_node *phynode = miphy_phy->phy->dev.of_node;
>>>       const char *name;
>>> -    const __be32 *taddr;
>>>       int type = miphy_phy->type;
>>>       int ret;
>>>   @@ -455,22 +454,6 @@ int miphy365x_get_addr(struct device *dev, struct
>>> miphy365x_phy *miphy_phy,
>>>           return ret;
>>>       }
>>>   -    if (!strncmp(name, "syscfg", 6)) {
>>> -        taddr = of_get_address(phynode, index, NULL, NULL);
>>> -        if (!taddr) {
>>> -            dev_err(dev, "failed to fetch syscfg address\n");
>>> -            return -EINVAL;
>>> -        }
>>> -
>>> -        miphy_phy->ctrlreg = of_translate_address(phynode, taddr);
>>> -        if (miphy_phy->ctrlreg == OF_BAD_ADDR) {
>>> -            dev_err(dev, "failed to translate syscfg address\n");
>>> -            return -EINVAL;
>>> -        }
>>> -
>>> -        return 0;
>>> -    }
>>> -
>>>       if (!((!strncmp(name, "sata", 4) && type == MIPHY_TYPE_SATA) ||
>>>             (!strncmp(name, "pcie", 4) && type == MIPHY_TYPE_PCIE)))
>>>           return 0;
>>> @@ -606,7 +589,15 @@ static int miphy365x_probe(struct platform_device *pdev)
>>>               return ret;
>>>             phy_set_drvdata(phy, miphy_dev->phys[port]);
>>> +
>>>           port++;
>>> +        /* sysconfig offsets are indexed from 1 */
>>> +        ret = of_property_read_u32_index(np, "st,syscfg", port,
>>> +                    &miphy_phy->ctrlreg);
>>> +        if (ret) {
>>> +            dev_err(&pdev->dev, "No sysconfig offset found\n");
>>> +            return ret;
>>> +        }
>>>       }
>>>         provider = devm_of_phy_provider_register(&pdev->dev, miphy365x_xlate);
>>>
> 

^ permalink raw reply

* Re: [PATCH] Repair soft lockup with monitor mode of ath9k_htc card
From: Oleksij Rempel @ 2015-01-30 10:55 UTC (permalink / raw)
  To: yuweizheng, linux-kernel, ath9k-devel, linux-wireless, kvalo,
	ath9k-devel
  Cc: netdev
In-Reply-To: <1422486872-16308-1-git-send-email-yuweizheng@139.com>


[-- Attachment #1.1: Type: text/plain, Size: 12525 bytes --]

Am 29.01.2015 um 00:14 schrieb yuweizheng@139.com:
> From: Yuwei Zheng <yuweizheng@139.com>
> 
> In the environment with heavy wifi traffic, set the ar9271 into monitor mode, will
> trigger a deadloop panic.
>  
> The ath9k_hif_usb_rx_cb function excute on  the interrupt context, and ath9k_rx_tasklet excute
> on the soft irq context. In other words, the ath9k_hif_usb_rx_cb have more chance to excute than
> ath9k_rx_tasklet.  So in the worst condition,  the rx.rxbuf receive list is always full,
> and the do {}while(true) loop will not be break. The kernel get a soft lockup panic. 

Please note, ath9k_htc is also used on HW where real hrtimer actually
exist. It should behave differently and produce different load on the
system. The overhead of setting up the hrtimers *may* not be worth it.
Depending on the load, it may provide lots of avoidable interrupts.
Did you tried to use simple tasklet_start?

> [59011.007210] BUG: soft lockup - CPU#0 stuck for 23s!
> [kworker/0:0:30609]
> [59011.030560] BUG: scheduling while atomic: kworker/0:0/30609/0x40010100
> [59013.804486] BUG: scheduling while atomic: kworker/0:0/30609/0x40010100
> [59013.858522] Kernel panic - not syncing: softlockup: hung tasks
>  
> [59014.038891] Exception stack(0xdf4bbc38 to 0xdf4bbc80)
> [59014.046834] bc20:                                                       de57b950 60000113
> [59014.059579] bc40: 00000000 bb32bb32 60000113 de57b948 de57b500 dc7bb440 df4bbcd0 00000000
> [59014.072337] bc60: de57b950 60000113 df4bbcd0 df4bbc80 c04c259d c04c25a0 60000133 ffffffff
> [59014.085233] [<c04c28db>] (__irq_svc+0x3b/0x5c) from [<c04c25a0>] (_raw_spin_unlock_irqrestore+0xc/0x10)
> [59014.100437] [<c04c25a0>] (_raw_spin_unlock_irqrestore+0xc/0x10) from [<bf9c2089>] (ath9k_rx_tasklet+0x290/0x490 [ath9k_htc])
> [59014.118267] [<bf9c2089>] (ath9k_rx_tasklet+0x290/0x490 [ath9k_htc]) from [<c0036d23>] (tasklet_action+0x3b/0x98)
> [59014.134132] [<c0036d23>] (tasklet_action+0x3b/0x98) from [<c0036709>] (__do_softirq+0x99/0x16c)
> [59014.147784] [<c0036709>] (__do_softirq+0x99/0x16c) from [<c00369f7>] (irq_exit+0x5b/0x5c)
> [59014.160653] [<c00369f7>] (irq_exit+0x5b/0x5c) from [<c000cfc3>] (handle_IRQ+0x37/0x78)
> [59014.173124] [<c000cfc3>] (handle_IRQ+0x37/0x78) from [<c00085df>] (omap3_intc_handle_irq+0x5f/0x68)
> [59014.187225] [<c00085df>] (omap3_intc_handle_irq+0x5f/0x68) from [<c04c28db>](__irq_svc+0x3b/0x5c)
>  
> This bug can be see with low performance board, such as uniprocessor beagle bone board.
>  
>  
> Signed-off-by: Yuwei Zheng <zhengyuwei@360.cn>
> Signed-off-by: Yuwei Zheng <yuweizheng@139.com>
> 
> 
> ---
>  drivers/net/wireless/ath/ath9k/hif_usb.c       | 58 ++++++++++++++++++++++----
>  drivers/net/wireless/ath/ath9k/hif_usb.h       |  5 +++
>  drivers/net/wireless/ath/ath9k/htc.h           | 13 ++++++
>  drivers/net/wireless/ath/ath9k/htc_drv_debug.c | 49 ++++++++++++++++++++++
>  drivers/net/wireless/ath/ath9k/htc_drv_txrx.c  | 26 ++++++++++++
>  5 files changed, 144 insertions(+), 7 deletions(-)
> 
> diff --git a/drivers/net/wireless/ath/ath9k/hif_usb.c b/drivers/net/wireless/ath/ath9k/hif_usb.c
> index 8e7153b..18c6f0e 100644
> --- a/drivers/net/wireless/ath/ath9k/hif_usb.c
> +++ b/drivers/net/wireless/ath/ath9k/hif_usb.c
> @@ -658,7 +658,6 @@ static void ath9k_hif_usb_rx_cb(struct urb *urb)
>  	default:
>  		goto resubmit;
>  	}
> -
>  	if (likely(urb->actual_length != 0)) {
>  		skb_put(skb, urb->actual_length);
>  		ath9k_hif_usb_rx_stream(hif_dev, skb);
> @@ -667,12 +666,18 @@ static void ath9k_hif_usb_rx_cb(struct urb *urb)
>  resubmit:
>  	skb_reset_tail_pointer(skb);
>  	skb_trim(skb, 0);
> -
> -	usb_anchor_urb(urb, &hif_dev->rx_submitted);
> -	ret = usb_submit_urb(urb, GFP_ATOMIC);
> -	if (ret) {
> -		usb_unanchor_urb(urb);
> -		goto free;
> +	if (atomic_read(&hif_dev->rx_urb_submit_delay) > 0) {
> +		usb_anchor_urb(urb, &hif_dev->rx_delayed_submitted);
> +		ret = tasklet_hrtimer_start(&hif_dev->rx_submit_timer,
> +						ktime_set(0, atomic_read(&hif_dev->rx_urb_submit_delay)*1000),
> +						HRTIMER_MODE_REL);
> +	} else {
> +		usb_anchor_urb(urb, &hif_dev->rx_submitted);
> +		ret = usb_submit_urb(urb, GFP_ATOMIC);
> +		if (ret) {
> +			usb_unanchor_urb(urb);
> +			goto free;
> +		}
>  	}
>  
>  	return;
> @@ -818,9 +823,39 @@ err:
>  	return -ENOMEM;
>  }
>  
> +static enum hrtimer_restart rx_urb_submit_timer_handler(struct hrtimer *me)
> +{
> +	struct tasklet_hrtimer *thr =
> +		container_of(me, struct tasklet_hrtimer, timer);
> +	struct  hif_device_usb *hif_dev =
> +		container_of(thr, struct hif_device_usb, rx_submit_timer);
> +	struct urb *urb = NULL;
> +	struct sk_buff *skb = NULL;
> +	int ret;
> +
> +	while (true) {
> +		urb = usb_get_from_anchor(&hif_dev->rx_delayed_submitted);
> +		if (urb != NULL) {
> +			skb = (struct sk_buff *)urb->context;
> +			ret = usb_submit_urb(urb, GFP_ATOMIC);
> +			if (ret != -EBUSY) {
> +				usb_unanchor_urb(urb);
> +				dev_kfree_skb_any(skb);
> +				urb->context = NULL;
> +			}
> +		} else {
> +			break;
> +		}
> +	}
> +
> +	return HRTIMER_NORESTART;
> +}
> +
>  static void ath9k_hif_usb_dealloc_rx_urbs(struct hif_device_usb *hif_dev)
>  {
>  	usb_kill_anchored_urbs(&hif_dev->rx_submitted);
> +	usb_kill_anchored_urbs(&hif_dev->rx_delayed_submitted);
> +	tasklet_hrtimer_cancel(&hif_dev->rx_submit_timer);
>  }
>  
>  static int ath9k_hif_usb_alloc_rx_urbs(struct hif_device_usb *hif_dev)
> @@ -830,6 +865,8 @@ static int ath9k_hif_usb_alloc_rx_urbs(struct hif_device_usb *hif_dev)
>  	int i, ret;
>  
>  	init_usb_anchor(&hif_dev->rx_submitted);
> +	init_usb_anchor(&hif_dev->rx_delayed_submitted);
> +
>  	spin_lock_init(&hif_dev->rx_lock);
>  
>  	for (i = 0; i < MAX_RX_URB_NUM; i++) {
> @@ -871,6 +908,13 @@ static int ath9k_hif_usb_alloc_rx_urbs(struct hif_device_usb *hif_dev)
>  		usb_free_urb(urb);
>  	}
>  
> +	/* add for flow control*/
> +	atomic_set(&hif_dev->rx_urb_submit_delay, 0);
> +	tasklet_hrtimer_init(&hif_dev->rx_submit_timer,
> +				rx_urb_submit_timer_handler,
> +				CLOCK_MONOTONIC,
> +				HRTIMER_MODE_REL);
> +
>  	return 0;
>  
>  err_submit:
> diff --git a/drivers/net/wireless/ath/ath9k/hif_usb.h b/drivers/net/wireless/ath/ath9k/hif_usb.h
> index 51496e7..56d6be8 100644
> --- a/drivers/net/wireless/ath/ath9k/hif_usb.h
> +++ b/drivers/net/wireless/ath/ath9k/hif_usb.h
> @@ -98,9 +98,14 @@ struct hif_device_usb {
>  	struct hif_usb_tx tx;
>  	struct usb_anchor regout_submitted;
>  	struct usb_anchor rx_submitted;
> +	struct usb_anchor rx_delayed_submitted; /* delayed submit anchor */
>  	struct usb_anchor reg_in_submitted;
>  	struct usb_anchor mgmt_submitted;
>  	struct sk_buff *remain_skb;
> +
> +	struct tasklet_hrtimer  rx_submit_timer;/* delayed submit hrtimer */
> +	atomic_t  rx_urb_submit_delay; /*us*/
> +
>  	const char *fw_name;
>  	int rx_remain_len;
>  	int rx_pkt_len;
> diff --git a/drivers/net/wireless/ath/ath9k/htc.h b/drivers/net/wireless/ath/ath9k/htc.h
> index 9dde265..453d0a8 100644
> --- a/drivers/net/wireless/ath/ath9k/htc.h
> +++ b/drivers/net/wireless/ath/ath9k/htc.h
> @@ -331,6 +331,10 @@ static inline struct ath9k_htc_tx_ctl *HTC_SKB_CB(struct sk_buff *skb)
>  
>  #define TX_QSTAT_INC(q) (priv->debug.tx_stats.queue_stats[q]++)
>  
> +#define TASKLETRX_STAT_INC(c) (hif_dev->htc_handle->drv_priv->debug.taskletrx_stats.c++)
> +#define TASKLETRX_STAT_ADD(c, a) (hif_dev->htc_handle->drv_priv->debug.taskletrx_stats.c += a)
> +#define TASKLETRX_STAT_SET(c, a) (hif_dev->htc_handle->drv_priv->debug.taskletrx_stats.c = a)
> +
>  void ath9k_htc_err_stat_rx(struct ath9k_htc_priv *priv,
>  			   struct ath_rx_status *rs);
>  
> @@ -352,11 +356,20 @@ struct ath_skbrx_stats {
>  	u32 skb_dropped;
>  };
>  
> +struct ath_taskletrx_stats {
> +	u32 taskletrx_looptimes;
> +	u32 taskletrx_highwater;
> +	u32 taskletrx_lowwater;
> +	u32 taskletrx_watermark_triggered;
> +	u32 taskletrx_urb_submit_delay;
> +};
> +
>  struct ath9k_debug {
>  	struct dentry *debugfs_phy;
>  	struct ath_tx_stats tx_stats;
>  	struct ath_rx_stats rx_stats;
>  	struct ath_skbrx_stats skbrx_stats;
> +	struct ath_taskletrx_stats taskletrx_stats;
>  };
>  
>  void ath9k_htc_get_et_strings(struct ieee80211_hw *hw,
> diff --git a/drivers/net/wireless/ath/ath9k/htc_drv_debug.c b/drivers/net/wireless/ath/ath9k/htc_drv_debug.c
> index 8cef1ed..7c8322e 100644
> --- a/drivers/net/wireless/ath/ath9k/htc_drv_debug.c
> +++ b/drivers/net/wireless/ath/ath9k/htc_drv_debug.c
> @@ -286,6 +286,51 @@ static const struct file_operations fops_skb_rx = {
>  	.llseek = default_llseek,
>  };
>  
> +static ssize_t read_file_tasklet_rx(struct file *file, char __user *user_buf,
> +				    size_t count, loff_t *ppos)
> +{
> +	struct ath9k_htc_priv *priv = file->private_data;
> +	char *buf;
> +	unsigned int len = 0, size = 1500;
> +	ssize_t retval = 0;
> +
> +	buf = kzalloc(size, GFP_KERNEL);
> +	if (buf == NULL)
> +		return -ENOMEM;
> +
> +	len += scnprintf(buf + len, size - len,
> +			"%20s : %10u\n", "Loop times",
> +			priv->debug.taskletrx_stats.taskletrx_looptimes);
> +	len += scnprintf(buf + len, size - len,
> +			"%20s : %10u\n", "High watermark",
> +			priv->debug.taskletrx_stats.taskletrx_highwater);
> +	len += scnprintf(buf + len, size - len,
> +			"%20s : %10u\n", "Low watermark",
> +			priv->debug.taskletrx_stats.taskletrx_lowwater);
> +
> +	len += scnprintf(buf + len, size - len,
> +			"%20s : %10u\n", "WM triggered",
> +			priv->debug.taskletrx_stats.taskletrx_watermark_triggered);
> +
> +	len += scnprintf(buf + len, size - len,
> +			"%20s : %10u\n", "URB delay",
> +			priv->debug.taskletrx_stats.taskletrx_urb_submit_delay);
> +	if (len > size)
> +		len = size;
> +
> +	retval = simple_read_from_buffer(user_buf, count, ppos, buf, len);
> +	kfree(buf);
> +
> +	return retval;
> +}
> +
> +static const struct file_operations fops_tasklet_rx = {
> +	.read = read_file_tasklet_rx,
> +	.open = simple_open,
> +	.owner = THIS_MODULE,
> +	.llseek = default_llseek,
> +};
> +
>  static ssize_t read_file_slot(struct file *file, char __user *user_buf,
>  			      size_t count, loff_t *ppos)
>  {
> @@ -518,7 +563,11 @@ int ath9k_htc_init_debug(struct ath_hw *ah)
>  	debugfs_create_file("skb_rx", S_IRUSR, priv->debug.debugfs_phy,
>  			    priv, &fops_skb_rx);
>  
> +	debugfs_create_file("tasklet_rx", S_IRUSR, priv->debug.debugfs_phy,
> +			    priv, &fops_tasklet_rx);
> +
>  	ath9k_cmn_debug_recv(priv->debug.debugfs_phy, &priv->debug.rx_stats);
> +
>  	ath9k_cmn_debug_phy_err(priv->debug.debugfs_phy, &priv->debug.rx_stats);
>  
>  	debugfs_create_file("slot", S_IRUSR, priv->debug.debugfs_phy,
> diff --git a/drivers/net/wireless/ath/ath9k/htc_drv_txrx.c b/drivers/net/wireless/ath/ath9k/htc_drv_txrx.c
> index a0f58e2..f5e6217 100644
> --- a/drivers/net/wireless/ath/ath9k/htc_drv_txrx.c
> +++ b/drivers/net/wireless/ath/ath9k/htc_drv_txrx.c
> @@ -1061,7 +1061,28 @@ void ath9k_rx_tasklet(unsigned long data)
>  	unsigned long flags;
>  	struct ieee80211_hdr *hdr;
>  
> +	/* add for adaptive flow control*/
> +	int looptimes = 0;
> +	int highwatermark = ATH9K_HTC_RXBUF*3/4;
> +	int lowwatermark = ATH9K_HTC_RXBUF/4;
> +	unsigned int delay = 0;
> +
> +	struct htc_target *htc = priv->htc;
> +	struct hif_device_usb *hif_dev = htc->hif_dev;
> +
> +	TASKLETRX_STAT_SET(taskletrx_highwater, highwatermark);
> +	TASKLETRX_STAT_SET(taskletrx_lowwater, lowwatermark);
> +
>  	do {
> +		looptimes++;
> +		TASKLETRX_STAT_SET(taskletrx_looptimes, looptimes);
> +		if (looptimes > highwatermark) {
> +			delay = looptimes*10;
> +			atomic_set(&hif_dev->rx_urb_submit_delay, delay);
> +			TASKLETRX_STAT_INC(taskletrx_watermark_triggered);
> +			TASKLETRX_STAT_SET(taskletrx_urb_submit_delay, delay);
> +		}
> +
>  		spin_lock_irqsave(&priv->rx.rxbuflock, flags);
>  		list_for_each_entry(tmp_buf, &priv->rx.rxbuf, list) {
>  			if (tmp_buf->in_process) {
> @@ -1072,6 +1093,11 @@ void ath9k_rx_tasklet(unsigned long data)
>  
>  		if (rxbuf == NULL) {
>  			spin_unlock_irqrestore(&priv->rx.rxbuflock, flags);
> +			if (looptimes < lowwatermark) {
> +				atomic_set(&hif_dev->rx_urb_submit_delay, 0);
> +				TASKLETRX_STAT_SET(taskletrx_urb_submit_delay, 0);
> +			}
> +
>  			break;
>  		}
>  
> 


-- 
Regards,
Oleksij


[-- Attachment #1.2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 213 bytes --]

[-- Attachment #2: Type: text/plain, Size: 154 bytes --]

_______________________________________________
ath9k-devel mailing list
ath9k-devel@lists.ath9k.org
https://lists.ath9k.org/mailman/listinfo/ath9k-devel

^ permalink raw reply

* Re: [patch -mainline] fm10k: drop upper bits of VLAN ID
From: Dan Carpenter @ 2015-01-30 10:50 UTC (permalink / raw)
  To: Jeff Kirsher, Alexander Duyck
  Cc: kernel-janitors, Linux NICS, e1000-devel, Bruce Allan,
	Jesse Brandeburg, John Ronciak, netdev
In-Reply-To: <20150130084127.GE21357@mwanda>

On Fri, Jan 30, 2015 at 11:41:27AM +0300, Dan Carpenter wrote:
> Static checkers complain that the shifts in "(vid << 4) >> 4" perfectly
> cancel each other out and the code is a no-op.  "vid" is a u16.  The
> comment says that the intention here is to drop the upper bits so I have
> added a cast to "u16" to do that.
> 
> Fixes: 401b5383c6c9 ('fm10k: Add support for configuring PF interface')
> Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>

Oh...  Rasmus Villemoes already fixed this.

regards,
dan carpenter


------------------------------------------------------------------------------
Dive into the World of Parallel Programming. The Go Parallel Website,
sponsored by Intel and developed in partnership with Slashdot Media, is your
hub for all things parallel software development, from weekly thought
leadership blogs to news, videos, case studies, tutorials and more. Take a
look and join the conversation now. http://goparallel.sourceforge.net/
_______________________________________________
E1000-devel mailing list
E1000-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/e1000-devel
To learn more about Intel&#174; Ethernet, visit http://communities.intel.com/community/wired

^ permalink raw reply

* Re: [PATCH 3/9] nftables: nft_rbtree: fix locking
From: Pablo Neira Ayuso @ 2015-01-30 10:52 UTC (permalink / raw)
  To: Patrick McHardy
  Cc: herbert, tgraf, davem, David.Laight, ying.xue, paulmck, netdev,
	netfilter-devel
In-Reply-To: <1422603994-5836-4-git-send-email-kaber@trash.net>

Hi Patrick,

On Fri, Jan 30, 2015 at 07:46:28AM +0000, Patrick McHardy wrote:
> Fix a race condition and unnecessary locking:
> 
> * the root rb_node must only be accessed under the lock in nft_rbtree_lookup()
> * the lock is not needed in lookup functions in netlink contexts
> 
> Signed-off-by: Patrick McHardy <kaber@trash.net>
> ---
>  net/netfilter/nft_rbtree.c | 12 +++---------
>  1 file changed, 3 insertions(+), 9 deletions(-)
> 
> diff --git a/net/netfilter/nft_rbtree.c b/net/netfilter/nft_rbtree.c
> index 46214f2..417796f 100644
> --- a/net/netfilter/nft_rbtree.c
> +++ b/net/netfilter/nft_rbtree.c
> @@ -37,10 +37,11 @@ static bool nft_rbtree_lookup(const struct nft_set *set,
>  {
>  	const struct nft_rbtree *priv = nft_set_priv(set);
>  	const struct nft_rbtree_elem *rbe, *interval = NULL;
> -	const struct rb_node *parent = priv->root.rb_node;
> +	const struct rb_node *parent;
>  	int d;
>  
>  	spin_lock_bh(&nft_rbtree_lock);
> +	parent = priv->root.rb_node;

Good catch.

>  	while (parent != NULL) {
>  		rbe = rb_entry(parent, struct nft_rbtree_elem, node);
>  
> @@ -158,7 +159,6 @@ static int nft_rbtree_get(const struct nft_set *set, struct nft_set_elem *elem)
>  	struct nft_rbtree_elem *rbe;
>  	int d;
>  
> -	spin_lock_bh(&nft_rbtree_lock);
>  	while (parent != NULL) {
>  		rbe = rb_entry(parent, struct nft_rbtree_elem, node);
>  
> @@ -173,11 +173,9 @@ static int nft_rbtree_get(const struct nft_set *set, struct nft_set_elem *elem)
>  			    !(rbe->flags & NFT_SET_ELEM_INTERVAL_END))
>  				nft_data_copy(&elem->data, rbe->data);
>  			elem->flags = rbe->flags;
> -			spin_unlock_bh(&nft_rbtree_lock);
>  			return 0;
>  		}
>  	}
> -	spin_unlock_bh(&nft_rbtree_lock);
>  	return -ENOENT;

this chunk looks fine to me, we always hold the nfnetlink mutex.

>  }
> @@ -190,7 +188,6 @@ static void nft_rbtree_walk(const struct nft_ctx *ctx,
>  	struct nft_set_elem elem;
>  	struct rb_node *node;
>  
> -	spin_lock_bh(&nft_rbtree_lock);
>  	for (node = rb_first(&priv->root); node != NULL; node = rb_next(node)) {
>  		if (iter->count < iter->skip)
>  			goto cont;
> @@ -203,14 +200,11 @@ static void nft_rbtree_walk(const struct nft_ctx *ctx,
>  		elem.flags = rbe->flags;
>  
>  		iter->err = iter->fn(ctx, set, iter, &elem);
> -		if (iter->err < 0) {
> -			spin_unlock_bh(&nft_rbtree_lock);
> +		if (iter->err < 0)
>  			return;
> -		}
>  cont:
>  		iter->count++;
>  	}
> -	spin_unlock_bh(&nft_rbtree_lock);
>  }
>  

I think that _walk still needs the lock there. This is called from
nf_tables_dump_set() for each recvmsg() in netlink, and IIRC unlike
rtnetlink the dump path in nfnetlink is lockless.

^ permalink raw reply

* Re: [PATCH v2 2/7] phy: miphy365x: Pass sysconfig register offsets via syscfg dt property.
From: Maxime Coquelin @ 2015-01-30 10:48 UTC (permalink / raw)
  To: Kishon Vijay Abraham I, Peter Griffin, linux-arm-kernel,
	linux-kernel, srinivas.kandagatla, patrice.chotard,
	peppe.cavallaro, arnd
  Cc: lee.jones, devicetree, netdev
In-Reply-To: <54CB5E7E.8050507@ti.com>

Hi Kishon,

On 01/30/2015 11:35 AM, Kishon Vijay Abraham I wrote:
> Hi,
>
> On Wednesday 07 January 2015 08:34 PM, Peter Griffin wrote:
>> Based on Arnds review comments here https://lkml.org/lkml/2014/11/13/161,
>> update the miphy365 phy driver to access sysconfig register offsets via
>> syscfg dt property.
>>
>> This is because the reg property should not be mixing address spaces
>> like it does currently for miphy365. This change then also aligns us
>> to how other platforms such as keystone and bcm7445 pass there syscon
>> offsets via DT.
>>
>> This patch breaks DT compatibility, but this platform is considered WIP,
>> and is only used by a few developers who are upstreaming support for it.
>> This change has been done as a single atomic commit to ensure it is
>> bisectable.
> I'm dropping this from my tree since I didn't get Ack from
> "arch/arm/boot/dts/stih416.dtsi" Maintainer.
Sorry, on cover letter, I replied the series looked good to me.
So you can add:

Acked-by: Maxime Coquelin <maxime.coquelin@st.com>

And even:

Tested-by: Maxime Coquelin <maxime.coquelin@st.com>

Kind regards,
Maxime

>
> Thanks
> Kishon
>> Signed-off-by: Peter Griffin <peter.griffin@linaro.org>
>> Reviewed-by: Arnd Bergmann <arnd@arndb.de>
>> ---
>>   .../devicetree/bindings/phy/phy-miphy365x.txt      | 15 +++++------
>>   arch/arm/boot/dts/stih416.dtsi                     | 10 ++++----
>>   drivers/phy/phy-miphy365x.c                        | 29 ++++++++--------------
>>   3 files changed, 23 insertions(+), 31 deletions(-)
>>
>> diff --git a/Documentation/devicetree/bindings/phy/phy-miphy365x.txt b/Documentation/devicetree/bindings/phy/phy-miphy365x.txt
>> index 42c8808..9802d5d 100644
>> --- a/Documentation/devicetree/bindings/phy/phy-miphy365x.txt
>> +++ b/Documentation/devicetree/bindings/phy/phy-miphy365x.txt
>> @@ -6,8 +6,10 @@ for SATA and PCIe.
>>   
>>   Required properties (controller (parent) node):
>>   - compatible    : Should be "st,miphy365x-phy"
>> -- st,syscfg     : Should be a phandle of the system configuration register group
>> -		  which contain the SATA, PCIe mode setting bits
>> +- st,syscfg     : Phandle / integer array property. Phandle of sysconfig group
>> +		  containing the miphy registers and integer array should contain
>> +		  an entry for each port sub-node, specifying the control
>> +		  register offset inside the sysconfig group.
>>   
>>   Required nodes	:  A sub-node is required for each channel the controller
>>   		   provides. Address range information including the usual
>> @@ -26,7 +28,6 @@ Required properties (port (child) node):
>>   		  registers filled in "reg":
>>   			- sata:   For SATA devices
>>   			- pcie:   For PCIe devices
>> -			- syscfg: To specify the syscfg based config register
>>   
>>   Optional properties (port (child) node):
>>   - st,sata-gen	     :	Generation of locally attached SATA IP. Expected values
>> @@ -39,20 +40,20 @@ Example:
>>   
>>   	miphy365x_phy: miphy365x@fe382000 {
>>   		compatible      = "st,miphy365x-phy";
>> -		st,syscfg  	= <&syscfg_rear>;
>> +		st,syscfg  	= <&syscfg_rear 0x824 0x828>;
>>   		#address-cells	= <1>;
>>   		#size-cells	= <1>;
>>   		ranges;
>>   
>>   		phy_port0: port@fe382000 {
>> -			reg = <0xfe382000 0x100>, <0xfe394000 0x100>, <0x824 0x4>;
>> -			reg-names = "sata", "pcie", "syscfg";
>> +			reg = <0xfe382000 0x100>, <0xfe394000 0x100>;
>> +			reg-names = "sata", "pcie";
>>   			#phy-cells = <1>;
>>   			st,sata-gen = <3>;
>>   		};
>>   
>>   		phy_port1: port@fe38a000 {
>> -			reg = <0xfe38a000 0x100>, <0xfe804000 0x100>, <0x828 0x4>;;
>> +			reg = <0xfe38a000 0x100>, <0xfe804000 0x100>;;
>>   			reg-names = "sata", "pcie", "syscfg";
>>   			#phy-cells = <1>;
>>   			st,pcie-tx-pol-inv;
>> diff --git a/arch/arm/boot/dts/stih416.dtsi b/arch/arm/boot/dts/stih416.dtsi
>> index fad9073..85afe01 100644
>> --- a/arch/arm/boot/dts/stih416.dtsi
>> +++ b/arch/arm/boot/dts/stih416.dtsi
>> @@ -283,21 +283,21 @@
>>   
>>   		miphy365x_phy: phy@fe382000 {
>>   			compatible      = "st,miphy365x-phy";
>> -			st,syscfg  	= <&syscfg_rear>;
>> +			st,syscfg	= <&syscfg_rear 0x824 0x828>;
>>   			#address-cells	= <1>;
>>   			#size-cells	= <1>;
>>   			ranges;
>>   
>>   			phy_port0: port@fe382000 {
>>   				#phy-cells = <1>;
>> -				reg = <0xfe382000 0x100>, <0xfe394000 0x100>, <0x824 0x4>;
>> -				reg-names = "sata", "pcie", "syscfg";
>> +				reg = <0xfe382000 0x100>, <0xfe394000 0x100>;
>> +				reg-names = "sata", "pcie";
>>   			};
>>   
>>   			phy_port1: port@fe38a000 {
>>   				#phy-cells = <1>;
>> -				reg = <0xfe38a000 0x100>, <0xfe804000 0x100>, <0x828 0x4>;
>> -				reg-names = "sata", "pcie", "syscfg";
>> +				reg = <0xfe38a000 0x100>, <0xfe804000 0x100>;
>> +				reg-names = "sata", "pcie";
>>   			};
>>   		};
>>   
>> diff --git a/drivers/phy/phy-miphy365x.c b/drivers/phy/phy-miphy365x.c
>> index 6ab43a8..6c80154 100644
>> --- a/drivers/phy/phy-miphy365x.c
>> +++ b/drivers/phy/phy-miphy365x.c
>> @@ -141,7 +141,7 @@ struct miphy365x_phy {
>>   	bool pcie_tx_pol_inv;
>>   	bool sata_tx_pol_inv;
>>   	u32 sata_gen;
>> -	u64 ctrlreg;
>> +	u32 ctrlreg;
>>   	u8 type;
>>   };
>>   
>> @@ -179,7 +179,7 @@ static int miphy365x_set_path(struct miphy365x_phy *miphy_phy,
>>   	bool sata = (miphy_phy->type == MIPHY_TYPE_SATA);
>>   
>>   	return regmap_update_bits(miphy_dev->regmap,
>> -				  (unsigned int)miphy_phy->ctrlreg,
>> +				  miphy_phy->ctrlreg,
>>   				  SYSCFG_SELECT_SATA_MASK,
>>   				  sata << SYSCFG_SELECT_SATA_POS);
>>   }
>> @@ -445,7 +445,6 @@ int miphy365x_get_addr(struct device *dev, struct miphy365x_phy *miphy_phy,
>>   {
>>   	struct device_node *phynode = miphy_phy->phy->dev.of_node;
>>   	const char *name;
>> -	const __be32 *taddr;
>>   	int type = miphy_phy->type;
>>   	int ret;
>>   
>> @@ -455,22 +454,6 @@ int miphy365x_get_addr(struct device *dev, struct miphy365x_phy *miphy_phy,
>>   		return ret;
>>   	}
>>   
>> -	if (!strncmp(name, "syscfg", 6)) {
>> -		taddr = of_get_address(phynode, index, NULL, NULL);
>> -		if (!taddr) {
>> -			dev_err(dev, "failed to fetch syscfg address\n");
>> -			return -EINVAL;
>> -		}
>> -
>> -		miphy_phy->ctrlreg = of_translate_address(phynode, taddr);
>> -		if (miphy_phy->ctrlreg == OF_BAD_ADDR) {
>> -			dev_err(dev, "failed to translate syscfg address\n");
>> -			return -EINVAL;
>> -		}
>> -
>> -		return 0;
>> -	}
>> -
>>   	if (!((!strncmp(name, "sata", 4) && type == MIPHY_TYPE_SATA) ||
>>   	      (!strncmp(name, "pcie", 4) && type == MIPHY_TYPE_PCIE)))
>>   		return 0;
>> @@ -606,7 +589,15 @@ static int miphy365x_probe(struct platform_device *pdev)
>>   			return ret;
>>   
>>   		phy_set_drvdata(phy, miphy_dev->phys[port]);
>> +
>>   		port++;
>> +		/* sysconfig offsets are indexed from 1 */
>> +		ret = of_property_read_u32_index(np, "st,syscfg", port,
>> +					&miphy_phy->ctrlreg);
>> +		if (ret) {
>> +			dev_err(&pdev->dev, "No sysconfig offset found\n");
>> +			return ret;
>> +		}
>>   	}
>>   
>>   	provider = devm_of_phy_provider_register(&pdev->dev, miphy365x_xlate);
>>

^ permalink raw reply

* Re: [PATCH] Repair soft lockup with monitor mode of ath9k_htc card
From: Kalle Valo @ 2015-01-30 10:44 UTC (permalink / raw)
  To: yuweizheng
  Cc: linux-kernel, ath9k-devel, linux-wireless, ath9k-devel, linux,
	netdev, Yuwei Zheng
In-Reply-To: <1422486872-16308-1-git-send-email-yuweizheng@139.com>

yuweizheng@139.com writes:

> From: Yuwei Zheng <yuweizheng@139.com>
>
> In the environment with heavy wifi traffic, set the ar9271 into
> monitor mode, will trigger a deadloop panic.
>  
> The ath9k_hif_usb_rx_cb function excute on  the interrupt context, and ath9k_rx_tasklet excute
> on the soft irq context. In other words, the ath9k_hif_usb_rx_cb have more chance to excute than
> ath9k_rx_tasklet.  So in the worst condition,  the rx.rxbuf receive list is always full,
> and the do {}while(true) loop will not be break. The kernel get a soft lockup panic. 

Word wrapping is too long and please prefix the title with "ath9k_htc: ".

-- 
Kalle Valo

^ permalink raw reply

* Re: [PATCH v2 2/7] phy: miphy365x: Pass sysconfig register offsets via syscfg dt property.
From: Kishon Vijay Abraham I @ 2015-01-30 10:35 UTC (permalink / raw)
  To: Peter Griffin, linux-arm-kernel, linux-kernel,
	srinivas.kandagatla, maxime.coquelin, patrice.chotard,
	peppe.cavallaro, arnd
  Cc: lee.jones, devicetree, netdev
In-Reply-To: <1420643052-4506-3-git-send-email-peter.griffin@linaro.org>

Hi,

On Wednesday 07 January 2015 08:34 PM, Peter Griffin wrote:
> Based on Arnds review comments here https://lkml.org/lkml/2014/11/13/161,
> update the miphy365 phy driver to access sysconfig register offsets via
> syscfg dt property.
> 
> This is because the reg property should not be mixing address spaces
> like it does currently for miphy365. This change then also aligns us
> to how other platforms such as keystone and bcm7445 pass there syscon
> offsets via DT.
> 
> This patch breaks DT compatibility, but this platform is considered WIP,
> and is only used by a few developers who are upstreaming support for it.
> This change has been done as a single atomic commit to ensure it is
> bisectable.

I'm dropping this from my tree since I didn't get Ack from
"arch/arm/boot/dts/stih416.dtsi" Maintainer.

Thanks
Kishon
> 
> Signed-off-by: Peter Griffin <peter.griffin@linaro.org>
> Reviewed-by: Arnd Bergmann <arnd@arndb.de>
> ---
>  .../devicetree/bindings/phy/phy-miphy365x.txt      | 15 +++++------
>  arch/arm/boot/dts/stih416.dtsi                     | 10 ++++----
>  drivers/phy/phy-miphy365x.c                        | 29 ++++++++--------------
>  3 files changed, 23 insertions(+), 31 deletions(-)
> 
> diff --git a/Documentation/devicetree/bindings/phy/phy-miphy365x.txt b/Documentation/devicetree/bindings/phy/phy-miphy365x.txt
> index 42c8808..9802d5d 100644
> --- a/Documentation/devicetree/bindings/phy/phy-miphy365x.txt
> +++ b/Documentation/devicetree/bindings/phy/phy-miphy365x.txt
> @@ -6,8 +6,10 @@ for SATA and PCIe.
>  
>  Required properties (controller (parent) node):
>  - compatible    : Should be "st,miphy365x-phy"
> -- st,syscfg     : Should be a phandle of the system configuration register group
> -		  which contain the SATA, PCIe mode setting bits
> +- st,syscfg     : Phandle / integer array property. Phandle of sysconfig group
> +		  containing the miphy registers and integer array should contain
> +		  an entry for each port sub-node, specifying the control
> +		  register offset inside the sysconfig group.
>  
>  Required nodes	:  A sub-node is required for each channel the controller
>  		   provides. Address range information including the usual
> @@ -26,7 +28,6 @@ Required properties (port (child) node):
>  		  registers filled in "reg":
>  			- sata:   For SATA devices
>  			- pcie:   For PCIe devices
> -			- syscfg: To specify the syscfg based config register
>  
>  Optional properties (port (child) node):
>  - st,sata-gen	     :	Generation of locally attached SATA IP. Expected values
> @@ -39,20 +40,20 @@ Example:
>  
>  	miphy365x_phy: miphy365x@fe382000 {
>  		compatible      = "st,miphy365x-phy";
> -		st,syscfg  	= <&syscfg_rear>;
> +		st,syscfg  	= <&syscfg_rear 0x824 0x828>;
>  		#address-cells	= <1>;
>  		#size-cells	= <1>;
>  		ranges;
>  
>  		phy_port0: port@fe382000 {
> -			reg = <0xfe382000 0x100>, <0xfe394000 0x100>, <0x824 0x4>;
> -			reg-names = "sata", "pcie", "syscfg";
> +			reg = <0xfe382000 0x100>, <0xfe394000 0x100>;
> +			reg-names = "sata", "pcie";
>  			#phy-cells = <1>;
>  			st,sata-gen = <3>;
>  		};
>  
>  		phy_port1: port@fe38a000 {
> -			reg = <0xfe38a000 0x100>, <0xfe804000 0x100>, <0x828 0x4>;;
> +			reg = <0xfe38a000 0x100>, <0xfe804000 0x100>;;
>  			reg-names = "sata", "pcie", "syscfg";
>  			#phy-cells = <1>;
>  			st,pcie-tx-pol-inv;
> diff --git a/arch/arm/boot/dts/stih416.dtsi b/arch/arm/boot/dts/stih416.dtsi
> index fad9073..85afe01 100644
> --- a/arch/arm/boot/dts/stih416.dtsi
> +++ b/arch/arm/boot/dts/stih416.dtsi
> @@ -283,21 +283,21 @@
>  
>  		miphy365x_phy: phy@fe382000 {
>  			compatible      = "st,miphy365x-phy";
> -			st,syscfg  	= <&syscfg_rear>;
> +			st,syscfg	= <&syscfg_rear 0x824 0x828>;
>  			#address-cells	= <1>;
>  			#size-cells	= <1>;
>  			ranges;
>  
>  			phy_port0: port@fe382000 {
>  				#phy-cells = <1>;
> -				reg = <0xfe382000 0x100>, <0xfe394000 0x100>, <0x824 0x4>;
> -				reg-names = "sata", "pcie", "syscfg";
> +				reg = <0xfe382000 0x100>, <0xfe394000 0x100>;
> +				reg-names = "sata", "pcie";
>  			};
>  
>  			phy_port1: port@fe38a000 {
>  				#phy-cells = <1>;
> -				reg = <0xfe38a000 0x100>, <0xfe804000 0x100>, <0x828 0x4>;
> -				reg-names = "sata", "pcie", "syscfg";
> +				reg = <0xfe38a000 0x100>, <0xfe804000 0x100>;
> +				reg-names = "sata", "pcie";
>  			};
>  		};
>  
> diff --git a/drivers/phy/phy-miphy365x.c b/drivers/phy/phy-miphy365x.c
> index 6ab43a8..6c80154 100644
> --- a/drivers/phy/phy-miphy365x.c
> +++ b/drivers/phy/phy-miphy365x.c
> @@ -141,7 +141,7 @@ struct miphy365x_phy {
>  	bool pcie_tx_pol_inv;
>  	bool sata_tx_pol_inv;
>  	u32 sata_gen;
> -	u64 ctrlreg;
> +	u32 ctrlreg;
>  	u8 type;
>  };
>  
> @@ -179,7 +179,7 @@ static int miphy365x_set_path(struct miphy365x_phy *miphy_phy,
>  	bool sata = (miphy_phy->type == MIPHY_TYPE_SATA);
>  
>  	return regmap_update_bits(miphy_dev->regmap,
> -				  (unsigned int)miphy_phy->ctrlreg,
> +				  miphy_phy->ctrlreg,
>  				  SYSCFG_SELECT_SATA_MASK,
>  				  sata << SYSCFG_SELECT_SATA_POS);
>  }
> @@ -445,7 +445,6 @@ int miphy365x_get_addr(struct device *dev, struct miphy365x_phy *miphy_phy,
>  {
>  	struct device_node *phynode = miphy_phy->phy->dev.of_node;
>  	const char *name;
> -	const __be32 *taddr;
>  	int type = miphy_phy->type;
>  	int ret;
>  
> @@ -455,22 +454,6 @@ int miphy365x_get_addr(struct device *dev, struct miphy365x_phy *miphy_phy,
>  		return ret;
>  	}
>  
> -	if (!strncmp(name, "syscfg", 6)) {
> -		taddr = of_get_address(phynode, index, NULL, NULL);
> -		if (!taddr) {
> -			dev_err(dev, "failed to fetch syscfg address\n");
> -			return -EINVAL;
> -		}
> -
> -		miphy_phy->ctrlreg = of_translate_address(phynode, taddr);
> -		if (miphy_phy->ctrlreg == OF_BAD_ADDR) {
> -			dev_err(dev, "failed to translate syscfg address\n");
> -			return -EINVAL;
> -		}
> -
> -		return 0;
> -	}
> -
>  	if (!((!strncmp(name, "sata", 4) && type == MIPHY_TYPE_SATA) ||
>  	      (!strncmp(name, "pcie", 4) && type == MIPHY_TYPE_PCIE)))
>  		return 0;
> @@ -606,7 +589,15 @@ static int miphy365x_probe(struct platform_device *pdev)
>  			return ret;
>  
>  		phy_set_drvdata(phy, miphy_dev->phys[port]);
> +
>  		port++;
> +		/* sysconfig offsets are indexed from 1 */
> +		ret = of_property_read_u32_index(np, "st,syscfg", port,
> +					&miphy_phy->ctrlreg);
> +		if (ret) {
> +			dev_err(&pdev->dev, "No sysconfig offset found\n");
> +			return ret;
> +		}
>  	}
>  
>  	provider = devm_of_phy_provider_register(&pdev->dev, miphy365x_xlate);
> 

^ permalink raw reply

* [PATCH] Repair soft lockup with monitor mode of ath9k_htc card
From: yuweizheng @ 2015-01-28 23:14 UTC (permalink / raw)
  To: linux-kernel, ath9k-devel, linux-wireless, kvalo, ath9k-devel,
	linux
  Cc: netdev, Yuwei Zheng, Yuwei Zheng

From: Yuwei Zheng <yuweizheng@139.com>

In the environment with heavy wifi traffic, set the ar9271 into monitor mode, will
trigger a deadloop panic.
 
The ath9k_hif_usb_rx_cb function excute on  the interrupt context, and ath9k_rx_tasklet excute
on the soft irq context. In other words, the ath9k_hif_usb_rx_cb have more chance to excute than
ath9k_rx_tasklet.  So in the worst condition,  the rx.rxbuf receive list is always full,
and the do {}while(true) loop will not be break. The kernel get a soft lockup panic. 
 
[59011.007210] BUG: soft lockup - CPU#0 stuck for 23s!
[kworker/0:0:30609]
[59011.030560] BUG: scheduling while atomic: kworker/0:0/30609/0x40010100
[59013.804486] BUG: scheduling while atomic: kworker/0:0/30609/0x40010100
[59013.858522] Kernel panic - not syncing: softlockup: hung tasks
 
[59014.038891] Exception stack(0xdf4bbc38 to 0xdf4bbc80)
[59014.046834] bc20:                                                       de57b950 60000113
[59014.059579] bc40: 00000000 bb32bb32 60000113 de57b948 de57b500 dc7bb440 df4bbcd0 00000000
[59014.072337] bc60: de57b950 60000113 df4bbcd0 df4bbc80 c04c259d c04c25a0 60000133 ffffffff
[59014.085233] [<c04c28db>] (__irq_svc+0x3b/0x5c) from [<c04c25a0>] (_raw_spin_unlock_irqrestore+0xc/0x10)
[59014.100437] [<c04c25a0>] (_raw_spin_unlock_irqrestore+0xc/0x10) from [<bf9c2089>] (ath9k_rx_tasklet+0x290/0x490 [ath9k_htc])
[59014.118267] [<bf9c2089>] (ath9k_rx_tasklet+0x290/0x490 [ath9k_htc]) from [<c0036d23>] (tasklet_action+0x3b/0x98)
[59014.134132] [<c0036d23>] (tasklet_action+0x3b/0x98) from [<c0036709>] (__do_softirq+0x99/0x16c)
[59014.147784] [<c0036709>] (__do_softirq+0x99/0x16c) from [<c00369f7>] (irq_exit+0x5b/0x5c)
[59014.160653] [<c00369f7>] (irq_exit+0x5b/0x5c) from [<c000cfc3>] (handle_IRQ+0x37/0x78)
[59014.173124] [<c000cfc3>] (handle_IRQ+0x37/0x78) from [<c00085df>] (omap3_intc_handle_irq+0x5f/0x68)
[59014.187225] [<c00085df>] (omap3_intc_handle_irq+0x5f/0x68) from [<c04c28db>](__irq_svc+0x3b/0x5c)
 
This bug can be see with low performance board, such as uniprocessor beagle bone board.
 
 
Signed-off-by: Yuwei Zheng <zhengyuwei@360.cn>
Signed-off-by: Yuwei Zheng <yuweizheng@139.com>


---
 drivers/net/wireless/ath/ath9k/hif_usb.c       | 58 ++++++++++++++++++++++----
 drivers/net/wireless/ath/ath9k/hif_usb.h       |  5 +++
 drivers/net/wireless/ath/ath9k/htc.h           | 13 ++++++
 drivers/net/wireless/ath/ath9k/htc_drv_debug.c | 49 ++++++++++++++++++++++
 drivers/net/wireless/ath/ath9k/htc_drv_txrx.c  | 26 ++++++++++++
 5 files changed, 144 insertions(+), 7 deletions(-)

diff --git a/drivers/net/wireless/ath/ath9k/hif_usb.c b/drivers/net/wireless/ath/ath9k/hif_usb.c
index 8e7153b..18c6f0e 100644
--- a/drivers/net/wireless/ath/ath9k/hif_usb.c
+++ b/drivers/net/wireless/ath/ath9k/hif_usb.c
@@ -658,7 +658,6 @@ static void ath9k_hif_usb_rx_cb(struct urb *urb)
 	default:
 		goto resubmit;
 	}
-
 	if (likely(urb->actual_length != 0)) {
 		skb_put(skb, urb->actual_length);
 		ath9k_hif_usb_rx_stream(hif_dev, skb);
@@ -667,12 +666,18 @@ static void ath9k_hif_usb_rx_cb(struct urb *urb)
 resubmit:
 	skb_reset_tail_pointer(skb);
 	skb_trim(skb, 0);
-
-	usb_anchor_urb(urb, &hif_dev->rx_submitted);
-	ret = usb_submit_urb(urb, GFP_ATOMIC);
-	if (ret) {
-		usb_unanchor_urb(urb);
-		goto free;
+	if (atomic_read(&hif_dev->rx_urb_submit_delay) > 0) {
+		usb_anchor_urb(urb, &hif_dev->rx_delayed_submitted);
+		ret = tasklet_hrtimer_start(&hif_dev->rx_submit_timer,
+						ktime_set(0, atomic_read(&hif_dev->rx_urb_submit_delay)*1000),
+						HRTIMER_MODE_REL);
+	} else {
+		usb_anchor_urb(urb, &hif_dev->rx_submitted);
+		ret = usb_submit_urb(urb, GFP_ATOMIC);
+		if (ret) {
+			usb_unanchor_urb(urb);
+			goto free;
+		}
 	}
 
 	return;
@@ -818,9 +823,39 @@ err:
 	return -ENOMEM;
 }
 
+static enum hrtimer_restart rx_urb_submit_timer_handler(struct hrtimer *me)
+{
+	struct tasklet_hrtimer *thr =
+		container_of(me, struct tasklet_hrtimer, timer);
+	struct  hif_device_usb *hif_dev =
+		container_of(thr, struct hif_device_usb, rx_submit_timer);
+	struct urb *urb = NULL;
+	struct sk_buff *skb = NULL;
+	int ret;
+
+	while (true) {
+		urb = usb_get_from_anchor(&hif_dev->rx_delayed_submitted);
+		if (urb != NULL) {
+			skb = (struct sk_buff *)urb->context;
+			ret = usb_submit_urb(urb, GFP_ATOMIC);
+			if (ret != -EBUSY) {
+				usb_unanchor_urb(urb);
+				dev_kfree_skb_any(skb);
+				urb->context = NULL;
+			}
+		} else {
+			break;
+		}
+	}
+
+	return HRTIMER_NORESTART;
+}
+
 static void ath9k_hif_usb_dealloc_rx_urbs(struct hif_device_usb *hif_dev)
 {
 	usb_kill_anchored_urbs(&hif_dev->rx_submitted);
+	usb_kill_anchored_urbs(&hif_dev->rx_delayed_submitted);
+	tasklet_hrtimer_cancel(&hif_dev->rx_submit_timer);
 }
 
 static int ath9k_hif_usb_alloc_rx_urbs(struct hif_device_usb *hif_dev)
@@ -830,6 +865,8 @@ static int ath9k_hif_usb_alloc_rx_urbs(struct hif_device_usb *hif_dev)
 	int i, ret;
 
 	init_usb_anchor(&hif_dev->rx_submitted);
+	init_usb_anchor(&hif_dev->rx_delayed_submitted);
+
 	spin_lock_init(&hif_dev->rx_lock);
 
 	for (i = 0; i < MAX_RX_URB_NUM; i++) {
@@ -871,6 +908,13 @@ static int ath9k_hif_usb_alloc_rx_urbs(struct hif_device_usb *hif_dev)
 		usb_free_urb(urb);
 	}
 
+	/* add for flow control*/
+	atomic_set(&hif_dev->rx_urb_submit_delay, 0);
+	tasklet_hrtimer_init(&hif_dev->rx_submit_timer,
+				rx_urb_submit_timer_handler,
+				CLOCK_MONOTONIC,
+				HRTIMER_MODE_REL);
+
 	return 0;
 
 err_submit:
diff --git a/drivers/net/wireless/ath/ath9k/hif_usb.h b/drivers/net/wireless/ath/ath9k/hif_usb.h
index 51496e7..56d6be8 100644
--- a/drivers/net/wireless/ath/ath9k/hif_usb.h
+++ b/drivers/net/wireless/ath/ath9k/hif_usb.h
@@ -98,9 +98,14 @@ struct hif_device_usb {
 	struct hif_usb_tx tx;
 	struct usb_anchor regout_submitted;
 	struct usb_anchor rx_submitted;
+	struct usb_anchor rx_delayed_submitted; /* delayed submit anchor */
 	struct usb_anchor reg_in_submitted;
 	struct usb_anchor mgmt_submitted;
 	struct sk_buff *remain_skb;
+
+	struct tasklet_hrtimer  rx_submit_timer;/* delayed submit hrtimer */
+	atomic_t  rx_urb_submit_delay; /*us*/
+
 	const char *fw_name;
 	int rx_remain_len;
 	int rx_pkt_len;
diff --git a/drivers/net/wireless/ath/ath9k/htc.h b/drivers/net/wireless/ath/ath9k/htc.h
index 9dde265..453d0a8 100644
--- a/drivers/net/wireless/ath/ath9k/htc.h
+++ b/drivers/net/wireless/ath/ath9k/htc.h
@@ -331,6 +331,10 @@ static inline struct ath9k_htc_tx_ctl *HTC_SKB_CB(struct sk_buff *skb)
 
 #define TX_QSTAT_INC(q) (priv->debug.tx_stats.queue_stats[q]++)
 
+#define TASKLETRX_STAT_INC(c) (hif_dev->htc_handle->drv_priv->debug.taskletrx_stats.c++)
+#define TASKLETRX_STAT_ADD(c, a) (hif_dev->htc_handle->drv_priv->debug.taskletrx_stats.c += a)
+#define TASKLETRX_STAT_SET(c, a) (hif_dev->htc_handle->drv_priv->debug.taskletrx_stats.c = a)
+
 void ath9k_htc_err_stat_rx(struct ath9k_htc_priv *priv,
 			   struct ath_rx_status *rs);
 
@@ -352,11 +356,20 @@ struct ath_skbrx_stats {
 	u32 skb_dropped;
 };
 
+struct ath_taskletrx_stats {
+	u32 taskletrx_looptimes;
+	u32 taskletrx_highwater;
+	u32 taskletrx_lowwater;
+	u32 taskletrx_watermark_triggered;
+	u32 taskletrx_urb_submit_delay;
+};
+
 struct ath9k_debug {
 	struct dentry *debugfs_phy;
 	struct ath_tx_stats tx_stats;
 	struct ath_rx_stats rx_stats;
 	struct ath_skbrx_stats skbrx_stats;
+	struct ath_taskletrx_stats taskletrx_stats;
 };
 
 void ath9k_htc_get_et_strings(struct ieee80211_hw *hw,
diff --git a/drivers/net/wireless/ath/ath9k/htc_drv_debug.c b/drivers/net/wireless/ath/ath9k/htc_drv_debug.c
index 8cef1ed..7c8322e 100644
--- a/drivers/net/wireless/ath/ath9k/htc_drv_debug.c
+++ b/drivers/net/wireless/ath/ath9k/htc_drv_debug.c
@@ -286,6 +286,51 @@ static const struct file_operations fops_skb_rx = {
 	.llseek = default_llseek,
 };
 
+static ssize_t read_file_tasklet_rx(struct file *file, char __user *user_buf,
+				    size_t count, loff_t *ppos)
+{
+	struct ath9k_htc_priv *priv = file->private_data;
+	char *buf;
+	unsigned int len = 0, size = 1500;
+	ssize_t retval = 0;
+
+	buf = kzalloc(size, GFP_KERNEL);
+	if (buf == NULL)
+		return -ENOMEM;
+
+	len += scnprintf(buf + len, size - len,
+			"%20s : %10u\n", "Loop times",
+			priv->debug.taskletrx_stats.taskletrx_looptimes);
+	len += scnprintf(buf + len, size - len,
+			"%20s : %10u\n", "High watermark",
+			priv->debug.taskletrx_stats.taskletrx_highwater);
+	len += scnprintf(buf + len, size - len,
+			"%20s : %10u\n", "Low watermark",
+			priv->debug.taskletrx_stats.taskletrx_lowwater);
+
+	len += scnprintf(buf + len, size - len,
+			"%20s : %10u\n", "WM triggered",
+			priv->debug.taskletrx_stats.taskletrx_watermark_triggered);
+
+	len += scnprintf(buf + len, size - len,
+			"%20s : %10u\n", "URB delay",
+			priv->debug.taskletrx_stats.taskletrx_urb_submit_delay);
+	if (len > size)
+		len = size;
+
+	retval = simple_read_from_buffer(user_buf, count, ppos, buf, len);
+	kfree(buf);
+
+	return retval;
+}
+
+static const struct file_operations fops_tasklet_rx = {
+	.read = read_file_tasklet_rx,
+	.open = simple_open,
+	.owner = THIS_MODULE,
+	.llseek = default_llseek,
+};
+
 static ssize_t read_file_slot(struct file *file, char __user *user_buf,
 			      size_t count, loff_t *ppos)
 {
@@ -518,7 +563,11 @@ int ath9k_htc_init_debug(struct ath_hw *ah)
 	debugfs_create_file("skb_rx", S_IRUSR, priv->debug.debugfs_phy,
 			    priv, &fops_skb_rx);
 
+	debugfs_create_file("tasklet_rx", S_IRUSR, priv->debug.debugfs_phy,
+			    priv, &fops_tasklet_rx);
+
 	ath9k_cmn_debug_recv(priv->debug.debugfs_phy, &priv->debug.rx_stats);
+
 	ath9k_cmn_debug_phy_err(priv->debug.debugfs_phy, &priv->debug.rx_stats);
 
 	debugfs_create_file("slot", S_IRUSR, priv->debug.debugfs_phy,
diff --git a/drivers/net/wireless/ath/ath9k/htc_drv_txrx.c b/drivers/net/wireless/ath/ath9k/htc_drv_txrx.c
index a0f58e2..f5e6217 100644
--- a/drivers/net/wireless/ath/ath9k/htc_drv_txrx.c
+++ b/drivers/net/wireless/ath/ath9k/htc_drv_txrx.c
@@ -1061,7 +1061,28 @@ void ath9k_rx_tasklet(unsigned long data)
 	unsigned long flags;
 	struct ieee80211_hdr *hdr;
 
+	/* add for adaptive flow control*/
+	int looptimes = 0;
+	int highwatermark = ATH9K_HTC_RXBUF*3/4;
+	int lowwatermark = ATH9K_HTC_RXBUF/4;
+	unsigned int delay = 0;
+
+	struct htc_target *htc = priv->htc;
+	struct hif_device_usb *hif_dev = htc->hif_dev;
+
+	TASKLETRX_STAT_SET(taskletrx_highwater, highwatermark);
+	TASKLETRX_STAT_SET(taskletrx_lowwater, lowwatermark);
+
 	do {
+		looptimes++;
+		TASKLETRX_STAT_SET(taskletrx_looptimes, looptimes);
+		if (looptimes > highwatermark) {
+			delay = looptimes*10;
+			atomic_set(&hif_dev->rx_urb_submit_delay, delay);
+			TASKLETRX_STAT_INC(taskletrx_watermark_triggered);
+			TASKLETRX_STAT_SET(taskletrx_urb_submit_delay, delay);
+		}
+
 		spin_lock_irqsave(&priv->rx.rxbuflock, flags);
 		list_for_each_entry(tmp_buf, &priv->rx.rxbuf, list) {
 			if (tmp_buf->in_process) {
@@ -1072,6 +1093,11 @@ void ath9k_rx_tasklet(unsigned long data)
 
 		if (rxbuf == NULL) {
 			spin_unlock_irqrestore(&priv->rx.rxbuflock, flags);
+			if (looptimes < lowwatermark) {
+				atomic_set(&hif_dev->rx_urb_submit_delay, 0);
+				TASKLETRX_STAT_SET(taskletrx_urb_submit_delay, 0);
+			}
+
 			break;
 		}
 
-- 
1.9.1

^ permalink raw reply related

* Re: Throughput regression with `tcp: refine TSO autosizing`
From: Arend van Spriel @ 2015-01-30 10:29 UTC (permalink / raw)
  To: Eric Dumazet; +Cc: Michal Kazior, linux-wireless, Network Development, eyalpe
In-Reply-To: <1422537297.21689.15.camel@edumazet-glaptop2.roam.corp.google.com>

On 01/29/15 14:14, Eric Dumazet wrote:
> On Thu, 2015-01-29 at 12:48 +0100, Michal Kazior wrote:
>> Hi,
>>
>> I'm not subscribed to netdev list and I can't find the message-id so I
>> can't reply directly to the original thread `BW regression after "tcp:
>> refine TSO autosizing"`.
>>
>> I've noticed a big TCP performance drop with ath10k
>> (drivers/net/wireless/ath/ath10k) on 3.19-rc5. Instead of 500mbps I
>> get 250mbps in my testbed.
>>
>> After bisecting I ended up at `tcp: refine TSO autosizing`. Reverting
>> `tcp: refine TSO autosizing` and `tcp: Do not apply TSO segment limit
>> to non-TSO packets` (for conflict free reverts) fixes the problem.
>>
>> My testing setup is as follows:
>>
>>   a) ath10k AP, github.com/kvalo/ath/tree/master 3.19-rc5, w/ reverts
>>   b) ath10k STA connected to (a), github.com/kvalo/ath/tree/master
>> 3.19-rc5, w/ reverts
>>   c) (b) w/o reverts
>>
>> Devices are 3x3 (AP) and 2x2 (Client) and are RF cabled. 11ac@80MHz
>> 2x2 has 866mbps modulation rate. In practice this should deliver
>> ~700mbps of real UDP traffic.
>>
>> Here are some numbers:
>>
>> UDP: (b) ->  (a): 672mbps
>> UDP: (a) ->  (b): 687mbps
>> TCP: (b) ->  (a): 526mbps
>> TCP: (a) ->  (b): 500mbps
>>
>> UDP: (c) ->  (a): 669mbps*
>> UDP: (a) ->  (c): 689mbps*
>> TCP: (c) ->  (a): 240mbps**
>> TCP: (a) ->  (c): 490mbps*
>>
>> * no changes/within error margin
>> ** the performance drop
>>
>> I'm using iperf:
>>    UDP: iperf -i1 -s -u vs iperf -i1 -c XX -u -B 200M -P5 -t 20
>>    TCP: iperf -i1 -s vs iperf -i1 -c XX -P5 -t 20
>>
>> Result values were obtained at the receiver side.
>>
>> Iperf reports a few frames lost and out-of-order at each UDP test
>> start (during first second) but later has no packet loss and no
>> out-of-order. This shouldn't have any effect on a TCP session, right?
>>
>> The device delivers batched up tx/rx completions (no way to change
>> that). I suppose this could be an issue for timing sensitive
>> algorithms. Also keep in mind 802.11n and 802.11ac devices have frame
>> aggregation windows so there's an inherent extra (and non-uniform)
>> latency when compared to, e.g. ethernet devices.
>>
>> The driver doesn't have GRO. I have an internal patch which implements
>> it. It improves overall TCP traffic (more stable, up to 600mbps TCP
>> which is ~100mbps more than without GRO) but the TCP: (c) ->  (a)
>> performance drop remains unaffected regardless.
>>
>> I've tried applying stretch ACK patchset (v2) on both machines and
>> re-run the above tests. I got no measurable difference in performance.
>>
>> I've also run these tests with iwlwifi 7260 (also a 2x2) as (b) and
>> (c). It didn't seem to be affected by the TSO patch at all (it runs at
>> ~360mbps of TCP regardless of the TSO patch).
>>
>> Any hints/ideas?
>>
>
> Hi Michal
>
> This patch restored original TSQ behavior, because the 1ms worth of data
> per flow had totally destroyed TSQ intent.
>
> vi +630 Documentation/networking/ip-sysctl.txt
>
> tcp_limit_output_bytes - INTEGER
>          Controls TCP Small Queue limit per tcp socket.
>          TCP bulk sender tends to increase packets in flight until it
>          gets losses notifications. With SNDBUF autotuning, this can
>          result in a large amount of packets queued in qdisc/device
>          on the local machine, hurting latency of other flows, for
>          typical pfifo_fast qdiscs.
>          tcp_limit_output_bytes limits the number of bytes on qdisc
>          or device to reduce artificial RTT/cwnd and reduce bufferbloat.
>          Default: 131072
>
> This is why I suggested to Eyal Perry to change the TX interrupt
> mitigation parameters as in :
>
> ethtool -C eth0 tx-frames 4 rx-frames 4
>
> With this change and the stretch ack fixes, I got 37Gbps of throughput
> on a single flow, on a 40Gbit NIC (mlx4)
>
> If a driver needs to buffer more than tcp_limit_output_bytes=131072 to
> get line rate, I suggest that you either :
>
> 1) tweak tcp_limit_output_bytes, but its not practical from a driver.
>
> 2) change the driver, knowing what are its exact requirements, by
> removing a fraction of skb->truesize at ndo_start_xmit() time as in :
>
> if ((skb->destructor == sock_wfree ||
>       skb->restuctor == tcp_wfree)&&
>      skb->sk) {
>      u32 fraction = skb->truesize / 2;
>
>      skb->truesize -= fraction;
>      atomic_sub(fraction,&skb->sk->sk_wmem_alloc);
> }

Hi Eric,

Your suggestions are still based on the fact that you consider wireless 
networking to be similar to ethernet, but as Michal indicated there are 
some fundamental differences starting with CSMA/CD versus CSMA/CA. Also 
the medium conditions are far from comparable. There is no shielding so 
it needs to deal with interference and dynamically drops the link rate 
so transmission of packets can take several milliseconds. Then with 11n 
they came up with aggregation with sends up to 64 packets in a single 
transmit over the air at worst case 6.5 Mbps (if I am not mistaken). The 
parameter value for tcp_limit_output_bytes of 131072 means that it 
allows queuing for about 1ms on a 1Gbps link, but I hope you can see 
this is not realistic for dealing with all variances of the wireless 
medium/standard. I suggested this as topic for the wireless workshop in 
Otawa [1], but I can not attend there. Still hope that there will be 
some discussions to get more awareness.

Regards,
Arend

[1] http://mid.gmane.org/54BE9791.1070706@broadcom.com

^ permalink raw reply

* Re: [PATCH v2 3/3] lib/string_helpers.c: Change semantics of string_escape_mem
From: Andy Shevchenko @ 2015-01-30 10:27 UTC (permalink / raw)
  To: Rasmus Villemoes
  Cc: Andrew Morton, Trond Myklebust, J. Bruce Fields, David S. Miller,
	linux-kernel, linux-nfs, netdev
In-Reply-To: <87oaphbqym.fsf@rasmusvillemoes.dk>

On Thu, 2015-01-29 at 15:29 +0100, Rasmus Villemoes wrote:
> On Thu, Jan 29 2015, Andy Shevchenko <andriy.shevchenko@linux.intel.com> wrote:
> 
> >>   *
> >>   * Return:
> >> - * The amount of the characters processed to the destination buffer, or
> >> - * %-ENOMEM if the size of buffer is not enough to put an escaped character is
> >> - * returned.
> >> - *
> >> - * Even in the case of error @dst pointer will be updated to point to the byte
> >> - * after the last processed character.
> >> + * The total size of the escaped output that would be generated for
> >> + * the given input and flags. To check whether the output was
> >> + * truncated, compare the return value to osz. There is room left in
> >> + * dst for a '\0' terminator if and only if ret < osz.
> >>   */
> >> -int string_escape_mem(const char *src, size_t isz, char **dst, size_t osz,
> >> -		      unsigned int flags, const char *esc)
> >> +size_t string_escape_mem(const char *src, size_t isz, char *dst, size_t osz,
> >> +			 unsigned int flags, const char *esc)
> >
> > I prefer to leave the prototype the same. int for return is okay. dst
> > should be updated accordingly.
> 
> Please explain exactly what you think the return value should be, and
> what *dst should be set to.

Return value like you proposed, *dst is incremented by it.

> 
> >>  {
> >> -	char *p = *dst;
> >> +	char *p = dst;
> >>  	char *end = p + osz;
> >>  	bool is_dict = esc && *esc;
> >> -	int ret;
> >>  
> >>  	while (isz--) {
> >>  		unsigned char c = *src++;
> >> @@ -466,13 +463,7 @@ int string_escape_mem(const char *src, size_t isz, char **dst, size_t osz,
> >>  
> >>  		escape_passthrough(c, &p, end);
> >>  	}
> >> -	if (p > end) {
> >> -		*dst = end;
> >> -		return -ENOMEM;
> >> -	}
> >>  
> >> -	ret = p - *dst;
> >> -	*dst = p;
> >> -	return ret;
> >> +	return p - dst;
> >>  }
> >>  EXPORT_SYMBOL(string_escape_mem);
> >> diff --git a/lib/test-string_helpers.c b/lib/test-string_helpers.c
> >> index ab0d30e1e18f..5f95114a2f86 100644
> >> --- a/lib/test-string_helpers.c
> >> +++ b/lib/test-string_helpers.c
> >> @@ -264,12 +264,12 @@ static __init void test_string_escape(const char *name,
> >>  				      const struct test_string_2 *s2,
> >>  				      unsigned int flags, const char *esc)
> >>  {
> >> -	int q_real = 512;
> >> -	char *out_test = kmalloc(q_real, GFP_KERNEL);
> >> -	char *out_real = kmalloc(q_real, GFP_KERNEL);
> >> +	size_t out_size = 512;
> >> +	char *out_test = kmalloc(out_size, GFP_KERNEL);
> >> +	char *out_real = kmalloc(out_size, GFP_KERNEL);
> >>  	char *in = kmalloc(256, GFP_KERNEL);
> >> -	char *buf = out_real;
> >> -	int p = 0, q_test = 0;
> >> +	size_t p = 0, q_test = 0;
> >> +	size_t q_real;
> >>  
> >>  	if (!out_test || !out_real || !in)
> >>  		goto out;
> >> @@ -301,29 +301,26 @@ static __init void test_string_escape(const char *name,
> >>  		q_test += len;
> >>  	}
> >>  
> >> -	q_real = string_escape_mem(in, p, &buf, q_real, flags, esc);
> >> +	q_real = string_escape_mem(in, p, out_real, out_size, flags, esc);
> >>  
> >>  	test_string_check_buf(name, flags, in, p, out_real, q_real, out_test,
> >>  			      q_test);
> >> +
> >> +	memset(out_real, 'Z', out_size);
> >> +	q_real = string_escape_mem(in, p, out_real, 0, flags, esc);
> >> +	if (q_real != q_test)
> >> +		pr_warn("Test '%s' failed: flags = %u, osz = 0, expected %zu, got %zu\n",
> >> +			name, flags, q_test, q_real);
> >> +	if (memchr_inv(out_real, 'Z', out_size))
> >> +		pr_warn("Test '%s' failed: osz = 0 but string_escape_mem wrote to the buffer\n",
> >> +			name);
> >
> > Could it be a part of nomem test still?
> 
> What nomem test? string_escape_mem with snprintf-like semantics cannot
> return an error; that has to be checked by the caller. 

Make this code a separate test, which actually still nomem, since you
have not enough memory in the destination buffer. What the problem to
check for proper return value and the last couple of characters written
to the destination buffer?

> 
> >> +
> >>  out:
> >>  	kfree(in);
> >>  	kfree(out_real);
> >>  	kfree(out_test);
> >>  }
> >>  
> >> -static __init void test_string_escape_nomem(void)
> >> -{
> >> -	char *in = "\eb \\C\007\"\x90\r]";
> >> -	char out[64], *buf = out;
> >> -	int rc = -ENOMEM, ret;
> >> -
> >> -	ret = string_escape_str_any_np(in, &buf, strlen(in), NULL);
> >> -	if (ret == rc)
> >> -		return;
> >> -
> >> -	pr_err("Test 'escape nomem' failed: got %d instead of %d\n", ret, rc);
> >> -}
> >> -
> >>  static int __init test_string_helpers_init(void)
> >>  {
> >>  	unsigned int i;
> >> @@ -342,8 +339,6 @@ static int __init test_string_helpers_init(void)
> >>  	for (i = 0; i < (ESCAPE_ANY_NP | ESCAPE_HEX) + 1; i++)
> >>  		test_string_escape("escape 1", escape1, i, TEST_STRING_2_DICT_1);
> >>  
> >> -	test_string_escape_nomem();
> >> -
> >>  	return -EINVAL;
> >>  }
> >>  module_init(test_string_helpers_init);
> >> diff --git a/lib/vsprintf.c b/lib/vsprintf.c
> >> index 3568e3906777..d02c394b5b58 100644
> >> --- a/lib/vsprintf.c
> >> +++ b/lib/vsprintf.c
> >> @@ -1160,7 +1160,7 @@ char *escaped_string(char *buf, char *end, u8 *addr, struct printf_spec spec,
> >>  	len = spec.field_width < 0 ? 1 : spec.field_width;
> >>  
> >>  	/* Ignore the error. We print as many characters as we can */
> >> -	string_escape_mem(addr, len, &buf, end - buf, flags, NULL);
> >> +	buf += string_escape_mem(addr, len, buf, buf < end ? end - buf : 0, flags, NULL);
> >
> > So, the problem is when we have end < buf, right?
> > How about to move this check out of the call parameters?
> >
> > [Keep in might the original prototype]
> >
> > if (buf < end)
> >  string_escape_mem(addr, len, &buf, end - buf, flags, NULL);
> > else
> >  string_escape_mem(addr, len, &buf, 0, flags, NULL);
> 
> In that case, I just did the same as is done for %pV, and prefer to keep
> it that way.

I've checked the other case, we may keep same style.

> >> diff --git a/net/sunrpc/cache.c b/net/sunrpc/cache.c
> >> index 33fb105d4352..22c4418057f4 100644
> >> --- a/net/sunrpc/cache.c
> >> +++ b/net/sunrpc/cache.c
> >> @@ -1068,12 +1068,14 @@ void qword_add(char **bpp, int *lp, char *str)
> >>  {
> >>  	char *bp = *bpp;
> >>  	int len = *lp;
> >> -	int ret;
> >> +	int ret, written;
> >>  
> >>  	if (len < 0) return;
> >>  
> >> -	ret = string_escape_str(str, &bp, len, ESCAPE_OCTAL, "\\ \n\t");
> >> -	if (ret < 0 || ret == len)
> >> +	ret = string_escape_str(str, bp, len, ESCAPE_OCTAL, "\\ \n\t");
> >> +	written = min(ret, len);
> >> +	bp += written;
> >> +	if (ret >= len)
> >>  		len = -1;
> >>  	else {
> >>  		len -= ret;
> >
> > For this part the comment from J. Bruce is needed.
> >
> > There is one more user, i.e. fs/proc/array.c::task_name().
> >
> > In all of them we have to amend a prepend commentary. Like changing
> > 'Ignore the error' to 'Ignore the overflow'.
> 
> I hadn't looked for users in -next. I'll leave it to you to amend that
> patch before it hits mainline.

When your series will be ready (and actually I recommend to push first
patch apart from the rest since it's not related) I may do the update
for fs/proc/array.c.


-- 
Andy Shevchenko <andriy.shevchenko@intel.com>
Intel Finland Oy

^ permalink raw reply

* Re: [PATCH net] hyperv: Fix the error processing in netvsc_send()
From: Jason Wang @ 2015-01-30 10:25 UTC (permalink / raw)
  To: Haiyang Zhang
  Cc: olaf, netdev, haiyangz, driverdev-devel, linux-kernel, davem
In-Reply-To: <1422563689-31036-1-git-send-email-haiyangz@microsoft.com>



On Fri, Jan 30, 2015 at 4:34 AM, Haiyang Zhang <haiyangz@microsoft.com> 
wrote:
> The existing code frees the skb in EAGAIN case, in which the skb will 
> be
> retried from upper layer and used again.
> Also, the existing code doesn't free send buffer slot in error case, 
> because
> there is no completion message for unsent packets.
> This patch fixes these problems.
> 
> (Please also include this patch for stable trees. Thanks!)
> 
> Signed-off-by: Haiyang Zhang <haiyangz@microsoft.com>
> Reviewed-by: K. Y. Srinivasan <kys@microsoft.com>
> ---
>  drivers/net/hyperv/netvsc.c |   11 ++++++++---
>  1 files changed, 8 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/net/hyperv/netvsc.c b/drivers/net/hyperv/netvsc.c
> index 9f49c01..7cd4eb3 100644
> --- a/drivers/net/hyperv/netvsc.c
> +++ b/drivers/net/hyperv/netvsc.c
> @@ -716,7 +716,7 @@ int netvsc_send(struct hv_device *device,
>  	u64 req_id;
>  	unsigned int section_index = NETVSC_INVALID_INDEX;
>  	u32 msg_size = 0;
> -	struct sk_buff *skb;
> +	struct sk_buff *skb = NULL;
>  	u16 q_idx = packet->q_idx;
>  
>  
> @@ -743,8 +743,6 @@ int netvsc_send(struct hv_device *device,
>  							   packet);
>  			skb = (struct sk_buff *)
>  			      (unsigned long)packet->send_completion_tid;
> -			if (skb)
> -				dev_kfree_skb_any(skb);
>  			packet->page_buf_cnt = 0;
>  		}
>  	}
> @@ -810,6 +808,13 @@ int netvsc_send(struct hv_device *device,
>  			   packet, ret);
>  	}
>  
> +	if (ret != 0) {
> +		if (section_index != NETVSC_INVALID_INDEX)
> +			netvsc_free_send_slot(net_device, section_index);

What if ret is -EINVAL or -ENOSPC? Looks like we need free the skb in 
this case also.
> 
> +	} else if (skb) {
> +		dev_kfree_skb_any(skb);

The caller - netvsc_start_xmit() do this also, may be handle this in 
caller is better since netvsc_start_xmit() is the only user that tries 
to send a skb?

btw, I find during netvsc_start_xmit(), ret was change to -ENOSPC when 
queue_sends[q_idx] < 1. But non of the caller check -ENOSPC in fact?

Thanks
> 
> +	}
> +
>  	return ret;
>  }
>  
> -- 
> 1.7.4.1
> 
> --
> To unsubscribe from this list: send the line "unsubscribe netdev" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply

* 答复: [ath9k-devel] [PATCH] Repair soft lockup with monitor mode of ath9k_htc card
From: 郑玉伟 @ 2015-01-30 10:24 UTC (permalink / raw)
  To: Oleksij Rempel, linux-kernel@vger.kernel.org,
	ath9k-devel@venema.h4ckr.net, linux-wireless@vger.kernel.org,
	kvalo@codeaurora.org, ath9k-devel@qca.qualcomm.com
  Cc: netdev@vger.kernel.org
In-Reply-To: <54CA10DB.3070903@rempel-privat.de>

Sorry, the patch ruined by the mail editor, I will send the patch with another mail account witch support smtp. 

I use tasklet_hrtimer_start for delayed submit, and the hrtimer callback function is excuted in the lower priority than normal tasklet. (TASKLET_SOFTIRQ 5, HRTIMER_SOFTIRQ 7 )

The hrtimer mode has been tested several weeks.  So I select this way currently.


-----邮件原件-----
发件人: Oleksij Rempel [mailto:linux@rempel-privat.de] 
发送时间: 2015年1月29日 18:52
收件人: 郑玉伟; linux-kernel@vger.kernel.org; ath9k-devel@venema.h4ckr.net; linux-wireless@vger.kernel.org; kvalo@codeaurora.org; ath9k-devel@qca.qualcomm.com
抄送: netdev@vger.kernel.org
主题: Re: [ath9k-devel] [PATCH] Repair soft lockup with monitor mode of ath9k_htc card

Am 29.01.2015 um 05:09 schrieb zhengyuwei@360.cn:
> From: Yuwei Zheng <zhengyuwei@360.cn>
> 
> In the environment with heavy wifi traffic, set the ar9271 into 
> monitor mode, will trigger a deadloop panic.
> 
> The ath9k_hif_usb_rx_cb function excute on  the interrupt context, and 
> ath9k_rx_tasklet excute on the soft irq context. In other words, the 
> ath9k_hif_usb_rx_cb have more chance to excute than ath9k_rx_tasklet.  
> So in the worst condition,  the rx.rxbuf receive list is always full, and the do {}while(true) loop will not be break. The kernel get a soft lockup panic.
>   
> [59011.007210] BUG: soft lockup - CPU#0 stuck for 23s! 
> [kworker/0:0:30609]
> [59011.030560] BUG: scheduling while atomic: 
> kworker/0:0/30609/0x40010100 [59013.804486] BUG: scheduling while 
> atomic: kworker/0:0/30609/0x40010100 [59013.858522] Kernel panic - not 
> syncing: softlockup: hung tasks
> 
> [59014.038891] Exception stack(0xdf4bbc38 to 0xdf4bbc80)
> [59014.046834] bc20:                                                       de57b950 60000113
> [59014.059579] bc40: 00000000 bb32bb32 60000113 de57b948 de57b500 
> dc7bb440 df4bbcd0 00000000 [59014.072337] bc60: de57b950 60000113 
> df4bbcd0 df4bbc80 c04c259d c04c25a0 60000133 ffffffff [59014.085233] 
> [<c04c28db>] (__irq_svc+0x3b/0x5c) from [<c04c25a0>] 
> (_raw_spin_unlock_irqrestore+0xc/0x10)
> [59014.100437] [<c04c25a0>] (_raw_spin_unlock_irqrestore+0xc/0x10) 
> from [<bf9c2089>] (ath9k_rx_tasklet+0x290/0x490 [ath9k_htc]) 
> [59014.118267] [<bf9c2089>] (ath9k_rx_tasklet+0x290/0x490 [ath9k_htc]) 
> from [<c0036d23>] (tasklet_action+0x3b/0x98) [59014.134132] 
> [<c0036d23>] (tasklet_action+0x3b/0x98) from [<c0036709>] 
> (__do_softirq+0x99/0x16c) [59014.147784] [<c0036709>] 
> (__do_softirq+0x99/0x16c) from [<c00369f7>] (irq_exit+0x5b/0x5c) 
> [59014.160653] [<c00369f7>] (irq_exit+0x5b/0x5c) from [<c000cfc3>] 
> (handle_IRQ+0x37/0x78) [59014.173124] [<c000cfc3>] 
> (handle_IRQ+0x37/0x78) from [<c00085df>] 
> (omap3_intc_handle_irq+0x5f/0x68) [59014.187225] [<c00085df>] 
> (omap3_intc_handle_irq+0x5f/0x68) from 
> [<c04c28db>](__irq_svc+0x3b/0x5c)
> 
> This bug can be see with low performance board, such as uniprocessor beagle bone board.
> Signed-off-by: Yuwei Zheng <zhengyuwei@360.cn>
> 
> ---
>  drivers/net/wireless/ath/ath9k/hif_usb.c       | 53 ++++++++++++++++++++++----
>  drivers/net/wireless/ath/ath9k/hif_usb.h       |  5 +++
>  drivers/net/wireless/ath/ath9k/htc.h           | 13 +++++++
>  drivers/net/wireless/ath/ath9k/htc_drv_debug.c | 49 
> ++++++++++++++++++++++++  
> drivers/net/wireless/ath/ath9k/htc_drv_txrx.c  | 26 +++++++++++++
>  5 files changed, 139 insertions(+), 7 deletions(-)

First of all, thank you for you work! :D

Please run ./scripts/checkpatch.pl yourpatch_path i get:
total: 139 errors, 12 warnings, 2 checks, 231 lines checked

You use tasklet_hrtimer_start. So far i know, there is no this kind of hrtimer which is actually hidden behind this word on this SoC.
Especially if requested value is any way in 1 millisecond range you probably can and should use normal priority tasklet. (correct me if i'm
wrong)

> diff --git a/drivers/net/wireless/ath/ath9k/hif_usb.c 
> b/drivers/net/wireless/ath/ath9k/hif_usb.c
> index 8e7153b..febea5e 100644
> --- a/drivers/net/wireless/ath/ath9k/hif_usb.c
> +++ b/drivers/net/wireless/ath/ath9k/hif_usb.c
> @@ -658,7 +658,6 @@ static void ath9k_hif_usb_rx_cb(struct urb *urb)
>  	default:
>  		goto resubmit;
>  	}
> -
>  	if (likely(urb->actual_length != 0)) {
>  		skb_put(skb, urb->actual_length);
>  		ath9k_hif_usb_rx_stream(hif_dev, skb); @@ -667,12 +666,18 @@ static 
> void ath9k_hif_usb_rx_cb(struct urb *urb)
>  resubmit:
>  	skb_reset_tail_pointer(skb);
>  	skb_trim(skb, 0);
> -
> -	usb_anchor_urb(urb, &hif_dev->rx_submitted);
> -	ret = usb_submit_urb(urb, GFP_ATOMIC);
> -	if (ret) {
> -		usb_unanchor_urb(urb);
> -		goto free;
> +	if (atomic_read(&hif_dev->rx_urb_submit_delay) > 0) {
> +		usb_anchor_urb(urb, &hif_dev->rx_delayed_submitted);
> +		ret = tasklet_hrtimer_start(&hif_dev->rx_submit_timer,
> +					    ktime_set(0, atomic_read(&hif_dev->rx_urb_submit_delay)*1000),
> +					    HRTIMER_MODE_REL);
> +	} else {
> +		usb_anchor_urb(urb, &hif_dev->rx_submitted);
> +		ret = usb_submit_urb(urb, GFP_ATOMIC);
> +		if (ret) {
> +			usb_unanchor_urb(urb);
> +			goto free;
> +		}
>  	}
>  
>  	return;
> @@ -818,9 +823,37 @@ err:
>  	return -ENOMEM;
>  }
>  
> +static enum hrtimer_restart rx_urb_submit_timer_handler(struct 
> +hrtimer *me) {
> +	struct tasklet_hrtimer *thr = container_of(me, struct tasklet_hrtimer, timer);
> +	struct  hif_device_usb *hif_dev = container_of(thr, struct hif_device_usb, rx_submit_timer);
> +	struct urb *urb = NULL;
> +	struct sk_buff *skb = NULL;
> +	int ret;
> +
> +	while (true) {
> +		urb = usb_get_from_anchor(&hif_dev->rx_delayed_submitted);
> +		if (urb != NULL) {
> +			skb = (struct sk_buff *)urb->context;
> +			ret = usb_submit_urb(urb, GFP_ATOMIC);
> +			if (ret != -EBUSY) {
> +				usb_unanchor_urb(urb);
> +				dev_kfree_skb_any(skb);
> +				urb->context = NULL;
> +			}
> +		} else {
> +			break;
> +		}
> +	}
> +
> +	return HRTIMER_NORESTART;
> +}
> +
>  static void ath9k_hif_usb_dealloc_rx_urbs(struct hif_device_usb 
> *hif_dev)  {
>  	usb_kill_anchored_urbs(&hif_dev->rx_submitted);
> +	usb_kill_anchored_urbs(&hif_dev->rx_delayed_submitted);
> +	tasklet_hrtimer_cancel(&hif_dev->rx_submit_timer);
>  }
>  
>  static int ath9k_hif_usb_alloc_rx_urbs(struct hif_device_usb 
> *hif_dev) @@ -830,6 +863,8 @@ static int ath9k_hif_usb_alloc_rx_urbs(struct hif_device_usb *hif_dev)
>  	int i, ret;
>  
>  	init_usb_anchor(&hif_dev->rx_submitted);
> +	init_usb_anchor(&hif_dev->rx_delayed_submitted);
> +
>  	spin_lock_init(&hif_dev->rx_lock);
>  
>  	for (i = 0; i < MAX_RX_URB_NUM; i++) { @@ -871,6 +906,10 @@ static 
> int ath9k_hif_usb_alloc_rx_urbs(struct hif_device_usb *hif_dev)
>  		usb_free_urb(urb);
>  	}
>  
> +	/* add for flow control*/
> +	atomic_set(&hif_dev->rx_urb_submit_delay, 0);
> +	tasklet_hrtimer_init(&hif_dev->rx_submit_timer, 
> +rx_urb_submit_timer_handler, CLOCK_MONOTONIC, HRTIMER_MODE_REL);
> +
>  	return 0;
>  
>  err_submit:
> diff --git a/drivers/net/wireless/ath/ath9k/hif_usb.h 
> b/drivers/net/wireless/ath/ath9k/hif_usb.h
> index 51496e7..56d6be8 100644
> --- a/drivers/net/wireless/ath/ath9k/hif_usb.h
> +++ b/drivers/net/wireless/ath/ath9k/hif_usb.h
> @@ -98,9 +98,14 @@ struct hif_device_usb {
>  	struct hif_usb_tx tx;
>  	struct usb_anchor regout_submitted;
>  	struct usb_anchor rx_submitted;
> +	struct usb_anchor rx_delayed_submitted; /* delayed submit anchor */
>  	struct usb_anchor reg_in_submitted;
>  	struct usb_anchor mgmt_submitted;
>  	struct sk_buff *remain_skb;
> +
> +	struct tasklet_hrtimer  rx_submit_timer;/* delayed submit hrtimer */
> +	atomic_t  rx_urb_submit_delay; /*us*/
> +
>  	const char *fw_name;
>  	int rx_remain_len;
>  	int rx_pkt_len;
> diff --git a/drivers/net/wireless/ath/ath9k/htc.h 
> b/drivers/net/wireless/ath/ath9k/htc.h
> index 9dde265..453d0a8 100644
> --- a/drivers/net/wireless/ath/ath9k/htc.h
> +++ b/drivers/net/wireless/ath/ath9k/htc.h
> @@ -331,6 +331,10 @@ static inline struct ath9k_htc_tx_ctl 
> *HTC_SKB_CB(struct sk_buff *skb)
>  
>  #define TX_QSTAT_INC(q) (priv->debug.tx_stats.queue_stats[q]++)
>  
> +#define TASKLETRX_STAT_INC(c) 
> +(hif_dev->htc_handle->drv_priv->debug.taskletrx_stats.c++)
> +#define TASKLETRX_STAT_ADD(c, a) 
> +(hif_dev->htc_handle->drv_priv->debug.taskletrx_stats.c += a) #define 
> +TASKLETRX_STAT_SET(c, a) 
> +(hif_dev->htc_handle->drv_priv->debug.taskletrx_stats.c = a)
> +
>  void ath9k_htc_err_stat_rx(struct ath9k_htc_priv *priv,
>  			   struct ath_rx_status *rs);
>  
> @@ -352,11 +356,20 @@ struct ath_skbrx_stats {
>  	u32 skb_dropped;
>  };
>  
> +struct ath_taskletrx_stats {
> +	u32 taskletrx_looptimes;
> +	u32 taskletrx_highwater;
> +	u32 taskletrx_lowwater;
> +	u32 taskletrx_watermark_triggered;
> +	u32 taskletrx_urb_submit_delay;
> +};
> +
>  struct ath9k_debug {
>  	struct dentry *debugfs_phy;
>  	struct ath_tx_stats tx_stats;
>  	struct ath_rx_stats rx_stats;
>  	struct ath_skbrx_stats skbrx_stats;
> +	struct ath_taskletrx_stats taskletrx_stats;
>  };
>  
>  void ath9k_htc_get_et_strings(struct ieee80211_hw *hw, diff --git 
> a/drivers/net/wireless/ath/ath9k/htc_drv_debug.c 
> b/drivers/net/wireless/ath/ath9k/htc_drv_debug.c
> index 8cef1ed..7c8322e 100644
> --- a/drivers/net/wireless/ath/ath9k/htc_drv_debug.c
> +++ b/drivers/net/wireless/ath/ath9k/htc_drv_debug.c
> @@ -286,6 +286,51 @@ static const struct file_operations fops_skb_rx = {
>  	.llseek = default_llseek,
>  };
>  
> +static ssize_t read_file_tasklet_rx(struct file *file, char __user *user_buf,
> +				    size_t count, loff_t *ppos)
> +{
> +	struct ath9k_htc_priv *priv = file->private_data;
> +	char *buf;
> +	unsigned int len = 0, size = 1500;
> +	ssize_t retval = 0;
> +
> +	buf = kzalloc(size, GFP_KERNEL);
> +	if (buf == NULL)
> +		return -ENOMEM;
> +
> +	len += scnprintf(buf + len, size - len,
> +			"%20s : %10u\n", "Loop times",
> +			priv->debug.taskletrx_stats.taskletrx_looptimes);
> +	len += scnprintf(buf + len, size - len,
> +			"%20s : %10u\n", "High watermark",
> +			priv->debug.taskletrx_stats.taskletrx_highwater);
> +	len += scnprintf(buf + len, size - len,
> +			"%20s : %10u\n", "Low watermark",
> +			priv->debug.taskletrx_stats.taskletrx_lowwater);
> +
> +	len += scnprintf(buf + len, size - len,
> +			"%20s : %10u\n", "WM triggered",
> +			priv->debug.taskletrx_stats.taskletrx_watermark_triggered);
> +
> +	len += scnprintf(buf + len, size - len,
> +			"%20s : %10u\n", "URB delay",
> +			priv->debug.taskletrx_stats.taskletrx_urb_submit_delay);
> +	if (len > size)
> +		len = size;
> +
> +	retval = simple_read_from_buffer(user_buf, count, ppos, buf, len);
> +	kfree(buf);
> +
> +	return retval;
> +}
> +
> +static const struct file_operations fops_tasklet_rx = {
> +	.read = read_file_tasklet_rx,
> +	.open = simple_open,
> +	.owner = THIS_MODULE,
> +	.llseek = default_llseek,
> +};
> +
>  static ssize_t read_file_slot(struct file *file, char __user *user_buf,
>  			      size_t count, loff_t *ppos)
>  {
> @@ -518,7 +563,11 @@ int ath9k_htc_init_debug(struct ath_hw *ah)
>  	debugfs_create_file("skb_rx", S_IRUSR, priv->debug.debugfs_phy,
>  			    priv, &fops_skb_rx);
>  
> +	debugfs_create_file("tasklet_rx", S_IRUSR, priv->debug.debugfs_phy,
> +			    priv, &fops_tasklet_rx);
> +
>  	ath9k_cmn_debug_recv(priv->debug.debugfs_phy, 
> &priv->debug.rx_stats);
> +
>  	ath9k_cmn_debug_phy_err(priv->debug.debugfs_phy, 
> &priv->debug.rx_stats);
>  
>  	debugfs_create_file("slot", S_IRUSR, priv->debug.debugfs_phy, diff 
> --git a/drivers/net/wireless/ath/ath9k/htc_drv_txrx.c 
> b/drivers/net/wireless/ath/ath9k/htc_drv_txrx.c
> index a0f58e2..f5e6217 100644
> --- a/drivers/net/wireless/ath/ath9k/htc_drv_txrx.c
> +++ b/drivers/net/wireless/ath/ath9k/htc_drv_txrx.c
> @@ -1061,7 +1061,28 @@ void ath9k_rx_tasklet(unsigned long data)
>  	unsigned long flags;
>  	struct ieee80211_hdr *hdr;
>  
> +	/* add for adaptive flow control*/
> +	int looptimes = 0;
> +	int highwatermark = ATH9K_HTC_RXBUF*3/4;
> +	int lowwatermark = ATH9K_HTC_RXBUF/4;
> +	unsigned int delay = 0;
> +
> +	struct htc_target *htc = priv->htc;
> +	struct hif_device_usb *hif_dev = htc->hif_dev;
> +
> +	TASKLETRX_STAT_SET(taskletrx_highwater, highwatermark);
> +	TASKLETRX_STAT_SET(taskletrx_lowwater, lowwatermark);
> +
>  	do {
> +		looptimes++;
> +		TASKLETRX_STAT_SET(taskletrx_looptimes, looptimes);
> +		if (looptimes > highwatermark) {
> +			delay = looptimes*10;
> +			atomic_set(&hif_dev->rx_urb_submit_delay, delay);
> +			TASKLETRX_STAT_INC(taskletrx_watermark_triggered);
> +			TASKLETRX_STAT_SET(taskletrx_urb_submit_delay, delay);
> +		}
> +
>  		spin_lock_irqsave(&priv->rx.rxbuflock, flags);
>  		list_for_each_entry(tmp_buf, &priv->rx.rxbuf, list) {
>  			if (tmp_buf->in_process) {
> @@ -1072,6 +1093,11 @@ void ath9k_rx_tasklet(unsigned long data)
>  
>  		if (rxbuf == NULL) {
>  			spin_unlock_irqrestore(&priv->rx.rxbuflock, flags);
> +			if (looptimes < lowwatermark) {
> +				atomic_set(&hif_dev->rx_urb_submit_delay, 0);
> +				TASKLETRX_STAT_SET(taskletrx_urb_submit_delay, 0);
> +			}
> +
>  			break;
>  		}
>  
> 


--
Regards,
Oleksij


^ permalink raw reply

* Re: [PATCH 9/9] netfilter: nf_tables: add support for dynamic set updates
From: Herbert Xu @ 2015-01-30 10:18 UTC (permalink / raw)
  To: Patrick McHardy
  Cc: tgraf, davem, David.Laight, ying.xue, paulmck, netdev,
	netfilter-devel
In-Reply-To: <C08F666A-DAAF-4C61-A9F3-DBB3D936B4B4@trash.net>

On Fri, Jan 30, 2015 at 10:08:46AM +0000, Patrick McHardy wrote:
> Am 30. Januar 2015 09:28:45 GMT+00:00, schrieb Herbert Xu <herbert@gondor.apana.org.au>:
> >On Fri, Jan 30, 2015 at 07:46:34AM +0000, Patrick McHardy wrote:
> >> Signed-off-by: Patrick McHardy <kaber@trash.net>
> >
> >I presume this can't create any jumps/gotos, right?
> 
> So far not, just data mappings. Not sure yet if there is a valid use case for jumps.

Well if they could create jumps/gotos in softirq context then
the loop verification would get pretty hairy :)

OK so it looks like you really do need a totally lockless walk.
So I'll reshape my iterators to do that.

Cheers,
-- 
Email: Herbert Xu <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

^ permalink raw reply

* Re: [PATCH 9/9] netfilter: nf_tables: add support for dynamic set updates
From: Patrick McHardy @ 2015-01-30 10:08 UTC (permalink / raw)
  To: Herbert Xu
  Cc: tgraf, davem, David.Laight, ying.xue, paulmck, netdev,
	netfilter-devel
In-Reply-To: <20150130092845.GA18725@gondor.apana.org.au>

Am 30. Januar 2015 09:28:45 GMT+00:00, schrieb Herbert Xu <herbert@gondor.apana.org.au>:
>On Fri, Jan 30, 2015 at 07:46:34AM +0000, Patrick McHardy wrote:
>> Signed-off-by: Patrick McHardy <kaber@trash.net>
>
>I presume this can't create any jumps/gotos, right?

So far not, just data mappings. Not sure yet if there is a valid use case for jumps.
>
>Thanks,



^ permalink raw reply

* Re: [PATCH V2 5/6] rtlwifi: btcoexist: Add routines for RTL8812AE kernel socket communications
From: Marcel Holtmann @ 2015-01-30  9:57 UTC (permalink / raw)
  To: Kalle Valo
  Cc: Larry Finger, linux-wireless-u79uwXL29TY76Z2rM5mHXA, Troy Tan,
	netdev-u79uwXL29TY76Z2rM5mHXA,
	linux-bluetooth-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <87h9v8zkuh.fsf-HodKDYzPHsUD5k0oWYwrnHL1okKdlPRT@public.gmane.org>

Hi Kalle,

> I'm adding bluetooth list to the discussion. Full patch is available
> here:
> 
> https://patchwork.kernel.org/patch/5712591/
> 
> Larry Finger <Larry.Finger-tQ5ms3gMjBLk1uMJSBkQmQ@public.gmane.org> writes:
> 
>> From: Troy Tan <troy_tan-kXabqFNEczNtrwSWzY7KCg@public.gmane.org>
>> 
>> This patch adds the routines used to communicate between the RTL8812AE (wifi)
>> device and the RTL8761AU (bluetooth) device that are part of the same chip.
>> Unlike other similar dual-function devices, this chip does not contain special
>> hardware that lets the firmware pass coexistence info from one part to the
>> other. As a result, this driver implements such communication as a kernel
>> socket.
>> 
>> Signed-off-by: Troy Tan <troy_tan-kXabqFNEczNtrwSWzY7KCg@public.gmane.org>
>> Signed-off-by: Larry Finger <Larry.Finger-tQ5ms3gMjBLk1uMJSBkQmQ@public.gmane.org>
>> ---
>> V2 - Add comments explaining the routine that sends a message via the
>> socket.
> 
> The commit log is not still explaining that much about the actual
> functionality, so I investigated on my own:
> 
>> +static u8 rtl_btcoex_create_kernel_socket(struct rtl_priv *rtlpriv,
>> +					  u8 is_invite)
>> +{
>> +	struct bt_coex_info *pcoex_info = &rtlpriv->coex_info;
>> +	s8 kernel_socket_err;
>> +
>> +	BTC_PRINT(BTC_MSG_SOCKET, SOCKET_CRITICAL,
>> +		  "%s CONNECT_PORT %d\n", __func__, CONNECT_PORT);
>> +
>> +	if (!pcoex_info) {
>> +		BTC_PRINT(BTC_MSG_SOCKET, SOCKET_CRITICAL, "coex_info: NULL\n");
>> +		return _FAIL;
>> +	}
>> +
>> +	kernel_socket_err = sock_create(PF_INET, SOCK_DGRAM, 0,
>> +					&pcoex_info->udpsock);
>> +	BTC_PRINT(BTC_MSG_SOCKET, SOCKET_CRITICAL,
>> +		  "binding socket, err = %d\n", kernel_socket_err);
>> +
>> +	if (kernel_socket_err < 0) {
>> +		BTC_PRINT(BTC_MSG_SOCKET, SOCKET_CRITICAL,
>> +			  "Error during creation of socket error:%d\n",
>> +			  kernel_socket_err);
>> +		return _FAIL;
>> +	}
>> +	memset(&pcoex_info->sin, 0, sizeof(pcoex_info->sin));
>> +	pcoex_info->sin.sin_family = AF_INET;
>> +	pcoex_info->sin.sin_port = htons(CONNECT_PORT);
>> +	pcoex_info->sin.sin_addr.s_addr = htonl(INADDR_LOOPBACK);
>> +
>> +	memset(&pcoex_info->bt_addr, 0, sizeof(pcoex_info->bt_addr));
>> +	pcoex_info->bt_addr.sin_family = AF_INET;
>> +	pcoex_info->bt_addr.sin_port = htons(CONNECT_PORT_BT);
>> +	pcoex_info->bt_addr.sin_addr.s_addr = htonl(INADDR_LOOPBACK);
>> +
>> +	pcoex_info->sk_store = NULL;
>> +
>> +	kernel_socket_err =
>> +	   pcoex_info->udpsock->ops->bind(pcoex_info->udpsock,
>> +					  (struct sockaddr *)&pcoex_info->sin,
>> +					  sizeof(pcoex_info->sin));
>> +	if (kernel_socket_err == 0) {
>> +		BTC_PRINT(BTC_MSG_SOCKET, SOCKET_CRITICAL,
>> +			  "binding socket success\n");
>> +		pcoex_info->udpsock->sk->sk_data_ready =
>> +			rtl_btcoex_recvmsg_int;
>> +		pcoex_info->sock_open |=  KERNEL_SOCKET_OK;
>> +		pcoex_info->bt_attend = false;
>> +		BTC_PRINT(BTC_MSG_SOCKET, SOCKET_CRITICAL,
>> +			  "WIFI sending attend_req\n");
>> +		rtl_btcoex_sendmsgbysocket(rtlpriv, attend_req,
>> +					   sizeof(attend_req), true);
>> +		return _SUCCESS;
> 
> So the wireless driver communicates with the bluetooth driver (which is
> not in upstream) via a localhost UDP connection?

I think the first order of business should be to get the Bluetooth driver upstream. Until that has happened this is all kinda pointless discussion.

>> +#define CONNECT_PORT 30000
>> +#define CONNECT_PORT_BT 30001
> 
> And these are the UDP ports used.
> 
>> +struct hci_link_info {
>> +	u16		connect_handle;
>> +	u8		incoming_traffic_mode;
>> +	u8		outgoing_traffic_mode;
>> +	u8		bt_profile;
>> +	u8		bt_corespec;
>> +	s8		bt_RSSI;
>> +	u8		traffic_profile;
>> +	u8		link_role;
>> +};
>> +
>> +#define	MAX_BT_ACL_LINK_NUM		8
>> +
>> +struct hci_ext_config {
>> +	struct hci_link_info	acl_link[MAX_BT_ACL_LINK_NUM];
>> +	u8	bt_operation_code;
>> +	u16	current_connect_handle;
>> +	u8	current_incoming_traffic_mode;
>> +	u8	current_outgoing_traffic_mode;
>> +
>> +	u8	number_of_acl;
>> +	u8	number_of_sco;
>> +	u8	current_bt_status;
>> +	u16	hci_ext_ver;
>> +	bool	enable_wifi_scan_notify;
>> +};
> 
> [...]
> 
>> +enum HCI_STATUS {
>> +	/* Success */
>> +	HCI_STATUS_SUCCESS				= 0x00,
>> +	/* Unknown HCI Command */
>> +	HCI_STATUS_UNKNOW_HCI_CMD			= 0x01,
>> +	/* Unknown Connection Identifier */
>> +	HCI_STATUS_UNKNOW_CONNECT_ID			= 0X02,
>> +	/* Hardware Failure */
>> +	HCI_STATUS_HW_FAIL				= 0X03,
>> +	/* Page Timeout */
>> +	HCI_STATUS_PAGE_TIMEOUT				= 0X04,
>> +	/* Authentication Failure */
>> +	HCI_STATUS_AUTH_FAIL				= 0X05,
>> +	/* PIN or Key Missing */
>> +	HCI_STATUS_PIN_OR_KEY_MISSING			= 0X06,
>> +	/* Memory Capacity Exceeded */
>> +	HCI_STATUS_MEM_CAP_EXCEED			= 0X07,
>> +	/* Connection Timeout */
>> +	HCI_STATUS_CONNECT_TIMEOUT			= 0X08,
>> +	/* Connection Limit Exceeded */
> 
> And here's part of the information exchanged between the drivers.
> 
> This is something which needs to be properly reviewed both in wireless
> and bluetooth lists, a wireless driver cannot just invent these on their
> own. And using UDP sockets for this is in my opinion just horrible.
> 
> I know there's a general need for something similar like this, but it
> needs to properly discussed and designed.

This is just insane. Clear NAK.

Two kernel modules will not use UDP ports over the loopback interface to communicate with each other.

Regards

Marcel

--
To unsubscribe from this list: send the line "unsubscribe linux-wireless" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox