| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <20251002120320.00003ab7@huawei.com>
Date: Thu, 2 Oct 2025 12:03:20 +0100
From: Jonathan Cameron <jonathan.cameron@...wei.com>
To: Shivank Garg <shivankg@....com>
CC: <akpm@...ux-foundation.org>, <david@...hat.com>, <ziy@...dia.com>,
<willy@...radead.org>, <matthew.brost@...el.com>, <joshua.hahnjy@...il.com>,
<rakie.kim@...com>, <byungchul@...com>, <gourry@...rry.net>,
<ying.huang@...ux.alibaba.com>, <apopple@...dia.com>,
<lorenzo.stoakes@...cle.com>, <Liam.Howlett@...cle.com>, <vbabka@...e.cz>,
<rppt@...nel.org>, <surenb@...gle.com>, <mhocko@...e.com>,
<vkoul@...nel.org>, <lucas.demarchi@...el.com>, <rdunlap@...radead.org>,
<jgg@...pe.ca>, <kuba@...nel.org>, <justonli@...omium.org>,
<ivecera@...hat.com>, <dave.jiang@...el.com>, <dan.j.williams@...el.com>,
<rientjes@...gle.com>, <Raghavendra.KodsaraThimmappa@....com>,
<bharata@....com>, <alirad.malek@...corp.com>, <yiannis@...corp.com>,
<weixugc@...gle.com>, <linux-kernel@...r.kernel.org>, <linux-mm@...ck.org>
Subject: Re: [RFC V3 4/9] mm/migrate: add migrate_folios_batch_move to
batch the folio move operations
On Tue, 23 Sep 2025 17:47:39 +0000
Shivank Garg <shivankg@....com> wrote:
> This is a preparatory patch that enables batch copying for folios
> undergoing migration. By enabling batch copying the folio content, we can
> efficiently utilize the capabilities of DMA hardware or multi-threaded
> folio copy. It uses MIGRATE_NO_COPY to skip folio copy during metadata
> copy process and performed the copies in a batch later.
>
> Currently, the folio move operation is performed individually for each
> folio in sequential manner:
> for_each_folio() {
> Copy folio metadata like flags and mappings
> Copy the folio content from src to dst
> Update page tables with dst folio
> }
>
> With this patch, we transition to a batch processing approach as shown
> below:
> for_each_folio() {
> Copy folio metadata like flags and mappings
> }
> Batch copy all src folios to dst
> for_each_folio() {
> Update page tables with dst folios
> }
>
> dst->private is used to store page states and possible anon_vma value,
> thus needs to be cleared during metadata copy process. To avoid additional
> memory allocation to store the data during batch copy process, src->private
> is used to store the data after metadata copy process, since src is no
> longer used.
>
> Co-developed-by: Zi Yan <ziy@...dia.com>
> Signed-off-by: Zi Yan <ziy@...dia.com>
> Signed-off-by: Shivank Garg <shivankg@....com>
> ---
> mm/migrate.c | 197 +++++++++++++++++++++++++++++++++++++++++++++++++--
> 1 file changed, 193 insertions(+), 4 deletions(-)
>
> diff --git a/mm/migrate.c b/mm/migrate.c
> index 3fe78ecb146a..ce94e73a930d 100644
> --- a/mm/migrate.c
> +++ b/mm/migrate.c
> @@ -843,12 +843,15 @@ static int __migrate_folio(struct address_space *mapping, struct folio *dst,
> enum migrate_mode mode)
> {
> int rc, expected_count = folio_expected_ref_count(src) + 1;
> + unsigned long dst_private = (unsigned long)dst->private;
Why not just stash it in a void * and void the casts?
>
> /* Check whether src does not have extra refs before we do more work */
> if (folio_ref_count(src) != expected_count)
> return -EAGAIN;
>
> - if (mode != MIGRATE_NO_COPY) {
> + if (mode == MIGRATE_NO_COPY) {
> + dst->private = NULL;
> + } else {
> rc = folio_mc_copy(dst, src);
> if (unlikely(rc))
> return rc;
> @@ -862,6 +865,10 @@ static int __migrate_folio(struct address_space *mapping, struct folio *dst,
> folio_attach_private(dst, folio_detach_private(src));
>
> folio_migrate_flags(dst, src);
> +
> + if (mode == MIGRATE_NO_COPY)
I'd add a comment on what you mention in the commit message about this being a safe place
to stash this.
> + src->private = (void *)dst_private;
> +
> return MIGRATEPAGE_SUCCESS;
> }
>
> @@ -1149,7 +1156,7 @@ static void __migrate_folio_record(struct folio *dst,
> dst->private = (void *)anon_vma + old_page_state;
> }
>
> -static void __migrate_folio_extract(struct folio *dst,
> +static void __migrate_folio_read(struct folio *dst,
> int *old_page_state,
> struct anon_vma **anon_vmap)
> {
> @@ -1157,6 +1164,12 @@ static void __migrate_folio_extract(struct folio *dst,
>
> *anon_vmap = (struct anon_vma *)(private & ~PAGE_OLD_STATES);
> *old_page_state = private & PAGE_OLD_STATES;
> +}
Probably a blank line here.
> +static void __migrate_folio_extract(struct folio *dst,
> + int *old_page_state,
> + struct anon_vma **anon_vmap)
> +{
> + __migrate_folio_read(dst, old_page_state, anon_vmap);
> dst->private = NULL;
> }
>
> @@ -1776,6 +1789,176 @@ static void migrate_folios_move(struct list_head *src_folios,
> }
> }
>
> +static void migrate_folios_batch_move(struct list_head *src_folios,
> + struct list_head *dst_folios,
> + free_folio_t put_new_folio, unsigned long private,
> + enum migrate_mode mode, int reason,
> + struct list_head *ret_folios,
> + struct migrate_pages_stats *stats,
> + int *retry, int *thp_retry, int *nr_failed,
> + int *nr_retry_pages)
> +{
> + struct folio *folio, *folio2, *dst, *dst2;
> + int rc, nr_pages = 0, nr_batched_folios = 0;
> + int old_page_state = 0;
> + struct anon_vma *anon_vma = NULL;
> + int is_thp = 0;
Always set in each loop before use. So no need to init here that I can see.
> + LIST_HEAD(err_src);
> + LIST_HEAD(err_dst);
> + /* Batch copy the folios */
> + rc = folios_mc_copy(dst_folios, src_folios, nr_batched_folios);
> +
> + /* TODO: Is there a better way of handling the poison
> + * recover for batch copy, instead of falling back to serial copy?
Is there a reason we might expect this to be common enough to care about
not using the serial path?
> + */
> + /* fallback to serial page copy if needed */
> + if (rc) {
> + dst = list_first_entry(dst_folios, struct folio, lru);
> + dst2 = list_next_entry(dst, lru);
> + list_for_each_entry_safe(folio, folio2, src_folios, lru) {
> + is_thp = folio_test_large(folio) &&
> + folio_test_pmd_mappable(folio);
> + nr_pages = folio_nr_pages(folio);
> + rc = folio_mc_copy(dst, folio);
> +
Powered by blists - more mailing lists