[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <82e98235-4436-4f2f-901c-a25cfacd2673@os.amperecomputing.com>
Date: Mon, 2 Dec 2024 08:05:44 -0800
From: Yang Shi <yang@...amperecomputing.com>
To: Catalin Marinas <catalin.marinas@....com>
Cc: Sasha Levin <sashal@...nel.org>,
Linus Torvalds <torvalds@...ux-foundation.org>, Will Deacon
<will@...nel.org>, linux-arm-kernel@...ts.infradead.org,
linux-kernel@...r.kernel.org, David Hildenbrand <david@...hat.com>
Subject: Re: [GIT PULL] arm64 updates for 6.13-rc1
On 11/28/24 6:12 AM, Catalin Marinas wrote:
> On Wed, Nov 27, 2024 at 05:21:37PM -0800, Yang Shi wrote:
>>>> diff --git a/arch/arm64/mm/copypage.c b/arch/arm64/mm/copypage.c
>>>> index 87b3f1a25535..ef303a2262c5 100644
>>>> --- a/arch/arm64/mm/copypage.c
>>>> +++ b/arch/arm64/mm/copypage.c
>>>> @@ -30,9 +30,9 @@ void copy_highpage(struct page *to, struct page *from)
>>>> if (!system_supports_mte())
>>>> return;
>>>> - if (folio_test_hugetlb(src) &&
>>>> - folio_test_hugetlb_mte_tagged(src)) {
>>>> - if (!folio_try_hugetlb_mte_tagging(dst))
>>>> + if (folio_test_hugetlb(src)) {
>>>> + if (!folio_test_hugetlb_mte_tagged(src) ||
>>>> + !folio_try_hugetlb_mte_tagging(dst))
>>>> return;
>>>> /*
>>> I wonder why we had a 'return' here originally rather than a
>>> WARN_ON_ONCE() as we do further down for the page case. Do you seen any
>>> issue with the hunk below? Destination should be a new folio and not
>>> tagged yet:
>> Yes, I did see problem. Because we copy tags for all sub pages then set
>> folio mte tagged when copying the data for the first subpage. The warning
>> will be triggered when we copy the second subpage.
> Ah, good point, copy_highpage() will be called multiple times for each
> subpage but we only do the copying once for the folio.
>
> Now, I wonder whether we should actually defer the tag copying until
> copy_page() is called on the head page. This way we can keep the warning
> for consistency with the non-compound page case:
Yeah, we can do this. Looks fine to me.
>
> diff --git a/arch/arm64/mm/copypage.c b/arch/arm64/mm/copypage.c
> index 87b3f1a25535..a86c897017df 100644
> --- a/arch/arm64/mm/copypage.c
> +++ b/arch/arm64/mm/copypage.c
> @@ -30,11 +30,13 @@ void copy_highpage(struct page *to, struct page *from)
> if (!system_supports_mte())
> return;
>
> - if (folio_test_hugetlb(src) &&
> - folio_test_hugetlb_mte_tagged(src)) {
> - if (!folio_try_hugetlb_mte_tagging(dst))
> + if (folio_test_hugetlb(src)) {
> + if (!folio_test_hugetlb_mte_tagged(src) ||
> + from != folio_page(src, 0))
> return;
>
> + WARN_ON_ONCE(!folio_try_hugetlb_mte_tagging(dst));
> +
> /*
> * Populate tags for all subpages.
> *
>
Powered by blists - more mailing lists