| Message ID | c6ed1ca0-3e39-714c-9590-54e13695b9b9@redhat.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Performance regression introduced by commit b667b8673443 ("pipe: Advance tail pointer inside of wait spinlock in pipe_read()") | expand |
On 1/17/20 12:29 PM, Waiman Long wrote: > On 1/17/20 12:05 PM, Linus Torvalds wrote: >> [ on mobile, sorry for html crud ] >> >> On Fri, Jan 17, 2020, 08:53 Waiman Long <longman@redhat.com >> <mailto:longman@redhat.com>> wrote: >> >> >> I had found that parallel kernel build became much slower when a >> 5.5-based kernel is used. On a 2-socket 96-thread x86-64 system, the >> "make -j88" time increased from less than 3 minutes with the 5.4 >> kernel >> to more than double with the 5.5 kernel. >> >> >> I suspect you may have hit the same bug in the GNU make jobserver >> that I did. >> >> It's timing-sensitive, and under the right circumstances the make >> jobserver loses job tickets to other jobservers that have a child >> that died, but they are blocked waiting for a new ticket, so they >> aren't releasing (or re-using) the one that the child death would >> free up. >> >> End result: a big lack of parallelism, and a much slower build. >> >> GNU make v4.2.1 is buggy. The fix was done over two years ago, but >> there hasn't been a new release since then, so a lot of distributions >> have the buggy version.. >> >> The fix is commit b552b05 ("[SV 51159] Use a non-blocking read with >> pselect to avoid hangs.") In the make the git tree. >> >> >> Linus > > Thanks for the information. > > Yes, I did use make v4.2.1 which is the version that is shipped in > RHEL8. I will build new make and try it. > > Thanks, > Longman > I built a make with the lastest make git tree and the problem was gone with the new make. So it was a bug in make not the kernel. Sorry for the noise. Cheers, Longman
On Fri, Jan 17, 2020 at 10:11 AM Waiman Long <longman@redhat.com> wrote: > > I built a make with the lastest make git tree and the problem was gone > with the new make. So it was a bug in make not the kernel. Sorry for the > noise. I think I spent about three days trying to figure it out. At least it felt that way. I looked at the pipe code a _lot_, also blaming the kernel for obvious reasons. Linus
On Fri, Jan 17, 2020 at 10:11 AM Waiman Long <longman@redhat.com> wrote: > > On 1/17/20 12:29 PM, Waiman Long wrote: > > On 1/17/20 12:05 PM, Linus Torvalds wrote: > >> GNU make v4.2.1 is buggy. The fix was done over two years ago, but > >> there hasn't been a new release since then, so a lot of distributions > >> have the buggy version.. > >> > >> The fix is commit b552b05 ("[SV 51159] Use a non-blocking read with > >> pselect to avoid hangs.") In the make the git tree. > >> Linus > > > > Yes, I did use make v4.2.1 which is the version that is shipped in > > RHEL8. I will build new make and try it. > > Longman > > > I built a make with the lastest make git tree and the problem was gone > with the new make. So it was a bug in make not the kernel. Sorry for the > noise. > > Longman If you are using RHEL8, building your own make is the only solution at this time. There is a bugzilla entry filed for this make bug but the progress is slow: https://bugzilla.redhat.com/show_bug.cgi?id=1774790 The same bug in Fedora make was dealt with fairly quickly, thanks to the great "pressure" from Linus. ;-) Akemi
diff --git a/fs/pipe.c b/fs/pipe.c index 69afeab8a73a..ea134f69a292 100644 --- a/fs/pipe.c +++ b/fs/pipe.c @@ -325,9 +325,14 @@ pipe_read(struct kiocb *iocb, struct iov_iter *to) if (!buf->len) { pipe_buf_release(pipe, buf); + spin_lock_irq(&pipe->wait.lock); tail++; pipe->tail = tail; - do_wakeup = 1; + do_wakeup = 0; + wake_up_interruptible_sync_poll_locked( + &pipe->wait, EPOLLOUT | EPOLLWRNORM); + spin_unlock_irq(&pipe->wait.lock); + kill_fasync(&pipe->fasync_writers, SIGIO, POLL_O }
David, I had found that parallel kernel build became much slower when a 5.5-based kernel is used. On a 2-socket 96-thread x86-64 system, the "make -j88" time increased from less than 3 minutes with the 5.4 kernel to more than double with the 5.5 kernel. So I used bisection to try to find the culprit: b667b867344301e24f21d4a4c844675ff61d89e1 is the first bad commit commit b667b867344301e24f21d4a4c844675ff61d89e1 Author: David Howells <dhowells@redhat.com> Date: Tue Sep 24 16:09:04 2019 +0100 pipe: Advance tail pointer inside of wait spinlock in pipe_read() Advance the pipe ring tail pointer inside of wait spinlock in pipe_read() so that the pipe can be written into with kernel notifications from contexts where pipe->mutex cannot be taken. Signed-off-by: David Howells <dhowells@redhat.com> I guess the make command may make heavy use of pipe. The adding of spinlock code in your patch may probably over-serialize the pipe operation. Could you achieve the same functionality without adding a lock? Cheers, Longman