Netdev List
 help / color / mirror / Atom feed
* Re: RTNL: assertion failed at net/core/dev.c (4494) and RTNL: assertion failed at net/core/rtnetlink.c (940)
From: Jay Vosburgh @ 2014-02-06 22:33 UTC (permalink / raw)
  To: Cong Wang
  Cc: Thomas Glanzmann, Eric Dumazet, netdev, Veaceslav Falico, andy,
	Jiří Pírko, sfeldma@cumulusnetworks.com
In-Reply-To: <CAHA+R7McuqYDshugsWcaeBbBsEg3QY59LiovYthcq6bH1fLTNg@mail.gmail.com>


Cong Wang <cwang@twopensource.com> wrote:

>On Thu, Feb 6, 2014 at 2:07 PM, Jay Vosburgh <fubar@us.ibm.com> wrote:
>> Jay Vosburgh <fubar@us.ibm.com> wrote:
>>
>>>Cong Wang <cwang@twopensource.com> wrote:
>>>
>>>
>>>       That would eliminate the warning, but is suboptimal.  Acquiring
>>>RTNL is not necessary on the vast majority of state machine runs
>>>(because no state changes take place, i.e., no ports are disabled or
>>>enabled).  The above change would add 10 round trips per second to RTNL,
>>>which seems excessive.
>>>
>>>       Also, we cannot unconditionally acquire RTNL in this function,
>>>as it would race with the call to cancel_delayed_work_sync from
>>>bond_close (via bond_work_cancel_all).
>
>OK.
>
>>
>>         Thought of one more problem: we can't hold a regular lock while
>> calling rtmsg_ifinfo, as it may sleep in alloc_skb.  The rtmsg_ifinfo
>> call has to be RTNL and nothing else.
>>
>
>s/GFP_KERNEL/GFP_ATOMIC/

	Yah, that would help with extra locks, but not totally solve
things.  I'm looking around, and seeing a number of other places that
will end up at one of these rtmsg_ifinfo calls with incorrect locking:

	bond_ab_arp_probe calls via bond_set_slave_active_flags and
bond_set_slave_inactive_flags without RTNL.

	bond_change_active_slave calls via bond_set_slave_inactive_flags
and bond_set_slave_active_flags with other locks held, and maybe without
RTNL; I'm not sure if bond_option_active_slave_set holds RTNL when it
calls bond_select_active_slave.

	bond_open calls via bond_set_slave_active_flags and
bond_set_slave_inactive_flags with RTNL, but also with other locks held.

	bond_loadbalance_arp_mon calls bond_set_active_slave and
bond_set_backup_slave without RTNL.

	This is in addition to the cases in the 802.3ad code from
__enable_port and __disable_port calls.

	-J

---
	-Jay Vosburgh, IBM Linux Technology Center, fubar@us.ibm.com

^ permalink raw reply

* Re: [GIT PULL] tree-wide: clean up no longer required #include <linux/init.h>
From: Stephen Rothwell @ 2014-02-06 22:25 UTC (permalink / raw)
  To: torvalds
  Cc: Paul Gortmaker, linux-alpha, linux-arch, linux-arm-kernel,
	linux-ia64, linux-m68k, linux-mips, linuxppc-dev, linux-s390,
	sparclinux, x86, netdev, kvm, rusty, gregkh, akpm
In-Reply-To: <1391547118-21967-1-git-send-email-paul.gortmaker@windriver.com>

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

Hi Linus,

On Tue, 4 Feb 2014 15:51:58 -0500 Paul Gortmaker <paul.gortmaker@windriver.com> wrote:
>
> We've had this in linux-next for 2+ weeks (thanks Stephen!) as a
> linux-stable like queue of patches, and as can be seen here:
> 
>   https://git.kernel.org/pub/scm/linux/kernel/git/paulg/init.git
> 
> most of the changes in the last week have been trivial adding acks
> or dropping patches that maintainers decided to take themselves.

Any thoughts on merging this?  I can feel it bitrotting :-(

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

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

^ permalink raw reply

* Re: Fw: [Bug 70071] New: Sending netconsole messages over a bridged network interface doesn't work anymore
From: Cong Wang @ 2014-02-06 22:18 UTC (permalink / raw)
  To: Toshiaki Makita; +Cc: Stephen Hemminger, netdev
In-Reply-To: <52F34D41.8000704@lab.ntt.co.jp>

On Thu, Feb 6, 2014 at 12:52 AM, Toshiaki Makita
<makita.toshiaki@lab.ntt.co.jp> wrote:
> (2014/02/06 15:49), Cong Wang wrote:
>> On Wed, Feb 5, 2014 at 9:44 PM, Toshiaki Makita
>> <makita.toshiaki@lab.ntt.co.jp> wrote:
>>> Tested this patch with latest net-tree and netconsole works with it.
>>> But I thinks it is better to move that "if" to br_add_if() because if we
>>> don't have npinfo, we don't have to alloc p->np in br_add_if(), right?
>>>
>>
>> Hmm, we shouldn't handle netpoll-specific code inside br_add_if(),
>> we probably need the attached patch instead. Please give it
>> a try, or I will test it tomorrow, it's too late here.
>>
>
> I tested whether netconsole works and whether it can be built
> with/without CONFIG_NET_POLL_CONTROLLER, and couldn't find any problem.
> This looks good to me.
>

Excellent! I will send it formally.

Thanks for testing!

^ permalink raw reply

* [PATCH iproute2 3/3] tcp_metrics: Allow removal based on the source-IP
From: Christoph Paasch @ 2014-02-06 22:15 UTC (permalink / raw)
  To: Stephen Hemminger; +Cc: netdev
In-Reply-To: <1391724904-30504-1-git-send-email-christoph.paasch@uclouvain.be>

This patch allows adding the source-IP attribute to the netlink-command.

Signed-off-by: Christoph Paasch <christoph.paasch@uclouvain.be>
---
 ip/tcp_metrics.c | 87 ++++++++++++++++++++++++++++++++++++++++----------------
 1 file changed, 62 insertions(+), 25 deletions(-)

diff --git a/ip/tcp_metrics.c b/ip/tcp_metrics.c
index a7215c86379f..e0f03449e94b 100644
--- a/ip/tcp_metrics.c
+++ b/ip/tcp_metrics.c
@@ -75,6 +75,7 @@ static struct
 	int flushe;
 	int cmd;
 	inet_prefix daddr;
+	inet_prefix saddr;
 } f;
 
 static int flush_update(void)
