* Re: [PATCH] r8169: Reduce looping in the interrupt handler.
From: Eric W. Biederman @ 2009-08-30 20:53 UTC (permalink / raw)
To: Francois Romieu
Cc: David Dillow, Michael Riepe, Michael Buesch, Rui Santos,
Michael B??ker, linux-kernel, netdev
In-Reply-To: <20090830203735.GA24912@electric-eye.fr.zoreil.com>
Francois Romieu <romieu@fr.zoreil.com> writes:
> David Dillow <dave@thedillows.org> :
> [...]
>> It'll be this weekend, but I can see cases where it can lock my chip up
>> -- they should be rare, but then I thought your case would be extremely
>> rare...
>
> I don't get it.
>
> Can you elaborate the relevant cases or give some sample scenarios for
> them ?
I think David is referring to the fact that in the NAPI loop there is
nothing that acks everything.
Eric
^ permalink raw reply
* [PATCH] net: Fix sock freeing before sock_init_data() with __sk_free()
From: Jarek Poplawski @ 2009-08-30 22:23 UTC (permalink / raw)
To: David Miller; +Cc: Eric Dumazet, netdev
After recent changes sk_free() frees socks conditionally and depends
on sk_wmem_alloc beeing set e.g. in sock_init_data(). But in some
cases sk_free() is called earlier, usually after other alloc errors.
This patch fixes it by exporting and using __sk_free() directly.
Signed-off-by: Jarek Poplawski <jarkao2@gmail.com>
---
include/net/sock.h | 1 +
net/ax25/af_ax25.c | 6 +++---
net/core/sock.c | 6 ++++--
net/irda/af_irda.c | 4 ++--
net/tipc/socket.c | 2 +-
5 files changed, 11 insertions(+), 8 deletions(-)
diff --git a/include/net/sock.h b/include/net/sock.h
index 950409d..e76ef65 100644
--- a/include/net/sock.h
+++ b/include/net/sock.h
@@ -935,6 +935,7 @@ extern void release_sock(struct sock *sk);
extern struct sock *sk_alloc(struct net *net, int family,
gfp_t priority,
struct proto *prot);
+extern void __sk_free(struct sock *sk);
extern void sk_free(struct sock *sk);
extern void sk_release_kernel(struct sock *sk);
extern struct sock *sk_clone(const struct sock *sk,
diff --git a/net/ax25/af_ax25.c b/net/ax25/af_ax25.c
index da0f64f..c05738c 100644
--- a/net/ax25/af_ax25.c
+++ b/net/ax25/af_ax25.c
@@ -851,7 +851,7 @@ static int ax25_create(struct net *net, struct socket *sock, int protocol)
ax25 = sk->sk_protinfo = ax25_create_cb();
if (!ax25) {
- sk_free(sk);
+ __sk_free(sk);
return -ENOMEM;
}
@@ -876,7 +876,7 @@ struct sock *ax25_make_new(struct sock *osk, struct ax25_dev *ax25_dev)
return NULL;
if ((ax25 = ax25_create_cb()) == NULL) {
- sk_free(sk);
+ __sk_free(sk);
return NULL;
}
@@ -886,7 +886,7 @@ struct sock *ax25_make_new(struct sock *osk, struct ax25_dev *ax25_dev)
case SOCK_SEQPACKET:
break;
default:
- sk_free(sk);
+ __sk_free(sk);
ax25_cb_put(ax25);
return NULL;
}
diff --git a/net/core/sock.c b/net/core/sock.c
index bbb25be..92793be 100644
--- a/net/core/sock.c
+++ b/net/core/sock.c
@@ -1031,7 +1031,7 @@ struct sock *sk_alloc(struct net *net, int family, gfp_t priority,
}
EXPORT_SYMBOL(sk_alloc);
-static void __sk_free(struct sock *sk)
+void __sk_free(struct sock *sk)
{
struct sk_filter *filter;
@@ -1054,13 +1054,15 @@ static void __sk_free(struct sock *sk)
put_net(sock_net(sk));
sk_prot_free(sk->sk_prot_creator, sk);
}
+EXPORT_SYMBOL(__sk_free);
void sk_free(struct sock *sk)
{
/*
* We substract one from sk_wmem_alloc and can know if
* some packets are still in some tx queue.
- * If not null, sock_wfree() will call __sk_free(sk) later
+ * If not null, sock_wfree() will call __sk_free(sk) later.
+ * (Before sock_init_data() etc. use __sk_free() instead.)
*/
if (atomic_dec_and_test(&sk->sk_wmem_alloc))
__sk_free(sk);
diff --git a/net/irda/af_irda.c b/net/irda/af_irda.c
index 50b43c5..9ed2060 100644
--- a/net/irda/af_irda.c
+++ b/net/irda/af_irda.c
@@ -1118,12 +1118,12 @@ static int irda_create(struct net *net, struct socket *sock, int protocol)
self->max_sdu_size_rx = TTP_SAR_UNBOUND;
break;
default:
- sk_free(sk);
+ __sk_free(sk);
return -ESOCKTNOSUPPORT;
}
break;
default:
- sk_free(sk);
+ __sk_free(sk);
return -ESOCKTNOSUPPORT;
}
diff --git a/net/tipc/socket.c b/net/tipc/socket.c
index 1848693..4c8435a 100644
--- a/net/tipc/socket.c
+++ b/net/tipc/socket.c
@@ -228,7 +228,7 @@ static int tipc_create(struct net *net, struct socket *sock, int protocol)
tp_ptr = tipc_createport_raw(sk, &dispatch, &wakeupdispatch,
TIPC_LOW_IMPORTANCE);
if (unlikely(!tp_ptr)) {
- sk_free(sk);
+ __sk_free(sk);
return -ENOMEM;
}
^ permalink raw reply related
* Re: Pull request: bluetooth-next-2.6 2009-08-30
From: David Miller @ 2009-08-31 4:47 UTC (permalink / raw)
To: marcel; +Cc: netdev
In-Reply-To: <cover.1251621661.git.marcel@holtmann.org>
From: Marcel Holtmann <marcel@holtmann.org>
Date: Sun, 30 Aug 2009 01:50:14 -0700
> these are the updates for the Bluetooth subsystem for the 2.6.32 merge
> window. On the driver side we have support for Marvell SDIO devices and
> the Bluetooth USB driver got support for auto-suspend. On the core stack
> part we have support for L2CAP enhanced retransmission mode now. The rest
> are random fixes and cleanups.
Looks good, all pulled into net-next-2.6
^ permalink raw reply
* Re: [PATCH 1/5] ucc_geth: Fix NULL pointer dereference in uec_get_ethtool_stats()
From: David Miller @ 2009-08-31 4:52 UTC (permalink / raw)
To: avorontsov; +Cc: leoli, galak, timur, scottwood, linuxppc-dev, netdev
In-Reply-To: <20090827173547.GA1580@oksana.dev.rtsoft.ru>
All 5 patches applied to net-next-2.6
I would have liked to have seen at least one powerpc ACK for
patch #2 but these were posted more than a week ago, the
patch looks pretty reasonable, and we can't wait forever for
stuff like this.
^ permalink raw reply
* Re: [PATCH 0/3] [v3] Revert Backoff on ICMP destination unreachable
From: David Miller @ 2009-08-31 4:56 UTC (permalink / raw)
To: damian; +Cc: netdev, ilpo.jarvinen
In-Reply-To: <4A950B76.1070002@tvk.rwth-aachen.de>
From: Damian Lukowski <damian@tvk.rwth-aachen.de>
Date: Wed, 26 Aug 2009 12:16:22 +0200
> This series of patches implements the TCP improvement of the Internet Draft
> "Make TCP more Robust to Long Connectivity Disruptions"
> (http://tools.ietf.org/html/draft-zimmermann-tcp-lcd).
>
> Exponential backoff is TCP's standard behaviour during long connectivity
> disruptions, which is a countermeasure against network congestion.
> If congestion can be excluded as the reason for RTO retransmission loss,
> backoff is not desirable, as it yields longer TCP recovery times, when
> the communication path is repaired shortly after an unsuccessful
> retransmission probe.
>
> 1) This patch renames the skb in tcp_v4_err() in preparation for patch 2.
> 2) Contains the main reversion logic.
> Reintroduces tcp_bound_rto() and __tcp_set_rto()
> 3) This patch modifies the interpretation of the tcp_retries{1,2} sysctls.
> It became necessary because with patch2 the assumption that the number
> of retransmits corresponds to an specific timeout value is not accurate
> anymore. With this patch tcp_retries{1,2} specifies a timeout value,
> equivalent to the time a connection with an rto value of MIN_RTO (200ms)
> would need to retransmit N segments. IOW: The meaning (in sense of time)
> is mostly preserved, but the actual connection timeout does not depend
> on the calculated rto of the connection, anymore.
>
> Signed-off-by: Damian Lukowski <damian@tvk.rwth-aachen.de>
Ilpo can you please review this new version of Damian's changes?
Thanks.
^ permalink raw reply
* Re: [PATCH 1/3] drop_monitor: fix trace_napi_poll_hit()
From: David Miller @ 2009-08-31 4:57 UTC (permalink / raw)
To: xiaoguangrong
Cc: mingo, rostedt, fweisbec, nhorman, yjwei, linux-kernel, netdev
In-Reply-To: <4A966288.8020402@cn.fujitsu.com>
Xiao, can you please resend this patch series cleanly?
I see two submissions of patch #3 and I have no copies
at all of patch #2.
Thanks.
^ permalink raw reply
* Re: [PATCH] WAN: bit and/or confusion
From: David Miller @ 2009-08-31 5:02 UTC (permalink / raw)
To: roel.kluin; +Cc: khc, akpm, romieu, netdev
In-Reply-To: <4A8D57F8.1050106@gmail.com>
From: Roel Kluin <roel.kluin@gmail.com>
Date: Thu, 20 Aug 2009 16:04:40 +0200
> Fix the tests that check whether Frame* bits are not set
>
> Signed-off-by: Roel Kluin <roel.kluin@gmail.com>
Applied to net-next-2.6, thanks.
^ permalink raw reply
* Re: [PATCH]gianfar: gfar_remove needs to call unregister_netdev()
From: David Miller @ 2009-08-31 5:04 UTC (permalink / raw)
To: uchiyama.toru; +Cc: netdev
In-Reply-To: <4A8D559B.3030108@jp.fujitsu.com>
From: Toru UCHIYAMA <uchiyama.toru@jp.fujitsu.com>
Date: Thu, 20 Aug 2009 22:54:35 +0900
> This patch solves the problem that the Oops(BUG_ON) occurs by rmmod.
...
> Signed-off-by: uchiyama.toru@jp.fujitsu.com
Applied, thank you.
In the future, please list your name as well as your email
address in the Signed-off-by: specification.
Thank you.
^ permalink raw reply
* Re: [PATCH] Re: iproute2 / tbf with large burst seems broken again
From: David Miller @ 2009-08-31 5:05 UTC (permalink / raw)
To: jarkao2; +Cc: denys, netdev
In-Reply-To: <20090826215956.GA5148@ami.dom.local>
From: Jarek Poplawski <jarkao2@gmail.com>
Date: Wed, 26 Aug 2009 23:59:56 +0200
> Denys Fedoryschenko wrote, On 08/25/2009 02:18 PM:
> ...
>> Just maybe good idea in next kernels to document this or handle overflow, that
>> it will give warning in kernel. Otherwise user will have non-functional qdisc
>> that will not pass traffic.
>
> OK, if this patch works for you send your Tested-by, please.
Denys, please give Jarek's patch some testing and let us know
if it triggers properly for you.
> Thanks,
> Jarek P.
> ---------------->
> pkt_sched: Warn on tokens overflow in tbf_dequeue
>
> After recent change of scheduling resolution some "extreme" configs
> can cause token overflows with stalls. This patch warns when tokens
> were negative before accounting for a packet length.
>
> Reported-by: Denys Fedoryschenko <denys@visp.net.lb>
> Signed-off-by: Jarek Poplawski <jarkao2@gmail.com>
> ---
>
> net/sched/sch_tbf.c | 6 ++++++
> 1 files changed, 6 insertions(+), 0 deletions(-)
>
> diff --git a/net/sched/sch_tbf.c b/net/sched/sch_tbf.c
> index e22dfe8..ef191c4 100644
> --- a/net/sched/sch_tbf.c
> +++ b/net/sched/sch_tbf.c
> @@ -191,6 +191,12 @@ static struct sk_buff *tbf_dequeue(struct Qdisc* sch)
> return skb;
> }
>
> + if (unlikely((long)(toks + L2T(q, len)) < 0 ||
> + (long)(ptoks + L2T_P(q, len) < 0)))
> + if (net_ratelimit())
> + pr_warning("tbf: tokens overflow;"
> + " fix limits.\n");
> +
> qdisc_watchdog_schedule(&q->watchdog,
> now + max_t(long, -toks, -ptoks));
>
> --
> To unsubscribe from this list: send the line "unsubscribe netdev" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: [PATCH][net-next] LRO: improve aggregation in case of zero TSecr packets
From: David Miller @ 2009-08-31 5:11 UTC (permalink / raw)
To: opurdila; +Cc: themann, raisch, eric.dumazet, netdev
In-Reply-To: <200908270208.31581.opurdila@ixiacom.com>
From: Octavian Purdila <opurdila@ixiacom.com>
Date: Thu, 27 Aug 2009 02:08:31 +0300
> This fixes a temporary performance issue we noticed in back to back
> TSO - LRO tests when such tests are run within five minutes after
> boot.
>
> The TSval field of TCP packets is filled in based on the current
> jiffie, which is initialized at -300*HZ. That means that in 5 minutes
> after reboot it will wrap to zero.
RFC1323 says we absolutely must ignore zero TSecr values.
It is a bug that the stack emits a zero value when it means to give a
real TSecr value that will be used.
Probably we can do something like emit '1' when we would emit '0'
based upon jiffies.
And this would be an improvement from now in that having a off-by-one
TSecr in this situation is better than emitting one which we can
guarentee will be ignored.
^ permalink raw reply
* Re: [PATCH net-next-2.6] can: use correct NET_RX_ return values
From: David Miller @ 2009-08-31 5:13 UTC (permalink / raw)
To: oliver; +Cc: netdev
In-Reply-To: <4A995B15.4020303@hartkopp.net>
From: Oliver Hartkopp <oliver@hartkopp.net>
Date: Sat, 29 Aug 2009 18:45:09 +0200
> Dropped skb's should be documented by an appropriate return value.
> Use the correct NET_RX_DROP and NET_RX_SUCCESS values for that reason.
>
> Signed-off-by: Oliver Hartkopp <oliver@hartkopp.net>
Applied, thanks.
^ permalink raw reply
* Re: [PATCH] netdevice: Use existing macros
From: David Miller @ 2009-08-31 5:15 UTC (permalink / raw)
To: krkumar2; +Cc: netdev
In-Reply-To: <20090830062121.6296.60149.sendpatchset@localhost.localdomain>
From: Krishna Kumar <krkumar2@in.ibm.com>
Date: Sun, 30 Aug 2009 11:51:21 +0530
> From: Krishna Kumar <krkumar2@in.ibm.com>
>
> netdevice: Consolidate to use existing macros where available.
>
> Patch compiled and 32 simultaneous netperf testing ran fine.
>
> Signed-off-by: Krishna Kumar <krkumar2@in.ibm.com>
Applied, thanks.
^ permalink raw reply
* Re: [PATCH] net: convert one non-symbolic return value in dev_queue_xmit
From: David Miller @ 2009-08-31 5:18 UTC (permalink / raw)
To: krkumar2; +Cc: netdev
In-Reply-To: <20090830062136.6299.31696.sendpatchset@localhost.localdomain>
From: Krishna Kumar <krkumar2@in.ibm.com>
Date: Sun, 30 Aug 2009 11:51:36 +0530
> From: Krishna Kumar <krkumar2@in.ibm.com>
>
> net: convert remaining non-symbolic return values in dev_queue_xmit
>
> Patch compiled and 32 simultaneous netperf testing ran fine.
>
> Signed-off-by: Krishna Kumar <krkumar2@in.ibm.com>
Applied, but please do me a favor.
Do not provide two commit message header lines like this.
They aren't even equal.
Whatever you give me in the Subject line after "[....] " is what
ends up being the commit message header line when I apply your
patch.
Adding another commit header line in your email message body just
makes the commit look confusing and ugly.
So just give me one in the subject line and elide the one in the
message body.
Thanks.
^ permalink raw reply
* Re: [RFC PATCH] sched: Fix resource limiting in pfifo_fast
From: David Miller @ 2009-08-31 5:19 UTC (permalink / raw)
To: krkumar2; +Cc: netdev
In-Reply-To: <20090830062344.6380.16713.sendpatchset@localhost.localdomain>
From: Krishna Kumar <krkumar2@in.ibm.com>
Date: Sun, 30 Aug 2009 11:53:44 +0530
> From: Krishna Kumar <krkumar2@in.ibm.com>
>
> pfifo_fast_enqueue has this check:
> if (skb_queue_len(list) < qdisc_dev(qdisc)->tx_queue_len) {
>
> which allows each band to enqueue upto tx_queue_len skbs for a
> total of 3*tx_queue_len skbs. I am not sure if this was the
> intention of limiting in qdisc.
>
> Patch compiled and 32 simultaneous netperf testing ran fine. Also:
> # tc -s qdisc show dev eth2
> qdisc pfifo_fast 0: root bands 3 priomap 1 2 2 2 1 2 0 0 1 1 1 1 1 1 1 1
> Sent 16835026752 bytes 373116 pkt (dropped 0, overlimits 0 requeues 25)
> rate 0bit 0pps backlog 0b 0p requeues 25
>
> (I am taking next week off, so sorry for any delay in responding)
>
> Signed-off-by: Krishna Kumar <krkumar2@in.ibm.com>
This is probably just a thinko, nice catch.
I think I'll apply this to net-next-2.6, thanks!
^ permalink raw reply
* RE: [PATCH 0/9] drivers/net/s2io.c: Cleanups
From: Sreenivasa Honnur @ 2009-08-31 5:21 UTC (permalink / raw)
To: Joe Perches, David Miller
Cc: netdev, Anil Murthy, Sivakumar Subramani, Rastapur Santosh,
Ramkrishna Vepa, akpm, linux-next
In-Reply-To: <1251326878.13026.52.camel@Joe-Laptop.home>
Joe,
Patches look fine, I reviewed and tested these patches on latest
davem-net-next-2.6 sources.
Acked-by: Sreenivasa Honnur <sreenivasa.honnur@neterion.com>
Regs
srini
-----Original Message-----
From: Joe Perches [mailto:joe@perches.com]
Sent: Thursday, August 27, 2009 4:18 AM
To: David Miller
Cc: netdev@vger.kernel.org; Anil Murthy; Sreenivasa Honnur; Sivakumar
Subramani; Rastapur Santosh; Ramkrishna Vepa; akpm@linux-foundation.org;
linux-next@vger.kernel.org
Subject: Re: [PATCH 0/9] drivers/net/s2io.c: Cleanups
On Wed, 2009-08-26 at 15:33 -0700, David Miller wrote:
> From: Joe Perches <joe@perches.com>
> Date: Mon, 24 Aug 2009 20:29:39 -0700
> > Just a few cleanups, compiled, untested. No hardware.
> > s2io.c: Use const for strings
> > s2io.c: Shorten code line length by using intermediate pointers
> > s2io.c: Use calculated size in kmallocs
> > s2io.c: use kzalloc
> > s2io.c: Make more conforming to normal kernel style
> > s2io.c: convert printks to pr_<level>
> > s2io.c: fix spelling explaination
> > s2io.c: Standardize statistics accessors
> > s2io.c: Convert skipped nic->config.tx_cfg[i]. to tx_cfg->
>
> Since this is a pretty serious set of cleanups, I'd like to let the
> S2IO driver folks have some time to look at this and at least have a
> chance to ACK/NACK them.
No worries. That's fine. It's only cleanups.
I haven't heard from them though and they haven't submitted or signed a
patch for s2io in over a year.
Hey! Neterion people! Is anybody home?
^ permalink raw reply
* Re: [PATCH] Re: iproute2 / tbf with large burst seems broken again
From: Jarek Poplawski @ 2009-08-31 5:30 UTC (permalink / raw)
To: David Miller; +Cc: denys, netdev
In-Reply-To: <20090830.220559.77181973.davem@davemloft.net>
On Sun, Aug 30, 2009 at 10:05:59PM -0700, David Miller wrote:
> From: Jarek Poplawski <jarkao2@gmail.com>
> Date: Wed, 26 Aug 2009 23:59:56 +0200
>
> > Denys Fedoryschenko wrote, On 08/25/2009 02:18 PM:
> > ...
> >> Just maybe good idea in next kernels to document this or handle overflow, that
> >> it will give warning in kernel. Otherwise user will have non-functional qdisc
> >> that will not pass traffic.
> >
> > OK, if this patch works for you send your Tested-by, please.
>
> Denys, please give Jarek's patch some testing and let us know
> if it triggers properly for you.
Actually, I think now, after 2.6.31, this patch might be a bit too
late. If people hit this problem they probably should've fixed it
until -next already, so let's forget about this patch until there are
more such reports.
Btw, I guess, Denys could submit some warnings into iproute's code
and/or documentation, as he proposed earlier.
Thanks,
Jarek P.
> > ---------------->
> > pkt_sched: Warn on tokens overflow in tbf_dequeue
> >
> > After recent change of scheduling resolution some "extreme" configs
> > can cause token overflows with stalls. This patch warns when tokens
> > were negative before accounting for a packet length.
> >
> > Reported-by: Denys Fedoryschenko <denys@visp.net.lb>
> > Signed-off-by: Jarek Poplawski <jarkao2@gmail.com>
> > ---
> >
> > net/sched/sch_tbf.c | 6 ++++++
> > 1 files changed, 6 insertions(+), 0 deletions(-)
> >
> > diff --git a/net/sched/sch_tbf.c b/net/sched/sch_tbf.c
> > index e22dfe8..ef191c4 100644
> > --- a/net/sched/sch_tbf.c
> > +++ b/net/sched/sch_tbf.c
> > @@ -191,6 +191,12 @@ static struct sk_buff *tbf_dequeue(struct Qdisc* sch)
> > return skb;
> > }
> >
> > + if (unlikely((long)(toks + L2T(q, len)) < 0 ||
> > + (long)(ptoks + L2T_P(q, len) < 0)))
> > + if (net_ratelimit())
> > + pr_warning("tbf: tokens overflow;"
> > + " fix limits.\n");
> > +
> > qdisc_watchdog_schedule(&q->watchdog,
> > now + max_t(long, -toks, -ptoks));
> >
> > --
> > To unsubscribe from this list: send the line "unsubscribe netdev" in
> > the body of a message to majordomo@vger.kernel.org
> > More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: [PATCH] Re: iproute2 / tbf with large burst seems broken again
From: David Miller @ 2009-08-31 5:32 UTC (permalink / raw)
To: jarkao2; +Cc: denys, netdev
In-Reply-To: <20090831053026.GA5005@ff.dom.local>
From: Jarek Poplawski <jarkao2@gmail.com>
Date: Mon, 31 Aug 2009 05:30:27 +0000
> Actually, I think now, after 2.6.31, this patch might be a bit too
> late. If people hit this problem they probably should've fixed it
> until -next already, so let's forget about this patch until there are
> more such reports.
Ok.
> Btw, I guess, Denys could submit some warnings into iproute's code
> and/or documentation, as he proposed earlier.
Yeah sounds like a good idea.
^ permalink raw reply
* Re: [PATCH 0/9] drivers/net/s2io.c: Cleanups
From: David Miller @ 2009-08-31 5:35 UTC (permalink / raw)
To: Sreenivasa.Honnur
Cc: joe, netdev, Anil.Murthy, Sivakumar.Subramani, santosh.rastapur,
Ramkrishna.Vepa, akpm, linux-next
In-Reply-To: <78C9135A3D2ECE4B8162EBDCE82CAD7705A1DF76@nekter>
From: "Sreenivasa Honnur" <Sreenivasa.Honnur@neterion.com>
Date: Mon, 31 Aug 2009 01:21:36 -0400
> Joe,
> Patches look fine, I reviewed and tested these patches on latest
> davem-net-next-2.6 sources.
>
> Acked-by: Sreenivasa Honnur <sreenivasa.honnur@neterion.com>
Great, all patches applied to net-next-2.6, thanks everyone.
^ permalink raw reply
* [PATCH resend] drop_monitor: fix trace_napi_poll_hit()
From: Xiao Guangrong @ 2009-08-31 6:10 UTC (permalink / raw)
To: David Miller; +Cc: Neil Horman, Wei Yongjun, Netdev, LKML
The net_dev of backlog napi is NULL, like below:
__get_cpu_var(softnet_data).backlog.dev == NULL
So, we should check it in napi tracepoint's probe function
Acked-by: Neil Horman <nhorman@tuxdriver.com>
Signed-off-by: Xiao Guangrong <xiaoguangrong@cn.fujitsu.com>
---
net/core/drop_monitor.c | 3 ++-
1 files changed, 2 insertions(+), 1 deletions(-)
diff --git a/net/core/drop_monitor.c b/net/core/drop_monitor.c
index 9d66fa9..d311202 100644
--- a/net/core/drop_monitor.c
+++ b/net/core/drop_monitor.c
@@ -182,7 +182,8 @@ static void trace_napi_poll_hit(struct napi_struct *napi)
/*
* Ratelimit our check time to dm_hw_check_delta jiffies
*/
- if (!time_after(jiffies, napi->dev->last_rx + dm_hw_check_delta))
+ if (!napi->dev ||
+ !time_after(jiffies, napi->dev->last_rx + dm_hw_check_delta))
return;
rcu_read_lock();
--
1.6.1.2
^ permalink raw reply related
* [PATCH resend] tracing/events: convert NAPI's tracepoint via TRACE_EVENT
From: Xiao Guangrong @ 2009-08-31 6:16 UTC (permalink / raw)
To: Ingo Molnar
Cc: David Miller, Neil Horman, Steven Rostedt, Frederic Weisbecker,
Wei Yongjun, Netdev, LKML
- Convert NAPI's tracepoint via TRACE_EVENT macro, the output information
like below:
sshd-2503 [000] 71.920846: napi_poll: ifname=eth0 state=0 weigth=16 poll=pcnet32_poll
sshd-2503 [000] 72.020291: napi_poll: ifname=eth0 state=0 weigth=16 poll=pcnet32_poll
sshd-2503 [000] 72.020418: napi_poll: ifname=eth0 state=NAPI_STATE_SCHED weigth=16 poll=pcnet32_poll
- Remove the "DECLARE_TRACE" definiens in include/trace/define_trace.h,
because TRACE_EVENT not use it
Signed-off-by: Xiao Guangrong <xiaoguangrong@cn.fujitsu.com>
---
include/trace/define_trace.h | 4 ---
include/trace/events/napi.h | 43 +++++++++++++++++++++++++++++++++++++----
2 files changed, 38 insertions(+), 9 deletions(-)
diff --git a/include/trace/define_trace.h b/include/trace/define_trace.h
index 2a4b3bf..6158741 100644
--- a/include/trace/define_trace.h
+++ b/include/trace/define_trace.h
@@ -31,10 +31,6 @@
assign, print, reg, unreg) \
DEFINE_TRACE_FN(name, reg, unreg)
-#undef DECLARE_TRACE
-#define DECLARE_TRACE(name, proto, args) \
- DEFINE_TRACE(name)
-
#undef TRACE_INCLUDE
#undef __TRACE_INCLUDE
diff --git a/include/trace/events/napi.h b/include/trace/events/napi.h
index a8989c4..5c7a192 100644
--- a/include/trace/events/napi.h
+++ b/include/trace/events/napi.h
@@ -1,11 +1,44 @@
-#ifndef _TRACE_NAPI_H_
-#define _TRACE_NAPI_H_
+#undef TRACE_SYSTEM
+#define TRACE_SYSTEM napi
+
+#if !defined(_TRACE_NAPI_H) || defined(TRACE_HEADER_MULTI_READ)
+#define _TRACE_NAPI_H
#include <linux/netdevice.h>
#include <linux/tracepoint.h>
-DECLARE_TRACE(napi_poll,
+#define show_napi_state(state) state ? __print_flags(state, "|", \
+ { (1UL << NAPI_STATE_SCHED), "NAPI_STATE_SCHED" }, \
+ { (1UL << NAPI_STATE_DISABLE), "NAPI_STATE_DISABLE" }, \
+ { (1UL << NAPI_STATE_NPSVC), "NAPI_STATE_NPSVC" }) : "0"
+
+TRACE_EVENT(napi_poll,
+
TP_PROTO(struct napi_struct *napi),
- TP_ARGS(napi));
-#endif
+ TP_ARGS(napi),
+
+ TP_STRUCT__entry(
+ __field( unsigned long, state )
+ __field( int, weight )
+ __field( void *, poll )
+ __string( ifname, (napi->dev ?
+ napi->dev->name : "backlog") )
+ ),
+
+ TP_fast_assign(
+ __entry->state = napi->state;
+ __entry->weight = napi->weight;
+ __entry->poll = napi->poll;
+ __assign_str(ifname, napi->dev ? napi->dev->name : "backlog")
+ ),
+
+ TP_printk("ifname=%s state=%s weight=%d poll=%pf",
+ __get_str(ifname), show_napi_state(__entry->state),
+ __entry->weight, __entry->poll)
+);
+
+#endif /* _TRACE_NAPI_H */
+
+/* This part must be outside protection */
+#include <trace/define_trace.h>
--
1.6.1.2
^ permalink raw reply related
* Re: [PATCH] net: Fix sock freeing before sock_init_data() with __sk_free()
From: Eric Dumazet @ 2009-08-31 6:26 UTC (permalink / raw)
To: Jarek Poplawski; +Cc: David Miller, netdev
In-Reply-To: <20090830222340.GA17454@ami.dom.local>
Jarek Poplawski a écrit :
> After recent changes sk_free() frees socks conditionally and depends
> on sk_wmem_alloc beeing set e.g. in sock_init_data(). But in some
> cases sk_free() is called earlier, usually after other alloc errors.
> This patch fixes it by exporting and using __sk_free() directly.
>
> Signed-off-by: Jarek Poplawski <jarkao2@gmail.com>
> ---
>
> include/net/sock.h | 1 +
> net/ax25/af_ax25.c | 6 +++---
> net/core/sock.c | 6 ++++--
> net/irda/af_irda.c | 4 ++--
> net/tipc/socket.c | 2 +-
> 5 files changed, 11 insertions(+), 8 deletions(-)
>
> diff --git a/include/net/sock.h b/include/net/sock.h
> index 950409d..e76ef65 100644
> --- a/include/net/sock.h
> +++ b/include/net/sock.h
> @@ -935,6 +935,7 @@ extern void release_sock(struct sock *sk);
> extern struct sock *sk_alloc(struct net *net, int family,
> gfp_t priority,
> struct proto *prot);
> +extern void __sk_free(struct sock *sk);
> extern void sk_free(struct sock *sk);
> extern void sk_release_kernel(struct sock *sk);
> extern struct sock *sk_clone(const struct sock *sk,
> diff --git a/net/ax25/af_ax25.c b/net/ax25/af_ax25.c
> index da0f64f..c05738c 100644
> --- a/net/ax25/af_ax25.c
> +++ b/net/ax25/af_ax25.c
> @@ -851,7 +851,7 @@ static int ax25_create(struct net *net, struct socket *sock, int protocol)
>
> ax25 = sk->sk_protinfo = ax25_create_cb();
> if (!ax25) {
> - sk_free(sk);
> + __sk_free(sk);
> return -ENOMEM;
> }
>
> @@ -876,7 +876,7 @@ struct sock *ax25_make_new(struct sock *osk, struct ax25_dev *ax25_dev)
> return NULL;
>
> if ((ax25 = ax25_create_cb()) == NULL) {
> - sk_free(sk);
> + __sk_free(sk);
> return NULL;
> }
>
> @@ -886,7 +886,7 @@ struct sock *ax25_make_new(struct sock *osk, struct ax25_dev *ax25_dev)
> case SOCK_SEQPACKET:
> break;
> default:
> - sk_free(sk);
> + __sk_free(sk);
> ax25_cb_put(ax25);
> return NULL;
> }
> diff --git a/net/core/sock.c b/net/core/sock.c
> index bbb25be..92793be 100644
> --- a/net/core/sock.c
> +++ b/net/core/sock.c
> @@ -1031,7 +1031,7 @@ struct sock *sk_alloc(struct net *net, int family, gfp_t priority,
> }
> EXPORT_SYMBOL(sk_alloc);
>
> -static void __sk_free(struct sock *sk)
> +void __sk_free(struct sock *sk)
> {
> struct sk_filter *filter;
>
> @@ -1054,13 +1054,15 @@ static void __sk_free(struct sock *sk)
> put_net(sock_net(sk));
> sk_prot_free(sk->sk_prot_creator, sk);
> }
> +EXPORT_SYMBOL(__sk_free);
>
> void sk_free(struct sock *sk)
> {
> /*
> * We substract one from sk_wmem_alloc and can know if
> * some packets are still in some tx queue.
> - * If not null, sock_wfree() will call __sk_free(sk) later
> + * If not null, sock_wfree() will call __sk_free(sk) later.
> + * (Before sock_init_data() etc. use __sk_free() instead.)
> */
> if (atomic_dec_and_test(&sk->sk_wmem_alloc))
> __sk_free(sk);
> diff --git a/net/irda/af_irda.c b/net/irda/af_irda.c
> index 50b43c5..9ed2060 100644
> --- a/net/irda/af_irda.c
> +++ b/net/irda/af_irda.c
> @@ -1118,12 +1118,12 @@ static int irda_create(struct net *net, struct socket *sock, int protocol)
> self->max_sdu_size_rx = TTP_SAR_UNBOUND;
> break;
> default:
> - sk_free(sk);
> + __sk_free(sk);
> return -ESOCKTNOSUPPORT;
> }
> break;
> default:
> - sk_free(sk);
> + __sk_free(sk);
> return -ESOCKTNOSUPPORT;
> }
>
> diff --git a/net/tipc/socket.c b/net/tipc/socket.c
> index 1848693..4c8435a 100644
> --- a/net/tipc/socket.c
> +++ b/net/tipc/socket.c
> @@ -228,7 +228,7 @@ static int tipc_create(struct net *net, struct socket *sock, int protocol)
> tp_ptr = tipc_createport_raw(sk, &dispatch, &wakeupdispatch,
> TIPC_LOW_IMPORTANCE);
> if (unlikely(!tp_ptr)) {
> - sk_free(sk);
> + __sk_free(sk);
> return -ENOMEM;
> }
>
> --
Very nice catch Jarek, but dont you think it would be cleaner to make sure
we can call sk_free() right after sk_alloc() instead, and not exporting
__sk_free() ?
ie initialize wmem_alloc in sk_alloc() instead of initializing it in
sock_init_data() ?
^ permalink raw reply
* Re: [PATCH resend] drop_monitor: fix trace_napi_poll_hit()
From: Eric Dumazet @ 2009-08-31 6:31 UTC (permalink / raw)
To: Xiao Guangrong; +Cc: David Miller, Neil Horman, Wei Yongjun, Netdev, LKML
In-Reply-To: <4A9B6963.5090207@cn.fujitsu.com>
Xiao Guangrong a écrit :
> The net_dev of backlog napi is NULL, like below:
>
> __get_cpu_var(softnet_data).backlog.dev == NULL
>
> So, we should check it in napi tracepoint's probe function
>
> Acked-by: Neil Horman <nhorman@tuxdriver.com>
> Signed-off-by: Xiao Guangrong <xiaoguangrong@cn.fujitsu.com>
> ---
> net/core/drop_monitor.c | 3 ++-
> 1 files changed, 2 insertions(+), 1 deletions(-)
>
> diff --git a/net/core/drop_monitor.c b/net/core/drop_monitor.c
> index 9d66fa9..d311202 100644
> --- a/net/core/drop_monitor.c
> +++ b/net/core/drop_monitor.c
> @@ -182,7 +182,8 @@ static void trace_napi_poll_hit(struct napi_struct *napi)
> /*
> * Ratelimit our check time to dm_hw_check_delta jiffies
> */
> - if (!time_after(jiffies, napi->dev->last_rx + dm_hw_check_delta))
> + if (!napi->dev ||
> + !time_after(jiffies, napi->dev->last_rx + dm_hw_check_delta))
> return;
>
> rcu_read_lock();
This reminds me dev->last_rx is not anymore updated, unless special conditions
are met.
Test done in trace_napi_poll_hit() is probably not good, even with a non null napi->dev
^ permalink raw reply
* Re: [PATCH] net: Fix sock freeing before sock_init_data() with __sk_free()
From: Jarek Poplawski @ 2009-08-31 6:36 UTC (permalink / raw)
To: Eric Dumazet; +Cc: David Miller, netdev
In-Reply-To: <4A9B6D23.9090505@gmail.com>
On Mon, Aug 31, 2009 at 08:26:43AM +0200, Eric Dumazet wrote:
> Jarek Poplawski a écrit :
> > After recent changes sk_free() frees socks conditionally and depends
> > on sk_wmem_alloc beeing set e.g. in sock_init_data(). But in some
> > cases sk_free() is called earlier, usually after other alloc errors.
> > This patch fixes it by exporting and using __sk_free() directly.
...
> Very nice catch Jarek, but dont you think it would be cleaner to make sure
> we can call sk_free() right after sk_alloc() instead, and not exporting
> __sk_free() ?
>
> ie initialize wmem_alloc in sk_alloc() instead of initializing it in
> sock_init_data() ?
>
Most probably it should be better. But I meant this fix for -net and
didn't wan't to break too much... So, if you're sure it's OK feel free
to send your version. (Or it could be changed like this in the -next.)
Jarek P.
^ permalink raw reply
* Re: [PATCH] net: Fix sock freeing before sock_init_data() with __sk_free()
From: Eric Dumazet @ 2009-08-31 6:50 UTC (permalink / raw)
To: Jarek Poplawski; +Cc: David Miller, netdev
In-Reply-To: <20090831063648.GB5005@ff.dom.local>
Jarek Poplawski a écrit :
> On Mon, Aug 31, 2009 at 08:26:43AM +0200, Eric Dumazet wrote:
>> Jarek Poplawski a écrit :
>>> After recent changes sk_free() frees socks conditionally and depends
>>> on sk_wmem_alloc beeing set e.g. in sock_init_data(). But in some
>>> cases sk_free() is called earlier, usually after other alloc errors.
>>> This patch fixes it by exporting and using __sk_free() directly.
> ...
>> Very nice catch Jarek, but dont you think it would be cleaner to make sure
>> we can call sk_free() right after sk_alloc() instead, and not exporting
>> __sk_free() ?
>>
>> ie initialize wmem_alloc in sk_alloc() instead of initializing it in
>> sock_init_data() ?
>>
>
> Most probably it should be better. But I meant this fix for -net and
> didn't wan't to break too much... So, if you're sure it's OK feel free
> to send your version. (Or it could be changed like this in the -next.)
Well, patch is yours, not mine, and I am confident it is OK.
We should check that no sk_alloc() user did a blind memset() or something
strange like that, before calling sock_init_data() or sk_free()
diff --git a/net/core/sock.c b/net/core/sock.c
index bbb25be..7633422 100644
--- a/net/core/sock.c
+++ b/net/core/sock.c
@@ -1025,6 +1025,7 @@ struct sock *sk_alloc(struct net *net, int family, gfp_t priority,
sk->sk_prot = sk->sk_prot_creator = prot;
sock_lock_init(sk);
sock_net_set(sk, get_net(net));
+ atomic_set(&sk->sk_wmem_alloc, 1);
}
return sk;
@@ -1872,7 +1873,6 @@ void sock_init_data(struct socket *sock, struct sock *sk)
*/
smp_wmb();
atomic_set(&sk->sk_refcnt, 1);
- atomic_set(&sk->sk_wmem_alloc, 1);
atomic_set(&sk->sk_drops, 0);
}
EXPORT_SYMBOL(sock_init_data);
^ permalink raw reply related
* Re: [PATCH] net: Fix sock freeing before sock_init_data() with __sk_free()
From: Jarek Poplawski @ 2009-08-31 7:07 UTC (permalink / raw)
To: Eric Dumazet; +Cc: David Miller, netdev
In-Reply-To: <4A9B72B1.2040908@gmail.com>
On Mon, Aug 31, 2009 at 08:50:25AM +0200, Eric Dumazet wrote:
> Jarek Poplawski a écrit :
> > On Mon, Aug 31, 2009 at 08:26:43AM +0200, Eric Dumazet wrote:
> >> Jarek Poplawski a écrit :
> >>> After recent changes sk_free() frees socks conditionally and depends
> >>> on sk_wmem_alloc beeing set e.g. in sock_init_data(). But in some
> >>> cases sk_free() is called earlier, usually after other alloc errors.
> >>> This patch fixes it by exporting and using __sk_free() directly.
> > ...
> >> Very nice catch Jarek, but dont you think it would be cleaner to make sure
> >> we can call sk_free() right after sk_alloc() instead, and not exporting
> >> __sk_free() ?
> >>
> >> ie initialize wmem_alloc in sk_alloc() instead of initializing it in
> >> sock_init_data() ?
> >>
> >
> > Most probably it should be better. But I meant this fix for -net and
> > didn't wan't to break too much... So, if you're sure it's OK feel free
> > to send your version. (Or it could be changed like this in the -next.)
>
> Well, patch is yours, not mine, and I am confident it is OK.
Well, it's from you, and I guess you'll sign off too, but if you
think so...
Signed-off-by: Jarek Poplawski <jarkao2@gmail.com>
Thanks,
Jarek P.
>
> We should check that no sk_alloc() user did a blind memset() or something
> strange like that, before calling sock_init_data() or sk_free()
>
> diff --git a/net/core/sock.c b/net/core/sock.c
> index bbb25be..7633422 100644
> --- a/net/core/sock.c
> +++ b/net/core/sock.c
> @@ -1025,6 +1025,7 @@ struct sock *sk_alloc(struct net *net, int family, gfp_t priority,
> sk->sk_prot = sk->sk_prot_creator = prot;
> sock_lock_init(sk);
> sock_net_set(sk, get_net(net));
> + atomic_set(&sk->sk_wmem_alloc, 1);
> }
>
> return sk;
> @@ -1872,7 +1873,6 @@ void sock_init_data(struct socket *sock, struct sock *sk)
> */
> smp_wmb();
> atomic_set(&sk->sk_refcnt, 1);
> - atomic_set(&sk->sk_wmem_alloc, 1);
> atomic_set(&sk->sk_drops, 0);
> }
> EXPORT_SYMBOL(sock_init_data);
>
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox