Netdev List
 help / color / mirror / Atom feed
* Re: BQL support in gianfar causes network hiccup
From: Per Dalén @ 2013-09-02 13:21 UTC (permalink / raw)
  To: netdev

 > On Mo, 2013-04-29 at 15:20 +0200, Tino Keitel wrote:
 > > On Mo, 2013-04-29 at 15:14 +0200, Claudiu Manoil wrote:
 > >
 > > > The proposed fix (passes this test):
 > > > http://patchwork.ozlabs.org/patch/240365/
 > > >
 > > > Does it work for you?

Hi,

This patch do not work for me. I have the same problems that Tino on my 
card. My card is a Freescale P2020 and I use "iperf -c 10.10.51.36 -n 
100M -P 50" to trigger the error.
The network interface is set to 100M half duplex.
I have tested this using 3.4.35 and 3.4.60 kernel.

NETDEV WATCHDOG: eth0 (fsl-gianfar): transmit queue 0 timed out
------------[ cut here ]------------
WARNING: at net/sched/sch_generic.c:256
Modules linked in:
NIP: c039bb94 LR: c039bb94 CTR: c02ee500
REGS: dffe7e60 TRAP: 0700   Not tainted  (3.4.60+)
MSR: 00029000 <CE,EE,ME>  CR: 40000084  XER: 20000000
TASK = df843200[0] 'swapper/1' THREAD: df864000 CPU: 1
GPR00: c039bb94 dffe7f10 df843200 00000046 00021000 ffffffff c02eecf0 
00029000
GPR08: 00000001 00008000 00463000 00003fff 80000028 00000000 00000000 
00000004
GPR16: 00000001 df858e14 c051f1f4 00000001 df858c14 df858a14 df858814 
ffffffff
GPR24: 00000001 df813234 00000004 df8f9a00 c05e0000 c05e0000 df813000 
00000000
NIP [c039bb94] dev_watchdog+0x2d4/0x2e4
LR [c039bb94] dev_watchdog+0x2d4/0x2e4
Call Trace:
[dffe7f10] [c039bb94] dev_watchdog+0x2d4/0x2e4 (unreliable)
[dffe7f40] [c003e944] run_timer_softirq+0x11c/0x20c
[dffe7fa0] [c00377dc] __do_softirq+0xe4/0x16c
[dffe7ff0] [c000c408] call_do_softirq+0x14/0x24
[df865e80] [c00043c4] do_softirq+0xb4/0xe4
[df865ea0] [c0037b4c] irq_exit+0xb0/0xcc
[df865eb0] [c0008958] timer_interrupt+0x188/0x1a4
[df865ee0] [c000db68] ret_from_except+0x0/0x18
--- Exception: 901 at cpu_idle+0x90/0xf4
     LR = cpu_idle+0x90/0xf4
[df865fa0] [c0007b30] cpu_idle+0x5c/0xf4 (unreliable)
[df865fc0] [c049d68c] start_secondary+0x2b0/0x2b4
[df865ff0] [c0001cf8] __secondary_start+0x30/0x84
Instruction dump:
4e800421 80fe0244 4bffff40 7fc3f378 4bfeab6d 7fc4f378 7c651b78 3c60c055
7fe6fb78 38633108 4cc63182 480fc889 <0fe00000> 39200001 993cea1a 4bffffb4
---[ end trace 4be011773b23dae3 ]---


When I revert "gianfar: Add support for byte queue limits.", commit 
d8a0f1b0af67679bba886784de10d8c21acc4e0e, the network test work fine 
without any "hiccup".
If the network interface is set to 1000M full duplex there's also no 
"hiccup".

 >
 > Hi,
 >
 > I tested the patch over the weekend and it looks fine. I then re-checked
 > without the patch and got the bug immediately. So the patch seems to
 > work fine.
 >

Thanks,
Per Dalén

^ permalink raw reply

* Re: 3.11-rc7:BUG: soft lockup
From: Baoquan He @ 2013-09-02 12:09 UTC (permalink / raw)
  To: Ding Tianhong
  Cc: Hillf Danton, Cong Wang, LKML, Linux Kernel Network Developers
In-Reply-To: <5224548C.8000603@huawei.com>

On 09/02/2013 05:04 PM, Ding Tianhong wrote:
> On 2013/9/2 16:24, Baoquan He wrote:
>> Hi Tianhong,
>>
>> I applied your patch and execute below cmd. 
>> Then keyboard inputting problems happened, 
>> I can't enter user/password correctly, then reboot again, it's OK now.
> Thanks for your work, I don't understand that the keyboard problem is the patch fault or your system problem, 
> if it is not happen again, I think the problem is fixed.

Yeah,  I will wait and see if it will happen again. Up to now, it's OK.

Thank you all for help.

Baoquan
Thanks

