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;
From:   "Huang, Ying" <ying.huang@intel.com>
To:     Bharata B Rao <bharata@amd.com>
Cc:     <linux-mm@kvack.org>, <linux-kernel@vger.kernel.org>,
        Andrew Morton <akpm@linux-foundation.org>,
        Zi Yan <ziy@nvidia.com>, Yang Shi <shy828301@gmail.com>,
        Baolin Wang <baolin.wang@linux.alibaba.com>,
        "Oscar Salvador" <osalvador@suse.de>,
        Matthew Wilcox <willy@infradead.org>
Subject: Re: [RFC 0/6] migrate_pages(): batch TLB flushing
References: <20220921060616.73086-1-ying.huang@intel.com>
        <477e50ab-9045-0ca2-6979-e2dca71be263@amd.com>
        <87bkr6jzmz.fsf@yhuang6-desk2.ccr.corp.intel.com>
        <b9fc8a52-74b5-2f78-fbf6-8b473c6a8a36@amd.com>
Date:   Wed, 28 Sep 2022 09:46:01 +0800
In-Reply-To: <b9fc8a52-74b5-2f78-fbf6-8b473c6a8a36@amd.com> (Bharata B. Rao's
        message of "Tue, 27 Sep 2022 16:16:42 +0530")
Message-ID: <878rm42rvq.fsf@yhuang6-desk2.ccr.corp.intel.com>
User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/27.1 (gnu/linux)
MIME-Version: 1.0
Content-Type: text/plain; charset=ascii
Precedence: bulk

Bharata B Rao <bharata@amd.com> writes:

> On 9/23/2022 1:22 PM, Huang, Ying wrote:
>> Bharata B Rao <bharata@amd.com> writes:
>>>
>>> Thanks for the patchset. I find it hitting the following BUG() when
>>> running mmtests/autonumabench:
>>>
>>> kernel BUG at mm/migrate.c:2432!
>>>
>>> This is BUG_ON(!list_empty(&migratepages)) in migrate_misplaced_page().
>> 
>> Thank you very much for reporting!  I haven't reproduced this yet.  But
>> I will pay special attention to this when develop the next version, even
>> if I cannot reproduce this finally.
>
> The following change fixes the above reported BUG_ON().
>
> diff --git a/mm/migrate.c b/mm/migrate.c
> index a0de0d9b4d41..c11dd82245e5 100644
> --- a/mm/migrate.c
> +++ b/mm/migrate.c
> @@ -1197,7 +1197,7 @@ static int migrate_page_unmap(new_page_t get_new_page, free_page_t put_new_page,
>          * references and be restored.
>          */
>         /* restore the page to right list. */
> -       if (rc != -EAGAIN)
> +       if (rc == -EAGAIN)
>                  ret = NULL;
>  
>         migrate_page_undo_page(page, page_was_mapped, anon_vma, locked, ret);
>
> The pages that returned from unmapping stage with -EAGAIN used to
> end up on "ret" list rather than continuing on the "from" list.

Yes.  You are right.  Thank you very much!

Digging some history, it is found that the code was correct in previous
versions, but became wrong for mistake during code rebasing.  Will be
more careful in the future and try to organize the patchset better to
make it easier to review the changes.

Best Regards,
Huang, Ying