Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
MIME-Version: 1.0
References: <000000000000886dbd05b7ffa8db@google.com> <20210104124153.0992b1f7fd1a145e193a333f@linux-foundation.org>
 <CAHk-=wi6hd8ATJ1W90goTxjgyvuoFsf0xZdAJmZ2c0dx5wcJSg@mail.gmail.com>
 <alpine.LSU.2.11.2101041839440.3466@eggly.anvils> <CAHk-=wi36CBggdRfdggACvf2hG+djM9kKnorrwsByN6uDvPExA@mail.gmail.com>
 <CAHk-=wh=5kDGukMs2sVZ8uHZJX4VL13oD5+xMAR4HvuY6QckLg@mail.gmail.com>
In-Reply-To: <CAHk-=wh=5kDGukMs2sVZ8uHZJX4VL13oD5+xMAR4HvuY6QckLg@mail.gmail.com>
From:   Linus Torvalds <torvalds@linux-foundation.org>
Date:   Fri, 8 Jan 2021 18:04:21 -0800
Message-ID: <CAHk-=wgD9GK5CeHopYmRHoYS9cNuCmDMsc=+MbM_KgJ0KB+=ng@mail.gmail.com>
Subject: Re: kernel BUG at mm/page-writeback.c:LINE!
To:     Hugh Dickins <hughd@google.com>
Cc:     Andrew Morton <akpm@linux-foundation.org>,
        Matthew Wilcox <willy@infradead.org>,
        syzbot <syzbot+2fc0712f8f8b8b8fa0ef@syzkaller.appspotmail.com>,
        Linux Kernel Mailing List <linux-kernel@vger.kernel.org>,
        Linux-MM <linux-mm@kvack.org>,
        syzkaller-bugs <syzkaller-bugs@googlegroups.com>
Content-Type: multipart/mixed; boundary="0000000000004c20ba05b86e1822"
Precedence: bulk

--0000000000004c20ba05b86e1822
Content-Type: text/plain; charset="UTF-8"

