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;
Feedback-ID: ie3994620:Fastmail
Date:   Fri, 24 Mar 2023 16:05:30 +0300
From:   "Kirill A. Shutemov" <kirill@shutemov.name>
To:     Michal Hocko <mhocko@suse.com>
Cc:     Linus Torvalds <torvalds@linux-foundation.org>,
        Naresh Kamboju <naresh.kamboju@linaro.org>,
        Joel Fernandes <joel@joelfernandes.org>,
        Andrew Morton <akpm@linux-foundation.org>, linux-mm@kvack.org,
        LKML <linux-kernel@vger.kernel.org>
Subject: Re: WARN_ON in move_normal_pmd
Message-ID: <20230324130530.xsmqcxapy4j2aaik@box.shutemov.name>
References: <ZB2GTBD/LWTrkOiO@dhcp22.suse.cz>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <ZB2GTBD/LWTrkOiO@dhcp22.suse.cz>
Precedence: bulk

On Fri, Mar 24, 2023 at 12:15:24PM +0100, Michal Hocko wrote:
> Hi,
> our QA is regularly hitting
> [  544.198822][T20518] WARNING: CPU: 1 PID: 20518 at ../mm/mremap.c:255 move_pgt_entry+0x4c6/0x510
> triggered by thp01 LTP test. This has been brought up in the past and
> resulted in f81fdd0c4ab7 ("mm: document warning in move_normal_pmd() and
> make it warn only once"). While it is good that the underlying problem
> is understood, it doesn't seem there is enough interest to fix it
> properly. Which is fair but I am wondering whether the WARN_ON gives
> us anything here.
> 
> Our QA process collects all unexpected side effects of tests and a WARN*
> in the log is certainly one of those. This trigger bugs which are mostly
> ignored as there is no upstream fix for them. This alone is nothing
> really critical but there are workloads which do run with panic on warn
> configured and this issue would put the system down which is unnecessary
> IMHO. Would it be sufficient to replace the WARN_ON_ONCE by
> pr_warn_once?

What about relaxing the check to exclude temporary stack from the WARN
condition:

diff --git a/mm/mremap.c b/mm/mremap.c
index 411a85682b58..eb0778b9d645 100644
--- a/mm/mremap.c
+++ b/mm/mremap.c
@@ -247,15 +247,12 @@ static bool move_normal_pmd(struct vm_area_struct *vma, unsigned long old_addr,
 	 * of any 4kB pages, but still there) PMD in the page table
 	 * tree.
 	 *
-	 * Warn on it once - because we really should try to figure
-	 * out how to do this better - but then say "I won't move
-	 * this pmd".
-	 *
-	 * One alternative might be to just unmap the target pmd at
-	 * this point, and verify that it really is empty. We'll see.
+	 * Warn on it once unless it is initial (temporary) stack.
 	 */
-	if (WARN_ON_ONCE(!pmd_none(*new_pmd)))
+	if (!pmd_none(*new_pmd)) {
+		WARN_ON_ONCE(!vma_is_temporary_stack(vma));
 		return false;
+	}

 	/*
 	 * We don't have to worry about the ordering of src and dst

-- 
  Kiryl Shutsemau / Kirill A. Shutemov