From: Eric Wong <hidden> Date: 2012-12-28 01:45:05
I'm finding ppoll() unexpectedly stuck when waiting for POLLIN on a
local TCP socket. The isolated code below can reproduces the issue
after many minutes (<1 hour). It might be easier to reproduce on
a busy system while disk I/O is happening.
This may also be related to an epoll-related issue reported
by Andreas Voellmy:
http://thread.gmane.org/gmane.linux.kernel/1408782/
My example involves a 3 thread data flow between two pairs
of (4) sockets:
send_loop -> recv_loop(recv_send) -> recv_loop(recv_only)
pair_a[1] -> (pair_a[0] -> pair_b[1]) -> pair_b[0]
At least 3.7 and 3.7.1 are affected.
I have tcp_low_latency=1 set, I will try 0 later
The last progress message I got was after receiving 2942052597760
bytes on fd=7 (out of 64-bit ULONG_MAX / 2)
strace:
3644 sendto(4, "\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0"..., 16384, 0, NULL, 0 <unfinished ...>
3643 sendto(6, "\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0"..., 16384, 0, NULL, 0 <unfinished ...>
3642 ppoll([{fd=7, events=POLLIN}], 1, NULL, NULL, 8 <unfinished ...>
3641 futex(0x7f23ed8129d0, FUTEX_WAIT, 3644, NULL <unfinished ...>
The first and last lines of the strace are expected:
+ 3644 sendto(4) is blocked because 3643 is blocked on sendto(fd=6)
and not able to call recv().
+ 3641 is the main thread calling pthread_join
What is unexpected is the tid=3643 and tid=3642 interaction. As confirmed
by lsof below, fd=6 is sending to wake up fd=7, but ppoll(fd=7) seems
to not be waking up.
lsof:
toosleepy 3641 ew 4u IPv4 12405 0t0 TCP localhost:55904->localhost:33249 (ESTABLISHED)
toosleepy 3641 ew 5u IPv4 12406 0t0 TCP localhost:33249->localhost:55904 (ESTABLISHED)
toosleepy 3641 ew 6u IPv4 12408 0t0 TCP localhost:48777->localhost:33348 (ESTABLISHED)
toosleepy 3641 ew 7u IPv4 12409 0t0 TCP localhost:33348->localhost:48777 (ESTABLISHED)
System info: Linux 3.7.1 x86_64 SMP PREEMPT
AMD Phenom(tm) II X4 945 Processor (4 cores)
Nothing interesting in dmesg, iptables rules are empty.
I have not yet been able to reproduce the issue using UNIX sockets,
only TCP, but you can run:
./toosleepy unix
...to test with UNIX sockets intead of TCP.
The following code is also available via git://bogomips.org/toosleepy
gcc -o toosleepy -O2 -Wall -lpthread toosleepy.c
-------------------------------- 8< ------------------------------------
#define _GNU_SOURCE
#include <poll.h>
#include <sys/ioctl.h>
#include <pthread.h>
#include <sys/types.h>
#include <sys/socket.h>
#include <arpa/inet.h>
#include <netinet/tcp.h>
#include <stdint.h>
#include <stdio.h>
#include <stdlib.h>
#include <errno.h>
#include <string.h>
#include <unistd.h>
#include <fcntl.h>
#include <assert.h>
#include <limits.h>
struct receiver {
int rfd;
int sfd;
};
/* blocking sender */
static void * send_loop(void *fdp)
{
int fd = *(int *)fdp;
char buf[16384];
ssize_t s;
size_t sent = 0;
size_t max = (size_t)ULONG_MAX / 2;
while (sent < max) {
s = send(fd, buf, sizeof(buf), 0);
if (s > 0)
sent += s;
if (s == -1)
assert(errno == EINTR);
}
dprintf(2, "%d done sending: %zu\n", fd, sent);
close(fd);
return NULL;
}
/* non-blocking receiver, using ppoll */
static void * recv_loop(void *p)
{
const struct receiver *rcvr = p;
char buf[16384];
nfds_t nfds = 1;
struct pollfd fds;
int rc;
ssize_t r, s;
size_t received = 0;
size_t sent = 0;
for (;;) {
r = recv(rcvr->rfd, buf, sizeof(buf), 0);
if (r == 0) {
break;
} else if (r == -1) {
assert(errno == EAGAIN);
fds.fd = rcvr->rfd;
fds.events = POLLIN;
errno = 0;
rc = ppoll(&fds, nfds, NULL, NULL);
assert(rc == 1);
} else {
assert(r > 0);
received += r;
if (rcvr->sfd >= 0) {
s = send(rcvr->sfd, buf, sizeof(buf), 0);
if (s > 0)
sent += s;
if (s == -1)
assert(errno == EINTR);
} else {
/* just burn some cycles */
write(-1, buf, sizeof(buf));
}
}
if ((received % (sizeof(buf) * sizeof(buf) * 16) == 0))
dprintf(2, " %d progress: %zu\n",
rcvr->rfd, received);
}
dprintf(2, "%d got: %zu\n", rcvr->rfd, received);
if (rcvr->sfd >= 0) {
dprintf(2, "%d sent: %zu\n", rcvr->sfd, sent);
close(rcvr->sfd);
}
return NULL;
}
static void tcp_socketpair(int sv[2], int accept_flags)
{
struct sockaddr_in addr;
socklen_t addrlen = sizeof(addr);
int l = socket(PF_INET, SOCK_STREAM, 0);
int c = socket(PF_INET, SOCK_STREAM, 0);
int a;
addr.sin_family = AF_INET;
addr.sin_addr.s_addr = INADDR_ANY;
addr.sin_port = 0;
assert(0 == bind(l, (struct sockaddr*)&addr, addrlen));
assert(0 == listen(l, 1024));
assert(0 == getsockname(l, (struct sockaddr *)&addr, &addrlen));
assert(0 == connect(c, (struct sockaddr *)&addr, addrlen));
a = accept4(l, NULL, NULL, accept_flags);
assert(a >= 0);
close(l);
sv[0] = a;
sv[1] = c;
}
int main(int argc, char *argv[])
{
int pair_a[2];
int pair_b[2];
pthread_t s, rs, r;
struct receiver recv_only;
struct receiver recv_send;
if (argc == 2 && strcmp(argv[1], "unix") == 0) {
int val;
assert(0 == socketpair(AF_UNIX, SOCK_STREAM, 0, pair_a));
assert(0 == socketpair(AF_UNIX, SOCK_STREAM, 0, pair_b));
/* only make the receiver non-blocking */
val = 1;
assert(0 == ioctl(pair_a[0], FIONBIO, &val));
val = 1;
assert(0 == ioctl(pair_b[0], FIONBIO, &val));
} else {
tcp_socketpair(pair_a, SOCK_NONBLOCK);
tcp_socketpair(pair_b, SOCK_NONBLOCK);
}
recv_send.rfd = pair_a[0];
recv_send.sfd = pair_b[1];
recv_only.rfd = pair_b[0];
recv_only.sfd = -1;
/*
* data flow:
* send_loop -> recv_loop(recv_send) -> recv_loop(recv_only)
* pair_a[1] -> (pair_a[0] -> pair_b[1]) -> pair_b[0]
*/
assert(0 == pthread_create(&r, NULL, recv_loop, &recv_only));
assert(0 == pthread_create(&rs, NULL, recv_loop, &recv_send));
assert(0 == pthread_create(&s, NULL, send_loop, &pair_a[1]));
assert(0 == pthread_join(s, NULL));
assert(0 == pthread_join(rs, NULL));
assert(0 == pthread_join(r, NULL));
return 0;
}
-------------------------------- 8< ------------------------------------
Any help/suggestions/test patches would be greatly appreciated.
Thanks for reading!
--
Eric Wong
From: Eric Wong <hidden> Date: 2012-12-28 07:06:52
Eric Wong [off-list ref] wrote:
I'm finding ppoll() unexpectedly stuck when waiting for POLLIN on a
local TCP socket. The isolated code below can reproduces the issue
after many minutes (<1 hour). It might be easier to reproduce on
a busy system while disk I/O is happening.
Ugh, I can't seem to reproduce this anymore... Will try something
else tomorrow.
From: Eric Wong <hidden> Date: 2012-12-29 11:34:34
Eric Wong [off-list ref] wrote:
Eric Wong [off-list ref] wrote:
quoted
I'm finding ppoll() unexpectedly stuck when waiting for POLLIN on a
local TCP socket. The isolated code below can reproduces the issue
after many minutes (<1 hour). It might be easier to reproduce on
a busy system while disk I/O is happening.
Ugh, I can't seem to reproduce this anymore... Will try something
else tomorrow.
The good news is I'm not imagining this...
The bad news is the issue is real and took a long time to reproduce
again. This issue happens even without preempt, and without
tcp_low_latency on 3.7.1
While running `toosleepy', I also needed to run heavy (not loopback)
network and disk activity (several USB, SATA, and eSATA drives
simultaneously) for many hours before hitting this.
Hopefully this report is helpful in solving the issue. Looking in at
the various pieces in net and select/poll paths, there's several
references to race conditions in the comments so this is hopefully
familiar territory to someone here...
From: Eric Wong <hidden> Date: 2012-12-31 13:21:00
This patch seems to fix my issue with ppoll() being stuck on my
SMP machine: http://article.gmane.org/gmane.linux.file-systems/70414
The change to sock_poll_wait() in
commit 626cf236608505d376e4799adb4f7eb00a8594af
(poll: add poll_requested_events() and poll_does_not_wait() functions)
seems to have allowed additional cases where the SMP memory barrier
is not issued before checking for readiness.
In my case, this affects the select()-family of functions
which register descriptors once and set _qproc to NULL before
checking events again (after poll_schedule_timeout() returns).
The set_mb() barrier in poll_schedule_timeout() appears to be
insufficient on my SMP x86-64 machine (as it's only an xchg()).
This may also be related to the epoll issue described by
Andreas Voellmy in http://thread.gmane.org/gmane.linux.kernel/1408782/
Signed-off-by: Eric Wong <redacted>
Cc: Hans Verkuil <redacted>
Cc: Jiri Olsa <redacted>
Cc: Jonathan Corbet <corbet@lwn.net>
Cc: Al Viro <viro@zeniv.linux.org.uk>
Cc: Davide Libenzi <redacted>
Cc: Hans de Goede <redacted>
Cc: Mauro Carvalho Chehab <redacted>
Cc: David Miller <davem@davemloft.net>
Cc: Eric Dumazet <redacted>
Cc: Andrew Morton <akpm@linux-foundation.org>
Cc: Linus Torvalds <torvalds@linux-foundation.org>
Cc: Andreas Voellmy <redacted>
Cc: "Junchang(Jason) Wang" <redacted>
Cc: netdev@vger.kernel.org
Cc: linux-fsdevel@vger.kernel.org
---
If this patch is correct, I think we can just drop the
poll_does_not_wait() function entirely since poll_wait()
does the same check anyways...
include/net/sock.h | 5 +++--
1 file changed, 3 insertions(+), 2 deletions(-)
@@ -1925,8 +1925,9 @@ static inline bool wq_has_sleeper(struct socket_wq *wq)staticinlinevoidsock_poll_wait(structfile*filp,wait_queue_head_t*wait_address,poll_table*p){-if(!poll_does_not_wait(p)&&wait_address){-poll_wait(filp,wait_address,p);+if(wait_address){+if(!poll_does_not_wait(p))+poll_wait(filp,wait_address,p);/* We need to be sure we are in sync with the*socketflagsmodification.*
OK, it doesn't fix my issue, but it seems to make it harder-to-hit...
The change to sock_poll_wait() in
commit 626cf236608505d376e4799adb4f7eb00a8594af
(poll: add poll_requested_events() and poll_does_not_wait() functions)
seems to have allowed additional cases where the SMP memory barrier
is not issued before checking for readiness.
In my case, this affects the select()-family of functions
which register descriptors once and set _qproc to NULL before
checking events again (after poll_schedule_timeout() returns).
The set_mb() barrier in poll_schedule_timeout() appears to be
insufficient on my SMP x86-64 machine (as it's only an xchg()).
This may also be related to the epoll issue described by
Andreas Voellmy in http://thread.gmane.org/gmane.linux.kernel/1408782/
However, I believe my patch will still fix Andreas' issue with epoll
due to how ep_modify() uses a NULL qproc when calling ->poll().
(I've never been able to reproduce Andreas' issue on my 4-core system,
but he's been hitting it since 3.4 (at least))
From: Junchang(Jason) Wang <hidden> Date: 2013-01-01 16:58:16
Hi Eric and list,
Thanks a lot. The patch solves our (Andreas and my) issue in using
epoll. Here's our test program
https://github.com/AndreasVoellmy/epollbug/blob/master/epollbug.c We
are using Linux 3.7.1 and a server with 80 cores.
Cheers!
--Jason
On Mon, Dec 31, 2012 at 6:24 PM, Eric Wong [off-list ref] wrote:
OK, it doesn't fix my issue, but it seems to make it harder-to-hit...
quoted
The change to sock_poll_wait() in
commit 626cf236608505d376e4799adb4f7eb00a8594af
(poll: add poll_requested_events() and poll_does_not_wait() functions)
seems to have allowed additional cases where the SMP memory barrier
is not issued before checking for readiness.
In my case, this affects the select()-family of functions
which register descriptors once and set _qproc to NULL before
checking events again (after poll_schedule_timeout() returns).
The set_mb() barrier in poll_schedule_timeout() appears to be
insufficient on my SMP x86-64 machine (as it's only an xchg()).
This may also be related to the epoll issue described by
Andreas Voellmy in http://thread.gmane.org/gmane.linux.kernel/1408782/
However, I believe my patch will still fix Andreas' issue with epoll
due to how ep_modify() uses a NULL qproc when calling ->poll().
(I've never been able to reproduce Andreas' issue on my 4-core system,
but he's been hitting it since 3.4 (at least))
From: Eric Dumazet <hidden> Date: 2013-01-01 18:42:30
On Mon, 2012-12-31 at 13:21 +0000, Eric Wong wrote:
This patch seems to fix my issue with ppoll() being stuck on my
SMP machine: http://article.gmane.org/gmane.linux.file-systems/70414
The change to sock_poll_wait() in
commit 626cf236608505d376e4799adb4f7eb00a8594af
(poll: add poll_requested_events() and poll_does_not_wait() functions)
seems to have allowed additional cases where the SMP memory barrier
is not issued before checking for readiness.
In my case, this affects the select()-family of functions
which register descriptors once and set _qproc to NULL before
checking events again (after poll_schedule_timeout() returns).
The set_mb() barrier in poll_schedule_timeout() appears to be
insufficient on my SMP x86-64 machine (as it's only an xchg()).
This may also be related to the epoll issue described by
Andreas Voellmy in http://thread.gmane.org/gmane.linux.kernel/1408782/
Hmm, the change seems not very logical to me.
If it helps, I would like to understand the real issue.
commit 626cf236608505d376e4799adb4f7eb00a8594af should not have this
side effect, at least for poll()/select() functions. The epoll() changes
I am not yet very confident.
I suspect a race already existed before this commit, it would be nice to
track it properly.
From: Eric Wong <hidden> Date: 2013-01-01 21:00:33
Eric Dumazet [off-list ref] wrote:
On Mon, 2012-12-31 at 13:21 +0000, Eric Wong wrote:
quoted
This patch seems to fix my issue with ppoll() being stuck on my
SMP machine: http://article.gmane.org/gmane.linux.file-systems/70414
The change to sock_poll_wait() in
commit 626cf236608505d376e4799adb4f7eb00a8594af
(poll: add poll_requested_events() and poll_does_not_wait() functions)
seems to have allowed additional cases where the SMP memory barrier
is not issued before checking for readiness.
In my case, this affects the select()-family of functions
which register descriptors once and set _qproc to NULL before
checking events again (after poll_schedule_timeout() returns).
The set_mb() barrier in poll_schedule_timeout() appears to be
insufficient on my SMP x86-64 machine (as it's only an xchg()).
This may also be related to the epoll issue described by
Andreas Voellmy in http://thread.gmane.org/gmane.linux.kernel/1408782/
Hmm, the change seems not very logical to me.
My original description was not complete and I'm still bisecting
my problem (ppoll + send stuck). However, my patch does solve the
issue Andreas encountered and I now understand why.
If it helps, I would like to understand the real issue.
commit 626cf236608505d376e4799adb4f7eb00a8594af should not have this
side effect, at least for poll()/select() functions. The epoll() changes
I am not yet very confident.
I have a better explanation of the epoll problem below.
An alternate version (limited to epoll) would be:
I suspect a race already existed before this commit, it would be nice to
track it properly.
I don't believe this race existed before that change.
Updated commit message below:
From 87bca82bc39a941d9b8d5b8bc08b39a071a9884f Mon Sep 17 00:00:00 2001
From: Eric Wong <redacted>
Date: Mon, 31 Dec 2012 13:20:23 +0000
Subject: [PATCH] epoll: prevent missed events on EPOLL_CTL_MOD
ep_modify() works on files that are already registered with a wait queue
(and thus should not reregister). For sockets, this means sk_sleep()
will return a non-NULL wait address.
ep_modify() must check for events that were received and ignored
_before_ ep_modify() was called. So it must call f_op->poll() to
fish for events _after_ changing epi->event.events.
When f_op->poll() calls tcp_poll() (and thus sock_poll_wait()),
wait_address is non-NULL because the socket was already registered by
epoll. Thus, ep_modify() passes a NULL pt to prevent re-registration.
When ep_modify() is called, sock_poll_wait() will see a wait_address,
but a NULL pt, and this caused the memory barrier to get skipped and
events to be missed (this memory barrier is described in the
documentation for wq_has_sleeper).
This regression appeared with the change to sock_poll_wait() in
commit 626cf236608505d376e4799adb4f7eb00a8594af
(poll: add poll_requested_events() and poll_does_not_wait() functions)
This issue was encountered by Andreas Voellmy and Junchang(Jason) Wang:
http://thread.gmane.org/gmane.linux.kernel/1408782/
Signed-off-by: Eric Wong <redacted>
Cc: Hans Verkuil <redacted>
Cc: Jiri Olsa <redacted>
Cc: Jonathan Corbet <corbet@lwn.net>
Cc: Al Viro <viro@zeniv.linux.org.uk>
Cc: Davide Libenzi <redacted>
Cc: Hans de Goede <redacted>
Cc: Mauro Carvalho Chehab <redacted>
Cc: David Miller <davem@davemloft.net>
Cc: Eric Dumazet <redacted>
Cc: Andrew Morton <akpm@linux-foundation.org>
Cc: Linus Torvalds <torvalds@linux-foundation.org>
Tested-by: Andreas Voellmy <redacted>
Tested-by: "Junchang(Jason) Wang" <redacted>
Cc: netdev@vger.kernel.org
Cc: linux-fsdevel@vger.kernel.org
---
include/net/sock.h | 5 +++--
1 file changed, 3 insertions(+), 2 deletions(-)
@@ -1925,8 +1925,9 @@ static inline bool wq_has_sleeper(struct socket_wq *wq)staticinlinevoidsock_poll_wait(structfile*filp,wait_queue_head_t*wait_address,poll_table*p){-if(!poll_does_not_wait(p)&&wait_address){-poll_wait(filp,wait_address,p);+if(wait_address){+if(!poll_does_not_wait(p))+poll_wait(filp,wait_address,p);/* We need to be sure we are in sync with the*socketflagsmodification.*
From: Eric Wong <hidden> Date: 2013-01-01 21:17:28
Eric Wong [off-list ref] wrote:
quoted hunk
Eric Dumazet [off-list ref] wrote:
quoted
commit 626cf236608505d376e4799adb4f7eb00a8594af should not have this
side effect, at least for poll()/select() functions. The epoll() changes
I am not yet very confident.
I have a better explanation of the epoll problem below.
An alternate version (limited to epoll) would be:
I was wrong, rereading 626cf236608505d376e4799adb4f7eb00a8594af,
I think this race existed before.
Perhaps my alternate patch above is a better fix.
Please document the barrier that this mb() pairs with, and then give
an explanation for the fix in the commit message, and I'll happily
take it. Even if it's just duplicating the comments above the
wq_has_sleeper() function, except modified for the ep_modify() case.
Of course, it would be good to get verification from Jason and Andreas
that the alternate patch also works for them.
Linus
From: Junchang(Jason) Wang <hidden> Date: 2013-01-01 23:21:03
Hi all,
The alternate patch from Eric works well too. Even though I didn't see
a performance boost compared with the old version, this one is clearer
to me. Thanks your guys.
Cheers!
--Jason
On Tue, Jan 1, 2013 at 5:53 PM, Linus Torvalds
[off-list ref] wrote:
On Tue, Jan 1, 2013 at 1:17 PM, Eric Wong [off-list ref] wrote:
I was wrong, rereading 626cf236608505d376e4799adb4f7eb00a8594af,
I think this race existed before.
Perhaps my alternate patch above is a better fix.
Please document the barrier that this mb() pairs with, and then give
an explanation for the fix in the commit message, and I'll happily
take it. Even if it's just duplicating the comments above the
wq_has_sleeper() function, except modified for the ep_modify() case.
Of course, it would be good to get verification from Jason and Andreas
that the alternate patch also works for them.
Linus
From: Eric Wong <hidden> Date: 2013-01-01 23:56:05
Linus Torvalds [off-list ref] wrote:
Please document the barrier that this mb() pairs with, and then give
an explanation for the fix in the commit message, and I'll happily
take it. Even if it's just duplicating the comments above the
wq_has_sleeper() function, except modified for the ep_modify() case.
Hopefully my explanation is correct and makes sense below,
I think both effects of the barrier are needed
Of course, it would be good to get verification from Jason and Andreas
that the alternate patch also works for them.
Jason just confirmed it.
------------------------------- 8< ----------------------------
From 02f43757d04bb6f2786e79eecf1cfa82e6574379 Mon Sep 17 00:00:00 2001
From: Eric Wong <redacted>
Date: Tue, 1 Jan 2013 21:20:27 +0000
Subject: [PATCH] epoll: prevent missed events on EPOLL_CTL_MOD
EPOLL_CTL_MOD sets the interest mask before calling f_op->poll() to
ensure events are not missed. Since the modifications to the interest
mask are not protected by the same lock as ep_poll_callback, we need to
ensure the change is visible to other CPUs calling ep_poll_callback.
We also need to ensure f_op->poll() has an up-to-date view of past
events which occured before we modified the interest mask. So this
barrier also pairs with the barrier in wq_has_sleeper().
This should guarantee either ep_poll_callback or f_op->poll() (or both)
will notice the readiness of a recently-ready/modified item.
This issue was encountered by Andreas Voellmy and Junchang(Jason) Wang in:
http://thread.gmane.org/gmane.linux.kernel/1408782/
Signed-off-by: Eric Wong <redacted>
Cc: Hans Verkuil <redacted>
Cc: Jiri Olsa <redacted>
Cc: Jonathan Corbet <corbet@lwn.net>
Cc: Al Viro <viro@zeniv.linux.org.uk>
Cc: Davide Libenzi <redacted>
Cc: Hans de Goede <redacted>
Cc: Mauro Carvalho Chehab <redacted>
Cc: David Miller <davem@davemloft.net>
Cc: Eric Dumazet <redacted>
Cc: Andrew Morton <akpm@linux-foundation.org>
Cc: Linus Torvalds <torvalds@linux-foundation.org>
Cc: Andreas Voellmy <redacted>
Tested-by: "Junchang(Jason) Wang" <redacted>
Cc: netdev@vger.kernel.org
Cc: linux-fsdevel@vger.kernel.org
---
fs/eventpoll.c | 22 +++++++++++++++++++++-
1 file changed, 21 insertions(+), 1 deletion(-)
From: Eric Dumazet <hidden> Date: 2013-01-02 17:45:53
On Tue, 2013-01-01 at 23:56 +0000, Eric Wong wrote:
quoted hunk
Linus Torvalds [off-list ref] wrote:
quoted
Please document the barrier that this mb() pairs with, and then give
an explanation for the fix in the commit message, and I'll happily
take it. Even if it's just duplicating the comments above the
wq_has_sleeper() function, except modified for the ep_modify() case.
Hopefully my explanation is correct and makes sense below,
I think both effects of the barrier are needed
quoted
Of course, it would be good to get verification from Jason and Andreas
that the alternate patch also works for them.
Jason just confirmed it.
------------------------------- 8< ----------------------------
From 02f43757d04bb6f2786e79eecf1cfa82e6574379 Mon Sep 17 00:00:00 2001
From: Eric Wong <redacted>
Date: Tue, 1 Jan 2013 21:20:27 +0000
Subject: [PATCH] epoll: prevent missed events on EPOLL_CTL_MOD
EPOLL_CTL_MOD sets the interest mask before calling f_op->poll() to
ensure events are not missed. Since the modifications to the interest
mask are not protected by the same lock as ep_poll_callback, we need to
ensure the change is visible to other CPUs calling ep_poll_callback.
We also need to ensure f_op->poll() has an up-to-date view of past
events which occured before we modified the interest mask. So this
barrier also pairs with the barrier in wq_has_sleeper().
This should guarantee either ep_poll_callback or f_op->poll() (or both)
will notice the readiness of a recently-ready/modified item.
This issue was encountered by Andreas Voellmy and Junchang(Jason) Wang in:
http://thread.gmane.org/gmane.linux.kernel/1408782/
Signed-off-by: Eric Wong <redacted>
Cc: Hans Verkuil <redacted>
Cc: Jiri Olsa <redacted>
Cc: Jonathan Corbet <corbet@lwn.net>
Cc: Al Viro <viro@zeniv.linux.org.uk>
Cc: Davide Libenzi <redacted>
Cc: Hans de Goede <redacted>
Cc: Mauro Carvalho Chehab <redacted>
Cc: David Miller <davem@davemloft.net>
Cc: Eric Dumazet <redacted>
Cc: Andrew Morton <akpm@linux-foundation.org>
Cc: Linus Torvalds <torvalds@linux-foundation.org>
Cc: Andreas Voellmy <redacted>
Tested-by: "Junchang(Jason) Wang" <redacted>
Cc: netdev@vger.kernel.org
Cc: linux-fsdevel@vger.kernel.org
---
fs/eventpoll.c | 22 +++++++++++++++++++++-
1 file changed, 21 insertions(+), 1 deletion(-)
First, thanks for working on this issue.
It seems the real problem is the epi->event.events = event->events;
which is done without taking ep->lock
While a smp_mb() could reduce the race window, I believe there is still
a race, and the following patch would close it.
From: Eric Dumazet <hidden> Date: 2013-01-02 19:03:45
On Wed, 2013-01-02 at 18:40 +0000, Eric Wong wrote:
Eric Dumazet [off-list ref] wrote:
quoted
First, thanks for working on this issue.
No problem!
quoted
It seems the real problem is the epi->event.events = event->events;
which is done without taking ep->lock
Yes. I am hoping it is possible to do it without a lock there,
but your change is more obviously correct.
quoted
While a smp_mb() could reduce the race window, I believe there is still
a race, and the following patch would close it.
I'm not an experienced kernel hacker, can you describe where the race
would be?
It would be for example in ep_send_events_proc() doing :
if (epi->event.events & EPOLLONESHOT)
epi->event.events &= EP_PRIVATE_BITS;
And this could happen at the same time.
From: Eric Wong <hidden> Date: 2013-01-02 19:32:27
Eric Dumazet [off-list ref] wrote:
On Wed, 2013-01-02 at 18:40 +0000, Eric Wong wrote:
quoted
Eric Dumazet [off-list ref] wrote:
quoted
It seems the real problem is the epi->event.events = event->events;
which is done without taking ep->lock
Yes. I am hoping it is possible to do it without a lock there,
but your change is more obviously correct.
quoted
While a smp_mb() could reduce the race window, I believe there is still
a race, and the following patch would close it.
I'm not an experienced kernel hacker, can you describe where the race
would be?
It would be for example in ep_send_events_proc() doing :
if (epi->event.events & EPOLLONESHOT)
epi->event.events &= EP_PRIVATE_BITS;
And this could happen at the same time.
That modification in ep_send_events_proc() is protected by ep->mtx
(as is ep_modify()), though. Maybe there are other places, but I
don't see it.
From: Eric Wong <hidden> Date: 2013-01-02 20:08:50
(changing Cc:)
Eric Wong [off-list ref] wrote:
I'm finding ppoll() unexpectedly stuck when waiting for POLLIN on a
local TCP socket. The isolated code below can reproduces the issue
after many minutes (<1 hour). It might be easier to reproduce on
a busy system while disk I/O is happening.
s/might be/is/
Strangely, I've bisected this seemingly networking-related issue down to
the following commit:
commit 1fb3f8ca0e9222535a39b884cb67a34628411b9f
Author: Mel Gorman [off-list ref]
Date: Mon Oct 8 16:29:12 2012 -0700
mm: compaction: capture a suitable high-order page immediately when it is made available
That commit doesn't revert cleanly on v3.7.1, and I don't feel
comfortable touching that code myself.
Instead, I disabled THP+compaction under v3.7.1 and I've been unable to
reproduce the issue without THP+compaction.
As I mention in http://mid.gmane.org/20121229113434.GA13336@dcvr.yhbt.net
I run my below test (`toosleepy') with heavy network and disk activity
for a long time before hitting this.
My disk activity involves copying large files around to different local
drives over loopback[1], so perhaps the duplicate pages get compacted
away? toosleepy also reuses the same 16K junk data all around.
[1] my full setup is very strange.
Other than the FUSE component I forgot to mention, little depends on
the kernel. With all this, the standalone toosleepy can get stuck.
I'll try to reproduce it with less...
(possibly relevant info, I don't expect you to duplicate my setup
as it requires many, many patched userspace components :x):
fusedav (with many bugfixes[2]) -> (FUSE device)
zbatery (Ruby 1.9.3-p362) -> omgdav (in zbatery process) -> (TCP)
MogileFS (patched[3]) -> (TCP)
cmogstored
The (zbatery -> omgdav -> MogileFS -> cmogstored) path is all userspace.
cmogstored uses sendfile and may talk to itself via MogileFS replication:
MogileFS(replicate) -> HTTP GET from cmogstored -> HTTP PUT to cmogstored
(MFS was designed for clusters, but I only have one machine right
now) MogileFS replicate does not use splice between sockets, just
read/write, cmogstored does not use splice (yet) either.
The stuck ppoll() I noticed is from Ruby (zbatery/omgdav) while the
send() was from fusedav (using neon).
[2] my patches on http://bugs.debian.org/fusedav and
git clone git://bogomips.org/fusedav.git home
[3] git clone git://bogomips.org/MogileFS-Server.git testing
(That epoll issue was unrelated and fixed while I was hunting this bug)
My example involves a 3 thread data flow between two pairs
of (4) sockets:
send_loop -> recv_loop(recv_send) -> recv_loop(recv_only)
pair_a[1] -> (pair_a[0] -> pair_b[1]) -> pair_b[0]
At least 3.7 and 3.7.1 are affected.
I have tcp_low_latency=1 set, I will try 0 later
The last progress message I got was after receiving 2942052597760
bytes on fd=7 (out of 64-bit ULONG_MAX / 2)
strace:
3644 sendto(4, "\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0"..., 16384, 0, NULL, 0 <unfinished ...>
3643 sendto(6, "\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0"..., 16384, 0, NULL, 0 <unfinished ...>
3642 ppoll([{fd=7, events=POLLIN}], 1, NULL, NULL, 8 <unfinished ...>
3641 futex(0x7f23ed8129d0, FUTEX_WAIT, 3644, NULL <unfinished ...>
The first and last lines of the strace are expected:
+ 3644 sendto(4) is blocked because 3643 is blocked on sendto(fd=6)
and not able to call recv().
+ 3641 is the main thread calling pthread_join
What is unexpected is the tid=3643 and tid=3642 interaction. As confirmed
by lsof below, fd=6 is sending to wake up fd=7, but ppoll(fd=7) seems
to not be waking up.
lsof:
toosleepy 3641 ew 4u IPv4 12405 0t0 TCP localhost:55904->localhost:33249 (ESTABLISHED)
toosleepy 3641 ew 5u IPv4 12406 0t0 TCP localhost:33249->localhost:55904 (ESTABLISHED)
toosleepy 3641 ew 6u IPv4 12408 0t0 TCP localhost:48777->localhost:33348 (ESTABLISHED)
toosleepy 3641 ew 7u IPv4 12409 0t0 TCP localhost:33348->localhost:48777 (ESTABLISHED)
System info: Linux 3.7.1 x86_64 SMP PREEMPT
AMD Phenom(tm) II X4 945 Processor (4 cores)
Nothing interesting in dmesg, iptables rules are empty.
I have not yet been able to reproduce the issue using UNIX sockets,
only TCP, but you can run:
./toosleepy unix
...to test with UNIX sockets intead of TCP.
The following code is also available via git://bogomips.org/toosleepy
gcc -o toosleepy -O2 -Wall -lpthread toosleepy.c
-------------------------------- 8< ------------------------------------
#define _GNU_SOURCE
#include <poll.h>
#include <sys/ioctl.h>
#include <pthread.h>
#include <sys/types.h>
#include <sys/socket.h>
#include <arpa/inet.h>
#include <netinet/tcp.h>
#include <stdint.h>
#include <stdio.h>
#include <stdlib.h>
#include <errno.h>
#include <string.h>
#include <unistd.h>
#include <fcntl.h>
#include <assert.h>
#include <limits.h>
struct receiver {
int rfd;
int sfd;
};
/* blocking sender */
static void * send_loop(void *fdp)
{
int fd = *(int *)fdp;
char buf[16384];
ssize_t s;
size_t sent = 0;
size_t max = (size_t)ULONG_MAX / 2;
while (sent < max) {
s = send(fd, buf, sizeof(buf), 0);
if (s > 0)
sent += s;
if (s == -1)
assert(errno == EINTR);
}
dprintf(2, "%d done sending: %zu\n", fd, sent);
close(fd);
return NULL;
}
/* non-blocking receiver, using ppoll */
static void * recv_loop(void *p)
{
const struct receiver *rcvr = p;
char buf[16384];
nfds_t nfds = 1;
struct pollfd fds;
int rc;
ssize_t r, s;
size_t received = 0;
size_t sent = 0;
for (;;) {
r = recv(rcvr->rfd, buf, sizeof(buf), 0);
if (r == 0) {
break;
} else if (r == -1) {
assert(errno == EAGAIN);
fds.fd = rcvr->rfd;
fds.events = POLLIN;
errno = 0;
rc = ppoll(&fds, nfds, NULL, NULL);
assert(rc == 1);
} else {
assert(r > 0);
received += r;
if (rcvr->sfd >= 0) {
s = send(rcvr->sfd, buf, sizeof(buf), 0);
if (s > 0)
sent += s;
if (s == -1)
assert(errno == EINTR);
} else {
/* just burn some cycles */
write(-1, buf, sizeof(buf));
}
}
if ((received % (sizeof(buf) * sizeof(buf) * 16) == 0))
dprintf(2, " %d progress: %zu\n",
rcvr->rfd, received);
}
dprintf(2, "%d got: %zu\n", rcvr->rfd, received);
if (rcvr->sfd >= 0) {
dprintf(2, "%d sent: %zu\n", rcvr->sfd, sent);
close(rcvr->sfd);
}
return NULL;
}
static void tcp_socketpair(int sv[2], int accept_flags)
{
struct sockaddr_in addr;
socklen_t addrlen = sizeof(addr);
int l = socket(PF_INET, SOCK_STREAM, 0);
int c = socket(PF_INET, SOCK_STREAM, 0);
int a;
addr.sin_family = AF_INET;
addr.sin_addr.s_addr = INADDR_ANY;
addr.sin_port = 0;
assert(0 == bind(l, (struct sockaddr*)&addr, addrlen));
assert(0 == listen(l, 1024));
assert(0 == getsockname(l, (struct sockaddr *)&addr, &addrlen));
assert(0 == connect(c, (struct sockaddr *)&addr, addrlen));
a = accept4(l, NULL, NULL, accept_flags);
assert(a >= 0);
close(l);
sv[0] = a;
sv[1] = c;
}
int main(int argc, char *argv[])
{
int pair_a[2];
int pair_b[2];
pthread_t s, rs, r;
struct receiver recv_only;
struct receiver recv_send;
if (argc == 2 && strcmp(argv[1], "unix") == 0) {
int val;
assert(0 == socketpair(AF_UNIX, SOCK_STREAM, 0, pair_a));
assert(0 == socketpair(AF_UNIX, SOCK_STREAM, 0, pair_b));
/* only make the receiver non-blocking */
val = 1;
assert(0 == ioctl(pair_a[0], FIONBIO, &val));
val = 1;
assert(0 == ioctl(pair_b[0], FIONBIO, &val));
} else {
tcp_socketpair(pair_a, SOCK_NONBLOCK);
tcp_socketpair(pair_b, SOCK_NONBLOCK);
}
recv_send.rfd = pair_a[0];
recv_send.sfd = pair_b[1];
recv_only.rfd = pair_b[0];
recv_only.sfd = -1;
/*
* data flow:
* send_loop -> recv_loop(recv_send) -> recv_loop(recv_only)
* pair_a[1] -> (pair_a[0] -> pair_b[1]) -> pair_b[0]
*/
assert(0 == pthread_create(&r, NULL, recv_loop, &recv_only));
assert(0 == pthread_create(&rs, NULL, recv_loop, &recv_send));
assert(0 == pthread_create(&s, NULL, send_loop, &pair_a[1]));
assert(0 == pthread_join(s, NULL));
assert(0 == pthread_join(rs, NULL));
assert(0 == pthread_join(r, NULL));
return 0;
}
-------------------------------- 8< ------------------------------------
Any help/suggestions/test patches would be greatly appreciated.
Thanks for reading!
--
Eric Wong
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>
From: Eric Wong <hidden> Date: 2013-01-02 20:47:15
Eric Wong [off-list ref] wrote:
[1] my full setup is very strange.
Other than the FUSE component I forgot to mention, little depends on
the kernel. With all this, the standalone toosleepy can get stuck.
I'll try to reproduce it with less...
I just confirmed my toosleepy processes will get stuck while just
doing "rsync -a" between local disks. So this does not depend on
sendfile or FUSE to reproduce.
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>
From: Eric Wong <hidden> Date: 2013-01-02 21:16:32
Eric Wong [off-list ref] wrote:
Linus Torvalds [off-list ref] wrote:
quoted
Please document the barrier that this mb() pairs with, and then give
an explanation for the fix in the commit message, and I'll happily
take it. Even if it's just duplicating the comments above the
wq_has_sleeper() function, except modified for the ep_modify() case.
Hopefully my explanation is correct and makes sense below,
I think both effects of the barrier are needed
I noticed Linus accepted this already. This should probably go to
stable, right?
From ancient git history[1], it seems this bug exists for all
From: Eric Dumazet <hidden> Date: 2013-01-02 22:08:31
On Wed, 2013-01-02 at 19:32 +0000, Eric Wong wrote:
That modification in ep_send_events_proc() is protected by ep->mtx
(as is ep_modify()), though. Maybe there are other places, but I
don't see it.
Yes, and using a mutex for protecting this field while its read from
interrupt context (so without mutex synch help) is why there were races.
Some users rely on barriers included in spin_lock/spin_unlock, others in
explicit barriers, or before your patch pure luck.
From: Eric Dumazet <hidden> Date: 2013-01-03 13:41:15
On Wed, 2013-01-02 at 20:47 +0000, Eric Wong wrote:
Eric Wong [off-list ref] wrote:
quoted
[1] my full setup is very strange.
Other than the FUSE component I forgot to mention, little depends on
the kernel. With all this, the standalone toosleepy can get stuck.
I'll try to reproduce it with less...
I just confirmed my toosleepy processes will get stuck while just
doing "rsync -a" between local disks. So this does not depend on
sendfile or FUSE to reproduce.
--
How do you tell your 'toosleepy' is stuck ?
If reading its output, you should change its logic, there is no
guarantee the recv() will deliver exactly 16384 bytes each round.
With the following patch, I cant reproduce the 'apparent stuck'
From: Eric Wong <hidden> Date: 2013-01-03 18:32:54
Eric Dumazet [off-list ref] wrote:
On Wed, 2013-01-02 at 20:47 +0000, Eric Wong wrote:
quoted
Eric Wong [off-list ref] wrote:
quoted
[1] my full setup is very strange.
Other than the FUSE component I forgot to mention, little depends on
the kernel. With all this, the standalone toosleepy can get stuck.
I'll try to reproduce it with less...
I just confirmed my toosleepy processes will get stuck while just
doing "rsync -a" between local disks. So this does not depend on
sendfile or FUSE to reproduce.
--
How do you tell your 'toosleepy' is stuck ?
My original post showed it stuck with strace (in ppoll + send).
I only strace after seeing it's not using any CPU in top.
http://mid.gmane.org/20121228014503.GA5017@dcvr.yhbt.net
(lsof also confirmed the ppoll/send sockets were peers)
If reading its output, you should change its logic, there is no
guarantee the recv() will deliver exactly 16384 bytes each round.
With the following patch, I cant reproduce the 'apparent stuck'
Right, the output is just an approximation and the logic there
was bogus.
Thanks for looking at this.
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>
From: Eric Wong <hidden> Date: 2013-01-03 23:46:01
Eric Wong [off-list ref] wrote:
Eric Dumazet [off-list ref] wrote:
quoted
With the following patch, I cant reproduce the 'apparent stuck'
Right, the output is just an approximation and the logic there
was bogus.
Thanks for looking at this.
I'm still able to reproduce the issue under v3.8-rc2 with your patch
for toosleepy.
(As expected when blocked,) TCP send() will eventually return
ETIMEOUT when I forget to check (and toosleepy will abort from it)
I think this requires frequent dirtying/cycling of pages to reproduce.
(from copying large files around) to interact with compaction.
I'll see if I can reproduce the issue with read-only FS activity.
With 3.7.1 and compaction/THP disabled, I was able to run ~21 hours
and copy a few TB around without anything getting stuck.
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>
From: Eric Wong <hidden> Date: 2013-01-04 00:26:39
Eric Wong [off-list ref] wrote:
I think this requires frequent dirtying/cycling of pages to reproduce.
(from copying large files around) to interact with compaction.
I'll see if I can reproduce the issue with read-only FS activity.
Still successfully running the read-only test on my main machine, will
provide another update in a few hours or so if it's still successful
(it usually takes <1 hour to hit).
I also fired up a VM on my laptop (still running v3.7) and was able to
get stuck with only 2 cores and 512M on the VM (x86_64). On the small
VM with little disk space, it doesn't need much dirty data to trigger.
I just did this:
find $45G_NFS_MOUNT -type f -print0 | \
xargs -0 -n1 -P4 sh -c 'cat "$1" >> tmp; > tmp' --
...while running two instances of toosleepy (one got stuck and aborted).
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>
From: Eric Wong <hidden> Date: 2013-01-04 03:52:58
Eric Wong [off-list ref] wrote:
Eric Wong [off-list ref] wrote:
quoted
I think this requires frequent dirtying/cycling of pages to reproduce.
(from copying large files around) to interact with compaction.
I'll see if I can reproduce the issue with read-only FS activity.
Still successfully running the read-only test on my main machine, will
provide another update in a few hours or so if it's still successful
(it usually takes <1 hour to hit).
The read-only test is still going on my main machine.
I think writes/dirty data is required to reproduce the issue...
On Wed, Jan 02, 2013 at 08:08:48PM +0000, Eric Wong wrote:
(changing Cc:)
Eric Wong [off-list ref] wrote:
quoted
I'm finding ppoll() unexpectedly stuck when waiting for POLLIN on a
local TCP socket. The isolated code below can reproduces the issue
after many minutes (<1 hour). It might be easier to reproduce on
a busy system while disk I/O is happening.
s/might be/is/
Strangely, I've bisected this seemingly networking-related issue down to
the following commit:
commit 1fb3f8ca0e9222535a39b884cb67a34628411b9f
Author: Mel Gorman [off-list ref]
Date: Mon Oct 8 16:29:12 2012 -0700
mm: compaction: capture a suitable high-order page immediately when it is made available
That commit doesn't revert cleanly on v3.7.1, and I don't feel
comfortable touching that code myself.
That patch introduced an accounting bug that was corrected by ef6c5be6
(fix incorrect NR_FREE_PAGES accounting (appears like memory leak)). In
some cases that could look like a hang and potentially confuses a bisection.
That said, I see that you report that 3.7.1 and 3.8-rc2 are affected that
includes that fix and the finger is pointed at compaction so something
is wrong.
Instead, I disabled THP+compaction under v3.7.1 and I've been unable to
reproduce the issue without THP+compaction.
Implying that it's stuck in compaction somewhere. It could be the case
that compaction alters timing enough to trigger another bug. You say it
tests differently depending on whether TCP or unix sockets are used
which might indicate multiple problems. However, lets try and see if
compaction is the primary problem or not.
Using a 3.7.1 or 3.8-rc2 kernel, can you reproduce the problem and then
answer the following questions please?
1. What are the contents of /proc/vmstat at the time it is stuck?
2. What are the contents of /proc/PID/stack for every toosleepy
process when they are stuck?
3. Can you do a sysrq+m and post the resulting dmesg?
What I'm looking for is a throttling bug (if pgscan_direct_throttle is
elevated), an isolated page accounting bug (nr_isolated_* is elevated
and process is stuck in congestion_wait in a too_many_isolated() loop)
or a free page accounting bug (big difference between nr_free_pages and
buddy list figures).
I'll try reproducing this early next week if none of that shows an
obvious candidate.
Thanks.
--
Mel Gorman
SUSE Labs
From: Eric Dumazet <hidden> Date: 2013-01-04 17:15:13
On Fri, 2013-01-04 at 16:01 +0000, Mel Gorman wrote:
Implying that it's stuck in compaction somewhere. It could be the case
that compaction alters timing enough to trigger another bug. You say it
tests differently depending on whether TCP or unix sockets are used
which might indicate multiple problems. However, lets try and see if
compaction is the primary problem or not.
One difference between TCP or unix socket is that :
Unix sockets try hard to limit the order of allocations.
For a 16KB (+ skb overhead) send(), we will probably use one order-2
page and one order-0 page as a frag (data_len being not 0) :
vi +1484 net/unix/af_unix.c
if (len > SKB_MAX_ALLOC)
data_len = min_t(size_t,
len - SKB_MAX_ALLOC,
MAX_SKB_FRAGS * PAGE_SIZE);
skb = sock_alloc_send_pskb(sk, len - data_len, data_len,
msg->msg_flags & MSG_DONTWAIT, &err);
While TCP could use order-3 pages if available
Eric, you could try to change SKB_FRAG_PAGE_ORDER in net/core/sock.c to
lower values (16384, 8192, 4096) and check if the hang can disappear or
not.
Alternatively (no kernel patching needed), you could try to hang AF_UNIX
using buffers of 90KB, to force order-3 allocations as well (one 32KB
allocation plus 16 * 4KB frags)
Thanks
From: Eric Wong <hidden> Date: 2013-01-04 17:59:25
Mel Gorman [off-list ref] wrote:
On Wed, Jan 02, 2013 at 08:08:48PM +0000, Eric Wong wrote:
quoted
Instead, I disabled THP+compaction under v3.7.1 and I've been unable to
reproduce the issue without THP+compaction.
Implying that it's stuck in compaction somewhere. It could be the case
that compaction alters timing enough to trigger another bug. You say it
tests differently depending on whether TCP or unix sockets are used
which might indicate multiple problems. However, lets try and see if
compaction is the primary problem or not.
I haven't managed to reproduce the issue on Unix sockets, yet, just TCP.
Trying Unix with 90KB as Eric Dumazet suggested.
I'll get the info you need from /proc soon.
Thank you for looking at this!
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>
From: Eric Wong <hidden> Date: 2013-01-05 01:07:18
Mel Gorman [off-list ref] wrote:
On Wed, Jan 02, 2013 at 08:08:48PM +0000, Eric Wong wrote:
quoted
Instead, I disabled THP+compaction under v3.7.1 and I've been unable to
reproduce the issue without THP+compaction.
Implying that it's stuck in compaction somewhere. It could be the case
that compaction alters timing enough to trigger another bug. You say it
tests differently depending on whether TCP or unix sockets are used
which might indicate multiple problems. However, lets try and see if
compaction is the primary problem or not.
I've only managed to encounter this issue with TCP sockets.
No luck reproducing the issue with Unix sockets, not even with 90K
buffers as suggested by Eric Dumazet. This seems unique to TCP.
Fwiw, I also tried going back to a 16K MTU on loopback a few days ago,
but was still able to reproduce the issue, so
commit 0cf833aefaa85bbfce3ff70485e5534e09254773 doesn't seem
to be a culprit, either.
2. What are the contents of /proc/PID/stack for every toosleepy
process when they are stuck?
Oops, I needed a rebuild with CONFIG_STACKTRACE=y (it took some effort
to get the right combination of options).
I probably enabled a few more debugging options than I needed and it
seems to have taken longer to reproduce the issue. Unfortunately I was
distracted when toosleepy got stuck and missed the change to inspect
before hitting ETIMEDOUT :x
Attempting to reproduce the issue while I'm looking.
3. Can you do a sysrq+m and post the resulting dmesg?
What I'm looking for is a throttling bug (if pgscan_direct_throttle is
elevated), an isolated page accounting bug (nr_isolated_* is elevated
and process is stuck in congestion_wait in a too_many_isolated() loop)
or a free page accounting bug (big difference between nr_free_pages and
buddy list figures).
I'll try reproducing this early next week if none of that shows an
obvious candidate.
Thanks! I'll try to get you more information as soon as possible.
On Sun, Jan 06, 2013 at 12:07:00PM +0000, Eric Wong wrote:
Mel Gorman [off-list ref] wrote:
quoted
Using a 3.7.1 or 3.8-rc2 kernel, can you reproduce the problem and then
answer the following questions please?
This is on my main machine running 3.8-rc2
quoted
1. What are the contents of /proc/vmstat at the time it is stuck?
===> /proc/vmstat <===
According to this, THP is barely being used -- only 24 THP pages at the time
and the LRU lists are dominated by file pages. The isolated and throttled
counters look fine. There is a lot of memory currently under writeback and
a large number of dirty pages are reaching the end of the LRU list which
is inefficient but does not account for the reported bug.
quoted
2. What are the contents of /proc/PID/stack for every toosleepy
process when they are stuck?
pid and tid stack info, 28018 is the thread I used to automate
reporting (pushed to git://bogomips.org/toosleepy.git)
===> 28014[28014]/stack <===
[<ffffffff8105a97b>] futex_wait_queue_me+0xb7/0xd2
[<ffffffff8105b7fc>] futex_wait+0xf6/0x1f6
[<ffffffff811bb3af>] cpumask_next_and+0x2b/0x37
[<ffffffff8104ebfa>] select_task_rq_fair+0x518/0x59a
[<ffffffff8105c8f1>] do_futex+0xa9/0x88f
[<ffffffff810509a4>] check_preempt_wakeup+0x10d/0x1a7
[<ffffffff8104757d>] check_preempt_curr+0x25/0x62
[<ffffffff8104d4cc>] wake_up_new_task+0x96/0xc2
[<ffffffff8105d1e9>] sys_futex+0x112/0x14d
[<ffffffff81322a49>] stub_clone+0x69/0x90
[<ffffffff81322769>] system_call_fastpath+0x16/0x1b
[<ffffffffffffffff>] 0xffffffffffffffff
This seems to be the guy that's stuck. It's waiting for more memory for
the socket but who or what is allocating that memory? There are a few other
bugs from over the weekend that I want to take a look at so I did not dig
further or try to reproduce this bug yet. I'm adding Eric Dumazet back to
the cc in case he has the quick answer.
Nothing wrong there that I can see. The free list contents roughly match
up with the NR_FREE_PAGES counter so it doesn't look like an accounting
bug. However, an accounting bug could have broken the bisection and
found a different bug.
When taking pages straight off the buddy list like this patch does,
there is a danger that the watermarks will be broken resulting in a
livelock but the watermarks are checked properly and the free pages are
over the min watermark above.
There is this patch https://lkml.org/lkml/2013/1/6/219 but it is
unlikely that it has anything to do with your workload as it does not
use splice().
Right now it's difficult to see how the capture could be the source of
this bug but I'm not ruling it out either so try the following (untested
but should be ok) patch. It's not a proper revert, it just disables the
capture page logic to see if it's at fault.
@@ -1054,9 +1054,6 @@ static int compact_zone(struct zone *zone, struct compact_control *cc)gotoout;}}--/* Capture a page now if it is a suitable size */-compact_capture_page(cc);}out:
@@ -2179,11 +2179,8 @@ __alloc_pages_direct_compact(gfp_t gfp_mask, unsigned int order,contended_compaction,&page);current->flags&=~PF_MEMALLOC;-/* If compaction captured a page, prep and use it */-if(page){-prep_new_page(page,order,gfp_mask);-gotogot_page;-}+/* capture page is disabled, this should be impossible */+BUG_ON(page);if(*did_some_progress!=COMPACT_SKIPPED){/* Page migration frees to the PCP lists but we want merging */
@@ -2195,7 +2192,6 @@ __alloc_pages_direct_compact(gfp_t gfp_mask, unsigned int order,alloc_flags&~ALLOC_NO_WATERMARKS,preferred_zone,migratetype);if(page){-got_page:preferred_zone->compact_blockskip_flush=false;preferred_zone->compact_considered=0;preferred_zone->compact_defer_shift=0;
This seems to be the guy that's stuck. It's waiting for more memory for
the socket but who or what is allocating that memory? There are a few other
bugs from over the weekend that I want to take a look at so I did not dig
further or try to reproduce this bug yet. I'm adding Eric Dumazet back to
the cc in case he has the quick answer.
Thanks Mel
It would not surprise me if sk_stream_wait_memory() have plain bug(s) or
race(s).
In 2010, in commit 482964e56e132 Nagendra Tomar fixed a pretty severe
long standing bug.
This path is not taken very often on most machines.
I would try the following patch :
@@ -126,6 +126,7 @@ int sk_stream_wait_memory(struct sock *sk, long *timeo_p)while(1){set_bit(SOCK_ASYNC_NOSPACE,&sk->sk_socket->flags);+set_bit(SOCK_NOSPACE,&sk->sk_socket->flags);prepare_to_wait(sk_sleep(sk),&wait,TASK_INTERRUPTIBLE);
@@ -139,7 +140,6 @@ int sk_stream_wait_memory(struct sock *sk, long *timeo_p)if(sk_stream_memory_free(sk)&&!vm_wait)break;-set_bit(SOCK_NOSPACE,&sk->sk_socket->flags);sk->sk_write_pending++;sk_wait_event(sk,¤t_timeo,sk->sk_err||(sk->sk_shutdown&SEND_SHUTDOWN)||--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>
From: Eric Wong <hidden> Date: 2013-01-07 22:38:52
Mel Gorman [off-list ref] wrote:
Right now it's difficult to see how the capture could be the source of
this bug but I'm not ruling it out either so try the following (untested
but should be ok) patch. It's not a proper revert, it just disables the
capture page logic to see if it's at fault.
Things look good so far with your change.
It's been running 2 hours on a VM and 1 hour on my regular machine.
Will update again in a few hours (or sooner if it's stuck again).
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>
From: Eric Wong <hidden> Date: 2013-01-08 00:21:32
Eric Dumazet [off-list ref] wrote:
It would not surprise me if sk_stream_wait_memory() have plain bug(s) or
race(s).
In 2010, in commit 482964e56e132 Nagendra Tomar fixed a pretty severe
long standing bug.
This path is not taken very often on most machines.
I would try the following patch :
From: Eric Wong <hidden> Date: 2013-01-08 20:14:20
Eric Wong [off-list ref] wrote:
Mel Gorman [off-list ref] wrote:
quoted
Right now it's difficult to see how the capture could be the source of
this bug but I'm not ruling it out either so try the following (untested
but should be ok) patch. It's not a proper revert, it just disables the
capture page logic to see if it's at fault.
Things look good so far with your change.
It's been running 2 hours on a VM and 1 hour on my regular machine.
Will update again in a few hours (or sooner if it's stuck again).
On Mon, Jan 07, 2013 at 10:38:50PM +0000, Eric Wong wrote:
Mel Gorman [off-list ref] wrote:
quoted
Right now it's difficult to see how the capture could be the source of
this bug but I'm not ruling it out either so try the following (untested
but should be ok) patch. It's not a proper revert, it just disables the
capture page logic to see if it's at fault.
Things look good so far with your change.
Ok, so minimally reverting is an option once 2e30abd1 is preserved. The
original motivation for the patch was to improve allocation success rates
under load but due to a bug in the patch the likely source of the improvement
was due to compacting more for THP allocations.
It's been running 2 hours on a VM and 1 hour on my regular machine.
Will update again in a few hours (or sooner if it's stuck again).
When I looked at it for long enough I found a number of problems. Most
affect timing but two serious issues are in there. One affects how long
kswapd spends compacting versus reclaiming and the other increases lock
contention meaning that async compaction can abort early. Both are serious
and could explain why a driver would fail high-order allocations.
Please try the following patch. However, even if it works the benefit of
capture may be so marginal that partially reverting it and simplifying
compaction.c is the better decision.
@@ -857,7 +857,8 @@ static int compact_finished(struct zone *zone,}else{unsignedintorder;for(order=cc->order;order<MAX_ORDER;order++){-structfree_area*area=&zone->free_area[cc->order];+structfree_area*area=&zone->free_area[order];+/* Job done if page is free of the right migratetype */if(!list_empty(&area->free_list[cc->migratetype]))returnCOMPACT_PARTIAL;
@@ -929,6 +930,11 @@ static void compact_capture_page(struct compact_control *cc)if(!cc->page||*cc->page)return;+/* Check that watermarks are satisifed before acquiring locks */+if(!zone_watermark_ok(cc->zone,cc->order,low_wmark_pages(cc->zone),+0,0))+return;+/**ForMIGRATE_MOVABLEallocationswecaptureasuitablepageASAP*regardlessofthemigratetypeofthefreelistisiscapturedfrom.
@@ -1118,7 +1124,6 @@ unsigned long try_to_compact_pages(struct zonelist *zonelist,structzoneref*z;structzone*zone;intrc=COMPACT_SKIPPED;-intalloc_flags=0;/* Check if the GFP flags allow compaction */if(!order||!may_enter_fs||!may_perform_io)
@@ -1126,10 +1131,6 @@ unsigned long try_to_compact_pages(struct zonelist *zonelist,count_compact_event(COMPACTSTALL);-#ifdef CONFIG_CMA-if(allocflags_to_migratetype(gfp_mask)==MIGRATE_MOVABLE)-alloc_flags|=ALLOC_CMA;-#endif/* Compact each zone in the list */for_each_zone_zonelist_nodemask(zone,z,zonelist,high_zoneidx,nodemask){
@@ -1139,9 +1140,8 @@ unsigned long try_to_compact_pages(struct zonelist *zonelist,contended,page);rc=max(status,rc);-/* If a normal allocation would succeed, stop compacting */-if(zone_watermark_ok(zone,order,low_wmark_pages(zone),0,-alloc_flags))+/* If a page was captured, stop compacting */+if(*page)break;}
@@ -2180,10 +2180,8 @@ __alloc_pages_direct_compact(gfp_t gfp_mask, unsigned int order,current->flags&=~PF_MEMALLOC;/* If compaction captured a page, prep and use it */-if(page){-prep_new_page(page,order,gfp_mask);+if(page&&!prep_new_page(page,order,gfp_mask))gotogot_page;-}if(*did_some_progress!=COMPACT_SKIPPED){/* Page migration frees to the PCP lists but we want merging */--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>
From: Eric Wong <hidden> Date: 2013-01-08 23:23:27
Mel Gorman [off-list ref] wrote:
Please try the following patch. However, even if it works the benefit of
capture may be so marginal that partially reverting it and simplifying
compaction.c is the better decision.
I already got my VM stuck on this one. I had two twosleepy instances,
2774 was the one that got stuck (also confirmed by watching top).
Btw, have you been able to reproduce this on your end?
I think the easiest reproduction on my 2-core VM is by running 2
twosleepy processes and doing the following to dirty a lot of pages:
while time find $LARGISH_NFS_MOUNT -type f -print0 | \
xargs -0 -n1 -P4 sh -c 'cat "$1" >> /tmp/z; > /tmp/z' --; do date; done
I've updated git://bogomips.org/toosleepy.git to automate the reporting
for me.
===> /proc/vmstat <===
nr_free_pages 2035
nr_inactive_anon 4044
nr_active_anon 3913
nr_inactive_file 98877
nr_active_file 4373
nr_unevictable 0
nr_mlock 0
nr_anon_pages 7839
nr_mapped 2350
nr_file_pages 103382
nr_dirty 512
nr_writeback 0
nr_slab_reclaimable 1578
nr_slab_unreclaimable 5642
nr_page_table_pages 800
nr_kernel_stack 170
nr_unstable 0
nr_bounce 0
nr_vmscan_write 0
nr_vmscan_immediate_reclaim 0
nr_writeback_temp 0
nr_isolated_anon 0
nr_isolated_file 0
nr_shmem 115
nr_dirtied 889731
nr_written 25225
nr_anon_transparent_hugepages 0
nr_free_cma 0
nr_dirty_threshold 22336
nr_dirty_background_threshold 11168
pgpgin 45284
pgpgout 101948
pswpin 0
pswpout 0
pgalloc_dma 299007
pgalloc_dma32 24235925
pgalloc_normal 0
pgalloc_movable 0
pgfree 24539843
pgactivate 5440
pgdeactivate 4476
pgfault 1072378
pgmajfault 338
pgrefill_dma 508
pgrefill_dma32 3968
pgrefill_normal 0
pgrefill_movable 0
pgsteal_kswapd_dma 22463
pgsteal_kswapd_dma32 553340
pgsteal_kswapd_normal 0
pgsteal_kswapd_movable 0
pgsteal_direct_dma 3956
pgsteal_direct_dma32 220354
pgsteal_direct_normal 0
pgsteal_direct_movable 0
pgscan_kswapd_dma 22463
pgscan_kswapd_dma32 554313
pgscan_kswapd_normal 0
pgscan_kswapd_movable 0
pgscan_direct_dma 3956
pgscan_direct_dma32 220397
pgscan_direct_normal 0
pgscan_direct_movable 0
pgscan_direct_throttle 0
pginodesteal 0
slabs_scanned 4096
kswapd_inodesteal 0
kswapd_low_wmark_hit_quickly 1726
kswapd_high_wmark_hit_quickly 21
kswapd_skip_congestion_wait 0
pageoutrun 9065
allocstall 4004
pgrotated 0
pgmigrate_success 1242
pgmigrate_fail 0
compact_migrate_scanned 141232
compact_free_scanned 181666
compact_isolated 52638
compact_stall 2024
compact_fail 1450
compact_success 574
unevictable_pgs_culled 1063
unevictable_pgs_scanned 0
unevictable_pgs_rescued 1653
unevictable_pgs_mlocked 1653
unevictable_pgs_munlocked 1652
unevictable_pgs_cleared 1
unevictable_pgs_stranded 0
thp_fault_alloc 0
thp_fault_fallback 0
thp_collapse_alloc 0
thp_collapse_alloc_failed 0
thp_split 0
thp_zero_page_alloc 0
thp_zero_page_alloc_failed 0
===> 2724[2724]/stack <===
[<ffffffff81077300>] futex_wait_queue_me+0xc0/0xf0
[<ffffffff81077a9d>] futex_wait+0x17d/0x280
[<ffffffff8107988c>] do_futex+0x11c/0xae0
[<ffffffff8107a2d8>] sys_futex+0x88/0x180
[<ffffffff813b0729>] system_call_fastpath+0x16/0x1b
[<ffffffffffffffff>] 0xffffffffffffffff
===> 2724[2725]/stack <===
[<ffffffff810f5944>] poll_schedule_timeout+0x44/0x60
[<ffffffff810f6d94>] do_sys_poll+0x374/0x4b0
[<ffffffff810f71ce>] sys_ppoll+0x19e/0x1b0
[<ffffffff813b0729>] system_call_fastpath+0x16/0x1b
[<ffffffffffffffff>] 0xffffffffffffffff
===> 2724[2726]/stack <===
[<ffffffffffffffff>] 0xffffffffffffffff
===> 2724[2727]/stack <===
[<ffffffff81310098>] sk_stream_wait_memory+0x1b8/0x250
[<ffffffff8134bea7>] tcp_sendmsg+0x697/0xd80
[<ffffffff81370d0e>] inet_sendmsg+0x5e/0xa0
[<ffffffff81300ab7>] sock_sendmsg+0x87/0xa0
[<ffffffff81303a99>] sys_sendto+0x119/0x160
[<ffffffff813b0729>] system_call_fastpath+0x16/0x1b
[<ffffffffffffffff>] 0xffffffffffffffff
===> 2724[2728]/stack <===
[<ffffffff8105d02c>] hrtimer_nanosleep+0x9c/0x150
[<ffffffff8105d13e>] sys_nanosleep+0x5e/0x80
[<ffffffff813b0729>] system_call_fastpath+0x16/0x1b
[<ffffffffffffffff>] 0xffffffffffffffff
===> 2774[2774]/stack <===
[<ffffffff81077300>] futex_wait_queue_me+0xc0/0xf0
[<ffffffff81077a9d>] futex_wait+0x17d/0x280
[<ffffffff8107988c>] do_futex+0x11c/0xae0
[<ffffffff8107a2d8>] sys_futex+0x88/0x180
[<ffffffff813b0729>] system_call_fastpath+0x16/0x1b
[<ffffffffffffffff>] 0xffffffffffffffff
===> 2774[2775]/stack <===
[<ffffffff810f5944>] poll_schedule_timeout+0x44/0x60
[<ffffffff810f6d94>] do_sys_poll+0x374/0x4b0
[<ffffffff810f71ce>] sys_ppoll+0x19e/0x1b0
[<ffffffff813b0729>] system_call_fastpath+0x16/0x1b
[<ffffffffffffffff>] 0xffffffffffffffff
===> 2774[2776]/stack <===
[<ffffffff81310098>] sk_stream_wait_memory+0x1b8/0x250
[<ffffffff8134bea7>] tcp_sendmsg+0x697/0xd80
[<ffffffff81370d0e>] inet_sendmsg+0x5e/0xa0
[<ffffffff81300ab7>] sock_sendmsg+0x87/0xa0
[<ffffffff81303a99>] sys_sendto+0x119/0x160
[<ffffffff813b0729>] system_call_fastpath+0x16/0x1b
[<ffffffffffffffff>] 0xffffffffffffffff
===> 2774[2777]/stack <===
[<ffffffff81310098>] sk_stream_wait_memory+0x1b8/0x250
[<ffffffff8134bea7>] tcp_sendmsg+0x697/0xd80
[<ffffffff81370d0e>] inet_sendmsg+0x5e/0xa0
[<ffffffff81300ab7>] sock_sendmsg+0x87/0xa0
[<ffffffff81303a99>] sys_sendto+0x119/0x160
[<ffffffff813b0729>] system_call_fastpath+0x16/0x1b
[<ffffffffffffffff>] 0xffffffffffffffff
===> 2774[2778]/stack <===
[<ffffffff810400b8>] do_wait+0x1f8/0x220
[<ffffffff81040ea0>] sys_wait4+0x70/0xf0
[<ffffffff813b0729>] system_call_fastpath+0x16/0x1b
[<ffffffffffffffff>] 0xffffffffffffffff
SysRq : Show Memory
Mem-Info:
DMA per-cpu:
CPU 0: hi: 0, btch: 1 usd: 0
CPU 1: hi: 0, btch: 1 usd: 0
DMA32 per-cpu:
CPU 0: hi: 186, btch: 31 usd: 108
CPU 1: hi: 186, btch: 31 usd: 162
active_anon:3990 inactive_anon:4042 isolated_anon:0
active_file:4362 inactive_file:98536 isolated_file:0
unevictable:0 dirty:513 writeback:0 unstable:0
free:1896 slab_reclaimable:1530 slab_unreclaimable:5661
mapped:2342 shmem:115 pagetables:784 bounce:0
free_cma:0
DMA free:2080kB min:84kB low:104kB high:124kB active_anon:0kB inactive_anon:0kB active_file:0kB inactive_file:12168kB unevictable:0kB isolated(anon):0kB isolated(file):0kB present:15644kB managed:15900kB mlocked:0kB dirty:0kB writeback:0kB mapped:0kB shmem:0kB slab_reclaimable:16kB slab_unreclaimable:192kB kernel_stack:0kB pagetables:0kB unstable:0kB bounce:0kB free_cma:0kB writeback_tmp:0kB pages_scanned:0 all_unreclaimable? no
lowmem_reserve[]: 0 488 488 488
DMA32 free:5568kB min:2784kB low:3480kB high:4176kB active_anon:15960kB inactive_anon:16168kB active_file:17448kB inactive_file:381976kB unevictable:0kB isolated(anon):0kB isolated(file):0kB present:499960kB managed:491256kB mlocked:0kB dirty:2052kB writeback:0kB mapped:9368kB shmem:460kB slab_reclaimable:6104kB slab_unreclaimable:22452kB kernel_stack:1416kB pagetables:3136kB unstable:0kB bounce:0kB free_cma:0kB writeback_tmp:0kB pages_scanned:0 all_unreclaimable? no
lowmem_reserve[]: 0 0 0 0
DMA: 6*4kB (UR) 5*8kB (UR) 2*16kB (U) 0*32kB 11*64kB (R) 2*128kB (R) 0*256kB 0*512kB 1*1024kB (R) 0*2048kB 0*4096kB = 2080kB
DMA32: 280*4kB (UEM) 66*8kB (UEM) 99*16kB (U) 40*32kB (UM) 16*64kB (M) 0*128kB 0*256kB 0*512kB 0*1024kB 0*2048kB 0*4096kB = 5536kB
103002 total pagecache pages
0 pages in swap cache
Swap cache stats: add 0, delete 0, find 0/0
Free swap = 392188kB
Total swap = 392188kB
131054 pages RAM
3820 pages reserved
411919 pages shared
116133 pages non-shared
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>
From: Eric Dumazet <hidden> Date: 2013-01-09 02:14:12
On Tue, 2013-01-08 at 23:23 +0000, Eric Wong wrote:
Mel Gorman [off-list ref] wrote:
quoted
Please try the following patch. However, even if it works the benefit of
capture may be so marginal that partially reverting it and simplifying
compaction.c is the better decision.
I already got my VM stuck on this one. I had two twosleepy instances,
2774 was the one that got stuck (also confirmed by watching top).
Btw, have you been able to reproduce this on your end?
I think the easiest reproduction on my 2-core VM is by running 2
twosleepy processes and doing the following to dirty a lot of pages:
Given the persistent sk_stream_wait_memory() traces I suspect a plain
TCP bug, triggered by some extra wait somewhere.
Please mm guys don't spend too much time right now, I'll try to
reproduce the problem.
Don't be confused by sk_stream_wait_memory() name.
A thread is stuck here because TCP stack is failing to wake it.
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>
From: Eric Dumazet <hidden> Date: 2013-01-09 02:32:33
On Tue, 2013-01-08 at 18:14 -0800, Eric Dumazet wrote:
On Tue, 2013-01-08 at 23:23 +0000, Eric Wong wrote:
quoted
Mel Gorman [off-list ref] wrote:
quoted
Please try the following patch. However, even if it works the benefit of
capture may be so marginal that partially reverting it and simplifying
compaction.c is the better decision.
I already got my VM stuck on this one. I had two twosleepy instances,
2774 was the one that got stuck (also confirmed by watching top).
Btw, have you been able to reproduce this on your end?
I think the easiest reproduction on my 2-core VM is by running 2
twosleepy processes and doing the following to dirty a lot of pages:
Given the persistent sk_stream_wait_memory() traces I suspect a plain
TCP bug, triggered by some extra wait somewhere.
Please mm guys don't spend too much time right now, I'll try to
reproduce the problem.
Don't be confused by sk_stream_wait_memory() name.
A thread is stuck here because TCP stack is failing to wake it.
Hmm, it seems sk_filter() can return -ENOMEM because skb has the
pfmemalloc() set.
It seems nobody really tested this stuff under memory stress.
Mel, it looks like you are the guy who could fix this, after all ;)
One TCP socket keeps retransmitting an SKB via loopback, and TCP stack
drops the packet again and again.
commit c93bdd0e03e848555d144eb44a1f275b871a8dd5
Author: Mel Gorman [off-list ref]
Date: Tue Jul 31 16:44:19 2012 -0700
netvm: allow skb allocation to use PFMEMALLOC reserves
Change the skb allocation API to indicate RX usage and use this to fall
back to the PFMEMALLOC reserve when needed. SKBs allocated from the
reserve are tagged in skb->pfmemalloc. If an SKB is allocated from the
reserve and the socket is later found to be unrelated to page reclaim, the
packet is dropped so that the memory remains available for page reclaim.
Network protocols are expected to recover from this packet loss.
[a.p.zijlstra@chello.nl: Ideas taken from various patches]
[davem@davemloft.net: Use static branches, coding style corrections]
[sebastian@breakpoint.cc: Avoid unnecessary cast, fix !CONFIG_NET build]
Signed-off-by: Mel Gorman [off-list ref]
Acked-by: David S. Miller [off-list ref]
Cc: Neil Brown [off-list ref]
Cc: Peter Zijlstra [off-list ref]
Cc: Mike Christie [off-list ref]
Cc: Eric B Munson [off-list ref]
Cc: Eric Dumazet [off-list ref]
Cc: Sebastian Andrzej Siewior [off-list ref]
Cc: Mel Gorman [off-list ref]
Cc: Christoph Lameter [off-list ref]
Signed-off-by: Andrew Morton [off-list ref]
Signed-off-by: Linus Torvalds [off-list ref]
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>
From: Eric Wong <hidden> Date: 2013-01-09 08:42:50
Eric Wong [off-list ref] wrote:
Eric Dumazet [off-list ref] wrote:
quoted
On Tue, 2013-01-08 at 18:32 -0800, Eric Dumazet wrote:
quoted
Hmm, it seems sk_filter() can return -ENOMEM because skb has the
pfmemalloc() set.
quoted
One TCP socket keeps retransmitting an SKB via loopback, and TCP stack
drops the packet again and again.
sock_init_data() sets sk->sk_allocation to GFP_KERNEL
Shouldnt it use (GFP_KERNEL | __GFP_NOMEMALLOC) instead ?
Thanks, things are running good after ~35 minutes so far.
Will report back if things break (hopefully I don't run out
of laptop battery power :x).
Oops, I had to restart my test :x. However, I was able to reproduce the
issue very quickly again with your patch. I've double-checked I'm
booting into the correct kernel, but I do have more load on this
laptop host now, so maybe that made it happen more quickly...
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>
From: Eric Wong <hidden> Date: 2013-01-09 08:51:29
Eric Wong [off-list ref] wrote:
Oops, I had to restart my test :x. However, I was able to reproduce the
issue very quickly again with your patch. I've double-checked I'm
booting into the correct kernel, but I do have more load on this
laptop host now, so maybe that made it happen more quickly...
On Tue, Jan 08, 2013 at 11:23:25PM +0000, Eric Wong wrote:
Mel Gorman [off-list ref] wrote:
quoted
Please try the following patch. However, even if it works the benefit of
capture may be so marginal that partially reverting it and simplifying
compaction.c is the better decision.
I already got my VM stuck on this one. I had two twosleepy instances,
2774 was the one that got stuck (also confirmed by watching top).
page->pfmemalloc can be left set for captured pages so try this but as
capture is rarely used I'm strongly favouring a partial revert even if
this works for you. I haven't reproduced this using your workload yet
but I have found that high-order allocation stress tests for 3.8-rc2 are
completely screwed. 71% success rates at rest in 3.7 and 6% in 3.8-rc2 so
I have to chase that down too.
@@ -2180,8 +2180,10 @@ __alloc_pages_direct_compact(gfp_t gfp_mask, unsigned int order,current->flags&=~PF_MEMALLOC;/* If compaction captured a page, prep and use it */-if(page&&!prep_new_page(page,order,gfp_mask))+if(page&&!prep_new_page(page,order,gfp_mask)){+page->pfmemalloc=false;gotogot_page;+}if(*did_some_progress!=COMPACT_SKIPPED){/* Page migration frees to the PCP lists but we want merging */--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>
On Tue, Jan 08, 2013 at 06:32:29PM -0800, Eric Dumazet wrote:
On Tue, 2013-01-08 at 18:14 -0800, Eric Dumazet wrote:
quoted
On Tue, 2013-01-08 at 23:23 +0000, Eric Wong wrote:
quoted
Mel Gorman [off-list ref] wrote:
quoted
Please try the following patch. However, even if it works the benefit of
capture may be so marginal that partially reverting it and simplifying
compaction.c is the better decision.
I already got my VM stuck on this one. I had two twosleepy instances,
2774 was the one that got stuck (also confirmed by watching top).
Btw, have you been able to reproduce this on your end?
I think the easiest reproduction on my 2-core VM is by running 2
twosleepy processes and doing the following to dirty a lot of pages:
Given the persistent sk_stream_wait_memory() traces I suspect a plain
TCP bug, triggered by some extra wait somewhere.
Please mm guys don't spend too much time right now, I'll try to
reproduce the problem.
Don't be confused by sk_stream_wait_memory() name.
A thread is stuck here because TCP stack is failing to wake it.
Hmm, it seems sk_filter() can return -ENOMEM because skb has the
pfmemalloc() set.
The skb should not have pfmemalloc set in most cases, particularly after
cfd19c5a (mm: only set page->pfmemalloc when ALLOC_NO_WATERMARKS was used)
but the capture patch also failed to clear pfmemalloc properly so it could
be set in error.
--
Mel Gorman
SUSE Labs
On Wed, Jan 09, 2013 at 01:37:46PM +0000, Mel Gorman wrote:
On Tue, Jan 08, 2013 at 11:23:25PM +0000, Eric Wong wrote:
quoted
Mel Gorman [off-list ref] wrote:
quoted
Please try the following patch. However, even if it works the benefit of
capture may be so marginal that partially reverting it and simplifying
compaction.c is the better decision.
I already got my VM stuck on this one. I had two twosleepy instances,
2774 was the one that got stuck (also confirmed by watching top).
page->pfmemalloc can be left set for captured pages so try this but as
capture is rarely used I'm strongly favouring a partial revert even if
this works for you.
@@ -816,6 +816,7 @@ static isolate_migrate_t isolate_migratepages(struct zone *zone,staticintcompact_finished(structzone*zone,structcompact_control*cc){+unsignedintorder;unsignedlongwatermark;if(fatal_signal_pending(current))
@@ -850,22 +851,15 @@ static int compact_finished(struct zone *zone,returnCOMPACT_CONTINUE;/* Direct compactor: Is a suitable page free? */-if(cc->page){-/* Was a suitable page captured? */-if(*cc->page)+for(order=cc->order;order<MAX_ORDER;order++){+structfree_area*area=&zone->free_area[cc->order];+/* Job done if page is free of the right migratetype */+if(!list_empty(&area->free_list[cc->migratetype]))+returnCOMPACT_PARTIAL;++/* Job done if allocation would set block type */+if(cc->order>=pageblock_order&&area->nr_free)returnCOMPACT_PARTIAL;-}else{-unsignedintorder;-for(order=cc->order;order<MAX_ORDER;order++){-structfree_area*area=&zone->free_area[cc->order];-/* Job done if page is free of the right migratetype */-if(!list_empty(&area->free_list[cc->migratetype]))-returnCOMPACT_PARTIAL;--/* Job done if allocation would set block type */-if(cc->order>=pageblock_order&&area->nr_free)-returnCOMPACT_PARTIAL;-}}returnCOMPACT_CONTINUE;
@@ -921,60 +915,6 @@ unsigned long compaction_suitable(struct zone *zone, int order)returnCOMPACT_CONTINUE;}-staticvoidcompact_capture_page(structcompact_control*cc)-{-unsignedlongflags;-intmtype,mtype_low,mtype_high;--if(!cc->page||*cc->page)-return;--/*-*ForMIGRATE_MOVABLEallocationswecaptureasuitablepageASAP-*regardlessofthemigratetypeofthefreelistisiscapturedfrom.-*Thisisfinebecausetheorderforahigh-orderMIGRATE_MOVABLE-*allocationistypicallyatleastapageblocksizeandoverall-*fragmentationisnotimpaired.Otherallocationtypesmust-*capturepagesfromtheirownmigratelistbecauseotherwisethey-*couldpolluteotherpageblockslikeMIGRATE_MOVABLEwith-*difficulttomovepagesandmakingfragmentationworseoverall.-*/-if(cc->migratetype==MIGRATE_MOVABLE){-mtype_low=0;-mtype_high=MIGRATE_PCPTYPES;-}else{-mtype_low=cc->migratetype;-mtype_high=cc->migratetype+1;-}--/* Speculatively examine the free lists without zone lock */-for(mtype=mtype_low;mtype<mtype_high;mtype++){-intorder;-for(order=cc->order;order<MAX_ORDER;order++){-structpage*page;-structfree_area*area;-area=&(cc->zone->free_area[order]);-if(list_empty(&area->free_list[mtype]))-continue;--/* Take the lock and attempt capture of the page */-if(!compact_trylock_irqsave(&cc->zone->lock,&flags,cc))-return;-if(!list_empty(&area->free_list[mtype])){-page=list_entry(area->free_list[mtype].next,-structpage,lru);-if(capture_free_page(page,cc->order,mtype)){-spin_unlock_irqrestore(&cc->zone->lock,-flags);-*cc->page=page;-return;-}-}-spin_unlock_irqrestore(&cc->zone->lock,flags);-}-}-}-staticintcompact_zone(structzone*zone,structcompact_control*cc){intret;
@@ -1054,9 +994,6 @@ static int compact_zone(struct zone *zone, struct compact_control *cc)gotoout;}}--/* Capture a page now if it is a suitable size */-compact_capture_page(cc);}out:
@@ -1080,7 +1016,6 @@ static unsigned long compact_zone_order(struct zone *zone,.migratetype=allocflags_to_migratetype(gfp_mask),.zone=zone,.sync=sync,-.page=page,};INIT_LIST_HEAD(&cc.freepages);INIT_LIST_HEAD(&cc.migratepages);
@@ -1110,7 +1045,7 @@ int sysctl_extfrag_threshold = 500;*/unsignedlongtry_to_compact_pages(structzonelist*zonelist,intorder,gfp_tgfp_mask,nodemask_t*nodemask,-boolsync,bool*contended,structpage**page)+boolsync,bool*contended){enumzone_typehigh_zoneidx=gfp_zone(gfp_mask);intmay_enter_fs=gfp_mask&__GFP_FS;
@@ -1136,7 +1071,7 @@ unsigned long try_to_compact_pages(struct zonelist *zonelist,intstatus;status=compact_zone_order(zone,order,gfp_mask,sync,-contended,page);+contended);rc=max(status,rc);/* If a normal allocation would succeed, stop compacting */
@@ -1192,7 +1127,6 @@ int compact_pgdat(pg_data_t *pgdat, int order)structcompact_controlcc={.order=order,.sync=false,-.page=NULL,};return__compact_pgdat(pgdat,&cc);
@@ -1203,7 +1137,6 @@ static int compact_node(int nid)structcompact_controlcc={.order=-1,.sync=true,-.page=NULL,};return__compact_pgdat(NODE_DATA(nid),&cc);
@@ -1413,7 +1405,7 @@ int capture_free_page(struct page *page, int alloc_order, int migratetype)if(!zone_watermark_ok(zone,0,watermark,0,0))return0;-__mod_zone_freepage_state(zone,-(1UL<<alloc_order),mt);+__mod_zone_freepage_state(zone,-(1UL<<order),mt);}/* Remove page from free list */
@@ -1421,11 +1413,7 @@ int capture_free_page(struct page *page, int alloc_order, int migratetype)zone->free_area[order].nr_free--;rmv_page_order(page);-if(alloc_order!=order)-expand(zone,page,alloc_order,order,-&zone->free_area[order],migratetype);--/* Set the pageblock if the captured page is at least a pageblock */+/* Set the pageblock if the isolated page is at least a pageblock */if(order>=pageblock_order-1){structpage*endpage=page+(1<<order)-1;for(;page<endpage;page+=pageblock_nr_pages){
@@ -1436,7 +1424,7 @@ int capture_free_page(struct page *page, int alloc_order, int migratetype)}}-return1UL<<alloc_order;+return1UL<<order;}/*
@@ -1451,13 +1439,12 @@ int capture_free_page(struct page *page, int alloc_order, int migratetype)*/intsplit_free_page(structpage*page){-unsignedintorder;+unsignedintorder=page_order(page);intnr_pages;-BUG_ON(!PageBuddy(page));order=page_order(page);-nr_pages=capture_free_page(page,order,0);+nr_pages=__isolate_free_page(page,order);if(!nr_pages)return0;
@@ -2163,8 +2150,6 @@ __alloc_pages_direct_compact(gfp_t gfp_mask, unsigned int order,bool*contended_compaction,bool*deferred_compaction,unsignedlong*did_some_progress){-structpage*page=NULL;-if(!order)returnNULL;
@@ -2176,16 +2161,12 @@ __alloc_pages_direct_compact(gfp_t gfp_mask, unsigned int order,current->flags|=PF_MEMALLOC;*did_some_progress=try_to_compact_pages(zonelist,order,gfp_mask,nodemask,sync_migration,-contended_compaction,&page);+contended_compaction);current->flags&=~PF_MEMALLOC;-/* If compaction captured a page, prep and use it */-if(page){-prep_new_page(page,order,gfp_mask);-gotogot_page;-}-if(*did_some_progress!=COMPACT_SKIPPED){+structpage*page;+/* Page migration frees to the PCP lists but we want merging */drain_pages(get_cpu());put_cpu();
@@ -2195,7 +2176,6 @@ __alloc_pages_direct_compact(gfp_t gfp_mask, unsigned int order,alloc_flags&~ALLOC_NO_WATERMARKS,preferred_zone,migratetype);if(page){-got_page:preferred_zone->compact_blockskip_flush=false;preferred_zone->compact_considered=0;preferred_zone->compact_defer_shift=0;--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>
From: Eric Wong <hidden> Date: 2013-01-09 21:29:10
Mel Gorman [off-list ref] wrote:
When I looked at it for long enough I found a number of problems. Most
affect timing but two serious issues are in there. One affects how long
kswapd spends compacting versus reclaiming and the other increases lock
contention meaning that async compaction can abort early. Both are serious
and could explain why a driver would fail high-order allocations.
Please try the following patch. However, even if it works the benefit of
capture may be so marginal that partially reverting it and simplifying
compaction.c is the better decision.
Btw, I'm still testing this patch with the "page->pfemalloc = false"
change on top of it.
@@ -857,7 +857,8 @@ static int compact_finished(struct zone *zone,}else{unsignedintorder;for(order=cc->order;order<MAX_ORDER;order++){-structfree_area*area=&zone->free_area[cc->order];+structfree_area*area=&zone->free_area[order];
I noticed something like this hunk wasn't in your latest partial revert
([off-list ref])
I admit I don't understand this code, but this jumped out at me.
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>
From: Eric Wong <hidden> Date: 2013-01-10 09:25:16
Mel Gorman [off-list ref] wrote:
quoted hunk
page->pfmemalloc can be left set for captured pages so try this but as
capture is rarely used I'm strongly favouring a partial revert even if
this works for you. I haven't reproduced this using your workload yet
but I have found that high-order allocation stress tests for 3.8-rc2 are
completely screwed. 71% success rates at rest in 3.7 and 6% in 3.8-rc2 so
I have to chase that down too.
@@ -2180,8 +2180,10 @@ __alloc_pages_direct_compact(gfp_t gfp_mask, unsigned int order,current->flags&=~PF_MEMALLOC;/* If compaction captured a page, prep and use it */-if(page&&!prep_new_page(page,order,gfp_mask))+if(page&&!prep_new_page(page,order,gfp_mask)){+page->pfmemalloc=false;gotogot_page;+}if(*did_some_progress!=COMPACT_SKIPPED){/* Page migration frees to the PCP lists but we want merging */
This (on top of your previous patch) seems to work great after several
hours of testing on both my VM and real machine. I haven't tried your
partial revert, yet. Will try that in a bit on the VM.
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>
On Thu, Jan 10, 2013 at 09:25:11AM +0000, Eric Wong wrote:
Mel Gorman [off-list ref] wrote:
quoted
page->pfmemalloc can be left set for captured pages so try this but as
capture is rarely used I'm strongly favouring a partial revert even if
this works for you. I haven't reproduced this using your workload yet
but I have found that high-order allocation stress tests for 3.8-rc2 are
completely screwed. 71% success rates at rest in 3.7 and 6% in 3.8-rc2 so
I have to chase that down too.
@@ -2180,8 +2180,10 @@ __alloc_pages_direct_compact(gfp_t gfp_mask, unsigned int order,current->flags&=~PF_MEMALLOC;/* If compaction captured a page, prep and use it */-if(page&&!prep_new_page(page,order,gfp_mask))+if(page&&!prep_new_page(page,order,gfp_mask)){+page->pfmemalloc=false;gotogot_page;+}if(*did_some_progress!=COMPACT_SKIPPED){/* Page migration frees to the PCP lists but we want merging */
This (on top of your previous patch) seems to work great after several
hours of testing on both my VM and real machine. I haven't tried your
partial revert, yet. Will try that in a bit on the VM.
Thanks Eric, it's much appreciated. However, I'm still very much in favour
of a partial revert as in retrospect the implementation of capture took the
wrong approach. Could you confirm the following patch works for you?
It's should functionally have the same effect as the first revert and
there are only minor changes from the last revert prototype I sent you
but there is no harm in being sure.
---8<---
mm: compaction: Partially revert capture of suitable high-order page
Eric Wong reported on 3.7 and 3.8-rc2 that ppoll() got stuck when waiting
for POLLIN on a local TCP socket. It was easier to trigger if there was disk
IO and dirty pages at the same time and he bisected it to commit 1fb3f8ca
"mm: compaction: capture a suitable high-order page immediately when it
is made available".
The intention of that patch was to improve high-order allocations under
memory pressure after changes made to reclaim in 3.6 drastically hurt
THP allocations but the approach was flawed. For Eric, the problem was
that page->pfmemalloc was not being cleared for captured pages leading to
a poor interaction with swap-over-NFS support causing the packets to be
dropped. However, I identified a few more problems with the patch including
the fact that it can increase contention on zone->lock in some cases which
could result in async direct compaction being aborted early.
In retrospect the capture patch took the wrong approach. What it should
have done is mark the pageblock being migrated as MIGRATE_ISOLATE if it
was allocating for THP and avoided races that way. While the patch was
showing to improve allocation success rates at the time, the benefit is
marginal given the relative complexity and it should be revisited from
scratch in the context of the other reclaim-related changes that have taken
place since the patch was first written and tested. This patch partially
reverts commit 1fb3f8ca "mm: compaction: capture a suitable high-order
page immediately when it is made available".
Reported-by: Eric Wong <redacted>
Cc: stable@vger.kernel.org
Signed-off-by: Mel Gorman <mgorman@suse.de>
---
include/linux/compaction.h | 4 +-
include/linux/mm.h | 1 -
mm/compaction.c | 92 +++++++-------------------------------------
mm/internal.h | 1 -
mm/page_alloc.c | 35 ++++-------------
5 files changed, 23 insertions(+), 110 deletions(-)
@@ -816,6 +816,7 @@ static isolate_migrate_t isolate_migratepages(struct zone *zone,staticintcompact_finished(structzone*zone,structcompact_control*cc){+unsignedintorder;unsignedlongwatermark;if(fatal_signal_pending(current))
@@ -850,22 +851,16 @@ static int compact_finished(struct zone *zone,returnCOMPACT_CONTINUE;/* Direct compactor: Is a suitable page free? */-if(cc->page){-/* Was a suitable page captured? */-if(*cc->page)+for(order=cc->order;order<MAX_ORDER;order++){+structfree_area*area=&zone->free_area[order];++/* Job done if page is free of the right migratetype */+if(!list_empty(&area->free_list[cc->migratetype]))+returnCOMPACT_PARTIAL;++/* Job done if allocation would set block type */+if(cc->order>=pageblock_order&&area->nr_free)returnCOMPACT_PARTIAL;-}else{-unsignedintorder;-for(order=cc->order;order<MAX_ORDER;order++){-structfree_area*area=&zone->free_area[cc->order];-/* Job done if page is free of the right migratetype */-if(!list_empty(&area->free_list[cc->migratetype]))-returnCOMPACT_PARTIAL;--/* Job done if allocation would set block type */-if(cc->order>=pageblock_order&&area->nr_free)-returnCOMPACT_PARTIAL;-}}returnCOMPACT_CONTINUE;
@@ -921,60 +916,6 @@ unsigned long compaction_suitable(struct zone *zone, int order)returnCOMPACT_CONTINUE;}-staticvoidcompact_capture_page(structcompact_control*cc)-{-unsignedlongflags;-intmtype,mtype_low,mtype_high;--if(!cc->page||*cc->page)-return;--/*-*ForMIGRATE_MOVABLEallocationswecaptureasuitablepageASAP-*regardlessofthemigratetypeofthefreelistisiscapturedfrom.-*Thisisfinebecausetheorderforahigh-orderMIGRATE_MOVABLE-*allocationistypicallyatleastapageblocksizeandoverall-*fragmentationisnotimpaired.Otherallocationtypesmust-*capturepagesfromtheirownmigratelistbecauseotherwisethey-*couldpolluteotherpageblockslikeMIGRATE_MOVABLEwith-*difficulttomovepagesandmakingfragmentationworseoverall.-*/-if(cc->migratetype==MIGRATE_MOVABLE){-mtype_low=0;-mtype_high=MIGRATE_PCPTYPES;-}else{-mtype_low=cc->migratetype;-mtype_high=cc->migratetype+1;-}--/* Speculatively examine the free lists without zone lock */-for(mtype=mtype_low;mtype<mtype_high;mtype++){-intorder;-for(order=cc->order;order<MAX_ORDER;order++){-structpage*page;-structfree_area*area;-area=&(cc->zone->free_area[order]);-if(list_empty(&area->free_list[mtype]))-continue;--/* Take the lock and attempt capture of the page */-if(!compact_trylock_irqsave(&cc->zone->lock,&flags,cc))-return;-if(!list_empty(&area->free_list[mtype])){-page=list_entry(area->free_list[mtype].next,-structpage,lru);-if(capture_free_page(page,cc->order,mtype)){-spin_unlock_irqrestore(&cc->zone->lock,-flags);-*cc->page=page;-return;-}-}-spin_unlock_irqrestore(&cc->zone->lock,flags);-}-}-}-staticintcompact_zone(structzone*zone,structcompact_control*cc){intret;
@@ -1054,9 +995,6 @@ static int compact_zone(struct zone *zone, struct compact_control *cc)gotoout;}}--/* Capture a page now if it is a suitable size */-compact_capture_page(cc);}out:
@@ -1080,7 +1017,6 @@ static unsigned long compact_zone_order(struct zone *zone,.migratetype=allocflags_to_migratetype(gfp_mask),.zone=zone,.sync=sync,-.page=page,};INIT_LIST_HEAD(&cc.freepages);INIT_LIST_HEAD(&cc.migratepages);
@@ -1110,7 +1046,7 @@ int sysctl_extfrag_threshold = 500;*/unsignedlongtry_to_compact_pages(structzonelist*zonelist,intorder,gfp_tgfp_mask,nodemask_t*nodemask,-boolsync,bool*contended,structpage**page)+boolsync,bool*contended){enumzone_typehigh_zoneidx=gfp_zone(gfp_mask);intmay_enter_fs=gfp_mask&__GFP_FS;
@@ -1136,7 +1072,7 @@ unsigned long try_to_compact_pages(struct zonelist *zonelist,intstatus;status=compact_zone_order(zone,order,gfp_mask,sync,-contended,page);+contended);rc=max(status,rc);/* If a normal allocation would succeed, stop compacting */
@@ -1192,7 +1128,6 @@ int compact_pgdat(pg_data_t *pgdat, int order)structcompact_controlcc={.order=order,.sync=false,-.page=NULL,};return__compact_pgdat(pgdat,&cc);
@@ -1203,7 +1138,6 @@ static int compact_node(int nid)structcompact_controlcc={.order=-1,.sync=true,-.page=NULL,};return__compact_pgdat(NODE_DATA(nid),&cc);
@@ -1404,7 +1398,6 @@ int capture_free_page(struct page *page, int alloc_order, int migratetype)BUG_ON(!PageBuddy(page));zone=page_zone(page);-order=page_order(page);mt=get_pageblock_migratetype(page);if(mt!=MIGRATE_ISOLATE){
@@ -1413,7 +1406,7 @@ int capture_free_page(struct page *page, int alloc_order, int migratetype)if(!zone_watermark_ok(zone,0,watermark,0,0))return0;-__mod_zone_freepage_state(zone,-(1UL<<alloc_order),mt);+__mod_zone_freepage_state(zone,-(1UL<<order),mt);}/* Remove page from free list */
@@ -1421,11 +1414,7 @@ int capture_free_page(struct page *page, int alloc_order, int migratetype)zone->free_area[order].nr_free--;rmv_page_order(page);-if(alloc_order!=order)-expand(zone,page,alloc_order,order,-&zone->free_area[order],migratetype);--/* Set the pageblock if the captured page is at least a pageblock */+/* Set the pageblock if the isolated page is at least a pageblock */if(order>=pageblock_order-1){structpage*endpage=page+(1<<order)-1;for(;page<endpage;page+=pageblock_nr_pages){
@@ -1436,7 +1425,7 @@ int capture_free_page(struct page *page, int alloc_order, int migratetype)}}-return1UL<<alloc_order;+return1UL<<order;}/*
@@ -1454,10 +1443,9 @@ int split_free_page(struct page *page)unsignedintorder;intnr_pages;-BUG_ON(!PageBuddy(page));order=page_order(page);-nr_pages=capture_free_page(page,order,0);+nr_pages=__isolate_free_page(page,order);if(!nr_pages)return0;
@@ -2163,8 +2151,6 @@ __alloc_pages_direct_compact(gfp_t gfp_mask, unsigned int order,bool*contended_compaction,bool*deferred_compaction,unsignedlong*did_some_progress){-structpage*page=NULL;-if(!order)returnNULL;
@@ -2176,16 +2162,12 @@ __alloc_pages_direct_compact(gfp_t gfp_mask, unsigned int order,current->flags|=PF_MEMALLOC;*did_some_progress=try_to_compact_pages(zonelist,order,gfp_mask,nodemask,sync_migration,-contended_compaction,&page);+contended_compaction);current->flags&=~PF_MEMALLOC;-/* If compaction captured a page, prep and use it */-if(page){-prep_new_page(page,order,gfp_mask);-gotogot_page;-}-if(*did_some_progress!=COMPACT_SKIPPED){+structpage*page;+/* Page migration frees to the PCP lists but we want merging */drain_pages(get_cpu());put_cpu();
@@ -2195,7 +2177,6 @@ __alloc_pages_direct_compact(gfp_t gfp_mask, unsigned int order,alloc_flags&~ALLOC_NO_WATERMARKS,preferred_zone,migratetype);if(page){-got_page:preferred_zone->compact_blockskip_flush=false;preferred_zone->compact_considered=0;preferred_zone->compact_defer_shift=0;
From: Eric Wong <hidden> Date: 2013-01-10 20:03:54
Mel Gorman [off-list ref] wrote:
Thanks Eric, it's much appreciated. However, I'm still very much in favour
of a partial revert as in retrospect the implementation of capture took the
wrong approach. Could you confirm the following patch works for you?
It's should functionally have the same effect as the first revert and
there are only minor changes from the last revert prototype I sent you
but there is no harm in being sure.
Thanks, I was just about to report back on the last partial revert
being successful :) Will start testing this one, now.
From: Eric Dumazet <hidden> Date: 2013-01-10 20:58:22
On Thu, 2013-01-10 at 19:42 +0000, Mel Gorman wrote:
Thanks Eric, it's much appreciated. However, I'm still very much in favour
of a partial revert as in retrospect the implementation of capture took the
wrong approach. Could you confirm the following patch works for you?
It's should functionally have the same effect as the first revert and
there are only minor changes from the last revert prototype I sent you
but there is no harm in being sure.
---8<---
mm: compaction: Partially revert capture of suitable high-order page
Eric Wong reported on 3.7 and 3.8-rc2 that ppoll() got stuck when waiting
for POLLIN on a local TCP socket. It was easier to trigger if there was disk
IO and dirty pages at the same time and he bisected it to commit 1fb3f8ca
"mm: compaction: capture a suitable high-order page immediately when it
is made available".
The intention of that patch was to improve high-order allocations under
memory pressure after changes made to reclaim in 3.6 drastically hurt
THP allocations but the approach was flawed. For Eric, the problem was
that page->pfmemalloc was not being cleared for captured pages leading to
a poor interaction with swap-over-NFS support causing the packets to be
dropped. However, I identified a few more problems with the patch including
the fact that it can increase contention on zone->lock in some cases which
could result in async direct compaction being aborted early.
In retrospect the capture patch took the wrong approach. What it should
have done is mark the pageblock being migrated as MIGRATE_ISOLATE if it
was allocating for THP and avoided races that way. While the patch was
showing to improve allocation success rates at the time, the benefit is
marginal given the relative complexity and it should be revisited from
scratch in the context of the other reclaim-related changes that have taken
place since the patch was first written and tested. This patch partially
reverts commit 1fb3f8ca "mm: compaction: capture a suitable high-order
page immediately when it is made available".
Reported-by: Eric Wong <redacted>
Cc: stable@vger.kernel.org
Signed-off-by: Mel Gorman <mgorman@suse.de>
---
It seems to solve the problem on my kvm testbed
(512 MB of ram, 2 vcpus)
Tested-by: Eric Dumazet <edumazet@google.com>
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>
From: Eric Wong <hidden> Date: 2013-01-11 00:51:07
Mel Gorman [off-list ref] wrote:
mm: compaction: Partially revert capture of suitable high-order page
<snip>
Reported-by: Eric Wong <redacted>
Cc: stable@vger.kernel.org
Signed-off-by: Mel Gorman <mgorman@suse.de>
Thanks, my original use case and test works great after several hours!
Tested-by: Eric Wong <redacted>
Unfortunately, I also hit a new bug in 3.8 (not in 3.7.x). based on Eric
Dumazet's observations, sk_stream_wait_memory may be to blame.
Fortunately this is easier to reproduce (I've cc-ed participants
on this thread already): [off-list ref]
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>
On Fri, Jan 11, 2013 at 12:51:05AM +0000, Eric Wong wrote:
Mel Gorman [off-list ref] wrote:
quoted
mm: compaction: Partially revert capture of suitable high-order page
<snip>
quoted
Reported-by: Eric Wong <redacted>
Cc: stable@vger.kernel.org
Signed-off-by: Mel Gorman <mgorman@suse.de>
Thanks, my original use case and test works great after several hours!
Tested-by: Eric Wong <redacted>
Thanks very much Eric. I've resent the patch to Andrew so it should make
its way to mainline. It'll fail to apply to 3.7-stable but I should get
a notification from Greg when that happens and fix it up.
Unfortunately, I also hit a new bug in 3.8 (not in 3.7.x). based on Eric
Dumazet's observations, sk_stream_wait_memory may be to blame.
Fortunately this is easier to reproduce (I've cc-ed participants
on this thread already): [off-list ref]
It looks like the relevant fix for this has already been written by Eric
Dumazet and picked up by David Miller.
--
Mel Gorman
SUSE Labs
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>