* Re: Question about __alloc_skb() speedup
From: Eric Dumazet @ 2010-12-04 14:49 UTC (permalink / raw)
To: Junchang Wang; +Cc: netdev
In-Reply-To: <1291474058.2806.96.camel@edumazet-laptop>
Le samedi 04 décembre 2010 à 15:47 +0100, Eric Dumazet a écrit :
> Le samedi 04 décembre 2010 à 22:18 +0800, Junchang Wang a écrit :
>
> > I added the prefetchw() in pktgen as follows:
> >
> > diff --git a/net/core/pktgen.c b/net/core/pktgen.c
> > index 2953b2a..512f1ae 100644
> > --- a/net/core/pktgen.c
> > +++ b/net/core/pktgen.c
> > @@ -2660,6 +2660,7 @@ static struct sk_buff *fill_packet_ipv4(struct net_device *odev,
> > sprintf(pkt_dev->result, "No memory");
> > return NULL;
> > }
> > + prefetchw(skb->data);
> >
> > skb_reserve(skb, datalen);
> >
> > This time, I can check it without rebooting the system. The performance
> > gain is 4%-5%(stable). Does 4% worth submitting it to the kernel?
>
> Yes I believe so, pktgen being very specific, but I have few questions :
>
> Is it with SLUB or SLAB ?
>
> How many buffers in TX ring on you nic (ethtool -g eth0) ?
>
> What is the datalen value here ? (you prefetch, then advance skb->data)
>
> 32 or 64bit kernel ?
>
> How many pps do you get before and after patch ?
>
> Thanks
>
Also, dont forget to include the prefetchw() in fill_packet_ipv6() as
well when submitting your patch.
^ permalink raw reply
* Re: [PATCH 3/3] ifb: move tq from ifb_private
From: jamal @ 2010-12-04 14:48 UTC (permalink / raw)
To: Jarek Poplawski; +Cc: Changli Gao, netdev
In-Reply-To: <1291472889.2159.18.camel@mojatatu>
On Sat, 2010-12-04 at 09:28 -0500, jamal wrote:
> The idea was to defer processing at the first error. Changli
> is changing it to continue despite the error.
> The initial goal was to yield whenever possible since we dont maintain
> a lot of state.
Changli:
Other points we could defer processing is in case of packets
being dropped by dev_queue_xmit and netif_rx
cheers,
jamal
^ permalink raw reply
* Re: Question about __alloc_skb() speedup
From: Eric Dumazet @ 2010-12-04 14:47 UTC (permalink / raw)
To: Junchang Wang; +Cc: netdev
In-Reply-To: <20101204141826.GA5830@Desktop-Junchang>
Le samedi 04 décembre 2010 à 22:18 +0800, Junchang Wang a écrit :
> I added the prefetchw() in pktgen as follows:
>
> diff --git a/net/core/pktgen.c b/net/core/pktgen.c
> index 2953b2a..512f1ae 100644
> --- a/net/core/pktgen.c
> +++ b/net/core/pktgen.c
> @@ -2660,6 +2660,7 @@ static struct sk_buff *fill_packet_ipv4(struct net_device *odev,
> sprintf(pkt_dev->result, "No memory");
> return NULL;
> }
> + prefetchw(skb->data);
>
> skb_reserve(skb, datalen);
>
> This time, I can check it without rebooting the system. The performance
> gain is 4%-5%(stable). Does 4% worth submitting it to the kernel?
Yes I believe so, pktgen being very specific, but I have few questions :
Is it with SLUB or SLAB ?
How many buffers in TX ring on you nic (ethtool -g eth0) ?
What is the datalen value here ? (you prefetch, then advance skb->data)
32 or 64bit kernel ?
How many pps do you get before and after patch ?
Thanks
^ permalink raw reply
* Re: [PATCH 3/3] ifb: move tq from ifb_private
From: Changli Gao @ 2010-12-04 14:45 UTC (permalink / raw)
To: hadi; +Cc: Jarek Poplawski, netdev
In-Reply-To: <1291472889.2159.18.camel@mojatatu>
On Sat, Dec 4, 2010 at 10:28 PM, jamal <hadi@cyberus.ca> wrote:
> On Sat, 2010-12-04 at 14:15 +0100, Jarek Poplawski wrote:
>
>> > @@ -87,7 +83,7 @@ static void ri_tasklet(unsigned long dev)
>> > rcu_read_unlock();
>> > dev_kfree_skb(skb);
>> > stats->tx_dropped++;
>> > - break;
>> > + continue;
>>
>> IMHO this line is a bugfix and should be a separate patch (for stable).
>
> Should be separate, yes.
> Bug? no.
If we breaks the loop when there are still skbs in tq and no skb in
rq, the skbs will be left in txq until new skbs are enqueued into rq.
In rare cases, no new skb is queued, then these skbs will stay in rq
forever.
- if (__netif_tx_trylock(txq)) {
- if ((skb = skb_peek(&dp->rq)) == NULL) {
- dp->tasklet_pending = 0;
- if (netif_queue_stopped(_dev))
- netif_wake_queue(_dev);
- } else {
- __netif_tx_unlock(txq);
- goto resched;
- }
- __netif_tx_unlock(txq);
- } else {
-resched:
- dp->tasklet_pending = 1;
- tasklet_schedule(&dp->ifb_tasklet);
- }
-
--
Regards,
Changli Gao(xiaosuo@gmail.com)
^ permalink raw reply
* Re: [PATCH 3/3] ifb: move tq from ifb_private
From: Changli Gao @ 2010-12-04 14:20 UTC (permalink / raw)
To: hadi; +Cc: netdev
In-Reply-To: <1291472282.2159.8.camel@mojatatu>
On Sat, Dec 4, 2010 at 10:18 PM, jamal <hadi@cyberus.ca> wrote:
>
> I like the splice idea but this patch makes me twitch
> a little. What test setup did you use to check it?
>
Just a simple test:
tc qdisc add dev eth0 ingress
tc filter add dev eth0 parent ffff: protocol ip basic action mirred
egress redirect dev ifb0
--
Regards,
Changli Gao(xiaosuo@gmail.com)
^ permalink raw reply
* Re: [PATCH 3/3] ifb: move tq from ifb_private
From: jamal @ 2010-12-04 14:42 UTC (permalink / raw)
To: Changli Gao; +Cc: netdev
In-Reply-To: <1291472282.2159.8.camel@mojatatu>
On Sat, 2010-12-04 at 09:18 -0500, jamal wrote:
> I like the splice idea but this patch makes me twitch
> a little. What test setup did you use to check it?
Ok, here's one thing you changed which is important. We do:
-->XXX-->rq-->tq-->XXX-->
rq is controlled by queue limit.
We only load rq to tq if all of tq is empty. If it is not
we dont move things over. Essentially this is a flow
control scheme. We dont want many sources to be overwhelming
us with packets and every time we grab a txqlen number of packets.
For this reason:
I would be comfortable if all you did was to add the splice
after you skb_peek() - i think that would be a good improvement
which is not bound to break anything else.
cheers,
jamal
^ permalink raw reply
* Re: [PATCH 3/3] ifb: move tq from ifb_private
From: jamal @ 2010-12-04 14:28 UTC (permalink / raw)
To: Jarek Poplawski; +Cc: Changli Gao, netdev
In-Reply-To: <4CFA3F01.20109@gmail.com>
On Sat, 2010-12-04 at 14:15 +0100, Jarek Poplawski wrote:
> > @@ -87,7 +83,7 @@ static void ri_tasklet(unsigned long dev)
> > rcu_read_unlock();
> > dev_kfree_skb(skb);
> > stats->tx_dropped++;
> > - break;
> > + continue;
>
> IMHO this line is a bugfix and should be a separate patch (for stable).
Should be separate, yes.
Bug? no.
Improvement, maybe ;->
The idea was to defer processing at the first error. Changli
is changing it to continue despite the error.
The initial goal was to yield whenever possible since we dont maintain
a lot of state.
cheers,
jamal
^ permalink raw reply
* Re: [PATCH 3/3] ifb: move tq from ifb_private
From: jamal @ 2010-12-04 14:18 UTC (permalink / raw)
To: Changli Gao; +Cc: netdev
In-Reply-To: <1291442121-3302-3-git-send-email-xiaosuo@gmail.com>
On Sat, 2010-12-04 at 13:55 +0800, Changli Gao wrote:
> tq is only used in ri_tasklet, so we move it from ifb_private to a
> stack variable of ri_tasklet.
>
> skb_queue_splice_tail_init() is used instead of the open coded and slow
> one.
I like the splice idea but this patch makes me twitch
a little. What test setup did you use to check it?
cheers,
jamal
^ permalink raw reply
* Re: Question about __alloc_skb() speedup
From: Junchang Wang @ 2010-12-04 14:18 UTC (permalink / raw)
To: Eric Dumazet; +Cc: netdev
In-Reply-To: <1291373429.2897.96.camel@edumazet-laptop>
On Fri, Dec 03, 2010 at 11:50:29AM +0100, Eric Dumazet wrote:
>
>This is because __alloc_skb() is generic :
>
>We dont know if the skb->data is going to be used right after or not at
>all.
>
>For example, NIC drivers call __alloc_skb() to refill their RX ring
>buffer. There is no gain to prefetch data in this case since the data is
>going to be written by the NIC hardware. The reverse would be needed
>actually : ask to local cpu to evict data from its cache, so that device
>can DMA it faster (less bus transactions)
Thanks for your explanation. Now I understand your patch. :)
>
>By the way, adding prefetchw() right before the "return skb;" is
>probably not very useful.
Did you mean adding prefetchw() right before the "return" instruction
doesn't gain beneficial effect?
My understanding is that what prefetch instructions do is to hint cpu
to hot some cache lines, so this kind of prefetch could also benefit
following functions.
>You can certainly try to add the prefetchw()
>in pktgen itself, since you know for sure you are going to write the
>data.
>
>I dont understand your 10% speedup because pktgen actually uses
>__netdev_alloc_skb(), so it calls skb_reserve(skb, NET_SKB_PAD) : your
>prefetchw is bringing a cache line that wont be used at all by pktgen.
Thanks for corrections. Please check the following code.
>
>I would say 10% sounds highly suspect to me...
>
I repeated the experiment a number of times (>10). The number of 10% was
careless, but I confirmed there's speedup, which fall into (%3, 8%). I
found it hard to give the exact number of speedup because I had to reboot
the system each time I added the prefetch code.
I noticed the hardware prefetch(Hardware Prefetcher and Adjacent Cache
Line Prefetch) was turn on by default. I doubt my faulty code gain benefit
from hardware prefetch. Without those two options, the performance of
both pktgens reduced by 5%-8% and I can hardly see beneficial effect from
my previous code.
I added the prefetchw() in pktgen as follows:
diff --git a/net/core/pktgen.c b/net/core/pktgen.c
index 2953b2a..512f1ae 100644
--- a/net/core/pktgen.c
+++ b/net/core/pktgen.c
@@ -2660,6 +2660,7 @@ static struct sk_buff *fill_packet_ipv4(struct net_device *odev,
sprintf(pkt_dev->result, "No memory");
return NULL;
}
+ prefetchw(skb->data);
skb_reserve(skb, datalen);
This time, I can check it without rebooting the system. The performance
gain is 4%-5%(stable). Does 4% worth submitting it to the kernel?
Thanks.
--Junchang
^ permalink raw reply related
* Re: [PATCH 2/3] ifb: remove unused macro TX_TIMEOUT
From: jamal @ 2010-12-04 14:13 UTC (permalink / raw)
To: Changli Gao; +Cc: netdev
In-Reply-To: <1291442121-3302-2-git-send-email-xiaosuo@gmail.com>
On Sat, 2010-12-04 at 13:55 +0800, Changli Gao wrote:
> Signed-off-by: Changli Gao <xiaosuo@gmail.com>
Signed-off-by: Jamal Hadi Salim <hadi@cyberus.ca>
cheers,
jamal
^ permalink raw reply
* Re: [PATCH 1/3] ifb: remove the useless debug stats
From: jamal @ 2010-12-04 14:12 UTC (permalink / raw)
To: Changli Gao; +Cc: netdev
In-Reply-To: <1291442121-3302-1-git-send-email-xiaosuo@gmail.com>
On Sat, 2010-12-04 at 13:55 +0800, Changli Gao wrote:
> These debug stats are not exported, and become useless.
>
> Signed-off-by: Changli Gao <xiaosuo@gmail.com>
Given the complexity of ifb, these stats have proved useful over
the years - I would typically ask someone to printk them etc.
If you want me to get me excited (in a good way) you could export these
via link XSTATS ;-> If you dont have time, then it is likely
we are stable enough we can kill them in which case, here you go:
Signed-off-by: Jamal Hadi Salim <hadi@cyberus.ca>
cheers,
jamal
^ permalink raw reply
* Re: [PATCH] net: init ingress queue
From: Jarek Poplawski @ 2010-12-04 13:36 UTC (permalink / raw)
To: Changli Gao
Cc: David S. Miller, Eric Dumazet, Tom Herbert, Jiri Pirko, netdev
In-Reply-To: <1291465901-3189-1-git-send-email-xiaosuo@gmail.com>
Changli Gao wrote:
> The dev field of ingress queue is forgot to initialized, then NULL
> pointer dereference happens in qdisc_alloc().
>
> Move inits of tx queues to netif_alloc_netdev_queues().
>
> Signed-off-by: Changli Gao <xiaosuo@gmail.com>
> ---
> v2: factorize the two inits in netdev_init_queues() and move inits of
> tx queues to netif_alloc_netdev_queues().
> net/core/dev.c | 35 +++++++++++++----------------------
> 1 file changed, 13 insertions(+), 22 deletions(-)
> diff --git a/net/core/dev.c b/net/core/dev.c
> index cd24374..4a7fc32 100644
> --- a/net/core/dev.c
> +++ b/net/core/dev.c
I guess you should mention it's net-next problem, because some people
might be unnecessarily concerned (why they don't have such an oops ;-)
Jarek P.
^ permalink raw reply
* Re: [PATCH 3/3] ifb: move tq from ifb_private
From: Changli Gao @ 2010-12-04 13:29 UTC (permalink / raw)
To: Jarek Poplawski; +Cc: jamal, netdev
In-Reply-To: <4CFA3F01.20109@gmail.com>
On Sat, Dec 4, 2010 at 9:15 PM, Jarek Poplawski <jarkao2@gmail.com> wrote:
> Changli Gao wrote:
>> tq is only used in ri_tasklet, so we move it from ifb_private to a
>> stack variable of ri_tasklet.
>>
>> skb_queue_splice_tail_init() is used instead of the open coded and slow
>> one.
>>
>> Signed-off-by: Changli Gao <xiaosuo@gmail.com>
>> ---
>> drivers/net/ifb.c | 49 ++++++++++++-------------------------------------
>> 1 file changed, 12 insertions(+), 37 deletions(-)
>> diff --git a/drivers/net/ifb.c b/drivers/net/ifb.c
>> index d1e362a..cd6e90d 100644
>> --- a/drivers/net/ifb.c
>> +++ b/drivers/net/ifb.c
>> @@ -39,9 +39,7 @@
>> #define TX_Q_LIMIT 32
>> struct ifb_private {
>> struct tasklet_struct ifb_tasklet;
>> - int tasklet_pending;
>> struct sk_buff_head rq;
>> - struct sk_buff_head tq;
>> };
>>
>> static int numifbs = 2;
>> @@ -53,27 +51,25 @@ static int ifb_close(struct net_device *dev);
>>
>> static void ri_tasklet(unsigned long dev)
>> {
>> -
>> struct net_device *_dev = (struct net_device *)dev;
>> struct ifb_private *dp = netdev_priv(_dev);
>> struct net_device_stats *stats = &_dev->stats;
>> struct netdev_queue *txq;
>> struct sk_buff *skb;
>> + struct sk_buff_head tq;
>>
>> + __skb_queue_head_init(&tq);
>> txq = netdev_get_tx_queue(_dev, 0);
>> - if ((skb = skb_peek(&dp->tq)) == NULL) {
>> - if (__netif_tx_trylock(txq)) {
>> - while ((skb = skb_dequeue(&dp->rq)) != NULL) {
>> - skb_queue_tail(&dp->tq, skb);
>> - }
>> - __netif_tx_unlock(txq);
>> - } else {
>> - /* reschedule */
>> - goto resched;
>> - }
>> + if (!__netif_tx_trylock(txq)) {
>> + tasklet_schedule(&dp->ifb_tasklet);
>> + return;
>> }
>> + skb_queue_splice_tail_init(&dp->rq, &tq);
>> + if (netif_tx_queue_stopped(txq))
>> + netif_tx_wake_queue(txq);
>> + __netif_tx_unlock(txq);
>>
>> - while ((skb = skb_dequeue(&dp->tq)) != NULL) {
>> + while ((skb = __skb_dequeue(&tq)) != NULL) {
>> u32 from = G_TC_FROM(skb->tc_verd);
>>
>> skb->tc_verd = 0;
>> @@ -87,7 +83,7 @@ static void ri_tasklet(unsigned long dev)
>> rcu_read_unlock();
>> dev_kfree_skb(skb);
>> stats->tx_dropped++;
>> - break;
>> + continue;
>
> IMHO this line is a bugfix and should be a separate patch (for stable).
It sounds reasonable.
>
> The rest looks OK to me but you could probably skip locking of dp->rq
> similarly to tq.
>
OK. Thanks.
--
Regards,
Changli Gao(xiaosuo@gmail.com)
^ permalink raw reply
* Re: [PATCH 3/3] ifb: move tq from ifb_private
From: Jarek Poplawski @ 2010-12-04 13:15 UTC (permalink / raw)
To: Changli Gao; +Cc: jamal, netdev
In-Reply-To: <1291442121-3302-3-git-send-email-xiaosuo@gmail.com>
Changli Gao wrote:
> tq is only used in ri_tasklet, so we move it from ifb_private to a
> stack variable of ri_tasklet.
>
> skb_queue_splice_tail_init() is used instead of the open coded and slow
> one.
>
> Signed-off-by: Changli Gao <xiaosuo@gmail.com>
> ---
> drivers/net/ifb.c | 49 ++++++++++++-------------------------------------
> 1 file changed, 12 insertions(+), 37 deletions(-)
> diff --git a/drivers/net/ifb.c b/drivers/net/ifb.c
> index d1e362a..cd6e90d 100644
> --- a/drivers/net/ifb.c
> +++ b/drivers/net/ifb.c
> @@ -39,9 +39,7 @@
> #define TX_Q_LIMIT 32
> struct ifb_private {
> struct tasklet_struct ifb_tasklet;
> - int tasklet_pending;
> struct sk_buff_head rq;
> - struct sk_buff_head tq;
> };
>
> static int numifbs = 2;
> @@ -53,27 +51,25 @@ static int ifb_close(struct net_device *dev);
>
> static void ri_tasklet(unsigned long dev)
> {
> -
> struct net_device *_dev = (struct net_device *)dev;
> struct ifb_private *dp = netdev_priv(_dev);
> struct net_device_stats *stats = &_dev->stats;
> struct netdev_queue *txq;
> struct sk_buff *skb;
> + struct sk_buff_head tq;
>
> + __skb_queue_head_init(&tq);
> txq = netdev_get_tx_queue(_dev, 0);
> - if ((skb = skb_peek(&dp->tq)) == NULL) {
> - if (__netif_tx_trylock(txq)) {
> - while ((skb = skb_dequeue(&dp->rq)) != NULL) {
> - skb_queue_tail(&dp->tq, skb);
> - }
> - __netif_tx_unlock(txq);
> - } else {
> - /* reschedule */
> - goto resched;
> - }
> + if (!__netif_tx_trylock(txq)) {
> + tasklet_schedule(&dp->ifb_tasklet);
> + return;
> }
> + skb_queue_splice_tail_init(&dp->rq, &tq);
> + if (netif_tx_queue_stopped(txq))
> + netif_tx_wake_queue(txq);
> + __netif_tx_unlock(txq);
>
> - while ((skb = skb_dequeue(&dp->tq)) != NULL) {
> + while ((skb = __skb_dequeue(&tq)) != NULL) {
> u32 from = G_TC_FROM(skb->tc_verd);
>
> skb->tc_verd = 0;
> @@ -87,7 +83,7 @@ static void ri_tasklet(unsigned long dev)
> rcu_read_unlock();
> dev_kfree_skb(skb);
> stats->tx_dropped++;
> - break;
> + continue;
IMHO this line is a bugfix and should be a separate patch (for stable).
The rest looks OK to me but you could probably skip locking of dp->rq
similarly to tq.
Jarek P.
> }
> rcu_read_unlock();
> skb->skb_iif = _dev->ifindex;
> @@ -100,23 +96,6 @@ static void ri_tasklet(unsigned long dev)
> } else
> BUG();
> }
> -
> - if (__netif_tx_trylock(txq)) {
> - if ((skb = skb_peek(&dp->rq)) == NULL) {
> - dp->tasklet_pending = 0;
> - if (netif_queue_stopped(_dev))
> - netif_wake_queue(_dev);
> - } else {
> - __netif_tx_unlock(txq);
> - goto resched;
> - }
> - __netif_tx_unlock(txq);
> - } else {
> -resched:
> - dp->tasklet_pending = 1;
> - tasklet_schedule(&dp->ifb_tasklet);
> - }
> -
> }
>
> static const struct net_device_ops ifb_netdev_ops = {
> @@ -162,10 +141,8 @@ static netdev_tx_t ifb_xmit(struct sk_buff *skb, struct net_device *dev)
> }
>
> skb_queue_tail(&dp->rq, skb);
> - if (!dp->tasklet_pending) {
> - dp->tasklet_pending = 1;
> + if (skb_queue_len(&dp->rq) == 1)
> tasklet_schedule(&dp->ifb_tasklet);
> - }
>
> return NETDEV_TX_OK;
> }
> @@ -177,7 +154,6 @@ static int ifb_close(struct net_device *dev)
> tasklet_kill(&dp->ifb_tasklet);
> netif_stop_queue(dev);
> skb_queue_purge(&dp->rq);
> - skb_queue_purge(&dp->tq);
> return 0;
> }
>
> @@ -187,7 +163,6 @@ static int ifb_open(struct net_device *dev)
>
> tasklet_init(&dp->ifb_tasklet, ri_tasklet, (unsigned long)dev);
> skb_queue_head_init(&dp->rq);
> - skb_queue_head_init(&dp->tq);
> netif_start_queue(dev);
>
> return 0;
^ permalink raw reply
* [PATCH] net: init ingress queue
From: Changli Gao @ 2010-12-04 12:31 UTC (permalink / raw)
To: David S. Miller
Cc: Eric Dumazet, Tom Herbert, Jiri Pirko, netdev, Changli Gao
The dev field of ingress queue is forgot to initialized, then NULL
pointer dereference happens in qdisc_alloc().
Move inits of tx queues to netif_alloc_netdev_queues().
Signed-off-by: Changli Gao <xiaosuo@gmail.com>
---
v2: factorize the two inits in netdev_init_queues() and move inits of
tx queues to netif_alloc_netdev_queues().
net/core/dev.c | 35 +++++++++++++----------------------
1 file changed, 13 insertions(+), 22 deletions(-)
diff --git a/net/core/dev.c b/net/core/dev.c
index cd24374..4a7fc32 100644
--- a/net/core/dev.c
+++ b/net/core/dev.c
@@ -5109,11 +5109,21 @@ static int netif_alloc_rx_queues(struct net_device *dev)
}
#endif
+static void netdev_init_one_queue(struct net_device *dev,
+ struct netdev_queue *queue, void *_unused)
+{
+ /* Initialize queue lock */
+ spin_lock_init(&queue->_xmit_lock);
+ netdev_set_xmit_lockdep_class(&queue->_xmit_lock, dev->type);
+ queue->xmit_lock_owner = -1;
+ netdev_queue_numa_node_write(queue, -1);
+ queue->dev = dev;
+}
+
static int netif_alloc_netdev_queues(struct net_device *dev)
{
unsigned int count = dev->num_tx_queues;
struct netdev_queue *tx;
- int i;
BUG_ON(count < 1);
@@ -5125,27 +5135,10 @@ static int netif_alloc_netdev_queues(struct net_device *dev)
}
dev->_tx = tx;
- for (i = 0; i < count; i++) {
- netdev_queue_numa_node_write(&tx[i], -1);
- tx[i].dev = dev;
- }
- return 0;
-}
-
-static void netdev_init_one_queue(struct net_device *dev,
- struct netdev_queue *queue,
- void *_unused)
-{
- /* Initialize queue lock */
- spin_lock_init(&queue->_xmit_lock);
- netdev_set_xmit_lockdep_class(&queue->_xmit_lock, dev->type);
- queue->xmit_lock_owner = -1;
-}
-
-static void netdev_init_queues(struct net_device *dev)
-{
netdev_for_each_tx_queue(dev, netdev_init_one_queue, NULL);
spin_lock_init(&dev->tx_global_lock);
+
+ return 0;
}
/**
@@ -5184,8 +5177,6 @@ int register_netdevice(struct net_device *dev)
dev->iflink = -1;
- netdev_init_queues(dev);
-
/* Init, if this function is available */
if (dev->netdev_ops->ndo_init) {
ret = dev->netdev_ops->ndo_init(dev);
^ permalink raw reply related
* Re: [PATCH 1/2] vsprintf: Add %pMbt, bluetooth mac address
From: Michał Mirosław @ 2010-12-04 11:03 UTC (permalink / raw)
To: Joe Perches; +Cc: Marcel Holtmann, Gustavo F. Padovan, linux-kernel, netdev
In-Reply-To: <a33a0b00eeb29713a08d91156cbb2d816176f990.1291419007.git.joe@perches.com>
2010/12/4 Joe Perches <joe@perches.com>:
> Bluetooth output the MAC address in reverse order.
> Bluetooth memory order: 00 01 02 03 04 05 is output "05:04:03:02:01:00".
>
> This can save overall text when bluetooth is compiled in.
>
> Bluetooth currently uses a very slightly unsafe local function (batostr)
> to output these formatted addresses.
>
> Adding %pMbt allows the batostr function to be removed.
Just a nitpick:
You could call it %pMR, as in 'Reverse', so it sounds better when/if
some other subsystem uses it. It would also be a hint of what is this
doing instead of where it came from.
Best Regards,
Michał Mirosław
^ permalink raw reply
* Re: [PATCH] net: init ingress queue
From: Changli Gao @ 2010-12-04 8:55 UTC (permalink / raw)
To: Eric Dumazet; +Cc: David S. Miller, Tom Herbert, Jiri Pirko, netdev
In-Reply-To: <1291452427.2806.90.camel@edumazet-laptop>
On Sat, Dec 4, 2010 at 4:47 PM, Eric Dumazet <eric.dumazet@gmail.com> wrote:
> Le samedi 04 décembre 2010 à 13:45 +0800, Changli Gao a écrit :
>> The dev field of ingress queue is forgot to initialized, then NULL
>> pointer dereference happens in qdisc_alloc().
>
> < deleted oops >
>
>> Signed-off-by: Changli Gao <xiaosuo@gmail.com>
>> ---
>> net/core/dev.c | 2 ++
>> 1 file changed, 2 insertions(+)
>> diff --git a/net/core/dev.c b/net/core/dev.c
>> index cd24374..8083c68 100644
>> --- a/net/core/dev.c
>> +++ b/net/core/dev.c
>> @@ -5577,6 +5577,8 @@ struct netdev_queue *dev_ingress_queue_create(struct net_device *dev)
>> queue = kzalloc(sizeof(*queue), GFP_KERNEL);
>> if (!queue)
>> return NULL;
>> + netdev_queue_numa_node_write(queue, -1);
>> + queue->dev = dev;
>> netdev_init_one_queue(dev, queue, NULL);
>> queue->qdisc = &noop_qdisc;
>> queue->qdisc_sleeping = &noop_qdisc;
>
> Hi Changli, thanks for bug report and patch.
>
> IMHO there is no point to include this long Oops report in Changelog for
> a net-next-2.6 temporary problem, there wont be any bugzilla report.
>
OK, Thanks.
> Instead, you could say it is a followup patch for commits 1d24eb4815d1e0
> and f2cd2d3e9b3ef960. Two or three lines with relevant information.
>
> I suggest you submit following patch instead, as this seems cleaner to
> me ?
>
> (factorize the two inits in netdev_init_one_queue())
>
>
> diff --git a/net/core/dev.c b/net/core/dev.c
> index cd24374..36e10b4 100644
> --- a/net/core/dev.c
> +++ b/net/core/dev.c
> @@ -5113,7 +5113,6 @@ static int netif_alloc_netdev_queues(struct net_device *dev)
> {
> unsigned int count = dev->num_tx_queues;
> struct netdev_queue *tx;
> - int i;
>
> BUG_ON(count < 1);
>
> @@ -5125,10 +5124,6 @@ static int netif_alloc_netdev_queues(struct net_device *dev)
> }
> dev->_tx = tx;
>
> - for (i = 0; i < count; i++) {
> - netdev_queue_numa_node_write(&tx[i], -1);
> - tx[i].dev = dev;
> - }
> return 0;
> }
>
> @@ -5140,6 +5135,8 @@ static void netdev_init_one_queue(struct net_device *dev,
> spin_lock_init(&queue->_xmit_lock);
> netdev_set_xmit_lockdep_class(&queue->_xmit_lock, dev->type);
> queue->xmit_lock_owner = -1;
> + netdev_queue_numa_node_write(queue, -1);
> + queue->dev = dev;
> }
>
> static void netdev_init_queues(struct net_device *dev)
>
>
>
I thought about it before submitting. I am not sure if there are some
users of txq->dev before netdev_init_one_queue().
--
Regards,
Changli Gao(xiaosuo@gmail.com)
^ permalink raw reply
* Re: [PATCH] net: init ingress queue
From: Eric Dumazet @ 2010-12-04 8:47 UTC (permalink / raw)
To: Changli Gao; +Cc: David S. Miller, Tom Herbert, Jiri Pirko, netdev
In-Reply-To: <1291441558-3196-1-git-send-email-xiaosuo@gmail.com>
Le samedi 04 décembre 2010 à 13:45 +0800, Changli Gao a écrit :
> The dev field of ingress queue is forgot to initialized, then NULL
> pointer dereference happens in qdisc_alloc().
< deleted oops >
> Signed-off-by: Changli Gao <xiaosuo@gmail.com>
> ---
> net/core/dev.c | 2 ++
> 1 file changed, 2 insertions(+)
> diff --git a/net/core/dev.c b/net/core/dev.c
> index cd24374..8083c68 100644
> --- a/net/core/dev.c
> +++ b/net/core/dev.c
> @@ -5577,6 +5577,8 @@ struct netdev_queue *dev_ingress_queue_create(struct net_device *dev)
> queue = kzalloc(sizeof(*queue), GFP_KERNEL);
> if (!queue)
> return NULL;
> + netdev_queue_numa_node_write(queue, -1);
> + queue->dev = dev;
> netdev_init_one_queue(dev, queue, NULL);
> queue->qdisc = &noop_qdisc;
> queue->qdisc_sleeping = &noop_qdisc;
Hi Changli, thanks for bug report and patch.
IMHO there is no point to include this long Oops report in Changelog for
a net-next-2.6 temporary problem, there wont be any bugzilla report.
Instead, you could say it is a followup patch for commits 1d24eb4815d1e0
and f2cd2d3e9b3ef960. Two or three lines with relevant information.
I suggest you submit following patch instead, as this seems cleaner to
me ?
(factorize the two inits in netdev_init_one_queue())
diff --git a/net/core/dev.c b/net/core/dev.c
index cd24374..36e10b4 100644
--- a/net/core/dev.c
+++ b/net/core/dev.c
@@ -5113,7 +5113,6 @@ static int netif_alloc_netdev_queues(struct net_device *dev)
{
unsigned int count = dev->num_tx_queues;
struct netdev_queue *tx;
- int i;
BUG_ON(count < 1);
@@ -5125,10 +5124,6 @@ static int netif_alloc_netdev_queues(struct net_device *dev)
}
dev->_tx = tx;
- for (i = 0; i < count; i++) {
- netdev_queue_numa_node_write(&tx[i], -1);
- tx[i].dev = dev;
- }
return 0;
}
@@ -5140,6 +5135,8 @@ static void netdev_init_one_queue(struct net_device *dev,
spin_lock_init(&queue->_xmit_lock);
netdev_set_xmit_lockdep_class(&queue->_xmit_lock, dev->type);
queue->xmit_lock_owner = -1;
+ netdev_queue_numa_node_write(queue, -1);
+ queue->dev = dev;
}
static void netdev_init_queues(struct net_device *dev)
^ permalink raw reply related
* Re: kernel panic with time-stamping in phy devices (monitor mode)
From: Eric Dumazet @ 2010-12-04 8:17 UTC (permalink / raw)
To: Richard Cochran; +Cc: Andrew Watts, netdev, David Miller
In-Reply-To: <20101204075750.GA3949@riccoc20.at.omicron.at>
Le samedi 04 décembre 2010 à 08:57 +0100, Richard Cochran a écrit :
> Date: Sat, 4 Dec 2010 08:55:04 +0100
> From: Richard Cochran <richardcochran@gmail.com>
> To: Eric Dumazet <eric.dumazet@gmail.com>
> Cc: Andrew Watts <akwatts@ymail.com>, netdev@vger.kernel.org,
> David Miller <davem@davemloft.net>
> Subject: Re: kernel panic with time-stamping in phy devices (monitor mode)
> Message-ID: <20101204075503.GA3490@riccoc20.at.omicron.at>
> References: <252997.92320.qm@web111013.mail.gq1.yahoo.com>
> <1291307884.2871.69.camel@edumazet-laptop>
> MIME-Version: 1.0
> Content-Type: text/plain; charset=us-ascii
> Content-Disposition: inline
> In-Reply-To: <1291307884.2871.69.camel@edumazet-laptop>
> User-Agent: Mutt/1.5.20 (2009-06-14)
>
> Ugh, new kernel code with no users is already causing trouble!
>
> On Thu, Dec 02, 2010 at 05:38:04PM +0100, Eric Dumazet wrote:
> > Thanks for the report
> >
> > Please try following patch.
>
> And thank you, Eric, for the quick patch.
>
> Can this fix go into 2.6.37, please?
Sure, I'll submit to David today, thanks !
^ permalink raw reply
* Re: kernel panic with time-stamping in phy devices (monitor mode)
From: Richard Cochran @ 2010-12-04 7:57 UTC (permalink / raw)
To: Eric Dumazet; +Cc: Andrew Watts, netdev, David Miller
In-Reply-To: <1291307884.2871.69.camel@edumazet-laptop>
Date: Sat, 4 Dec 2010 08:55:04 +0100
From: Richard Cochran <richardcochran@gmail.com>
To: Eric Dumazet <eric.dumazet@gmail.com>
Cc: Andrew Watts <akwatts@ymail.com>, netdev@vger.kernel.org,
David Miller <davem@davemloft.net>
Subject: Re: kernel panic with time-stamping in phy devices (monitor mode)
Message-ID: <20101204075503.GA3490@riccoc20.at.omicron.at>
References: <252997.92320.qm@web111013.mail.gq1.yahoo.com>
<1291307884.2871.69.camel@edumazet-laptop>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <1291307884.2871.69.camel@edumazet-laptop>
User-Agent: Mutt/1.5.20 (2009-06-14)
Ugh, new kernel code with no users is already causing trouble!
On Thu, Dec 02, 2010 at 05:38:04PM +0100, Eric Dumazet wrote:
> Thanks for the report
>
> Please try following patch.
And thank you, Eric, for the quick patch.
Can this fix go into 2.6.37, please?
Thanks,
Richard
^ permalink raw reply
* [PATCH 3/3] ifb: move tq from ifb_private
From: Changli Gao @ 2010-12-04 5:55 UTC (permalink / raw)
To: jamal; +Cc: netdev, jamal, Changli Gao
In-Reply-To: <1291442121-3302-1-git-send-email-xiaosuo@gmail.com>
tq is only used in ri_tasklet, so we move it from ifb_private to a
stack variable of ri_tasklet.
skb_queue_splice_tail_init() is used instead of the open coded and slow
one.
Signed-off-by: Changli Gao <xiaosuo@gmail.com>
---
drivers/net/ifb.c | 49 ++++++++++++-------------------------------------
1 file changed, 12 insertions(+), 37 deletions(-)
diff --git a/drivers/net/ifb.c b/drivers/net/ifb.c
index d1e362a..cd6e90d 100644
--- a/drivers/net/ifb.c
+++ b/drivers/net/ifb.c
@@ -39,9 +39,7 @@
#define TX_Q_LIMIT 32
struct ifb_private {
struct tasklet_struct ifb_tasklet;
- int tasklet_pending;
struct sk_buff_head rq;
- struct sk_buff_head tq;
};
static int numifbs = 2;
@@ -53,27 +51,25 @@ static int ifb_close(struct net_device *dev);
static void ri_tasklet(unsigned long dev)
{
-
struct net_device *_dev = (struct net_device *)dev;
struct ifb_private *dp = netdev_priv(_dev);
struct net_device_stats *stats = &_dev->stats;
struct netdev_queue *txq;
struct sk_buff *skb;
+ struct sk_buff_head tq;
+ __skb_queue_head_init(&tq);
txq = netdev_get_tx_queue(_dev, 0);
- if ((skb = skb_peek(&dp->tq)) == NULL) {
- if (__netif_tx_trylock(txq)) {
- while ((skb = skb_dequeue(&dp->rq)) != NULL) {
- skb_queue_tail(&dp->tq, skb);
- }
- __netif_tx_unlock(txq);
- } else {
- /* reschedule */
- goto resched;
- }
+ if (!__netif_tx_trylock(txq)) {
+ tasklet_schedule(&dp->ifb_tasklet);
+ return;
}
+ skb_queue_splice_tail_init(&dp->rq, &tq);
+ if (netif_tx_queue_stopped(txq))
+ netif_tx_wake_queue(txq);
+ __netif_tx_unlock(txq);
- while ((skb = skb_dequeue(&dp->tq)) != NULL) {
+ while ((skb = __skb_dequeue(&tq)) != NULL) {
u32 from = G_TC_FROM(skb->tc_verd);
skb->tc_verd = 0;
@@ -87,7 +83,7 @@ static void ri_tasklet(unsigned long dev)
rcu_read_unlock();
dev_kfree_skb(skb);
stats->tx_dropped++;
- break;
+ continue;
}
rcu_read_unlock();
skb->skb_iif = _dev->ifindex;
@@ -100,23 +96,6 @@ static void ri_tasklet(unsigned long dev)
} else
BUG();
}
-
- if (__netif_tx_trylock(txq)) {
- if ((skb = skb_peek(&dp->rq)) == NULL) {
- dp->tasklet_pending = 0;
- if (netif_queue_stopped(_dev))
- netif_wake_queue(_dev);
- } else {
- __netif_tx_unlock(txq);
- goto resched;
- }
- __netif_tx_unlock(txq);
- } else {
-resched:
- dp->tasklet_pending = 1;
- tasklet_schedule(&dp->ifb_tasklet);
- }
-
}
static const struct net_device_ops ifb_netdev_ops = {
@@ -162,10 +141,8 @@ static netdev_tx_t ifb_xmit(struct sk_buff *skb, struct net_device *dev)
}
skb_queue_tail(&dp->rq, skb);
- if (!dp->tasklet_pending) {
- dp->tasklet_pending = 1;
+ if (skb_queue_len(&dp->rq) == 1)
tasklet_schedule(&dp->ifb_tasklet);
- }
return NETDEV_TX_OK;
}
@@ -177,7 +154,6 @@ static int ifb_close(struct net_device *dev)
tasklet_kill(&dp->ifb_tasklet);
netif_stop_queue(dev);
skb_queue_purge(&dp->rq);
- skb_queue_purge(&dp->tq);
return 0;
}
@@ -187,7 +163,6 @@ static int ifb_open(struct net_device *dev)
tasklet_init(&dp->ifb_tasklet, ri_tasklet, (unsigned long)dev);
skb_queue_head_init(&dp->rq);
- skb_queue_head_init(&dp->tq);
netif_start_queue(dev);
return 0;
^ permalink raw reply related
* [PATCH 2/3] ifb: remove unused macro TX_TIMEOUT
From: Changli Gao @ 2010-12-04 5:55 UTC (permalink / raw)
To: jamal; +Cc: netdev, jamal, Changli Gao
In-Reply-To: <1291442121-3302-1-git-send-email-xiaosuo@gmail.com>
Signed-off-by: Changli Gao <xiaosuo@gmail.com>
---
drivers/net/ifb.c | 2 --
1 file changed, 2 deletions(-)
diff --git a/drivers/net/ifb.c b/drivers/net/ifb.c
index 4387525..d1e362a 100644
--- a/drivers/net/ifb.c
+++ b/drivers/net/ifb.c
@@ -36,8 +36,6 @@
#include <net/pkt_sched.h>
#include <net/net_namespace.h>
-#define TX_TIMEOUT (2*HZ)
-
#define TX_Q_LIMIT 32
struct ifb_private {
struct tasklet_struct ifb_tasklet;
^ permalink raw reply related
* [PATCH 1/3] ifb: remove the useless debug stats
From: Changli Gao @ 2010-12-04 5:55 UTC (permalink / raw)
To: jamal; +Cc: netdev, jamal, Changli Gao
These debug stats are not exported, and become useless.
Signed-off-by: Changli Gao <xiaosuo@gmail.com>
---
drivers/net/ifb.c | 19 -------------------
1 file changed, 19 deletions(-)
diff --git a/drivers/net/ifb.c b/drivers/net/ifb.c
index ab9f675..4387525 100644
--- a/drivers/net/ifb.c
+++ b/drivers/net/ifb.c
@@ -42,16 +42,6 @@
struct ifb_private {
struct tasklet_struct ifb_tasklet;
int tasklet_pending;
- /* mostly debug stats leave in for now */
- unsigned long st_task_enter; /* tasklet entered */
- unsigned long st_txq_refl_try; /* transmit queue refill attempt */
- unsigned long st_rxq_enter; /* receive queue entered */
- unsigned long st_rx2tx_tran; /* receive to trasmit transfers */
- unsigned long st_rxq_notenter; /*receiveQ not entered, resched */
- unsigned long st_rx_frm_egr; /* received from egress path */
- unsigned long st_rx_frm_ing; /* received from ingress path */
- unsigned long st_rxq_check;
- unsigned long st_rxq_rsch;
struct sk_buff_head rq;
struct sk_buff_head tq;
};
@@ -73,19 +63,14 @@ static void ri_tasklet(unsigned long dev)
struct sk_buff *skb;
txq = netdev_get_tx_queue(_dev, 0);
- dp->st_task_enter++;
if ((skb = skb_peek(&dp->tq)) == NULL) {
- dp->st_txq_refl_try++;
if (__netif_tx_trylock(txq)) {
- dp->st_rxq_enter++;
while ((skb = skb_dequeue(&dp->rq)) != NULL) {
skb_queue_tail(&dp->tq, skb);
- dp->st_rx2tx_tran++;
}
__netif_tx_unlock(txq);
} else {
/* reschedule */
- dp->st_rxq_notenter++;
goto resched;
}
}
@@ -110,10 +95,8 @@ static void ri_tasklet(unsigned long dev)
skb->skb_iif = _dev->ifindex;
if (from & AT_EGRESS) {
- dp->st_rx_frm_egr++;
dev_queue_xmit(skb);
} else if (from & AT_INGRESS) {
- dp->st_rx_frm_ing++;
skb_pull(skb, skb->dev->hard_header_len);
netif_rx(skb);
} else
@@ -121,13 +104,11 @@ static void ri_tasklet(unsigned long dev)
}
if (__netif_tx_trylock(txq)) {
- dp->st_rxq_check++;
if ((skb = skb_peek(&dp->rq)) == NULL) {
dp->tasklet_pending = 0;
if (netif_queue_stopped(_dev))
netif_wake_queue(_dev);
} else {
- dp->st_rxq_rsch++;
__netif_tx_unlock(txq);
goto resched;
}
^ permalink raw reply related
* [PATCH] net: init ingress queue
From: Changli Gao @ 2010-12-04 5:45 UTC (permalink / raw)
To: David S. Miller
Cc: Eric Dumazet, Tom Herbert, Jiri Pirko, netdev, Changli Gao
The dev field of ingress queue is forgot to initialized, then NULL
pointer dereference happens in qdisc_alloc().
[ 303.019348] BUG: unable to handle kernel NULL pointer dereference at 0000000000000398
[ 303.020068] IP: [<ffffffff81472aab>] qdisc_alloc+0x9b/0xc0
[ 303.020068] PGD 3d637067 PUD 3d03f067 PMD 0
[ 303.020068] Oops: 0000 [#1] SMP
[ 303.020068] last sysfs file: /sys/kernel/uevent_seqnum
[ 303.020068] CPU 0
[ 303.020068] Modules linked in: sch_ingress ipv6
[ 303.020068]
[ 303.020068] Pid: 3138, comm: tc Not tainted 2.6.37-rc1+ #90 /VirtualBox
[ 303.020068] RIP: 0010:[<ffffffff81472aab>] [<ffffffff81472aab>] qdisc_alloc+0x9b/0xc0
[ 303.020068] RSP: 0018:ffff88003daf1938 EFLAGS: 00010246
[ 303.020068] RAX: ffff88003db01400 RBX: ffffffffa00612a0 RCX: 0000000000000000
[ 303.020068] RDX: 0000000000000000 RSI: 0000000000000000 RDI: ffff88003db01600
[ 303.020068] RBP: ffff88003daf1948 R08: 0000000000000000 R09: ffff88003db01400
[ 303.020068] R10: ffff88003fbe9fe8 R11: dead000000200200 R12: ffff88003d37c600
[ 303.020068] R13: 00000000fffffff1 R14: ffffffffa00612a0 R15: 00000000fffffff1
[ 303.020068] FS: 00007f1b3575a700(0000) GS:ffff88003fc00000(0000) knlGS:0000000000000000
[ 303.020068] CS: 0010 DS: 0000 ES: 0000 CR0: 000000008005003b
[ 303.020068] CR2: 0000000000000398 CR3: 000000003d2a9000 CR4: 00000000000006f0
[ 303.020068] DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000
[ 303.020068] DR3: 0000000000000000 DR6: 00000000ffff0ff0 DR7: 0000000000000400
[ 303.020068] Process tc (pid: 3138, threadinfo ffff88003daf0000, task ffff88003d407500)
[ 303.020068] Stack:
[ 303.020068] ffff88003e231000 ffff88003d6e1024 ffff88003daf19b8 ffffffff81474d68
[ 303.020068] 0000000000000001 0000000000000000 ffff88003d37c600 ffff88003daf1a08
[ 303.020068] 0073736572676e69 0000000000000000 ffff88003daf19b8 ffff88003d6e1010
[ 303.020068] Call Trace:
[ 303.020068] [<ffffffff81474d68>] qdisc_create+0x78/0x350
[ 303.020068] [<ffffffff81476849>] tc_modify_qdisc+0x339/0x590
[ 303.020068] [<ffffffff8146622f>] rtnetlink_rcv_msg+0x16f/0x280
[ 303.020068] [<ffffffff814660c0>] ? rtnetlink_rcv_msg+0x0/0x280
[ 303.020068] [<ffffffff8147d6d9>] netlink_rcv_skb+0xa9/0xd0
[ 303.020068] [<ffffffff814660b0>] rtnetlink_rcv+0x20/0x30
[ 303.020068] [<ffffffff8147ce85>] netlink_unicast+0x2c5/0x2e0
[ 303.020068] [<ffffffff8147e195>] netlink_sendmsg+0x245/0x360
[ 303.020068] [<ffffffff81444874>] sock_sendmsg+0xe4/0x110
[ 303.020068] [<ffffffff810c5e19>] ? find_get_page+0x19/0x90
[ 303.020068] [<ffffffff810c716a>] ? filemap_fault+0xca/0x4b0
[ 303.020068] [<ffffffff810c6cd5>] ? unlock_page+0x25/0x30
[ 303.020068] [<ffffffff81444b6d>] ? move_addr_to_kernel+0x5d/0x60
[ 303.020068] [<ffffffff814500bd>] ? verify_iovec+0x7d/0xf0
[ 303.020068] [<ffffffff814465b5>] sys_sendmsg+0x1e5/0x330
[ 303.020068] [<ffffffff8147b5c8>] ? netlink_table_ungrab+0x28/0x30
[ 303.020068] [<ffffffff81516fac>] ? do_page_fault+0x1dc/0x4c0
[ 303.020068] [<ffffffff81444abb>] ? move_addr_to_user+0x9b/0xb0
[ 303.020068] [<ffffffff8112463b>] ? alloc_fd+0x4b/0x140
[ 303.020068] [<ffffffff81446394>] ? sys_recvmsg+0x44/0x80
[ 303.020068] [<ffffffff81002eab>] system_call_fastpath+0x16/0x1b
[ 303.020068] Code: 8d 90 88 00 00 00 48 89 90 88 00 00 00 48 89 90 90 00 00 00 48 8b 53 28 48 89 10 48 8b 53 30 4c 89 60 68 48 89 50 08 49 8b 14 24 <48> 8b 92 98 03 00 00 65 ff 02 c7 40 40 01 00 00 00 5b 41 5c c9
[ 303.020068] RIP [<ffffffff81472aab>] qdisc_alloc+0x9b/0xc0
[ 303.020068] RSP <ffff88003daf1938>
[ 303.020068] CR2: 0000000000000398
[ 303.135737] ---[ end trace 5b6e09a3328c82e4 ]---
Signed-off-by: Changli Gao <xiaosuo@gmail.com>
---
net/core/dev.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/net/core/dev.c b/net/core/dev.c
index cd24374..8083c68 100644
--- a/net/core/dev.c
+++ b/net/core/dev.c
@@ -5577,6 +5577,8 @@ struct netdev_queue *dev_ingress_queue_create(struct net_device *dev)
queue = kzalloc(sizeof(*queue), GFP_KERNEL);
if (!queue)
return NULL;
+ netdev_queue_numa_node_write(queue, -1);
+ queue->dev = dev;
netdev_init_one_queue(dev, queue, NULL);
queue->qdisc = &noop_qdisc;
queue->qdisc_sleeping = &noop_qdisc;
^ permalink raw reply related
* [PATCH 2/2] bluetooth: Use printf extension %pMbt
From: Joe Perches @ 2010-12-04 2:33 UTC (permalink / raw)
To: Marcel Holtmann, Gustavo F. Padovan
Cc: netdev, David S. Miller, linux-bluetooth, linux-kernel
In-Reply-To: <cover.1291419007.git.joe@perches.com>
Save some text and bss.
Remove function batostr so there's no possibility of bad output.
from the net/bluetooth directory:
$ size built-in.o.*
text data bss dec hex filename
293562 16265 70088 379915 5cc0b built-in.o.allyesconfig.new
294619 16269 70480 381368 5d1b8 built-in.o.allyesconfig.old
30359 772 56 31187 79d3 built-in.o.btonly.new
30555 776 92 31423 7abf built-in.o.btonly.old
Signed-off-by: Joe Perches <joe@perches.com>
---
net/bluetooth/bnep/core.c | 3 +--
net/bluetooth/cmtp/core.c | 2 +-
net/bluetooth/hci_conn.c | 6 +++---
net/bluetooth/hci_core.c | 8 ++++----
net/bluetooth/hci_event.c | 6 +++---
net/bluetooth/hci_sysfs.c | 10 +++++-----
net/bluetooth/hidp/core.c | 4 ++--
net/bluetooth/l2cap.c | 19 +++++++++----------
net/bluetooth/lib.c | 14 --------------
net/bluetooth/rfcomm/core.c | 16 ++++++++--------
net/bluetooth/rfcomm/sock.c | 8 ++++----
net/bluetooth/rfcomm/tty.c | 6 +++---
net/bluetooth/sco.c | 12 ++++++------
13 files changed, 49 insertions(+), 65 deletions(-)
diff --git a/net/bluetooth/bnep/core.c b/net/bluetooth/bnep/core.c
index 5868597..3d4530f 100644
--- a/net/bluetooth/bnep/core.c
+++ b/net/bluetooth/bnep/core.c
@@ -199,8 +199,7 @@ static int bnep_ctrl_set_mcfilter(struct bnep_session *s, u8 *data, int len)
memcpy(a1, data, ETH_ALEN); data += ETH_ALEN;
a2 = data; data += ETH_ALEN;
- BT_DBG("mc filter %s -> %s",
- batostr((void *) a1), batostr((void *) a2));
+ BT_DBG("mc filter %pMbt -> %pMbt", a1, a2);
#define INCA(a) { int i = 5; while (i >=0 && ++a[i--] == 0); }
diff --git a/net/bluetooth/cmtp/core.c b/net/bluetooth/cmtp/core.c
index 8e5f292..f72bca7 100644
--- a/net/bluetooth/cmtp/core.c
+++ b/net/bluetooth/cmtp/core.c
@@ -344,7 +344,7 @@ int cmtp_add_connection(struct cmtp_connadd_req *req, struct socket *sock)
BT_DBG("mtu %d", session->mtu);
- sprintf(session->name, "%s", batostr(&bt_sk(sock->sk)->dst));
+ sprintf(session->name, "%pMbt", &bt_sk(sock->sk)->dst);
session->sock = sock;
session->state = BT_CONFIG;
diff --git a/net/bluetooth/hci_conn.c b/net/bluetooth/hci_conn.c
index 0b1e460..d9e3eb3 100644
--- a/net/bluetooth/hci_conn.c
+++ b/net/bluetooth/hci_conn.c
@@ -221,7 +221,7 @@ struct hci_conn *hci_conn_add(struct hci_dev *hdev, int type, bdaddr_t *dst)
{
struct hci_conn *conn;
- BT_DBG("%s dst %s", hdev->name, batostr(dst));
+ BT_DBG("%s dst %pMbt", hdev->name, dst);
conn = kzalloc(sizeof(struct hci_conn), GFP_ATOMIC);
if (!conn)
@@ -325,7 +325,7 @@ struct hci_dev *hci_get_route(bdaddr_t *dst, bdaddr_t *src)
struct hci_dev *hdev = NULL;
struct list_head *p;
- BT_DBG("%s -> %s", batostr(src), batostr(dst));
+ BT_DBG("%pMbt -> %pMbt", src, dst);
read_lock_bh(&hci_dev_list_lock);
@@ -366,7 +366,7 @@ struct hci_conn *hci_connect(struct hci_dev *hdev, int type, bdaddr_t *dst, __u8
struct hci_conn *acl;
struct hci_conn *sco;
- BT_DBG("%s dst %s", hdev->name, batostr(dst));
+ BT_DBG("%s dst %pMbt", hdev->name, dst);
if (!(acl = hci_conn_hash_lookup_ba(hdev, ACL_LINK, dst))) {
if (!(acl = hci_conn_add(hdev, ACL_LINK, dst)))
diff --git a/net/bluetooth/hci_core.c b/net/bluetooth/hci_core.c
index bc2a052..47962af 100644
--- a/net/bluetooth/hci_core.c
+++ b/net/bluetooth/hci_core.c
@@ -338,7 +338,7 @@ struct inquiry_entry *hci_inquiry_cache_lookup(struct hci_dev *hdev, bdaddr_t *b
struct inquiry_cache *cache = &hdev->inq_cache;
struct inquiry_entry *e;
- BT_DBG("cache %p, %s", cache, batostr(bdaddr));
+ BT_DBG("cache %p, %pMbt", cache, bdaddr);
for (e = cache->list; e; e = e->next)
if (!bacmp(&e->data.bdaddr, bdaddr))
@@ -351,7 +351,7 @@ void hci_inquiry_cache_update(struct hci_dev *hdev, struct inquiry_data *data)
struct inquiry_cache *cache = &hdev->inq_cache;
struct inquiry_entry *e;
- BT_DBG("cache %p, %s", cache, batostr(&data->bdaddr));
+ BT_DBG("cache %p, %pMbt", cache, &data->bdaddr);
if (!(e = hci_inquiry_cache_lookup(hdev, &data->bdaddr))) {
/* Entry not in the cache. Add new one. */
@@ -1478,8 +1478,8 @@ static inline void hci_acl_tx_to(struct hci_dev *hdev)
list_for_each(p, &h->list) {
c = list_entry(p, struct hci_conn, list);
if (c->type == ACL_LINK && c->sent) {
- BT_ERR("%s killing stalled ACL connection %s",
- hdev->name, batostr(&c->dst));
+ BT_ERR("%s killing stalled ACL connection %pMbt",
+ hdev->name, &c->dst);
hci_acl_disconn(c, 0x13);
}
}
diff --git a/net/bluetooth/hci_event.c b/net/bluetooth/hci_event.c
index 3c1957c..53f833f 100644
--- a/net/bluetooth/hci_event.c
+++ b/net/bluetooth/hci_event.c
@@ -566,7 +566,7 @@ static inline void hci_cs_create_conn(struct hci_dev *hdev, __u8 status)
conn = hci_conn_hash_lookup_ba(hdev, ACL_LINK, &cp->bdaddr);
- BT_DBG("%s bdaddr %s conn %p", hdev->name, batostr(&cp->bdaddr), conn);
+ BT_DBG("%s bdaddr %pMbt conn %p", hdev->name, &cp->bdaddr, conn);
if (status) {
if (conn && conn->state == BT_CONNECT) {
@@ -984,8 +984,8 @@ static inline void hci_conn_request_evt(struct hci_dev *hdev, struct sk_buff *sk
struct hci_ev_conn_request *ev = (void *) skb->data;
int mask = hdev->link_mode;
- BT_DBG("%s bdaddr %s type 0x%x", hdev->name,
- batostr(&ev->bdaddr), ev->link_type);
+ BT_DBG("%s bdaddr %pMbt type 0x%x",
+ hdev->name, &ev->bdaddr, ev->link_type);
mask |= hci_proto_connect_ind(hdev, &ev->bdaddr, ev->link_type);
diff --git a/net/bluetooth/hci_sysfs.c b/net/bluetooth/hci_sysfs.c
index 5fce3d6..5dac407 100644
--- a/net/bluetooth/hci_sysfs.c
+++ b/net/bluetooth/hci_sysfs.c
@@ -37,7 +37,7 @@ static ssize_t show_link_type(struct device *dev, struct device_attribute *attr,
static ssize_t show_link_address(struct device *dev, struct device_attribute *attr, char *buf)
{
struct hci_conn *conn = dev_get_drvdata(dev);
- return sprintf(buf, "%s\n", batostr(&conn->dst));
+ return sprintf(buf, "%pMbt\n", &conn->dst);
}
static ssize_t show_link_features(struct device *dev, struct device_attribute *attr, char *buf)
@@ -236,7 +236,7 @@ static ssize_t show_class(struct device *dev, struct device_attribute *attr, cha
static ssize_t show_address(struct device *dev, struct device_attribute *attr, char *buf)
{
struct hci_dev *hdev = dev_get_drvdata(dev);
- return sprintf(buf, "%s\n", batostr(&hdev->bdaddr));
+ return sprintf(buf, "%pMbt\n", &hdev->bdaddr);
}
static ssize_t show_features(struct device *dev, struct device_attribute *attr, char *buf)
@@ -404,8 +404,8 @@ static int inquiry_cache_show(struct seq_file *f, void *p)
for (e = cache->list; e; e = e->next) {
struct inquiry_data *data = &e->data;
- seq_printf(f, "%s %d %d %d 0x%.2x%.2x%.2x 0x%.4x %d %d %u\n",
- batostr(&data->bdaddr),
+ seq_printf(f, "%pMbt %d %d %d 0x%.2x%.2x%.2x 0x%.4x %d %d %u\n",
+ &data->bdaddr,
data->pscan_rep_mode, data->pscan_period_mode,
data->pscan_mode, data->dev_class[2],
data->dev_class[1], data->dev_class[0],
@@ -442,7 +442,7 @@ static int blacklist_show(struct seq_file *f, void *p)
b = list_entry(l, struct bdaddr_list, list);
- seq_printf(f, "%s\n", batostr(&b->bdaddr));
+ seq_printf(f, "%pMbt\n", &b->bdaddr);
}
hci_dev_unlock_bh(hdev);
diff --git a/net/bluetooth/hidp/core.c b/net/bluetooth/hidp/core.c
index 29544c2..cde8827 100644
--- a/net/bluetooth/hidp/core.c
+++ b/net/bluetooth/hidp/core.c
@@ -787,8 +787,8 @@ static int hidp_setup_hid(struct hidp_session *session,
hid->country = req->country;
strncpy(hid->name, req->name, 128);
- strncpy(hid->phys, batostr(&bt_sk(session->ctrl_sock->sk)->src), 64);
- strncpy(hid->uniq, batostr(&bt_sk(session->ctrl_sock->sk)->dst), 64);
+ snprintf(hid->phys, 64, "%pMbt", &bt_sk(session->ctrl_sock->sk)->src);
+ snprintf(hid->uniq, 64, "%pMbt", &bt_sk(session->ctrl_sock->sk)->dst);
hid->dev.parent = hidp_get_device(session);
hid->ll_driver = &hidp_hid_driver;
diff --git a/net/bluetooth/l2cap.c b/net/bluetooth/l2cap.c
index 12b4aa2..72b8306 100644
--- a/net/bluetooth/l2cap.c
+++ b/net/bluetooth/l2cap.c
@@ -1057,8 +1057,7 @@ static int l2cap_do_connect(struct sock *sk)
__u8 auth_type;
int err;
- BT_DBG("%s -> %s psm 0x%2.2x", batostr(src), batostr(dst),
- l2cap_pi(sk)->psm);
+ BT_DBG("%pMbt -> %pMbt psm 0x%2.2x", src, dst, l2cap_pi(sk)->psm);
hdev = hci_get_route(dst, src);
if (!hdev)
@@ -4525,7 +4524,7 @@ static int l2cap_connect_ind(struct hci_dev *hdev, bdaddr_t *bdaddr, u8 type)
if (type != ACL_LINK)
return -EINVAL;
- BT_DBG("hdev %s, bdaddr %s", hdev->name, batostr(bdaddr));
+ BT_DBG("hdev %s, bdaddr %pMbt", hdev->name, bdaddr);
/* Find listening sockets and check their link_mode */
read_lock(&l2cap_sk_list.lock);
@@ -4553,7 +4552,7 @@ static int l2cap_connect_cfm(struct hci_conn *hcon, u8 status)
{
struct l2cap_conn *conn;
- BT_DBG("hcon %p bdaddr %s status %d", hcon, batostr(&hcon->dst), status);
+ BT_DBG("hcon %p bdaddr %pMbt status %d", hcon, &hcon->dst, status);
if (hcon->type != ACL_LINK)
return -EINVAL;
@@ -4798,12 +4797,12 @@ static int l2cap_debugfs_show(struct seq_file *f, void *p)
sk_for_each(sk, node, &l2cap_sk_list.head) {
struct l2cap_pinfo *pi = l2cap_pi(sk);
- seq_printf(f, "%s %s %d %d 0x%4.4x 0x%4.4x %d %d %d\n",
- batostr(&bt_sk(sk)->src),
- batostr(&bt_sk(sk)->dst),
- sk->sk_state, __le16_to_cpu(pi->psm),
- pi->scid, pi->dcid,
- pi->imtu, pi->omtu, pi->sec_level);
+ seq_printf(f, "%pMbt %pMbt %d %d 0x%4.4x 0x%4.4x %d %d %d\n",
+ &bt_sk(sk)->src,
+ &bt_sk(sk)->dst,
+ sk->sk_state, __le16_to_cpu(pi->psm),
+ pi->scid, pi->dcid,
+ pi->imtu, pi->omtu, pi->sec_level);
}
read_unlock_bh(&l2cap_sk_list.lock);
diff --git a/net/bluetooth/lib.c b/net/bluetooth/lib.c
index b826d1b..bfc8bbb 100644
--- a/net/bluetooth/lib.c
+++ b/net/bluetooth/lib.c
@@ -44,20 +44,6 @@ void baswap(bdaddr_t *dst, bdaddr_t *src)
}
EXPORT_SYMBOL(baswap);
-char *batostr(bdaddr_t *ba)
-{
- static char str[2][18];
- static int i = 1;
-
- i ^= 1;
- sprintf(str[i], "%2.2X:%2.2X:%2.2X:%2.2X:%2.2X:%2.2X",
- ba->b[5], ba->b[4], ba->b[3],
- ba->b[2], ba->b[1], ba->b[0]);
-
- return str[i];
-}
-EXPORT_SYMBOL(batostr);
-
/* Bluetooth error codes to Unix errno mapping */
int bt_err(__u16 code)
{
diff --git a/net/bluetooth/rfcomm/core.c b/net/bluetooth/rfcomm/core.c
index fa642aa..532e5ad 100644
--- a/net/bluetooth/rfcomm/core.c
+++ b/net/bluetooth/rfcomm/core.c
@@ -393,8 +393,8 @@ static int __rfcomm_dlc_open(struct rfcomm_dlc *d, bdaddr_t *src, bdaddr_t *dst,
int err = 0;
u8 dlci;
- BT_DBG("dlc %p state %ld %s %s channel %d",
- d, d->state, batostr(src), batostr(dst), channel);
+ BT_DBG("dlc %p state %ld %pMbt %pMbt channel %d",
+ d, d->state, src, dst, channel);
if (channel < 1 || channel > 30)
return -EINVAL;
@@ -692,7 +692,7 @@ static struct rfcomm_session *rfcomm_session_create(bdaddr_t *src,
struct socket *sock;
struct sock *sk;
- BT_DBG("%s %s", batostr(src), batostr(dst));
+ BT_DBG("%pMbt %pMbt", src, dst);
*err = rfcomm_l2sock_create(&sock);
if (*err < 0)
@@ -2120,11 +2120,11 @@ static int rfcomm_dlc_debugfs_show(struct seq_file *f, void *x)
struct sock *sk = s->sock->sk;
struct rfcomm_dlc *d = list_entry(pp, struct rfcomm_dlc, list);
- seq_printf(f, "%s %s %ld %d %d %d %d\n",
- batostr(&bt_sk(sk)->src),
- batostr(&bt_sk(sk)->dst),
- d->state, d->dlci, d->mtu,
- d->rx_credits, d->tx_credits);
+ seq_printf(f, "%pMbt %pMbt %ld %d %d %d %d\n",
+ &bt_sk(sk)->src,
+ &bt_sk(sk)->dst,
+ d->state, d->dlci, d->mtu,
+ d->rx_credits, d->tx_credits);
}
}
diff --git a/net/bluetooth/rfcomm/sock.c b/net/bluetooth/rfcomm/sock.c
index 0207bd6..bbbc7479 100644
--- a/net/bluetooth/rfcomm/sock.c
+++ b/net/bluetooth/rfcomm/sock.c
@@ -350,7 +350,7 @@ static int rfcomm_sock_bind(struct socket *sock, struct sockaddr *addr, int addr
struct sock *sk = sock->sk;
int err = 0;
- BT_DBG("sk %p %s", sk, batostr(&sa->rc_bdaddr));
+ BT_DBG("sk %p %pMbt", sk, &sa->rc_bdaddr);
if (!addr || addr->sa_family != AF_BLUETOOTH)
return -EINVAL;
@@ -979,9 +979,9 @@ static int rfcomm_sock_debugfs_show(struct seq_file *f, void *p)
read_lock_bh(&rfcomm_sk_list.lock);
sk_for_each(sk, node, &rfcomm_sk_list.head) {
- seq_printf(f, "%s %s %d %d\n",
- batostr(&bt_sk(sk)->src),
- batostr(&bt_sk(sk)->dst),
+ seq_printf(f, "%pMbt %pMbt %d %d\n",
+ &bt_sk(sk)->src,
+ &bt_sk(sk)->dst,
sk->sk_state, rfcomm_pi(sk)->channel);
}
diff --git a/net/bluetooth/rfcomm/tty.c b/net/bluetooth/rfcomm/tty.c
index a9b81f5..740c99b 100644
--- a/net/bluetooth/rfcomm/tty.c
+++ b/net/bluetooth/rfcomm/tty.c
@@ -183,7 +183,7 @@ static struct device *rfcomm_get_device(struct rfcomm_dev *dev)
static ssize_t show_address(struct device *tty_dev, struct device_attribute *attr, char *buf)
{
struct rfcomm_dev *dev = dev_get_drvdata(tty_dev);
- return sprintf(buf, "%s\n", batostr(&dev->dst));
+ return sprintf(buf, "%pMbt\n", &dev->dst);
}
static ssize_t show_channel(struct device *tty_dev, struct device_attribute *attr, char *buf)
@@ -685,8 +685,8 @@ static int rfcomm_tty_open(struct tty_struct *tty, struct file *filp)
if (!dev)
return -ENODEV;
- BT_DBG("dev %p dst %s channel %d opened %d", dev, batostr(&dev->dst),
- dev->channel, atomic_read(&dev->opened));
+ BT_DBG("dev %p dst %pMbt channel %d opened %d",
+ dev, &dev->dst, dev->channel, atomic_read(&dev->opened));
if (atomic_inc_return(&dev->opened) > 1)
return 0;
diff --git a/net/bluetooth/sco.c b/net/bluetooth/sco.c
index 66b9e5c..ac8370e 100644
--- a/net/bluetooth/sco.c
+++ b/net/bluetooth/sco.c
@@ -183,7 +183,7 @@ static int sco_connect(struct sock *sk)
struct hci_dev *hdev;
int err, type;
- BT_DBG("%s -> %s", batostr(src), batostr(dst));
+ BT_DBG("%pMbt -> %pMbt", src, dst);
if (!(hdev = hci_get_route(dst, src)))
return -EHOSTUNREACH;
@@ -457,7 +457,7 @@ static int sco_sock_bind(struct socket *sock, struct sockaddr *addr, int addr_le
bdaddr_t *src = &sa->sco_bdaddr;
int err = 0;
- BT_DBG("sk %p %s", sk, batostr(&sa->sco_bdaddr));
+ BT_DBG("sk %p %pMbt", sk, src);
if (!addr || addr->sa_family != AF_BLUETOOTH)
return -EINVAL;
@@ -884,7 +884,7 @@ static int sco_connect_ind(struct hci_dev *hdev, bdaddr_t *bdaddr, __u8 type)
if (type != SCO_LINK && type != ESCO_LINK)
return -EINVAL;
- BT_DBG("hdev %s, bdaddr %s", hdev->name, batostr(bdaddr));
+ BT_DBG("hdev %s, bdaddr %pMbt", hdev->name, bdaddr);
/* Find listening sockets */
read_lock(&sco_sk_list.lock);
@@ -905,7 +905,7 @@ static int sco_connect_ind(struct hci_dev *hdev, bdaddr_t *bdaddr, __u8 type)
static int sco_connect_cfm(struct hci_conn *hcon, __u8 status)
{
- BT_DBG("hcon %p bdaddr %s status %d", hcon, batostr(&hcon->dst), status);
+ BT_DBG("hcon %p bdaddr %pMbt status %d", hcon, &hcon->dst, status);
if (hcon->type != SCO_LINK && hcon->type != ESCO_LINK)
return -EINVAL;
@@ -961,8 +961,8 @@ static int sco_debugfs_show(struct seq_file *f, void *p)
read_lock_bh(&sco_sk_list.lock);
sk_for_each(sk, node, &sco_sk_list.head) {
- seq_printf(f, "%s %s %d\n", batostr(&bt_sk(sk)->src),
- batostr(&bt_sk(sk)->dst), sk->sk_state);
+ seq_printf(f, "%pMbt %pMbt %d\n",
+ &bt_sk(sk)->src, &bt_sk(sk)->dst, sk->sk_state);
}
read_unlock_bh(&sco_sk_list.lock);
--
1.7.3.2.245.g03276.dirty
^ permalink raw reply related
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