| Message ID | 4928CECE.602@cosmosbay.com |
|---|---|
| State | Rejected, archived |
| Delegated to: | David Miller |
| Headers | show |
On Sun, 23 Nov 2008, Eric Dumazet wrote: > Ilpo Järvinen a écrit : > > I got plenty of these from sock_prot_inuse_add: > > > > BUG: using smp_processor_id() in preemptible [00000000] code: rcS/1146 > > > > caller is sock_prot_inuse_add+0x24/0x42 > > Pid: 1146, comm: rcS Not tainted 2.6.28-rc6-01121-g89a2f15 #76 > > Call Trace: > > [<ffffffff80365767>] debug_smp_processor_id+0xd3/0xe8 > > [<ffffffff80485a0f>] sock_prot_inuse_add+0x24/0x42 > > [<ffffffff80514cdd>] unix_create1+0x161/0x176 > > [<ffffffff80514d52>] unix_create+0x60/0x6b > > [<ffffffff80483f66>] __sock_create+0x144/0x1bd > > [<ffffffff80483edb>] ? __sock_create+0xb9/0x1bd > > [<ffffffff8048402d>] sock_create+0x2d/0x2f > > [<ffffffff80484274>] sys_socket+0x29/0x5b > > [<ffffffff8020c10a>] system_call_fastpath+0x16/0x1b > > > > > > Thanks Ilpo for this report > > I guess the following is necessary. > > Once Christopher and Rusty work on percpu variables is finished, > the preempt_enable()/disable() wont be necessary anymore, so > its a temporary workaround anyway. > > [PATCH] net: make sock_prot_inuse_add() preempt safe > > Ilpo Järvinen reported that commit a8076d8db98de6da61394b2e942320e4612643ac > (net: af_unix should update its inuse counter) was triggering > a warning in smp_processor_id(), being called in a preemptible code. > > Fix is to make sock_prot_inuse_add() safe in this regard. This fix > can be reverted when new percpu infrastructure is ready, allowing > a cpu to safely do a increment/decrement on a percpu var. > > Signed-off-by: Eric Dumazet <dada1@cosmosbay.com> Works. Tested-by: Ilpo Järvinen <ilpo.jarvinen@helsinki.fi>
From: Eric Dumazet <dada1@cosmosbay.com> Date: Sun, 23 Nov 2008 04:32:30 +0100 > [PATCH] net: make sock_prot_inuse_add() preempt safe > > Ilpo Järvinen reported that commit a8076d8db98de6da61394b2e942320e4612643ac > (net: af_unix should update its inuse counter) was triggering > a warning in smp_processor_id(), being called in a preemptible code. > > Fix is to make sock_prot_inuse_add() safe in this regard. This fix > can be reverted when new percpu infrastructure is ready, allowing > a cpu to safely do a increment/decrement on a percpu var. > > Signed-off-by: Eric Dumazet <dada1@cosmosbay.com> Eric, you added this bug by starting to use this interface in situations where BH's were not disabled. Ever existing use adhered to that rule. If you therefore want to call this interface in new locations, you have to make sure those locations follow the rule too. Making it expensive for all the existing cases which were already safe, is not the way to fix this. And saying some future not-merged change justifies this added cost is besides the point. What is valid is getting rid of the BH disables you need to add to net/unix/af_unix.c et al.'s call sites, once those percpu patches are added. Not the other way around. -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/net/core/sock.c b/net/core/sock.c index a4e840e..d4f5ad7 100644 --- a/net/core/sock.c +++ b/net/core/sock.c @@ -1942,8 +1942,8 @@ static DECLARE_BITMAP(proto_inuse_idx, PROTO_INUSE_NR); #ifdef CONFIG_NET_NS void sock_prot_inuse_add(struct net *net, struct proto *prot, int val) { - int cpu = smp_processor_id(); - per_cpu_ptr(net->core.inuse, cpu)->val[prot->inuse_idx] += val; + per_cpu_ptr(net->core.inuse, get_cpu())->val[prot->inuse_idx] += val; + put_cpu(); } EXPORT_SYMBOL_GPL(sock_prot_inuse_add); @@ -1989,7 +1989,9 @@ static DEFINE_PER_CPU(struct prot_inuse, prot_inuse); void sock_prot_inuse_add(struct net *net, struct proto *prot, int val) { + preempt_disable(); __get_cpu_var(prot_inuse).val[prot->inuse_idx] += val; + preempt_enable(); } EXPORT_SYMBOL_GPL(sock_prot_inuse_add);