diff options
author | Jiri Olsa <jolsa@redhat.com> | 2009-07-08 08:09:13 -0400 |
---|---|---|
committer | David S. Miller <davem@davemloft.net> | 2009-07-09 20:06:57 -0400 |
commit | a57de0b4336e48db2811a2030bb68dba8dd09d88 (patch) | |
tree | a01c189d5fd55c69c9e2e842241e84b46728bc60 /include/net | |
parent | 1b614fb9a00e97b1eab54d4e442d405229c059dd (diff) |
net: adding memory barrier to the poll and receive callbacks
Adding memory barrier after the poll_wait function, paired with
receive callbacks. Adding fuctions sock_poll_wait and sk_has_sleeper
to wrap the memory barrier.
Without the memory barrier, following race can happen.
The race fires, when following code paths meet, and the tp->rcv_nxt
and __add_wait_queue updates stay in CPU caches.
CPU1 CPU2
sys_select receive packet
... ...
__add_wait_queue update tp->rcv_nxt
... ...
tp->rcv_nxt check sock_def_readable
... {
schedule ...
if (sk->sk_sleep && waitqueue_active(sk->sk_sleep))
wake_up_interruptible(sk->sk_sleep)
...
}
If there was no cache the code would work ok, since the wait_queue and
rcv_nxt are opposit to each other.
Meaning that once tp->rcv_nxt is updated by CPU2, the CPU1 either already
passed the tp->rcv_nxt check and sleeps, or will get the new value for
tp->rcv_nxt and will return with new data mask.
In both cases the process (CPU1) is being added to the wait queue, so the
waitqueue_active (CPU2) call cannot miss and will wake up CPU1.
The bad case is when the __add_wait_queue changes done by CPU1 stay in its
cache, and so does the tp->rcv_nxt update on CPU2 side. The CPU1 will then
endup calling schedule and sleep forever if there are no more data on the
socket.
Calls to poll_wait in following modules were ommited:
net/bluetooth/af_bluetooth.c
net/irda/af_irda.c
net/irda/irnet/irnet_ppp.c
net/mac80211/rc80211_pid_debugfs.c
net/phonet/socket.c
net/rds/af_rds.c
net/rfkill/core.c
net/sunrpc/cache.c
net/sunrpc/rpc_pipe.c
net/tipc/socket.c
Signed-off-by: Jiri Olsa <jolsa@redhat.com>
Signed-off-by: Eric Dumazet <eric.dumazet@gmail.com>
Signed-off-by: David S. Miller <davem@davemloft.net>
Diffstat (limited to 'include/net')
-rw-r--r-- | include/net/sock.h | 66 |
1 files changed, 66 insertions, 0 deletions
diff --git a/include/net/sock.h b/include/net/sock.h index 352f06bbd7a9..4eb8409249f6 100644 --- a/include/net/sock.h +++ b/include/net/sock.h | |||
@@ -54,6 +54,7 @@ | |||
54 | 54 | ||
55 | #include <linux/filter.h> | 55 | #include <linux/filter.h> |
56 | #include <linux/rculist_nulls.h> | 56 | #include <linux/rculist_nulls.h> |
57 | #include <linux/poll.h> | ||
57 | 58 | ||
58 | #include <asm/atomic.h> | 59 | #include <asm/atomic.h> |
59 | #include <net/dst.h> | 60 | #include <net/dst.h> |
@@ -1241,6 +1242,71 @@ static inline int sk_has_allocations(const struct sock *sk) | |||
1241 | return sk_wmem_alloc_get(sk) || sk_rmem_alloc_get(sk); | 1242 | return sk_wmem_alloc_get(sk) || sk_rmem_alloc_get(sk); |
1242 | } | 1243 | } |
1243 | 1244 | ||
1245 | /** | ||
1246 | * sk_has_sleeper - check if there are any waiting processes | ||
1247 | * @sk: socket | ||
1248 | * | ||
1249 | * Returns true if socket has waiting processes | ||
1250 | * | ||
1251 | * The purpose of the sk_has_sleeper and sock_poll_wait is to wrap the memory | ||
1252 | * barrier call. They were added due to the race found within the tcp code. | ||
1253 | * | ||
1254 | * Consider following tcp code paths: | ||
1255 | * | ||
1256 | * CPU1 CPU2 | ||
1257 | * | ||
1258 | * sys_select receive packet | ||
1259 | * ... ... | ||
1260 | * __add_wait_queue update tp->rcv_nxt | ||
1261 | * ... ... | ||
1262 | * tp->rcv_nxt check sock_def_readable | ||
1263 | * ... { | ||
1264 | * schedule ... | ||
1265 | * if (sk->sk_sleep && waitqueue_active(sk->sk_sleep)) | ||
1266 | * wake_up_interruptible(sk->sk_sleep) | ||
1267 | * ... | ||
1268 | * } | ||
1269 | * | ||
1270 | * The race for tcp fires when the __add_wait_queue changes done by CPU1 stay | ||
1271 | * in its cache, and so does the tp->rcv_nxt update on CPU2 side. The CPU1 | ||
1272 | * could then endup calling schedule and sleep forever if there are no more | ||
1273 | * data on the socket. | ||
1274 | */ | ||
1275 | static inline int sk_has_sleeper(struct sock *sk) | ||
1276 | { | ||
1277 | /* | ||
1278 | * We need to be sure we are in sync with the | ||
1279 | * add_wait_queue modifications to the wait queue. | ||
1280 | * | ||
1281 | * This memory barrier is paired in the sock_poll_wait. | ||
1282 | */ | ||
1283 | smp_mb(); | ||
1284 | return sk->sk_sleep && waitqueue_active(sk->sk_sleep); | ||
1285 | } | ||
1286 | |||
1287 | /** | ||
1288 | * sock_poll_wait - place memory barrier behind the poll_wait call. | ||
1289 | * @filp: file | ||
1290 | * @wait_address: socket wait queue | ||
1291 | * @p: poll_table | ||
1292 | * | ||
1293 | * See the comments in the sk_has_sleeper function. | ||
1294 | */ | ||
1295 | static inline void sock_poll_wait(struct file *filp, | ||
1296 | wait_queue_head_t *wait_address, poll_table *p) | ||
1297 | { | ||
1298 | if (p && wait_address) { | ||
1299 | poll_wait(filp, wait_address, p); | ||
1300 | /* | ||
1301 | * We need to be sure we are in sync with the | ||
1302 | * socket flags modification. | ||
1303 | * | ||
1304 | * This memory barrier is paired in the sk_has_sleeper. | ||
1305 | */ | ||
1306 | smp_mb(); | ||
1307 | } | ||
1308 | } | ||
1309 | |||
1244 | /* | 1310 | /* |
1245 | * Queue a received datagram if it will fit. Stream and sequenced | 1311 | * Queue a received datagram if it will fit. Stream and sequenced |
1246 | * protocols can't normally use this as they need to fit buffers in | 1312 | * protocols can't normally use this as they need to fit buffers in |