Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::3:2 as permitted sender) client-ip=2620:137:e000::3:2;
References: <20231204105440.61448-1-ryan.roberts@arm.com>
 <20231204105440.61448-3-ryan.roberts@arm.com>
User-agent: mu4e 1.8.13; emacs 29.1
From:   Alistair Popple <apopple@nvidia.com>
To:     Ryan Roberts <ryan.roberts@arm.com>
Cc:     Catalin Marinas <catalin.marinas@arm.com>,
        Will Deacon <will@kernel.org>,
        Ard Biesheuvel <ardb@kernel.org>,
        Marc Zyngier <maz@kernel.org>,
        Oliver Upton <oliver.upton@linux.dev>,
        James Morse <james.morse@arm.com>,
        Suzuki K Poulose <suzuki.poulose@arm.com>,
        Zenghui Yu <yuzenghui@huawei.com>,
        Andrey Ryabinin <ryabinin.a.a@gmail.com>,
        Alexander Potapenko <glider@google.com>,
        Andrey Konovalov <andreyknvl@gmail.com>,
        Dmitry Vyukov <dvyukov@google.com>,
        Vincenzo Frascino <vincenzo.frascino@arm.com>,
        Andrew Morton <akpm@linux-foundation.org>,
        Anshuman Khandual <anshuman.khandual@arm.com>,
        Matthew Wilcox <willy@infradead.org>,
        Yu Zhao <yuzhao@google.com>,
        Mark Rutland <mark.rutland@arm.com>,
        David Hildenbrand <david@redhat.com>,
        Kefeng Wang <wangkefeng.wang@huawei.com>,
        John Hubbard <jhubbard@nvidia.com>, Zi Yan <ziy@nvidia.com>,
        Barry Song <21cnbao@gmail.com>, Yang Shi <shy828301@gmail.com>,
        linux-arm-kernel@lists.infradead.org, linux-mm@kvack.org,
        linux-kernel@vger.kernel.org
