All of lore.kernel.org
 help / color / mirror / Atom feed
From: "Ghimiray, Himal Prasad" <himal.prasad.ghimiray@intel.com>
To: Matthew Brost <matthew.brost@intel.com>
Cc: <intel-xe@lists.freedesktop.org>
Subject: Re: [PATCH v3 6/6] drm/pagemap: Add fault injection for higher-order RAM folio allocation
Date: Thu, 20 Aug 2026 15:58:16 +0530	[thread overview]
Message-ID: <3a0fd8de-6e2e-45d0-803f-a0274d4630ce@intel.com> (raw)
In-Reply-To: <aoam2CCqmye2vWxF@gsse-cloud1.jf.intel.com>



On 20-08-2026 12:33, Matthew Brost wrote:
> On Sun, Aug 16, 2026 at 08:44:21PM +0530, Ghimiray, Himal Prasad wrote:
>>
>>
>> On 06-08-2026 04:40, Matthew Brost wrote:
>>> Migrating a device-private THP back to system memory has two distinct
>>> paths in __migrate_device_pages(): the fast path where both source and
>>> destination carry MIGRATE_PFN_COMPOUND, and the fallback path where the
>>> destination could only be satisfied with order-0 folios and the source
>>> THP therefore has to be split via migrate_vma_split_unmapped_folio().
>>>
>>> The fallback path only triggers under genuine memory pressure, which
>>> makes it both rare and awkward to reproduce, yet it is the path where
>>> the interesting refcounting happens (the CPU fault holds an extra
>>> reference on the device folio taken by do_huge_pmd_device_private()).
>>>
>>> Add a fault_attr, modelled on backup_fault_inject in ttm_pool.c, that
>>> forces the higher-order allocation in
>>> drm_pagemap_migrate_populate_ram_pfn() to fail so the existing order-0
>>> fallback is taken deterministically.
>>>
>>> The attribute is exposed at /sys/kernel/debug/drm_pagemap_fault_inject
>>> and requires CONFIG_FAULT_INJECTION_DEBUG_FS. With
>>> CONFIG_FAULT_INJECTION disabled the helper compiles out to a constant
>>> false and the injection has no cost.
>>>
>>> Cc: Andrew Morton <akpm@linux-foundation.org>
>>> Cc: David Hildenbrand <david@kernel.org>
>>> Cc: Lorenzo Stoakes <ljs@kernel.org>
>>> Cc: Zi Yan <ziy@nvidia.com>
>>> Cc: Baolin Wang <baolin.wang@linux.alibaba.com>
>>> Cc: Liam R. Howlett <liam@infradead.org>
>>> Cc: Nico Pache <nico.pache@linux.dev>
>>> Cc: Ryan Roberts <ryan.roberts@arm.com>
>>> Cc: Dev Jain <dev.jain@arm.com>
>>> Cc: Barry Song <baohua@kernel.org>
>>> Cc: Lance Yang <lance.yang@linux.dev>
>>> Cc: Usama Arif <usama.arif@linux.dev>
>>> Cc: Joshua Hahn <joshua.hahnjy@gmail.com>
>>> Cc: Rakie Kim <rakie.kim@sk.com>
>>> Cc: Byungchul Park <byungchul@sk.com>
>>> Cc: Gregory Price <gourry@gourry.net>
>>> Cc: Ying Huang <ying.huang@linux.alibaba.com>
>>> Cc: Alistair Popple <apopple@nvidia.com>
>>> Cc: Balbir Singh <balbirs@nvidia.com>
>>> Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
>>> Cc: Maxime Ripard <mripard@kernel.org>
>>> Cc: Thomas Zimmermann <tzimmermann@suse.de>
>>> Cc: David Airlie <airlied@gmail.com>
>>> Cc: Simona Vetter <simona@ffwll.ch>
>>> Cc: Thomas Hellström <thomas.hellstrom@linux.intel.com>
>>> Cc: Francois Dugast <francois.dugast@intel.com>
>>> Cc: dri-devel@lists.freedesktop.org
>>> Cc: linux-mm@kvack.org
>>> Cc: linux-kernel@vger.kernel.org
>>> Assisted-by: GitHub_Copilot:claude-opus-5
>>> Signed-off-by: Matthew Brost <matthew.brost@intel.com>
>>> ---
>>>    drivers/gpu/drm/drm_pagemap.c | 36 ++++++++++++++++++++++++++++++++++-
>>>    1 file changed, 35 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/drivers/gpu/drm/drm_pagemap.c b/drivers/gpu/drm/drm_pagemap.c
>>> index 51c6f12e4256..6ae8c9aa36cc 100644
>>> --- a/drivers/gpu/drm/drm_pagemap.c
>>> +++ b/drivers/gpu/drm/drm_pagemap.c
>>> @@ -3,6 +3,7 @@
>>>     * Copyright © 2024-2025 Intel Corporation
>>>     */
>>> +#include <linux/debugfs.h>
>>>    #include <linux/dma-fence.h>
>>>    #include <linux/dma-mapping.h>
>>>    #include <linux/migrate.h>
>>> @@ -12,6 +13,27 @@
>>>    #include <drm/drm_pagemap_util.h>
>>>    #include <drm/drm_print.h>
>>> +#ifdef CONFIG_FAULT_INJECTION
>>> +#include <linux/fault-inject.h>
>>> +static DECLARE_FAULT_ATTR(migrate_to_ram_fault_inject);
>>> +
>>> +/*
>>> + * Force a higher-order destination folio allocation to fail in
>>> + * drm_pagemap_migrate_populate_ram_pfn(), exercising the order-0 fallback
>>> + * (and, in turn, the THP split path in __migrate_device_pages()) without
>>> + * having to drive the system into actual memory pressure.
>>> + */
>>> +static bool drm_pagemap_fault_inject_folio(void)
>>> +{
>>> +	return should_fail(&migrate_to_ram_fault_inject, 1);
>>> +}
>>> +#else
>>> +static bool drm_pagemap_fault_inject_folio(void)
>>> +{
>>> +	return false;
>>> +}
>>> +#endif
>>> +
>>>    /**
>>>     * DOC: Overview
>>>     *
>>> @@ -960,7 +982,9 @@ static int drm_pagemap_migrate_populate_ram_pfn(struct vm_area_struct *vas,
>>>    		if (order)
>>>    			gfp |= __GFP_NOWARN;
>>> -		if (vas)
>>> +		if (order && drm_pagemap_fault_inject_folio())
>>> +			folio = NULL;
>>> +		else if (vas)
>>>    			folio = vma_alloc_folio(gfp, order, vas, addr);
>>>    		else
>>>    			folio = folio_alloc(gfp, order);
>>> @@ -1554,6 +1578,16 @@ void drm_pagemap_destroy(struct drm_pagemap *dpagemap, bool is_atomic_or_reclaim
>>>    		kfree(dpagemap);
>>>    }
>>> +static int __init drm_pagemap_module_init(void)
>>> +{
>>> +#if defined(CONFIG_DEBUG_FS) && defined(CONFIG_FAULT_INJECTION)
>>> +	fault_create_debugfs_attr("drm_pagemap_fault_inject", NULL,
>>> +				  &migrate_to_ram_fault_inject);
>>> +#endif
>>> +	return 0;
>>> +}
>>> +module_init(drm_pagemap_module_init);
>>> +
>>>    static void drm_pagemap_exit(void)
>>>    {
>>
>> Missed fault injection debugfs removal ?
>>
>> Sashiko flags it and looks valid concern.
> 
> I checked on this and kernel wide no code seems to undo
> fault_create_debugfs_attr on module unload, nor is there a function in
> linux/fault-inject.h to undo all debugfs entries setup.

I believe the cleanup is done via standard debugfs_remove_recursive via 
passing the fault_create_debugfs_attr dir or parent. Here we have no 
parent so

dir = fault_create_debugfs_attr at init

and debugfs_remove_recursive(dir) should be sufficient during 
drm_pagemap_exit

I assume not cleaning it exit might leave the debugfs entries incase of 
module unload.
  >
> So IMO this is either everyone is kernel is doing this wrong or this is
> a non-issue. debugfs_create_file kernel doc seems to indicate all
> debugfs enteries should be removed with debugfs_remove though (?).
> Either way I'd say this out of scope for this series.
> 
> Matt
> 
>>>    	flush_work(&drm_pagemap_work);
>>


  reply	other threads:[~2026-08-20 10:28 UTC|newest]

Thread overview: 29+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-08-05 23:10 [PATCH v3 0/6] Fix device page migration in low memory fallback Matthew Brost
2026-08-05 23:10 ` [PATCH v3 1/6] mm/migrate_device: Clear stale mapping after freeing swapcache Matthew Brost
2026-08-05 23:22   ` sashiko-bot
2026-08-05 23:10 ` [PATCH v3 2/6] mm/migrate_device: Do not write past the end of the src_pfns array Matthew Brost
2026-08-05 23:25   ` sashiko-bot
2026-08-05 23:29   ` Balbir Singh
2026-08-05 23:10 ` [PATCH v3 3/6] mm/migrate_device: Fix THP splitting of a CPU faulted device private folio Matthew Brost
2026-08-06  8:10   ` Balbir Singh
2026-08-10  2:26   ` Huang, Ying
2026-08-10 19:43     ` Matthew Brost
2026-08-12  8:20       ` Huang, Ying
2026-08-12 23:33         ` Matthew Brost
2026-08-13  1:54           ` Huang, Ying
2026-08-13  8:33             ` Matthew Brost
2026-08-05 23:10 ` [PATCH v3 4/6] drm/pagemap: dma-unmap pages before handling migration errors Matthew Brost
2026-08-05 23:22   ` sashiko-bot
2026-08-16 15:04   ` Ghimiray, Himal Prasad
2026-08-05 23:10 ` [PATCH v3 5/6] drm/pagemap: Fix folio allocation fallback and use-after-put Matthew Brost
2026-08-16 15:36   ` Ghimiray, Himal Prasad
2026-08-05 23:10 ` [PATCH v3 6/6] drm/pagemap: Add fault injection for higher-order RAM folio allocation Matthew Brost
2026-08-05 23:24   ` sashiko-bot
2026-08-16 15:14   ` Ghimiray, Himal Prasad
2026-08-20  7:03     ` Matthew Brost
2026-08-20 10:28       ` Ghimiray, Himal Prasad [this message]
2026-08-20 15:22         ` Matthew Brost
2026-08-05 23:17 ` ✗ CI.checkpatch: warning for Fix device page migration in low memory fallback (rev3) Patchwork
2026-08-05 23:18 ` ✓ CI.KUnit: success " Patchwork
2026-08-06  0:02 ` ✓ Xe.CI.BAT: " Patchwork
2026-08-06  9:15 ` ✓ Xe.CI.FULL: " Patchwork

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=3a0fd8de-6e2e-45d0-803f-a0274d4630ce@intel.com \
    --to=himal.prasad.ghimiray@intel.com \
    --cc=intel-xe@lists.freedesktop.org \
    --cc=matthew.brost@intel.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.