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: <20230808020917.2230692-1-fengwei.yin@intel.com> <20230808020917.2230692-3-fengwei.yin@intel.com>
In-Reply-To: <20230808020917.2230692-3-fengwei.yin@intel.com>
From:   Yu Zhao <yuzhao@google.com>
Date:   Mon, 7 Aug 2023 20:49:19 -0600
Message-ID: <CAOUHufZnYk5BtTB9Cm0Jnf_KJPXdsLD4H7Fd4wdxP2Rm+QW+Ag@mail.gmail.com>
Subject: Re: [PATCH v2 2/3] madvise:madvise_free_huge_pmd(): don't use
 mapcount() against large folio for sharing check
To:     Yin Fengwei <fengwei.yin@intel.com>, akpm@linux-foundation.org
Cc:     linux-mm@kvack.org, linux-kernel@vger.kernel.org,
        stable@vger.kernel.org, willy@infradead.org,
        vishal.moola@gmail.com, wangkefeng.wang@huawei.com,
        minchan@kernel.org, david@redhat.com, ryan.roberts@arm.com,
        shy828301@gmail.com
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable
Precedence: bulk

On Mon, Aug 7, 2023 at 8:11=E2=80=AFPM Yin Fengwei <fengwei.yin@intel.com> =
wrote:
>
> Commit fc986a38b670 ("mm: huge_memory: convert madvise_free_huge_pmd to
> use a folio") replaced the page_mapcount() with folio_mapcount() to check
> whether the folio is shared by other mapping.
>
> It's not correct for large folios. folio_mapcount() returns the total
> mapcount of large folio which is not suitable to detect whether the folio
> is shared.
>
> Use folio_estimated_sharers() which returns a estimated number of shares.
> That means it's not 100% correct. It should be OK for madvise case here.
>
> User-visible effects is that the THP is skipped when user call madvise.
> But the correct behavior is THP should be split and processed then.
>
> NOTE: this change is a temporary fix to reduce the user-visible effects
> before the long term fix from David is ready.
>
> Fixes: fc986a38b670 ("mm: huge_memory: convert madvise_free_huge_pmd to u=
se a folio")
> Cc: stable@vger.kernel.org

Andrew, this one isn't really a bug fix but an optimization, so please
feel free to drop the Fixes and Cc tags above. (It seems to me it
doesn't hurt for stable to take it though.)

Thank you.


> Signed-off-by: Yin Fengwei <fengwei.yin@intel.com>
> Reviewed-by: Yu Zhao <yuzhao@google.com>
> Reviewed-by: Ryan Roberts <ryan.roberts@arm.com>
> ---
>  mm/huge_memory.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/mm/huge_memory.c b/mm/huge_memory.c
> index 154c210892a1..0b709d2c46c6 100644
> --- a/mm/huge_memory.c
> +++ b/mm/huge_memory.c
> @@ -1612,7 +1612,7 @@ bool madvise_free_huge_pmd(struct mmu_gather *tlb, =
struct vm_area_struct *vma,
>          * If other processes are mapping this folio, we couldn't discard
>          * the folio unless they all do MADV_FREE so let's skip the folio=
.
>          */
> -       if (folio_mapcount(folio) !=3D 1)
> +       if (folio_estimated_sharers(folio) !=3D 1)
>                 goto out;
>
>         if (!folio_trylock(folio))
> --
> 2.39.2
>