On Mon, 2005-04-04 at 19:40 +0200, Ingo Molnar wrote:
> * Peter Zijlstra <[email protected]> wrote:
>
> > Hi Ingo,
> >
> > I need the following two patches to keep my system alive and avoid
> > the BUGs in the log send to you earlier (private mail).
>
> hm, the second patch does not apply (and the merge didnt look trivial) -
> maybe it depends on some patch in -mm that is not yet upstream?
>
I messed it up; apparently I type :wa to often and forgot to keep a
decent base file (yes I'm a vim user). I also seem to have gotten the
patch base dir wrong.
The two patches again; now hopefully with correct patch level and proper
base. They apply to my tree in order 2.6.12-rc1-icmp{,-rt}.patch.
Not ran nor compile tested on vanilla; just made then apply cleanly.
Could you please comment on the patches since I'm learning this stuff as
I go; and validation of my ideas would be most welcome. For instance;
was I right to convert the per cpu var __icmp_socket to a _LOCKED?
I think so since without the extra lock one could have race conditions
within the spin_trylock arguments double dereference.
Kind regards,
Peter Zijlstra
Since there is no lock held yet we are still preemptable; and since
icmp_socket is a per cpu variable switching cpus gives weird results.
Signed-off-by: Peter Zijlstra <[email protected]>
--- linux-2.6.12-rc1-RT-V0.7.42-08/net/ipv4/icmp_base.c 2005-04-04 20:26:23.000000000 +0200
+++ linux-2.6.12-rc1-RT-V0.7.42-08/net/ipv4/icmp.c 2005-04-04 20:26:38.000000000 +0200
@@ -376,8 +376,8 @@
static void icmp_reply(struct icmp_bxm *icmp_param, struct sk_buff *skb)
{
- struct sock *sk = icmp_socket->sk;
- struct inet_sock *inet = inet_sk(sk);
+ struct sock *sk;
+ struct inet_sock *inet;
struct ipcm_cookie ipc;
struct rtable *rt = (struct rtable *)skb->dst;
u32 daddr;
@@ -388,6 +388,9 @@
if (icmp_xmit_lock())
return;
+ sk = icmp_socket->sk;
+ inet = inet_sk(sk);
+
icmp_param->data.icmph.checksum = 0;
icmp_out_count(icmp_param->data.icmph.type);
Since on PREEMPT_RT spinlocks allow preemption, it's possible to
change cpu at (almost) any point in time. Make sure we stick to
the per cpu variable we started with.
Signed-off-by: Peter Zijlstra <[email protected]>
--- linux-2.6.12-rc1-RT-V0.7.42-08/net/ipv4/icmp_p1.c 2005-04-04 20:26:38.000000000 +0200
+++ linux-2.6.12-rc1-RT-V0.7.42-08/net/ipv4/icmp.c 2005-04-04 20:27:54.000000000 +0200
@@ -228,26 +228,32 @@
*
* On SMP we have one ICMP socket per-cpu.
*/
-static DEFINE_PER_CPU(struct socket *, __icmp_socket) = NULL;
-#define icmp_socket __get_cpu_var(__icmp_socket)
+static DEFINE_PER_CPU_LOCKED(struct socket *, __icmp_socket) = NULL;
-static __inline__ int icmp_xmit_lock(void)
+static __inline__ struct socket * icmp_xmit_lock(int cpu)
{
+ struct socket *icmp_socket;
+
local_bh_disable();
+ icmp_socket = get_cpu_var_locked(__icmp_socket, cpu);
if (unlikely(!spin_trylock(&icmp_socket->sk->sk_lock.slock))) {
/* This can happen if the output path signals a
* dst_link_failure() for an outgoing ICMP packet.
*/
+ put_cpu_var_locked(__icmp_socket, cpu);
local_bh_enable();
- return 1;
+ return NULL;
}
- return 0;
+
+ return icmp_socket;
}
-static void icmp_xmit_unlock(void)
+static void icmp_xmit_unlock(int cpu)
{
+ struct socket *icmp_socket = __get_cpu_var_locked(__icmp_socket, cpu);
spin_unlock_bh(&icmp_socket->sk->sk_lock.slock);
+ put_cpu_var_locked(__icmp_socket, cpu);
}
/*
@@ -344,7 +350,8 @@
}
static void icmp_push_reply(struct icmp_bxm *icmp_param,
- struct ipcm_cookie *ipc, struct rtable *rt)
+ struct ipcm_cookie *ipc, struct rtable *rt,
+ struct socket *icmp_socket)
{
struct sk_buff *skb;
@@ -376,16 +383,19 @@
static void icmp_reply(struct icmp_bxm *icmp_param, struct sk_buff *skb)
{
+ struct socket *icmp_socket;
struct sock *sk;
struct inet_sock *inet;
struct ipcm_cookie ipc;
struct rtable *rt = (struct rtable *)skb->dst;
u32 daddr;
+ int cpu;
if (ip_options_echo(&icmp_param->replyopts, skb))
goto out;
- if (icmp_xmit_lock())
+ cpu = _smp_processor_id();
+ if (!(icmp_socket = icmp_xmit_lock(cpu)))
return;
sk = icmp_socket->sk;
@@ -413,10 +423,10 @@
}
if (icmpv4_xrlim_allow(rt, icmp_param->data.icmph.type,
icmp_param->data.icmph.code))
- icmp_push_reply(icmp_param, &ipc, rt);
+ icmp_push_reply(icmp_param, &ipc, rt, icmp_socket);
ip_rt_put(rt);
out_unlock:
- icmp_xmit_unlock();
+ icmp_xmit_unlock(cpu);
out:;
}
@@ -434,8 +444,9 @@
void icmp_send(struct sk_buff *skb_in, int type, int code, u32 info)
{
+ struct socket *icmp_socket;
struct iphdr *iph;
- int room;
+ int room, cpu;
struct icmp_bxm icmp_param;
struct rtable *rt = (struct rtable *)skb_in->dst;
struct ipcm_cookie ipc;
@@ -506,7 +517,8 @@
}
}
- if (icmp_xmit_lock())
+ cpu = _smp_processor_id();
+ if (!(icmp_socket = icmp_xmit_lock(cpu)))
return;
/*
@@ -579,11 +591,11 @@
icmp_param.data_len = room;
icmp_param.head_len = sizeof(struct icmphdr);
- icmp_push_reply(&icmp_param, &ipc, rt);
+ icmp_push_reply(&icmp_param, &ipc, rt, icmp_socket);
ende:
ip_rt_put(rt);
out_unlock:
- icmp_xmit_unlock();
+ icmp_xmit_unlock(cpu);
out:;
}
@@ -1115,20 +1127,20 @@
continue;
err = sock_create_kern(PF_INET, SOCK_RAW, IPPROTO_ICMP,
- &per_cpu(__icmp_socket, i));
+ &__get_cpu_var_locked(__icmp_socket, i));
if (err < 0)
panic("Failed to create the ICMP control socket.\n");
- per_cpu(__icmp_socket, i)->sk->sk_allocation = GFP_ATOMIC;
+ __get_cpu_var_locked(__icmp_socket, i)->sk->sk_allocation = GFP_ATOMIC;
/* Enough space for 2 64K ICMP packets, including
* sk_buff struct overhead.
*/
- per_cpu(__icmp_socket, i)->sk->sk_sndbuf =
+ __get_cpu_var_locked(__icmp_socket, i)->sk->sk_sndbuf =
(2 * ((64 * 1024) + sizeof(struct sk_buff)));
- inet = inet_sk(per_cpu(__icmp_socket, i)->sk);
+ inet = inet_sk(__get_cpu_var_locked(__icmp_socket, i)->sk);
inet->uc_ttl = -1;
inet->pmtudisc = IP_PMTUDISC_DONT;
@@ -1136,7 +1148,7 @@
* see it, we do not wish this socket to see incoming
* packets.
*/
- per_cpu(__icmp_socket, i)->sk->sk_prot->unhash(per_cpu(__icmp_socket, i)->sk);
+ __get_cpu_var_locked(__icmp_socket, i)->sk->sk_prot->unhash(__get_cpu_var_locked(__icmp_socket, i)->sk);
}
}
[Index of Archives]
[Kernel Newbies]
[Netfilter]
[Bugtraq]
[Photo]
[Stuff]
[Gimp]
[Yosemite News]
[MIPS Linux]
[ARM Linux]
[Linux Security]
[Linux RAID]
[Video 4 Linux]
[Linux for the blind]
[Linux Resources]