Subject: Re: [PATCH v3 02/15] mm: Batch-clear PTE ranges during zap_pte_range()
Date:   Fri, 08 Dec 2023 12:30:40 +1100
In-reply-to: <20231204105440.61448-3-ryan.roberts@arm.com>
Message-ID: <87h6kta3ap.fsf@nvdebian.thelocal>
Content-Type: text/plain
MIME-Version: 1.0
X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-MessageData-0: =?us-ascii?Q?BW91A+iigNx64R+C7naPWZrUGn6YaWMe1I59dDd9rcWVqNz/6Jrrl9XKjfFy?=
 =?us-ascii?Q?yRKb+33HfsnykGn/PPFVNtQDdihg8NlJTEE8lpifdaNIgpNVAc6dqzFrm+7M?=
 =?us-ascii?Q?5+f9nI0wiZ+mBV1fgeUIDmAVsPEUc8hXlnppRXuUDCmt7RJWx8H5IP/Agw/B?=
 =?us-ascii?Q?N3eXmNNumptUoxYcvmRaNsw6wkCmhcIVAec1v+82h2hF8DjYKvlavT3ABL6Q?=
 =?us-ascii?Q?E9ZWt/AKcQCBhMCZncIxYem4Moy1YJqvs+LEhXfFbdqM2hWTNbbBNLQZbwjO?=
 =?us-ascii?Q?xvKFxhbQeRMjmV5WLUPgrYd9OR0lPg3VvPuz6+2ZIkUZ4sI/N7HZJS9bMerq?=
 =?us-ascii?Q?aY42zu37yLHf1VN2x/T4hI1c2a+/CyhVtoxP7/PVzyEnLS/DWu7glOIkYKKx?=
 =?us-ascii?Q?uPZJljPr9SvaoTBh9q8rGIijgTeADy5JXYRChVhk0XYi5KKk4w0+HYb3fKKK?=
 =?us-ascii?Q?kY3J/lYKhOUAWD0iJ+QLtjuK+jXDEuaeWaPXFd3jtJK8cMeM5MmcaR8bRw5M?=
 =?us-ascii?Q?dCd5lWa1pBp37JROqZjr2WlpL8mhhZnQF7TI0r5Y/1SdbgUqt5FRU/8sPK6U?=
 =?us-ascii?Q?Mvqzp4QxB6KdidhPIBzSGqfgbaxcD1D5yNn7f9WgBp3xECNfXIQ6oW8fGcHk?=
 =?us-ascii?Q?t28uFodr60McPLm+3XZy7FZl238ce0Wp0KjO6td26YHDX8sOZCiEVjwpGc7O?=
 =?us-ascii?Q?buKeS1pbu0vJay0v9UP53yKJtHo66pM7wFRRS1JhCUyvFCpPoWEnAngqSwNd?=
 =?us-ascii?Q?DvzwSDaPp4AR+B9BVyH+tHSlrG6+b/rtBSKnzqKRByRZrA/L9Lhww1QosxPp?=
 =?us-ascii?Q?nCP/l8LPD5d1Unj0fOw4AlpEald3Hay83YL81QvhhdWGDwfX8uAUO8waLQEt?=
 =?us-ascii?Q?+QhcFriwg8/T8CZ8Pqx4UjzwQbiqQRsy8fXIlIGu2Pa5Nnt2funk3DCpvc1+?=
 =?us-ascii?Q?RzAUMg18ilcN4d4VsfeL/FJmGTfIpvbSpGM4mXfN8iUYP6Cvvms7Ab3DGjBy?=
 =?us-ascii?Q?DprjSypvzrmK4lzhD42yBTY8nBX0i6apmn0u0vULs1lcxJaxrju17lXpMkel?=
 =?us-ascii?Q?p7534dfjjMv8VSLNnWTaw5Xfa99wbkwM92wfu9CqluyZ/tGGQfG89UgIv/RF?=
 =?us-ascii?Q?mHMd4ZouUGNxL6LgWeJYOCIs34qZ+Z+rWsUmEQnElhMx7wbXVqXAfSSfCpaV?=
 =?us-ascii?Q?101axDzcIFbBlUqm+TU2VhkYhbmYSUFaO7HQ1MPARtHNFBx0mbikEDPtPDYi?=
 =?us-ascii?Q?81540cjSh425BcyZe7xoOey2ziwENTpdlTtvjfWXFr3oIRKnCtCryRW8h1Fd?=
 =?us-ascii?Q?+LACuPmnU42c9TiuodKm36J78JNy+9K0zJtkMIi/zjhs5ICZt3mL+vQGyHZq?=
 =?us-ascii?Q?9x1s9JuHr1zLcZw5eGRfFqRKKLPdPeT/LL65/VLUMgIdtxxUsfqGR1TZcZ+t?=
 =?us-ascii?Q?DcjIzYolaGVA7nDzWEZ+XJ2I+6dBNGR6+EmCJGe3ir6Cgy+Up2iLinf78wV6?=
 =?us-ascii?Q?gPNwJmDEjs9fEsZMmW68YSitUDA5tQzfMSIsYZXVA5Z3J6cKnZeoPSLhmOQz?=
 =?us-ascii?Q?gWp42GKaGBMDRUQQsZ1LAxWyIfhKoN9StoSbNjzf?=
X-OriginatorOrg: Nvidia.com
X-MS-Exchange-CrossTenant-Network-Message-Id: 988e53ad-4cbc-43a3-bd50-08dbf78d54f7
X-MS-Exchange-CrossTenant-AuthSource: BYAPR12MB3176.namprd12.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 08 Dec 2023 01:30:59.5540
 (UTC)
X-MS-Exchange-CrossTenant-FromEntityHeader: Hosted
X-MS-Exchange-CrossTenant-Id: 43083d15-7273-40c1-b7db-39efd9ccc17a
X-MS-Exchange-CrossTenant-MailboxType: HOSTED
X-MS-Exchange-CrossTenant-UserPrincipalName: SwzqxcqGE1I35K0kU/F/JurJB/tzz4PRB6yz2zN8tpCrHXn3Wa1pvu9XMsFF+jRIuoZJHr9xL5ejXJCuENbIAQ==
X-MS-Exchange-Transport-CrossTenantHeadersStamped: BL1PR12MB5828
X-Spam-Status: No, score=-0.9 required=5.0 tests=DKIMWL_WL_HIGH,DKIM_SIGNED,
	DKIM_VALID,DKIM_VALID_AU,HEADER_FROM_DIFFERENT_DOMAINS,
	MAILING_LIST_MULTI,SPF_HELO_NONE,SPF_PASS,T_SCC_BODY_TEXT_LINE
	autolearn=unavailable autolearn_force=no version=3.4.6