>
>> The original bug still can't be reproduced.
>>
>> make SUBDIRS=net/bridge/netfilter/
>> cp /net/bridge/netfilter/ebtables.ko
>> /lib/modules/3.11.0-rc7+/kernel/net/bridge/netfilter/ebtables.ko
>> depmod
>> modprobe ebtables.ko
>> reboot
>>
>> Baoquan
>> Thanks
>>
>>> please try this patch and give me the result, thanks.
>>>
>>> Return the correct value if mutex_lock_interruptible() failed, avoid
>>> confusion with that the modules is not exist, and deal with the return
>>> value in right way.
>>>
>>> if mutex_lock_interrupt() failed, sh
>>>
>>> Signed-off-by: root <root@linux-yocto.site>
>>> ---
>>>  net/bridge/netfilter/ebtables.c | 17 +++++++----------
>>>  1 file changed, 7 insertions(+), 10 deletions(-)
>>>
>>> diff --git a/net/bridge/netfilter/ebtables.c b/net/bridge/netfilter/ebtables.c
>>> index ac78024..e7fe9f8 100644
>>> --- a/net/bridge/netfilter/ebtables.c
>>> +++ b/net/bridge/netfilter/ebtables.c
>>> @@ -322,17 +322,14 @@ static inline void *
>>>  find_inlist_lock_noload(struct list_head *head, const char *name, int *error,
>>>     struct mutex *mutex)
>>>  {
>>> -	struct {
>>> -		struct list_head list;
>>> -		char name[EBT_FUNCTION_MAXNAMELEN];
>>> -	} *e;
>>> +	struct ebt_table *e;
>>>  
>>>  	*error = mutex_lock_interruptible(mutex);
>>>  	if (*error != 0)
>>> -		return NULL;
>>> +		return ERR_PTR(-EINTR);
>>>  
>>>  	list_for_each_entry(e, head, list) {
>>> -		if (strcmp(e->name, name) == 0)
>>> +		 if (strcmp(e->name, name) == 0 && try_module_get(e->me))
>>>  			return e;
>>>  	}
>>>  	*error = -ENOENT;
>>> @@ -1005,7 +1002,7 @@ static int do_replace_finish(struct net *net, struct ebt_replace *repl,
>>>  		goto free_counterstmp;
>>>  
>>>  	t = find_table_lock(net, repl->name, &ret, &ebt_mutex);
>>> -	if (!t) {
>>> +	if (IS_ERR_OR_NULL(t)) {
>>>  		ret = -ENOENT;
>>>  		goto free_iterate;
>>>  	}
>>> @@ -1284,7 +1281,7 @@ static int do_update_counters(struct net *net, const char *name,
>>>  		return -ENOMEM;
>>>  
>>>  	t = find_table_lock(net, name, &ret, &ebt_mutex);
>>> -	if (!t)
>>> +	if (IS_ERR_OR_NULL(t))
>>>  		goto free_tmp;
>>>  
>>>  	if (num_counters != t->private->nentries) {
>>> @@ -1504,7 +1501,7 @@ static int do_ebt_get_ctl(struct sock *sk, int cmd, void __user *user, int *len)
>>>  		return -EFAULT;
>>>  
>>>  	t = find_table_lock(net, tmp.name, &ret, &ebt_mutex);
>>> -	if (!t)
>>> +	if (IS_ERR_OR_NULL(t))
>>>  		return ret;
>>>  
>>>  	switch(cmd) {
>>> @@ -2319,7 +2316,7 @@ static int compat_do_ebt_get_ctl(struct sock *sk, int cmd,
>>>  		return -EFAULT;
>>>  
>>>  	t = find_table_lock(net, tmp.name, &ret, &ebt_mutex);
>>> -	if (!t)
>>> +	if (IS_ERR_OR_NULL(t))
>>>  		return ret;
>>>  
>>>  	xt_compat_lock(NFPROTO_BRIDGE);
>>
>> .
>>
>

^ permalink raw reply

* [PATCH net-next 3/5] bonding: simplify bond_3ad_update_lacp_rate and use RTNL for sync
From: Nikolay Aleksandrov @ 2013-09-02 11:51 UTC (permalink / raw)
  To: netdev; +Cc: andy, fubar, davem
In-Reply-To: <1378122702-28943-1-git-send-email-nikolay@redhat.com>

We can drop the use of bond->lock for mutual exclusion in
bond_3ad_update_lacp_rate and use RTNL in the sysfs store function
instead. This way we'll prevent races with mode change and interface
up/down as well as simplify update_lacp_rate by removing the check for
port->slave because it'll always be initialized (done while enslaving
with RTNL). This change will also help in the future removal of reader
bond->lock from bond_enslave.

Signed-off-by: Nikolay Aleksandrov <nikolay@redhat.com>
---
 drivers/net/bonding/bond_3ad.c   | 8 +-------
 drivers/net/bonding/bond_sysfs.c | 7 ++++++-
 2 files changed, 7 insertions(+), 8 deletions(-)

diff --git a/drivers/net/bonding/bond_3ad.c b/drivers/net/bonding/bond_3ad.c
index 9010265..0d8f427 100644
--- a/drivers/net/bonding/bond_3ad.c
+++ b/drivers/net/bonding/bond_3ad.c
@@ -2514,17 +2514,13 @@ int bond_3ad_lacpdu_recv(const struct sk_buff *skb, struct bonding *bond,
  */
 void bond_3ad_update_lacp_rate(struct bonding *bond)
 {
-	struct slave *slave;
 	struct port *port = NULL;
+	struct slave *slave;
 	int lacp_fast;
 
-	write_lock_bh(&bond->lock);
 	lacp_fast = bond->params.lacp_fast;
-
 	bond_for_each_slave(bond, slave) {
 		port = &(SLAVE_AD_INFO(slave).port);
-		if (port->slave == NULL)
-			continue;
 		__get_state_machine_lock(port);
 		if (lacp_fast)
 			port->actor_oper_port_state |= AD_STATE_LACP_TIMEOUT;
@@ -2532,6 +2528,4 @@ void bond_3ad_update_lacp_rate(struct bonding *bond)
 			port->actor_oper_port_state &= ~AD_STATE_LACP_TIMEOUT;
 		__release_state_machine_lock(port);
 	}
-
-	write_unlock_bh(&bond->lock);
 }
diff --git a/drivers/net/bonding/bond_sysfs.c b/drivers/net/bonding/bond_sysfs.c
index 0f539de..ce46776 100644
--- a/drivers/net/bonding/bond_sysfs.c
+++ b/drivers/net/bonding/bond_sysfs.c
@@ -852,8 +852,11 @@ static ssize_t bonding_store_lacp(struct device *d,
 				  struct device_attribute *attr,
 				  const char *buf, size_t count)
 {
-	int new_value, ret = count;
 	struct bonding *bond = to_bond(d);
+	int new_value, ret = count;
+
+	if (!rtnl_trylock())
+		return restart_syscall();
 
 	if (bond->dev->flags & IFF_UP) {
 		pr_err("%s: Unable to update LACP rate because interface is up.\n",
@@ -883,6 +886,8 @@ static ssize_t bonding_store_lacp(struct device *d,
 		ret = -EINVAL;
 	}
 out:
+	rtnl_unlock();
+
 	return ret;
 }
 static DEVICE_ATTR(lacp_rate, S_IRUGO | S_IWUSR,
-- 
1.8.1.4

^ permalink raw reply related

* [PATCH net-next 5/5] bonding: drop read_lock in bond_compute_features
From: Nikolay Aleksandrov @ 2013-09-02 11:51 UTC (permalink / raw)
  To: netdev; +Cc: andy, fubar, davem
In-Reply-To: <1378122702-28943-1-git-send-email-nikolay@redhat.com>

bond_compute_features is always called with RTNL held, so we can safely
drop the read bond->lock.

Signed-off-by: Nikolay Aleksandrov <nikolay@redhat.com>
---
 drivers/net/bonding/bond_main.c | 10 +++-------
 1 file changed, 3 insertions(+), 7 deletions(-)

diff --git a/drivers/net/bonding/bond_main.c b/drivers/net/bonding/bond_main.c
index c5ebdc5..39e5b1c 100644
--- a/drivers/net/bonding/bond_main.c
+++ b/drivers/net/bonding/bond_main.c
@@ -1116,15 +1116,13 @@ static netdev_features_t bond_fix_features(struct net_device *dev,
 
 static void bond_compute_features(struct bonding *bond)
 {
-	struct slave *slave;
-	struct net_device *bond_dev = bond->dev;
+	unsigned int flags, dst_release_flag = IFF_XMIT_DST_RELEASE;
 	netdev_features_t vlan_features = BOND_VLAN_FEATURES;
 	unsigned short max_hard_header_len = ETH_HLEN;
 	unsigned int gso_max_size = GSO_MAX_SIZE;
+	struct net_device *bond_dev = bond->dev;
 	u16 gso_max_segs = GSO_MAX_SEGS;
-	unsigned int flags, dst_release_flag = IFF_XMIT_DST_RELEASE;
-
-	read_lock(&bond->lock);
+	struct slave *slave;
 
 	if (list_empty(&bond->slave_list))
 		goto done;
@@ -1150,8 +1148,6 @@ done:
 	flags = bond_dev->priv_flags & ~IFF_XMIT_DST_RELEASE;
 	bond_dev->priv_flags = flags | dst_release_flag;
 
-	read_unlock(&bond->lock);
-
 	netdev_change_features(bond_dev);
 }
 
-- 
1.8.1.4

^ permalink raw reply related

* [PATCH net-next 4/5] bonding: drop read_lock in bond_fix_features
From: Nikolay Aleksandrov @ 2013-09-02 11:51 UTC (permalink / raw)
  To: netdev; +Cc: andy, fubar, davem
In-Reply-To: <1378122702-28943-1-git-send-email-nikolay@redhat.com>

We're protected by RTNL so nothing can happen and we can safely drop the
read bond->lock.

Signed-off-by: Nikolay Aleksandrov <nikolay@redhat.com>
---
 drivers/net/bonding/bond_main.c | 10 +++-------
 1 file changed, 3 insertions(+), 7 deletions(-)

diff --git a/drivers/net/bonding/bond_main.c b/drivers/net/bonding/bond_main.c
index 486e041..c5ebdc5 100644
--- a/drivers/net/bonding/bond_main.c
+++ b/drivers/net/bonding/bond_main.c
@@ -1084,18 +1084,16 @@ static void bond_netpoll_cleanup(struct net_device *bond_dev)
 /*---------------------------------- IOCTL ----------------------------------*/
 
 static netdev_features_t bond_fix_features(struct net_device *dev,
-	netdev_features_t features)
+					   netdev_features_t features)
 {
-	struct slave *slave;
 	struct bonding *bond = netdev_priv(dev);
 	netdev_features_t mask;
-
-	read_lock(&bond->lock);
+	struct slave *slave;
 
 	if (list_empty(&bond->slave_list)) {
 		/* Disable adding VLANs to empty bond. But why? --mq */
 		features |= NETIF_F_VLAN_CHALLENGED;
-		goto out;
+		return features;
 	}
 
 	mask = features;
@@ -1109,8 +1107,6 @@ static netdev_features_t bond_fix_features(struct net_device *dev,
 	}
 	features = netdev_add_tso_features(features, mask);
 
-out:
-	read_unlock(&bond->lock);
 	return features;
 }
 
-- 
1.8.1.4

^ permalink raw reply related

* [PATCH net-next 2/5] bonding: trivial: remove outdated comment and braces
From: Nikolay Aleksandrov @ 2013-09-02 11:51 UTC (permalink / raw)
  To: netdev; +Cc: andy, fubar, davem
In-Reply-To: <1378122702-28943-1-git-send-email-nikolay@redhat.com>

We don't have to release all slaves when closing the bond dev, so remove
the outdated comment and the braces around the left single statement.

Signed-off-by: Nikolay Aleksandrov <nikolay@redhat.com>
---
 drivers/net/bonding/bond_main.c | 6 +-----
 1 file changed, 1 insertion(+), 5 deletions(-)

diff --git a/drivers/net/bonding/bond_main.c b/drivers/net/bonding/bond_main.c
index 05e638b..486e041 100644
--- a/drivers/net/bonding/bond_main.c
+++ b/drivers/net/bonding/bond_main.c
@@ -3210,12 +3210,8 @@ static int bond_close(struct net_device *bond_dev)
 
 	bond_work_cancel_all(bond);
 	bond->send_peer_notif = 0;
-	if (bond_is_lb(bond)) {
-		/* Must be called only after all
-		 * slaves have been released
-		 */
+	if (bond_is_lb(bond))
 		bond_alb_deinitialize(bond);
-	}
 	bond->recv_probe = NULL;
 
 	return 0;
-- 
1.8.1.4

^ permalink raw reply related

* [PATCH net-next 1/5] bonding: simplify and fix peer notification
From: Nikolay Aleksandrov @ 2013-09-02 11:51 UTC (permalink / raw)
  To: netdev; +Cc: andy, fubar, davem
In-Reply-To: <1378122702-28943-1-git-send-email-nikolay@redhat.com>

This patch aims to remove a use of the bond->lock for mutual exclusion
which will later allow easier migration to RCU of the users of this
functionality. We use RTNL as a synchronizing mechanism since it's
always held when send_peer_notif is set, and when it is decremented from
the notifier function. We can also drop some locking, and fix the
leakage of the send_peer_notif counter.

Signed-off-by: Nikolay Aleksandrov <nikolay@redhat.com>
---
 drivers/net/bonding/bond_main.c | 22 +++++++---------------
 1 file changed, 7 insertions(+), 15 deletions(-)

diff --git a/drivers/net/bonding/bond_main.c b/drivers/net/bonding/bond_main.c
index c50679f..05e638b 100644
--- a/drivers/net/bonding/bond_main.c
+++ b/drivers/net/bonding/bond_main.c
@@ -828,7 +828,6 @@ static bool bond_should_notify_peers(struct bonding *bond)
 	    test_bit(__LINK_STATE_LINKWATCH_PENDING, &slave->dev->state))
 		return false;
 
-	bond->send_peer_notif--;
 	return true;
 }
 
@@ -2259,12 +2258,8 @@ re_arm:
 	read_unlock(&bond->lock);
 
 	if (should_notify_peers) {
-		if (!rtnl_trylock()) {
-			read_lock(&bond->lock);
-			bond->send_peer_notif++;
-			read_unlock(&bond->lock);
+		if (!rtnl_trylock())
 			return;
-		}
 		call_netdevice_notifiers(NETDEV_NOTIFY_PEERS, bond->dev);
 		rtnl_unlock();
 	}
@@ -2876,12 +2871,8 @@ re_arm:
 	read_unlock(&bond->lock);
 
 	if (should_notify_peers) {
-		if (!rtnl_trylock()) {
-			read_lock(&bond->lock);
-			bond->send_peer_notif++;
-			read_unlock(&bond->lock);
+		if (!rtnl_trylock())
 			return;
-		}
 		call_netdevice_notifiers(NETDEV_NOTIFY_PEERS, bond->dev);
 		rtnl_unlock();
 	}
@@ -2916,6 +2907,10 @@ static int bond_master_netdev_event(unsigned long event,
 	case NETDEV_REGISTER:
 		bond_create_proc_entry(event_bond);
 		break;
+	case NETDEV_NOTIFY_PEERS:
+		if (event_bond->send_peer_notif)
+			event_bond->send_peer_notif--;
+		break;
 	default:
 		break;
 	}
@@ -3213,11 +3208,8 @@ static int bond_close(struct net_device *bond_dev)
 {
 	struct bonding *bond = netdev_priv(bond_dev);
 
-	write_lock_bh(&bond->lock);
-	bond->send_peer_notif = 0;
-	write_unlock_bh(&bond->lock);
-
 	bond_work_cancel_all(bond);
+	bond->send_peer_notif = 0;
 	if (bond_is_lb(bond)) {
 		/* Must be called only after all
 		 * slaves have been released
-- 
1.8.1.4

^ permalink raw reply related

* [PATCH net-next 0/5] bonding: locking simplifications and cleanup
From: Nikolay Aleksandrov @ 2013-09-02 11:51 UTC (permalink / raw)
  To: netdev; +Cc: andy, fubar, davem

Hello,
This small patchset aims to remove some use cases of bond->lock for mutual
exclusion which will help in the RCUfication of the function users. It also
does some small style cleanups and fixes.

Patch 01 - Drops the use of bond->lock as mutual exclusion for peer
notification and relies on RTNL being held when send_peer_notif is modified
Patch 02 - trivial outdated comment removal
Patch 03 - Drops the use of bond->lock as mutual exclusion for lacp rate
update and relies on RTNL, also fixes possible races with mode change
Patch 04 - Drops read_lock in bond_fix_features because RTNL is held
Patch 05 - Drops read_lock in bond_compute_features because RTNL is held
whenever it's called

Best regards,
 Nikolay Aleksandrov

Nikolay Aleksandrov (5):
  bonding: simplify and fix peer notification
  bonding: trivial: remove outdated comment and braces
  bonding: simplify bond_3ad_update_lacp_rate and use RTNL for sync
  bonding: drop read_lock in bond_fix_features
  bonding: drop read_lock in bond_compute_features

 drivers/net/bonding/bond_3ad.c   |  8 +------
 drivers/net/bonding/bond_main.c  | 48 ++++++++++++----------------------------
 drivers/net/bonding/bond_sysfs.c |  7 +++++-
 3 files changed, 21 insertions(+), 42 deletions(-)

-- 
1.8.1.4

^ permalink raw reply

* [PATCH] net: fec: fix the error to get the previous BD entry
From: Fugang Duan @ 2013-09-02 11:35 UTC (permalink / raw)
  To: b20596, davem; +Cc: netdev, bhutchings, stephen

Bug: error to get the previous BD entry. When the current BD
is the first BD, the previous BD entry must be the last BD,
not "bdp - 1" in current logic.

V2:
  Add tx_ring_size and rx_ring_size to struct fec_enet_private.
  Replace api fec_enet_get_nextdesc() with next_bd().
  Replace api fec_enet_get_prevdesc() with pre_bd().

  Move all ring size check logic to next_bd() and pre_bd, which
  simplifies the code redundancy.

V1:
  Add BD ring size check to get the previous BD entry in correctly.

Signed-off-by: Fugang Duan  <B38611@freescale.com>
---
 drivers/net/ethernet/freescale/fec.h      |    3 +
 drivers/net/ethernet/freescale/fec_main.c |   94 +++++++++++++++-------------
 2 files changed, 53 insertions(+), 44 deletions(-)

diff --git a/drivers/net/ethernet/freescale/fec.h b/drivers/net/ethernet/freescale/fec.h
index ae23600..0120217 100644
--- a/drivers/net/ethernet/freescale/fec.h
+++ b/drivers/net/ethernet/freescale/fec.h
@@ -296,6 +296,9 @@ struct fec_enet_private {
 	/* The ring entries to be free()ed */
 	struct bufdesc	*dirty_tx;
 
+	unsigned short tx_ring_size;
+	unsigned short rx_ring_size;
+
 	struct	platform_device *pdev;
 
 	int	opened;
diff --git a/drivers/net/ethernet/freescale/fec_main.c b/drivers/net/ethernet/freescale/fec_main.c
index 4ea1555..9894dd3 100644
--- a/drivers/net/ethernet/freescale/fec_main.c
+++ b/drivers/net/ethernet/freescale/fec_main.c
@@ -239,22 +239,35 @@ MODULE_PARM_DESC(macaddr, "FEC Ethernet MAC address");
 
 static int mii_cnt;
 
-static struct bufdesc *fec_enet_get_nextdesc(struct bufdesc *bdp, int is_ex)
+static inline
+struct bufdesc *next_bd(struct bufdesc *bdp, struct bufdesc *base,
+			int ring_size, int is_ex)
 {
-	struct bufdesc_ex *ex = (struct bufdesc_ex *)bdp;
+	struct bufdesc *new_bd = bdp + 1;
+	struct bufdesc_ex *ex_new_bd = (struct bufdesc_ex *)bdp + 1;
+	struct bufdesc_ex *ex_base = (struct bufdesc_ex *)base;
+
 	if (is_ex)
-		return (struct bufdesc *)(ex + 1);
+		return (struct bufdesc *)((ex_new_bd >= (ex_base + ring_size)) ?
+			(ex_new_bd - ring_size) : ex_new_bd);
 	else
-		return bdp + 1;
+		return (new_bd >= (base + ring_size)) ?
+			(new_bd - ring_size) : new_bd;
 }
 
-static struct bufdesc *fec_enet_get_prevdesc(struct bufdesc *bdp, int is_ex)
+static inline
+struct bufdesc *pre_bd(struct bufdesc *bdp, struct bufdesc *base,
+			int ring_size, int is_ex)
 {
-	struct bufdesc_ex *ex = (struct bufdesc_ex *)bdp;
+	struct bufdesc *new_bd = bdp - 1;
+	struct bufdesc_ex *ex_new_bd = (struct bufdesc_ex *)bdp - 1;
+	struct bufdesc_ex *ex_base = (struct bufdesc_ex *)base;
+
 	if (is_ex)
-		return (struct bufdesc *)(ex - 1);
+		return (struct bufdesc *)((ex_new_bd < ex_base) ?
+			(ex_new_bd + ring_size) : ex_new_bd);
 	else
-		return bdp - 1;
+		return (new_bd < base) ? (new_bd + ring_size) : new_bd;
 }
 
 static void *swap_buffer(void *bufaddr, int len)
@@ -380,7 +393,7 @@ fec_enet_start_xmit(struct sk_buff *skb, struct net_device *ndev)
 		}
 	}
 
-	bdp_pre = fec_enet_get_prevdesc(bdp, fep->bufdesc_ex);
+	bdp_pre = pre_bd(bdp, fep->tx_bd_base, fep->tx_ring_size, fep->bufdesc_ex);
 	if ((id_entry->driver_data & FEC_QUIRK_ERR006358) &&
 	    !(bdp_pre->cbd_sc & BD_ENET_TX_READY)) {
 		fep->delay_work.trig_tx = true;
@@ -389,10 +402,7 @@ fec_enet_start_xmit(struct sk_buff *skb, struct net_device *ndev)
 	}
 
 	/* If this was the last BD in the ring, start at the beginning again. */
-	if (status & BD_ENET_TX_WRAP)
-		bdp = fep->tx_bd_base;
-	else
-		bdp = fec_enet_get_nextdesc(bdp, fep->bufdesc_ex);
+	bdp = next_bd(bdp, fep->tx_bd_base, fep->tx_ring_size, fep->bufdesc_ex);
 
 	fep->cur_tx = bdp;
 
@@ -417,18 +427,18 @@ static void fec_enet_bd_init(struct net_device *dev)
 
 	/* Initialize the receive buffer descriptors. */
 	bdp = fep->rx_bd_base;
-	for (i = 0; i < RX_RING_SIZE; i++) {
+	for (i = 0; i < fep->rx_ring_size; i++) {
 
 		/* Initialize the BD for every fragment in the page. */
 		if (bdp->cbd_bufaddr)
 			bdp->cbd_sc = BD_ENET_RX_EMPTY;
 		else
 			bdp->cbd_sc = 0;
-		bdp = fec_enet_get_nextdesc(bdp, fep->bufdesc_ex);
+		bdp = next_bd(bdp, fep->rx_bd_base, fep->rx_ring_size, fep->bufdesc_ex);
 	}
 
 	/* Set the last buffer to wrap */
-	bdp = fec_enet_get_prevdesc(bdp, fep->bufdesc_ex);
+	bdp = pre_bd(bdp, fep->rx_bd_base, fep->rx_ring_size, fep->bufdesc_ex);
 	bdp->cbd_sc |= BD_SC_WRAP;
 
 	fep->cur_rx = fep->rx_bd_base;
@@ -436,7 +446,7 @@ static void fec_enet_bd_init(struct net_device *dev)
 	/* ...and the same for transmit */
 	bdp = fep->tx_bd_base;
 	fep->cur_tx = bdp;
-	for (i = 0; i < TX_RING_SIZE; i++) {
+	for (i = 0; i < fep->tx_ring_size; i++) {
 
 		/* Initialize the BD for every fragment in the page. */
 		bdp->cbd_sc = 0;
@@ -445,11 +455,11 @@ static void fec_enet_bd_init(struct net_device *dev)
 			fep->tx_skbuff[i] = NULL;
 		}
 		bdp->cbd_bufaddr = 0;
-		bdp = fec_enet_get_nextdesc(bdp, fep->bufdesc_ex);
+		bdp = next_bd(bdp, fep->tx_bd_base, fep->tx_ring_size, fep->bufdesc_ex);
 	}
 
 	/* Set the last buffer to wrap */
-	bdp = fec_enet_get_prevdesc(bdp, fep->bufdesc_ex);
+	bdp = pre_bd(bdp, fep->tx_bd_base, fep->tx_ring_size, fep->bufdesc_ex);
 	bdp->cbd_sc |= BD_SC_WRAP;
 	fep->dirty_tx = bdp;
 }
@@ -510,10 +520,10 @@ fec_restart(struct net_device *ndev, int duplex)
 	writel(fep->bd_dma, fep->hwp + FEC_R_DES_START);
 	if (fep->bufdesc_ex)
 		writel((unsigned long)fep->bd_dma + sizeof(struct bufdesc_ex)
-			* RX_RING_SIZE, fep->hwp + FEC_X_DES_START);
+			* fep->rx_ring_size, fep->hwp + FEC_X_DES_START);
 	else
 		writel((unsigned long)fep->bd_dma + sizeof(struct bufdesc)
-			* RX_RING_SIZE,	fep->hwp + FEC_X_DES_START);
+			* fep->rx_ring_size,	fep->hwp + FEC_X_DES_START);
 
 
 	for (i = 0; i <= TX_RING_MOD_MASK; i++) {
@@ -727,10 +737,7 @@ fec_enet_tx(struct net_device *ndev)
 	bdp = fep->dirty_tx;
 
 	/* get next bdp of dirty_tx */
-	if (bdp->cbd_sc & BD_ENET_TX_WRAP)
-		bdp = fep->tx_bd_base;
-	else
-		bdp = fec_enet_get_nextdesc(bdp, fep->bufdesc_ex);
+	bdp = next_bd(bdp, fep->tx_bd_base, fep->tx_ring_size, fep->bufdesc_ex);
 
 	while (((status = bdp->cbd_sc) & BD_ENET_TX_READY) == 0) {
 
@@ -800,10 +807,7 @@ fec_enet_tx(struct net_device *ndev)
 		fep->dirty_tx = bdp;
 
 		/* Update pointer to next buffer descriptor to be transmitted */
-		if (status & BD_ENET_TX_WRAP)
-			bdp = fep->tx_bd_base;
-		else
-			bdp = fec_enet_get_nextdesc(bdp, fep->bufdesc_ex);
+		bdp = next_bd(bdp, fep->tx_bd_base, fep->tx_ring_size, fep->bufdesc_ex);
 
 		/* Since we have freed up a buffer, the ring is no longer full
 		 */
@@ -994,10 +998,8 @@ rx_processing_done:
 		}
 
 		/* Update BD pointer to next entry */
-		if (status & BD_ENET_RX_WRAP)
-			bdp = fep->rx_bd_base;
-		else
-			bdp = fec_enet_get_nextdesc(bdp, fep->bufdesc_ex);
+		bdp = next_bd(bdp, fep->rx_bd_base, fep->rx_ring_size, fep->bufdesc_ex);
+
 		/* Doing this here will keep the FEC running while we process
 		 * incoming frames.  On a heavily loaded network, we should be
 		 * able to keep up at the expense of system resources.
@@ -1663,7 +1665,7 @@ static void fec_enet_free_buffers(struct net_device *ndev)
 	struct bufdesc	*bdp;
 
 	bdp = fep->rx_bd_base;
-	for (i = 0; i < RX_RING_SIZE; i++) {
+	for (i = 0; i < fep->rx_ring_size; i++) {
 		skb = fep->rx_skbuff[i];
 
 		if (bdp->cbd_bufaddr)
@@ -1671,11 +1673,11 @@ static void fec_enet_free_buffers(struct net_device *ndev)
 					FEC_ENET_RX_FRSIZE, DMA_FROM_DEVICE);
 		if (skb)
 			dev_kfree_skb(skb);
-		bdp = fec_enet_get_nextdesc(bdp, fep->bufdesc_ex);
+		bdp = next_bd(bdp, fep->rx_bd_base, fep->rx_ring_size, fep->bufdesc_ex);
 	}
 
 	bdp = fep->tx_bd_base;
-	for (i = 0; i < TX_RING_SIZE; i++)
+	for (i = 0; i < fep->tx_ring_size; i++)
 		kfree(fep->tx_bounce[i]);
 }
 
@@ -1687,7 +1689,7 @@ static int fec_enet_alloc_buffers(struct net_device *ndev)
 	struct bufdesc	*bdp;
 
 	bdp = fep->rx_bd_base;
-	for (i = 0; i < RX_RING_SIZE; i++) {
+	for (i = 0; i < fep->rx_ring_size; i++) {
 		skb = netdev_alloc_skb(ndev, FEC_ENET_RX_FRSIZE);
 		if (!skb) {
 			fec_enet_free_buffers(ndev);
@@ -1704,15 +1706,15 @@ static int fec_enet_alloc_buffers(struct net_device *ndev)
 			ebdp->cbd_esc = BD_ENET_RX_INT;
 		}
 
-		bdp = fec_enet_get_nextdesc(bdp, fep->bufdesc_ex);
+		bdp = next_bd(bdp, fep->rx_bd_base, fep->rx_ring_size, fep->bufdesc_ex);
 	}
 
 	/* Set the last buffer to wrap. */
-	bdp = fec_enet_get_prevdesc(bdp, fep->bufdesc_ex);
+	bdp = pre_bd(bdp, fep->rx_bd_base, fep->rx_ring_size, fep->bufdesc_ex);
 	bdp->cbd_sc |= BD_SC_WRAP;
 
 	bdp = fep->tx_bd_base;
-	for (i = 0; i < TX_RING_SIZE; i++) {
+	for (i = 0; i < fep->tx_ring_size; i++) {
 		fep->tx_bounce[i] = kmalloc(FEC_ENET_TX_FRSIZE, GFP_KERNEL);
 
 		bdp->cbd_sc = 0;
@@ -1723,11 +1725,11 @@ static int fec_enet_alloc_buffers(struct net_device *ndev)
 			ebdp->cbd_esc = BD_ENET_TX_INT;
 		}
 
-		bdp = fec_enet_get_nextdesc(bdp, fep->bufdesc_ex);
+		bdp = next_bd(bdp, fep->tx_bd_base, fep->tx_ring_size, fep->bufdesc_ex);
 	}
 
 	/* Set the last buffer to wrap. */
-	bdp = fec_enet_get_prevdesc(bdp, fep->bufdesc_ex);
+	bdp = pre_bd(bdp, fep->tx_bd_base, fep->tx_ring_size, fep->bufdesc_ex);
 	bdp->cbd_sc |= BD_SC_WRAP;
 
 	return 0;
@@ -1967,13 +1969,17 @@ static int fec_enet_init(struct net_device *ndev)
 	/* Get the Ethernet address */
 	fec_get_mac(ndev);
 
+	/* init the tx & rx ring size */
+	fep->tx_ring_size = TX_RING_SIZE;
+	fep->rx_ring_size = RX_RING_SIZE;
+
 	/* Set receive and transmit descriptor base. */
 	fep->rx_bd_base = cbd_base;
 	if (fep->bufdesc_ex)
 		fep->tx_bd_base = (struct bufdesc *)
-			(((struct bufdesc_ex *)cbd_base) + RX_RING_SIZE);
+			(((struct bufdesc_ex *)cbd_base) + fep->rx_ring_size);
 	else
-		fep->tx_bd_base = cbd_base + RX_RING_SIZE;
+		fep->tx_bd_base = cbd_base + fep->rx_ring_size;
 
 	/* The FEC Ethernet specific entries in the device structure */
 	ndev->watchdog_timeo = TX_TIMEOUT;
-- 
1.7.1

^ permalink raw reply related

* [PATCH 2/2] drivers:net: delete premature free_irq
From: Julia Lawall @ 2013-09-02  9:54 UTC (permalink / raw)
  To: Wan ZongShun; +Cc: kernel-janitors, linux-arm-kernel, netdev, linux-kernel
In-Reply-To: <1378115661-5371-1-git-send-email-Julia.Lawall@lip6.fr>

From: Julia Lawall <Julia.Lawall@lip6.fr>

Free_irq is not needed if there has been no request_irq.  Free_irq is
removed from both the probe and remove functions.  The correct request_irq
and free_irq are found in the open and close functions.

A simplified version of the semantic match that finds this problem is as
follows: (http://coccinelle.lip6.fr/)

// <smpl>
@@
expression e;
@@

*e = platform_get_irq(...);
... when != request_irq(e,...)
*free_irq(e,...)
// </smpl>

Signed-off-by: Julia Lawall <Julia.Lawall@lip6.fr>

---
Not tested.

 drivers/net/ethernet/nuvoton/w90p910_ether.c |   11 ++---------
 1 file changed, 2 insertions(+), 9 deletions(-)

diff --git a/drivers/net/ethernet/nuvoton/w90p910_ether.c b/drivers/net/ethernet/nuvoton/w90p910_ether.c
index dcfe58f..79645f7 100644
--- a/drivers/net/ethernet/nuvoton/w90p910_ether.c
+++ b/drivers/net/ethernet/nuvoton/w90p910_ether.c
@@ -1014,7 +1014,7 @@ static int w90p910_ether_probe(struct platform_device *pdev)
 	if (ether->rxirq < 0) {
 		dev_err(&pdev->dev, "failed to get ether rx irq\n");
 		error = -ENXIO;
-		goto failed_free_txirq;
+		goto failed_free_io;
 	}
 
 	platform_set_drvdata(pdev, dev);
@@ -1023,7 +1023,7 @@ static int w90p910_ether_probe(struct platform_device *pdev)
 	if (IS_ERR(ether->clk)) {
 		dev_err(&pdev->dev, "failed to get ether clock\n");
 		error = PTR_ERR(ether->clk);
-		goto failed_free_rxirq;
+		goto failed_free_io;
 	}
 
 	ether->rmiiclk = clk_get(&pdev->dev, "RMII");
@@ -1049,10 +1049,6 @@ failed_put_rmiiclk:
 	clk_put(ether->rmiiclk);
 failed_put_clk:
 	clk_put(ether->clk);
-failed_free_rxirq:
-	free_irq(ether->rxirq, pdev);
-failed_free_txirq:
-	free_irq(ether->txirq, pdev);
 failed_free_io:
 	iounmap(ether->reg);
 failed_free_mem:
@@ -1075,9 +1071,6 @@ static int w90p910_ether_remove(struct platform_device *pdev)
 	iounmap(ether->reg);
 	release_mem_region(ether->res->start, resource_size(ether->res));
 
-	free_irq(ether->txirq, dev);
-	free_irq(ether->rxirq, dev);
-
 	del_timer_sync(&ether->check_timer);
 
 	free_netdev(dev);

^ permalink raw reply related

* [PATCH 0/2] delete premature free_irq
From: Julia Lawall @ 2013-09-02  9:54 UTC (permalink / raw)
  To: linux-spi; +Cc: kernel-janitors, linux-kernel, linux-arm-kernel, netdev

platform_get_request, or its wrapper platform_get_irq, doesn't allocate
anything, so there is nothing to free.  Free_irq matches up with
request_irq and related functions.

The complete semantic patch used is as follows
(http://coccinelle.lip6.fr/)

// <smpl>
@@
identifier l;
statement S1;
expression e,e1;
@@

*e = platform_get_irq(...);
... when != request_irq(e,...)
    when != request_threaded_irq(e,...)
    when != request_any_context_irq(e,...)
    when != if (...) { <+... request_irq(e,...) ...+> } else S1
    when != if (...) { <+... request_threaded_irq(e,...) ...+> } else S1
    when != if (...) { <+... request_any_context_irq(e,...) ...+> } else S1
    when != e1 = e
*goto l;
...
*free_irq(e,...)

@@
identifier l;
statement S,S1;
expression e,e1;
@@

*e = platform_get_irq(...);
... when != request_irq(e,...)
    when != request_threaded_irq(e,...)
    when != request_any_context_irq(e,...)
    when != if (...) { <+... request_irq(e,...) ...+> } else S1
    when != if (...) { <+... request_threaded_irq(e,...) ...+> } else S1
    when != if (...) { <+... request_any_context_irq(e,...) ...+> } else S1
*e1 = e;
... when != request_irq(\(e\|e1\),...)
    when != request_threaded_irq(\(e\|e1\),...)
    when != request_any_context_irq(\(e\|e1\),...)
    when != if (...) { <+... request_irq(\(e\|e1\),...) ...+> } else S
    when != if (...) { <+... request_threaded_irq(\(e\|e1\),...) ...+> } else S
    when != if (...) { <+... request_any_context_irq(\(e\|e1\),...) ...+> } else S
*goto l;
...
*free_irq(\(e\|e1\),...)

@@
identifier l;
statement S;
expression e,e1;
@@

*e1 = e = platform_get_irq(...);
... when != request_irq(\(e\|e1\),...)
    when != request_threaded_irq(\(e\|e1\),...)
    when != request_any_context_irq(\(e\|e1\),...)
    when != if (...) { <+... request_irq(\(e\|e1\),...) ...+> } else S
    when != if (...) { <+... request_threaded_irq(\(e\|e1\),...) ...+> } else S
    when != if (...) { <+... request_any_context_irq(\(e\|e1\),...) ...+> } else S
*goto l;
...
*free_irq(\(e\|e1\),...)

// -------------------------------------------------------------------------

@@
identifier l;
statement S,S1;
expression e,e2,e3;
@@

*e = platform_get_resource(e2,IORESOURCE_IRQ,e3);
... when != request_irq(e->start,...)
    when != request_threaded_irq(e->start,...)
    when != request_any_context_irq(e->start,...)
    when != if (...) { <+... request_irq(e->start,...) ...+> } else S1
    when != if (...) { <+... request_threaded_irq(e->start,...) ...+> } else S1
    when != if (...) { <+... request_any_context_irq(e->start,...) ...+> } else S1
*goto l;
...
*free_irq(e->start,...)

@@
identifier l;
statement S,S1;
expression e,e1,e2,e3;
@@

*e = platform_get_resource(e2,IORESOURCE_IRQ,e3);
... when != request_irq(e->start,...)
    when != request_threaded_irq(e->start,...)
    when != request_any_context_irq(e->start,...)
    when != if (...) { <+... request_irq(e->start,...) ...+> } else S1
    when != if (...) { <+... request_threaded_irq(e->start,...) ...+> } else S1
    when != if (...) { <+... request_any_context_irq(e->start,...) ...+> } else S1
e1 = e->start;
... when != request_irq(\(e->start\|e1\),...)
    when != request_threaded_irq(\(e->start\|e1\),...)
    when != request_any_context_irq(\(e->start\|e1\),...)
    when != if (...) { <+... request_irq(\(e->start\|e1\),...) ...+> } else S
    when != if (...) { <+... request_threaded_irq(\(e->start\|e1\),...) ...+> } else S
    when != if (...) { <+... request_any_context_irq(\(e->start\|e1\),...) ...+> } else S
*goto l;
...
*free_irq(\(e->start\|e1\),...)
// </smpl>

^ permalink raw reply

* Re: Realtek r8168 hangs when sending data at full speed on a gigabit link
From: Frédéric Leroy @ 2013-09-02  9:46 UTC (permalink / raw)
  To: Francois Romieu; +Cc: netdev, Realtek linux nic maintainers
In-Reply-To: <20130901213610.GA25211@electric-eye.fr.zoreil.com>

Hello Francois,

Le 01/09/2013 23:36, Francois Romieu a écrit :
> Frédéric Leroy <fredo@starox.org> :
> [...]
> > For triggering the bug, I use iperf, which either works, or hangs almost
> > immediately
> > with multiple threads :
> > 
> >         iperf -c myiperfserver -P 3 -t 60
> > 
> > Could you help me to fix this ?
>
> Does it behave the same if you don't load the vbox modules at all ?
>

When I don't load the vbox modules, it behaves the same.

I forget to say that with an unique tcp stream, it hangs after a longer
time.
I took one capture of a hanged tcp stream.
You can find it there : http://www.starox.org/pub/linux/r8169/

Regards,

-- 
Frédéric Leroy

^ permalink raw reply

* Re: linux-next: build failure after merge of the final tree (net-next tree related)
From: Cong Wang @ 2013-09-02  9:18 UTC (permalink / raw)
  To: Stephen Rothwell; +Cc: David Miller, netdev, linux-next, linux-kernel
In-Reply-To: <20130902191120.645dab701e2a68bb2ac6ba33@canb.auug.org.au>

On Mon, 2013-09-02 at 19:11 +1000, Stephen Rothwell wrote:
> Hi all,
> 
> After merging the final tree, today's linux-next build (powerpc
> allyesconfig) failed like this:
> 
> drivers/net/vxlan.c: In function 'vxlan6_xmit_skb':
> drivers/net/vxlan.c:1441:3: error: implicit declaration of function 'csum_ipv6_magic' [-Werror=implicit-function-declaration]
>    uh->check = csum_ipv6_magic(saddr, daddr, skb->len,
>    ^
> 
> Caused by commit e4c7ed415387 ("vxlan: add ipv6 support").
> 
> Please read Rule 1 in Documentation/SubmitChecklist.
> 
> I added the following patch for today:

Hi, Stephen

I sent a same patch this morning:
http://marc.info/?l=linux-netdev&m=137808763132336&w=2

Thanks!

^ permalink raw reply

* linux-next: build failure after merge of the final tree (net-next tree related)
From: Stephen Rothwell @ 2013-09-02  9:11 UTC (permalink / raw)
  To: David Miller, netdev; +Cc: linux-next, linux-kernel, Cong Wang

[-- Attachment #1: Type: text/plain, Size: 1204 bytes --]

Hi all,

After merging the final tree, today's linux-next build (powerpc
allyesconfig) failed like this:

drivers/net/vxlan.c: In function 'vxlan6_xmit_skb':
drivers/net/vxlan.c:1441:3: error: implicit declaration of function 'csum_ipv6_magic' [-Werror=implicit-function-declaration]
   uh->check = csum_ipv6_magic(saddr, daddr, skb->len,
   ^

Caused by commit e4c7ed415387 ("vxlan: add ipv6 support").

Please read Rule 1 in Documentation/SubmitChecklist.

I added the following patch for today:

From: Stephen Rothwell <sfr@canb.auug.org.au>
Date: Mon, 2 Sep 2013 19:04:51 +1000
Subject: [PATCH] vxlan: include the file that declares csum_ipv6_magic

Signed-off-by: Stephen Rothwell <sfr@canb.auug.org.au>
---
 drivers/net/vxlan.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/net/vxlan.c b/drivers/net/vxlan.c
index 3ffb22d..35c78b3 100644
--- a/drivers/net/vxlan.c
+++ b/drivers/net/vxlan.c
@@ -44,6 +44,7 @@
 #include <net/ipv6.h>
 #include <net/addrconf.h>
 #include <net/ip6_tunnel.h>
+#include <net/ip6_checksum.h>
 #endif
 
 #define VXLAN_VERSION	"0.1"
-- 
1.8.4.rc3

-- 
Cheers,
Stephen Rothwell                    sfr@canb.auug.org.au

[-- Attachment #2: Type: application/pgp-signature, Size: 836 bytes --]

^ permalink raw reply related

* [patch] sfc: check for allocation failure
From: Dan Carpenter @ 2013-09-02  9:04 UTC (permalink / raw)
  To: Solarflare linux maintainers; +Cc: Ben Hutchings, netdev, kernel-janitors

It upsets static analyzers when we don't check for allocation failure.

Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>

diff --git a/drivers/net/ethernet/sfc/mtd.c b/drivers/net/ethernet/sfc/mtd.c
index ba6c87a..aac6897 100644
--- a/drivers/net/ethernet/sfc/mtd.c
+++ b/drivers/net/ethernet/sfc/mtd.c
@@ -384,6 +384,8 @@ static int falcon_mtd_probe(struct efx_nic *efx)
 
 	/* Allocate space for maximum number of partitions */
 	parts = kcalloc(2, sizeof(*parts), GFP_KERNEL);
+	if (!parts)
+		return -ENOMEM;
 	n_parts = 0;
 
 	spi = &nic_data->spi_flash;

^ permalink raw reply related

* Re: 3.11-rc7:BUG: soft lockup
From: Ding Tianhong @ 2013-09-02  9:04 UTC (permalink / raw)
  To: Baoquan He; +Cc: Hillf Danton, Cong Wang, LKML, Linux Kernel Network Developers
In-Reply-To: <52244B44.4030703@gmail.com>

On 2013/9/2 16:24, Baoquan He wrote:
> Hi Tianhong,
> 
> I applied your patch and execute below cmd. 
> Then keyboard inputting problems happened, 
> I can't enter user/password correctly, then reboot again, it's OK now.

Thanks for your work, I don't understand that the keyboard problem is the patch fault or your system problem, 
if it is not happen again, I think the problem is fixed.

> 
> The original bug still can't be reproduced.
> 
> make SUBDIRS=net/bridge/netfilter/
> cp /net/bridge/netfilter/ebtables.ko
> /lib/modules/3.11.0-rc7+/kernel/net/bridge/netfilter/ebtables.ko
> depmod
> modprobe ebtables.ko
> reboot
> 
> Baoquan
> Thanks
> 
>> please try this patch and give me the result, thanks.
>>
>> Return the correct value if mutex_lock_interruptible() failed, avoid
>> confusion with that the modules is not exist, and deal with the return
>> value in right way.
>>
>> if mutex_lock_interrupt() failed, sh
>>
>> Signed-off-by: root <root@linux-yocto.site>
>> ---
>>  net/bridge/netfilter/ebtables.c | 17 +++++++----------
>>  1 file changed, 7 insertions(+), 10 deletions(-)
>>
>> diff --git a/net/bridge/netfilter/ebtables.c b/net/bridge/netfilter/ebtables.c
>> index ac78024..e7fe9f8 100644
>> --- a/net/bridge/netfilter/ebtables.c
>> +++ b/net/bridge/netfilter/ebtables.c
>> @@ -322,17 +322,14 @@ static inline void *
>>  find_inlist_lock_noload(struct list_head *head, const char *name, int *error,
>>     struct mutex *mutex)
>>  {
>> -	struct {
>> -		struct list_head list;
>> -		char name[EBT_FUNCTION_MAXNAMELEN];
>> -	} *e;
>> +	struct ebt_table *e;
>>  
>>  	*error = mutex_lock_interruptible(mutex);
>>  	if (*error != 0)
>> -		return NULL;
>> +		return ERR_PTR(-EINTR);
>>  
>>  	list_for_each_entry(e, head, list) {
>> -		if (strcmp(e->name, name) == 0)
>> +		 if (strcmp(e->name, name) == 0 && try_module_get(e->me))
>>  			return e;
>>  	}
>>  	*error = -ENOENT;
>> @@ -1005,7 +1002,7 @@ static int do_replace_finish(struct net *net, struct ebt_replace *repl,
>>  		goto free_counterstmp;
>>  
>>  	t = find_table_lock(net, repl->name, &ret, &ebt_mutex);
>> -	if (!t) {
>> +	if (IS_ERR_OR_NULL(t)) {
>>  		ret = -ENOENT;
>>  		goto free_iterate;
>>  	}
>> @@ -1284,7 +1281,7 @@ static int do_update_counters(struct net *net, const char *name,
>>  		return -ENOMEM;
>>  
>>  	t = find_table_lock(net, name, &ret, &ebt_mutex);
>> -	if (!t)
>> +	if (IS_ERR_OR_NULL(t))
>>  		goto free_tmp;
>>  
>>  	if (num_counters != t->private->nentries) {
>> @@ -1504,7 +1501,7 @@ static int do_ebt_get_ctl(struct sock *sk, int cmd, void __user *user, int *len)
>>  		return -EFAULT;
>>  
>>  	t = find_table_lock(net, tmp.name, &ret, &ebt_mutex);
>> -	if (!t)
>> +	if (IS_ERR_OR_NULL(t))
>>  		return ret;
>>  
>>  	switch(cmd) {
>> @@ -2319,7 +2316,7 @@ static int compat_do_ebt_get_ctl(struct sock *sk, int cmd,
>>  		return -EFAULT;
>>  
>>  	t = find_table_lock(net, tmp.name, &ret, &ebt_mutex);
>> -	if (!t)
>> +	if (IS_ERR_OR_NULL(t))
>>  		return ret;
>>  
>>  	xt_compat_lock(NFPROTO_BRIDGE);
> 
> 
> .
> 

^ permalink raw reply

* Re: [PATCH 2/2] sh_eth: add device tree support
From: Mark Rutland @ 2013-09-02  8:52 UTC (permalink / raw)
  To: Sergei Shtylyov
  Cc: netdev@vger.kernel.org, rob.herring@calxeda.com, Pawel Moll,
	swarren@wwwdotorg.org, ian.campbell@citrix.com,
	grant.likely@linaro.org, devicetree@vger.kernel.org,
	nobuhiro.iwamatsu.yj@renesas.com, linux-sh@vger.kernel.org,
	rob@landley.net, linux-doc@vger.kernel.org
In-Reply-To: <201308310429.34856.sergei.shtylyov@cogentembedded.com>

On Sat, Aug 31, 2013 at 01:29:33AM +0100, Sergei Shtylyov wrote:
> Add support of the device tree probing for the Renesas SH-Mobile SoCs.
> 
> This work is loosely based on an original patch by Nobuhiro Iwamatsu
> <nobuhiro.iwamatsu.yj@renesas.com>.
> 
> Signed-off-by: Sergei Shtylyov <sergei.shtylyov@cogentembedded.com>
> 
> ---
> This patch is against Dave's 'net-next.git' repo.
> 
>  Documentation/devicetree/bindings/net/sh_eth.txt |   40 +++++++++++++
>  drivers/net/ethernet/renesas/sh_eth.c            |   66 ++++++++++++++++++++++-
>  2 files changed, 105 insertions(+), 1 deletion(-)
> 
> Index: net-next/Documentation/devicetree/bindings/net/sh_eth.txt
> ===================================================================
> --- /dev/null
> +++ net-next/Documentation/devicetree/bindings/net/sh_eth.txt
> @@ -0,0 +1,40 @@
> +* Renesas Electronics SH EtherMAC
> +
> +This file provides information on what the device node for the SH EtherMAC
> +interface contains.
> +
> +Required properties:
> +- compatible: "renesas,gether-r8a7740" if the device is a part of R8A7740 SoC.
> +	      "renesas,ether-r8a7779" if the device is a part of R8A7778/9 SoCs.
> +	      "renesas,ether-r8a7790" if the device is a part of R8A7790/1 SoCs.

What are the functional differences between the blocks in these devices
that mean they have different compatible strings?

> +- reg: offset and length of the register set for the device; if the device has
> +       TSU registers, you need to specify two register sets here.

This doesn't explicitly state ordering, and doesn't describe what the
first register set is (control registers?). If possible, it would be
nice to refer to the set of registers by the name given in
documentation; is there any available?

I think we should have something like the below to ensure it's explicit.
In general we need more consistency in the the way bindings describe reg
properties.

	- reg: offset and length of:
	     [1] the control registers of the device (required)
	     [2] the TSU registers for the device (optional)

> +- interrupt-parent: the phandle for the interrupt controller that services
> +		    interrupts for this device.

Why is that required?

> +- interrupts: interrupt mapping for the interrupt source.

Interrupts are defined in terms of interrupt-specifiers. How about:

	- interrupts: an interrupt-specifier for the sole interrupt
	              generated by the device.

> +- phy-mode: string, operation mode of the PHY interface (a string that
> +	    of_get_phy_mode() can understand).

That looks suspicious. Bindings should *not* refer to Linux internals.
Instead, we should document the phy-handle and phy-mode properties and
how they are meant to be used in a generic binding document (I couldn't
see a generic document doing this so far...).

> +- phy-handle: phandle of the PHY device.
> +
> +Optional properties:
> +- local-mac-address: 6 bytes, MAC address.
> +- renesas,no-ether-link: specify when a board does not provide a proper LINK
> +			 signal.
> +- renesas,ether-link-active-low: specify when the LINK signal is active-low.

What types are these? I know local-mac-address is a byte-string by
ePAPR, presumably the last two are empty (boolean)?

> +
> +Example (Armadillo800EVA board):
> +
> +	ethernet@e9a00000 {
> +		compatible = "renesas,gether-r8a7740";
> +		reg = <0xe9a00000 0x800>, <0xe9a01800 0x800>;
> +		interrupt-parent = <&gic>;
> +		interrupts = <0 142 0x4>;
> +		phy-mode = "mii";
> +		phy-handle = <&phy0>;
> +		#address-cells = <1>;
> +		#size-cells = <0>;
> +
> +		phy0: ethernet-phy@0 {
> +			reg = <0>;
> +		};

The binding didn't state anything about sub-nodes. Is it a general
property of phy bindings that they may be embedded within a consumer's
node?

> +	};
> Index: net-next/drivers/net/ethernet/renesas/sh_eth.c
> ===================================================================
> --- net-next.orig/drivers/net/ethernet/renesas/sh_eth.c
> +++ net-next/drivers/net/ethernet/renesas/sh_eth.c
> @@ -32,6 +32,9 @@
>  #include <linux/platform_device.h>
>  #include <linux/mdio-bitbang.h>
>  #include <linux/netdevice.h>
> +#include <linux/of.h>
> +#include <linux/of_device.h>
> +#include <linux/of_net.h>
>  #include <linux/phy.h>
>  #include <linux/cache.h>
>  #include <linux/io.h>
> @@ -2600,6 +2603,52 @@ static const struct net_device_ops sh_et
>  	.ndo_change_mtu		= eth_change_mtu,
>  };
>  
> +#ifdef CONFIG_OF
> +static struct sh_eth_plat_data *sh_eth_parse_dt(struct device *dev)
> +{
> +	struct device_node *np = dev->of_node;
> +	struct sh_eth_plat_data *pdata;
> +	struct device_node *phy;
> +	const char *mac_addr;
> +
> +	pdata = devm_kzalloc(dev, sizeof(*pdata), GFP_KERNEL);
> +	if (!pdata)
> +		return NULL;
> +
> +	pdata->phy_interface = of_get_phy_mode(np);
> +
> +	phy = of_parse_phandle(np, "phy-handle", 0);
> +	if (!phy || of_property_read_u32(phy, "reg", &pdata->phy)) {

NAK. You didn't describe the format of the phy node, yet you are reading
values from it from a logically separate driver.

Thanks,
Mark.

^ permalink raw reply

* [PATCH V3 6/6] vhost_net: correctly limit the max pending buffers
From: Jason Wang @ 2013-09-02  8:41 UTC (permalink / raw)
  To: mst, kvm, virtualization, netdev, linux-kernel
In-Reply-To: <1378111261-14826-1-git-send-email-jasowang@redhat.com>

As Michael point out, We used to limit the max pending DMAs to get better cache
utilization. But it was not done correctly since it was one done when there's no
new buffers submitted from guest. Guest can easily exceeds the limitation by
keeping sending packets.

So this patch moves the check into main loop. Tests shows about 5%-10%
improvement on per cpu throughput for guest tx.

Signed-off-by: Jason Wang <jasowang@redhat.com>
---
 drivers/vhost/net.c |   18 +++++++-----------
 1 files changed, 7 insertions(+), 11 deletions(-)

diff --git a/drivers/vhost/net.c b/drivers/vhost/net.c
index 8e9dc55..831eb4f 100644
--- a/drivers/vhost/net.c
+++ b/drivers/vhost/net.c
@@ -363,6 +363,13 @@ static void handle_tx(struct vhost_net *net)
 		if (zcopy)
 			vhost_zerocopy_signal_used(net, vq);
 
+		/* If more outstanding DMAs, queue the work.
+		 * Handle upend_idx wrap around
+		 */
+		if (unlikely((nvq->upend_idx + vq->num - VHOST_MAX_PEND)
+			      % UIO_MAXIOV == nvq->done_idx))
+			break;
+
 		head = vhost_get_vq_desc(&net->dev, vq, vq->iov,
 					 ARRAY_SIZE(vq->iov),
 					 &out, &in,
@@ -372,17 +379,6 @@ static void handle_tx(struct vhost_net *net)
 			break;
 		/* Nothing new?  Wait for eventfd to tell us they refilled. */
 		if (head == vq->num) {
-			int num_pends;
-
-			/* If more outstanding DMAs, queue the work.
-			 * Handle upend_idx wrap around
-			 */
-			num_pends = likely(nvq->upend_idx >= nvq->done_idx) ?
-				    (nvq->upend_idx - nvq->done_idx) :
-				    (nvq->upend_idx + UIO_MAXIOV -
-				     nvq->done_idx);
-			if (unlikely(num_pends > VHOST_MAX_PEND))
-				break;
 			if (unlikely(vhost_enable_notify(&net->dev, vq))) {
 				vhost_disable_notify(&net->dev, vq);
 				continue;
-- 
1.7.1

^ permalink raw reply related

* [PATCH V3 5/6] vhost_net: poll vhost queue after marking DMA is done
From: Jason Wang @ 2013-09-02  8:41 UTC (permalink / raw)
  To: mst, kvm, virtualization, netdev, linux-kernel; +Cc: Jason Wang
In-Reply-To: <1378111261-14826-1-git-send-email-jasowang@redhat.com>

We used to poll vhost queue before making DMA is done, this is racy if vhost
thread were waked up before marking DMA is done which can result the signal to
be missed. Fix this by always polling the vhost thread before DMA is done.

Signed-off-by: Jason Wang <jasowang@redhat.com>
---
- The patch is needed for stable 3.4+
---
 drivers/vhost/net.c |    9 +++++----
 1 files changed, 5 insertions(+), 4 deletions(-)

diff --git a/drivers/vhost/net.c b/drivers/vhost/net.c
index 3f89dea..8e9dc55 100644
--- a/drivers/vhost/net.c
+++ b/drivers/vhost/net.c
@@ -308,6 +308,11 @@ static void vhost_zerocopy_callback(struct ubuf_info *ubuf, bool success)
 	struct vhost_virtqueue *vq = ubufs->vq;
 	int cnt = atomic_read(&ubufs->kref.refcount);
 
+	/* set len to mark this desc buffers done DMA */
+	vq->heads[ubuf->desc].len = success ?
+		VHOST_DMA_DONE_LEN : VHOST_DMA_FAILED_LEN;
+	vhost_net_ubuf_put(ubufs);
+
 	/*
 	 * Trigger polling thread if guest stopped submitting new buffers:
 	 * in this case, the refcount after decrement will eventually reach 1
@@ -318,10 +323,6 @@ static void vhost_zerocopy_callback(struct ubuf_info *ubuf, bool success)
 	 */
 	if (cnt <= 2 || !(cnt % 16))
 		vhost_poll_queue(&vq->poll);
-	/* set len to mark this desc buffers done DMA */
-	vq->heads[ubuf->desc].len = success ?
-		VHOST_DMA_DONE_LEN : VHOST_DMA_FAILED_LEN;
-	vhost_net_ubuf_put(ubufs);
 }
 
 /* Expects to be always run from workqueue - which acts as
-- 
1.7.1

^ permalink raw reply related

* [PATCH V3 4/6] vhost_net: determine whether or not to use zerocopy at one time
From: Jason Wang @ 2013-09-02  8:40 UTC (permalink / raw)
  To: mst, kvm, virtualization, netdev, linux-kernel
In-Reply-To: <1378111261-14826-1-git-send-email-jasowang@redhat.com>

Currently, even if the packet length is smaller than VHOST_GOODCOPY_LEN, if
upend_idx != done_idx we still set zcopy_used to true and rollback this choice
later. This could be avoided by determining zerocopy once by checking all
conditions at one time before.

Signed-off-by: Jason Wang <jasowang@redhat.com>
---
 drivers/vhost/net.c |   47 ++++++++++++++++++++---------------------------
 1 files changed, 20 insertions(+), 27 deletions(-)

diff --git a/drivers/vhost/net.c b/drivers/vhost/net.c
index 8a6dd0d..3f89dea 100644
--- a/drivers/vhost/net.c
+++ b/drivers/vhost/net.c
@@ -404,43 +404,36 @@ static void handle_tx(struct vhost_net *net)
 			       iov_length(nvq->hdr, s), hdr_size);
 			break;
 		}
-		zcopy_used = zcopy && (len >= VHOST_GOODCOPY_LEN ||
-				       nvq->upend_idx != nvq->done_idx);
+
+		zcopy_used = zcopy && len >= VHOST_GOODCOPY_LEN
+				   && (nvq->upend_idx + 1) % UIO_MAXIOV !=
+				      nvq->done_idx
+				   && vhost_net_tx_select_zcopy(net);
 
 		/* use msg_control to pass vhost zerocopy ubuf info to skb */
 		if (zcopy_used) {
+			struct ubuf_info *ubuf;
+			ubuf = nvq->ubuf_info + nvq->upend_idx;
+
 			vq->heads[nvq->upend_idx].id = head;
-			if (!vhost_net_tx_select_zcopy(net) ||
-			    len < VHOST_GOODCOPY_LEN) {
-				/* copy don't need to wait for DMA done */
-				vq->heads[nvq->upend_idx].len =
-							VHOST_DMA_DONE_LEN;
-				msg.msg_control = NULL;
-				msg.msg_controllen = 0;
-				ubufs = NULL;
-			} else {
-				struct ubuf_info *ubuf;
-				ubuf = nvq->ubuf_info + nvq->upend_idx;
-
-				vq->heads[nvq->upend_idx].len =
-					VHOST_DMA_IN_PROGRESS;
-				ubuf->callback = vhost_zerocopy_callback;
-				ubuf->ctx = nvq->ubufs;
-				ubuf->desc = nvq->upend_idx;
-				msg.msg_control = ubuf;
-				msg.msg_controllen = sizeof(ubuf);
-				ubufs = nvq->ubufs;
-				kref_get(&ubufs->kref);
-			}
+			vq->heads[nvq->upend_idx].len = VHOST_DMA_IN_PROGRESS;
+			ubuf->callback = vhost_zerocopy_callback;
+			ubuf->ctx = nvq->ubufs;
+			ubuf->desc = nvq->upend_idx;
+			msg.msg_control = ubuf;
+			msg.msg_controllen = sizeof(ubuf);
+			ubufs = nvq->ubufs;
+			kref_get(&ubufs->kref);
 			nvq->upend_idx = (nvq->upend_idx + 1) % UIO_MAXIOV;
-		} else
+		} else {
 			msg.msg_control = NULL;
+			ubufs = NULL;
+		}
 		/* TODO: Check specific error and bomb out unless ENOBUFS? */
 		err = sock->ops->sendmsg(NULL, sock, &msg, len);
 		if (unlikely(err < 0)) {
 			if (zcopy_used) {
-				if (ubufs)
-					vhost_net_ubuf_put(ubufs);
+				vhost_net_ubuf_put(ubufs);
 				nvq->upend_idx = ((unsigned)nvq->upend_idx - 1)
 					% UIO_MAXIOV;
 			}
-- 
1.7.1

^ permalink raw reply related

* [PATCH V3 3/6] vhost: switch to use vhost_add_used_n()
From: Jason Wang @ 2013-09-02  8:40 UTC (permalink / raw)
  To: mst, kvm, virtualization, netdev, linux-kernel
In-Reply-To: <1378111261-14826-1-git-send-email-jasowang@redhat.com>

Let vhost_add_used() to use vhost_add_used_n() to reduce the code
duplication. To avoid the overhead brought by __copy_to_user(). We will use
put_user() when one used need to be added.

Signed-off-by: Jason Wang <jasowang@redhat.com>
---
 drivers/vhost/vhost.c |   54 ++++++++++--------------------------------------
 1 files changed, 12 insertions(+), 42 deletions(-)

diff --git a/drivers/vhost/vhost.c b/drivers/vhost/vhost.c
index 448efe0..9a9502a 100644
--- a/drivers/vhost/vhost.c
+++ b/drivers/vhost/vhost.c
@@ -1332,48 +1332,9 @@ EXPORT_SYMBOL_GPL(vhost_discard_vq_desc);
  * want to notify the guest, using eventfd. */
 int vhost_add_used(struct vhost_virtqueue *vq, unsigned int head, int len)
 {
-	struct vring_used_elem __user *used;
+	struct vring_used_elem heads = { head, len };
 
-	/* The virtqueue contains a ring of used buffers.  Get a pointer to the
-	 * next entry in that used ring. */
-	used = &vq->used->ring[vq->last_used_idx % vq->num];
-	if (__put_user(head, &used->id)) {
-		vq_err(vq, "Failed to write used id");
-		return -EFAULT;
-	}
-	if (__put_user(len, &used->len)) {
-		vq_err(vq, "Failed to write used len");
-		return -EFAULT;
-	}
-	/* Make sure buffer is written before we update index. */
-	smp_wmb();
-	if (__put_user(vq->last_used_idx + 1, &vq->used->idx)) {
-		vq_err(vq, "Failed to increment used idx");
-		return -EFAULT;
-	}
-	if (unlikely(vq->log_used)) {
-		/* Make sure data is seen before log. */
-		smp_wmb();
-		/* Log used ring entry write. */
-		log_write(vq->log_base,
-			  vq->log_addr +
-			   ((void __user *)used - (void __user *)vq->used),
-			  sizeof *used);
-		/* Log used index update. */
-		log_write(vq->log_base,
-			  vq->log_addr + offsetof(struct vring_used, idx),
-			  sizeof vq->used->idx);
-		if (vq->log_ctx)
-			eventfd_signal(vq->log_ctx, 1);
-	}
-	vq->last_used_idx++;
-	/* If the driver never bothers to signal in a very long while,
-	 * used index might wrap around. If that happens, invalidate
-	 * signalled_used index we stored. TODO: make sure driver
-	 * signals at least once in 2^16 and remove this. */
-	if (unlikely(vq->last_used_idx == vq->signalled_used))
-		vq->signalled_used_valid = false;
-	return 0;
+	return vhost_add_used_n(vq, &heads, 1);
 }
 EXPORT_SYMBOL_GPL(vhost_add_used);
 
@@ -1387,7 +1348,16 @@ static int __vhost_add_used_n(struct vhost_virtqueue *vq,
 
 	start = vq->last_used_idx % vq->num;
 	used = vq->used->ring + start;
-	if (__copy_to_user(used, heads, count * sizeof *used)) {
+	if (count == 1) {
+		if (__put_user(heads[0].id, &used->id)) {
+			vq_err(vq, "Failed to write used id");
+			return -EFAULT;
+		}
+		if (__put_user(heads[0].len, &used->len)) {
+			vq_err(vq, "Failed to write used len");
+			return -EFAULT;
+		}
+	} else if (__copy_to_user(used, heads, count * sizeof *used)) {
 		vq_err(vq, "Failed to write used");
 		return -EFAULT;
 	}
-- 
1.7.1

^ permalink raw reply related

* [PATCH V3 2/6] vhost_net: use vhost_add_used_and_signal_n() in vhost_zerocopy_signal_used()
From: Jason Wang @ 2013-09-02  8:40 UTC (permalink / raw)
  To: mst, kvm, virtualization, netdev, linux-kernel
In-Reply-To: <1378111261-14826-1-git-send-email-jasowang@redhat.com>

We tend to batch the used adding and signaling in vhost_zerocopy_callback()
which may result more than 100 used buffers to be updated in
vhost_zerocopy_signal_used() in some cases. So switch to use
vhost_add_used_and_signal_n() to avoid multiple calls to
vhost_add_used_and_signal(). Which means much less times of used index
updating and memory barriers.

2% performance improvement were seen on netperf TCP_RR test.

Signed-off-by: Jason Wang <jasowang@redhat.com>
---
 drivers/vhost/net.c |   13 ++++++++-----
 1 files changed, 8 insertions(+), 5 deletions(-)

diff --git a/drivers/vhost/net.c b/drivers/vhost/net.c
index 280ee66..8a6dd0d 100644
--- a/drivers/vhost/net.c
+++ b/drivers/vhost/net.c
@@ -281,7 +281,7 @@ static void vhost_zerocopy_signal_used(struct vhost_net *net,
 {
 	struct vhost_net_virtqueue *nvq =
 		container_of(vq, struct vhost_net_virtqueue, vq);
-	int i;
+	int i, add;
 	int j = 0;
 
 	for (i = nvq->done_idx; i != nvq->upend_idx; i = (i + 1) % UIO_MAXIOV) {
@@ -289,14 +289,17 @@ static void vhost_zerocopy_signal_used(struct vhost_net *net,
 			vhost_net_tx_err(net);
 		if (VHOST_DMA_IS_DONE(vq->heads[i].len)) {
 			vq->heads[i].len = VHOST_DMA_CLEAR_LEN;
-			vhost_add_used_and_signal(vq->dev, vq,
-						  vq->heads[i].id, 0);
 			++j;
 		} else
 			break;
 	}
-	if (j)
-		nvq->done_idx = i;
+	while (j) {
+		add = min(UIO_MAXIOV - nvq->done_idx, j);
+		vhost_add_used_and_signal_n(vq->dev, vq,
+					    &vq->heads[nvq->done_idx], add);
+		nvq->done_idx = (nvq->done_idx + add) % UIO_MAXIOV;
+		j -= add;
+	}
 }
 
 static void vhost_zerocopy_callback(struct ubuf_info *ubuf, bool success)
-- 
1.7.1

^ permalink raw reply related

* [PATCH V3 1/6] vhost_net: make vhost_zerocopy_signal_used() return void
From: Jason Wang @ 2013-09-02  8:40 UTC (permalink / raw)
  To: mst, kvm, virtualization, netdev, linux-kernel
In-Reply-To: <1378111261-14826-1-git-send-email-jasowang@redhat.com>

None of its caller use its return value, so let it return void.

Signed-off-by: Jason Wang <jasowang@redhat.com>
---
 drivers/vhost/net.c |    5 ++---
 1 files changed, 2 insertions(+), 3 deletions(-)

diff --git a/drivers/vhost/net.c b/drivers/vhost/net.c
index 969a859..280ee66 100644
--- a/drivers/vhost/net.c
+++ b/drivers/vhost/net.c
@@ -276,8 +276,8 @@ static void copy_iovec_hdr(const struct iovec *from, struct iovec *to,
  * of used idx. Once lower device DMA done contiguously, we will signal KVM
  * guest used idx.
  */
-static int vhost_zerocopy_signal_used(struct vhost_net *net,
-				      struct vhost_virtqueue *vq)
+static void vhost_zerocopy_signal_used(struct vhost_net *net,
+				       struct vhost_virtqueue *vq)
 {
 	struct vhost_net_virtqueue *nvq =
 		container_of(vq, struct vhost_net_virtqueue, vq);
@@ -297,7 +297,6 @@ static int vhost_zerocopy_signal_used(struct vhost_net *net,
 	}
 	if (j)
 		nvq->done_idx = i;
-	return j;
 }
 
 static void vhost_zerocopy_callback(struct ubuf_info *ubuf, bool success)
-- 
1.7.1

^ permalink raw reply related

* [PATCH V3 0/6] vhost code cleanup and minor enhancement
From: Jason Wang @ 2013-09-02  8:40 UTC (permalink / raw)
  To: mst, kvm, virtualization, netdev, linux-kernel

This series tries to unify and simplify vhost codes especially for
zerocopy. With this series, 5% - 10% improvement for per cpu throughput were
seen during netperf guest sending test.

Plase review.

Changes from V2:
- Typo fixes and code style fix
- Add performance gain in the commit log of patch 2/6
- Retest the update the result in patch 6/6

Changes from V1:
- Fix the zerocopy enabling check by changing the check of upend_idx != done_idx
  to (upend_idx + 1) % UIO_MAXIOV == done_idx.
- Switch to use put_user() in __vhost_add_used_n() if there's only one used
- Keep the max pending check based on Michael's suggestion.

Jason Wang (6):
  vhost_net: make vhost_zerocopy_signal_used() return void
  vhost_net: use vhost_add_used_and_signal_n() in
    vhost_zerocopy_signal_used()
  vhost: switch to use vhost_add_used_n()
  vhost_net: determine whether or not to use zerocopy at one time
  vhost_net: poll vhost queue after marking DMA is done
  vhost_net: correctly limit the max pending buffers

 drivers/vhost/net.c   |   92 ++++++++++++++++++++++--------------------------
 drivers/vhost/vhost.c |   54 ++++++----------------------
 2 files changed, 54 insertions(+), 92 deletions(-)

^ permalink raw reply

* Re: [PATCH V2 6/6] vhost_net: correctly limit the max pending buffers
From: Jason Wang @ 2013-09-02  8:37 UTC (permalink / raw)
  To: Michael S. Tsirkin; +Cc: netdev, linux-kernel, kvm, virtualization
In-Reply-To: <52243071.5090303@redhat.com>

On 09/02/2013 02:30 PM, Jason Wang wrote:
> On 09/02/2013 01:56 PM, Michael S. Tsirkin wrote:
>> > On Fri, Aug 30, 2013 at 12:29:22PM +0800, Jason Wang wrote:
>>> >> As Michael point out, We used to limit the max pending DMAs to get better cache
>>> >> utilization. But it was not done correctly since it was one done when there's no
>>> >> new buffers submitted from guest. Guest can easily exceeds the limitation by
>>> >> keeping sending packets.
>>> >>
>>> >> So this patch moves the check into main loop. Tests shows about 5%-10%
>>> >> improvement on per cpu throughput for guest tx. But a 5% drop on per cpu
>>> >> transaction rate for a single session TCP_RR.
>> > Any explanation for the drop? single session TCP_RR is unlikely to
>> > exceed VHOST_MAX_PEND, correct?
> Unlikely to exceed. Recheck the result, looks like it was not stable
> enough. Will re-test and report.

Re-tested with more iterations, result shows no difference on TCP_RR
test and 5%-10% improvement on per cpu throughput for guest tx.

Will post V3 soon.

^ 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