Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
MIME-Version: 1.0
References: <ZKIsoMOT71uwCIZX@xsang-OptiPlex-9020> <CAHk-=wg1_8L+e09_RuX=Z_49oLn8=w82YzCk+kybS-ymSd_zbA@mail.gmail.com>
 <ZKN6CdkKyxBShPHi@xsang-OptiPlex-9020>
In-Reply-To: <ZKN6CdkKyxBShPHi@xsang-OptiPlex-9020>
From:   Linus Torvalds <torvalds@linux-foundation.org>
Date:   Mon, 3 Jul 2023 19:29:48 -0700
Message-ID: <CAHk-=wjXEzCV7HGPS=2zgJJ8R14e97hAesQ_7fjFZNS=jOADYw@mail.gmail.com>
Subject: Re: [mm] 408579cd62: WARNING:suspicious_RCU_usage
To:     Oliver Sang <oliver.sang@intel.com>
Cc:     "Liam R. Howlett" <Liam.Howlett@oracle.com>,
        LKML <linux-kernel@vger.kernel.org>, lkp@lists.01.org,
        lkp@intel.com
Content-Type: multipart/mixed; boundary="000000000000a1f7f705ffa00fe0"
Precedence: bulk

--000000000000a1f7f705ffa00fe0
Content-Type: text/plain; charset="UTF-8"

On Mon, 3 Jul 2023 at 18:48, Oliver Sang <oliver.sang@intel.com> wrote:
>
> by patch [1], we found the warning is not fixed.

Hmm. I already committed that "fix" as obvious, since the main
difference in commit 408579cd627a ("mm: Update do_vmi_align_munmap()
return semantics") around that validate_mm() call was how it did that
mmap_read_unlock().

> we also found there are some changes in stack backtrace. now it's as below:
> (detail dmesg is attached)
>
> [   26.412372][    T1] stack backtrace:
> [   26.412846][    T1] CPU: 0 PID: 1 Comm: systemd Not tainted 6.4.0-09908-gcb226fb1fb7a #1
> [   26.413506][    T1] Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS 1.16.2-debian-1.16.2-1 04/01/2014
> [   26.414326][    T1] Call Trace:
> [   26.414605][    T1]  <TASK>
> [   26.414847][    T1]  dump_stack_lvl+0x73/0xc0
> [   26.415225][    T1]  lockdep_rcu_suspicious+0x1b7/0x280
> [   26.415669][    T1]  mas_start+0x280/0x400
> [   26.416037][    T1]  mas_find+0x27a/0x400
> [   26.416391][    T1]  validate_mm+0x8b/0x2c0
> [   26.416757][    T1]  __se_sys_brk+0xa35/0xc00

Ok, that is indeed a very different stack trace.

So maybe the fix is a real fix, but the first complaint shut up
lockdep, so this is the *second* and unrelated complaint.

And indeed: it turns out that do_vma_munmap() does this:

        ret = do_vmi_align_munmap(vmi, vma, mm, start, end, uf, unlock);
        validate_mm(mm);

and so we have *another* validate_mm() that is now done outside the lock.

That one is actually pretty pointless. We've *just* validated the mm
already inside do_vmi_align_munmap(), except we only did it in one of
the two return cases.

So I think the fix is to just move that validate_mm() into the other
return case of do_vmi_align_munmap(), and remove it from the caller.

IOW, something like the attached (NOTE! This is in *addition* to the
previous patch, which is the same as the one you quoted, just with
slightly different whitespace as commit ae80b4041984: "mm: validate
the mm before dropping the mmap lock").

               Linus

--000000000000a1f7f705ffa00fe0
Content-Type: text/x-patch; charset="US-ASCII"; name="patch.diff"
Content-Disposition: attachment; filename="patch.diff"
Content-Transfer-Encoding: base64
Content-ID: <f_ljno9i7t0>
X-Attachment-Id: f_ljno9i7t0

IG1tL21tYXAuYyB8IDYgKystLS0tCiAxIGZpbGUgY2hhbmdlZCwgMiBpbnNlcnRpb25zKCspLCA0
IGRlbGV0aW9ucygtKQoKZGlmZiAtLWdpdCBhL21tL21tYXAuYyBiL21tL21tYXAuYwppbmRleCA1
NDdiNDA1MzE3OTEuLjIwNGRkY2Q1MjYyNSAxMDA2NDQKLS0tIGEvbW0vbW1hcC5jCisrKyBiL21t
L21tYXAuYwpAQCAtMjU3MSw2ICsyNTcxLDcgQEAgZG9fdm1pX2FsaWduX211bm1hcChzdHJ1Y3Qg
dm1hX2l0ZXJhdG9yICp2bWksIHN0cnVjdCB2bV9hcmVhX3N0cnVjdCAqdm1hLAogCV9fbXRfZGVz
dHJveSgmbXRfZGV0YWNoKTsKIHN0YXJ0X3NwbGl0X2ZhaWxlZDoKIG1hcF9jb3VudF9leGNlZWRl
ZDoKKwl2YWxpZGF0ZV9tbShtbSk7CiAJcmV0dXJuIGVycm9yOwogfQogCkBAIC0zMDE5LDEyICsz
MDIwLDkgQEAgaW50IGRvX3ZtYV9tdW5tYXAoc3RydWN0IHZtYV9pdGVyYXRvciAqdm1pLCBzdHJ1
Y3Qgdm1fYXJlYV9zdHJ1Y3QgKnZtYSwKIAkJYm9vbCB1bmxvY2spCiB7CiAJc3RydWN0IG1tX3N0
cnVjdCAqbW0gPSB2bWEtPnZtX21tOwotCWludCByZXQ7CiAKIAlhcmNoX3VubWFwKG1tLCBzdGFy
dCwgZW5kKTsKLQlyZXQgPSBkb192bWlfYWxpZ25fbXVubWFwKHZtaSwgdm1hLCBtbSwgc3RhcnQs
IGVuZCwgdWYsIHVubG9jayk7Ci0JdmFsaWRhdGVfbW0obW0pOwotCXJldHVybiByZXQ7CisJcmV0
dXJuIGRvX3ZtaV9hbGlnbl9tdW5tYXAodm1pLCB2bWEsIG1tLCBzdGFydCwgZW5kLCB1ZiwgdW5s
b2NrKTsKIH0KIAogLyoK
--000000000000a1f7f705ffa00fe0--