X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on agentk.vger.email
Precedence: bulk
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org
X-Greylist: Sender passed SPF test, not delayed by milter-greylist-4.6.4 (agentk.vger.email [0.0.0.0]); Thu, 07 Dec 2023 17:31:18 -0800 (PST)


Ryan Roberts <ryan.roberts@arm.com> writes:

> Convert zap_pte_range() to clear a set of ptes in a batch. A given batch
> maps a physically contiguous block of memory, all belonging to the same
> folio. This will likely improve performance by a tiny amount due to
> removing duplicate calls to mark the folio dirty and accessed. And also
> provides us with a future opportunity to batch the rmap removal.
>
> However, the primary motivation for this change is to reduce the number
> of tlb maintenance operations that the arm64 backend has to perform
> during exit and other syscalls that cause zap_pte_range() (e.g. munmap,
> madvise(DONTNEED), etc.), as it is about to add transparent support for
> the "contiguous bit" in its ptes. By clearing ptes using the new
> clear_ptes() API, the backend doesn't have to perform an expensive
> unfold operation when a PTE being cleared is part of a contpte block.
> Instead it can just clear the whole block immediately.
>
> This change addresses the core-mm refactoring only, and introduces
> clear_ptes() with a default implementation that calls
> ptep_get_and_clear_full() for each pte in the range. Note that this API
> returns the pte at the beginning of the batch, but with the dirty and
> young bits set if ANY of the ptes in the cleared batch had those bits
> set; this information is applied to the folio by the core-mm. Given the
> batch is garranteed to cover only a single folio, collapsing this state

Nit: s/garranteed/guaranteed/

> does not lose any useful information.
>
> A separate change will implement clear_ptes() in the arm64 backend to
> realize the performance improvement as part of the work to enable
> contpte mappings.
>
> Signed-off-by: Ryan Roberts <ryan.roberts@arm.com>
> ---
>  include/asm-generic/tlb.h |  9 ++++++
>  include/linux/pgtable.h   | 26 ++++++++++++++++
>  mm/memory.c               | 63 ++++++++++++++++++++++++++-------------
>  mm/mmu_gather.c           | 14 +++++++++
>  4 files changed, 92 insertions(+), 20 deletions(-)

<snip>

> diff --git a/mm/mmu_gather.c b/mm/mmu_gather.c
> index 4f559f4ddd21..57b4d5f0dfa4 100644
> --- a/mm/mmu_gather.c
> +++ b/mm/mmu_gather.c
> @@ -47,6 +47,20 @@ static bool tlb_next_batch(struct mmu_gather *tlb)
>  	return true;
>  }
>  
> +unsigned int tlb_get_guaranteed_space(struct mmu_gather *tlb)
> +{
> +	struct mmu_gather_batch *batch = tlb->active;
> +	unsigned int nr_next = 0;
> +
> +	/* Allocate next batch so we can guarrantee at least one batch. */
> +	if (tlb_next_batch(tlb)) {
> +		tlb->active = batch;

Rather than calling tlb_next_batch(tlb) and then undoing some of what it
does I think it would be clearer to factor out the allocation part of
tlb_next_batch(tlb) into a separate function (eg. tlb_alloc_batch) that
you can call from both here and tlb_next_batch().

Otherwise I think this overall direction looks better than trying to
play funny games in the arch layer as it's much clearer what's going on
to core-mm code.

 - Alistair

> +		nr_next = batch->next->max;
> +	}
> +
> +	return batch->max - batch->nr + nr_next;
> +}
> +
>  #ifdef CONFIG_SMP
>  static void tlb_flush_rmap_batch(struct mmu_gather_batch *batch, struct vm_area_struct *vma)
>  {