* Re: VLAN egress performance
From: Mattias Rönnblom @ 2010-02-08 16:58 UTC (permalink / raw)
To: Patrick McHardy; +Cc: netdev
In-Reply-To: <4B700DF5.1080406@trash.net>
Patrick McHardy <kaber@trash.net> writes:
>> Ingress VLAN Egress VLAN Packet Rate CPU utilization (all cores)
>> No No 5.0 Mpacket/s ~70%
>> Yes No 5.0 Mpacket/s ~75%
>> No Yes 1.4 Mpacket/s ~26%
>> Yes Yes 1.3 Mpacket/s ~26%
>>
> 2.6.32 contains VLAN multiqueue support and should scale better.
OK, excellent. I tried out 2.6.32.7 and it did 4.0 Mpacket/s with VLAN
on both ingress and egress, and ~4.7 Mpacket/s without any VLANs.
Thanks,
Mattias
^ permalink raw reply
* netfilter 00/05: netfilter fixes
From: Patrick McHardy @ 2010-02-08 17:10 UTC (permalink / raw)
To: davem; +Cc: netdev, Patrick McHardy, netfilter-devel
Hi Dave,
following are the bugfixes for nf_conntrack discussed over the past
days, as well as a bugfix for the use of pointer to a local variable
outside the scope of the variable:
- a fix for use count initialization of the "untracked" conntrack,
fixing freeing of memory in the data section
- a patch for per netns conntrack cache pointers to fix issues
with SLAB_DESTROY_BY_RCU
- a patch to disable conntrack expect hash size modification at runtime
- a patch for xtables to fix out of scope usage of a local variable
- a patch to fix conntrack hash resizing with multiple namespaces by
moving the hashsize into the per netns data
Please apply or pull from:
git://git.kernel.org/pub/scm/linux/kernel/git/kaber/nf-2.6.git master
Please note: I forgot to add "Cc: stable@kernel.org" to two of these
patches and manually added it to the patch files, so I'd appreciate
if you could apply the patches manually instead of pulling from the
git tree this time.
Thanks!
include/net/netns/conntrack.h | 3 +
include/net/netns/ipv4.h | 1 +
net/ipv4/netfilter/arp_tables.c | 4 +-
net/ipv4/netfilter/ip_tables.c | 4 +-
net/ipv4/netfilter/nf_conntrack_l3proto_ipv4.c | 2 +-
.../netfilter/nf_conntrack_l3proto_ipv4_compat.c | 4 +-
net/ipv4/netfilter/nf_nat_core.c | 22 ++--
net/ipv6/netfilter/ip6_tables.c | 4 +-
net/netfilter/nf_conntrack_core.c | 116 +++++++++++---------
net/netfilter/nf_conntrack_expect.c | 4 +-
net/netfilter/nf_conntrack_helper.c | 2 +-
net/netfilter/nf_conntrack_netlink.c | 2 +-
net/netfilter/nf_conntrack_standalone.c | 7 +-
13 files changed, 93 insertions(+), 82 deletions(-)
Alexey Dobriyan (2):
netfilter: nf_conntrack: restrict runtime expect hashsize modifications
netfilter: xtables: compat out of scope fix
Eric Dumazet (1):
netfilter: nf_conntrack: per netns nf_conntrack_cachep
Patrick McHardy (2):
netfilter: nf_conntrack: fix memory corruption with multiple namespaces
netfilter: nf_conntrack: fix hash resizing with namespaces
^ permalink raw reply
* netfilter 01/05: nf_conntrack: fix memory corruption with multiple namespaces
From: Patrick McHardy @ 2010-02-08 17:10 UTC (permalink / raw)
To: davem; +Cc: netdev, Patrick McHardy, netfilter-devel
In-Reply-To: <20100208171024.15522.8136.sendpatchset@x2.localnet>
commit 056ff3e3bd1563969a311697323ff929df94415c
Author: Patrick McHardy <kaber@trash.net>
Date: Wed Feb 3 12:58:06 2010 +0100
netfilter: nf_conntrack: fix memory corruption with multiple namespaces
As discovered by Jon Masters <jonathan@jonmasters.org>, the "untracked"
conntrack, which is located in the data section, might be accidentally
freed when a new namespace is instantiated while the untracked conntrack
is attached to a skb because the reference count it re-initialized.
The best fix would be to use a seperate untracked conntrack per
namespace since it includes a namespace pointer. Unfortunately this is
not possible without larger changes since the namespace is not easily
available everywhere we need it. For now move the untracked conntrack
initialization to the init_net setup function to make sure the reference
count is not re-initialized and handle cleanup in the init_net cleanup
function to make sure namespaces can exit properly while the untracked
conntrack is in use in other namespaces.
Cc: stable@kernel.org
Signed-off-by: Patrick McHardy <kaber@trash.net>
diff --git a/net/netfilter/nf_conntrack_core.c b/net/netfilter/nf_conntrack_core.c
index 0e98c32..37e2b88 100644
--- a/net/netfilter/nf_conntrack_core.c
+++ b/net/netfilter/nf_conntrack_core.c
@@ -1113,6 +1113,10 @@ static void nf_ct_release_dying_list(struct net *net)
static void nf_conntrack_cleanup_init_net(void)
{
+ /* wait until all references to nf_conntrack_untracked are dropped */
+ while (atomic_read(&nf_conntrack_untracked.ct_general.use) > 1)
+ schedule();
+
nf_conntrack_helper_fini();
nf_conntrack_proto_fini();
kmem_cache_destroy(nf_conntrack_cachep);
@@ -1127,9 +1131,6 @@ static void nf_conntrack_cleanup_net(struct net *net)
schedule();
goto i_see_dead_people;
}
- /* wait until all references to nf_conntrack_untracked are dropped */
- while (atomic_read(&nf_conntrack_untracked.ct_general.use) > 1)
- schedule();
nf_ct_free_hashtable(net->ct.hash, net->ct.hash_vmalloc,
nf_conntrack_htable_size);
@@ -1288,6 +1289,14 @@ static int nf_conntrack_init_init_net(void)
if (ret < 0)
goto err_helper;
+ /* Set up fake conntrack: to never be deleted, not in any hashes */
+#ifdef CONFIG_NET_NS
+ nf_conntrack_untracked.ct_net = &init_net;
+#endif
+ atomic_set(&nf_conntrack_untracked.ct_general.use, 1);
+ /* - and look it like as a confirmed connection */
+ set_bit(IPS_CONFIRMED_BIT, &nf_conntrack_untracked.status);
+
return 0;
err_helper:
@@ -1333,15 +1342,6 @@ static int nf_conntrack_init_net(struct net *net)
if (ret < 0)
goto err_ecache;
- /* Set up fake conntrack:
- - to never be deleted, not in any hashes */
-#ifdef CONFIG_NET_NS
- nf_conntrack_untracked.ct_net = &init_net;
-#endif
- atomic_set(&nf_conntrack_untracked.ct_general.use, 1);
- /* - and look it like as a confirmed connection */
- set_bit(IPS_CONFIRMED_BIT, &nf_conntrack_untracked.status);
-
return 0;
err_ecache:
^ permalink raw reply related
* netfilter 03/05: nf_conntrack: restrict runtime expect hashsize modifications
From: Patrick McHardy @ 2010-02-08 17:10 UTC (permalink / raw)
To: davem; +Cc: netdev, Patrick McHardy, netfilter-devel
In-Reply-To: <20100208171024.15522.8136.sendpatchset@x2.localnet>
commit 38c7233b287481dfb3327dde136801ce500aba58
Author: Alexey Dobriyan <adobriyan@gmail.com>
Date: Thu Feb 4 18:24:06 2010 +0100
netfilter: nf_conntrack: restrict runtime expect hashsize modifications
Expectation hashtable size was simply glued to a variable with no code
to rehash expectations, so it was a bug to allow writing to it.
Make "expect_hashsize" readonly.
Signed-off-by: Alexey Dobriyan <adobriyan@gmail.com>
Cc: stable@kernel.org
Signed-off-by: Patrick McHardy <kaber@trash.net>
diff --git a/net/netfilter/nf_conntrack_expect.c b/net/netfilter/nf_conntrack_expect.c
index fdf5d2a..4ad7d1d 100644
--- a/net/netfilter/nf_conntrack_expect.c
+++ b/net/netfilter/nf_conntrack_expect.c
@@ -569,7 +569,7 @@ static void exp_proc_remove(struct net *net)
#endif /* CONFIG_PROC_FS */
}
-module_param_named(expect_hashsize, nf_ct_expect_hsize, uint, 0600);
+module_param_named(expect_hashsize, nf_ct_expect_hsize, uint, 0400);
int nf_conntrack_expect_init(struct net *net)
{
^ permalink raw reply related
* netfilter 02/05: nf_conntrack: per netns nf_conntrack_cachep
From: Patrick McHardy @ 2010-02-08 17:10 UTC (permalink / raw)
To: davem; +Cc: netdev, Patrick McHardy, netfilter-devel
In-Reply-To: <20100208171024.15522.8136.sendpatchset@x2.localnet>
commit ab59b19be78aac65cdd599fb5002c9019885e061
Author: Eric Dumazet <eric.dumazet@gmail.com>
Date: Thu Feb 4 14:54:05 2010 +0100
netfilter: nf_conntrack: per netns nf_conntrack_cachep
nf_conntrack_cachep is currently shared by all netns instances, but
because of SLAB_DESTROY_BY_RCU special semantics, this is wrong.
If we use a shared slab cache, one object can instantly flight between
one hash table (netns ONE) to another one (netns TWO), and concurrent
reader (doing a lookup in netns ONE, 'finding' an object of netns TWO)
can be fooled without notice, because no RCU grace period has to be
observed between object freeing and its reuse.
We dont have this problem with UDP/TCP slab caches because TCP/UDP
hashtables are global to the machine (and each object has a pointer to
its netns).
If we use per netns conntrack hash tables, we also *must* use per netns
conntrack slab caches, to guarantee an object can not escape from one
namespace to another one.
Signed-off-by: Eric Dumazet <eric.dumazet@gmail.com>
[Patrick: added unique slab name allocation]
Cc: stable@kernel.org
Signed-off-by: Patrick McHardy <kaber@trash.net>
diff --git a/include/net/netns/conntrack.h b/include/net/netns/conntrack.h
index ba1ba0c..aed23b6 100644
--- a/include/net/netns/conntrack.h
+++ b/include/net/netns/conntrack.h
@@ -11,6 +11,7 @@ struct nf_conntrack_ecache;
struct netns_ct {
atomic_t count;
unsigned int expect_count;
+ struct kmem_cache *nf_conntrack_cachep;
struct hlist_nulls_head *hash;
struct hlist_head *expect_hash;
struct hlist_nulls_head unconfirmed;
@@ -28,5 +29,6 @@ struct netns_ct {
#endif
int hash_vmalloc;
int expect_vmalloc;
+ char *slabname;
};
#endif
diff --git a/net/netfilter/nf_conntrack_core.c b/net/netfilter/nf_conntrack_core.c
index 37e2b88..9de4bd4 100644
--- a/net/netfilter/nf_conntrack_core.c
+++ b/net/netfilter/nf_conntrack_core.c
@@ -63,8 +63,6 @@ EXPORT_SYMBOL_GPL(nf_conntrack_max);
struct nf_conn nf_conntrack_untracked __read_mostly;
EXPORT_SYMBOL_GPL(nf_conntrack_untracked);
-static struct kmem_cache *nf_conntrack_cachep __read_mostly;
-
static int nf_conntrack_hash_rnd_initted;
static unsigned int nf_conntrack_hash_rnd;
@@ -572,7 +570,7 @@ struct nf_conn *nf_conntrack_alloc(struct net *net,
* Do not use kmem_cache_zalloc(), as this cache uses
* SLAB_DESTROY_BY_RCU.
*/
- ct = kmem_cache_alloc(nf_conntrack_cachep, gfp);
+ ct = kmem_cache_alloc(net->ct.nf_conntrack_cachep, gfp);
if (ct == NULL) {
pr_debug("nf_conntrack_alloc: Can't alloc conntrack.\n");
atomic_dec(&net->ct.count);
@@ -611,7 +609,7 @@ void nf_conntrack_free(struct nf_conn *ct)
nf_ct_ext_destroy(ct);
atomic_dec(&net->ct.count);
nf_ct_ext_free(ct);
- kmem_cache_free(nf_conntrack_cachep, ct);
+ kmem_cache_free(net->ct.nf_conntrack_cachep, ct);
}
EXPORT_SYMBOL_GPL(nf_conntrack_free);
@@ -1119,7 +1117,6 @@ static void nf_conntrack_cleanup_init_net(void)
nf_conntrack_helper_fini();
nf_conntrack_proto_fini();
- kmem_cache_destroy(nf_conntrack_cachep);
}
static void nf_conntrack_cleanup_net(struct net *net)
@@ -1137,6 +1134,8 @@ static void nf_conntrack_cleanup_net(struct net *net)
nf_conntrack_ecache_fini(net);
nf_conntrack_acct_fini(net);
nf_conntrack_expect_fini(net);
+ kmem_cache_destroy(net->ct.nf_conntrack_cachep);
+ kfree(net->ct.slabname);
free_percpu(net->ct.stat);
}
@@ -1272,15 +1271,6 @@ static int nf_conntrack_init_init_net(void)
NF_CONNTRACK_VERSION, nf_conntrack_htable_size,
nf_conntrack_max);
- nf_conntrack_cachep = kmem_cache_create("nf_conntrack",
- sizeof(struct nf_conn),
- 0, SLAB_DESTROY_BY_RCU, NULL);
- if (!nf_conntrack_cachep) {
- printk(KERN_ERR "Unable to create nf_conn slab cache\n");
- ret = -ENOMEM;
- goto err_cache;
- }
-
ret = nf_conntrack_proto_init();
if (ret < 0)
goto err_proto;
@@ -1302,8 +1292,6 @@ static int nf_conntrack_init_init_net(void)
err_helper:
nf_conntrack_proto_fini();
err_proto:
- kmem_cache_destroy(nf_conntrack_cachep);
-err_cache:
return ret;
}
@@ -1325,6 +1313,21 @@ static int nf_conntrack_init_net(struct net *net)
ret = -ENOMEM;
goto err_stat;
}
+
+ net->ct.slabname = kasprintf(GFP_KERNEL, "nf_conntrack_%p", net);
+ if (!net->ct.slabname) {
+ ret = -ENOMEM;
+ goto err_slabname;
+ }
+
+ net->ct.nf_conntrack_cachep = kmem_cache_create(net->ct.slabname,
+ sizeof(struct nf_conn), 0,
+ SLAB_DESTROY_BY_RCU, NULL);
+ if (!net->ct.nf_conntrack_cachep) {
+ printk(KERN_ERR "Unable to create nf_conn slab cache\n");
+ ret = -ENOMEM;
+ goto err_cache;
+ }
net->ct.hash = nf_ct_alloc_hashtable(&nf_conntrack_htable_size,
&net->ct.hash_vmalloc, 1);
if (!net->ct.hash) {
@@ -1352,6 +1355,10 @@ err_expect:
nf_ct_free_hashtable(net->ct.hash, net->ct.hash_vmalloc,
nf_conntrack_htable_size);
err_hash:
+ kmem_cache_destroy(net->ct.nf_conntrack_cachep);
+err_cache:
+ kfree(net->ct.slabname);
+err_slabname:
free_percpu(net->ct.stat);
err_stat:
return ret;
^ permalink raw reply related
* netfilter 04/05: xtables: compat out of scope fix
From: Patrick McHardy @ 2010-02-08 17:10 UTC (permalink / raw)
To: davem; +Cc: netdev, Patrick McHardy, netfilter-devel
In-Reply-To: <20100208171024.15522.8136.sendpatchset@x2.localnet>
commit dab1531a07ad7c5be4ebe715a3d08742f0c638e3
Author: Alexey Dobriyan <adobriyan@gmail.com>
Date: Mon Feb 8 15:44:07 2010 +0100
netfilter: xtables: compat out of scope fix
As per C99 6.2.4(2) when temporary table data goes out of scope,
the behaviour is undefined:
if (compat) {
struct foo tmp;
...
private = &tmp;
}
[dereference private]
Signed-off-by: Alexey Dobriyan <adobriyan@gmail.com>
Cc: stable@kernel.org
Signed-off-by: Patrick McHardy <kaber@trash.net>
diff --git a/net/ipv4/netfilter/arp_tables.c b/net/ipv4/netfilter/arp_tables.c
index 0663276..90203e1 100644
--- a/net/ipv4/netfilter/arp_tables.c
+++ b/net/ipv4/netfilter/arp_tables.c
@@ -925,10 +925,10 @@ static int get_info(struct net *net, void __user *user, int *len, int compat)
if (t && !IS_ERR(t)) {
struct arpt_getinfo info;
const struct xt_table_info *private = t->private;
-
#ifdef CONFIG_COMPAT
+ struct xt_table_info tmp;
+
if (compat) {
- struct xt_table_info tmp;
ret = compat_table_info(private, &tmp);
xt_compat_flush_offsets(NFPROTO_ARP);
private = &tmp;
diff --git a/net/ipv4/netfilter/ip_tables.c b/net/ipv4/netfilter/ip_tables.c
index 572330a..3ce53cf 100644
--- a/net/ipv4/netfilter/ip_tables.c
+++ b/net/ipv4/netfilter/ip_tables.c
@@ -1132,10 +1132,10 @@ static int get_info(struct net *net, void __user *user, int *len, int compat)
if (t && !IS_ERR(t)) {
struct ipt_getinfo info;
const struct xt_table_info *private = t->private;
-
#ifdef CONFIG_COMPAT
+ struct xt_table_info tmp;
+
if (compat) {
- struct xt_table_info tmp;
ret = compat_table_info(private, &tmp);
xt_compat_flush_offsets(AF_INET);
private = &tmp;
diff --git a/net/ipv6/netfilter/ip6_tables.c b/net/ipv6/netfilter/ip6_tables.c
index 480d7f8..8a7e0f5 100644
--- a/net/ipv6/netfilter/ip6_tables.c
+++ b/net/ipv6/netfilter/ip6_tables.c
@@ -1164,10 +1164,10 @@ static int get_info(struct net *net, void __user *user, int *len, int compat)
if (t && !IS_ERR(t)) {
struct ip6t_getinfo info;
const struct xt_table_info *private = t->private;
-
#ifdef CONFIG_COMPAT
+ struct xt_table_info tmp;
+
if (compat) {
- struct xt_table_info tmp;
ret = compat_table_info(private, &tmp);
xt_compat_flush_offsets(AF_INET6);
private = &tmp;
^ permalink raw reply related
* netfilter 05/05: nf_conntrack: fix hash resizing with namespaces
From: Patrick McHardy @ 2010-02-08 17:10 UTC (permalink / raw)
To: davem; +Cc: netdev, Patrick McHardy, netfilter-devel
In-Reply-To: <20100208171024.15522.8136.sendpatchset@x2.localnet>
commit 9ab48ddcb144fdee908708669448dd136cf4894a
Author: Patrick McHardy <kaber@trash.net>
Date: Mon Feb 8 17:35:23 2010 +0100
netfilter: nf_conntrack: fix hash resizing with namespaces
As noticed by Jon Masters <jonathan@jonmasters.org>, the conntrack hash
size is global and not per namespace, but modifiable at runtime through
/sys/module/nf_conntrack/hashsize. Changing the hash size will only
resize the hash in the current namespace however, so other namespaces
will use an invalid hash size. This can cause crashes when enlarging
the hashsize, or false negative lookups when shrinking it.
Move the hash size into the per-namespace data and only use the global
hash size to initialize the per-namespace value when instanciating a
new namespace. Additionally restrict hash resizing to init_net for
now as other namespaces are not handled currently.
Cc: stable@kernel.org
Signed-off-by: Patrick McHardy <kaber@trash.net>
diff --git a/include/net/netns/conntrack.h b/include/net/netns/conntrack.h
index aed23b6..63d4498 100644
--- a/include/net/netns/conntrack.h
+++ b/include/net/netns/conntrack.h
@@ -11,6 +11,7 @@ struct nf_conntrack_ecache;
struct netns_ct {
atomic_t count;
unsigned int expect_count;
+ unsigned int htable_size;
struct kmem_cache *nf_conntrack_cachep;
struct hlist_nulls_head *hash;
struct hlist_head *expect_hash;
diff --git a/include/net/netns/ipv4.h b/include/net/netns/ipv4.h
index 2eb3814..9a4b8b7 100644
--- a/include/net/netns/ipv4.h
+++ b/include/net/netns/ipv4.h
@@ -40,6 +40,7 @@ struct netns_ipv4 {
struct xt_table *iptable_security;
struct xt_table *nat_table;
struct hlist_head *nat_bysource;
+ unsigned int nat_htable_size;
int nat_vmalloced;
#endif
diff --git a/net/ipv4/netfilter/nf_conntrack_l3proto_ipv4.c b/net/ipv4/netfilter/nf_conntrack_l3proto_ipv4.c
index d171b12..d1ea38a 100644
--- a/net/ipv4/netfilter/nf_conntrack_l3proto_ipv4.c
+++ b/net/ipv4/netfilter/nf_conntrack_l3proto_ipv4.c
@@ -210,7 +210,7 @@ static ctl_table ip_ct_sysctl_table[] = {
},
{
.procname = "ip_conntrack_buckets",
- .data = &nf_conntrack_htable_size,
+ .data = &init_net.ct.htable_size,
.maxlen = sizeof(unsigned int),
.mode = 0444,
.proc_handler = proc_dointvec,
diff --git a/net/ipv4/netfilter/nf_conntrack_l3proto_ipv4_compat.c b/net/ipv4/netfilter/nf_conntrack_l3proto_ipv4_compat.c
index 8668a3d..2fb7b76 100644
--- a/net/ipv4/netfilter/nf_conntrack_l3proto_ipv4_compat.c
+++ b/net/ipv4/netfilter/nf_conntrack_l3proto_ipv4_compat.c
@@ -32,7 +32,7 @@ static struct hlist_nulls_node *ct_get_first(struct seq_file *seq)
struct hlist_nulls_node *n;
for (st->bucket = 0;
- st->bucket < nf_conntrack_htable_size;
+ st->bucket < net->ct.htable_size;
st->bucket++) {
n = rcu_dereference(net->ct.hash[st->bucket].first);
if (!is_a_nulls(n))
@@ -50,7 +50,7 @@ static struct hlist_nulls_node *ct_get_next(struct seq_file *seq,
head = rcu_dereference(head->next);
while (is_a_nulls(head)) {
if (likely(get_nulls_value(head) == st->bucket)) {
- if (++st->bucket >= nf_conntrack_htable_size)
+ if (++st->bucket >= net->ct.htable_size)
return NULL;
}
head = rcu_dereference(net->ct.hash[st->bucket].first);
diff --git a/net/ipv4/netfilter/nf_nat_core.c b/net/ipv4/netfilter/nf_nat_core.c
index fe1a644..26066a2 100644
--- a/net/ipv4/netfilter/nf_nat_core.c
+++ b/net/ipv4/netfilter/nf_nat_core.c
@@ -35,9 +35,6 @@ static DEFINE_SPINLOCK(nf_nat_lock);
static struct nf_conntrack_l3proto *l3proto __read_mostly;
-/* Calculated at init based on memory size */
-static unsigned int nf_nat_htable_size __read_mostly;
-
#define MAX_IP_NAT_PROTO 256
static const struct nf_nat_protocol *nf_nat_protos[MAX_IP_NAT_PROTO]
__read_mostly;
@@ -72,7 +69,7 @@ EXPORT_SYMBOL_GPL(nf_nat_proto_put);
/* We keep an extra hash for each conntrack, for fast searching. */
static inline unsigned int
-hash_by_src(const struct nf_conntrack_tuple *tuple)
+hash_by_src(const struct net *net, const struct nf_conntrack_tuple *tuple)
{
unsigned int hash;
@@ -80,7 +77,7 @@ hash_by_src(const struct nf_conntrack_tuple *tuple)
hash = jhash_3words((__force u32)tuple->src.u3.ip,
(__force u32)tuple->src.u.all,
tuple->dst.protonum, 0);
- return ((u64)hash * nf_nat_htable_size) >> 32;
+ return ((u64)hash * net->ipv4.nat_htable_size) >> 32;
}
/* Is this tuple already taken? (not by us) */
@@ -147,7 +144,7 @@ find_appropriate_src(struct net *net,
struct nf_conntrack_tuple *result,
const struct nf_nat_range *range)
{
- unsigned int h = hash_by_src(tuple);
+ unsigned int h = hash_by_src(net, tuple);
const struct nf_conn_nat *nat;
const struct nf_conn *ct;
const struct hlist_node *n;
@@ -330,7 +327,7 @@ nf_nat_setup_info(struct nf_conn *ct,
if (have_to_hash) {
unsigned int srchash;
- srchash = hash_by_src(&ct->tuplehash[IP_CT_DIR_ORIGINAL].tuple);
+ srchash = hash_by_src(net, &ct->tuplehash[IP_CT_DIR_ORIGINAL].tuple);
spin_lock_bh(&nf_nat_lock);
/* nf_conntrack_alter_reply might re-allocate exntension aera */
nat = nfct_nat(ct);
@@ -679,8 +676,10 @@ nfnetlink_parse_nat_setup(struct nf_conn *ct,
static int __net_init nf_nat_net_init(struct net *net)
{
- net->ipv4.nat_bysource = nf_ct_alloc_hashtable(&nf_nat_htable_size,
- &net->ipv4.nat_vmalloced, 0);
+ /* Leave them the same for the moment. */
+ net->ipv4.nat_htable_size = net->ct.htable_size;
+ net->ipv4.nat_bysource = nf_ct_alloc_hashtable(&net->ipv4.nat_htable_size,
+ &net->ipv4.nat_vmalloced, 0);
if (!net->ipv4.nat_bysource)
return -ENOMEM;
return 0;
@@ -703,7 +702,7 @@ static void __net_exit nf_nat_net_exit(struct net *net)
nf_ct_iterate_cleanup(net, &clean_nat, NULL);
synchronize_rcu();
nf_ct_free_hashtable(net->ipv4.nat_bysource, net->ipv4.nat_vmalloced,
- nf_nat_htable_size);
+ net->ipv4.nat_htable_size);
}
static struct pernet_operations nf_nat_net_ops = {
@@ -724,9 +723,6 @@ static int __init nf_nat_init(void)
return ret;
}
- /* Leave them the same for the moment. */
- nf_nat_htable_size = nf_conntrack_htable_size;
-
ret = register_pernet_subsys(&nf_nat_net_ops);
if (ret < 0)
goto cleanup_extend;
diff --git a/net/netfilter/nf_conntrack_core.c b/net/netfilter/nf_conntrack_core.c
index 9de4bd4..4d79e3c 100644
--- a/net/netfilter/nf_conntrack_core.c
+++ b/net/netfilter/nf_conntrack_core.c
@@ -30,6 +30,7 @@
#include <linux/netdevice.h>
#include <linux/socket.h>
#include <linux/mm.h>
+#include <linux/nsproxy.h>
#include <linux/rculist_nulls.h>
#include <net/netfilter/nf_conntrack.h>
@@ -84,9 +85,10 @@ static u_int32_t __hash_conntrack(const struct nf_conntrack_tuple *tuple,
return ((u64)h * size) >> 32;
}
-static inline u_int32_t hash_conntrack(const struct nf_conntrack_tuple *tuple)
+static inline u_int32_t hash_conntrack(const struct net *net,
+ const struct nf_conntrack_tuple *tuple)
{
- return __hash_conntrack(tuple, nf_conntrack_htable_size,
+ return __hash_conntrack(tuple, net->ct.htable_size,
nf_conntrack_hash_rnd);
}
@@ -294,7 +296,7 @@ __nf_conntrack_find(struct net *net, const struct nf_conntrack_tuple *tuple)
{
struct nf_conntrack_tuple_hash *h;
struct hlist_nulls_node *n;
- unsigned int hash = hash_conntrack(tuple);
+ unsigned int hash = hash_conntrack(net, tuple);
/* Disable BHs the entire time since we normally need to disable them
* at least once for the stats anyway.
@@ -364,10 +366,11 @@ static void __nf_conntrack_hash_insert(struct nf_conn *ct,
void nf_conntrack_hash_insert(struct nf_conn *ct)
{
+ struct net *net = nf_ct_net(ct);
unsigned int hash, repl_hash;
- hash = hash_conntrack(&ct->tuplehash[IP_CT_DIR_ORIGINAL].tuple);
- repl_hash = hash_conntrack(&ct->tuplehash[IP_CT_DIR_REPLY].tuple);
+ hash = hash_conntrack(net, &ct->tuplehash[IP_CT_DIR_ORIGINAL].tuple);
+ repl_hash = hash_conntrack(net, &ct->tuplehash[IP_CT_DIR_REPLY].tuple);
__nf_conntrack_hash_insert(ct, hash, repl_hash);
}
@@ -395,8 +398,8 @@ __nf_conntrack_confirm(struct sk_buff *skb)
if (CTINFO2DIR(ctinfo) != IP_CT_DIR_ORIGINAL)
return NF_ACCEPT;
- hash = hash_conntrack(&ct->tuplehash[IP_CT_DIR_ORIGINAL].tuple);
- repl_hash = hash_conntrack(&ct->tuplehash[IP_CT_DIR_REPLY].tuple);
+ hash = hash_conntrack(net, &ct->tuplehash[IP_CT_DIR_ORIGINAL].tuple);
+ repl_hash = hash_conntrack(net, &ct->tuplehash[IP_CT_DIR_REPLY].tuple);
/* We're not in hash table, and we refuse to set up related
connections for unconfirmed conns. But packet copies and
@@ -466,7 +469,7 @@ nf_conntrack_tuple_taken(const struct nf_conntrack_tuple *tuple,
struct net *net = nf_ct_net(ignored_conntrack);
struct nf_conntrack_tuple_hash *h;
struct hlist_nulls_node *n;
- unsigned int hash = hash_conntrack(tuple);
+ unsigned int hash = hash_conntrack(net, tuple);
/* Disable BHs the entire time since we need to disable them at
* least once for the stats anyway.
@@ -501,7 +504,7 @@ static noinline int early_drop(struct net *net, unsigned int hash)
int dropped = 0;
rcu_read_lock();
- for (i = 0; i < nf_conntrack_htable_size; i++) {
+ for (i = 0; i < net->ct.htable_size; i++) {
hlist_nulls_for_each_entry_rcu(h, n, &net->ct.hash[hash],
hnnode) {
tmp = nf_ct_tuplehash_to_ctrack(h);
@@ -521,7 +524,7 @@ static noinline int early_drop(struct net *net, unsigned int hash)
if (cnt >= NF_CT_EVICTION_RANGE)
break;
- hash = (hash + 1) % nf_conntrack_htable_size;
+ hash = (hash + 1) % net->ct.htable_size;
}
rcu_read_unlock();
@@ -555,7 +558,7 @@ struct nf_conn *nf_conntrack_alloc(struct net *net,
if (nf_conntrack_max &&
unlikely(atomic_read(&net->ct.count) > nf_conntrack_max)) {
- unsigned int hash = hash_conntrack(orig);
+ unsigned int hash = hash_conntrack(net, orig);
if (!early_drop(net, hash)) {
atomic_dec(&net->ct.count);
if (net_ratelimit())
@@ -1012,7 +1015,7 @@ get_next_corpse(struct net *net, int (*iter)(struct nf_conn *i, void *data),
struct hlist_nulls_node *n;
spin_lock_bh(&nf_conntrack_lock);
- for (; *bucket < nf_conntrack_htable_size; (*bucket)++) {
+ for (; *bucket < net->ct.htable_size; (*bucket)++) {
hlist_nulls_for_each_entry(h, n, &net->ct.hash[*bucket], hnnode) {
ct = nf_ct_tuplehash_to_ctrack(h);
if (iter(ct, data))
@@ -1130,7 +1133,7 @@ static void nf_conntrack_cleanup_net(struct net *net)
}
nf_ct_free_hashtable(net->ct.hash, net->ct.hash_vmalloc,
- nf_conntrack_htable_size);
+ net->ct.htable_size);
nf_conntrack_ecache_fini(net);
nf_conntrack_acct_fini(net);
nf_conntrack_expect_fini(net);
@@ -1190,10 +1193,12 @@ int nf_conntrack_set_hashsize(const char *val, struct kernel_param *kp)
{
int i, bucket, vmalloced, old_vmalloced;
unsigned int hashsize, old_size;
- int rnd;
struct hlist_nulls_head *hash, *old_hash;
struct nf_conntrack_tuple_hash *h;
+ if (current->nsproxy->net_ns != &init_net)
+ return -EOPNOTSUPP;
+
/* On boot, we can set this without any fancy locking. */
if (!nf_conntrack_htable_size)
return param_set_uint(val, kp);
@@ -1206,33 +1211,29 @@ int nf_conntrack_set_hashsize(const char *val, struct kernel_param *kp)
if (!hash)
return -ENOMEM;
- /* We have to rehahs for the new table anyway, so we also can
- * use a newrandom seed */
- get_random_bytes(&rnd, sizeof(rnd));
-
/* Lookups in the old hash might happen in parallel, which means we
* might get false negatives during connection lookup. New connections
* created because of a false negative won't make it into the hash
* though since that required taking the lock.
*/
spin_lock_bh(&nf_conntrack_lock);
- for (i = 0; i < nf_conntrack_htable_size; i++) {
+ for (i = 0; i < init_net.ct.htable_size; i++) {
while (!hlist_nulls_empty(&init_net.ct.hash[i])) {
h = hlist_nulls_entry(init_net.ct.hash[i].first,
struct nf_conntrack_tuple_hash, hnnode);
hlist_nulls_del_rcu(&h->hnnode);
- bucket = __hash_conntrack(&h->tuple, hashsize, rnd);
+ bucket = __hash_conntrack(&h->tuple, hashsize,
+ nf_conntrack_hash_rnd);
hlist_nulls_add_head_rcu(&h->hnnode, &hash[bucket]);
}
}
- old_size = nf_conntrack_htable_size;
+ old_size = init_net.ct.htable_size;
old_vmalloced = init_net.ct.hash_vmalloc;
old_hash = init_net.ct.hash;
- nf_conntrack_htable_size = hashsize;
+ init_net.ct.htable_size = nf_conntrack_htable_size = hashsize;
init_net.ct.hash_vmalloc = vmalloced;
init_net.ct.hash = hash;
- nf_conntrack_hash_rnd = rnd;
spin_unlock_bh(&nf_conntrack_lock);
nf_ct_free_hashtable(old_hash, old_vmalloced, old_size);
@@ -1328,7 +1329,9 @@ static int nf_conntrack_init_net(struct net *net)
ret = -ENOMEM;
goto err_cache;
}
- net->ct.hash = nf_ct_alloc_hashtable(&nf_conntrack_htable_size,
+
+ net->ct.htable_size = nf_conntrack_htable_size;
+ net->ct.hash = nf_ct_alloc_hashtable(&net->ct.htable_size,
&net->ct.hash_vmalloc, 1);
if (!net->ct.hash) {
ret = -ENOMEM;
@@ -1353,7 +1356,7 @@ err_acct:
nf_conntrack_expect_fini(net);
err_expect:
nf_ct_free_hashtable(net->ct.hash, net->ct.hash_vmalloc,
- nf_conntrack_htable_size);
+ net->ct.htable_size);
err_hash:
kmem_cache_destroy(net->ct.nf_conntrack_cachep);
err_cache:
diff --git a/net/netfilter/nf_conntrack_expect.c b/net/netfilter/nf_conntrack_expect.c
index 4ad7d1d..2f25ff6 100644
--- a/net/netfilter/nf_conntrack_expect.c
+++ b/net/netfilter/nf_conntrack_expect.c
@@ -577,7 +577,7 @@ int nf_conntrack_expect_init(struct net *net)
if (net_eq(net, &init_net)) {
if (!nf_ct_expect_hsize) {
- nf_ct_expect_hsize = nf_conntrack_htable_size / 256;
+ nf_ct_expect_hsize = net->ct.htable_size / 256;
if (!nf_ct_expect_hsize)
nf_ct_expect_hsize = 1;
}
diff --git a/net/netfilter/nf_conntrack_helper.c b/net/netfilter/nf_conntrack_helper.c
index 65c2a7b..4b1a56b 100644
--- a/net/netfilter/nf_conntrack_helper.c
+++ b/net/netfilter/nf_conntrack_helper.c
@@ -192,7 +192,7 @@ static void __nf_conntrack_helper_unregister(struct nf_conntrack_helper *me,
/* Get rid of expecteds, set helpers to NULL. */
hlist_nulls_for_each_entry(h, nn, &net->ct.unconfirmed, hnnode)
unhelp(h, me);
- for (i = 0; i < nf_conntrack_htable_size; i++) {
+ for (i = 0; i < net->ct.htable_size; i++) {
hlist_nulls_for_each_entry(h, nn, &net->ct.hash[i], hnnode)
unhelp(h, me);
}
diff --git a/net/netfilter/nf_conntrack_netlink.c b/net/netfilter/nf_conntrack_netlink.c
index 42f21c0..0ffe689 100644
--- a/net/netfilter/nf_conntrack_netlink.c
+++ b/net/netfilter/nf_conntrack_netlink.c
@@ -594,7 +594,7 @@ ctnetlink_dump_table(struct sk_buff *skb, struct netlink_callback *cb)
rcu_read_lock();
last = (struct nf_conn *)cb->args[1];
- for (; cb->args[0] < nf_conntrack_htable_size; cb->args[0]++) {
+ for (; cb->args[0] < init_net.ct.htable_size; cb->args[0]++) {
restart:
hlist_nulls_for_each_entry_rcu(h, n, &init_net.ct.hash[cb->args[0]],
hnnode) {
diff --git a/net/netfilter/nf_conntrack_standalone.c b/net/netfilter/nf_conntrack_standalone.c
index 028aba6..e310f15 100644
--- a/net/netfilter/nf_conntrack_standalone.c
+++ b/net/netfilter/nf_conntrack_standalone.c
@@ -51,7 +51,7 @@ static struct hlist_nulls_node *ct_get_first(struct seq_file *seq)
struct hlist_nulls_node *n;
for (st->bucket = 0;
- st->bucket < nf_conntrack_htable_size;
+ st->bucket < net->ct.htable_size;
st->bucket++) {
n = rcu_dereference(net->ct.hash[st->bucket].first);
if (!is_a_nulls(n))
@@ -69,7 +69,7 @@ static struct hlist_nulls_node *ct_get_next(struct seq_file *seq,
head = rcu_dereference(head->next);
while (is_a_nulls(head)) {
if (likely(get_nulls_value(head) == st->bucket)) {
- if (++st->bucket >= nf_conntrack_htable_size)
+ if (++st->bucket >= net->ct.htable_size)
return NULL;
}
head = rcu_dereference(net->ct.hash[st->bucket].first);
@@ -355,7 +355,7 @@ static ctl_table nf_ct_sysctl_table[] = {
},
{
.procname = "nf_conntrack_buckets",
- .data = &nf_conntrack_htable_size,
+ .data = &init_net.ct.htable_size,
.maxlen = sizeof(unsigned int),
.mode = 0444,
.proc_handler = proc_dointvec,
@@ -421,6 +421,7 @@ static int nf_conntrack_standalone_init_sysctl(struct net *net)
goto out_kmemdup;
table[1].data = &net->ct.count;
+ table[2].data = &net->ct.htable_size;
table[3].data = &net->ct.sysctl_checksum;
table[4].data = &net->ct.sysctl_log_invalid;
^ permalink raw reply related
* Re: [PATCH 0/3 v4] macvtap driver
From: Ed Swierk @ 2010-02-08 17:14 UTC (permalink / raw)
To: arnd; +Cc: netdev
In-Reply-To: <20100203.202104.151098461.davem@davemloft.net>
> From: Arnd Bergmann <arnd@arndb.de>
> Date: Sat, 30 Jan 2010 23:22:15 +0100
>
>> This is the fourth version of the macvtap driver,
>> based on the comments I got for the last version
>> I got a few days ago. Very few changes:
>>
>> * release netdev in chardev open function so
>> we can destroy it properly.
>> * Implement TUNSETSNDBUF
>> * fix sleeping call in rcu_read_lock
>> * Fix comment in namespace isolation patch
>> * Fix small context difference to make it apply
>> to net-next
>>
>> I can't really test here while travelling, so please
>> give it a go if you're interested in this driver.
I'm seeing complaints from might_sleep():
Feb 8 16:21:06 ti102 kernel: BUG: sleeping function called from
invalid context at include/linux/kernel.h:155
Feb 8 16:21:06 ti102 kernel: in_atomic(): 1, irqs_disabled(): 0, pid:
2881, name: qemu-kvm
Feb 8 16:21:06 ti102 kernel: Pid: 2881, comm: qemu-kvm Not tainted
2.6.29.6.Ar-224527.2009eswierk8 #1
Feb 8 16:21:06 ti102 kernel: Call Trace:
Feb 8 16:21:06 ti102 kernel: [<c0119250>] __might_sleep+0xdc/0xe3
Feb 8 16:21:06 ti102 kernel: [<c0210f7c>] copy_to_user+0x36/0x106
Feb 8 16:21:06 ti102 kernel: [<c02af568>] memcpy_toiovec+0x2c/0x50
Feb 8 16:21:06 ti102 kernel: [<c02afbb3>] skb_copy_datagram_iovec+0x47/0x184
Feb 8 16:21:06 ti102 kernel: [<c034bd07>] ? _spin_unlock_irqrestore+0x17/0x2c
Feb 8 16:21:06 ti102 kernel: [<f829a776>]
macvtap_aio_read+0x102/0x158 [macvtap]
Feb 8 16:21:06 ti102 kernel: [<c011eaf7>] ? default_wake_function+0x0/0xd
Feb 8 16:21:06 ti102 kernel: [<c016c75f>] do_sync_read+0xab/0xe9
Feb 8 16:21:06 ti102 kernel: [<c0133933>] ? autoremove_wake_function+0x0/0x33
Feb 8 16:21:06 ti102 kernel: [<c019211f>] ? eventfd_read+0x121/0x156
Feb 8 16:21:06 ti102 kernel: [<c011eaf7>] ? default_wake_function+0x0/0xd
Feb 8 16:21:06 ti102 kernel: [<c016d101>] vfs_read+0xb5/0x129
Feb 8 16:21:06 ti102 kernel: [<c016d20e>] sys_read+0x3b/0x60
Feb 8 16:21:06 ti102 kernel: [<c0102e71>] sysenter_do_call+0x12/0x25
Feb 8 16:21:08 ti102 kernel: BUG: sleeping function called from
invalid context at include/linux/kernel.h:155
Feb 8 16:21:08 ti102 kernel: in_atomic(): 1, irqs_disabled(): 0, pid:
2882, name: qemu-kvm
Feb 8 16:21:08 ti102 kernel: Pid: 2882, comm: qemu-kvm Not tainted
2.6.29.6.Ar-224527.2009eswierk8 #1
Feb 8 16:21:08 ti102 kernel: Call Trace:
Feb 8 16:21:08 ti102 kernel: [<c0119250>] __might_sleep+0xdc/0xe3
Feb 8 16:21:08 ti102 kernel: [<c0210e5f>] copy_from_user+0x34/0x11b
Feb 8 16:21:08 ti102 kernel: [<c02af388>] memcpy_fromiovec+0x2c/0x50
Feb 8 16:21:08 ti102 kernel: [<c02afa30>]
skb_copy_datagram_from_iovec+0x47/0x183
Feb 8 16:21:08 ti102 kernel: [<f829a618>]
macvtap_aio_write+0xaa/0x106 [macvtap]
Feb 8 16:21:08 ti102 kernel: [<c034a553>] ? mutex_unlock+0x8/0xa
Feb 8 16:21:08 ti102 kernel: [<c016c58d>] do_sync_readv_writev+0xa1/0xdf
Feb 8 16:21:08 ti102 kernel: [<c0133933>] ? autoremove_wake_function+0x0/0x33
Feb 8 16:21:08 ti102 kernel: [<c016c44f>] ? rw_copy_check_uvector+0x5b/0xc3
Feb 8 16:21:08 ti102 kernel: [<c016cbec>] do_readv_writev+0x82/0x165
Feb 8 16:21:08 ti102 kernel: [<f829a56e>] ?
macvtap_aio_write+0x0/0x106 [macvtap]
Feb 8 16:21:08 ti102 kernel: [<c0177461>] ? do_vfs_ioctl+0x4a3/0x4dc
Feb 8 16:21:08 ti102 kernel: [<c0112d83>] ? read_hpet+0xf/0x13
Feb 8 16:21:08 ti102 kernel: [<c016cd1e>] vfs_writev+0x4f/0x6b
Feb 8 16:21:08 ti102 kernel: [<c016cd75>] sys_writev+0x3b/0x60
Feb 8 16:21:08 ti102 kernel: [<c0102e71>] sysenter_do_call+0x12/0x25
I backported your patch to kernel 2.6.29.6, an i386 kernel with
CONFIG_PREEMPT=y and CONFIG_CLASSIC_RCU=y. It's entirely possible that
I screwed something up in the backport; I can post my modified patch
if it would help.
--Ed
^ permalink raw reply
* RE: MAX_SKB_FRAGS and GRO
From: Duyck, Alexander H @ 2010-02-08 17:35 UTC (permalink / raw)
To: Herbert Xu, Anton Blanchard
Cc: Kirsher, Jeffrey T, Brandeburg, Jesse, Allan, Bruce W,
Waskiewicz Jr, Peter P, Ronciak, John, divy@chelsio.com,
netdev@vger.kernel.org
In-Reply-To: <20100208124704.GA8538@gondor.apana.org.au>
Herbert Xu wrote:
> On Mon, Feb 08, 2010 at 09:03:07PM +1100, Anton Blanchard wrote:
>>
>> I was looking through the hardware GRO support in various drivers
>> and I think we have a couple of issues with PAGE_SIZE > 4k. For
>> example, if we have a 64kB page size then MAX_SKB_FRAGS ends up as 3:
>>
>> #define MAX_SKB_FRAGS (65536/PAGE_SIZE + 2)
>>
>> This should be fine for hardware and software GSO, but we encounter
>> issues with hardware GRO (not sure about software GRO).
>>
>> In the ixgbe case we use MAX_SKB_FRAGS to program the max number of
>> GRO descriptors, even though we assemble GRO packets using
>> ->frag_list:
>>
>> #if (MAX_SKB_FRAGS > 16)
>> rscctrl |= IXGBE_RSCCTL_MAXDESC_16; #elif
>> (MAX_SKB_FRAGS > 8) rscctrl |=
>> IXGBE_RSCCTL_MAXDESC_8; #elif (MAX_SKB_FRAGS > 4)
>> rscctrl |= IXGBE_RSCCTL_MAXDESC_4;
>> #else
>> rscctrl |= IXGBE_RSCCTL_MAXDESC_1;
>> #endif
>
> First of all this isn't GRO, but RSC. With GRO we impose extra
> restrictions on what packets can be merged while RSC is more
> permissive.
>
> In fact I think the ixgbe code may be broken as it is since it's
> not marking RSC packets in any way to prevent them from being
> forwarded through another interface.
>
> As to your problem with RSC on a 64K page system, I'm sure one
> of the Intel developers would be able to help you out.
>
>> Thinking out aloud, would setting a pessimistic value for
>> MAX_SKB_FRAGS
>> be one way to fix this? ie:
>>
>> #define MAX_SKB_FRAGS (65536/4096 + 2)
>
> While I can't think of any serious issues with this, as this is
> an entirely ixgbe-specific problem, the fix should probably stay
> there.
>
> Cheers,
Herbert is 100% correct, this is HW RSC and not GRO.
When we are using packet split mode we don't use the frag_list, we use frags and just append the buffers as pages. This is why we use MAX_SKB_FRAGS to determine how many descriptors can be used for a single RSC receive. The reason for doing it this way is because we can then hand it off to a process like GRO to join up to 3 frames together on systems with 64K pages.
In regards to the bridging/forwarding issue I don't think there is a problem there since we use the LRO flag to determine if HW RSC can be enabled and so typically if you put the port into a forwarding/bridging state HW RSC is automatically disabled. I will recommend that our validation team double check that though.
If you are running in single buffer mode, which is the mode that uses the frag_list, then you can join up to 16 descriptors as I recall. However we must avoid joining enough descriptors to result in 64K or more bytes per frame so usually we end up getting a max of 32K per RSC context simply due to the fact that everything is based on powers of 2.
Thanks,
Alex
^ permalink raw reply
* Re: MAX_SKB_FRAGS and GRO
From: Brandeburg, Jesse @ 2010-02-08 17:41 UTC (permalink / raw)
To: Herbert Xu
Cc: Anton Blanchard, Kirsher, Jeffrey T, Allan, Bruce W,
Duyck, Alexander H, Waskiewicz Jr, Peter P, Ronciak, John,
divy@chelsio.com, netdev@vger.kernel.org
In-Reply-To: <20100208124704.GA8538@gondor.apana.org.au>
On Mon, 8 Feb 2010, Herbert Xu wrote:
> On Mon, Feb 08, 2010 at 09:03:07PM +1100, Anton Blanchard wrote:
> >
> > I was looking through the hardware GRO support in various drivers and I think
> > we have a couple of issues with PAGE_SIZE > 4k. For example, if we have a 64kB
> > page size then MAX_SKB_FRAGS ends up as 3:
> >
> > #define MAX_SKB_FRAGS (65536/PAGE_SIZE + 2)
> >
> > This should be fine for hardware and software GSO, but we encounter issues
> > with hardware GRO (not sure about software GRO).
> >
> > In the ixgbe case we use MAX_SKB_FRAGS to program the max number of GRO
> > descriptors, even though we assemble GRO packets using ->frag_list:
> >
> > #if (MAX_SKB_FRAGS > 16)
> > rscctrl |= IXGBE_RSCCTL_MAXDESC_16;
> > #elif (MAX_SKB_FRAGS > 8)
> > rscctrl |= IXGBE_RSCCTL_MAXDESC_8;
> > #elif (MAX_SKB_FRAGS > 4)
> > rscctrl |= IXGBE_RSCCTL_MAXDESC_4;
> > #else
> > rscctrl |= IXGBE_RSCCTL_MAXDESC_1;
> > #endif
>
> First of all this isn't GRO, but RSC. With GRO we impose extra
> restrictions on what packets can be merged while RSC is more
> permissive.
Herbert is right. Just for clarity lets not call it "hardware GRO" but
instead just call it RSC or hardware RSC (receive side coalescing)
> In fact I think the ixgbe code may be broken as it is since it's
> not marking RSC packets in any way to prevent them from being
> forwarded through another interface.
Herbert, what are we missing? Is there some interface for us to do what
you describe? do we need to set DODGY?
> As to your problem with RSC on a 64K page system, I'm sure one
> of the Intel developers would be able to help you out.
I'm here, Anton, what can I do or test specifically for you?
> > Thinking out aloud, would setting a pessimistic value for MAX_SKB_FRAGS
> > be one way to fix this? ie:
> >
> > #define MAX_SKB_FRAGS (65536/4096 + 2)
>
> While I can't think of any serious issues with this, as this is
> an entirely ixgbe-specific problem, the fix should probably stay
> there.
I've always seen MAX_SKB_FRAGS as being the primary way of making sure
that an SKB never has more than 64kB of data in it. If we can stuff >
64kB of data in an skb then some of the above problem with 64kB pages goes
away.
^ permalink raw reply
* Re: MAX_SKB_FRAGS and GRO
From: Rick Jones @ 2010-02-08 17:53 UTC (permalink / raw)
To: Brandeburg, Jesse
Cc: Herbert Xu, Anton Blanchard, Kirsher, Jeffrey T, Allan, Bruce W,
Duyck, Alexander H, Waskiewicz Jr, Peter P, Ronciak, John,
divy@chelsio.com, netdev@vger.kernel.org
In-Reply-To: <alpine.WNT.2.00.1002080936180.6892@jbrandeb-desk1.amr.corp.intel.com>
>>First of all this isn't GRO, but RSC. With GRO we impose extra
>>restrictions on what packets can be merged while RSC is more
>>permissive.
>
>
> Herbert is right. Just for clarity lets not call it "hardware GRO" but
> instead just call it RSC or hardware RSC (receive side coalescing)
I'll paint a stripe on the bikeshed and suggest something even more
explicit - if it is the hardware, how about HRC (Hardware Receive
Coalescing) or NRC (Nic Receive Coalescing).
rick jones
^ permalink raw reply
* Re: [PATCH 0/3 v4] macvtap driver
From: Sridhar Samudrala @ 2010-02-08 18:55 UTC (permalink / raw)
To: Ed Swierk; +Cc: arnd, netdev
In-Reply-To: <9ae48b021002080914j2811e7cejd1c29aa3653ba8de@mail.gmail.com>
On Mon, 2010-02-08 at 09:14 -0800, Ed Swierk wrote:
> > From: Arnd Bergmann <arnd@arndb.de>
> > Date: Sat, 30 Jan 2010 23:22:15 +0100
> >
> >> This is the fourth version of the macvtap driver,
> >> based on the comments I got for the last version
> >> I got a few days ago. Very few changes:
> >>
> >> * release netdev in chardev open function so
> >> we can destroy it properly.
> >> * Implement TUNSETSNDBUF
> >> * fix sleeping call in rcu_read_lock
> >> * Fix comment in namespace isolation patch
> >> * Fix small context difference to make it apply
> >> to net-next
> >>
> >> I can't really test here while travelling, so please
> >> give it a go if you're interested in this driver.
>
> I'm seeing complaints from might_sleep():
>
> Feb 8 16:21:06 ti102 kernel: BUG: sleeping function called from
> invalid context at include/linux/kernel.h:155
> Feb 8 16:21:06 ti102 kernel: in_atomic(): 1, irqs_disabled(): 0, pid:
> 2881, name: qemu-kvm
> Feb 8 16:21:06 ti102 kernel: Pid: 2881, comm: qemu-kvm Not tainted
> 2.6.29.6.Ar-224527.2009eswierk8 #1
> Feb 8 16:21:06 ti102 kernel: Call Trace:
> Feb 8 16:21:06 ti102 kernel: [<c0119250>] __might_sleep+0xdc/0xe3
> Feb 8 16:21:06 ti102 kernel: [<c0210f7c>] copy_to_user+0x36/0x106
> Feb 8 16:21:06 ti102 kernel: [<c02af568>] memcpy_toiovec+0x2c/0x50
> Feb 8 16:21:06 ti102 kernel: [<c02afbb3>] skb_copy_datagram_iovec+0x47/0x184
> Feb 8 16:21:06 ti102 kernel: [<c034bd07>] ? _spin_unlock_irqrestore+0x17/0x2c
> Feb 8 16:21:06 ti102 kernel: [<f829a776>]
> macvtap_aio_read+0x102/0x158 [macvtap]
> Feb 8 16:21:06 ti102 kernel: [<c011eaf7>] ? default_wake_function+0x0/0xd
> Feb 8 16:21:06 ti102 kernel: [<c016c75f>] do_sync_read+0xab/0xe9
> Feb 8 16:21:06 ti102 kernel: [<c0133933>] ? autoremove_wake_function+0x0/0x33
> Feb 8 16:21:06 ti102 kernel: [<c019211f>] ? eventfd_read+0x121/0x156
> Feb 8 16:21:06 ti102 kernel: [<c011eaf7>] ? default_wake_function+0x0/0xd
> Feb 8 16:21:06 ti102 kernel: [<c016d101>] vfs_read+0xb5/0x129
> Feb 8 16:21:06 ti102 kernel: [<c016d20e>] sys_read+0x3b/0x60
> Feb 8 16:21:06 ti102 kernel: [<c0102e71>] sysenter_do_call+0x12/0x25
I am also seeing this issue with net-next-2.6.
Basically macvtap_put_user() and macvtap_get_user() call copy_to/from_user
from within a RCU read-side critical section.
The following patch fixes this issue by releasing the RCU read lock before
calling these routines, but instead hold a reference to q->sk.
Signed-off-by: Sridhar Samudrala <sri@us.ibm.com>
diff --git a/drivers/net/macvtap.c b/drivers/net/macvtap.c
index ad1f6ef..e3102ab 100644
--- a/drivers/net/macvtap.c
+++ b/drivers/net/macvtap.c
@@ -320,7 +320,7 @@ out:
}
/* Get packet from user space buffer */
-static ssize_t macvtap_get_user(struct macvtap_queue *q,
+static ssize_t macvtap_get_user(struct macvlan_dev *vlan, struct sock *sk,
const struct iovec *iv, size_t count,
int noblock)
{
@@ -331,10 +331,10 @@ static ssize_t macvtap_get_user(struct macvtap_queue *q,
if (unlikely(len < ETH_HLEN))
return -EINVAL;
- skb = sock_alloc_send_skb(&q->sk, NET_IP_ALIGN + len, noblock, &err);
+ skb = sock_alloc_send_skb(sk, NET_IP_ALIGN + len, noblock, &err);
if (!skb) {
- macvlan_count_rx(q->vlan, 0, false, false);
+ macvlan_count_rx(vlan, 0, false, false);
return err;
}
@@ -342,14 +342,14 @@ static ssize_t macvtap_get_user(struct macvtap_queue *q,
skb_put(skb, count);
if (skb_copy_datagram_from_iovec(skb, 0, iv, 0, len)) {
- macvlan_count_rx(q->vlan, 0, false, false);
+ macvlan_count_rx(vlan, 0, false, false);
kfree_skb(skb);
return -EFAULT;
}
skb_set_network_header(skb, ETH_HLEN);
- macvlan_start_xmit(skb, q->vlan->dev);
+ macvlan_start_xmit(skb, vlan->dev);
return count;
}
@@ -360,23 +360,29 @@ static ssize_t macvtap_aio_write(struct kiocb *iocb, const struct iovec *iv,
struct file *file = iocb->ki_filp;
ssize_t result = -ENOLINK;
struct macvtap_queue *q = macvtap_file_get_queue(file);
+ struct macvlan_dev *vlan;
+ struct sock *sk;
if (!q)
goto out;
- result = macvtap_get_user(q, iv, iov_length(iv, count),
+ vlan = q->vlan;
+ sk = &q->sk;
+ sock_hold(sk);
+ macvtap_file_put_queue();
+
+ result = macvtap_get_user(vlan, sk, iv, iov_length(iv, count),
file->f_flags & O_NONBLOCK);
+ sock_put(sk);
out:
- macvtap_file_put_queue();
return result;
}
/* Put packet to the user space buffer */
-static ssize_t macvtap_put_user(struct macvtap_queue *q,
+static ssize_t macvtap_put_user(struct macvlan_dev *vlan,
const struct sk_buff *skb,
const struct iovec *iv, int len)
{
- struct macvlan_dev *vlan = q->vlan;
int ret;
len = min_t(int, skb->len, len);
@@ -393,15 +399,20 @@ static ssize_t macvtap_aio_read(struct kiocb *iocb, const struct iovec *iv,
{
struct file *file = iocb->ki_filp;
struct macvtap_queue *q = macvtap_file_get_queue(file);
+ struct macvlan_dev *vlan;
+ struct sock *sk;
DECLARE_WAITQUEUE(wait, current);
struct sk_buff *skb;
ssize_t len, ret = 0;
- if (!q) {
- ret = -ENOLINK;
- goto out;
- }
+ if (!q)
+ return -ENOLINK;
+
+ vlan = q->vlan;
+ sk = &q->sk;
+ sock_hold(sk);
+ macvtap_file_put_queue();
len = iov_length(iv, count);
if (len < 0) {
@@ -409,12 +420,12 @@ static ssize_t macvtap_aio_read(struct kiocb *iocb, const struct iovec *iv,
goto out;
}
- add_wait_queue(q->sk.sk_sleep, &wait);
+ add_wait_queue(sk->sk_sleep, &wait);
while (len) {
current->state = TASK_INTERRUPTIBLE;
/* Read frames from the queue */
- skb = skb_dequeue(&q->sk.sk_receive_queue);
+ skb = skb_dequeue(&sk->sk_receive_queue);
if (!skb) {
if (file->f_flags & O_NONBLOCK) {
ret = -EAGAIN;
@@ -428,16 +439,16 @@ static ssize_t macvtap_aio_read(struct kiocb *iocb, const struct iovec *iv,
schedule();
continue;
}
- ret = macvtap_put_user(q, skb, iv, len);
+ ret = macvtap_put_user(vlan, skb, iv, len);
kfree_skb(skb);
break;
}
current->state = TASK_RUNNING;
- remove_wait_queue(q->sk.sk_sleep, &wait);
+ remove_wait_queue(sk->sk_sleep, &wait);
out:
- macvtap_file_put_queue();
+ sock_put(sk);
return ret;
}
^ permalink raw reply related
* Re: MAX_SKB_FRAGS and GRO
From: Ben Hutchings @ 2010-02-08 19:12 UTC (permalink / raw)
To: Rick Jones
Cc: Brandeburg, Jesse, Herbert Xu, Anton Blanchard,
Kirsher, Jeffrey T, Allan, Bruce W, Duyck, Alexander H,
Waskiewicz Jr, Peter P, Ronciak, John, divy@chelsio.com,
netdev@vger.kernel.org
In-Reply-To: <4B704FAF.4010209@hp.com>
On Mon, 2010-02-08 at 09:53 -0800, Rick Jones wrote:
> >>First of all this isn't GRO, but RSC. With GRO we impose extra
> >>restrictions on what packets can be merged while RSC is more
> >>permissive.
> >
> >
> > Herbert is right. Just for clarity lets not call it "hardware GRO" but
> > instead just call it RSC or hardware RSC (receive side coalescing)
>
> I'll paint a stripe on the bikeshed and suggest something even more
> explicit - if it is the hardware, how about HRC (Hardware Receive
> Coalescing) or NRC (Nic Receive Coalescing).
The term 'Large Receive Offload' (LRO) has been around for a while;
what's wrong with that? Qualify it with 'hardware' (HLRO) if necessary.
Any name involving 'coalescing' would easily be confused with interrupt
moderation, which ethtool calls coalescing. Also, using anything close
to a proprietary name would only reward and encourage the marketing
geniuses who invent redundant and confusing terminology.
Ben.
--
Ben Hutchings, Senior Software Engineer, Solarflare Communications
Not speaking for my employer; that's the marketing department's job.
They asked us to note that Solarflare product names are trademarked.
^ permalink raw reply
* Re: netfilter 00/05: netfilter fixes
From: David Miller @ 2010-02-08 19:15 UTC (permalink / raw)
To: kaber; +Cc: netdev, netfilter-devel
In-Reply-To: <20100208171024.15522.8136.sendpatchset@x2.localnet>
From: Patrick McHardy <kaber@trash.net>
Date: Mon, 8 Feb 2010 18:10:26 +0100 (MET)
> Please apply or pull from:
>
> git://git.kernel.org/pub/scm/linux/kernel/git/kaber/nf-2.6.git master
>
> Please note: I forgot to add "Cc: stable@kernel.org" to two of these
> patches and manually added it to the patch files, so I'd appreciate
> if you could apply the patches manually instead of pulling from the
> git tree this time.
Ok, I'll apply these by hand.
Thanks Patrick.
^ permalink raw reply
* [PATCH] dst: call cond_resched() in dst_gc_task()
From: Eric Dumazet @ 2010-02-08 19:32 UTC (permalink / raw)
To: Paweł Staszewski, David Miller; +Cc: Linux Network Development list
In-Reply-To: <1265639549.3048.33.camel@edumazet-laptop>
Le lundi 08 février 2010 à 15:32 +0100, Eric Dumazet a écrit :
> Le lundi 08 février 2010 à 15:16 +0100, Paweł Staszewski a écrit :
>
> > >
> > Some day ago after info about route cache i was have also this info:
>
> > Code: fe 79 4c 00 48 85 db 74 14 48 8b 74 24 10 48 89 ef ff 13 48 83 c3 08 48
> > 83 3b 00 eb ea 48 83 c4 18 5b 5d 41 5c 41 5d 41 5e 41 5f<c3> 55 48 89 f5 53 48
> > 89 fb 48 83 ec 08 48 8b 76 18 48 2b 75 10
> > Call Trace:
> > <IRQ> [<ffffffff8126826f>] ? e1000_put_txbuf+0x62/0x74
> > [<ffffffff8126834a>] ? e1000_clean_tx_irq+0xc9/0x235
> > [<ffffffff8126b71b>] ? e1000_clean+0x5c/0x21c
> > [<ffffffff812f29a3>] ? net_rx_action+0x71/0x15d
> > [<ffffffff81035311>] ? __do_softirq+0xd7/0x196
> > [<ffffffff81002dac>] ? call_softirq+0x1c/0x28
> > [<ffffffff812f768f>] ? dst_gc_task+0x0/0x1a7
> > [<ffffffff81002dac>] ? call_softirq+0x1c/0x28
> > <EOI> [<ffffffff81004599>] ? do_softirq+0x31/0x63
> > [<ffffffff81034ec1>] ? local_bh_enable_ip+0x75/0x86
> > [<ffffffff812f768f>] ? dst_gc_task+0x0/0x1a7
> > [<ffffffff812f775d>] ? dst_gc_task+0xce/0x1a7
> > [<ffffffff8136b08c>] ? schedule+0x82c/0x906
> > [<ffffffff8103c44f>] ? lock_timer_base+0x26/0x4b
> > [<ffffffff810a41d6>] ? cache_reap+0x0/0x11d
> > [<ffffffff81044c38>] ? worker_thread+0x14c/0x1dc
> > [<ffffffff81047dcd>] ? autoremove_wake_function+0x0/0x2e
> > [<ffffffff81044aec>] ? worker_thread+0x0/0x1dc
> > [<ffffffff810479bd>] ? kthread+0x79/0x81
> > [<ffffffff81002cb4>] ? kernel_thread_helper+0x4/0x10
> > [<ffffffff81047944>] ? kthread+0x0/0x81
> >
> >
> > [<ffffffff81002cb0>] ? kernel_thread_helper+0x0/0x10
> >
> >
>
> This trace is indeed very interesting, since dst_gc_task() is run from a
> work queue, and there is no scheduling point in it.
>
> We might need add a scheduling point in dst_gc_task() in case huge
> number of entries were flushed.
>
David, here is the patch I sent to Pawel to solve this problem.
This probaby is a stable candidate.
Thanks
[PATCH] dst: call cond_resched() in dst_gc_task()
On some workloads, it is quite possible to get a huge dst list to
process in dst_gc_task(), and trigger soft lockup detection.
Fix is to call cond_resched(), as we run in process context.
Reported-by: Pawel Staszewski <pstaszewski@itcare.pl>
Tested-by: Pawel Staszewski <pstaszewski@itcare.pl>
Signed-off-by: Eric Dumazet <eric.dumazet@gmail.com>
---
diff --git a/net/core/dst.c b/net/core/dst.c
index 57bc4d5..cb1b348 100644
--- a/net/core/dst.c
+++ b/net/core/dst.c
@@ -17,6 +17,7 @@
#include <linux/string.h>
#include <linux/types.h>
#include <net/net_namespace.h>
+#include <linux/sched.h>
#include <net/dst.h>
@@ -79,6 +80,7 @@ loop:
while ((dst = next) != NULL) {
next = dst->next;
prefetch(&next->next);
+ cond_resched();
if (likely(atomic_read(&dst->__refcnt))) {
last->next = dst;
last = dst;
^ permalink raw reply related
* [PATCH v2 00/41] CAPI: Major rework, tons of bug fixes
From: Jan Kiszka @ 2010-02-08 20:12 UTC (permalink / raw)
To: David Miller, Karsten Keil
Cc: linux-kernel, i4ldeveloper, isdn4linux, netdev, Alan Cox,
Marcel Holtmann
Here is the second take of my CAPI rework. I tried to address all
feedback on v1, so this one comes with the following changes:
o rebased over net-next
o reworked locking of the NCCI TTY using latest and greatest tty_port
features and kref, hopefully in the right way
o more fine-grained patch steps for the tricky TTY rework
o dynamic major for NCCI TTYs, ie. CAPI no longer claims 191
o dynamic TTY minor registrations, completely obsoleting capifs
o schedule capifs for removal
o some small additional cleanups
I kept my capifs fixes though this thing should die in the future. The
work is done, and people may still use it due to their distro
configuration (and the hard-wired loading by capiinit - needs fixing).
You can pull this series from
git://git.kiszka.org/linux-2.6.git capi
Jan
Jan Kiszka (41):
CAPI: Fix leaks in capifs_new_ncci
CAPI: Sanitize capifs API
CAPI: Eliminate capifs_root variable
CAPI: Pin capifs instead of mounting it
CAPI: Reduce chattiness during module loading/removal
CAPI: Call a controller 'controller', not 'card'
CAPI: Convert capi drivers rwlock into mutex
CAPI: Rework capi_ctr_ready/down
CAPI: Rework controller state notifier
CAPI: Rework locking of controller data structures
CAPI: Rework application locking
CAPI: Reduce #ifdef mess around CONFIG_ISDN_CAPI_MIDDLEWARE
CAPI: Convert capidev_list_lock into a mutex
CAPI: Clean up capi_open/release
CAPI: Rework locking of capidev members
CAPI: Use non-atomic allocation during NCCI setup
CAPI: Fix racy capi_read
CAPI: Switch NCCI list to standard doubly linked list
CAPI: Switch capiminor list to array
CAPI: Clean up capinc_tty_init/exit
CAPI: Dynamically register minor devices
CAPI: Use dynamic major for NCCI TTYs by default
CAPI: Use kref on capiminor
CAPI: Establish install/cleanup handlers for capiminor TTYs
CAPI: Use tty_port to keep track of capiminor's tty
CAPI: Drop remaining NULL checks on tty->driver_data
CAPI: Issue synchronous hangup on capincci_free_minor
CAPI: Drop obsolete nccip from capiminor struct
CAPI: Clean up capiminors_lock
CAPI: Drop atomic ttyopencount
CAPI: Drop handle_minor_recv from capinc_tty_write
CAPI: Rework capiminor RX handler
CAPI: Rename datahandle_queue -> ackqueue_entry
CAPI: Use atomics for capiminor's datahandle and msgid
CAPI: Drop capiminor's unused inbytes counter
CAPI: Fix locking around capiminor's output queue and drop
workaround_lock
CAPI: Clean up capiminor_*_ack
CAPI: Drop return value of handle_minor_send
CAPI: Drop special controller lookup from capi20_put_message
CAPI: Schedule capifs for removal
CAPI: Remove experimental tag from middleware feature
Documentation/feature-removal-schedule.txt | 10 +
drivers/isdn/capi/Kconfig | 16 +-
drivers/isdn/capi/capi.c | 1108 ++++++++++++++--------------
drivers/isdn/capi/capidrv.c | 48 +-
drivers/isdn/capi/capifs.c | 126 ++--
drivers/isdn/capi/capifs.h | 21 +-
drivers/isdn/capi/kcapi.c | 805 +++++++++++---------
drivers/isdn/capi/kcapi.h | 13 +-
drivers/isdn/capi/kcapi_proc.c | 41 +-
include/linux/isdn/capilli.h | 5 +-
include/linux/kernelcapi.h | 17 +-
11 files changed, 1151 insertions(+), 1059 deletions(-)
^ permalink raw reply
* [PATCH v2 01/41] CAPI: Fix leaks in capifs_new_ncci
From: Jan Kiszka @ 2010-02-08 20:12 UTC (permalink / raw)
To: David Miller, Karsten Keil
Cc: linux-kernel, i4ldeveloper, isdn4linux, netdev, Alan Cox,
Marcel Holtmann
In-Reply-To: <cover.1265659933.git.jan.kiszka@web.de>
When something went wrong during capifs_new_ncci, the looked up dentry
was not properly released. Neither was the allocated inode. Refactor the
function to avoid leaks.
Signed-off-by: Jan Kiszka <jan.kiszka@web.de>
---
drivers/isdn/capi/capifs.c | 25 ++++++++++++++++++-------
1 files changed, 18 insertions(+), 7 deletions(-)
diff --git a/drivers/isdn/capi/capifs.c b/drivers/isdn/capi/capifs.c
index 9f8f67b..dc68fcb 100644
--- a/drivers/isdn/capi/capifs.c
+++ b/drivers/isdn/capi/capifs.c
@@ -152,22 +152,33 @@ static struct dentry *get_node(int num)
void capifs_new_ncci(unsigned int number, dev_t device)
{
struct dentry *dentry;
- struct inode *inode = new_inode(capifs_mnt->mnt_sb);
- if (!inode)
- return;
- inode->i_ino = number+2;
+ struct inode *inode;
dentry = get_node(number);
+ if (IS_ERR(dentry))
+ goto unlock_out;
+
+ if (dentry->d_inode) {
+ dput(dentry);
+ goto unlock_out;
+ }
+
+ inode = new_inode(capifs_mnt->mnt_sb);
+ if (!inode) {
+ dput(dentry);
+ goto unlock_out;
+ }
/* config contents is protected by root's i_mutex */
inode->i_uid = config.setuid ? config.uid : current_fsuid();
inode->i_gid = config.setgid ? config.gid : current_fsgid();
inode->i_mtime = inode->i_atime = inode->i_ctime = CURRENT_TIME;
+ inode->i_ino = number + 2;
init_special_inode(inode, S_IFCHR|config.mode, device);
- //inode->i_op = &capifs_file_inode_operations;
- if (!IS_ERR(dentry) && !dentry->d_inode)
- d_instantiate(dentry, inode);
+ d_instantiate(dentry, inode);
+
+unlock_out:
mutex_unlock(&capifs_root->d_inode->i_mutex);
}
--
1.6.0.2
^ permalink raw reply related
* [PATCH v2 02/41] CAPI: Sanitize capifs API
From: Jan Kiszka @ 2010-02-08 20:12 UTC (permalink / raw)
To: David Miller, Karsten Keil
Cc: linux-kernel, i4ldeveloper, isdn4linux, netdev, Alan Cox,
Marcel Holtmann
In-Reply-To: <cover.1265659933.git.jan.kiszka@web.de>
Instead of looking up the dentry of an NCCI node again in
capifs_free_ncci pass the pointer via the capifs user.
This patch also reduces the #ifdef mess in capi.c a bit as far as capifs
was causing it.
Signed-off-by: Jan Kiszka <jan.kiszka@web.de>
---
drivers/isdn/capi/capi.c | 14 +++++-------
drivers/isdn/capi/capifs.c | 50 +++++++++++++++++++++++++------------------
drivers/isdn/capi/capifs.h | 21 ++++++++++++++++-
3 files changed, 54 insertions(+), 31 deletions(-)
diff --git a/drivers/isdn/capi/capi.c b/drivers/isdn/capi/capi.c
index 79f9364..dc5ac52 100644
--- a/drivers/isdn/capi/capi.c
+++ b/drivers/isdn/capi/capi.c
@@ -42,9 +42,8 @@
#include <linux/moduleparam.h>
#include <linux/isdn/capiutil.h>
#include <linux/isdn/capicmd.h>
-#if defined(CONFIG_ISDN_CAPI_CAPIFS) || defined(CONFIG_ISDN_CAPI_CAPIFS_MODULE)
+
#include "capifs.h"
-#endif
static char *revision = "$Revision: 1.1.2.7 $";
@@ -96,6 +95,7 @@ struct capiminor {
struct list_head list;
struct capincci *nccip;
unsigned int minor;
+ struct dentry *capifs_dentry;
struct capi20_appl *ap;
u32 ncci;
@@ -328,9 +328,9 @@ static struct capincci *capincci_alloc(struct capidev *cdev, u32 ncci)
#ifdef _DEBUG_REFCOUNT
printk(KERN_DEBUG "set mp->nccip\n");
#endif
-#if defined(CONFIG_ISDN_CAPI_CAPIFS) || defined(CONFIG_ISDN_CAPI_CAPIFS_MODULE)
- capifs_new_ncci(mp->minor, MKDEV(capi_ttymajor, mp->minor));
-#endif
+ mp->capifs_dentry =
+ capifs_new_ncci(mp->minor,
+ MKDEV(capi_ttymajor, mp->minor));
}
#endif /* CONFIG_ISDN_CAPI_MIDDLEWARE */
for (pp=&cdev->nccis; *pp; pp = &(*pp)->next)
@@ -353,9 +353,7 @@ static void capincci_free(struct capidev *cdev, u32 ncci)
*pp = (*pp)->next;
#ifdef CONFIG_ISDN_CAPI_MIDDLEWARE
if ((mp = np->minorp) != NULL) {
-#if defined(CONFIG_ISDN_CAPI_CAPIFS) || defined(CONFIG_ISDN_CAPI_CAPIFS_MODULE)
- capifs_free_ncci(mp->minor);
-#endif
+ capifs_free_ncci(mp->capifs_dentry);
if (mp->tty) {
mp->nccip = NULL;
#ifdef _DEBUG_REFCOUNT
diff --git a/drivers/isdn/capi/capifs.c b/drivers/isdn/capi/capifs.c
index dc68fcb..91aafad 100644
--- a/drivers/isdn/capi/capifs.c
+++ b/drivers/isdn/capi/capifs.c
@@ -141,31 +141,32 @@ static struct file_system_type capifs_fs_type = {
.kill_sb = kill_anon_super,
};
-static struct dentry *get_node(int num)
-{
- char s[10];
- struct dentry *root = capifs_root;
- mutex_lock(&root->d_inode->i_mutex);
- return lookup_one_len(s, root, sprintf(s, "%d", num));
-}
-
-void capifs_new_ncci(unsigned int number, dev_t device)
+struct dentry *capifs_new_ncci(unsigned int number, dev_t device)
{
struct dentry *dentry;
struct inode *inode;
+ char name[10];
+ int namelen;
- dentry = get_node(number);
- if (IS_ERR(dentry))
+ mutex_lock(&capifs_root->d_inode->i_mutex);
+
+ namelen = sprintf(name, "%d", number);
+ dentry = lookup_one_len(name, capifs_root, namelen);
+ if (IS_ERR(dentry)) {
+ dentry = NULL;
goto unlock_out;
+ }
if (dentry->d_inode) {
dput(dentry);
+ dentry = NULL;
goto unlock_out;
}
inode = new_inode(capifs_mnt->mnt_sb);
if (!inode) {
dput(dentry);
+ dentry = NULL;
goto unlock_out;
}
@@ -177,24 +178,31 @@ void capifs_new_ncci(unsigned int number, dev_t device)
init_special_inode(inode, S_IFCHR|config.mode, device);
d_instantiate(dentry, inode);
+ dget(dentry);
unlock_out:
mutex_unlock(&capifs_root->d_inode->i_mutex);
+
+ return dentry;
}
-void capifs_free_ncci(unsigned int number)
+void capifs_free_ncci(struct dentry *dentry)
{
- struct dentry *dentry = get_node(number);
-
- if (!IS_ERR(dentry)) {
- struct inode *inode = dentry->d_inode;
- if (inode) {
- inode->i_nlink--;
- d_delete(dentry);
- dput(dentry);
- }
+ struct inode *inode;
+
+ if (!dentry)
+ return;
+
+ mutex_lock(&capifs_root->d_inode->i_mutex);
+
+ inode = dentry->d_inode;
+ if (inode) {
+ drop_nlink(inode);
+ d_delete(dentry);
dput(dentry);
}
+ dput(dentry);
+
mutex_unlock(&capifs_root->d_inode->i_mutex);
}
diff --git a/drivers/isdn/capi/capifs.h b/drivers/isdn/capi/capifs.h
index d0bd4c3..e193d11 100644
--- a/drivers/isdn/capi/capifs.h
+++ b/drivers/isdn/capi/capifs.h
@@ -7,5 +7,22 @@
*
*/
-void capifs_new_ncci(unsigned int num, dev_t device);
-void capifs_free_ncci(unsigned int num);
+#include <linux/dcache.h>
+
+#if defined(CONFIG_ISDN_CAPI_CAPIFS) || defined(CONFIG_ISDN_CAPI_CAPIFS_MODULE)
+
+struct dentry *capifs_new_ncci(unsigned int num, dev_t device);
+void capifs_free_ncci(struct dentry *dentry);
+
+#else
+
+static inline struct dentry *capifs_new_ncci(unsigned int num, dev_t device)
+{
+ return NULL;
+}
+
+static inline void capifs_free_ncci(struct dentry *dentry)
+{
+}
+
+#endif
--
1.6.0.2
^ permalink raw reply related
* [PATCH v2 05/41] CAPI: Reduce chattiness during module loading/removal
From: Jan Kiszka @ 2010-02-08 20:12 UTC (permalink / raw)
To: David Miller, Karsten Keil
Cc: linux-kernel, i4ldeveloper, isdn4linux, netdev, Alan Cox,
Marcel Holtmann
In-Reply-To: <cover.1265659933.git.jan.kiszka@web.de>
The CVS revisions dumped by all CAPI modules are meaningless today. And
that some CAPI module is loaded or removed does not necessarily deserve
a message. Just keep the message of the central module, capi.ko, drop
the rest.
Signed-off-by: Jan Kiszka <jan.kiszka@web.de>
---
drivers/isdn/capi/capi.c | 19 +++----------------
drivers/isdn/capi/capidrv.c | 26 --------------------------
drivers/isdn/capi/capifs.c | 20 +-------------------
drivers/isdn/capi/kcapi.c | 27 +++++----------------------
4 files changed, 9 insertions(+), 83 deletions(-)
diff --git a/drivers/isdn/capi/capi.c b/drivers/isdn/capi/capi.c
index dc5ac52..3b07797 100644
--- a/drivers/isdn/capi/capi.c
+++ b/drivers/isdn/capi/capi.c
@@ -45,8 +45,6 @@
#include "capifs.h"
-static char *revision = "$Revision: 1.1.2.7 $";
-
MODULE_DESCRIPTION("CAPI4Linux: Userspace /dev/capi20 interface");
MODULE_AUTHOR("Carsten Paeth");
MODULE_LICENSE("GPL");
@@ -1489,21 +1487,11 @@ static void __exit proc_exit(void)
/* -------- init function and module interface ---------------------- */
-static char rev[32];
-
static int __init capi_init(void)
{
- char *p;
- char *compileinfo;
+ const char *compileinfo;
int major_ret;
- if ((p = strchr(revision, ':')) != NULL && p[1]) {
- strlcpy(rev, p + 2, sizeof(rev));
- if ((p = strchr(rev, '$')) != NULL && p > rev)
- *(p-1) = 0;
- } else
- strcpy(rev, "1.0");
-
major_ret = register_chrdev(capi_major, "capi20", &capi_fops);
if (major_ret < 0) {
printk(KERN_ERR "capi20: unable to get major %d\n", capi_major);
@@ -1537,8 +1525,8 @@ static int __init capi_init(void)
#else
compileinfo = " (no middleware)";
#endif
- printk(KERN_NOTICE "capi20: Rev %s: started up with major %d%s\n",
- rev, capi_major, compileinfo);
+ printk(KERN_NOTICE "CAPI 2.0 started up with major %d%s\n",
+ capi_major, compileinfo);
return 0;
}
@@ -1554,7 +1542,6 @@ static void __exit capi_exit(void)
#ifdef CONFIG_ISDN_CAPI_MIDDLEWARE
capinc_tty_exit();
#endif
- printk(KERN_NOTICE "capi: Rev %s: unloaded\n", rev);
}
module_init(capi_init);
diff --git a/drivers/isdn/capi/capidrv.c b/drivers/isdn/capi/capidrv.c
index bb45015..7d8899a 100644
--- a/drivers/isdn/capi/capidrv.c
+++ b/drivers/isdn/capi/capidrv.c
@@ -35,7 +35,6 @@
#include <linux/isdn/capicmd.h>
#include "capidrv.h"
-static char *revision = "$Revision: 1.1.2.2 $";
static int debugmode = 0;
MODULE_DESCRIPTION("CAPI4Linux: Interface to ISDN4Linux");
@@ -2266,19 +2265,9 @@ static void __exit proc_exit(void)
static int __init capidrv_init(void)
{
capi_profile profile;
- char rev[32];
- char *p;
u32 ncontr, contr;
u16 errcode;
- if ((p = strchr(revision, ':')) != NULL && p[1]) {
- strncpy(rev, p + 2, sizeof(rev));
- rev[sizeof(rev)-1] = 0;
- if ((p = strchr(rev, '$')) != NULL && p > rev)
- *(p-1) = 0;
- } else
- strcpy(rev, "1.0");
-
global.ap.rparam.level3cnt = -2; /* number of bchannels twice */
global.ap.rparam.datablkcnt = 16;
global.ap.rparam.datablklen = 2048;
@@ -2306,29 +2295,14 @@ static int __init capidrv_init(void)
}
proc_init();
- printk(KERN_NOTICE "capidrv: Rev %s: loaded\n", rev);
return 0;
}
static void __exit capidrv_exit(void)
{
- char rev[32];
- char *p;
-
- if ((p = strchr(revision, ':')) != NULL) {
- strncpy(rev, p + 1, sizeof(rev));
- rev[sizeof(rev)-1] = 0;
- if ((p = strchr(rev, '$')) != NULL)
- *p = 0;
- } else {
- strcpy(rev, " ??? ");
- }
-
capi20_release(&global.ap);
proc_exit();
-
- printk(KERN_NOTICE "capidrv: Rev%s: unloaded\n", rev);
}
module_init(capidrv_init);
diff --git a/drivers/isdn/capi/capifs.c b/drivers/isdn/capi/capifs.c
index 51c01ef..8596bd1 100644
--- a/drivers/isdn/capi/capifs.c
+++ b/drivers/isdn/capi/capifs.c
@@ -25,10 +25,6 @@ MODULE_LICENSE("GPL");
/* ------------------------------------------------------------------ */
-static char *revision = "$Revision: 1.1.2.3 $";
-
-/* ------------------------------------------------------------------ */
-
#define CAPIFS_SUPER_MAGIC (('C'<<8)|'N')
static struct vfsmount *capifs_mnt;
@@ -227,21 +223,7 @@ void capifs_free_ncci(struct dentry *dentry)
static int __init capifs_init(void)
{
- char rev[32];
- char *p;
- int err;
-
- if ((p = strchr(revision, ':')) != NULL && p[1]) {
- strlcpy(rev, p + 2, sizeof(rev));
- if ((p = strchr(rev, '$')) != NULL && p > rev)
- *(p-1) = 0;
- } else
- strcpy(rev, "1.0");
-
- err = register_filesystem(&capifs_fs_type);
- if (!err)
- printk(KERN_NOTICE "capifs: Rev %s\n", rev);
- return err;
+ return register_filesystem(&capifs_fs_type);
}
static void __exit capifs_exit(void)
diff --git a/drivers/isdn/capi/kcapi.c b/drivers/isdn/capi/kcapi.c
index b0bacf3..ef564ee 100644
--- a/drivers/isdn/capi/kcapi.c
+++ b/drivers/isdn/capi/kcapi.c
@@ -35,10 +35,6 @@
#endif
#include <linux/mutex.h>
-static char *revision = "$Revision: 1.1.2.8 $";
-
-/* ------------------------------------------------------------- */
-
static int showcapimsgs = 0;
MODULE_DESCRIPTION("CAPI4Linux: kernel CAPI layer");
@@ -1165,25 +1161,12 @@ EXPORT_SYMBOL(capi20_set_callback);
static int __init kcapi_init(void)
{
- char *p;
- char rev[32];
- int ret;
-
- ret = cdebug_init();
- if (ret)
- return ret;
- kcapi_proc_init();
+ int err;
- if ((p = strchr(revision, ':')) != NULL && p[1]) {
- strlcpy(rev, p + 2, sizeof(rev));
- if ((p = strchr(rev, '$')) != NULL && p > rev)
- *(p-1) = 0;
- } else
- strcpy(rev, "1.0");
-
- printk(KERN_NOTICE "CAPI Subsystem Rev %s\n", rev);
-
- return 0;
+ err = cdebug_init();
+ if (!err)
+ kcapi_proc_init();
+ return err;
}
static void __exit kcapi_exit(void)
--
1.6.0.2
^ permalink raw reply related
* [PATCH v2 06/41] CAPI: Call a controller 'controller', not 'card'
From: Jan Kiszka @ 2010-02-08 20:12 UTC (permalink / raw)
To: David Miller, Karsten Keil
Cc: linux-kernel, i4ldeveloper, isdn4linux, netdev, Alan Cox,
Marcel Holtmann
In-Reply-To: <cover.1265659933.git.jan.kiszka@web.de>
At least for our internal use, fix the misnomers that refer to a CAPI
controller as 'card'. No functional changes.
Signed-off-by: Jan Kiszka <jan.kiszka@web.de>
---
drivers/isdn/capi/kcapi.c | 320 +++++++++++++++++++++-------------------
drivers/isdn/capi/kcapi.h | 8 +-
drivers/isdn/capi/kcapi_proc.c | 17 +-
include/linux/isdn/capilli.h | 2 +-
4 files changed, 178 insertions(+), 169 deletions(-)
diff --git a/drivers/isdn/capi/kcapi.c b/drivers/isdn/capi/kcapi.c
index ef564ee..f37c13b 100644
--- a/drivers/isdn/capi/kcapi.c
+++ b/drivers/isdn/capi/kcapi.c
@@ -67,24 +67,24 @@ static DEFINE_RWLOCK(application_lock);
static DEFINE_MUTEX(controller_mutex);
struct capi20_appl *capi_applications[CAPI_MAXAPPL];
-struct capi_ctr *capi_cards[CAPI_MAXCONTR];
+struct capi_ctr *capi_controller[CAPI_MAXCONTR];
-static int ncards;
+static int ncontrollers;
/* -------- controller ref counting -------------------------------------- */
static inline struct capi_ctr *
-capi_ctr_get(struct capi_ctr *card)
+capi_ctr_get(struct capi_ctr *ctr)
{
- if (!try_module_get(card->owner))
+ if (!try_module_get(ctr->owner))
return NULL;
- return card;
+ return ctr;
}
static inline void
-capi_ctr_put(struct capi_ctr *card)
+capi_ctr_put(struct capi_ctr *ctr)
{
- module_put(card->owner);
+ module_put(ctr->owner);
}
/* ------------------------------------------------------------- */
@@ -94,7 +94,7 @@ static inline struct capi_ctr *get_capi_ctr_by_nr(u16 contr)
if (contr - 1 >= CAPI_MAXCONTR)
return NULL;
- return capi_cards[contr - 1];
+ return capi_controller[contr - 1];
}
static inline struct capi20_appl *get_capi_appl_by_nr(u16 applid)
@@ -144,46 +144,48 @@ static inline int capi_subcmd_valid(u8 subcmd)
/* ------------------------------------------------------------ */
-static void register_appl(struct capi_ctr *card, u16 applid, capi_register_params *rparam)
+static void
+register_appl(struct capi_ctr *ctr, u16 applid, capi_register_params *rparam)
{
- card = capi_ctr_get(card);
+ ctr = capi_ctr_get(ctr);
- if (card)
- card->register_appl(card, applid, rparam);
+ if (ctr)
+ ctr->register_appl(ctr, applid, rparam);
else
- printk(KERN_WARNING "%s: cannot get card resources\n", __func__);
+ printk(KERN_WARNING "%s: cannot get controller resources\n",
+ __func__);
}
-static void release_appl(struct capi_ctr *card, u16 applid)
+static void release_appl(struct capi_ctr *ctr, u16 applid)
{
DBG("applid %#x", applid);
- card->release_appl(card, applid);
- capi_ctr_put(card);
+ ctr->release_appl(ctr, applid);
+ capi_ctr_put(ctr);
}
/* -------- KCI_CONTRUP --------------------------------------- */
static void notify_up(u32 contr)
{
- struct capi_ctr *card = get_capi_ctr_by_nr(contr);
+ struct capi_ctr *ctr = get_capi_ctr_by_nr(contr);
struct capi20_appl *ap;
u16 applid;
if (showcapimsgs & 1) {
printk(KERN_DEBUG "kcapi: notify up contr %d\n", contr);
}
- if (!card) {
+ if (!ctr) {
printk(KERN_WARNING "%s: invalid contr %d\n", __func__, contr);
return;
}
for (applid = 1; applid <= CAPI_MAXAPPL; applid++) {
ap = get_capi_appl_by_nr(applid);
if (!ap || ap->release_in_progress) continue;
- register_appl(card, applid, &ap->rparam);
+ register_appl(ctr, applid, &ap->rparam);
if (ap->callback && !ap->release_in_progress)
- ap->callback(KCI_CONTRUP, contr, &card->profile);
+ ap->callback(KCI_CONTRUP, contr, &ctr->profile);
}
}
@@ -269,14 +271,15 @@ static void recv_handler(struct work_struct *work)
/**
* capi_ctr_handle_message() - handle incoming CAPI message
- * @card: controller descriptor structure.
+ * @ctr: controller descriptor structure.
* @appl: application ID.
* @skb: message.
*
* Called by hardware driver to pass a CAPI message to the application.
*/
-void capi_ctr_handle_message(struct capi_ctr * card, u16 appl, struct sk_buff *skb)
+void capi_ctr_handle_message(struct capi_ctr *ctr, u16 appl,
+ struct sk_buff *skb)
{
struct capi20_appl *ap;
int showctl = 0;
@@ -284,43 +287,45 @@ void capi_ctr_handle_message(struct capi_ctr * card, u16 appl, struct sk_buff *s
unsigned long flags;
_cdebbuf *cdb;
- if (card->cardstate != CARD_RUNNING) {
+ if (ctr->state != CAPI_CTR_RUNNING) {
cdb = capi_message2str(skb->data);
if (cdb) {
printk(KERN_INFO "kcapi: controller [%03d] not active, got: %s",
- card->cnr, cdb->buf);
+ ctr->cnr, cdb->buf);
cdebbuf_free(cdb);
} else
printk(KERN_INFO "kcapi: controller [%03d] not active, cannot trace\n",
- card->cnr);
+ ctr->cnr);
goto error;
}
cmd = CAPIMSG_COMMAND(skb->data);
subcmd = CAPIMSG_SUBCOMMAND(skb->data);
if (cmd == CAPI_DATA_B3 && subcmd == CAPI_IND) {
- card->nrecvdatapkt++;
- if (card->traceflag > 2) showctl |= 2;
+ ctr->nrecvdatapkt++;
+ if (ctr->traceflag > 2)
+ showctl |= 2;
} else {
- card->nrecvctlpkt++;
- if (card->traceflag) showctl |= 2;
+ ctr->nrecvctlpkt++;
+ if (ctr->traceflag)
+ showctl |= 2;
}
- showctl |= (card->traceflag & 1);
+ showctl |= (ctr->traceflag & 1);
if (showctl & 2) {
if (showctl & 1) {
printk(KERN_DEBUG "kcapi: got [%03d] id#%d %s len=%u\n",
- card->cnr, CAPIMSG_APPID(skb->data),
+ ctr->cnr, CAPIMSG_APPID(skb->data),
capi_cmd2str(cmd, subcmd),
CAPIMSG_LEN(skb->data));
} else {
cdb = capi_message2str(skb->data);
if (cdb) {
printk(KERN_DEBUG "kcapi: got [%03d] %s\n",
- card->cnr, cdb->buf);
+ ctr->cnr, cdb->buf);
cdebbuf_free(cdb);
} else
printk(KERN_DEBUG "kcapi: got [%03d] id#%d %s len=%u, cannot trace\n",
- card->cnr, CAPIMSG_APPID(skb->data),
+ ctr->cnr, CAPIMSG_APPID(skb->data),
capi_cmd2str(cmd, subcmd),
CAPIMSG_LEN(skb->data));
}
@@ -356,74 +361,75 @@ EXPORT_SYMBOL(capi_ctr_handle_message);
/**
* capi_ctr_ready() - signal CAPI controller ready
- * @card: controller descriptor structure.
+ * @ctr: controller descriptor structure.
*
* Called by hardware driver to signal that the controller is up and running.
*/
-void capi_ctr_ready(struct capi_ctr * card)
+void capi_ctr_ready(struct capi_ctr *ctr)
{
- card->cardstate = CARD_RUNNING;
+ ctr->state = CAPI_CTR_RUNNING;
- printk(KERN_NOTICE "kcapi: card [%03d] \"%s\" ready.\n",
- card->cnr, card->name);
+ printk(KERN_NOTICE "kcapi: controller [%03d] \"%s\" ready.\n",
+ ctr->cnr, ctr->name);
- notify_push(KCI_CONTRUP, card->cnr, 0, 0);
+ notify_push(KCI_CONTRUP, ctr->cnr, 0, 0);
}
EXPORT_SYMBOL(capi_ctr_ready);
/**
* capi_ctr_down() - signal CAPI controller not ready
- * @card: controller descriptor structure.
+ * @ctr: controller descriptor structure.
*
* Called by hardware driver to signal that the controller is down and
* unavailable for use.
*/
-void capi_ctr_down(struct capi_ctr * card)
+void capi_ctr_down(struct capi_ctr *ctr)
{
u16 appl;
DBG("");
- if (card->cardstate == CARD_DETECTED)
+ if (ctr->state == CAPI_CTR_DETECTED)
return;
- card->cardstate = CARD_DETECTED;
+ ctr->state = CAPI_CTR_DETECTED;
- memset(card->manu, 0, sizeof(card->manu));
- memset(&card->version, 0, sizeof(card->version));
- memset(&card->profile, 0, sizeof(card->profile));
- memset(card->serial, 0, sizeof(card->serial));
+ memset(ctr->manu, 0, sizeof(ctr->manu));
+ memset(&ctr->version, 0, sizeof(ctr->version));
+ memset(&ctr->profile, 0, sizeof(ctr->profile));
+ memset(ctr->serial, 0, sizeof(ctr->serial));
for (appl = 1; appl <= CAPI_MAXAPPL; appl++) {
struct capi20_appl *ap = get_capi_appl_by_nr(appl);
if (!ap || ap->release_in_progress)
continue;
- capi_ctr_put(card);
+ capi_ctr_put(ctr);
}
- printk(KERN_NOTICE "kcapi: card [%03d] down.\n", card->cnr);
+ printk(KERN_NOTICE "kcapi: controller [%03d] down.\n", ctr->cnr);
- notify_push(KCI_CONTRDOWN, card->cnr, 0, 0);
+ notify_push(KCI_CONTRDOWN, ctr->cnr, 0, 0);
}
EXPORT_SYMBOL(capi_ctr_down);
/**
* capi_ctr_suspend_output() - suspend controller
- * @card: controller descriptor structure.
+ * @ctr: controller descriptor structure.
*
* Called by hardware driver to stop data flow.
*/
-void capi_ctr_suspend_output(struct capi_ctr *card)
+void capi_ctr_suspend_output(struct capi_ctr *ctr)
{
- if (!card->blocked) {
- printk(KERN_DEBUG "kcapi: card [%03d] suspend\n", card->cnr);
- card->blocked = 1;
+ if (!ctr->blocked) {
+ printk(KERN_DEBUG "kcapi: controller [%03d] suspend\n",
+ ctr->cnr);
+ ctr->blocked = 1;
}
}
@@ -431,16 +437,17 @@ EXPORT_SYMBOL(capi_ctr_suspend_output);
/**
* capi_ctr_resume_output() - resume controller
- * @card: controller descriptor structure.
+ * @ctr: controller descriptor structure.
*
* Called by hardware driver to resume data flow.
*/
-void capi_ctr_resume_output(struct capi_ctr *card)
+void capi_ctr_resume_output(struct capi_ctr *ctr)
{
- if (card->blocked) {
- printk(KERN_DEBUG "kcapi: card [%03d] resume\n", card->cnr);
- card->blocked = 0;
+ if (ctr->blocked) {
+ printk(KERN_DEBUG "kcapi: controller [%03d] resumed\n",
+ ctr->cnr);
+ ctr->blocked = 0;
}
}
@@ -450,21 +457,20 @@ EXPORT_SYMBOL(capi_ctr_resume_output);
/**
* attach_capi_ctr() - register CAPI controller
- * @card: controller descriptor structure.
+ * @ctr: controller descriptor structure.
*
* Called by hardware driver to register a controller with the CAPI subsystem.
* Return value: 0 on success, error code < 0 on error
*/
-int
-attach_capi_ctr(struct capi_ctr *card)
+int attach_capi_ctr(struct capi_ctr *ctr)
{
int i;
mutex_lock(&controller_mutex);
for (i = 0; i < CAPI_MAXCONTR; i++) {
- if (capi_cards[i] == NULL)
+ if (!capi_controller[i])
break;
}
if (i == CAPI_MAXCONTR) {
@@ -472,25 +478,25 @@ attach_capi_ctr(struct capi_ctr *card)
printk(KERN_ERR "kcapi: out of controller slots\n");
return -EBUSY;
}
- capi_cards[i] = card;
+ capi_controller[i] = ctr;
mutex_unlock(&controller_mutex);
- card->nrecvctlpkt = 0;
- card->nrecvdatapkt = 0;
- card->nsentctlpkt = 0;
- card->nsentdatapkt = 0;
- card->cnr = i + 1;
- card->cardstate = CARD_DETECTED;
- card->blocked = 0;
- card->traceflag = showcapimsgs;
-
- sprintf(card->procfn, "capi/controllers/%d", card->cnr);
- card->procent = proc_create_data(card->procfn, 0, NULL, card->proc_fops, card);
-
- ncards++;
- printk(KERN_NOTICE "kcapi: Controller [%03d]: %s attached\n",
- card->cnr, card->name);
+ ctr->nrecvctlpkt = 0;
+ ctr->nrecvdatapkt = 0;
+ ctr->nsentctlpkt = 0;
+ ctr->nsentdatapkt = 0;
+ ctr->cnr = i + 1;
+ ctr->state = CAPI_CTR_DETECTED;
+ ctr->blocked = 0;
+ ctr->traceflag = showcapimsgs;
+
+ sprintf(ctr->procfn, "capi/controllers/%d", ctr->cnr);
+ ctr->procent = proc_create_data(ctr->procfn, 0, NULL, ctr->proc_fops, ctr);
+
+ ncontrollers++;
+ printk(KERN_NOTICE "kcapi: controller [%03d]: %s attached\n",
+ ctr->cnr, ctr->name);
return 0;
}
@@ -498,27 +504,27 @@ EXPORT_SYMBOL(attach_capi_ctr);
/**
* detach_capi_ctr() - unregister CAPI controller
- * @card: controller descriptor structure.
+ * @ctr: controller descriptor structure.
*
* Called by hardware driver to remove the registration of a controller
* with the CAPI subsystem.
* Return value: 0 on success, error code < 0 on error
*/
-int detach_capi_ctr(struct capi_ctr *card)
+int detach_capi_ctr(struct capi_ctr *ctr)
{
- if (card->cardstate != CARD_DETECTED)
- capi_ctr_down(card);
+ if (ctr->state != CAPI_CTR_DETECTED)
+ capi_ctr_down(ctr);
- ncards--;
+ ncontrollers--;
- if (card->procent) {
- remove_proc_entry(card->procfn, NULL);
- card->procent = NULL;
+ if (ctr->procent) {
+ remove_proc_entry(ctr->procfn, NULL);
+ ctr->procent = NULL;
}
- capi_cards[card->cnr - 1] = NULL;
- printk(KERN_NOTICE "kcapi: Controller [%03d]: %s unregistered\n",
- card->cnr, card->name);
+ capi_controller[ctr->cnr - 1] = NULL;
+ printk(KERN_NOTICE "kcapi: controller [%03d]: %s unregistered\n",
+ ctr->cnr, ctr->name);
return 0;
}
@@ -576,7 +582,8 @@ u16 capi20_isinstalled(void)
{
int i;
for (i = 0; i < CAPI_MAXCONTR; i++) {
- if (capi_cards[i] && capi_cards[i]->cardstate == CARD_RUNNING)
+ if (capi_controller[i] &&
+ capi_controller[i]->state == CAPI_CTR_RUNNING)
return CAPI_NOERROR;
}
return CAPI_REGNOTINSTALLED;
@@ -635,9 +642,10 @@ u16 capi20_register(struct capi20_appl *ap)
mutex_lock(&controller_mutex);
for (i = 0; i < CAPI_MAXCONTR; i++) {
- if (!capi_cards[i] || capi_cards[i]->cardstate != CARD_RUNNING)
+ if (!capi_controller[i] ||
+ capi_controller[i]->state != CAPI_CTR_RUNNING)
continue;
- register_appl(capi_cards[i], applid, &ap->rparam);
+ register_appl(capi_controller[i], applid, &ap->rparam);
}
mutex_unlock(&controller_mutex);
@@ -674,9 +682,10 @@ u16 capi20_release(struct capi20_appl *ap)
mutex_lock(&controller_mutex);
for (i = 0; i < CAPI_MAXCONTR; i++) {
- if (!capi_cards[i] || capi_cards[i]->cardstate != CARD_RUNNING)
+ if (!capi_controller[i] ||
+ capi_controller[i]->state != CAPI_CTR_RUNNING)
continue;
- release_appl(capi_cards[i], ap->applid);
+ release_appl(capi_controller[i], ap->applid);
}
mutex_unlock(&controller_mutex);
@@ -703,13 +712,13 @@ EXPORT_SYMBOL(capi20_release);
u16 capi20_put_message(struct capi20_appl *ap, struct sk_buff *skb)
{
- struct capi_ctr *card;
+ struct capi_ctr *ctr;
int showctl = 0;
u8 cmd, subcmd;
DBG("applid %#x", ap->applid);
- if (ncards == 0)
+ if (ncontrollers == 0)
return CAPI_REGNOTINSTALLED;
if ((ap->applid == 0) || ap->release_in_progress)
return CAPI_ILLAPPNR;
@@ -717,28 +726,30 @@ u16 capi20_put_message(struct capi20_appl *ap, struct sk_buff *skb)
|| !capi_cmd_valid(CAPIMSG_COMMAND(skb->data))
|| !capi_subcmd_valid(CAPIMSG_SUBCOMMAND(skb->data)))
return CAPI_ILLCMDORSUBCMDORMSGTOSMALL;
- card = get_capi_ctr_by_nr(CAPIMSG_CONTROLLER(skb->data));
- if (!card || card->cardstate != CARD_RUNNING) {
- card = get_capi_ctr_by_nr(1); // XXX why?
- if (!card || card->cardstate != CARD_RUNNING)
+ ctr = get_capi_ctr_by_nr(CAPIMSG_CONTROLLER(skb->data));
+ if (!ctr || ctr->state != CAPI_CTR_RUNNING) {
+ ctr = get_capi_ctr_by_nr(1); /* XXX why? */
+ if (!ctr || ctr->state != CAPI_CTR_RUNNING)
return CAPI_REGNOTINSTALLED;
}
- if (card->blocked)
+ if (ctr->blocked)
return CAPI_SENDQUEUEFULL;
cmd = CAPIMSG_COMMAND(skb->data);
subcmd = CAPIMSG_SUBCOMMAND(skb->data);
if (cmd == CAPI_DATA_B3 && subcmd== CAPI_REQ) {
- card->nsentdatapkt++;
+ ctr->nsentdatapkt++;
ap->nsentdatapkt++;
- if (card->traceflag > 2) showctl |= 2;
+ if (ctr->traceflag > 2)
+ showctl |= 2;
} else {
- card->nsentctlpkt++;
+ ctr->nsentctlpkt++;
ap->nsentctlpkt++;
- if (card->traceflag) showctl |= 2;
+ if (ctr->traceflag)
+ showctl |= 2;
}
- showctl |= (card->traceflag & 1);
+ showctl |= (ctr->traceflag & 1);
if (showctl & 2) {
if (showctl & 1) {
printk(KERN_DEBUG "kcapi: put [%03d] id#%d %s len=%u\n",
@@ -761,7 +772,7 @@ u16 capi20_put_message(struct capi20_appl *ap, struct sk_buff *skb)
CAPIMSG_LEN(skb->data));
}
}
- return card->send_message(card, skb);
+ return ctr->send_message(ctr, skb);
}
EXPORT_SYMBOL(capi20_put_message);
@@ -778,16 +789,16 @@ EXPORT_SYMBOL(capi20_put_message);
u16 capi20_get_manufacturer(u32 contr, u8 *buf)
{
- struct capi_ctr *card;
+ struct capi_ctr *ctr;
if (contr == 0) {
strlcpy(buf, capi_manufakturer, CAPI_MANUFACTURER_LEN);
return CAPI_NOERROR;
}
- card = get_capi_ctr_by_nr(contr);
- if (!card || card->cardstate != CARD_RUNNING)
+ ctr = get_capi_ctr_by_nr(contr);
+ if (!ctr || ctr->state != CAPI_CTR_RUNNING)
return CAPI_REGNOTINSTALLED;
- strlcpy(buf, card->manu, CAPI_MANUFACTURER_LEN);
+ strlcpy(buf, ctr->manu, CAPI_MANUFACTURER_LEN);
return CAPI_NOERROR;
}
@@ -805,17 +816,17 @@ EXPORT_SYMBOL(capi20_get_manufacturer);
u16 capi20_get_version(u32 contr, struct capi_version *verp)
{
- struct capi_ctr *card;
+ struct capi_ctr *ctr;
if (contr == 0) {
*verp = driver_version;
return CAPI_NOERROR;
}
- card = get_capi_ctr_by_nr(contr);
- if (!card || card->cardstate != CARD_RUNNING)
+ ctr = get_capi_ctr_by_nr(contr);
+ if (!ctr || ctr->state != CAPI_CTR_RUNNING)
return CAPI_REGNOTINSTALLED;
- memcpy((void *) verp, &card->version, sizeof(capi_version));
+ memcpy(verp, &ctr->version, sizeof(capi_version));
return CAPI_NOERROR;
}
@@ -833,17 +844,17 @@ EXPORT_SYMBOL(capi20_get_version);
u16 capi20_get_serial(u32 contr, u8 *serial)
{
- struct capi_ctr *card;
+ struct capi_ctr *ctr;
if (contr == 0) {
strlcpy(serial, driver_serial, CAPI_SERIAL_LEN);
return CAPI_NOERROR;
}
- card = get_capi_ctr_by_nr(contr);
- if (!card || card->cardstate != CARD_RUNNING)
+ ctr = get_capi_ctr_by_nr(contr);
+ if (!ctr || ctr->state != CAPI_CTR_RUNNING)
return CAPI_REGNOTINSTALLED;
- strlcpy((void *) serial, card->serial, CAPI_SERIAL_LEN);
+ strlcpy(serial, ctr->serial, CAPI_SERIAL_LEN);
return CAPI_NOERROR;
}
@@ -861,18 +872,17 @@ EXPORT_SYMBOL(capi20_get_serial);
u16 capi20_get_profile(u32 contr, struct capi_profile *profp)
{
- struct capi_ctr *card;
+ struct capi_ctr *ctr;
if (contr == 0) {
- profp->ncontroller = ncards;
+ profp->ncontroller = ncontrollers;
return CAPI_NOERROR;
}
- card = get_capi_ctr_by_nr(contr);
- if (!card || card->cardstate != CARD_RUNNING)
+ ctr = get_capi_ctr_by_nr(contr);
+ if (!ctr || ctr->state != CAPI_CTR_RUNNING)
return CAPI_REGNOTINSTALLED;
- memcpy((void *) profp, &card->profile,
- sizeof(struct capi_profile));
+ memcpy(profp, &ctr->profile, sizeof(struct capi_profile));
return CAPI_NOERROR;
}
@@ -885,7 +895,7 @@ static int old_capi_manufacturer(unsigned int cmd, void __user *data)
avmb1_extcarddef cdef;
avmb1_resetdef rdef;
capicardparams cparams;
- struct capi_ctr *card;
+ struct capi_ctr *ctr;
struct capi_driver *driver = NULL;
capiloaddata ldata;
struct list_head *l;
@@ -958,26 +968,26 @@ static int old_capi_manufacturer(unsigned int cmd, void __user *data)
sizeof(avmb1_loadandconfigdef)))
return -EFAULT;
}
- card = get_capi_ctr_by_nr(ldef.contr);
- if (!card)
+ ctr = get_capi_ctr_by_nr(ldef.contr);
+ if (!ctr)
return -EINVAL;
- card = capi_ctr_get(card);
- if (!card)
+ ctr = capi_ctr_get(ctr);
+ if (!ctr)
return -ESRCH;
- if (card->load_firmware == NULL) {
+ if (ctr->load_firmware == NULL) {
printk(KERN_DEBUG "kcapi: load: no load function\n");
- capi_ctr_put(card);
+ capi_ctr_put(ctr);
return -ESRCH;
}
if (ldef.t4file.len <= 0) {
printk(KERN_DEBUG "kcapi: load: invalid parameter: length of t4file is %d ?\n", ldef.t4file.len);
- capi_ctr_put(card);
+ capi_ctr_put(ctr);
return -EINVAL;
}
if (ldef.t4file.data == NULL) {
printk(KERN_DEBUG "kcapi: load: invalid parameter: dataptr is 0\n");
- capi_ctr_put(card);
+ capi_ctr_put(ctr);
return -EINVAL;
}
@@ -988,46 +998,46 @@ static int old_capi_manufacturer(unsigned int cmd, void __user *data)
ldata.configuration.data = ldef.t4config.data;
ldata.configuration.len = ldef.t4config.len;
- if (card->cardstate != CARD_DETECTED) {
+ if (ctr->state != CAPI_CTR_DETECTED) {
printk(KERN_INFO "kcapi: load: contr=%d not in detect state\n", ldef.contr);
- capi_ctr_put(card);
+ capi_ctr_put(ctr);
return -EBUSY;
}
- card->cardstate = CARD_LOADING;
+ ctr->state = CAPI_CTR_LOADING;
- retval = card->load_firmware(card, &ldata);
+ retval = ctr->load_firmware(ctr, &ldata);
if (retval) {
- card->cardstate = CARD_DETECTED;
- capi_ctr_put(card);
+ ctr->state = CAPI_CTR_DETECTED;
+ capi_ctr_put(ctr);
return retval;
}
- while (card->cardstate != CARD_RUNNING) {
+ while (ctr->state != CAPI_CTR_RUNNING) {
msleep_interruptible(100); /* 0.1 sec */
if (signal_pending(current)) {
- capi_ctr_put(card);
+ capi_ctr_put(ctr);
return -EINTR;
}
}
- capi_ctr_put(card);
+ capi_ctr_put(ctr);
return 0;
case AVMB1_RESETCARD:
if (copy_from_user(&rdef, data, sizeof(avmb1_resetdef)))
return -EFAULT;
- card = get_capi_ctr_by_nr(rdef.contr);
- if (!card)
+ ctr = get_capi_ctr_by_nr(rdef.contr);
+ if (!ctr)
return -ESRCH;
- if (card->cardstate == CARD_DETECTED)
+ if (ctr->state == CAPI_CTR_DETECTED)
return 0;
- card->reset_ctr(card);
+ ctr->reset_ctr(ctr);
- while (card->cardstate > CARD_DETECTED) {
+ while (ctr->state > CAPI_CTR_DETECTED) {
msleep_interruptible(100); /* 0.1 sec */
@@ -1052,7 +1062,7 @@ static int old_capi_manufacturer(unsigned int cmd, void __user *data)
int capi20_manufacturer(unsigned int cmd, void __user *data)
{
- struct capi_ctr *card;
+ struct capi_ctr *ctr;
switch (cmd) {
#ifdef AVMB1_COMPAT
@@ -1070,13 +1080,13 @@ int capi20_manufacturer(unsigned int cmd, void __user *data)
if (copy_from_user(&fdef, data, sizeof(kcapi_flagdef)))
return -EFAULT;
- card = get_capi_ctr_by_nr(fdef.contr);
- if (!card)
+ ctr = get_capi_ctr_by_nr(fdef.contr);
+ if (!ctr)
return -ESRCH;
- card->traceflag = fdef.flag;
+ ctr->traceflag = fdef.flag;
printk(KERN_INFO "kcapi: contr [%03d] set trace=%d\n",
- card->cnr, card->traceflag);
+ ctr->cnr, ctr->traceflag);
return 0;
}
case KCAPI_CMD_ADDCARD:
diff --git a/drivers/isdn/capi/kcapi.h b/drivers/isdn/capi/kcapi.h
index 244711f..f62c53b 100644
--- a/drivers/isdn/capi/kcapi.h
+++ b/drivers/isdn/capi/kcapi.h
@@ -24,16 +24,16 @@ printk(KERN_DEBUG "%s: " format "\n" , __func__ , ## arg); \
#endif
enum {
- CARD_DETECTED = 1,
- CARD_LOADING = 2,
- CARD_RUNNING = 3,
+ CAPI_CTR_DETECTED = 1,
+ CAPI_CTR_LOADING = 2,
+ CAPI_CTR_RUNNING = 3,
};
extern struct list_head capi_drivers;
extern rwlock_t capi_drivers_list_lock;
extern struct capi20_appl *capi_applications[CAPI_MAXAPPL];
-extern struct capi_ctr *capi_cards[CAPI_MAXCONTR];
+extern struct capi_ctr *capi_controller[CAPI_MAXCONTR];
#ifdef CONFIG_PROC_FS
diff --git a/drivers/isdn/capi/kcapi_proc.c b/drivers/isdn/capi/kcapi_proc.c
index 09d4db7..59fd16a 100644
--- a/drivers/isdn/capi/kcapi_proc.c
+++ b/drivers/isdn/capi/kcapi_proc.c
@@ -15,13 +15,12 @@
#include <linux/seq_file.h>
#include <linux/init.h>
-static char *
-cardstate2str(unsigned short cardstate)
+static char *state2str(unsigned short state)
{
- switch (cardstate) {
- case CARD_DETECTED: return "detected";
- case CARD_LOADING: return "loading";
- case CARD_RUNNING: return "running";
+ switch (state) {
+ case CAPI_CTR_DETECTED: return "detected";
+ case CAPI_CTR_LOADING: return "loading";
+ case CAPI_CTR_RUNNING: return "running";
default: return "???";
}
}
@@ -38,7 +37,7 @@ cardstate2str(unsigned short cardstate)
static void *controller_start(struct seq_file *seq, loff_t *pos)
{
if (*pos < CAPI_MAXCONTR)
- return &capi_cards[*pos];
+ return &capi_controller[*pos];
return NULL;
}
@@ -47,7 +46,7 @@ static void *controller_next(struct seq_file *seq, void *v, loff_t *pos)
{
++*pos;
if (*pos < CAPI_MAXCONTR)
- return &capi_cards[*pos];
+ return &capi_controller[*pos];
return NULL;
}
@@ -65,7 +64,7 @@ static int controller_show(struct seq_file *seq, void *v)
seq_printf(seq, "%d %-10s %-8s %-16s %s\n",
ctr->cnr, ctr->driver_name,
- cardstate2str(ctr->cardstate),
+ state2str(ctr->state),
ctr->name,
ctr->procinfo ? ctr->procinfo(ctr) : "");
diff --git a/include/linux/isdn/capilli.h b/include/linux/isdn/capilli.h
index d3e5e9d..856f38e 100644
--- a/include/linux/isdn/capilli.h
+++ b/include/linux/isdn/capilli.h
@@ -66,7 +66,7 @@ struct capi_ctr {
unsigned long nsentdatapkt;
int cnr; /* controller number */
- volatile unsigned short cardstate; /* controller state */
+ volatile unsigned short state; /* controller state */
volatile int blocked; /* output blocked */
int traceflag; /* capi trace */
--
1.6.0.2
^ permalink raw reply related
* [PATCH v2 07/41] CAPI: Convert capi drivers rwlock into mutex
From: Jan Kiszka @ 2010-02-08 20:12 UTC (permalink / raw)
To: David Miller, Karsten Keil
Cc: linux-kernel, i4ldeveloper, isdn4linux, netdev, Alan Cox,
Marcel Holtmann
In-Reply-To: <cover.1265659933.git.jan.kiszka@web.de>
Turn the lock protecting registered capi drivers into a mutex and apply
it consistently.
Signed-off-by: Jan Kiszka <jan.kiszka@web.de>
---
drivers/isdn/capi/kcapi.c | 49 ++++++++++++++++++----------------------
drivers/isdn/capi/kcapi.h | 2 +-
drivers/isdn/capi/kcapi_proc.c | 8 +++---
3 files changed, 27 insertions(+), 32 deletions(-)
diff --git a/drivers/isdn/capi/kcapi.c b/drivers/isdn/capi/kcapi.c
index f37c13b..c46964f 100644
--- a/drivers/isdn/capi/kcapi.c
+++ b/drivers/isdn/capi/kcapi.c
@@ -61,7 +61,7 @@ static char capi_manufakturer[64] = "AVM Berlin";
#define NCCI2CTRL(ncci) (((ncci) >> 24) & 0x7f)
LIST_HEAD(capi_drivers);
-DEFINE_RWLOCK(capi_drivers_list_lock);
+DEFINE_MUTEX(capi_drivers_lock);
static DEFINE_RWLOCK(application_lock);
static DEFINE_MUTEX(controller_mutex);
@@ -540,11 +540,9 @@ EXPORT_SYMBOL(detach_capi_ctr);
void register_capi_driver(struct capi_driver *driver)
{
- unsigned long flags;
-
- write_lock_irqsave(&capi_drivers_list_lock, flags);
+ mutex_lock(&capi_drivers_lock);
list_add_tail(&driver->list, &capi_drivers);
- write_unlock_irqrestore(&capi_drivers_list_lock, flags);
+ mutex_unlock(&capi_drivers_lock);
}
EXPORT_SYMBOL(register_capi_driver);
@@ -558,11 +556,9 @@ EXPORT_SYMBOL(register_capi_driver);
void unregister_capi_driver(struct capi_driver *driver)
{
- unsigned long flags;
-
- write_lock_irqsave(&capi_drivers_list_lock, flags);
+ mutex_lock(&capi_drivers_lock);
list_del(&driver->list);
- write_unlock_irqrestore(&capi_drivers_list_lock, flags);
+ mutex_unlock(&capi_drivers_lock);
}
EXPORT_SYMBOL(unregister_capi_driver);
@@ -899,7 +895,6 @@ static int old_capi_manufacturer(unsigned int cmd, void __user *data)
struct capi_driver *driver = NULL;
capiloaddata ldata;
struct list_head *l;
- unsigned long flags;
int retval;
switch (cmd) {
@@ -919,7 +914,8 @@ static int old_capi_manufacturer(unsigned int cmd, void __user *data)
cparams.irq = cdef.irq;
cparams.cardnr = cdef.cardnr;
- read_lock_irqsave(&capi_drivers_list_lock, flags);
+ mutex_lock(&capi_drivers_lock);
+
switch (cdef.cardtype) {
case AVM_CARDTYPE_B1:
list_for_each(l, &capi_drivers) {
@@ -940,18 +936,15 @@ static int old_capi_manufacturer(unsigned int cmd, void __user *data)
break;
}
if (!driver) {
- read_unlock_irqrestore(&capi_drivers_list_lock, flags);
printk(KERN_ERR "kcapi: driver not loaded.\n");
- return -EIO;
- }
- if (!driver->add_card) {
- read_unlock_irqrestore(&capi_drivers_list_lock, flags);
+ retval = -EIO;
+ } else if (!driver->add_card) {
printk(KERN_ERR "kcapi: driver has no add card function.\n");
- return -EIO;
- }
+ retval = -EIO;
+ } else
+ retval = driver->add_card(driver, &cparams);
- retval = driver->add_card(driver, &cparams);
- read_unlock_irqrestore(&capi_drivers_list_lock, flags);
+ mutex_unlock(&capi_drivers_lock);
return retval;
case AVMB1_LOAD:
@@ -1107,6 +1100,8 @@ int capi20_manufacturer(unsigned int cmd, void __user *data)
cparams.cardtype = 0;
cdef.driver[sizeof(cdef.driver)-1] = 0;
+ mutex_lock(&capi_drivers_lock);
+
list_for_each(l, &capi_drivers) {
driver = list_entry(l, struct capi_driver, list);
if (strcmp(driver->name, cdef.driver) == 0)
@@ -1115,15 +1110,15 @@ int capi20_manufacturer(unsigned int cmd, void __user *data)
if (driver == NULL) {
printk(KERN_ERR "kcapi: driver \"%s\" not loaded.\n",
cdef.driver);
- return -ESRCH;
- }
-
- if (!driver->add_card) {
+ retval = -ESRCH;
+ } else if (!driver->add_card) {
printk(KERN_ERR "kcapi: driver \"%s\" has no add card function.\n", cdef.driver);
- return -EIO;
- }
+ retval = -EIO;
+ } else
+ retval = driver->add_card(driver, &cparams);
- return driver->add_card(driver, &cparams);
+ mutex_unlock(&capi_drivers_lock);
+ return retval;
}
default:
diff --git a/drivers/isdn/capi/kcapi.h b/drivers/isdn/capi/kcapi.h
index f62c53b..07c5850 100644
--- a/drivers/isdn/capi/kcapi.h
+++ b/drivers/isdn/capi/kcapi.h
@@ -30,7 +30,7 @@ enum {
};
extern struct list_head capi_drivers;
-extern rwlock_t capi_drivers_list_lock;
+extern struct mutex capi_drivers_lock;
extern struct capi20_appl *capi_applications[CAPI_MAXAPPL];
extern struct capi_ctr *capi_controller[CAPI_MAXCONTR];
diff --git a/drivers/isdn/capi/kcapi_proc.c b/drivers/isdn/capi/kcapi_proc.c
index 59fd16a..71b0761 100644
--- a/drivers/isdn/capi/kcapi_proc.c
+++ b/drivers/isdn/capi/kcapi_proc.c
@@ -238,9 +238,9 @@ static const struct file_operations proc_applstats_ops = {
// ---------------------------------------------------------------------------
static void *capi_driver_start(struct seq_file *seq, loff_t *pos)
- __acquires(&capi_drivers_list_lock)
+ __acquires(&capi_drivers_lock)
{
- read_lock(&capi_drivers_list_lock);
+ mutex_lock(&capi_drivers_lock);
return seq_list_start(&capi_drivers, *pos);
}
@@ -250,9 +250,9 @@ static void *capi_driver_next(struct seq_file *seq, void *v, loff_t *pos)
}
static void capi_driver_stop(struct seq_file *seq, void *v)
- __releases(&capi_drivers_list_lock)
+ __releases(&capi_drivers_lock)
{
- read_unlock(&capi_drivers_list_lock);
+ mutex_unlock(&capi_drivers_lock);
}
static int capi_driver_show(struct seq_file *seq, void *v)
--
1.6.0.2
^ permalink raw reply related
* [PATCH v2 08/41] CAPI: Rework capi_ctr_ready/down
From: Jan Kiszka @ 2010-02-08 20:12 UTC (permalink / raw)
To: David Miller, Karsten Keil
Cc: linux-kernel, i4ldeveloper, isdn4linux, netdev, Alan Cox,
Marcel Holtmann
In-Reply-To: <cover.1265659933.git.jan.kiszka@web.de>
This step prepares the application of proper controller locking: Push
all state changing work into the notify handler that are called by
capi_ctr_ready and capi_ctr_down, switch detach_capi_ctr to issue a
synchronous ctr_down. Also ensure that we do not go through any action
if the state did not change.
Signed-off-by: Jan Kiszka <jan.kiszka@web.de>
---
drivers/isdn/capi/kcapi.c | 95 +++++++++++++++++++++++---------------------
1 files changed, 50 insertions(+), 45 deletions(-)
diff --git a/drivers/isdn/capi/kcapi.c b/drivers/isdn/capi/kcapi.c
index c46964f..9362a7a 100644
--- a/drivers/isdn/capi/kcapi.c
+++ b/drivers/isdn/capi/kcapi.c
@@ -169,44 +169,74 @@ static void release_appl(struct capi_ctr *ctr, u16 applid)
static void notify_up(u32 contr)
{
- struct capi_ctr *ctr = get_capi_ctr_by_nr(contr);
struct capi20_appl *ap;
+ struct capi_ctr *ctr;
u16 applid;
- if (showcapimsgs & 1) {
+ if (showcapimsgs & 1)
printk(KERN_DEBUG "kcapi: notify up contr %d\n", contr);
- }
- if (!ctr) {
+
+ ctr = get_capi_ctr_by_nr(contr);
+ if (ctr) {
+ if (ctr->state == CAPI_CTR_RUNNING)
+ return;
+
+ ctr->state = CAPI_CTR_RUNNING;
+
+ for (applid = 1; applid <= CAPI_MAXAPPL; applid++) {
+ ap = get_capi_appl_by_nr(applid);
+ if (!ap || ap->release_in_progress)
+ continue;
+ register_appl(ctr, applid, &ap->rparam);
+ if (ap->callback && !ap->release_in_progress)
+ ap->callback(KCI_CONTRUP, contr,
+ &ctr->profile);
+ }
+ } else
printk(KERN_WARNING "%s: invalid contr %d\n", __func__, contr);
- return;
- }
- for (applid = 1; applid <= CAPI_MAXAPPL; applid++) {
- ap = get_capi_appl_by_nr(applid);
- if (!ap || ap->release_in_progress) continue;
- register_appl(ctr, applid, &ap->rparam);
- if (ap->callback && !ap->release_in_progress)
- ap->callback(KCI_CONTRUP, contr, &ctr->profile);
- }
}
/* -------- KCI_CONTRDOWN ------------------------------------- */
-static void notify_down(u32 contr)
+static void ctr_down(struct capi_ctr *ctr)
{
struct capi20_appl *ap;
u16 applid;
- if (showcapimsgs & 1) {
- printk(KERN_DEBUG "kcapi: notify down contr %d\n", contr);
- }
+ if (ctr->state == CAPI_CTR_DETECTED)
+ return;
+
+ ctr->state = CAPI_CTR_DETECTED;
+
+ memset(ctr->manu, 0, sizeof(ctr->manu));
+ memset(&ctr->version, 0, sizeof(ctr->version));
+ memset(&ctr->profile, 0, sizeof(ctr->profile));
+ memset(ctr->serial, 0, sizeof(ctr->serial));
for (applid = 1; applid <= CAPI_MAXAPPL; applid++) {
ap = get_capi_appl_by_nr(applid);
- if (ap && ap->callback && !ap->release_in_progress)
- ap->callback(KCI_CONTRDOWN, contr, NULL);
+ if (ap && !ap->release_in_progress) {
+ if (ap->callback)
+ ap->callback(KCI_CONTRDOWN, ctr->cnr, NULL);
+ capi_ctr_put(ctr);
+ }
}
}
+static void notify_down(u32 contr)
+{
+ struct capi_ctr *ctr;
+
+ if (showcapimsgs & 1)
+ printk(KERN_DEBUG "kcapi: notify down contr %d\n", contr);
+
+ ctr = get_capi_ctr_by_nr(contr);
+ if (ctr)
+ ctr_down(ctr);
+ else
+ printk(KERN_WARNING "%s: invalid contr %d\n", __func__, contr);
+}
+
static void notify_handler(struct work_struct *work)
{
struct capi_notifier *np =
@@ -368,8 +398,6 @@ EXPORT_SYMBOL(capi_ctr_handle_message);
void capi_ctr_ready(struct capi_ctr *ctr)
{
- ctr->state = CAPI_CTR_RUNNING;
-
printk(KERN_NOTICE "kcapi: controller [%03d] \"%s\" ready.\n",
ctr->cnr, ctr->name);
@@ -388,28 +416,6 @@ EXPORT_SYMBOL(capi_ctr_ready);
void capi_ctr_down(struct capi_ctr *ctr)
{
- u16 appl;
-
- DBG("");
-
- if (ctr->state == CAPI_CTR_DETECTED)
- return;
-
- ctr->state = CAPI_CTR_DETECTED;
-
- memset(ctr->manu, 0, sizeof(ctr->manu));
- memset(&ctr->version, 0, sizeof(ctr->version));
- memset(&ctr->profile, 0, sizeof(ctr->profile));
- memset(ctr->serial, 0, sizeof(ctr->serial));
-
- for (appl = 1; appl <= CAPI_MAXAPPL; appl++) {
- struct capi20_appl *ap = get_capi_appl_by_nr(appl);
- if (!ap || ap->release_in_progress)
- continue;
-
- capi_ctr_put(ctr);
- }
-
printk(KERN_NOTICE "kcapi: controller [%03d] down.\n", ctr->cnr);
notify_push(KCI_CONTRDOWN, ctr->cnr, 0, 0);
@@ -513,8 +519,7 @@ EXPORT_SYMBOL(attach_capi_ctr);
int detach_capi_ctr(struct capi_ctr *ctr)
{
- if (ctr->state != CAPI_CTR_DETECTED)
- capi_ctr_down(ctr);
+ ctr_down(ctr);
ncontrollers--;
--
1.6.0.2
^ permalink raw reply related
* [PATCH v2 11/41] CAPI: Rework application locking
From: Jan Kiszka @ 2010-02-08 20:12 UTC (permalink / raw)
To: David Miller, Karsten Keil
Cc: linux-kernel, i4ldeveloper, isdn4linux, netdev, Alan Cox,
Marcel Holtmann
In-Reply-To: <cover.1265659933.git.jan.kiszka@web.de>
Drop the application rw-lock in favour of RCU. This synchronizes
capi20_release against capi_ctr_handle_message which may dereference an
application from (soft-)IRQ context. Any other access to the application
list is now protected by the capi_controller_lock as well. This also
allows to safely inspect applications for /proc dumping by holding
capi_controller_lock.
At this chance, drop some useless release_in_progress checks where we
obtained the application pointer from the list (which becomes NULL on
release_in_progress).
Signed-off-by: Jan Kiszka <jan.kiszka@web.de>
---
drivers/isdn/capi/kcapi.c | 52 +++++++++++++++++-----------------------
drivers/isdn/capi/kcapi_proc.c | 11 +++++---
2 files changed, 29 insertions(+), 34 deletions(-)
diff --git a/drivers/isdn/capi/kcapi.c b/drivers/isdn/capi/kcapi.c
index a99f7e3..0b4c8a7 100644
--- a/drivers/isdn/capi/kcapi.c
+++ b/drivers/isdn/capi/kcapi.c
@@ -34,6 +34,7 @@
#include <linux/b1lli.h>
#endif
#include <linux/mutex.h>
+#include <linux/rcupdate.h>
static int showcapimsgs = 0;
@@ -64,8 +65,6 @@ DEFINE_MUTEX(capi_drivers_lock);
struct capi_ctr *capi_controller[CAPI_MAXCONTR];
DEFINE_MUTEX(capi_controller_lock);
-static DEFINE_RWLOCK(application_lock);
-
struct capi20_appl *capi_applications[CAPI_MAXAPPL];
static int ncontrollers;
@@ -103,7 +102,7 @@ static inline struct capi20_appl *get_capi_appl_by_nr(u16 applid)
if (applid - 1 >= CAPI_MAXAPPL)
return NULL;
- return capi_applications[applid - 1];
+ return rcu_dereference(capi_applications[applid - 1]);
}
/* -------- util functions ------------------------------------ */
@@ -186,7 +185,7 @@ static void notify_up(u32 contr)
for (applid = 1; applid <= CAPI_MAXAPPL; applid++) {
ap = get_capi_appl_by_nr(applid);
- if (!ap || ap->release_in_progress)
+ if (!ap)
continue;
register_appl(ctr, applid, &ap->rparam);
}
@@ -216,7 +215,7 @@ static void ctr_down(struct capi_ctr *ctr, int new_state)
for (applid = 1; applid <= CAPI_MAXAPPL; applid++) {
ap = get_capi_appl_by_nr(applid);
- if (ap && !ap->release_in_progress)
+ if (ap)
capi_ctr_put(ctr);
}
@@ -336,7 +335,6 @@ void capi_ctr_handle_message(struct capi_ctr *ctr, u16 appl,
struct capi20_appl *ap;
int showctl = 0;
u8 cmd, subcmd;
- unsigned long flags;
_cdebbuf *cdb;
if (ctr->state != CAPI_CTR_RUNNING) {
@@ -384,10 +382,10 @@ void capi_ctr_handle_message(struct capi_ctr *ctr, u16 appl,
}
- read_lock_irqsave(&application_lock, flags);
+ rcu_read_lock();
ap = get_capi_appl_by_nr(CAPIMSG_APPID(skb->data));
- if ((!ap) || (ap->release_in_progress)) {
- read_unlock_irqrestore(&application_lock, flags);
+ if (!ap) {
+ rcu_read_unlock();
cdb = capi_message2str(skb->data);
if (cdb) {
printk(KERN_ERR "kcapi: handle_message: applid %d state released (%s)\n",
@@ -401,7 +399,7 @@ void capi_ctr_handle_message(struct capi_ctr *ctr, u16 appl,
}
skb_queue_tail(&ap->recv_queue, skb);
schedule_work(&ap->recv_work);
- read_unlock_irqrestore(&application_lock, flags);
+ rcu_read_unlock();
return;
@@ -656,40 +654,35 @@ u16 capi20_register(struct capi20_appl *ap)
{
int i;
u16 applid;
- unsigned long flags;
DBG("");
if (ap->rparam.datablklen < 128)
return CAPI_LOGBLKSIZETOSMALL;
- write_lock_irqsave(&application_lock, flags);
+ ap->nrecvctlpkt = 0;
+ ap->nrecvdatapkt = 0;
+ ap->nsentctlpkt = 0;
+ ap->nsentdatapkt = 0;
+ mutex_init(&ap->recv_mtx);
+ skb_queue_head_init(&ap->recv_queue);
+ INIT_WORK(&ap->recv_work, recv_handler);
+ ap->release_in_progress = 0;
+
+ mutex_lock(&capi_controller_lock);
for (applid = 1; applid <= CAPI_MAXAPPL; applid++) {
if (capi_applications[applid - 1] == NULL)
break;
}
if (applid > CAPI_MAXAPPL) {
- write_unlock_irqrestore(&application_lock, flags);
+ mutex_unlock(&capi_controller_lock);
return CAPI_TOOMANYAPPLS;
}
ap->applid = applid;
capi_applications[applid - 1] = ap;
- ap->nrecvctlpkt = 0;
- ap->nrecvdatapkt = 0;
- ap->nsentctlpkt = 0;
- ap->nsentdatapkt = 0;
- mutex_init(&ap->recv_mtx);
- skb_queue_head_init(&ap->recv_queue);
- INIT_WORK(&ap->recv_work, recv_handler);
- ap->release_in_progress = 0;
-
- write_unlock_irqrestore(&application_lock, flags);
-
- mutex_lock(&capi_controller_lock);
-
for (i = 0; i < CAPI_MAXCONTR; i++) {
if (!capi_controller[i] ||
capi_controller[i]->state != CAPI_CTR_RUNNING)
@@ -721,16 +714,15 @@ EXPORT_SYMBOL(capi20_register);
u16 capi20_release(struct capi20_appl *ap)
{
int i;
- unsigned long flags;
DBG("applid %#x", ap->applid);
- write_lock_irqsave(&application_lock, flags);
+ mutex_lock(&capi_controller_lock);
+
ap->release_in_progress = 1;
capi_applications[ap->applid - 1] = NULL;
- write_unlock_irqrestore(&application_lock, flags);
- mutex_lock(&capi_controller_lock);
+ synchronize_rcu();
for (i = 0; i < CAPI_MAXCONTR; i++) {
if (!capi_controller[i] ||
diff --git a/drivers/isdn/capi/kcapi_proc.c b/drivers/isdn/capi/kcapi_proc.c
index 3e6e17a..ea2dff6 100644
--- a/drivers/isdn/capi/kcapi_proc.c
+++ b/drivers/isdn/capi/kcapi_proc.c
@@ -139,9 +139,11 @@ static const struct file_operations proc_contrstats_ops = {
// applid nrecvctlpkt nrecvdatapkt nsentctlpkt nsentdatapkt
// ---------------------------------------------------------------------------
-static void *
-applications_start(struct seq_file *seq, loff_t *pos)
+static void *applications_start(struct seq_file *seq, loff_t *pos)
+ __acquires(capi_controller_lock)
{
+ mutex_lock(&capi_controller_lock);
+
if (*pos < CAPI_MAXAPPL)
return &capi_applications[*pos];
@@ -158,9 +160,10 @@ applications_next(struct seq_file *seq, void *v, loff_t *pos)
return NULL;
}
-static void
-applications_stop(struct seq_file *seq, void *v)
+static void applications_stop(struct seq_file *seq, void *v)
+ __releases(capi_controller_lock)
{
+ mutex_unlock(&capi_controller_lock);
}
static int
--
1.6.0.2
^ permalink raw reply related
* [PATCH v2 12/41] CAPI: Reduce #ifdef mess around CONFIG_ISDN_CAPI_MIDDLEWARE
From: Jan Kiszka @ 2010-02-08 20:12 UTC (permalink / raw)
To: David Miller, Karsten Keil
Cc: linux-kernel, i4ldeveloper, isdn4linux, netdev, Alan Cox,
Marcel Holtmann
In-Reply-To: <cover.1265659933.git.jan.kiszka@web.de>
Make the code a bit more readable be providing stub functions for the
!CONFIG_ISDN_CAPI_MIDDLEWARE case. Though a few lines are moved around,
this comes with no functional changes.
Signed-off-by: Jan Kiszka <jan.kiszka@web.de>
---
drivers/isdn/capi/capi.c | 150 +++++++++++++++++++++++++--------------------
1 files changed, 83 insertions(+), 67 deletions(-)
diff --git a/drivers/isdn/capi/capi.c b/drivers/isdn/capi/capi.c
index 3b07797..7d2ca6b 100644
--- a/drivers/isdn/capi/capi.c
+++ b/drivers/isdn/capi/capi.c
@@ -23,14 +23,10 @@
#include <linux/smp_lock.h>
#include <linux/timer.h>
#include <linux/wait.h>
-#ifdef CONFIG_ISDN_CAPI_MIDDLEWARE
#include <linux/tty.h>
-#ifdef CONFIG_PPP
#include <linux/netdevice.h>
#include <linux/ppp_defs.h>
#include <linux/if_ppp.h>
-#endif /* CONFIG_PPP */
-#endif /* CONFIG_ISDN_CAPI_MIDDLEWARE */
#include <linux/skbuff.h>
#include <linux/proc_fs.h>
#include <linux/seq_file.h>
@@ -56,17 +52,17 @@ MODULE_LICENSE("GPL");
/* -------- driver information -------------------------------------- */
static struct class *capi_class;
-
static int capi_major = 68; /* allocated */
+
+module_param_named(major, capi_major, uint, 0);
+
#ifdef CONFIG_ISDN_CAPI_MIDDLEWARE
-#define CAPINC_NR_PORTS 32
+#define CAPINC_NR_PORTS 32
#define CAPINC_MAX_PORTS 256
+
static int capi_ttymajor = 191;
static int capi_ttyminors = CAPINC_NR_PORTS;
-#endif /* CONFIG_ISDN_CAPI_MIDDLEWARE */
-module_param_named(major, capi_major, uint, 0);
-#ifdef CONFIG_ISDN_CAPI_MIDDLEWARE
module_param_named(ttymajor, capi_ttymajor, uint, 0);
module_param_named(ttyminors, capi_ttyminors, uint, 0);
#endif /* CONFIG_ISDN_CAPI_MIDDLEWARE */
@@ -81,7 +77,6 @@ module_param_named(ttyminors, capi_ttyminors, uint, 0);
struct capidev;
struct capincci;
-#ifdef CONFIG_ISDN_CAPI_MIDDLEWARE
struct capiminor;
struct datahandle_queue {
@@ -116,7 +111,6 @@ struct capiminor {
int nack;
spinlock_t ackqlock;
};
-#endif /* CONFIG_ISDN_CAPI_MIDDLEWARE */
/* FIXME: The following lock is a sledgehammer-workaround to a
* locking issue with the capiminor (and maybe other) data structure(s).
@@ -156,14 +150,13 @@ static DEFINE_RWLOCK(capidev_list_lock);
static LIST_HEAD(capidev_list);
#ifdef CONFIG_ISDN_CAPI_MIDDLEWARE
+
static DEFINE_RWLOCK(capiminor_list_lock);
static LIST_HEAD(capiminor_list);
-#endif /* CONFIG_ISDN_CAPI_MIDDLEWARE */
-#ifdef CONFIG_ISDN_CAPI_MIDDLEWARE
/* -------- datahandles --------------------------------------------- */
-static int capincci_add_ack(struct capiminor *mp, u16 datahandle)
+static int capiminor_add_ack(struct capiminor *mp, u16 datahandle)
{
struct datahandle_queue *n;
unsigned long flags;
@@ -301,26 +294,17 @@ static struct capiminor *capiminor_find(unsigned int minor)
return p;
}
-#endif /* CONFIG_ISDN_CAPI_MIDDLEWARE */
/* -------- struct capincci ----------------------------------------- */
-static struct capincci *capincci_alloc(struct capidev *cdev, u32 ncci)
+static void capincci_alloc_minor(struct capidev *cdev, struct capincci *np)
{
- struct capincci *np, **pp;
-#ifdef CONFIG_ISDN_CAPI_MIDDLEWARE
- struct capiminor *mp = NULL;
-#endif /* CONFIG_ISDN_CAPI_MIDDLEWARE */
+ struct capiminor *mp;
- np = kzalloc(sizeof(*np), GFP_ATOMIC);
- if (!np)
- return NULL;
- np->ncci = ncci;
- np->cdev = cdev;
-#ifdef CONFIG_ISDN_CAPI_MIDDLEWARE
- mp = NULL;
- if (cdev->userflags & CAPIFLAG_HIGHJACKING)
- mp = np->minorp = capiminor_alloc(&cdev->ap, ncci);
+ if (!(cdev->userflags & CAPIFLAG_HIGHJACKING))
+ return;
+
+ mp = np->minorp = capiminor_alloc(&cdev->ap, np->ncci);
if (mp) {
mp->nccip = np;
#ifdef _DEBUG_REFCOUNT
@@ -330,7 +314,58 @@ static struct capincci *capincci_alloc(struct capidev *cdev, u32 ncci)
capifs_new_ncci(mp->minor,
MKDEV(capi_ttymajor, mp->minor));
}
-#endif /* CONFIG_ISDN_CAPI_MIDDLEWARE */
+}
+
+static void capincci_free_minor(struct capincci *np)
+{
+ struct capiminor *mp = np->minorp;
+
+ if (mp) {
+ capifs_free_ncci(mp->capifs_dentry);
+ if (mp->tty) {
+ mp->nccip = NULL;
+#ifdef _DEBUG_REFCOUNT
+ printk(KERN_DEBUG "reset mp->nccip\n");
+#endif
+ tty_hangup(mp->tty);
+ } else {
+ capiminor_free(mp);
+ }
+ }
+}
+
+static inline unsigned int capincci_minor_opencount(struct capincci *np)
+{
+ struct capiminor *mp = np->minorp;
+
+ return mp ? atomic_read(&mp->ttyopencount) : 0;
+}
+
+#else /* !CONFIG_ISDN_CAPI_MIDDLEWARE */
+
+static inline void
+capincci_alloc_minor(struct capidev *cdev, struct capincci *np) { }
+static inline void capincci_free_minor(struct capincci *np) { }
+
+static inline unsigned int capincci_minor_opencount(struct capincci *np)
+{
+ return 0;
+}
+
+#endif /* !CONFIG_ISDN_CAPI_MIDDLEWARE */
+
+static struct capincci *capincci_alloc(struct capidev *cdev, u32 ncci)
+{
+ struct capincci *np, **pp;
+
+ np = kzalloc(sizeof(*np), GFP_ATOMIC);
+ if (!np)
+ return NULL;
+ np->ncci = ncci;
+ np->cdev = cdev;
+
+ capincci_alloc_minor(cdev, np);
+
for (pp=&cdev->nccis; *pp; pp = &(*pp)->next)
;
*pp = np;
@@ -340,29 +375,13 @@ static struct capincci *capincci_alloc(struct capidev *cdev, u32 ncci)
static void capincci_free(struct capidev *cdev, u32 ncci)
{
struct capincci *np, **pp;
-#ifdef CONFIG_ISDN_CAPI_MIDDLEWARE
- struct capiminor *mp;
-#endif /* CONFIG_ISDN_CAPI_MIDDLEWARE */
pp=&cdev->nccis;
while (*pp) {
np = *pp;
if (ncci == 0xffffffff || np->ncci == ncci) {
*pp = (*pp)->next;
-#ifdef CONFIG_ISDN_CAPI_MIDDLEWARE
- if ((mp = np->minorp) != NULL) {
- capifs_free_ncci(mp->capifs_dentry);
- if (mp->tty) {
- mp->nccip = NULL;
-#ifdef _DEBUG_REFCOUNT
- printk(KERN_DEBUG "reset mp->nccip\n");
-#endif
- tty_hangup(mp->tty);
- } else {
- capiminor_free(mp);
- }
- }
-#endif /* CONFIG_ISDN_CAPI_MIDDLEWARE */
+ capincci_free_minor(np);
kfree(np);
if (*pp == NULL) return;
} else {
@@ -552,7 +571,7 @@ static int handle_minor_send(struct capiminor *mp)
capimsg_setu16(skb->data, 18, datahandle);
capimsg_setu16(skb->data, 20, 0); /* Flags */
- if (capincci_add_ack(mp, datahandle) < 0) {
+ if (capiminor_add_ack(mp, datahandle) < 0) {
skb_pull(skb, CAPI_DATA_B3_REQ_LEN);
skb_queue_head(&mp->outqueue, skb);
return count;
@@ -628,10 +647,13 @@ static void capi_recv_message(struct capi20_appl *ap, struct sk_buff *skb)
spin_unlock_irqrestore(&workaround_lock, flags);
return;
}
+
#ifndef CONFIG_ISDN_CAPI_MIDDLEWARE
skb_queue_tail(&cdev->recvqueue, skb);
wake_up_interruptible(&cdev->recvwait);
+
#else /* CONFIG_ISDN_CAPI_MIDDLEWARE */
+
mp = np->minorp;
if (!mp) {
skb_queue_tail(&cdev->recvqueue, skb);
@@ -639,10 +661,7 @@ static void capi_recv_message(struct capi20_appl *ap, struct sk_buff *skb)
spin_unlock_irqrestore(&workaround_lock, flags);
return;
}
-
-
if (CAPIMSG_SUBCOMMAND(skb->data) == CAPI_IND) {
-
datahandle = CAPIMSG_U16(skb->data, CAPIMSG_BASELEN+4+4+2);
#ifdef _DEBUG_DATAFLOW
printk(KERN_DEBUG "capi_signal: DATA_B3_IND %u len=%d\n",
@@ -672,6 +691,7 @@ static void capi_recv_message(struct capi20_appl *ap, struct sk_buff *skb)
wake_up_interruptible(&cdev->recvwait);
}
#endif /* CONFIG_ISDN_CAPI_MIDDLEWARE */
+
spin_unlock_irqrestore(&workaround_lock, flags);
}
@@ -929,9 +949,6 @@ capi_ioctl(struct inode *inode, struct file *file,
case CAPI_NCCI_OPENCOUNT:
{
struct capincci *nccip;
-#ifdef CONFIG_ISDN_CAPI_MIDDLEWARE
- struct capiminor *mp;
-#endif /* CONFIG_ISDN_CAPI_MIDDLEWARE */
unsigned ncci;
int count = 0;
if (copy_from_user(&ncci, argp, sizeof(ncci)))
@@ -942,11 +959,7 @@ capi_ioctl(struct inode *inode, struct file *file,
mutex_unlock(&cdev->ncci_list_mtx);
return 0;
}
-#ifdef CONFIG_ISDN_CAPI_MIDDLEWARE
- if ((mp = nccip->minorp) != NULL) {
- count += atomic_read(&mp->ttyopencount);
- }
-#endif /* CONFIG_ISDN_CAPI_MIDDLEWARE */
+ count += capincci_minor_opencount(nccip);
mutex_unlock(&cdev->ncci_list_mtx);
return count;
}
@@ -1396,7 +1409,16 @@ static void capinc_tty_exit(void)
put_tty_driver(drv);
}
-#endif /* CONFIG_ISDN_CAPI_MIDDLEWARE */
+#else /* !CONFIG_ISDN_CAPI_MIDDLEWARE */
+
+static inline int capinc_tty_init(void)
+{
+ return 0;
+}
+
+static inline void capinc_tty_exit(void) { }
+
+#endif /* !CONFIG_ISDN_CAPI_MIDDLEWARE */
/* -------- /proc functions ----------------------------------------- */
@@ -1505,23 +1527,19 @@ static int __init capi_init(void)
device_create(capi_class, NULL, MKDEV(capi_major, 0), NULL, "capi");
-#ifdef CONFIG_ISDN_CAPI_MIDDLEWARE
if (capinc_tty_init() < 0) {
device_destroy(capi_class, MKDEV(capi_major, 0));
class_destroy(capi_class);
unregister_chrdev(capi_major, "capi20");
return -ENOMEM;
}
-#endif /* CONFIG_ISDN_CAPI_MIDDLEWARE */
proc_init();
-#ifdef CONFIG_ISDN_CAPI_MIDDLEWARE
#if defined(CONFIG_ISDN_CAPI_CAPIFS) || defined(CONFIG_ISDN_CAPI_CAPIFS_MODULE)
compileinfo = " (middleware+capifs)";
-#else
+#elif defined(CONFIG_ISDN_CAPI_MIDDLEWARE)
compileinfo = " (no capifs)";
-#endif
#else
compileinfo = " (no middleware)";
#endif
@@ -1539,9 +1557,7 @@ static void __exit capi_exit(void)
class_destroy(capi_class);
unregister_chrdev(capi_major, "capi20");
-#ifdef CONFIG_ISDN_CAPI_MIDDLEWARE
capinc_tty_exit();
-#endif
}
module_init(capi_init);
--
1.6.0.2
^ permalink raw reply related
* [PATCH v2 15/41] CAPI: Rework locking of capidev members
From: Jan Kiszka @ 2010-02-08 20:12 UTC (permalink / raw)
To: David Miller, Karsten Keil
Cc: linux-kernel, i4ldeveloper, isdn4linux, netdev, Alan Cox,
Marcel Holtmann
In-Reply-To: <cover.1265659933.git.jan.kiszka@web.de>
Rename 'ncci_list_mtx' to 'lock', expressing that it now protects a
larger set of capidev members: the NCCI list, ap.applid (ie. the
registration of the application), and modifications of userflags.
We do not need to protect each and every check for ap.applid because,
once an application is registered, it will stay for the whole lifetime
of the device.
Also, there is no need to apply the capidev mutex during release (if
there could be concurrent users, we would crash them anyway by freeing
the device at the end of capi_release).
Signed-off-by: Jan Kiszka <jan.kiszka@web.de>
---
drivers/isdn/capi/capi.c | 181 ++++++++++++++++++++++------------------------
1 files changed, 88 insertions(+), 93 deletions(-)
diff --git a/drivers/isdn/capi/capi.c b/drivers/isdn/capi/capi.c
index 9d7c369..403bf8f 100644
--- a/drivers/isdn/capi/capi.c
+++ b/drivers/isdn/capi/capi.c
@@ -141,7 +141,7 @@ struct capidev {
struct capincci *nccis;
- struct mutex ncci_list_mtx;
+ struct mutex lock;
};
/* -------- global variables ---------------------------------------- */
@@ -574,38 +574,31 @@ static void capi_recv_message(struct capi20_appl *ap, struct sk_buff *skb)
u16 datahandle;
#endif /* CONFIG_ISDN_CAPI_MIDDLEWARE */
struct capincci *np;
- u32 ncci;
unsigned long flags;
+ mutex_lock(&cdev->lock);
+
if (CAPIMSG_CMD(skb->data) == CAPI_CONNECT_B3_CONF) {
u16 info = CAPIMSG_U16(skb->data, 12); // Info field
- if ((info & 0xff00) == 0) {
- mutex_lock(&cdev->ncci_list_mtx);
+ if ((info & 0xff00) == 0)
capincci_alloc(cdev, CAPIMSG_NCCI(skb->data));
- mutex_unlock(&cdev->ncci_list_mtx);
- }
}
- if (CAPIMSG_CMD(skb->data) == CAPI_CONNECT_B3_IND) {
- mutex_lock(&cdev->ncci_list_mtx);
+ if (CAPIMSG_CMD(skb->data) == CAPI_CONNECT_B3_IND)
capincci_alloc(cdev, CAPIMSG_NCCI(skb->data));
- mutex_unlock(&cdev->ncci_list_mtx);
- }
+
spin_lock_irqsave(&workaround_lock, flags);
if (CAPIMSG_COMMAND(skb->data) != CAPI_DATA_B3) {
skb_queue_tail(&cdev->recvqueue, skb);
wake_up_interruptible(&cdev->recvwait);
- spin_unlock_irqrestore(&workaround_lock, flags);
- return;
+ goto unlock_out;
}
- ncci = CAPIMSG_CONTROL(skb->data);
- for (np = cdev->nccis; np && np->ncci != ncci; np = np->next)
- ;
+
+ np = capincci_find(cdev, CAPIMSG_CONTROL(skb->data));
if (!np) {
printk(KERN_ERR "BUG: capi_signal: ncci not found\n");
skb_queue_tail(&cdev->recvqueue, skb);
wake_up_interruptible(&cdev->recvwait);
- spin_unlock_irqrestore(&workaround_lock, flags);
- return;
+ goto unlock_out;
}
#ifndef CONFIG_ISDN_CAPI_MIDDLEWARE
@@ -618,8 +611,7 @@ static void capi_recv_message(struct capi20_appl *ap, struct sk_buff *skb)
if (!mp) {
skb_queue_tail(&cdev->recvqueue, skb);
wake_up_interruptible(&cdev->recvwait);
- spin_unlock_irqrestore(&workaround_lock, flags);
- return;
+ goto unlock_out;
}
if (CAPIMSG_SUBCOMMAND(skb->data) == CAPI_IND) {
datahandle = CAPIMSG_U16(skb->data, CAPIMSG_BASELEN+4+4+2);
@@ -652,7 +644,9 @@ static void capi_recv_message(struct capi20_appl *ap, struct sk_buff *skb)
}
#endif /* CONFIG_ISDN_CAPI_MIDDLEWARE */
+unlock_out:
spin_unlock_irqrestore(&workaround_lock, flags);
+ mutex_unlock(&cdev->lock);
}
/* -------- file_operations for capidev ----------------------------- */
@@ -730,9 +724,9 @@ capi_write(struct file *file, const char __user *buf, size_t count, loff_t *ppos
CAPIMSG_SETAPPID(skb->data, cdev->ap.applid);
if (CAPIMSG_CMD(skb->data) == CAPI_DISCONNECT_B3_RESP) {
- mutex_lock(&cdev->ncci_list_mtx);
+ mutex_lock(&cdev->lock);
capincci_free(cdev, CAPIMSG_NCCI(skb->data));
- mutex_unlock(&cdev->ncci_list_mtx);
+ mutex_unlock(&cdev->lock);
}
cdev->errcode = capi20_put_message(&cdev->ap, skb);
@@ -765,30 +759,35 @@ capi_ioctl(struct inode *inode, struct file *file,
unsigned int cmd, unsigned long arg)
{
struct capidev *cdev = file->private_data;
- struct capi20_appl *ap = &cdev->ap;
capi_ioctl_struct data;
int retval = -EINVAL;
void __user *argp = (void __user *)arg;
switch (cmd) {
case CAPI_REGISTER:
- {
- if (ap->applid)
- return -EEXIST;
+ mutex_lock(&cdev->lock);
- if (copy_from_user(&cdev->ap.rparam, argp,
- sizeof(struct capi_register_params)))
- return -EFAULT;
-
- cdev->ap.private = cdev;
- cdev->ap.recv_message = capi_recv_message;
- cdev->errcode = capi20_register(ap);
- if (cdev->errcode) {
- ap->applid = 0;
- return -EIO;
- }
+ if (cdev->ap.applid) {
+ retval = -EEXIST;
+ goto register_out;
+ }
+ if (copy_from_user(&cdev->ap.rparam, argp,
+ sizeof(struct capi_register_params))) {
+ retval = -EFAULT;
+ goto register_out;
+ }
+ cdev->ap.private = cdev;
+ cdev->ap.recv_message = capi_recv_message;
+ cdev->errcode = capi20_register(&cdev->ap);
+ retval = (int)cdev->ap.applid;
+ if (cdev->errcode) {
+ cdev->ap.applid = 0;
+ retval = -EIO;
}
- return (int)ap->applid;
+
+register_out:
+ mutex_unlock(&cdev->lock);
+ return retval;
case CAPI_GET_VERSION:
{
@@ -887,68 +886,67 @@ capi_ioctl(struct inode *inode, struct file *file,
return 0;
case CAPI_SET_FLAGS:
- case CAPI_CLR_FLAGS:
- {
- unsigned userflags;
- if (copy_from_user(&userflags, argp,
- sizeof(userflags)))
- return -EFAULT;
- if (cmd == CAPI_SET_FLAGS)
- cdev->userflags |= userflags;
- else
- cdev->userflags &= ~userflags;
- }
- return 0;
+ case CAPI_CLR_FLAGS: {
+ unsigned userflags;
+
+ if (copy_from_user(&userflags, argp, sizeof(userflags)))
+ return -EFAULT;
+ mutex_lock(&cdev->lock);
+ if (cmd == CAPI_SET_FLAGS)
+ cdev->userflags |= userflags;
+ else
+ cdev->userflags &= ~userflags;
+ mutex_unlock(&cdev->lock);
+ return 0;
+ }
case CAPI_GET_FLAGS:
if (copy_to_user(argp, &cdev->userflags,
sizeof(cdev->userflags)))
return -EFAULT;
return 0;
- case CAPI_NCCI_OPENCOUNT:
- {
- struct capincci *nccip;
- unsigned ncci;
- int count = 0;
- if (copy_from_user(&ncci, argp, sizeof(ncci)))
- return -EFAULT;
+ case CAPI_NCCI_OPENCOUNT: {
+ struct capincci *nccip;
+ unsigned ncci;
+ int count = 0;
- mutex_lock(&cdev->ncci_list_mtx);
- if ((nccip = capincci_find(cdev, (u32) ncci)) == NULL) {
- mutex_unlock(&cdev->ncci_list_mtx);
- return 0;
- }
- count += capincci_minor_opencount(nccip);
- mutex_unlock(&cdev->ncci_list_mtx);
- return count;
- }
- return 0;
+ if (copy_from_user(&ncci, argp, sizeof(ncci)))
+ return -EFAULT;
+
+ mutex_lock(&cdev->lock);
+ nccip = capincci_find(cdev, (u32)ncci);
+ if (nccip)
+ count = capincci_minor_opencount(nccip);
+ mutex_unlock(&cdev->lock);
+ return count;
+ }
#ifdef CONFIG_ISDN_CAPI_MIDDLEWARE
- case CAPI_NCCI_GETUNIT:
- {
- struct capincci *nccip;
- struct capiminor *mp;
- unsigned ncci;
- int unit = 0;
- if (copy_from_user(&ncci, argp,
- sizeof(ncci)))
- return -EFAULT;
- mutex_lock(&cdev->ncci_list_mtx);
- nccip = capincci_find(cdev, (u32) ncci);
- if (!nccip || (mp = nccip->minorp) == NULL) {
- mutex_unlock(&cdev->ncci_list_mtx);
- return -ESRCH;
- }
- unit = mp->minor;
- mutex_unlock(&cdev->ncci_list_mtx);
- return unit;
+ case CAPI_NCCI_GETUNIT: {
+ struct capincci *nccip;
+ struct capiminor *mp;
+ unsigned ncci;
+ int unit = -ESRCH;
+
+ if (copy_from_user(&ncci, argp, sizeof(ncci)))
+ return -EFAULT;
+
+ mutex_lock(&cdev->lock);
+ nccip = capincci_find(cdev, (u32)ncci);
+ if (nccip) {
+ mp = nccip->minorp;
+ if (mp)
+ unit = mp->minor;
}
- return 0;
+ mutex_unlock(&cdev->lock);
+ return unit;
+ }
#endif /* CONFIG_ISDN_CAPI_MIDDLEWARE */
+
+ default:
+ return -EINVAL;
}
- return -EINVAL;
}
static int capi_open(struct inode *inode, struct file *file)
@@ -959,7 +957,7 @@ static int capi_open(struct inode *inode, struct file *file)
if (!cdev)
return -ENOMEM;
- mutex_init(&cdev->ncci_list_mtx);
+ mutex_init(&cdev->lock);
skb_queue_head_init(&cdev->recvqueue);
init_waitqueue_head(&cdev->recvwait);
file->private_data = cdev;
@@ -979,15 +977,10 @@ static int capi_release(struct inode *inode, struct file *file)
list_del(&cdev->list);
mutex_unlock(&capidev_list_lock);
- if (cdev->ap.applid) {
+ if (cdev->ap.applid)
capi20_release(&cdev->ap);
- cdev->ap.applid = 0;
- }
skb_queue_purge(&cdev->recvqueue);
-
- mutex_lock(&cdev->ncci_list_mtx);
capincci_free(cdev, 0xffffffff);
- mutex_unlock(&cdev->ncci_list_mtx);
kfree(cdev);
return 0;
@@ -1446,11 +1439,13 @@ static int capi20ncci_proc_show(struct seq_file *m, void *v)
mutex_lock(&capidev_list_lock);
list_for_each(l, &capidev_list) {
cdev = list_entry(l, struct capidev, list);
+ mutex_lock(&cdev->lock);
for (np=cdev->nccis; np; np = np->next) {
seq_printf(m, "%d 0x%x\n",
cdev->ap.applid,
np->ncci);
}
+ mutex_unlock(&cdev->lock);
}
mutex_unlock(&capidev_list_lock);
return 0;
--
1.6.0.2
^ 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