@@ -157,6 +158,12 @@ static int process_msg(const struct sockaddr_nl *who, struct nlmsghdr *n,
 
 	if (f.daddr.family && f.daddr.bitlen >= 0 &&
 	    inet_addr_match(&daddr, &f.daddr, f.daddr.bitlen))
+	       return 0;
+	/* Only check for the source-address if the kernel supports it,
+	 * meaning slen != 0.
+	 */
+	if (slen && f.saddr.family && f.saddr.bitlen >= 0 &&
+	    inet_addr_match(&saddr, &f.saddr, f.saddr.bitlen))
 		return 0;
 
 	if (f.flushb) {
@@ -165,6 +172,9 @@ static int process_msg(const struct sockaddr_nl *who, struct nlmsghdr *n,
 
 		addattr_l(&req2.n, sizeof(req2), atype, &daddr.data,
 			  daddr.bytelen);
+		if (slen)
+			addattr_l(&req2.n, sizeof(req2), stype, &saddr.data,
+				  saddr.bytelen);
 
 		if (NLMSG_ALIGN(f.flushp) + req2.n.nlmsg_len > f.flushe) {
 			if (flush_update())
@@ -283,12 +293,14 @@ static int process_msg(const struct sockaddr_nl *who, struct nlmsghdr *n,
 static int tcpm_do_cmd(int cmd, int argc, char **argv)
 {
 	TCPM_REQUEST(req, 1024, TCP_METRICS_CMD_GET, NLM_F_REQUEST);
-	int atype = -1;
+	int atype = -1, stype = -1;
 	int ack;
 
 	memset(&f, 0, sizeof(f));
 	f.daddr.bitlen = -1;
 	f.daddr.family = preferred_family;
+	f.saddr.bitlen = -1;
+	f.saddr.family = preferred_family;
 
 	switch (preferred_family) {
 	case AF_UNSPEC:
@@ -301,31 +313,53 @@ static int tcpm_do_cmd(int cmd, int argc, char **argv)
 	}
 
 	for (; argc > 0; argc--, argv++) {
-		char *who = "address";
-
-		if (strcmp(*argv, "addr") == 0 ||
-		    strcmp(*argv, "address") == 0) {
-			who = *argv;
+		if (strcmp(*argv, "src") == 0 ||
+		    strcmp(*argv, "source") == 0) {
+			char *who = *argv;
 			NEXT_ARG();
-		}
-		if (matches(*argv, "help") == 0)
-			usage();
-		if (f.daddr.bitlen >= 0)
-			duparg2(who, *argv);
-
-		get_prefix(&f.daddr, *argv, preferred_family);
-		if (f.daddr.bytelen && f.daddr.bytelen * 8 == f.daddr.bitlen) {
-			if (f.daddr.family == AF_INET)
-				atype = TCP_METRICS_ATTR_ADDR_IPV4;
-			else if (f.daddr.family == AF_INET6)
-				atype = TCP_METRICS_ATTR_ADDR_IPV6;
-		}
-		if ((CMD_DEL & cmd) && atype < 0) {
-			fprintf(stderr, "Error: a specific IP address is expected rather than \"%s\"\n",
-				*argv);
-			return -1;
-		}
+			if (matches(*argv, "help") == 0)
+				usage();
+			if (f.saddr.bitlen >= 0)
+				duparg2(who, *argv);
+
+			get_prefix(&f.saddr, *argv, preferred_family);
+			if (f.saddr.bytelen && f.saddr.bytelen * 8 == f.saddr.bitlen) {
+				if (f.saddr.family == AF_INET)
+					stype = TCP_METRICS_ATTR_SADDR_IPV4;
+				else if (f.saddr.family == AF_INET6)
+					stype = TCP_METRICS_ATTR_SADDR_IPV6;
+			}
 
+			if (stype < 0) {
+				fprintf(stderr, "Error: a specific IP address is expected rather than \"%s\"\n",
+					*argv);
+				return -1;
+			}
+		} else {
+			char *who = "address";
+			if (strcmp(*argv, "addr") == 0 ||
+			    strcmp(*argv, "address") == 0) {
+				who = *argv;
+				NEXT_ARG();
+			}
+			if (matches(*argv, "help") == 0)
+				usage();
+			if (f.daddr.bitlen >= 0)
+				duparg2(who, *argv);
+
+			get_prefix(&f.daddr, *argv, preferred_family);
+			if (f.daddr.bytelen && f.daddr.bytelen * 8 == f.daddr.bitlen) {
+				if (f.daddr.family == AF_INET)
+					atype = TCP_METRICS_ATTR_ADDR_IPV4;
+				else if (f.daddr.family == AF_INET6)
+					atype = TCP_METRICS_ATTR_ADDR_IPV6;
+			}
+			if ((CMD_DEL & cmd) && atype < 0) {
+				fprintf(stderr, "Error: a specific IP address is expected rather than \"%s\"\n",
+					*argv);
+				return -1;
+			}
+		}
 		argc--; argv++;
 	}
 
@@ -338,7 +372,7 @@ static int tcpm_do_cmd(int cmd, int argc, char **argv)
 
 	/* flush for all addresses ? Single del without address */
 	if (cmd == CMD_FLUSH && f.daddr.bitlen <= 0 &&
-	    preferred_family == AF_UNSPEC) {
+	    f.saddr.bitlen <= 0 && preferred_family == AF_UNSPEC) {
 		cmd = CMD_DEL;
 		req.g.cmd = TCP_METRICS_CMD_DEL;
 		ack = 1;
@@ -367,6 +401,9 @@ static int tcpm_do_cmd(int cmd, int argc, char **argv)
 		if (atype >= 0)
 			addattr_l(&req.n, sizeof(req), atype, &f.daddr.data,
 				  f.daddr.bytelen);
+		if (stype >= 0)
+			addattr_l(&req.n, sizeof(req), stype, &f.saddr.data,
+				  f.saddr.bytelen);
 	} else {
 		req.n.nlmsg_flags |= NLM_F_DUMP;
 	}
-- 
1.8.3.2

^ permalink raw reply related

* [PATCH iproute2 2/3] tcp_metrics: Display source-address
From: Christoph Paasch @ 2014-02-06 22:15 UTC (permalink / raw)
  To: Stephen Hemminger; +Cc: netdev
In-Reply-To: <1391724904-30504-1-git-send-email-christoph.paasch@uclouvain.be>

This patch allows to display the source-IP.
stype will be used in the next patch that allows to remove based on the
source-IP.

Signed-off-by: Christoph Paasch <christoph.paasch@uclouvain.be>
---
 include/linux/tcp_metrics.h |  2 ++
 ip/tcp_metrics.c            | 30 ++++++++++++++++++++++++++++--
 2 files changed, 30 insertions(+), 2 deletions(-)

diff --git a/include/linux/tcp_metrics.h b/include/linux/tcp_metrics.h
index cb5157b55f32..54a37b13f2c4 100644
--- a/include/linux/tcp_metrics.h
+++ b/include/linux/tcp_metrics.h
@@ -35,6 +35,8 @@ enum {
 	TCP_METRICS_ATTR_FOPEN_SYN_DROPS,	/* u16, count of drops */
 	TCP_METRICS_ATTR_FOPEN_SYN_DROP_TS,	/* msecs age */
 	TCP_METRICS_ATTR_FOPEN_COOKIE,		/* binary */
+	TCP_METRICS_ATTR_SADDR_IPV4,		/* u32 */
+	TCP_METRICS_ATTR_SADDR_IPV6,		/* binary */
 
 	__TCP_METRICS_ATTR_MAX,
 };
diff --git a/ip/tcp_metrics.c b/ip/tcp_metrics.c
index 4b30771ae3a8..a7215c86379f 100644
--- a/ip/tcp_metrics.c
+++ b/ip/tcp_metrics.c
@@ -95,8 +95,8 @@ static int process_msg(const struct sockaddr_nl *who, struct nlmsghdr *n,
 	struct rtattr *attrs[TCP_METRICS_ATTR_MAX + 1], *a;
 	int len = n->nlmsg_len;
 	char abuf[256];
-	inet_prefix daddr;
-	int family, i, atype, dlen = 0;
+	inet_prefix daddr, saddr;
+	int family, i, atype, stype, dlen = 0, slen = 0;
 
 	if (n->nlmsg_type != genl_family)
 		return -1;
@@ -135,6 +135,26 @@ static int process_msg(const struct sockaddr_nl *who, struct nlmsghdr *n,
 			return 0;
 	}
 
+	a = attrs[TCP_METRICS_ATTR_SADDR_IPV4];
+	if (a) {
+		if (f.saddr.family && f.saddr.family != AF_INET)
+			return 0;
+		memcpy(&saddr.data, RTA_DATA(a), 4);
+		saddr.bytelen = 4;
+		stype = TCP_METRICS_ATTR_SADDR_IPV4;
+		slen = RTA_PAYLOAD(a);
+	} else {
+		a = attrs[TCP_METRICS_ATTR_SADDR_IPV6];
+		if (a) {
+			if (f.saddr.family && f.saddr.family != AF_INET6)
+				return 0;
+			memcpy(&saddr.data, RTA_DATA(a), 16);
+			saddr.bytelen = 16;
+			stype = TCP_METRICS_ATTR_SADDR_IPV6;
+			slen = RTA_PAYLOAD(a);
+		}
+	}
+
 	if (f.daddr.family && f.daddr.bitlen >= 0 &&
 	    inet_addr_match(&daddr, &f.daddr, f.daddr.bitlen))
 		return 0;
@@ -248,6 +268,12 @@ static int process_msg(const struct sockaddr_nl *who, struct nlmsghdr *n,
 		fprintf(fp, " fo_cookie %s", cookie);
 	}
 
+	if (slen) {
+		fprintf(fp, " source %s",
+			format_host(family, slen, &saddr.data, abuf,
+				    sizeof(abuf)));
+	}
+
 	fprintf(fp, "\n");
 
 	fflush(fp);
-- 
1.8.3.2

^ permalink raw reply related

* [PATCH iproute2 0/3] Support for tcp-metrics source address
From: Christoph Paasch @ 2014-02-06 22:15 UTC (permalink / raw)
  To: Stephen Hemminger; +Cc: netdev

This patchset implements support for showing and deleting tcp-metrics
based on the source-address.

Christoph Paasch (3):
  tcp_metrics: Rename addr to daddr and add local variable
  tcp_metrics: Display source-address
  tcp_metrics: Allow removal based on the source-IP

 include/linux/tcp_metrics.h |   2 +
 ip/tcp_metrics.c            | 154 +++++++++++++++++++++++++++++++-------------
 2 files changed, 111 insertions(+), 45 deletions(-)

-- 
1.8.3.2

^ permalink raw reply

* [PATCH iproute2 1/3] tcp_metrics: Rename addr to daddr and add local variable
From: Christoph Paasch @ 2014-02-06 22:15 UTC (permalink / raw)
  To: Stephen Hemminger; +Cc: netdev
In-Reply-To: <1391724904-30504-1-git-send-email-christoph.paasch@uclouvain.be>

Renaming addr to daddr, because we will introduce saddr later.

The local variable is necessary to store RTA_PAYLOAD(a) temporarily.

Signed-off-by: Christoph Paasch <christoph.paasch@uclouvain.be>
---
 ip/tcp_metrics.c | 51 ++++++++++++++++++++++++++-------------------------
 1 file changed, 26 insertions(+), 25 deletions(-)

diff --git a/ip/tcp_metrics.c b/ip/tcp_metrics.c
index c6be3c94415f..4b30771ae3a8 100644
--- a/ip/tcp_metrics.c
+++ b/ip/tcp_metrics.c
@@ -74,7 +74,7 @@ static struct
 	int flushp;
 	int flushe;
 	int cmd;
-	inet_prefix addr;
+	inet_prefix daddr;
 } f;
 
 static int flush_update(void)
@@ -95,8 +95,8 @@ static int process_msg(const struct sockaddr_nl *who, struct nlmsghdr *n,
 	struct rtattr *attrs[TCP_METRICS_ATTR_MAX + 1], *a;
 	int len = n->nlmsg_len;
 	char abuf[256];
-	inet_prefix addr;
-	int family, i, atype;
+	inet_prefix daddr;
+	int family, i, atype, dlen = 0;
 
 	if (n->nlmsg_type != genl_family)
 		return -1;
@@ -114,35 +114,37 @@ static int process_msg(const struct sockaddr_nl *who, struct nlmsghdr *n,
 
 	a = attrs[TCP_METRICS_ATTR_ADDR_IPV4];
 	if (a) {
-		if (f.addr.family && f.addr.family != AF_INET)
+		if (f.daddr.family && f.daddr.family != AF_INET)
 			return 0;
-		memcpy(&addr.data, RTA_DATA(a), 4);
-		addr.bytelen = 4;
+		memcpy(&daddr.data, RTA_DATA(a), 4);
+		daddr.bytelen = 4;
 		family = AF_INET;
 		atype = TCP_METRICS_ATTR_ADDR_IPV4;
+		dlen = RTA_PAYLOAD(a);
 	} else {
 		a = attrs[TCP_METRICS_ATTR_ADDR_IPV6];
 		if (a) {
-			if (f.addr.family && f.addr.family != AF_INET6)
+			if (f.daddr.family && f.daddr.family != AF_INET6)
 				return 0;
-			memcpy(&addr.data, RTA_DATA(a), 16);
-			addr.bytelen = 16;
+			memcpy(&daddr.data, RTA_DATA(a), 16);
+			daddr.bytelen = 16;
 			family = AF_INET6;
 			atype = TCP_METRICS_ATTR_ADDR_IPV6;
+			dlen = RTA_PAYLOAD(a);
 		} else
 			return 0;
 	}
 
-	if (f.addr.family && f.addr.bitlen >= 0 &&
-	    inet_addr_match(&addr, &f.addr, f.addr.bitlen))
+	if (f.daddr.family && f.daddr.bitlen >= 0 &&
+	    inet_addr_match(&daddr, &f.daddr, f.daddr.bitlen))
 		return 0;
 
 	if (f.flushb) {
 		struct nlmsghdr *fn;
 		TCPM_REQUEST(req2, 128, TCP_METRICS_CMD_DEL, NLM_F_REQUEST);
 
-		addattr_l(&req2.n, sizeof(req2), atype, &addr.data,
-			  addr.bytelen);
+		addattr_l(&req2.n, sizeof(req2), atype, &daddr.data,
+			  daddr.bytelen);
 
 		if (NLMSG_ALIGN(f.flushp) + req2.n.nlmsg_len > f.flushe) {
 			if (flush_update())
@@ -161,8 +163,7 @@ static int process_msg(const struct sockaddr_nl *who, struct nlmsghdr *n,
 		fprintf(fp, "Deleted ");
 
 	fprintf(fp, "%s",
-		format_host(family, RTA_PAYLOAD(a), &addr.data,
-			    abuf, sizeof(abuf)));
+		format_host(family, dlen, &daddr.data, abuf, sizeof(abuf)));
 
 	a = attrs[TCP_METRICS_ATTR_AGE];
 	if (a) {
@@ -260,8 +261,8 @@ static int tcpm_do_cmd(int cmd, int argc, char **argv)
 	int ack;
 
 	memset(&f, 0, sizeof(f));
-	f.addr.bitlen = -1;
-	f.addr.family = preferred_family;
+	f.daddr.bitlen = -1;
+	f.daddr.family = preferred_family;
 
 	switch (preferred_family) {
 	case AF_UNSPEC:
@@ -283,14 +284,14 @@ static int tcpm_do_cmd(int cmd, int argc, char **argv)
 		}
 		if (matches(*argv, "help") == 0)
 			usage();
-		if (f.addr.bitlen >= 0)
+		if (f.daddr.bitlen >= 0)
 			duparg2(who, *argv);
 
-		get_prefix(&f.addr, *argv, preferred_family);
-		if (f.addr.bytelen && f.addr.bytelen * 8 == f.addr.bitlen) {
-			if (f.addr.family == AF_INET)
+		get_prefix(&f.daddr, *argv, preferred_family);
+		if (f.daddr.bytelen && f.daddr.bytelen * 8 == f.daddr.bitlen) {
+			if (f.daddr.family == AF_INET)
 				atype = TCP_METRICS_ATTR_ADDR_IPV4;
-			else if (f.addr.family == AF_INET6)
+			else if (f.daddr.family == AF_INET6)
 				atype = TCP_METRICS_ATTR_ADDR_IPV6;
 		}
 		if ((CMD_DEL & cmd) && atype < 0) {
@@ -310,7 +311,7 @@ static int tcpm_do_cmd(int cmd, int argc, char **argv)
 		cmd = CMD_DEL;
 
 	/* flush for all addresses ? Single del without address */
-	if (cmd == CMD_FLUSH && f.addr.bitlen <= 0 &&
+	if (cmd == CMD_FLUSH && f.daddr.bitlen <= 0 &&
 	    preferred_family == AF_UNSPEC) {
 		cmd = CMD_DEL;
 		req.g.cmd = TCP_METRICS_CMD_DEL;
@@ -338,8 +339,8 @@ static int tcpm_do_cmd(int cmd, int argc, char **argv)
 		if (ack)
 			req.n.nlmsg_flags |= NLM_F_ACK;
 		if (atype >= 0)
-			addattr_l(&req.n, sizeof(req), atype, &f.addr.data,
-				  f.addr.bytelen);
+			addattr_l(&req.n, sizeof(req), atype, &f.daddr.data,
+				  f.daddr.bytelen);
 	} else {
 		req.n.nlmsg_flags |= NLM_F_DUMP;
 	}
-- 
1.8.3.2

^ permalink raw reply related

* [PATCH net] tg3: Fix deadlock in tg3_change_mtu()
From: Nithin Nayak Sujir @ 2014-02-06 22:13 UTC (permalink / raw)
  To: davem; +Cc: netdev, david.vrabel, Nithin Nayak Sujir, stable, Michael Chan

Quoting David Vrabel -
"5780 cards cannot have jumbo frames and TSO enabled together.  When
jumbo frames are enabled by setting the MTU, the TSO feature must be
cleared.  This is done indirectly by calling netdev_update_features()
which will call tg3_fix_features() to actually clear the flags.

netdev_update_features() will also trigger a new netlink message for the
feature change event which will result in a call to tg3_get_stats64()
which deadlocks on the tg3 lock."

tg3_set_mtu() does not need to be under the tg3 lock since converting
the flags to use set_bit(). Move it out to after tg3_netif_stop().

Cc: <stable@vger.kernel.org>
Reported-by: David Vrabel <david.vrabel@citrix.com>
Tested-by: David Vrabel <david.vrabel@citrix.com>
Signed-off-by: Michael Chan <mchan@broadcom.com>
Signed-off-by: Nithin Nayak Sujir <nsujir@broadcom.com>
---
 drivers/net/ethernet/broadcom/tg3.c |    4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/net/ethernet/broadcom/tg3.c b/drivers/net/ethernet/broadcom/tg3.c
index e2ca03e..0bb79b8 100644
--- a/drivers/net/ethernet/broadcom/tg3.c
+++ b/drivers/net/ethernet/broadcom/tg3.c
@@ -14113,12 +14113,12 @@ static int tg3_change_mtu(struct net_device *dev, int new_mtu)
 
 	tg3_netif_stop(tp);
 
+	tg3_set_mtu(dev, tp, new_mtu);
+
 	tg3_full_lock(tp, 1);
 
 	tg3_halt(tp, RESET_KIND_SHUTDOWN, 1);
 
-	tg3_set_mtu(dev, tp, new_mtu);
-
 	/* Reset PHY, otherwise the read DMA engine will be in a mode that
 	 * breaks all requests to 256 bytes.
 	 */
-- 
1.7.9.5

^ permalink raw reply related

* Re: RTNL: assertion failed at net/core/dev.c (4494) and RTNL: assertion failed at net/core/rtnetlink.c (940)
From: Cong Wang @ 2014-02-06 22:12 UTC (permalink / raw)
  To: Jay Vosburgh
  Cc: Thomas Glanzmann, Eric Dumazet, netdev, Veaceslav Falico, andy,
	Jiří Pírko
In-Reply-To: <31272.1391724462@death.nxdomain>

On Thu, Feb 6, 2014 at 2:07 PM, Jay Vosburgh <fubar@us.ibm.com> wrote:
> Jay Vosburgh <fubar@us.ibm.com> wrote:
>
>>Cong Wang <cwang@twopensource.com> wrote:
>>
>>
>>       That would eliminate the warning, but is suboptimal.  Acquiring
>>RTNL is not necessary on the vast majority of state machine runs
>>(because no state changes take place, i.e., no ports are disabled or
>>enabled).  The above change would add 10 round trips per second to RTNL,
>>which seems excessive.
>>
>>       Also, we cannot unconditionally acquire RTNL in this function,
>>as it would race with the call to cancel_delayed_work_sync from
>>bond_close (via bond_work_cancel_all).

OK.

>
>         Thought of one more problem: we can't hold a regular lock while
> calling rtmsg_ifinfo, as it may sleep in alloc_skb.  The rtmsg_ifinfo
> call has to be RTNL and nothing else.
>

s/GFP_KERNEL/GFP_ATOMIC/

^ permalink raw reply

* Re: [PATCH net] netpoll: fix netconsole IPv6 setup
From: Cong Wang @ 2014-02-06 22:09 UTC (permalink / raw)
  To: Sabrina Dubroca; +Cc: David Miller, netdev
In-Reply-To: <20140206205850.GA12704@kria>

On Thu, Feb 6, 2014 at 12:58 PM, Sabrina Dubroca <sd@queasysnail.net> wrote:
> 2014-02-06, 12:34:10 -0800, Cong Wang wrote:
>> On Thu, Feb 6, 2014 at 9:34 AM, Sabrina Dubroca <sd@queasysnail.net> wrote:
>> >  net/core/netpoll.c | 4 +++-
>> >  1 file changed, 3 insertions(+), 1 deletion(-)
>> >
>> > diff --git a/net/core/netpoll.c b/net/core/netpoll.c
>> > index c03f3de..a664f78 100644
>> > --- a/net/core/netpoll.c
>> > +++ b/net/core/netpoll.c
>> > @@ -948,6 +948,7 @@ int netpoll_parse_options(struct netpoll *np, char *opt)
>> >  {
>> >         char *cur=opt, *delim;
>> >         int ipv6;
>> > +       bool ipversion_set = false;
>> >
>>
>> Or initialize 'ipv6' to -1 and then check if it is -1?
>
> It's overwritten when we parse the remote address. And np->ipv6 is a
> bool, so we can't store it there either.
>


Yeah, I misunderstood the problem. Your fix looks good.

Acked-by: Cong Wang <cwang@twopensource.com>

For net-next, I think we should change bool np->ipv6 to int np->ip_version,
so that we can tell if it is set or not.

Thanks!

^ permalink raw reply

* Re: RTNL: assertion failed at net/core/dev.c (4494) and RTNL: assertion failed at net/core/rtnetlink.c (940)
From: Jay Vosburgh @ 2014-02-06 22:07 UTC (permalink / raw)
  To: Cong Wang
  Cc: Thomas Glanzmann, Eric Dumazet, netdev, Veaceslav Falico, andy,
	Jiří Pírko
In-Reply-To: <30988.1391723318@death.nxdomain>

Jay Vosburgh <fubar@us.ibm.com> wrote:

>Cong Wang <cwang@twopensource.com> wrote:
>
>>On Thu, Feb 6, 2014 at 12:51 PM, Thomas Glanzmann <thomas@glanzmann.de> wrote:
>>> Hello,
>>> this morning I checked out Linus tip and compiled it after booting my
>>> dmesg is full of:
>>>
>>> [    8.944991] RTNL: assertion failed at net/core/dev.c (4494)
>>> [    8.950640] CPU: 3 PID: 388 Comm: kworker/u24:4 Not tainted 3.14.0-rc1+ #3
>>> [    8.950642] Hardware name: Supermicro X9SRD-F/X9SRD-F, BIOS 1.0a 10/15/2012
>>> [    8.950654] Workqueue: bond0 bond_3ad_state_machine_handler [bonding]
>>> [    8.950658]  0000000000000000 ffff881020c88000 ffffffff8138e219 ffff881020c88000
>>> [    8.950664]  ffffffff812d3091 ffff881023961040 ffffffff812e3132 0000000000000246
>>> [    8.950670]  0000000000000020 ffff881020ab1be8 0000000020ab1ba8 0000000000000000
>>> [    8.950675] Call Trace:
>>> [    8.950686]  [<ffffffff8138e219>] ? dump_stack+0x41/0x51
>>> [    8.950694]  [<ffffffff812d3091>] ? netdev_master_upper_dev_get+0x2a/0x4d
>>> [    8.950699]  [<ffffffff812e3132>] ? rtnl_fill_ifinfo+0x2c/0xac4
>>> [    8.950707]  [<ffffffff81072211>] ? print_time.part.5+0x50/0x54
>>> [    8.950715]  [<ffffffff812caf94>] ? __kmalloc_reserve.isra.42+0x2a/0x6d
>>> [    8.950721]  [<ffffffff81102040>] ? ksize+0x12/0x1e
>>> [    8.950726]  [<ffffffff812cb2b7>] ? __alloc_skb+0xb5/0x1a9
>>> [    8.950731]  [<ffffffff812e4626>] ? rtmsg_ifinfo+0x6c/0xd6
>>> [    8.950739]  [<ffffffffa035f4f9>] ? __enable_port.isra.17+0x51/0x5a [bonding]
>>> [    8.950747]  [<ffffffffa0360463>] ? ad_agg_selection_logic+0x3d3/0x3ed [bonding]
>>> [    8.950754]  [<ffffffffa0360d40>] ? bond_3ad_state_machine_handler+0x555/0x918 [bonding]
>>> [    8.950761]  [<ffffffff8104db2d>] ? process_one_work+0x191/0x293
>>> [    8.950766]  [<ffffffff8104dfde>] ? worker_thread+0x121/0x1e7
>>> [    8.950770]  [<ffffffff8104debd>] ? rescuer_thread+0x269/0x269
>>> [    8.950777]  [<ffffffff810527b6>] ? kthread+0x99/0xa1
>>> [    8.950782]  [<ffffffff8105271d>] ? __kthread_parkme+0x59/0x59
>>> [    8.950789]  [<ffffffff8139733c>] ? ret_from_fork+0x7c/0xb0
>>> [    8.950794]  [<ffffffff8105271d>] ? __kthread_parkme+0x59/0x59
>>
>>
>>Hmm, rtmsg_ifinfo() should be called with rtnl lock, but
>>__enable_port() is called
>>with rcu_read_lock() which means we can't block inside it, therefore we probably
>>should take rtnl lock outside:
>>
>>diff --git a/drivers/net/bonding/bond_3ad.c b/drivers/net/bonding/bond_3ad.c
>>index cce1f1b..3c09ffa 100644
>>--- a/drivers/net/bonding/bond_3ad.c
>>+++ b/drivers/net/bonding/bond_3ad.c
>>@@ -2065,6 +2065,7 @@ void bond_3ad_state_machine_handler(struct
>>work_struct *work)
>>        struct slave *slave;
>>        struct port *port;
>>
>>+       rtnl_lock();
>>        read_lock(&bond->lock);
>>        rcu_read_lock();
>>
>>@@ -2123,6 +2124,7 @@ void bond_3ad_state_machine_handler(struct
>>work_struct *work)
>> re_arm:
>>        rcu_read_unlock();
>>        read_unlock(&bond->lock);
>>+       rtnl_unlock();
>>        queue_delayed_work(bond->wq, &bond->ad_work, ad_delta_in_ticks);
>> }
>
>	That would eliminate the warning, but is suboptimal.  Acquiring
>RTNL is not necessary on the vast majority of state machine runs
>(because no state changes take place, i.e., no ports are disabled or
>enabled).  The above change would add 10 round trips per second to RTNL,
>which seems excessive.
>
>	Also, we cannot unconditionally acquire RTNL in this function,
>as it would race with the call to cancel_delayed_work_sync from
>bond_close (via bond_work_cancel_all).

	Thought of one more problem: we can't hold a regular lock while
calling rtmsg_ifinfo, as it may sleep in alloc_skb.  The rtmsg_ifinfo
call has to be RTNL and nothing else.

	-J

---
	-Jay Vosburgh, IBM Linux Technology Center, fubar@us.ibm.com

^ permalink raw reply

* Re: [PATCH] inet: defines IPPROTO_* needed for module alias generation
From: Cong Wang @ 2014-02-06 21:51 UTC (permalink / raw)
  To: Carlos O'Donell
  Cc: Jan Moskyto Matejka, David S. Miller, netdev,
	linux-kernel@vger.kernel.org
In-Reply-To: <52F3FFBC.1010505@redhat.com>

On Thu, Feb 6, 2014 at 1:33 PM, Carlos O'Donell <carlos@redhat.com> wrote:
> On 02/06/2014 06:10 AM, Jan Moskyto Matejka wrote:
>> Reverting this change back to define's to fix the aliases.
>>
>> modinfo mip6 (before this change)
>> alias:          xfrm-type-10-IPPROTO_DSTOPTS
>> alias:          xfrm-type-10-IPPROTO_ROUTING
>>
>> modinfo mip6 (after this change)
>> alias:          xfrm-type-10-43
>> alias:          xfrm-type-10-60
>
> Instead of reverting these changes I suggest someone fix
> whatever is processing that information.

It is stringfy by c preprocessor. enum should be processed
after preprocessing, therefore fails to be converted to it
real value at preprocessing stage.

^ permalink raw reply

* Re: RTNL: assertion failed at net/core/dev.c (4494) and RTNL: assertion failed at net/core/rtnetlink.c (940)
From: Jay Vosburgh @ 2014-02-06 21:48 UTC (permalink / raw)
  To: Cong Wang
  Cc: Thomas Glanzmann, Eric Dumazet, netdev, Veaceslav Falico, andy,
	Jiří Pírko
In-Reply-To: <CAHA+R7OGS_KG1un36KBixbgs7LTPWGRYdJ+Vsn2iM0FzNV5FzA@mail.gmail.com>

Cong Wang <cwang@twopensource.com> wrote:

>On Thu, Feb 6, 2014 at 12:51 PM, Thomas Glanzmann <thomas@glanzmann.de> wrote:
>> Hello,
>> this morning I checked out Linus tip and compiled it after booting my
>> dmesg is full of:
>>
>> [    8.944991] RTNL: assertion failed at net/core/dev.c (4494)
>> [    8.950640] CPU: 3 PID: 388 Comm: kworker/u24:4 Not tainted 3.14.0-rc1+ #3
>> [    8.950642] Hardware name: Supermicro X9SRD-F/X9SRD-F, BIOS 1.0a 10/15/2012
>> [    8.950654] Workqueue: bond0 bond_3ad_state_machine_handler [bonding]
>> [    8.950658]  0000000000000000 ffff881020c88000 ffffffff8138e219 ffff881020c88000
>> [    8.950664]  ffffffff812d3091 ffff881023961040 ffffffff812e3132 0000000000000246
>> [    8.950670]  0000000000000020 ffff881020ab1be8 0000000020ab1ba8 0000000000000000
>> [    8.950675] Call Trace:
>> [    8.950686]  [<ffffffff8138e219>] ? dump_stack+0x41/0x51
>> [    8.950694]  [<ffffffff812d3091>] ? netdev_master_upper_dev_get+0x2a/0x4d
>> [    8.950699]  [<ffffffff812e3132>] ? rtnl_fill_ifinfo+0x2c/0xac4
>> [    8.950707]  [<ffffffff81072211>] ? print_time.part.5+0x50/0x54
>> [    8.950715]  [<ffffffff812caf94>] ? __kmalloc_reserve.isra.42+0x2a/0x6d
>> [    8.950721]  [<ffffffff81102040>] ? ksize+0x12/0x1e
>> [    8.950726]  [<ffffffff812cb2b7>] ? __alloc_skb+0xb5/0x1a9
>> [    8.950731]  [<ffffffff812e4626>] ? rtmsg_ifinfo+0x6c/0xd6
>> [    8.950739]  [<ffffffffa035f4f9>] ? __enable_port.isra.17+0x51/0x5a [bonding]
>> [    8.950747]  [<ffffffffa0360463>] ? ad_agg_selection_logic+0x3d3/0x3ed [bonding]
>> [    8.950754]  [<ffffffffa0360d40>] ? bond_3ad_state_machine_handler+0x555/0x918 [bonding]
>> [    8.950761]  [<ffffffff8104db2d>] ? process_one_work+0x191/0x293
>> [    8.950766]  [<ffffffff8104dfde>] ? worker_thread+0x121/0x1e7
>> [    8.950770]  [<ffffffff8104debd>] ? rescuer_thread+0x269/0x269
>> [    8.950777]  [<ffffffff810527b6>] ? kthread+0x99/0xa1
>> [    8.950782]  [<ffffffff8105271d>] ? __kthread_parkme+0x59/0x59
>> [    8.950789]  [<ffffffff8139733c>] ? ret_from_fork+0x7c/0xb0
>> [    8.950794]  [<ffffffff8105271d>] ? __kthread_parkme+0x59/0x59
>
>
>Hmm, rtmsg_ifinfo() should be called with rtnl lock, but
>__enable_port() is called
>with rcu_read_lock() which means we can't block inside it, therefore we probably
>should take rtnl lock outside:
>
>diff --git a/drivers/net/bonding/bond_3ad.c b/drivers/net/bonding/bond_3ad.c
>index cce1f1b..3c09ffa 100644
>--- a/drivers/net/bonding/bond_3ad.c
>+++ b/drivers/net/bonding/bond_3ad.c
>@@ -2065,6 +2065,7 @@ void bond_3ad_state_machine_handler(struct
>work_struct *work)
>        struct slave *slave;
>        struct port *port;
>
>+       rtnl_lock();
>        read_lock(&bond->lock);
>        rcu_read_lock();
>
>@@ -2123,6 +2124,7 @@ void bond_3ad_state_machine_handler(struct
>work_struct *work)
> re_arm:
>        rcu_read_unlock();
>        read_unlock(&bond->lock);
>+       rtnl_unlock();
>        queue_delayed_work(bond->wq, &bond->ad_work, ad_delta_in_ticks);
> }

	That would eliminate the warning, but is suboptimal.  Acquiring
RTNL is not necessary on the vast majority of state machine runs
(because no state changes take place, i.e., no ports are disabled or
enabled).  The above change would add 10 round trips per second to RTNL,
which seems excessive.

	Also, we cannot unconditionally acquire RTNL in this function,
as it would race with the call to cancel_delayed_work_sync from
bond_close (via bond_work_cancel_all).

	-J

---
	-Jay Vosburgh, IBM Linux Technology Center, fubar@us.ibm.com

^ permalink raw reply

* Re: RTNL: assertion failed at net/core/dev.c (4494) and RTNL: assertion failed at net/core/rtnetlink.c (940)
From: Jay Vosburgh @ 2014-02-06 21:45 UTC (permalink / raw)
  To: Thomas Glanzmann; +Cc: Eric Dumazet, netdev, vfalico, andy, jiri
In-Reply-To: <20140206205106.GA10488@glanzmann.de>

Thomas Glanzmann <thomas@glanzmann.de> wrote:

>Hello,
>this morning I checked out Linus tip and compiled it after booting my
>dmesg is full of:
>
>[    8.944991] RTNL: assertion failed at net/core/dev.c (4494)
>[    8.950640] CPU: 3 PID: 388 Comm: kworker/u24:4 Not tainted 3.14.0-rc1+ #3
>[    8.950642] Hardware name: Supermicro X9SRD-F/X9SRD-F, BIOS 1.0a 10/15/2012
>[    8.950654] Workqueue: bond0 bond_3ad_state_machine_handler [bonding]
>[    8.950658]  0000000000000000 ffff881020c88000 ffffffff8138e219 ffff881020c88000
>[    8.950664]  ffffffff812d3091 ffff881023961040 ffffffff812e3132 0000000000000246
>[    8.950670]  0000000000000020 ffff881020ab1be8 0000000020ab1ba8 0000000000000000
>[    8.950675] Call Trace:
>[    8.950686]  [<ffffffff8138e219>] ? dump_stack+0x41/0x51
>[    8.950694]  [<ffffffff812d3091>] ? netdev_master_upper_dev_get+0x2a/0x4d
>[    8.950699]  [<ffffffff812e3132>] ? rtnl_fill_ifinfo+0x2c/0xac4
>[    8.950707]  [<ffffffff81072211>] ? print_time.part.5+0x50/0x54
>[    8.950715]  [<ffffffff812caf94>] ? __kmalloc_reserve.isra.42+0x2a/0x6d
>[    8.950721]  [<ffffffff81102040>] ? ksize+0x12/0x1e
>[    8.950726]  [<ffffffff812cb2b7>] ? __alloc_skb+0xb5/0x1a9
>[    8.950731]  [<ffffffff812e4626>] ? rtmsg_ifinfo+0x6c/0xd6
>[    8.950739]  [<ffffffffa035f4f9>] ? __enable_port.isra.17+0x51/0x5a [bonding]
>[    8.950747]  [<ffffffffa0360463>] ? ad_agg_selection_logic+0x3d3/0x3ed [bonding]
>[    8.950754]  [<ffffffffa0360d40>] ? bond_3ad_state_machine_handler+0x555/0x918 [bonding]

	This was apparently introduced by commit:

commit 1d3ee88ae0d605629bf369ab0b868dae8ca62a48
Author: sfeldma@cumulusnetworks.com <sfeldma@cumulusnetworks.com>
Date:   Thu Jan 16 22:57:56 2014 -0800

    bonding: add netlink attributes to slave link dev
    

	which adds an "rtmsg_ifinfo" call to bond_set_active_slave and
bond_set_backup_slave, both of which are invoked in several places in
the 802.3ad code without RTNL held (via __enable_port and
__disable_port).

	Still looking into a fix.

	-J

---
	-Jay Vosburgh, IBM Linux Technology Center, fubar@us.ibm.com

^ permalink raw reply

* Re: RTNL: assertion failed at net/core/dev.c (4494) and RTNL: assertion failed at net/core/rtnetlink.c (940)
From: Cong Wang @ 2014-02-06 21:40 UTC (permalink / raw)
  To: Thomas Glanzmann
  Cc: Eric Dumazet, netdev, fubar, Veaceslav Falico, andy,
	Jiří Pírko
In-Reply-To: <20140206205106.GA10488@glanzmann.de>

On Thu, Feb 6, 2014 at 12:51 PM, Thomas Glanzmann <thomas@glanzmann.de> wrote:
> Hello,
> this morning I checked out Linus tip and compiled it after booting my
> dmesg is full of:
>
> [    8.944991] RTNL: assertion failed at net/core/dev.c (4494)
> [    8.950640] CPU: 3 PID: 388 Comm: kworker/u24:4 Not tainted 3.14.0-rc1+ #3
> [    8.950642] Hardware name: Supermicro X9SRD-F/X9SRD-F, BIOS 1.0a 10/15/2012
> [    8.950654] Workqueue: bond0 bond_3ad_state_machine_handler [bonding]
> [    8.950658]  0000000000000000 ffff881020c88000 ffffffff8138e219 ffff881020c88000
> [    8.950664]  ffffffff812d3091 ffff881023961040 ffffffff812e3132 0000000000000246
> [    8.950670]  0000000000000020 ffff881020ab1be8 0000000020ab1ba8 0000000000000000
> [    8.950675] Call Trace:
> [    8.950686]  [<ffffffff8138e219>] ? dump_stack+0x41/0x51
> [    8.950694]  [<ffffffff812d3091>] ? netdev_master_upper_dev_get+0x2a/0x4d
> [    8.950699]  [<ffffffff812e3132>] ? rtnl_fill_ifinfo+0x2c/0xac4
> [    8.950707]  [<ffffffff81072211>] ? print_time.part.5+0x50/0x54
> [    8.950715]  [<ffffffff812caf94>] ? __kmalloc_reserve.isra.42+0x2a/0x6d
> [    8.950721]  [<ffffffff81102040>] ? ksize+0x12/0x1e
> [    8.950726]  [<ffffffff812cb2b7>] ? __alloc_skb+0xb5/0x1a9
> [    8.950731]  [<ffffffff812e4626>] ? rtmsg_ifinfo+0x6c/0xd6
> [    8.950739]  [<ffffffffa035f4f9>] ? __enable_port.isra.17+0x51/0x5a [bonding]
> [    8.950747]  [<ffffffffa0360463>] ? ad_agg_selection_logic+0x3d3/0x3ed [bonding]
> [    8.950754]  [<ffffffffa0360d40>] ? bond_3ad_state_machine_handler+0x555/0x918 [bonding]
> [    8.950761]  [<ffffffff8104db2d>] ? process_one_work+0x191/0x293
> [    8.950766]  [<ffffffff8104dfde>] ? worker_thread+0x121/0x1e7
> [    8.950770]  [<ffffffff8104debd>] ? rescuer_thread+0x269/0x269
> [    8.950777]  [<ffffffff810527b6>] ? kthread+0x99/0xa1
> [    8.950782]  [<ffffffff8105271d>] ? __kthread_parkme+0x59/0x59
> [    8.950789]  [<ffffffff8139733c>] ? ret_from_fork+0x7c/0xb0
> [    8.950794]  [<ffffffff8105271d>] ? __kthread_parkme+0x59/0x59


Hmm, rtmsg_ifinfo() should be called with rtnl lock, but
__enable_port() is called
with rcu_read_lock() which means we can't block inside it, therefore we probably
should take rtnl lock outside:

diff --git a/drivers/net/bonding/bond_3ad.c b/drivers/net/bonding/bond_3ad.c
index cce1f1b..3c09ffa 100644
--- a/drivers/net/bonding/bond_3ad.c
+++ b/drivers/net/bonding/bond_3ad.c
@@ -2065,6 +2065,7 @@ void bond_3ad_state_machine_handler(struct
work_struct *work)
        struct slave *slave;
        struct port *port;

+       rtnl_lock();
        read_lock(&bond->lock);
        rcu_read_lock();

@@ -2123,6 +2124,7 @@ void bond_3ad_state_machine_handler(struct
work_struct *work)
 re_arm:
        rcu_read_unlock();
        read_unlock(&bond->lock);
+       rtnl_unlock();
        queue_delayed_work(bond->wq, &bond->ad_work, ad_delta_in_ticks);
 }

^ permalink raw reply related

* Re: [PATCH] net: use __GFP_NORETRY for high order allocations
From: David Rientjes @ 2014-02-06 21:39 UTC (permalink / raw)
  To: Joe Perches
  Cc: Eric Dumazet, David Miller, netdev, linux-kernel@vger.kernel.org
In-Reply-To: <1391722444.15777.28.camel@joe-AO722>

On Thu, 6 Feb 2014, Joe Perches wrote:

> > If they absolutely require local memory to 
> > currnet's cpu node then that would make sense,
> 
> I presumed THISNODE would be used only with NORETRY
> 

Unfortunately, that would avoid attempting to defragment or reclaim 
remotely for the order-3 allocation and only allocate remotely for 
order-0.

So we have to make a decision here what is more important: allocating 
high-order memory or local memory to current's cpu at this given time?

The policy of transparent hugepages, for example, is always to allocate 
order-9 memory remotely instead of falling back to allocating a smallpage 
locally.

The original code never cared about local vs remote memory, so I don't 
think there's any restriction to allocating remotely.  I also don't know 
many allocators that do __GFP_THISNODE and care about local memory to 
current's cpu at this point in time, usually users of that flag are 
allocating memory that must be local to a specific node (for things like 
kmem_cache_alloc_node()).

^ permalink raw reply

* Re: [PATCH] net: use __GFP_NORETRY for high order allocations
From: Joe Perches @ 2014-02-06 21:34 UTC (permalink / raw)
  To: David Rientjes
  Cc: Eric Dumazet, David Miller, netdev, linux-kernel@vger.kernel.org
In-Reply-To: <alpine.DEB.2.02.1402061302020.9567@chino.kir.corp.google.com>

On Thu, 2014-02-06 at 13:03 -0800, David Rientjes wrote:
> On Thu, 6 Feb 2014, Joe Perches wrote:
> 
> > On Thu, 2014-02-06 at 10:42 -0800, Eric Dumazet wrote:
> > > sock_alloc_send_pskb() & sk_page_frag_refill()
> > > have a loop trying high order allocations to prepare
> > > skb with low number of fragments as this increases performance.
> > > 
> > > Problem is that under memory pressure/fragmentation, this can
> > > trigger OOM while the intent was only to try the high order
> > > allocations, then fallback to order-0 allocations.
> > []
> > > Call Trace:
> > >  [<ffffffff8043766c>] dump_header+0xe1/0x23e
> > >  [<ffffffff80437a02>] oom_kill_process+0x6a/0x323
> > >  [<ffffffff80438443>] out_of_memory+0x4b3/0x50d
> > >  [<ffffffff8043a4a6>] __alloc_pages_may_oom+0xa2/0xc7
> > >  [<ffffffff80236f42>] __alloc_pages_nodemask+0x1002/0x17f0
> > >  [<ffffffff8024bd23>] alloc_pages_current+0x103/0x2b0
> > >  [<ffffffff8028567f>] sk_page_frag_refill+0x8f/0x160
> > []
> > > diff --git a/net/core/sock.c b/net/core/sock.c
> > []
> > > @@ -1775,7 +1775,9 @@ struct sk_buff *sock_alloc_send_pskb(struct sock *sk, unsigned long header_len,
> > >  			while (order) {
> > >  				if (npages >= 1 << order) {
> > >  					page = alloc_pages(sk->sk_allocation |
> > > -							   __GFP_COMP | __GFP_NOWARN,
> > > +							   __GFP_COMP |
> > > +							   __GFP_NOWARN |
> > > +							   __GFP_NORETRY,
> > >  							   order);
> > >  					if (page)
> > >  						goto fill_page;
> > > @@ -1845,7 +1847,7 @@ bool skb_page_frag_refill(unsigned int sz, struct page_frag *pfrag, gfp_t prio)
> > >  		gfp_t gfp = prio;
> > >  
> > >  		if (order)
> > > -			gfp |= __GFP_COMP | __GFP_NOWARN;
> > > +			gfp |= __GFP_COMP | __GFP_NOWARN | __GFP_NORETRY;
> > 
> > Perhaps add __GFP_THISNODE too ?
> > 
> 
> How does __GFP_THISNODE have anything to do with avoiding oom killing due 
> to high-order fragmentation?

I don't think it does.

> If they absolutely require local memory to 
> currnet's cpu node then that would make sense,

I presumed THISNODE would be used only with NORETRY

> but the fallback still 
> allocates order-0 memory remotely and with __GFP_THISNODE on this attempt 
> we wouldn't even attempt remote reclaim.
any other alloc attempt could work on other cpus.

It was just a thought, ignore it if it's a dumb thought.

^ permalink raw reply

* Re: [PATCH] inet: defines IPPROTO_* needed for module alias generation
From: Carlos O'Donell @ 2014-02-06 21:33 UTC (permalink / raw)
  To: Jan Moskyto Matejka, David S. Miller; +Cc: netdev, linux-kernel
In-Reply-To: <1391685000-7346-1-git-send-email-mq@suse.cz>

On 02/06/2014 06:10 AM, Jan Moskyto Matejka wrote:
> Commit cfd280c91253 ("net: sync some IP headers with glibc") changed a set of
> define's to an enum (with no explanation why) which introduced a bug
> in module mip6 where aliases are generated using the IPPROTO_* defines;
> mip6 doesn't load if require_module called with the aliases from
> xfrm_get_type().

I wrote that code and I apologize for not giving a reason at
the time.

There are two reasons:

* It makes the debuginfo better and debugging easier via the enum.

* It harmonizes those headers with what is already in glibc.

Harmonizing this header with glibc makes it easier for userspace
to synchronize changes and perhaps eventually use the UAPI headers
directly.

> Reverting this change back to define's to fix the aliases.
> 
> modinfo mip6 (before this change)
> alias:          xfrm-type-10-IPPROTO_DSTOPTS
> alias:          xfrm-type-10-IPPROTO_ROUTING
> 
> modinfo mip6 (after this change)
> alias:          xfrm-type-10-43
> alias:          xfrm-type-10-60

Instead of reverting these changes I suggest someone fix
whatever is processing that information.

I do not condone the application of this patch for the
above two reasons. Though you might argue that I should
just make all debuggers and compilers better at dealing
with DW_at_macro_info/DW_MACINFO_* debug info... and 
you also would not be wrong.

I hope that answers your question.

> Signed-off-by: Jan Moskyto Matejka <mq@suse.cz>
> ---
>  include/uapi/linux/in6.h | 23 +++++++----------------
>  1 file changed, 7 insertions(+), 16 deletions(-)
> 
> diff --git a/include/uapi/linux/in6.h b/include/uapi/linux/in6.h
> index 633b93c..e9a1d2d97 100644
> --- a/include/uapi/linux/in6.h
> +++ b/include/uapi/linux/in6.h
> @@ -128,22 +128,13 @@ struct in6_flowlabel_req {
>   *	IPV6 extension headers
>   */
>  #if __UAPI_DEF_IPPROTO_V6
> -enum {
> -  IPPROTO_HOPOPTS = 0,		/* IPv6 hop-by-hop options      */
> -#define IPPROTO_HOPOPTS		IPPROTO_HOPOPTS
> -  IPPROTO_ROUTING = 43,		/* IPv6 routing header          */
> -#define IPPROTO_ROUTING		IPPROTO_ROUTING
> -  IPPROTO_FRAGMENT = 44,	/* IPv6 fragmentation header    */
> -#define IPPROTO_FRAGMENT	IPPROTO_FRAGMENT
> -  IPPROTO_ICMPV6 = 58,		/* ICMPv6                       */
> -#define IPPROTO_ICMPV6		IPPROTO_ICMPV6
> -  IPPROTO_NONE = 59,		/* IPv6 no next header          */
> -#define IPPROTO_NONE		IPPROTO_NONE
> -  IPPROTO_DSTOPTS = 60,		/* IPv6 destination options     */
> -#define IPPROTO_DSTOPTS		IPPROTO_DSTOPTS
> -  IPPROTO_MH = 135,		/* IPv6 mobility header         */
> -#define IPPROTO_MH		IPPROTO_MH
> -};
> +#define IPPROTO_HOPOPTS		0	/* IPv6 hop-by-hop options	*/
> +#define IPPROTO_ROUTING		43	/* IPv6 routing header		*/
> +#define IPPROTO_FRAGMENT	44	/* IPv6 fragmentation header	*/
> +#define IPPROTO_ICMPV6		58	/* ICMPv6			*/
> +#define IPPROTO_NONE		59	/* IPv6 no next header		*/
> +#define IPPROTO_DSTOPTS		60	/* IPv6 destination options	*/
> +#define IPPROTO_MH		135	/* IPv6 mobility header		*/
>  #endif /* __UAPI_DEF_IPPROTO_V6 */
>  
>  /*
> 

Cheers,
Carlos.

^ permalink raw reply

* Re: [PATCH] net: use __GFP_NORETRY for high order allocations
From: David Rientjes @ 2014-02-06 21:03 UTC (permalink / raw)
  To: Joe Perches
  Cc: Eric Dumazet, David Miller, netdev, linux-kernel@vger.kernel.org
In-Reply-To: <1391718270.15777.20.camel@joe-AO722>

On Thu, 6 Feb 2014, Joe Perches wrote:

> On Thu, 2014-02-06 at 10:42 -0800, Eric Dumazet wrote:
> > sock_alloc_send_pskb() & sk_page_frag_refill()
> > have a loop trying high order allocations to prepare
> > skb with low number of fragments as this increases performance.
> > 
> > Problem is that under memory pressure/fragmentation, this can
> > trigger OOM while the intent was only to try the high order
> > allocations, then fallback to order-0 allocations.
> []
> > Call Trace:
> >  [<ffffffff8043766c>] dump_header+0xe1/0x23e
> >  [<ffffffff80437a02>] oom_kill_process+0x6a/0x323
> >  [<ffffffff80438443>] out_of_memory+0x4b3/0x50d
> >  [<ffffffff8043a4a6>] __alloc_pages_may_oom+0xa2/0xc7
> >  [<ffffffff80236f42>] __alloc_pages_nodemask+0x1002/0x17f0
> >  [<ffffffff8024bd23>] alloc_pages_current+0x103/0x2b0
> >  [<ffffffff8028567f>] sk_page_frag_refill+0x8f/0x160
> []
> > diff --git a/net/core/sock.c b/net/core/sock.c
> []
> > @@ -1775,7 +1775,9 @@ struct sk_buff *sock_alloc_send_pskb(struct sock *sk, unsigned long header_len,
> >  			while (order) {
> >  				if (npages >= 1 << order) {
> >  					page = alloc_pages(sk->sk_allocation |
> > -							   __GFP_COMP | __GFP_NOWARN,
> > +							   __GFP_COMP |
> > +							   __GFP_NOWARN |
> > +							   __GFP_NORETRY,
> >  							   order);
> >  					if (page)
> >  						goto fill_page;
> > @@ -1845,7 +1847,7 @@ bool skb_page_frag_refill(unsigned int sz, struct page_frag *pfrag, gfp_t prio)
> >  		gfp_t gfp = prio;
> >  
> >  		if (order)
> > -			gfp |= __GFP_COMP | __GFP_NOWARN;
> > +			gfp |= __GFP_COMP | __GFP_NOWARN | __GFP_NORETRY;
> 
> Perhaps add __GFP_THISNODE too ?
> 

How does __GFP_THISNODE have anything to do with avoiding oom killing due 
to high-order fragmentation?  If they absolutely require local memory to 
currnet's cpu node then that would make sense, but the fallback still 
allocates order-0 memory remotely and with __GFP_THISNODE on this attempt 
we wouldn't even attempt remote reclaim.

^ permalink raw reply

* Re: [PATCH] net: use __GFP_NORETRY for high order allocations
From: Eric Dumazet @ 2014-02-06 21:00 UTC (permalink / raw)
  To: Joe Perches
  Cc: David Miller, netdev, David Rientjes,
	linux-kernel@vger.kernel.org
In-Reply-To: <1391718270.15777.20.camel@joe-AO722>

On Thu, 2014-02-06 at 12:24 -0800, Joe Perches wrote:

> Perhaps add __GFP_THISNODE too ?

Why ?

^ permalink raw reply

* RTNL: assertion failed at net/core/dev.c (4494) and RTNL: assertion failed at net/core/rtnetlink.c (940)
From: Thomas Glanzmann @ 2014-02-06 20:51 UTC (permalink / raw)
  To: Eric Dumazet, netdev, fubar, vfalico, andy, jiri

Hello,
this morning I checked out Linus tip and compiled it after booting my
dmesg is full of:

[    8.944991] RTNL: assertion failed at net/core/dev.c (4494)
[    8.950640] CPU: 3 PID: 388 Comm: kworker/u24:4 Not tainted 3.14.0-rc1+ #3
[    8.950642] Hardware name: Supermicro X9SRD-F/X9SRD-F, BIOS 1.0a 10/15/2012
[    8.950654] Workqueue: bond0 bond_3ad_state_machine_handler [bonding]
[    8.950658]  0000000000000000 ffff881020c88000 ffffffff8138e219 ffff881020c88000
[    8.950664]  ffffffff812d3091 ffff881023961040 ffffffff812e3132 0000000000000246
[    8.950670]  0000000000000020 ffff881020ab1be8 0000000020ab1ba8 0000000000000000
[    8.950675] Call Trace:
[    8.950686]  [<ffffffff8138e219>] ? dump_stack+0x41/0x51
[    8.950694]  [<ffffffff812d3091>] ? netdev_master_upper_dev_get+0x2a/0x4d
[    8.950699]  [<ffffffff812e3132>] ? rtnl_fill_ifinfo+0x2c/0xac4
[    8.950707]  [<ffffffff81072211>] ? print_time.part.5+0x50/0x54
[    8.950715]  [<ffffffff812caf94>] ? __kmalloc_reserve.isra.42+0x2a/0x6d
[    8.950721]  [<ffffffff81102040>] ? ksize+0x12/0x1e
[    8.950726]  [<ffffffff812cb2b7>] ? __alloc_skb+0xb5/0x1a9
[    8.950731]  [<ffffffff812e4626>] ? rtmsg_ifinfo+0x6c/0xd6
[    8.950739]  [<ffffffffa035f4f9>] ? __enable_port.isra.17+0x51/0x5a [bonding]
[    8.950747]  [<ffffffffa0360463>] ? ad_agg_selection_logic+0x3d3/0x3ed [bonding]
[    8.950754]  [<ffffffffa0360d40>] ? bond_3ad_state_machine_handler+0x555/0x918 [bonding]
[    8.950761]  [<ffffffff8104db2d>] ? process_one_work+0x191/0x293
[    8.950766]  [<ffffffff8104dfde>] ? worker_thread+0x121/0x1e7
[    8.950770]  [<ffffffff8104debd>] ? rescuer_thread+0x269/0x269
[    8.950777]  [<ffffffff810527b6>] ? kthread+0x99/0xa1
[    8.950782]  [<ffffffff8105271d>] ? __kthread_parkme+0x59/0x59
[    8.950789]  [<ffffffff8139733c>] ? ret_from_fork+0x7c/0xb0
[    8.950794]  [<ffffffff8105271d>] ? __kthread_parkme+0x59/0x59
[    8.950797] RTNL: assertion failed at net/core/rtnetlink.c (940)
[    8.956863] CPU: 3 PID: 388 Comm: kworker/u24:4 Not tainted 3.14.0-rc1+ #3
[    8.956871] Hardware name: Supermicro X9SRD-F/X9SRD-F, BIOS 1.0a 10/15/2012
[    8.956877] Workqueue: bond0 bond_3ad_state_machine_handler [bonding]
[    8.956879]  0000000000000000 ffff881020c88000 ffffffff8138e219 ffff881023961040
[    8.956884]  ffffffff812e315e 0000000000000246 0000000000000020 ffff881020ab1be8
[    8.956890]  0000000020ab1ba8 0000000000000000 ffffffff817ef530 0000000000000008
[    8.956895] Call Trace:
[    8.956899]  [<ffffffff8138e219>] ? dump_stack+0x41/0x51
[    8.956903]  [<ffffffff812e315e>] ? rtnl_fill_ifinfo+0x58/0xac4
[    8.956909]  [<ffffffff81072211>] ? print_time.part.5+0x50/0x54
[    8.956915]  [<ffffffff812caf94>] ? __kmalloc_reserve.isra.42+0x2a/0x6d
[    8.956920]  [<ffffffff81102040>] ? ksize+0x12/0x1e
[    8.956925]  [<ffffffff812cb2b7>] ? __alloc_skb+0xb5/0x1a9
[    8.956929]  [<ffffffff812e4626>] ? rtmsg_ifinfo+0x6c/0xd6
[    8.956936]  [<ffffffffa035f4f9>] ? __enable_port.isra.17+0x51/0x5a [bonding]
[    8.956942]  [<ffffffffa0360463>] ? ad_agg_selection_logic+0x3d3/0x3ed [bonding]
[    8.956949]  [<ffffffffa0360d40>] ? bond_3ad_state_machine_handler+0x555/0x918 [bonding]
[    8.956954]  [<ffffffff8104db2d>] ? process_one_work+0x191/0x293
[    8.956958]  [<ffffffff8104dfde>] ? worker_thread+0x121/0x1e7
[    8.956962]  [<ffffffff8104debd>] ? rescuer_thread+0x269/0x269
[    8.956968]  [<ffffffff810527b6>] ? kthread+0x99/0xa1
[    8.956973]  [<ffffffff8105271d>] ? __kthread_parkme+0x59/0x59
[    8.956978]  [<ffffffff8139733c>] ? ret_from_fork+0x7c/0xb0
[    8.956983]  [<ffffffff8105271d>] ? __kthread_parkme+0x59/0x59

Full dmesg is at: http://pbot.rmdir.de/dJZsX0d71RFPaZAAjcTBWQ

I'm running Debian Wheezy and my Interface Konfiguration is:

auto lo
iface lo inet loopback

auto bond0
iface bond0 inet static
       address 10.100.4.62
       netmask 255.255.0.0
       gateway 10.100.0.1
       slaves eth0 eth1
       bond-mode 802.3ad
       bond-miimon 100

auto bond0.101
iface bond0.101 inet static
       address 10.101.99.4
       netmask 255.255.0.0

auto bond0.102
iface bond0.102 inet static
       address 10.102.99.4
       netmask 255.255.0.0

auto bond0.103
iface bond0.103 inet static
       address 10.103.99.4
       netmask 255.255.0.0

auto bond1
iface bond1 inet static
       address 10.100.5.62
       netmask 255.255.0.0
       slaves eth2 eth3
       bond-mode 802.3ad
       bond-miimon 100

auto bond1.101
iface bond1.101 inet static
       address 10.101.99.5
       netmask 255.255.0.0

auto bond1.102
iface bond1.102 inet static
       address 10.102.99.5
       netmask 255.255.0.0

auto bond1.103
iface bond1.103 inet static
       address 10.103.99.5
       netmask 255.255.0.0

My IPv4 interface Konfiguration is:

(node-62) [~/work/linux-2.6] ip -4 a s
1: lo: <LOOPBACK,UP,LOWER_UP> mtu 65536 qdisc noqueue state UNKNOWN
    inet 127.0.0.1/8 scope host lo
       valid_lft forever preferred_lft forever
6: bond0: <BROADCAST,MULTICAST,MASTER,UP,LOWER_UP> mtu 1500 qdisc noqueue state UP
    inet 10.100.4.62/16 brd 10.100.255.255 scope global bond0
       valid_lft forever preferred_lft forever
7: bond0.101@bond0: <BROADCAST,MULTICAST,UP,LOWER_UP> mtu 1500 qdisc noqueue state UP
    inet 10.101.99.4/16 brd 10.101.255.255 scope global bond0.101
       valid_lft forever preferred_lft forever
8: bond0.102@bond0: <BROADCAST,MULTICAST,UP,LOWER_UP> mtu 1500 qdisc noqueue state UP
    inet 10.102.99.4/16 brd 10.102.255.255 scope global bond0.102
       valid_lft forever preferred_lft forever
9: bond0.103@bond0: <BROADCAST,MULTICAST,UP,LOWER_UP> mtu 1500 qdisc noqueue state UP
    inet 10.103.99.4/16 brd 10.103.255.255 scope global bond0.103
       valid_lft forever preferred_lft forever
10: bond1: <BROADCAST,MULTICAST,MASTER,UP,LOWER_UP> mtu 1500 qdisc noqueue state UP
    inet 10.100.5.62/16 brd 10.100.255.255 scope global bond1
       valid_lft forever preferred_lft forever
11: bond1.101@bond1: <BROADCAST,MULTICAST,UP,LOWER_UP> mtu 1500 qdisc noqueue state UP
    inet 10.101.99.5/16 brd 10.101.255.255 scope global bond1.101
       valid_lft forever preferred_lft forever
12: bond1.102@bond1: <BROADCAST,MULTICAST,UP,LOWER_UP> mtu 1500 qdisc noqueue state UP
    inet 10.102.99.5/16 brd 10.102.255.255 scope global bond1.102
       valid_lft forever preferred_lft forever
13: bond1.103@bond1: <BROADCAST,MULTICAST,UP,LOWER_UP> mtu 1500 qdisc noqueue state UP
    inet 10.103.99.5/16 brd 10.103.255.255 scope global bond1.103
       valid_lft forever preferred_lft forever

I also have IPv6 with link-local and global addresses configured.

Kernel Config is at: http://pbot.rmdir.de/VTAnhVv8ECP7a7SPaxMsFA

I'm available for testing fixes or providing ssh access to the system if you
want to do further tests. I have the same config running on 3.13.0 on the same
machine without any problems whatsoever. If I should bisect it, let me know.

Cheers,
        Thomas

^ permalink raw reply

* Re: [PATCH net] netpoll: fix netconsole IPv6 setup
From: Sabrina Dubroca @ 2014-02-06 20:58 UTC (permalink / raw)
  To: Cong Wang; +Cc: David Miller, netdev
In-Reply-To: <CAHA+R7OQYZ5RLn3v0nucKB=CVpDCZqp=HyNL=qgC9bViVKEuAQ@mail.gmail.com>

2014-02-06, 12:34:10 -0800, Cong Wang wrote:
> On Thu, Feb 6, 2014 at 9:34 AM, Sabrina Dubroca <sd@queasysnail.net> wrote:
> >  net/core/netpoll.c | 4 +++-
> >  1 file changed, 3 insertions(+), 1 deletion(-)
> >
> > diff --git a/net/core/netpoll.c b/net/core/netpoll.c
> > index c03f3de..a664f78 100644
> > --- a/net/core/netpoll.c
> > +++ b/net/core/netpoll.c
> > @@ -948,6 +948,7 @@ int netpoll_parse_options(struct netpoll *np, char *opt)
> >  {
> >         char *cur=opt, *delim;
> >         int ipv6;
> > +       bool ipversion_set = false;
> >
> 
> Or initialize 'ipv6' to -1 and then check if it is -1?

It's overwritten when we parse the remote address. And np->ipv6 is a
bool, so we can't store it there either.

-- 
Sabrina

^ permalink raw reply

* Re: [RFC PATCH] udp4: Don't take socket reference in receive path
From: Eric Dumazet @ 2014-02-06 20:58 UTC (permalink / raw)
  To: Tom Herbert; +Cc: davem, netdev, edumazet
In-Reply-To: <alpine.DEB.2.02.1402061154580.17707@tomh.mtv.corp.google.com>

On Thu, 2014-02-06 at 11:58 -0800, Tom Herbert wrote:
> The reference counting in the UDP receive path is quite expensive for
> a socket that is share amoungst CPUs. This is probably true for normal
> sockets, but really is painful when just using the socket for
> receive encapsulation.
> 
> udp4_lib_lookup always takes a socket reference, and we also put back
> the reference after calling udp_queue_rcv_skb in the normal receive
> path, so the need for taking the reference seems to be to hold the
> socket after doing rcu_read_unlock. This patch modifies udp_lib_lookup
> to optionally take a reference and is always called with rcu_read_lock.
> In udp4_lib_rcv we call lib_lookup and udp_queue_rcv under the
> rcu_read_lock but without having taken the reference.
> 
> Requesting comments because I suspect there are nuances to this!
> 
> Signed-off-by: Tom Herbert <therbert@google.com>
> ---

Unfortunately this cant work.

When I did the RCU implementation for TCP/UDP, we chose to use
SLAB_DESTROY_BY_RCU.

This meant we have to take a reference, then check again the keys for
the lookup.

If we remove SLAB_DESTROY_BY_RCU, we kill performance for short lived
sessions, because of call_rcu() added latencies.

(One UDP socket is about 1024 bytes in memory, call_rcu() grace period
is throwing away 1024 bytes from cpu caches)

Sure, in your case you know your udp sessions are not short lived,
but many applications used UDP for DNS lookups, using few packets per
socket.

^ permalink raw reply

* Re: [PATCH 2/2] rtlwifi: Fix incorrect return from rtl_ps_enable_nic()
From: Larry Finger @ 2014-02-06 20:39 UTC (permalink / raw)
  To: Olivier Langlois, linville, chaoming_li
  Cc: linux-wireless, netdev, linux-kernel, Stable
In-Reply-To: <1391235070-23180-2-git-send-email-olivier@trillion01.com>

On 02/01/2014 12:11 AM, Olivier Langlois wrote:
> rtl_ps_enable_nic() is called from loops that will loop until this function returns true or a
> maximum number of retries is performed.
>
> hw_init() returns non-zero on error. In that situation return false to
> restore the original design intent to retry hw init when it fails.
>
> Signed-off-by: Olivier Langlois <olivier@trillion01.com>
> Cc: Stable <stable@vger.kernel.org>

Acked-by: Larry Finger <Larry.Finger.net>

Larry

> ---
>   drivers/net/wireless/rtlwifi/ps.c | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/net/wireless/rtlwifi/ps.c b/drivers/net/wireless/rtlwifi/ps.c
> index 0d81f76..a56e9b3 100644
> --- a/drivers/net/wireless/rtlwifi/ps.c
> +++ b/drivers/net/wireless/rtlwifi/ps.c
> @@ -48,7 +48,7 @@ bool rtl_ps_enable_nic(struct ieee80211_hw *hw)
>
>   	/*<2> Enable Adapter */
>   	if (rtlpriv->cfg->ops->hw_init(hw))
> -		return 1;
> +		return false;
>   	RT_CLEAR_PS_LEVEL(ppsc, RT_RF_OFF_LEVL_HALT_NIC);
>
>   	/*<3> Enable Interrupt */
>

^ permalink raw reply

* Re: [PATCH 1/2] rtlwifi: rtl8192ce: Fix too long disable of IRQs
From: Larry Finger @ 2014-02-06 20:38 UTC (permalink / raw)
  To: Olivier Langlois, linville, chaoming_li
  Cc: linux-wireless, netdev, linux-kernel, Stable
In-Reply-To: <1391235070-23180-1-git-send-email-olivier@trillion01.com>

On 02/01/2014 12:11 AM, Olivier Langlois wrote:
> rtl8192ce is disabling for too long the local interrupts during hw initiatialisation when performing scans
>
> The observable symptoms in dmesg can be:
>
> - underruns from ALSA playback
> - clock freezes (tstamps do not change for several dmesg entries until irqs are finaly reenabled):
>
> [  250.817669] rtlwifi:rtl_op_config():<0-0-0> 0x100
> [  250.817685] rtl8192ce:_rtl92ce_phy_set_rf_power_state():<0-1-0> IPS Set eRf nic enable
> [  250.817732] rtl8192ce:_rtl92ce_init_mac():<0-1-0> reg0xec:18051d59:11
> [  250.817796] rtl8192ce:_rtl92ce_init_mac():<0-1-0> reg0xec:18051d59:11
> [  250.817910] rtl8192ce:_rtl92ce_init_mac():<0-1-0> reg0xec:18051d59:11
> [  250.818024] rtl8192ce:_rtl92ce_init_mac():<0-1-0> reg0xec:18051d59:11
> [  250.818139] rtl8192ce:_rtl92ce_init_mac():<0-1-0> reg0xec:18051d59:11
> [  250.818253] rtl8192ce:_rtl92ce_init_mac():<0-1-0> reg0xec:18051d59:11
> [  250.818367] rtl8192ce:_rtl92ce_init_mac():<0-1-0> reg0xec:18051d59:11
> [  250.818472] rtl8192ce:_rtl92ce_init_mac():<0-1-0> reg0xec:18051d59:11
> [  250.818472] rtl8192ce:_rtl92ce_init_mac():<0-1-0> reg0xec:18051d59:11
> [  250.818472] rtl8192ce:_rtl92ce_init_mac():<0-1-0> reg0xec:18051d59:11
> [  250.818472] rtl8192ce:_rtl92ce_init_mac():<0-1-0> reg0xec:18051d59:11
> [  250.818472] rtl8192ce:_rtl92ce_init_mac():<0-1-0> reg0xec:98053f15:10
> [  250.818472] rtl8192ce:rtl92ce_sw_led_on():<0-1-0> LedAddr:4E ledpin=1
> [  250.818472] rtl8192c_common:rtl92c_download_fw():<0-1-0> Firmware Version(49), Signature(0x88c1),Size(32)
> [  250.818472] rtl8192ce:rtl92ce_enable_hw_security_config():<0-1-0> PairwiseEncAlgorithm = 0 GroupEncAlgorithm = 0
> [  250.818472] rtl8192ce:rtl92ce_enable_hw_security_config():<0-1-0> The SECR-value cc
> [  250.818472] rtl8192c_common:rtl92c_dm_check_txpower_tracking_thermal_meter():<0-1-0> Schedule TxPowerTracking direct call!!
> [  250.818472] rtl8192c_common:rtl92c_dm_txpower_tracking_callback_thermalmeter():<0-1-0> rtl92c_dm_txpower_tracking_callback_thermalmeter
> [  250.818472] rtl8192c_common:rtl92c_dm_txpower_tracking_callback_thermalmeter():<0-1-0> Readback Thermal Meter = 0xe pre thermal meter 0xf eeprom_thermalmeter 0xf
> [  250.818472] rtl8192c_common:rtl92c_dm_txpower_tracking_callback_thermalmeter():<0-1-0> Initial pathA ele_d reg0xc80 = 0x40000000, ofdm_index=0xc
> [  250.818472] rtl8192c_common:rtl92c_dm_txpower_tracking_callback_thermalmeter():<0-1-0> Initial reg0xa24 = 0x90e1317, cck_index=0xc, ch14 0
> [  250.818472] rtl8192c_common:rtl92c_dm_txpower_tracking_callback_thermalmeter():<0-1-0> Readback Thermal Meter = 0xe pre thermal meter 0xf eeprom_thermalmeter 0xf delta 0x1 delta_lck 0x0 delta_iqk 0x0
> [  250.818472] rtl8192c_common:rtl92c_dm_txpower_tracking_callback_thermalmeter():<0-1-0> <===
> [  250.818472] rtl8192c_common:rtl92c_dm_initialize_txpower_tracking_thermalmeter():<0-1-0> pMgntInfo->txpower_tracking = 1
> [  250.818472] rtl8192ce:rtl92ce_led_control():<0-1-0> ledaction 3
> [  250.818472] rtl8192ce:rtl92ce_sw_led_on():<0-1-0> LedAddr:4E ledpin=1
> [  250.818472] rtlwifi:rtl_ips_nic_on():<0-1-0> before spin_unlock_irqrestore
> [  251.154656] PCM: Lost interrupts? [Q]-0 (stream=0, delta=15903, new_hw_ptr=293408, old_hw_ptr=277505)
>
> The exact code flow that causes that is:
>
> 1. wpa_supplicant send a start_scan request to the nl80211 driver
> 2. mac80211 module call rtl_op_config with IEEE80211_CONF_CHANGE_IDLE
> 3.   rtl_ips_nic_on is called which disable local irqs
> 4.     rtl92c_phy_set_rf_power_state() is called
> 5.       rtl_ps_enable_nic() is called and hw_init()is executed and then the interrupts on the device are enabled
>
> A good solution could be to refactor the code to avoid calling rtl92ce_hw_init() with the irqs disabled
> but a quick and dirty solution that has proven to work is
> to reenable the irqs during the function rtl92ce_hw_init().
>
> I think that it is safe doing so since the device interrupt will only be enabled after the init function succeed.
>
> Signed-off-by: Olivier Langlois <olivier@trillion01.com>
> Cc: Stable <stable@vger.kernel.org>

Acked-by: Larry Finger <Larry.Finger@lwfinger.net>

Larry

> ---
>   drivers/net/wireless/rtlwifi/rtl8192ce/hw.c | 18 ++++++++++++++++--
>   1 file changed, 16 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/net/wireless/rtlwifi/rtl8192ce/hw.c b/drivers/net/wireless/rtlwifi/rtl8192ce/hw.c
> index a82b30a..2eb0b38 100644
> --- a/drivers/net/wireless/rtlwifi/rtl8192ce/hw.c
> +++ b/drivers/net/wireless/rtlwifi/rtl8192ce/hw.c
> @@ -937,14 +937,26 @@ int rtl92ce_hw_init(struct ieee80211_hw *hw)
>   	bool is92c;
>   	int err;
>   	u8 tmp_u1b;
> +	unsigned long flags;
>
>   	rtlpci->being_init_adapter = true;
> +
> +	/* Since this function can take a very long time (up to 350 ms)
> +	 * and can be called with irqs disabled, reenable the irqs
> +	 * to let the other devices continue being serviced.
> +	 *
> +	 * It is safe doing so since our own interrupts will only be enabled
> +	 * in a subsequent step.
> +	 */
> +	local_save_flags(flags);
> +	local_irq_enable();
> +
>   	rtlpriv->intf_ops->disable_aspm(hw);
>   	rtstatus = _rtl92ce_init_mac(hw);
>   	if (!rtstatus) {
>   		RT_TRACE(rtlpriv, COMP_ERR, DBG_EMERG, "Init MAC failed\n");
>   		err = 1;
> -		return err;
> +		goto exit;
>   	}
>
>   	err = rtl92c_download_fw(hw);
> @@ -952,7 +964,7 @@ int rtl92ce_hw_init(struct ieee80211_hw *hw)
>   		RT_TRACE(rtlpriv, COMP_ERR, DBG_WARNING,
>   			 "Failed to download FW. Init HW without FW now..\n");
>   		err = 1;
> -		return err;
> +		goto exit;
>   	}
>
>   	rtlhal->last_hmeboxnum = 0;
> @@ -1032,6 +1044,8 @@ int rtl92ce_hw_init(struct ieee80211_hw *hw)
>   		RT_TRACE(rtlpriv, COMP_INIT, DBG_TRACE, "under 1.5V\n");
>   	}
>   	rtl92c_dm_init(hw);
> +exit:
> +	local_irq_restore(flags);
>   	rtlpci->being_init_adapter = false;
>   	return err;
>   }
>

^ 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