On Tue, Jan 5, 2021 at 11:53 AM Linus Torvalds
<torvalds@linux-foundation.org> wrote:
>
> I took your "way to go" statement as an ack, and made it all be commit
> c2407cf7d22d ("mm: make wait_on_page_writeback() wait for multiple
> pending writebacks").

Oh, and Michael Larabel (of phoronix) reports that that one-liner does
something bad to a few PostgreSQL tests, on the order of 5-10%
regression on some machines (but apparently not others).

I suspect that's a sign of instability in the benchmark numbers, but
it probably also means that we have some silly condition where
multiple threads want to clean the same page.

I sent him a patch to try if it ends up being better to just not wake
things up early at all (instead of the "if" -> "while") conversion.
That trivial patch appended here in case anybody has comments.

Just the fact that that one-liner made a performance impact makes me
go "hmm", though. Michael didn't see the BUG_ON(), so it's presumably
some _other_ user of wait_on_page_writeback() than the
write_cache_pages() one that causes issues.

Anybody got any suspicions? Honestly, when working on the page wait
queues, I was working under the assumption that it's really just the
page lock that truly matters.

I'm thinking things like __filemap_fdatawait_range(), which doesn't
hold the page lock at all, so it's all kinds of non-serialized, and
could now be waiting for any number of IO's ro complete..

Oh well. This email doesn't really have a point, it's more of a
heads-up that that "wait to see one or multiple writebacks" thing
seems to matter more than I would have expected for some loads..

            Linus

--0000000000004c20ba05b86e1822
Content-Type: application/octet-stream; name=patch
Content-Disposition: attachment; filename=patch
Content-Transfer-Encoding: base64
Content-ID: <f_kjp1dr5k0>
X-Attachment-Id: f_kjp1dr5k0

IG1tL2ZpbGVtYXAuYyAgICAgICAgfCA2ICsrKystLQogbW0vcGFnZS13cml0ZWJhY2suYyB8IDIg
Ky0KIDIgZmlsZXMgY2hhbmdlZCwgNSBpbnNlcnRpb25zKCspLCAzIGRlbGV0aW9ucygtKQoKZGlm
ZiAtLWdpdCBhL21tL2ZpbGVtYXAuYyBiL21tL2ZpbGVtYXAuYwppbmRleCA1YzlkNTY0MzE3YTUu
LjAzMWQxOWQwZjRiMiAxMDA2NDQKLS0tIGEvbW0vZmlsZW1hcC5jCisrKyBiL21tL2ZpbGVtYXAu
YwpAQCAtMTA1MiwxNCArMTA1MiwxNiBAQCBzdGF0aWMgaW50IHdha2VfcGFnZV9mdW5jdGlvbih3
YWl0X3F1ZXVlX2VudHJ5X3QgKndhaXQsIHVuc2lnbmVkIG1vZGUsIGludCBzeW5jLAogCWlmICgh
d2FrZV9wYWdlX21hdGNoKHdhaXRfcGFnZSwga2V5KSkKIAkJcmV0dXJuIDA7CiAKKwkvKiBTdG9w
IHdhbGtpbmcgaWYgdGhlIGJpdCB3YXMgYWxyZWFkeSB0YWtlbiAqLworCWlmICh0ZXN0X2JpdChr
ZXktPmJpdF9uciwgJmtleS0+cGFnZS0+ZmxhZ3MpKQorCQlyZXR1cm4gLTE7CisKIAkvKgogCSAq
IElmIGl0J3MgYSBsb2NrIGhhbmRvZmYgd2FpdCwgd2UgZ2V0IHRoZSBiaXQgZm9yIGl0LCBhbmQK
IAkgKiBzdG9wIHdhbGtpbmcgKGFuZCBkbyBub3Qgd2FrZSBpdCB1cCkgaWYgd2UgY2FuJ3QuCiAJ
ICovCiAJZmxhZ3MgPSB3YWl0LT5mbGFnczsKIAlpZiAoZmxhZ3MgJiBXUV9GTEFHX0VYQ0xVU0lW
RSkgewotCQlpZiAodGVzdF9iaXQoa2V5LT5iaXRfbnIsICZrZXktPnBhZ2UtPmZsYWdzKSkKLQkJ
CXJldHVybiAtMTsKIAkJaWYgKGZsYWdzICYgV1FfRkxBR19DVVNUT00pIHsKIAkJCWlmICh0ZXN0
X2FuZF9zZXRfYml0KGtleS0+Yml0X25yLCAma2V5LT5wYWdlLT5mbGFncykpCiAJCQkJcmV0dXJu
IC0xOwpkaWZmIC0tZ2l0IGEvbW0vcGFnZS13cml0ZWJhY2suYyBiL21tL3BhZ2Utd3JpdGViYWNr
LmMKaW5kZXggZWIzNGQyMDRkNGVlLi41ODYwNDI0NzJhYzkgMTAwNjQ0Ci0tLSBhL21tL3BhZ2Ut
d3JpdGViYWNrLmMKKysrIGIvbW0vcGFnZS13cml0ZWJhY2suYwpAQCAtMjgyNiw3ICsyODI2LDcg
QEAgRVhQT1JUX1NZTUJPTChfX3Rlc3Rfc2V0X3BhZ2Vfd3JpdGViYWNrKTsKICAqLwogdm9pZCB3
YWl0X29uX3BhZ2Vfd3JpdGViYWNrKHN0cnVjdCBwYWdlICpwYWdlKQogewotCXdoaWxlIChQYWdl
V3JpdGViYWNrKHBhZ2UpKSB7CisJaWYgKFBhZ2VXcml0ZWJhY2socGFnZSkpIHsKIAkJdHJhY2Vf
d2FpdF9vbl9wYWdlX3dyaXRlYmFjayhwYWdlLCBwYWdlX21hcHBpbmcocGFnZSkpOwogCQl3YWl0
X29uX3BhZ2VfYml0KHBhZ2UsIFBHX3dyaXRlYmFjayk7CiAJfQo=
--0000000000004c20ba05b86e1822--