* [PATCH 07/18 v2] isdn: avm: call disable_pci_device() if pci_probe() failed
From: Kulikov Vasiliy @ 2010-08-09 19:50 UTC (permalink / raw)
To: kernel-janitors
Cc: Karsten Keil, David S. Miller, Alexey Dobriyan, Tilman Schmidt,
netdev
Driver should call disable_pci_device() if it returns from pci_probe()
with error.
Signed-off-by: Kulikov Vasiliy <segooon@gmail.com>
---
drivers/isdn/hardware/avm/t1pci.c | 1 +
1 files changed, 1 insertions(+), 0 deletions(-)
diff --git a/drivers/isdn/hardware/avm/t1pci.c b/drivers/isdn/hardware/avm/t1pci.c
index 5a3f830..a79eb5a 100644
--- a/drivers/isdn/hardware/avm/t1pci.c
+++ b/drivers/isdn/hardware/avm/t1pci.c
@@ -210,6 +210,7 @@ static int __devinit t1pci_probe(struct pci_dev *dev,
if (retval != 0) {
printk(KERN_ERR "t1pci: no AVM-T1-PCI at i/o %#x, irq %d detected, mem %#x\n",
param.port, param.irq, param.membase);
+ pci_disable_device(dev);
return -ENODEV;
}
return 0;
--
1.7.0.4
^ permalink raw reply related
* [PATCH 06/18 v2] isdn: avm: call disable_pci_device() if pci_probe() failed
From: Kulikov Vasiliy @ 2010-08-09 19:50 UTC (permalink / raw)
To: kernel-janitors
Cc: Karsten Keil, David S. Miller, Alexey Dobriyan, Tilman Schmidt,
Tejun Heo, netdev
Driver should call disable_pci_device() if it returns from pci_probe()
with error.
Signed-off-by: Kulikov Vasiliy <segooon@gmail.com>
---
drivers/isdn/hardware/avm/c4.c | 1 +
1 files changed, 1 insertions(+), 0 deletions(-)
diff --git a/drivers/isdn/hardware/avm/c4.c b/drivers/isdn/hardware/avm/c4.c
index 7715d32..d3530f6 100644
--- a/drivers/isdn/hardware/avm/c4.c
+++ b/drivers/isdn/hardware/avm/c4.c
@@ -1273,6 +1273,7 @@ static int __devinit c4_probe(struct pci_dev *dev,
if (retval != 0) {
printk(KERN_ERR "c4: no AVM-C%d at i/o %#x, irq %d detected, mem %#x\n",
nr, param.port, param.irq, param.membase);
+ pci_disable_device(dev);
return -ENODEV;
}
return 0;
--
1.7.0.4
^ permalink raw reply related
* Re: [net-next] net: Provide details on watchdog timeout.
From: Stephen Hemminger @ 2010-08-09 19:28 UTC (permalink / raw)
To: Ben Greear; +Cc: netdev
In-Reply-To: <1281381401-5554-1-git-send-email-greearb@candelatech.com>
On Mon, 9 Aug 2010 12:16:41 -0700
Ben Greear <greearb@candelatech.com> wrote:
> This also makes it printed under net_ratelimit instead
> of WARN_ON_ONCE. The backtrace never helped me much.
>
> Signed-off-by: Ben Greear <greearb@candelatech.com>
>
Please don't remove the WARN_ because it is used by oops.kernel.org
automated tracking.
^ permalink raw reply
* [net-next] net: Provide details on watchdog timeout.
From: Ben Greear @ 2010-08-09 19:16 UTC (permalink / raw)
To: netdev; +Cc: Ben Greear
This also makes it printed under net_ratelimit instead
of WARN_ON_ONCE. The backtrace never helped me much.
Signed-off-by: Ben Greear <greearb@candelatech.com>
---
:100644 100644 2aeb3a4... b52c450... M net/sched/sch_generic.c
net/sched/sch_generic.c | 12 +++++++++---
1 files changed, 9 insertions(+), 3 deletions(-)
diff --git a/net/sched/sch_generic.c b/net/sched/sch_generic.c
index 2aeb3a4..b52c450 100644
--- a/net/sched/sch_generic.c
+++ b/net/sched/sch_generic.c
@@ -253,9 +253,15 @@ static void dev_watchdog(unsigned long arg)
}
if (some_queue_timedout) {
- char drivername[64];
- WARN_ONCE(1, KERN_INFO "NETDEV WATCHDOG: %s (%s): transmit queue %u timed out\n",
- dev->name, netdev_drivername(dev, drivername, 64), i);
+ if (net_ratelimit()) {
+ char drivername[64];
+ printk(KERN_INFO "NETDEV WATCHDOG: %s (%s): "
+ "transmit queue %u timed out"
+ " trans-start: %lu jiffies: %lu\n",
+ dev->name,
+ netdev_drivername(dev, drivername, 64),
+ i, trans_start, jiffies);
+ }
dev->netdev_ops->ndo_tx_timeout(dev);
}
if (!mod_timer(&dev->watchdog_timer,
--
1.6.2.5
^ permalink raw reply related
* [net-next] net: More verbose message for too-many-orphans.
From: Ben Greear @ 2010-08-09 19:00 UTC (permalink / raw)
To: netdev; +Cc: Ben Greear
The original message doesn't give much clue as to what
is actually going wrong or how to fix it.
Signed-off-by: Ben Greear <greearb@candelatech.com>
---
:100644 100644 808bb92... 96a7ec2... M net/ipv4/tcp_timer.c
net/ipv4/tcp_timer.c | 6 +++++-
1 files changed, 5 insertions(+), 1 deletions(-)
diff --git a/net/ipv4/tcp_timer.c b/net/ipv4/tcp_timer.c
index 808bb92..96a7ec2 100644
--- a/net/ipv4/tcp_timer.c
+++ b/net/ipv4/tcp_timer.c
@@ -79,7 +79,11 @@ static int tcp_out_of_resources(struct sock *sk, int do_reset)
if (tcp_too_many_orphans(sk, orphans)) {
if (net_ratelimit())
- printk(KERN_INFO "Out of socket memory\n");
+ printk(KERN_INFO "Out of socket memory, orphans: %d/%d"
+ " tcp_memory_allocated: %d/%d\n",
+ orphans, sysctl_tcp_max_orphans,
+ atomic_read(&tcp_memory_allocated),
+ sysctl_tcp_mem[2]);
/* Catch exceptional cases, when connection requires reset.
* 1. Last segment was sent recently. */
--
1.6.2.5
^ permalink raw reply related
* Question: Is multipath routing supported for IPv6
From: Mahesh Kelkar @ 2010-08-09 18:48 UTC (permalink / raw)
To: netdev
I tried configuring multipath Ipv6 route using "iproute2" tool and it failed:
> ip route add 143:2:3:4::/64 nexthop dev eth0
RTNETLINK answers: No such device
Same Command works for Ipv4. I looked into the source code and I don't
see support for RTA_MULTIPATH in IPV6 codebase.
I wonder if netlink configuration part is missing, or entire support
for multipath is missing. Is there any historical reason for not
supporting it?
Thanks
Mahesh
^ permalink raw reply
* [net-next] net: Use NET_XMIT_SUCCESS where possible.
From: Ben Greear @ 2010-08-09 18:43 UTC (permalink / raw)
To: netdev; +Cc: Ben Greear
This is based on work originally done by Patric McHardy.
Signed-off-by: Ben Greear <greearb@candelatech.com>
---
:100644 100644 e114f23... 3406627... M net/sched/sch_atm.c
:100644 100644 c657628... a5a2915... M net/sched/sch_sfq.c
:100644 100644 0991c64... 641a30d... M net/sched/sch_tbf.c
:100644 100644 807643b... feaabc1... M net/sched/sch_teql.c
net/sched/sch_atm.c | 4 ++--
net/sched/sch_sfq.c | 2 +-
net/sched/sch_tbf.c | 4 ++--
net/sched/sch_teql.c | 2 +-
4 files changed, 6 insertions(+), 6 deletions(-)
diff --git a/net/sched/sch_atm.c b/net/sched/sch_atm.c
index e114f23..3406627 100644
--- a/net/sched/sch_atm.c
+++ b/net/sched/sch_atm.c
@@ -418,7 +418,7 @@ static int atm_tc_enqueue(struct sk_buff *skb, struct Qdisc *sch)
}
ret = qdisc_enqueue(skb, flow->q);
- if (ret != 0) {
+ if (ret != NET_XMIT_SUCCESS) {
drop: __maybe_unused
if (net_xmit_drop_count(ret)) {
sch->qstats.drops++;
@@ -442,7 +442,7 @@ drop: __maybe_unused
*/
if (flow == &p->link) {
sch->q.qlen++;
- return 0;
+ return NET_XMIT_SUCCESS;
}
tasklet_schedule(&p->task);
return NET_XMIT_SUCCESS | __NET_XMIT_BYPASS;
diff --git a/net/sched/sch_sfq.c b/net/sched/sch_sfq.c
index c657628..a5a2915 100644
--- a/net/sched/sch_sfq.c
+++ b/net/sched/sch_sfq.c
@@ -323,7 +323,7 @@ sfq_enqueue(struct sk_buff *skb, struct Qdisc *sch)
if (++sch->q.qlen <= q->limit) {
sch->bstats.bytes += qdisc_pkt_len(skb);
sch->bstats.packets++;
- return 0;
+ return NET_XMIT_SUCCESS;
}
sfq_drop(sch);
diff --git a/net/sched/sch_tbf.c b/net/sched/sch_tbf.c
index 0991c64..641a30d 100644
--- a/net/sched/sch_tbf.c
+++ b/net/sched/sch_tbf.c
@@ -127,7 +127,7 @@ static int tbf_enqueue(struct sk_buff *skb, struct Qdisc* sch)
return qdisc_reshape_fail(skb, sch);
ret = qdisc_enqueue(skb, q->qdisc);
- if (ret != 0) {
+ if (ret != NET_XMIT_SUCCESS) {
if (net_xmit_drop_count(ret))
sch->qstats.drops++;
return ret;
@@ -136,7 +136,7 @@ static int tbf_enqueue(struct sk_buff *skb, struct Qdisc* sch)
sch->q.qlen++;
sch->bstats.bytes += qdisc_pkt_len(skb);
sch->bstats.packets++;
- return 0;
+ return NET_XMIT_SUCCESS;
}
static unsigned int tbf_drop(struct Qdisc* sch)
diff --git a/net/sched/sch_teql.c b/net/sched/sch_teql.c
index 807643b..feaabc1 100644
--- a/net/sched/sch_teql.c
+++ b/net/sched/sch_teql.c
@@ -85,7 +85,7 @@ teql_enqueue(struct sk_buff *skb, struct Qdisc* sch)
__skb_queue_tail(&q->q, skb);
sch->bstats.bytes += qdisc_pkt_len(skb);
sch->bstats.packets++;
- return 0;
+ return NET_XMIT_SUCCESS;
}
kfree_skb(skb);
--
1.6.2.5
^ permalink raw reply related
* Re: [patch 2/2] qlcnic: using too much stack
From: Dan Carpenter @ 2010-08-09 18:42 UTC (permalink / raw)
To: Anirban Chakraborty
Cc: Amit Salecha, Linux Driver, David S. Miller, Sucheta Chakraborty,
netdev@vger.kernel.org, kernel-janitors@vger.kernel.org
In-Reply-To: <0081C370-65E2-434F-BBC6-C688F6F39750@qlogic.com>
On Mon, Aug 09, 2010 at 09:46:32AM -0700, Anirban Chakraborty wrote:
>
> It looks fine except that I'd use kzalloc instead of kcalloc above.
>
It's no problem to do that, and I'm already respinning the patches but
I'm confused. It looks like pci_info gets initialized correctly. What
am I missing?
regards,
dan carpenter
> thanks,
> -Anirban
^ permalink raw reply
* Re: [PATCH] net: allow netdev_wait_allrefs() to run faster
From: Ben Greear @ 2010-08-09 18:03 UTC (permalink / raw)
To: Benjamin LaHaise
Cc: Eric W. Biederman, Eric Dumazet, Octavian Purdila, netdev,
Cosmin Ratiu
In-Reply-To: <20100809174856.GS30010@kvack.org>
On 08/09/2010 10:48 AM, Benjamin LaHaise wrote:
> On Mon, Aug 09, 2010 at 10:44:14AM -0700, Ben Greear wrote:
>> We also have a few rules per interface, and notice that it takes around 10ms
>> per rule when we are removing them, even when using batching in 'ip':
> ...
>> Patrick thought had an idea, but I don't think he had time to
>> look at it further:
>>
>> "Its probably the synchronize_rcu() in fib_nl_delrule() and
>> the route flushing happening after rule removal."
>
> Yes, that would be a problem, but the issue is deeper than that -- if I'm
> not mistaken it's on the packet processing path that iptables doesn't scale
> for 100k interfaces with 1 rule per interface. It's been a while since I
> ran the tests, but I don't think it's changed much.
It would be nice to tie the rules based on 'iif' to a specific
interface. Seems it should give near constant time lookup for rules
if we only have a few per interface....
Thanks,
Ben
--
Ben Greear <greearb@candelatech.com>
Candela Technologies Inc http://www.candelatech.com
^ permalink raw reply
* Re: [PATCH] net: allow netdev_wait_allrefs() to run faster
From: Benjamin LaHaise @ 2010-08-09 17:34 UTC (permalink / raw)
To: Ben Greear
Cc: Eric W. Biederman, Eric Dumazet, Octavian Purdila, netdev,
Cosmin Ratiu
In-Reply-To: <4C603999.1030801@candelatech.com>
Hello Ben,
On Mon, Aug 09, 2010 at 10:23:37AM -0700, Ben Greear wrote:
> I was just comparing my out-of-tree patch set to .35, and it appears
> little or none of the patches discussed in this thread are in the
> upstream kernel yet.
I was waiting on Eric's sysfs changes for namespaces to settle down, but
ended up getting busy on other things. I guess now is a good time to pick
this back up and try to merge my changes for improving interface scaling.
I'll send out a new version of the patches sometime in the next couple of
days. I'm also about to make a new Babylon release as well, I just need
to write some more documentation. :-/
Btw, one thing I noticed but haven't been able to come up with a fix for
yet is that iptables has scaling issues with lots of interfaces.
Specifically, we had to start adding one iptables rule per interface for smtp
filtering (not all subscribers are permitted to send smtp directly out to
the net, so it has to be per-interface). It seems that those all get
dumped into a giant list. What I'd like to do is to be able to attach rules
directly to the interface, but I haven't really had the time to do a mergable
set of changes for that. Thoughts anyone?
-ben
^ permalink raw reply
* Re: [PATCH] net: allow netdev_wait_allrefs() to run faster
From: Benjamin LaHaise @ 2010-08-09 17:48 UTC (permalink / raw)
To: Ben Greear
Cc: Eric W. Biederman, Eric Dumazet, Octavian Purdila, netdev,
Cosmin Ratiu
In-Reply-To: <4C603E6E.1060309@candelatech.com>
On Mon, Aug 09, 2010 at 10:44:14AM -0700, Ben Greear wrote:
> We also have a few rules per interface, and notice that it takes around 10ms
> per rule when we are removing them, even when using batching in 'ip':
...
> Patrick thought had an idea, but I don't think he had time to
> look at it further:
>
> "Its probably the synchronize_rcu() in fib_nl_delrule() and
> the route flushing happening after rule removal."
Yes, that would be a problem, but the issue is deeper than that -- if I'm
not mistaken it's on the packet processing path that iptables doesn't scale
for 100k interfaces with 1 rule per interface. It's been a while since I
ran the tests, but I don't think it's changed much.
-ben
^ permalink raw reply
* Missing device binding relating to tcp_v4_send_reset?
From: Ben Greear @ 2010-08-09 17:46 UTC (permalink / raw)
To: NetDev, Patrick McHardy
This snippet is from some patches Patrick did for me some time
back. I think the rest of his work has been merged upstream, but
this patch was not. I'm honestly not sure if it's needed or not,
but we've been running with it for at least a year or so and it's
been working fine for us.
diff --git a/net/ipv4/tcp_ipv4.c b/net/ipv4/tcp_ipv4.c
index 0207662..1af47db 100644
--- a/net/ipv4/tcp_ipv4.c
+++ b/net/ipv4/tcp_ipv4.c
@@ -641,6 +641,7 @@ static void tcp_v4_send_reset(struct sock *sk, struct sk_buff *skb)
arg.iov[0].iov_len, IPPROTO_TCP, 0);
arg.csumoffset = offsetof(struct tcphdr, check) / 2;
arg.flags = (sk && inet_sk(sk)->transparent) ? IP_REPLY_ARG_NOSRCCHECK : 0;
+ arg.bound_dev_if = skb_rtable(skb)->fl.iif;
net = dev_net(skb_dst(skb)->dev);
ip_send_reply(net->ipv4.tcp_sock, skb,
Thanks,
Ben
--
Ben Greear <greearb@candelatech.com>
Candela Technologies Inc http://www.candelatech.com
^ permalink raw reply related
* Re: [PATCH] net: allow netdev_wait_allrefs() to run faster
From: Ben Greear @ 2010-08-09 17:44 UTC (permalink / raw)
To: Benjamin LaHaise
Cc: Eric W. Biederman, Eric Dumazet, Octavian Purdila, netdev,
Cosmin Ratiu
In-Reply-To: <20100809173429.GR30010@kvack.org>
On 08/09/2010 10:34 AM, Benjamin LaHaise wrote:
> Hello Ben,
>
> On Mon, Aug 09, 2010 at 10:23:37AM -0700, Ben Greear wrote:
>> I was just comparing my out-of-tree patch set to .35, and it appears
>> little or none of the patches discussed in this thread are in the
>> upstream kernel yet.
>
> I was waiting on Eric's sysfs changes for namespaces to settle down, but
> ended up getting busy on other things. I guess now is a good time to pick
> this back up and try to merge my changes for improving interface scaling.
> I'll send out a new version of the patches sometime in the next couple of
> days. I'm also about to make a new Babylon release as well, I just need
> to write some more documentation. :-/
>
> Btw, one thing I noticed but haven't been able to come up with a fix for
> yet is that iptables has scaling issues with lots of interfaces.
> Specifically, we had to start adding one iptables rule per interface for smtp
> filtering (not all subscribers are permitted to send smtp directly out to
> the net, so it has to be per-interface). It seems that those all get
> dumped into a giant list. What I'd like to do is to be able to attach rules
> directly to the interface, but I haven't really had the time to do a mergable
> set of changes for that. Thoughts anyone?
We also have a few rules per interface, and notice that it takes around 10ms
per rule when we are removing them, even when using batching in 'ip':
This is on a high-end core i7, otherwise lightly loaded.
Total IPv4 rule listings: 2097
Cleaning 2094 rules with ip -batch...
time -p ip -4 -force -batch /tmp/crr_batch_cmds_4.txt
real 17.81
user 0.05
sys 0.00
Patrick thought had an idea, but I don't think he had time to
look at it further:
"Its probably the synchronize_rcu() in fib_nl_delrule() and
the route flushing happening after rule removal."
Thanks,
Ben
--
Ben Greear <greearb@candelatech.com>
Candela Technologies Inc http://www.candelatech.com
^ permalink raw reply
* Re: [PATCH] net: allow netdev_wait_allrefs() to run faster
From: Ben Greear @ 2010-08-09 17:23 UTC (permalink / raw)
To: Benjamin LaHaise
Cc: Eric W. Biederman, Eric Dumazet, Octavian Purdila, netdev,
Cosmin Ratiu
In-Reply-To: <20091029233848.GV3141@kvack.org>
On 10/29/2009 04:38 PM, Benjamin LaHaise wrote:
> On Thu, Oct 29, 2009 at 04:07:18PM -0700, Eric W. Biederman wrote:
>> Could you keep me in the loop with that. I have some pending cleanups for
>> all of those pieces of code and may be able to help/advice/review.
>
> Here are the sysfs scaling improvements. I have to break them up, as there
> are 3 separate changes in this patch: 1. use an rbtree for name lookup in
> sysfs, 2. keep track of the number of directories for the purpose of
> generating the link count, as otherwise too much cpu time is spent in
> sysfs_count_nlink when new entries are added, and 3. when adding a new
> sysfs_dirent, walk the list backwards when linking it in, as higher
> numbered inodes tend to be at the end of the list, not the beginning.
I was just comparing my out-of-tree patch set to .35, and it appears
little or none of the patches discussed in this thread are in the
upstream kernel yet.
Specifically, there is still that msleep(250) in
netdev_wait_allrefs
Is anyone still trying to get the improvements needed for adding/deleting
lots of interfaces into the kernel?
Thanks,
Ben
--
Ben Greear <greearb@candelatech.com>
Candela Technologies Inc http://www.candelatech.com
^ permalink raw reply
* RE: [PATCH] [Bug 16494] NFS client over TCP hangs due to packet loss
From: Trond Myklebust @ 2010-08-09 16:55 UTC (permalink / raw)
To: Andy Chittenden
Cc: 'Andrew Morton', 'David Miller', kuznet, pekkas,
jmorris, yoshfuji, kaber, eric.dumazet, William.Allen.Simpson,
gilad, ilpo.jarvinen, netdev, linux-kernel, linux-nfs,
'J. Bruce Fields', 'Neil Brown',
'Chuck Lever', 'Benny Halevy',
'Alexandros Batsakis', 'Joe Perches',
Andy Chittenden
In-Reply-To: <4c5fc9f3.487e0e0a.2960.ffffe4ec@mx.google.com>
On Mon, 2010-08-09 at 10:27 +0100, Andy Chittenden wrote:
> A weekend run with that patch applied to 2.6.34.2 was successful. As nobody has objected, what's the next step to getting it applied to the official source trees?
Please resend me a version with a cleaned up changelog entry. I can then
push it as a bugfix.
Cheers
Trond
^ permalink raw reply
* Re: [patch 2/2] qlcnic: using too much stack
From: Anirban Chakraborty @ 2010-08-09 16:46 UTC (permalink / raw)
To: Dan Carpenter
Cc: Amit Salecha, Linux Driver, David S. Miller, Sucheta Chakraborty,
netdev@vger.kernel.org, kernel-janitors@vger.kernel.org
In-Reply-To: <20100809103727.GH9031@bicker>
On Aug 9, 2010, at 4:07 PM, Dan Carpenter wrote:
> qlcnic_pci_info structs are 128 bytes so an array of 8 uses 1024 bytes.
> That's a lot if you run with 4K stacks. I allocated them with kcalloc()
> instead.
>
> Signed-off-by: Dan Carpenter <error27@gmail.com>
>
> diff --git a/drivers/net/qlcnic/qlcnic_main.c b/drivers/net/qlcnic/qlcnic_main.c
> index 7f27e2a..da84229 100644
> --- a/drivers/net/qlcnic/qlcnic_main.c
> +++ b/drivers/net/qlcnic/qlcnic_main.c
> @@ -473,14 +473,20 @@ qlcnic_cleanup_pci_map(struct qlcnic_adapter *adapter)
> static int
> qlcnic_init_pci_info(struct qlcnic_adapter *adapter)
> {
> - struct qlcnic_pci_info pci_info[QLCNIC_MAX_PCI_FUNC];
> + struct qlcnic_pci_info *pci_info;
> int i, ret = 0, err;
> u8 pfn;
>
> + pci_info = kcalloc(QLCNIC_MAX_PCI_FUNC, sizeof(*pci_info), GFP_KERNEL);
> + if (!pci_info)
> + return -ENOMEM;
> +
> adapter->npars = kzalloc(sizeof(struct qlcnic_npar_info) *
> QLCNIC_MAX_PCI_FUNC, GFP_KERNEL);
> - if (!adapter->npars)
> - return -ENOMEM;
> + if (!adapter->npars) {
> + err = -ENOMEM;
> + goto err_pci_info;
> + }
>
> adapter->eswitch = kzalloc(sizeof(struct qlcnic_eswitch) *
> QLCNIC_NIU_MAX_XG_PORTS, GFP_KERNEL);
> @@ -508,6 +514,7 @@ qlcnic_init_pci_info(struct qlcnic_adapter *adapter)
> for (i = 0; i < QLCNIC_NIU_MAX_XG_PORTS; i++)
> adapter->eswitch[i].flags |= QLCNIC_SWITCH_ENABLE;
>
> + kfree(pci_info);
> return 0;
>
> err_eswitch:
> @@ -516,6 +523,8 @@ err_eswitch:
> err_npars:
> kfree(adapter->npars);
> adapter->eswitch = NULL;
> +err_pci_info:
> + kfree(pci_info);
>
> return ret;
> }
> @@ -3362,15 +3371,21 @@ qlcnic_sysfs_read_pci_config(struct file *file, struct kobject *kobj,
> struct device *dev = container_of(kobj, struct device, kobj);
> struct qlcnic_adapter *adapter = dev_get_drvdata(dev);
> struct qlcnic_pci_func_cfg pci_cfg[QLCNIC_MAX_PCI_FUNC];
> - struct qlcnic_pci_info pci_info[QLCNIC_MAX_PCI_FUNC];
> + struct qlcnic_pci_info *pci_info;
> int i, ret;
>
> if (size != sizeof(pci_cfg))
> return QL_STATUS_INVALID_PARAM;
>
> + pci_info = kcalloc(QLCNIC_MAX_PCI_FUNC, sizeof(*pci_info), GFP_KERNEL);
> + if (!pci_info)
> + return -ENOMEM;
> +
> ret = qlcnic_get_pci_info(adapter, pci_info);
> - if (ret)
> + if (ret) {
> + kfree(pci_info);
> return ret;
> + }
>
> for (i = 0; i < QLCNIC_MAX_PCI_FUNC ; i++) {
> pci_cfg[i].pci_func = pci_info[i].id;
> @@ -3381,8 +3396,8 @@ qlcnic_sysfs_read_pci_config(struct file *file, struct kobject *kobj,
> memcpy(&pci_cfg[i].def_mac_addr, &pci_info[i].mac, ETH_ALEN);
> }
> memcpy(buf, &pci_cfg, size);
> + kfree(pci_info);
> return size;
> -
> }
> static struct bin_attribute bin_attr_npar_config = {
> .attr = {.name = "npar_config", .mode = (S_IRUGO | S_IWUSR)},
It looks fine except that I'd use kzalloc instead of kcalloc above.
thanks,
-Anirban
^ permalink raw reply
* [PATCH] 3c59x: fix deadlock when using netconsole with 3c59x
From: Neil Horman @ 2010-08-09 16:32 UTC (permalink / raw)
To: netdev; +Cc: klassert, davem, nhorman
When using netpoll, its possible to deadlock the 3c59x driver. Since it takes
an internal spinlock (vp->lock) that serializes boomerang_interrupt and parts
of boomerang_start_xmit, if we call pr_debug in the former, we can go through
the tx path on the same cpu, and recurse into the same driver again, deadlocking
in the transmit routine.
This patch fixes that problem by stopping the queues during interrupt
processing, so that subsequent transmits will get queued until a later point.
Its not a great solution, but we need to find some way to serialize access to
the register file on the card without enforcing a deadlock. I think the queue
stop is the best way to do that. And since we only print things in
boomerang_interrupt when we have debug enabled, we can mitigate the impact of
this change to only stop the queues when debug is on.
Signed-off-by: Neil Horman <nhorman@tuxdriver.com>
---
drivers/net/3c59x.c | 12 ++++++++++++
1 files changed, 12 insertions(+), 0 deletions(-)
diff --git a/drivers/net/3c59x.c b/drivers/net/3c59x.c
index c754d88..e5c8757 100644
--- a/drivers/net/3c59x.c
+++ b/drivers/net/3c59x.c
@@ -2338,7 +2338,17 @@ boomerang_interrupt(int irq, void *dev_id)
/*
* It seems dopey to put the spinlock this early, but we could race against vortex_tx_timeout
* and boomerang_start_xmit
+ * We also need to disable the tx queue in the event that we print
+ * anything from this path. if pr_debug is called, we run the risk of
+ * recursing through the transmit path, which also takes the vp->lock,
+ * and deadlocks us. This only happens if we're using netpoll
+ * but we need to be ready for it
+ * We can mitigate the perf impact here if we only
+ * do this is vortex_debug is != 0
*/
+ if (vortex_debug)
+ netif_stop_queue(dev);
+
spin_lock(&vp->lock);
status = ioread16(ioaddr + EL3_STATUS);
@@ -2447,6 +2457,8 @@ boomerang_interrupt(int irq, void *dev_id)
pr_debug("%s: exiting interrupt, status %4.4x.\n",
dev->name, status);
handler_exit:
+ if (vortex_debug)
+ netif_start_queue(dev);
spin_unlock(&vp->lock);
return IRQ_HANDLED;
}
--
1.7.2.1
^ permalink raw reply related
* Re: [PATCH 1/6] bna: Brocade 10Gb Ethernet device driver
From: Stephen Hemminger @ 2010-08-09 15:15 UTC (permalink / raw)
To: Debashis Dutt; +Cc: netdev@vger.kernel.org
In-Reply-To: <F363E7AC84E1B646A0358B281A46F4AEA9A0F6565F@HQ1-EXCH03.corp.brocade.com>
On Fri, 6 Aug 2010 20:23:21 -0700
Debashis Dutt <ddutt@Brocade.COM> wrote:
>
> Hi Stephen,
>
> Thanks a lot for your comments.
>
> > + if (likely
> > + (wis > BNA_QE_FREE_CNT(tcb, tcb->q_depth) ||
> > + vectors > BNA_QE_FREE_CNT(unmap_q, unmap_q->q_depth))) {
> > + BNAD_UPDATE_CTR(bnad, netif_queue_stop);
> > + return NETDEV_TX_BUSY;
>
> >The transmit routine should check for available space after
> > queueing to device, so you can avoid having to return
> >TX_BUSY.
>
> However your above comment is not very clear to me.
>
> Our Tx routine already cleans up the Tx buffers at the end, through a tasklet.
>
> Cleaning of Tx buffers happens
> 1) In the sending context at the end of the Tx routine.
> 2) In the IRQ context when we get a Tx completion interrupt.
>
> Only thing that we could have done, is do this check again at the end of the Tx routine
> and called netif_stop_queue() if required, so that the stack stops sending. Even then I am
> not sure that we could avoid the current check and returning TX_BUSY.
>
> It would be nice if you clarify, so that I understand this better.
>
> Thanks
> --Debashis
> Linux LL Driver Team.
>
>
> -----Original Message-----
> From: Stephen Hemminger [mailto:shemminger@vyatta.com]
> Sent: Wednesday, August 04, 2010 10:09 AM
> To: Rasesh Mody
> Cc: netdev@vger.kernel.org; Debashis Dutt; Jing Huang
> Subject: Re: [PATCH 1/6] bna: Brocade 10Gb Ethernet device driver
>
> On Tue, 3 Aug 2010 22:15:36 -0700
> Rasesh Mody <rmody@brocade.com> wrote:
>
> > From: Rasesh Mody <rmody@brocade.com>
> >
> > This is patch 1/6 which contains linux driver source for
> > Brocade's BR1010/BR1020 10Gb CEE capable ethernet adapter.
> > Source is based against net-next-2.6.
> >
> > We wish this patch to be considered for inclusion in net-next-2.6
> >
> > Signed-off-by: Rasesh Mody <rmody@brocade.com>
> > ---
> > bnad.c | 3326 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++
> > bnad.h | 474 ++++++++
> > bnad_ethtool.c | 1269 +++++++++++++++++++++
> > 3 files changed, 5069 insertions(+)
> >
> > diff -ruP net-next-2.6.35-rc1-orig/drivers/net/bna/bnad.c net-next-2.6.35-rc1-mod/drivers/net/bna/bnad.c
> > --- net-next-2.6.35-rc1-orig/drivers/net/bna/bnad.c 1969-12-31 16:00:00.000000000 -0800
> > +++ net-next-2.6.35-rc1-mod/drivers/net/bna/bnad.c 2010-08-02 17:19:19.447239000 -0700
> > @@ -0,0 +1,3326 @@
> > +/*
> > + * Linux network driver for Brocade Converged Network Adapter.
> > + *
> > + * This program is free software; you can redistribute it and/or modify it
> > + * under the terms of the GNU General Public License (GPL) Version 2 as
> > + * published by the Free Software Foundation
> > + *
> > + * This program is distributed in the hope that it will be useful, but
> > + * WITHOUT ANY WARRANTY; without even the implied warranty of
> > + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
> > + * General Public License for more details.
> > + */
> > +/*
> > + * Copyright (c) 2005-2009 Brocade Communications Systems, Inc.
> > + * All rights reserved
> > + * www.brocade.com
> > + */
> > +#include <linux/netdevice.h>
> > +#include <linux/skbuff.h>
> > +#include <linux/etherdevice.h>
> > +#include <linux/in.h>
> > +#include <linux/ethtool.h>
> > +#include <linux/if_vlan.h>
> > +#include <linux/if_ether.h>
> > +#include <linux/ip.h>
> > +
> > +#include "bnad.h"
> > +#include "bna.h"
> > +#include "cna.h"
> > +
> > +DEFINE_MUTEX(bnad_fwimg_mutex);
> > +
> > +/*
> > + * Module params
> > + */
> > +static uint bnad_msix_disable;
> > +module_param(bnad_msix_disable, uint, 0444);
> > +MODULE_PARM_DESC(bnad_msix_disable, "Disable MSIX mode");
> > +
> > +static uint bnad_ioc_auto_recover = 1;
> > +module_param(bnad_ioc_auto_recover, uint, 0444);
> > +MODULE_PARM_DESC(bnad_ioc_auto_recover, "Enable / Disable auto recovery");
> > +
> > +/*
> > + * Global variables
> > + */
> > +u32 bna_id;
> > +u32 bnad_rxqs_per_cq = 2;
> > +
> > +DECLARE_MUTEX(bnad_list_sem);
> > +LIST_HEAD(bnad_list);
> > +
> > +const u8 bnad_bcast_addr[] = {0xff, 0xff, 0xff, 0xff, 0xff, 0xff};
>
> Surprised this isn't defined somewhere else.
>
> > +
> > +/*
> > + * Local MACROS
> > + */
> > +#define BNAD_TX_UNMAPQ_DEPTH (bnad->txq_depth * 2)
> > +
> > +#define BNAD_RX_UNMAPQ_DEPTH (bnad->rxq_depth)
> > +
> > +#define BNAD_GET_MBOX_IRQ(_bnad) \
> > + (((_bnad)->cfg_flags & BNAD_CF_MSIX) ? \
> > + ((_bnad)->msix_table[(_bnad)->msix_num - 1].vector) : \
> > + ((_bnad)->pcidev->irq))
> > +
> > +#define BNAD_FILL_UNMAPQ_MEM_REQ(_res_info, _num, _depth) \
> > +do { \
> > + (_res_info)->res_type = BNA_RES_T_MEM; \
> > + (_res_info)->res_u.mem_info.mem_type = BNA_MEM_T_KVA; \
> > + (_res_info)->res_u.mem_info.num = (_num); \
> > + (_res_info)->res_u.mem_info.len = \
> > + sizeof(struct bnad_unmap_q) + \
> > + (sizeof(struct bnad_skb_unmap) * ((_depth) - 1)); \
> > +} while (0)
> > +
> > +void
> > +bnad_add_to_list(struct bnad *bnad)
> > +{
> > + down(&bnad_list_sem);
> > + list_add_tail(&bnad->list_entry, &bnad_list);
> > + bna_id++;
> > + up(&bnad_list_sem);
> > +}
>
> Why do you need to list semaphore? Isn't RTNL mutex held
> when this is done. If you have to have own exclusion use
> a mutex for this.
>
> > +void
> > +bnad_remove_from_list(struct bnad *bnad)
> > +{
> > + down(&bnad_list_sem);
> > + list_del(&bnad->list_entry);
> > + up(&bnad_list_sem);
> > +}
> > +
> > +const struct pci_device_id bnad_pci_id_table[] = {
> > + {
> > + PCI_DEVICE(PCI_VENDOR_ID_BROCADE,
> > + PCI_DEVICE_ID_BROCADE_CT),
> > + .class = PCI_CLASS_NETWORK_ETHERNET << 8,
> > + .class_mask = 0xffff00
> > + }, {0, }
> > +};
>
> Why is this not static?
>
>
> > +/* TX */
> > +/* bnad_start_xmit : Netdev entry point for Transmit */
> > +/* Called under lock held by net_device */
> > +
> > +netdev_tx_t
> > +bnad_start_xmit(struct sk_buff *skb, struct net_device *netdev)
>
> Should also be static...
>
> > +{
> > + struct bnad *bnad = netdev_priv(netdev);
> > +
> > + u16 txq_prod, vlan_tag = 0;
> > + u32 unmap_prod, wis, wis_used, wi_range;
> > + u32 vectors, vect_id, i, acked;
> > + u32 tx_id;
> > + int err;
> > +
> > + struct bnad_tx_info *tx_info;
> > + struct bna_tcb *tcb;
> > + struct bnad_unmap_q *unmap_q;
> > + dma_addr_t dma_addr;
> > + struct bna_txq_entry *txqent;
> > + bna_txq_wi_ctrl_flag_t flags;
> > +
> > + if (unlikely
> > + (skb->len <= ETH_HLEN || skb->len > BFI_TX_MAX_DATA_PER_PKT)) {
> > + dev_kfree_skb(skb);
> > + return NETDEV_TX_OK;
> > + }
> > +
> > + /*
> > + * Takes care of the Tx that is scheduled between clearing the flag
> > + * and the netif_stop_queue() call.
> > + */
> > + if (unlikely(!test_bit(BNAD_RF_TX_STARTED, &bnad->run_flags))) {
> > + dev_kfree_skb(skb);
> > + return NETDEV_TX_OK;
> > + }
> > +
> > + tx_id = BNAD_GET_TX_ID(skb);
> > +
> > + tx_info = &bnad->tx_info[tx_id];
> > + tcb = tx_info->tcb[tx_id];
> > + unmap_q = tcb->unmap_q;
> > +
> > + vectors = 1 + skb_shinfo(skb)->nr_frags;
> > + if (vectors > BFI_TX_MAX_VECTORS_PER_PKT) {
> > + dev_kfree_skb(skb);
> > + return NETDEV_TX_OK;
> > + }
> > + wis = BNA_TXQ_WI_NEEDED(vectors); /* 4 vectors per work item */
> > + acked = 0;
> > + if (unlikely
> > + (wis > BNA_QE_FREE_CNT(tcb, tcb->q_depth) ||
> > + vectors > BNA_QE_FREE_CNT(unmap_q, unmap_q->q_depth))) {
> > + if ((u16) (*tcb->hw_consumer_index) !=
> > + tcb->consumer_index &&
> > + !test_and_set_bit(BNAD_TXQ_FREE_SENT, &tcb->flags)) {
> > + acked = bnad_free_txbufs(bnad, tcb);
> > + bna_ib_ack(tcb->i_dbell, acked);
> > + smp_mb__before_clear_bit();
> > + clear_bit(BNAD_TXQ_FREE_SENT, &tcb->flags);
> > + } else {
> > + netif_stop_queue(netdev);
> > + BNAD_UPDATE_CTR(bnad, netif_queue_stop);
> > + }
> > +
> > + smp_mb();
> > + /*
> > + * Check again to deal with race condition between
> > + * netif_stop_queue here, and netif_wake_queue in
> > + * interrupt handler which is not inside netif tx lock.
> > + */
> > + if (likely
> > + (wis > BNA_QE_FREE_CNT(tcb, tcb->q_depth) ||
> > + vectors > BNA_QE_FREE_CNT(unmap_q, unmap_q->q_depth))) {
> > + BNAD_UPDATE_CTR(bnad, netif_queue_stop);
> > + return NETDEV_TX_BUSY;
>
> The transmit routine should check for available space after
> queueing to device, so you can avoid having to return
> TX_BUSY.
The problem is that if device returns TX_BUSY, the net transmit scheduler
will end up re-calling the transmit routine. This looks ok in your driver
because it will set netif_stop_queue
^ permalink raw reply
* Re: Re: [PATCH] sfq: add dummy bind/unbind handles
From: Franchoze Eric @ 2010-08-09 15:01 UTC (permalink / raw)
To: Jarek Poplawski; +Cc: David Miller, shemminger, netdev
In-Reply-To: <4C5E570E.6020908@gmail.com>
08.08.10, 11:04, "Jarek Poplawski" <jarkao2@gmail.com>:
> David Miller wrote, On 08.08.2010 07:45:
>
> > From: Jarek Poplawski
> > Date: Sat, 07 Aug 2010 01:17:07 +0200
> >
> >> Stephen Hemminger wrote, On 07.08.2010 00:23:
> >>
> >>> Applying a filter to an SFQ qdisc would cause null dereference
> >>> in tcf_bind_filter because although SFQ is classful it didn't
> >>> have all the necessary equipment.
> >>>
> >>> Better alternative to changing tcf_bind API is to just fix
> >>> SFQ. This should go to net-2.6 and stable.
> >>>
> >>
> >> Hmm... FYI, actually I've sent already a similar patch to the
> >> original bug report thread (except .unbind_tcf method which
> >> doesn't matter for fixing this bug, so should be rather
> >> implemented in a separate patch, if needed at all in this
> >> case).
> >
> > Agreed, I can't see a way that unbind can ever be invoked
> > if the bind call always returns zero.
>
> To tell the truth, I think unbind should be implemented anyway,
> just for consistency, safety, and easier verification. But, looking
> at a similar case of .get and .put in the same driver, Patrick
> seemed to do it purposely, so I expected some discussion about the
> rules yet, and made it minimal to ease merging to older kernels.
>
> Thanks,
> Jarek P.
>
>
As for me it's better to add unbind now that get unexpected null derefance in future with API changing...
^ permalink raw reply
* [PATCH] am79c961a: Use net_device_stats from struct net_device
From: Tobias Klauser @ 2010-08-09 15:04 UTC (permalink / raw)
To: David S. Miller, netdev; +Cc: Russell King, Kulikov Vasiliy
struct net_device has its own struct net_device_stats member, so we can
use this one instead of a private copy in the dev_priv struct.
Cc: Kulikov Vasiliy <segooon@gmail.com>
Signed-off-by: Tobias Klauser <tklauser@distanz.ch>
---
drivers/net/arm/am79c961a.c | 35 ++++++++++++++++-------------------
drivers/net/arm/am79c961a.h | 1 -
2 files changed, 16 insertions(+), 20 deletions(-)
diff --git a/drivers/net/arm/am79c961a.c b/drivers/net/arm/am79c961a.c
index 8c496fb..62f2110 100644
--- a/drivers/net/arm/am79c961a.c
+++ b/drivers/net/arm/am79c961a.c
@@ -300,8 +300,6 @@ am79c961_open(struct net_device *dev)
struct dev_priv *priv = netdev_priv(dev);
int ret;
- memset (&priv->stats, 0, sizeof (priv->stats));
-
ret = request_irq(dev->irq, am79c961_interrupt, 0, dev->name, dev);
if (ret)
return ret;
@@ -347,8 +345,7 @@ am79c961_close(struct net_device *dev)
*/
static struct net_device_stats *am79c961_getstats (struct net_device *dev)
{
- struct dev_priv *priv = netdev_priv(dev);
- return &priv->stats;
+ return &dev->stats;
}
static void am79c961_mc_hash(char *addr, unsigned short *hash)
@@ -510,14 +507,14 @@ am79c961_rx(struct net_device *dev, struct dev_priv *priv)
if ((status & (RMD_ERR|RMD_STP|RMD_ENP)) != (RMD_STP|RMD_ENP)) {
am_writeword (dev, hdraddr + 2, RMD_OWN);
- priv->stats.rx_errors ++;
+ dev->stats.rx_errors++;
if (status & RMD_ERR) {
if (status & RMD_FRAM)
- priv->stats.rx_frame_errors ++;
+ dev->stats.rx_frame_errors++;
if (status & RMD_CRC)
- priv->stats.rx_crc_errors ++;
+ dev->stats.rx_crc_errors++;
} else if (status & RMD_STP)
- priv->stats.rx_length_errors ++;
+ dev->stats.rx_length_errors++;
continue;
}
@@ -531,12 +528,12 @@ am79c961_rx(struct net_device *dev, struct dev_priv *priv)
am_writeword(dev, hdraddr + 2, RMD_OWN);
skb->protocol = eth_type_trans(skb, dev);
netif_rx(skb);
- priv->stats.rx_bytes += len;
- priv->stats.rx_packets ++;
+ dev->stats.rx_bytes += len;
+ dev->stats.rx_packets++;
} else {
am_writeword (dev, hdraddr + 2, RMD_OWN);
printk (KERN_WARNING "%s: memory squeeze, dropping packet.\n", dev->name);
- priv->stats.rx_dropped ++;
+ dev->stats.rx_dropped++;
break;
}
} while (1);
@@ -565,7 +562,7 @@ am79c961_tx(struct net_device *dev, struct dev_priv *priv)
if (status & TMD_ERR) {
u_int status2;
- priv->stats.tx_errors ++;
+ dev->stats.tx_errors++;
status2 = am_readword (dev, hdraddr + 6);
@@ -575,18 +572,18 @@ am79c961_tx(struct net_device *dev, struct dev_priv *priv)
am_writeword (dev, hdraddr + 6, 0);
if (status2 & TST_RTRY)
- priv->stats.collisions += 16;
+ dev->stats.collisions += 16;
if (status2 & TST_LCOL)
- priv->stats.tx_window_errors ++;
+ dev->stats.tx_window_errors++;
if (status2 & TST_LCAR)
- priv->stats.tx_carrier_errors ++;
+ dev->stats.tx_carrier_errors++;
if (status2 & TST_UFLO)
- priv->stats.tx_fifo_errors ++;
+ dev->stats.tx_fifo_errors++;
continue;
}
- priv->stats.tx_packets ++;
+ dev->stats.tx_packets++;
len = am_readword (dev, hdraddr + 4);
- priv->stats.tx_bytes += -len;
+ dev->stats.tx_bytes += -len;
} while (priv->txtail != priv->txhead);
netif_wake_queue(dev);
@@ -616,7 +613,7 @@ am79c961_interrupt(int irq, void *dev_id)
}
if (status & CSR0_MISS) {
handled = 1;
- priv->stats.rx_dropped ++;
+ dev->stats.rx_dropped++;
}
if (status & CSR0_CERR) {
handled = 1;
diff --git a/drivers/net/arm/am79c961a.h b/drivers/net/arm/am79c961a.h
index 483009f..fd634d3 100644
--- a/drivers/net/arm/am79c961a.h
+++ b/drivers/net/arm/am79c961a.h
@@ -130,7 +130,6 @@
#define ISALED0_LNKST 0x8000
struct dev_priv {
- struct net_device_stats stats;
unsigned long rxbuffer[RX_BUFFERS];
unsigned long txbuffer[TX_BUFFERS];
unsigned char txhead;
--
1.7.0.4
^ permalink raw reply related
* Re: [patch 1/2] qlcnic: clean up qlcnic_init_pci_info()
From: Dan Carpenter @ 2010-08-09 11:32 UTC (permalink / raw)
To: Amit Kumar Salecha
Cc: Anirban Chakraborty, linux-driver, David S. Miller,
Sucheta Chakraborty, netdev, kernel-janitors
In-Reply-To: <20100809103542.GG9031@bicker>
On Mon, Aug 09, 2010 at 12:36:05PM +0200, Dan Carpenter wrote:
> +err_eswitch:
> kfree(adapter->eswitch);
> adapter->eswitch = NULL;
> -err_eswitch:
> +err_npars:
> kfree(adapter->npars);
> + adapter->eswitch = NULL;
^^^^^^^^^^^^^^^^^^^^^^^^
Oops. Typo. Will send a corrected fix. Sorry.
regards,
dan carpenter
>
> return ret;
> }
^ permalink raw reply
* RE: [patch 1/2] qlcnic: clean up qlcnic_init_pci_info()
From: Amit Salecha @ 2010-08-09 11:30 UTC (permalink / raw)
To: Dan Carpenter
Cc: Anirban Chakraborty, Linux Driver, David S. Miller,
Sucheta Chakraborty, netdev@vger.kernel.org,
kernel-janitors@vger.kernel.org, Ameen Rahman
In-Reply-To: <20100809103542.GG9031@bicker>
> In the original code we allocated memory conditionally
>......
Looks good from me.
Thanks Dan.
-----Original Message-----
From: Dan Carpenter [mailto:error27@gmail.com]
Sent: Monday, August 09, 2010 4:06 PM
To: Amit Salecha
Cc: Anirban Chakraborty; Linux Driver; David S. Miller; Sucheta Chakraborty; netdev@vger.kernel.org; kernel-janitors@vger.kernel.org
Subject: [patch 1/2] qlcnic: clean up qlcnic_init_pci_info()
In the original code we allocated memory conditionally and freed it in
the error handling unconditionally. It turns out that this function is
only called during initialization and "adapter->npars" and
"adapter->eswitch" are always NULL at the start of the function. I
removed those checks.
Also since I was cleaning things, I changed the error handling for
qlcnic_get_pci_info() and pulled everything in an indent level.
Signed-off-by: Dan Carpenter <error27@gmail.com>
diff --git a/drivers/net/qlcnic/qlcnic_main.c b/drivers/net/qlcnic/qlcnic_main.c
index b9615bd..2b0bc95 100644
--- a/drivers/net/qlcnic/qlcnic_main.c
+++ b/drivers/net/qlcnic/qlcnic_main.c
@@ -477,44 +477,45 @@ qlcnic_init_pci_info(struct qlcnic_adapter *adapter)
int i, ret = 0, err;
u8 pfn;
- if (!adapter->npars)
- adapter->npars = kzalloc(sizeof(struct qlcnic_npar_info) *
+ adapter->npars = kzalloc(sizeof(struct qlcnic_npar_info) *
QLCNIC_MAX_PCI_FUNC, GFP_KERNEL);
if (!adapter->npars)
return -ENOMEM;
- if (!adapter->eswitch)
- adapter->eswitch = kzalloc(sizeof(struct qlcnic_eswitch) *
+ adapter->eswitch = kzalloc(sizeof(struct qlcnic_eswitch) *
QLCNIC_NIU_MAX_XG_PORTS, GFP_KERNEL);
if (!adapter->eswitch) {
err = -ENOMEM;
- goto err_eswitch;
+ goto err_npars;
}
ret = qlcnic_get_pci_info(adapter, pci_info);
- if (!ret) {
- for (i = 0; i < QLCNIC_MAX_PCI_FUNC; i++) {
- pfn = pci_info[i].id;
- if (pfn > QLCNIC_MAX_PCI_FUNC)
- return QL_STATUS_INVALID_PARAM;
- adapter->npars[pfn].active = pci_info[i].active;
- adapter->npars[pfn].type = pci_info[i].type;
- adapter->npars[pfn].phy_port = pci_info[i].default_port;
- adapter->npars[pfn].mac_learning = DEFAULT_MAC_LEARN;
- adapter->npars[pfn].min_bw = pci_info[i].tx_min_bw;
- adapter->npars[pfn].max_bw = pci_info[i].tx_max_bw;
- }
-
- for (i = 0; i < QLCNIC_NIU_MAX_XG_PORTS; i++)
- adapter->eswitch[i].flags |= QLCNIC_SWITCH_ENABLE;
+ if (ret)
+ goto err_eswitch;
- return ret;
+ for (i = 0; i < QLCNIC_MAX_PCI_FUNC; i++) {
+ pfn = pci_info[i].id;
+ if (pfn > QLCNIC_MAX_PCI_FUNC)
+ return QL_STATUS_INVALID_PARAM;
+ adapter->npars[pfn].active = pci_info[i].active;
+ adapter->npars[pfn].type = pci_info[i].type;
+ adapter->npars[pfn].phy_port = pci_info[i].default_port;
+ adapter->npars[pfn].mac_learning = DEFAULT_MAC_LEARN;
+ adapter->npars[pfn].min_bw = pci_info[i].tx_min_bw;
+ adapter->npars[pfn].max_bw = pci_info[i].tx_max_bw;
}
+ for (i = 0; i < QLCNIC_NIU_MAX_XG_PORTS; i++)
+ adapter->eswitch[i].flags |= QLCNIC_SWITCH_ENABLE;
+
+ return 0;
+
+err_eswitch:
kfree(adapter->eswitch);
adapter->eswitch = NULL;
-err_eswitch:
+err_npars:
kfree(adapter->npars);
+ adapter->eswitch = NULL;
return ret;
}
^ permalink raw reply related
* [RFD] dccp: Rate Mismatch Controller -- still requires high-res timers?
From: Gerrit Renker @ 2010-08-09 10:54 UTC (permalink / raw)
To: dccp, netdev, Ivo Calado, Leandro; +Cc: ldecicco.gmail.com
I have finished a simple proof-of-concept implementation of the Rate Mismatch Controller for TFRC and put the patches/results on
http://www.erg.abdn.ac.uk/users/gerrit/dccp/notes/ccid3/sender_notes/rate_mismatch_controller/
The results are not as good as had been hoped - in particular, it still seems
necessary to use high-res timers in order to achieve reasonable stability and
accuracy.
Perhaps I have missed something, but as it stands, I can not see an advantage
in using this for TFRC - comments welcome.
Gerrit
^ permalink raw reply
* [patch 2/2] qlcnic: using too much stack
From: Dan Carpenter @ 2010-08-09 10:37 UTC (permalink / raw)
To: Amit Kumar Salecha
Cc: Anirban Chakraborty, linux-driver, David S. Miller,
Sucheta Chakraborty, netdev, kernel-janitors
qlcnic_pci_info structs are 128 bytes so an array of 8 uses 1024 bytes.
That's a lot if you run with 4K stacks. I allocated them with kcalloc()
instead.
Signed-off-by: Dan Carpenter <error27@gmail.com>
diff --git a/drivers/net/qlcnic/qlcnic_main.c b/drivers/net/qlcnic/qlcnic_main.c
index 7f27e2a..da84229 100644
--- a/drivers/net/qlcnic/qlcnic_main.c
+++ b/drivers/net/qlcnic/qlcnic_main.c
@@ -473,14 +473,20 @@ qlcnic_cleanup_pci_map(struct qlcnic_adapter *adapter)
static int
qlcnic_init_pci_info(struct qlcnic_adapter *adapter)
{
- struct qlcnic_pci_info pci_info[QLCNIC_MAX_PCI_FUNC];
+ struct qlcnic_pci_info *pci_info;
int i, ret = 0, err;
u8 pfn;
+ pci_info = kcalloc(QLCNIC_MAX_PCI_FUNC, sizeof(*pci_info), GFP_KERNEL);
+ if (!pci_info)
+ return -ENOMEM;
+
adapter->npars = kzalloc(sizeof(struct qlcnic_npar_info) *
QLCNIC_MAX_PCI_FUNC, GFP_KERNEL);
- if (!adapter->npars)
- return -ENOMEM;
+ if (!adapter->npars) {
+ err = -ENOMEM;
+ goto err_pci_info;
+ }
adapter->eswitch = kzalloc(sizeof(struct qlcnic_eswitch) *
QLCNIC_NIU_MAX_XG_PORTS, GFP_KERNEL);
@@ -508,6 +514,7 @@ qlcnic_init_pci_info(struct qlcnic_adapter *adapter)
for (i = 0; i < QLCNIC_NIU_MAX_XG_PORTS; i++)
adapter->eswitch[i].flags |= QLCNIC_SWITCH_ENABLE;
+ kfree(pci_info);
return 0;
err_eswitch:
@@ -516,6 +523,8 @@ err_eswitch:
err_npars:
kfree(adapter->npars);
adapter->eswitch = NULL;
+err_pci_info:
+ kfree(pci_info);
return ret;
}
@@ -3362,15 +3371,21 @@ qlcnic_sysfs_read_pci_config(struct file *file, struct kobject *kobj,
struct device *dev = container_of(kobj, struct device, kobj);
struct qlcnic_adapter *adapter = dev_get_drvdata(dev);
struct qlcnic_pci_func_cfg pci_cfg[QLCNIC_MAX_PCI_FUNC];
- struct qlcnic_pci_info pci_info[QLCNIC_MAX_PCI_FUNC];
+ struct qlcnic_pci_info *pci_info;
int i, ret;
if (size != sizeof(pci_cfg))
return QL_STATUS_INVALID_PARAM;
+ pci_info = kcalloc(QLCNIC_MAX_PCI_FUNC, sizeof(*pci_info), GFP_KERNEL);
+ if (!pci_info)
+ return -ENOMEM;
+
ret = qlcnic_get_pci_info(adapter, pci_info);
- if (ret)
+ if (ret) {
+ kfree(pci_info);
return ret;
+ }
for (i = 0; i < QLCNIC_MAX_PCI_FUNC ; i++) {
pci_cfg[i].pci_func = pci_info[i].id;
@@ -3381,8 +3396,8 @@ qlcnic_sysfs_read_pci_config(struct file *file, struct kobject *kobj,
memcpy(&pci_cfg[i].def_mac_addr, &pci_info[i].mac, ETH_ALEN);
}
memcpy(buf, &pci_cfg, size);
+ kfree(pci_info);
return size;
-
}
static struct bin_attribute bin_attr_npar_config = {
.attr = {.name = "npar_config", .mode = (S_IRUGO | S_IWUSR)},
^ permalink raw reply related
* [patch 1/2] qlcnic: clean up qlcnic_init_pci_info()
From: Dan Carpenter @ 2010-08-09 10:36 UTC (permalink / raw)
To: Amit Kumar Salecha
Cc: Anirban Chakraborty, linux-driver, David S. Miller,
Sucheta Chakraborty, netdev, kernel-janitors
In the original code we allocated memory conditionally and freed it in
the error handling unconditionally. It turns out that this function is
only called during initialization and "adapter->npars" and
"adapter->eswitch" are always NULL at the start of the function. I
removed those checks.
Also since I was cleaning things, I changed the error handling for
qlcnic_get_pci_info() and pulled everything in an indent level.
Signed-off-by: Dan Carpenter <error27@gmail.com>
diff --git a/drivers/net/qlcnic/qlcnic_main.c b/drivers/net/qlcnic/qlcnic_main.c
index b9615bd..2b0bc95 100644
--- a/drivers/net/qlcnic/qlcnic_main.c
+++ b/drivers/net/qlcnic/qlcnic_main.c
@@ -477,44 +477,45 @@ qlcnic_init_pci_info(struct qlcnic_adapter *adapter)
int i, ret = 0, err;
u8 pfn;
- if (!adapter->npars)
- adapter->npars = kzalloc(sizeof(struct qlcnic_npar_info) *
+ adapter->npars = kzalloc(sizeof(struct qlcnic_npar_info) *
QLCNIC_MAX_PCI_FUNC, GFP_KERNEL);
if (!adapter->npars)
return -ENOMEM;
- if (!adapter->eswitch)
- adapter->eswitch = kzalloc(sizeof(struct qlcnic_eswitch) *
+ adapter->eswitch = kzalloc(sizeof(struct qlcnic_eswitch) *
QLCNIC_NIU_MAX_XG_PORTS, GFP_KERNEL);
if (!adapter->eswitch) {
err = -ENOMEM;
- goto err_eswitch;
+ goto err_npars;
}
ret = qlcnic_get_pci_info(adapter, pci_info);
- if (!ret) {
- for (i = 0; i < QLCNIC_MAX_PCI_FUNC; i++) {
- pfn = pci_info[i].id;
- if (pfn > QLCNIC_MAX_PCI_FUNC)
- return QL_STATUS_INVALID_PARAM;
- adapter->npars[pfn].active = pci_info[i].active;
- adapter->npars[pfn].type = pci_info[i].type;
- adapter->npars[pfn].phy_port = pci_info[i].default_port;
- adapter->npars[pfn].mac_learning = DEFAULT_MAC_LEARN;
- adapter->npars[pfn].min_bw = pci_info[i].tx_min_bw;
- adapter->npars[pfn].max_bw = pci_info[i].tx_max_bw;
- }
-
- for (i = 0; i < QLCNIC_NIU_MAX_XG_PORTS; i++)
- adapter->eswitch[i].flags |= QLCNIC_SWITCH_ENABLE;
+ if (ret)
+ goto err_eswitch;
- return ret;
+ for (i = 0; i < QLCNIC_MAX_PCI_FUNC; i++) {
+ pfn = pci_info[i].id;
+ if (pfn > QLCNIC_MAX_PCI_FUNC)
+ return QL_STATUS_INVALID_PARAM;
+ adapter->npars[pfn].active = pci_info[i].active;
+ adapter->npars[pfn].type = pci_info[i].type;
+ adapter->npars[pfn].phy_port = pci_info[i].default_port;
+ adapter->npars[pfn].mac_learning = DEFAULT_MAC_LEARN;
+ adapter->npars[pfn].min_bw = pci_info[i].tx_min_bw;
+ adapter->npars[pfn].max_bw = pci_info[i].tx_max_bw;
}
+ for (i = 0; i < QLCNIC_NIU_MAX_XG_PORTS; i++)
+ adapter->eswitch[i].flags |= QLCNIC_SWITCH_ENABLE;
+
+ return 0;
+
+err_eswitch:
kfree(adapter->eswitch);
adapter->eswitch = NULL;
-err_eswitch:
+err_npars:
kfree(adapter->npars);
+ adapter->eswitch = NULL;
return ret;
}
^ 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