From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from gabe.freedesktop.org (gabe.freedesktop.org [131.252.210.177]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.lore.kernel.org (Postfix) with ESMTPS id 00A37D6D232 for ; Thu, 18 Dec 2025 16:21:57 +0000 (UTC) Received: from gabe.freedesktop.org (localhost [127.0.0.1]) by gabe.freedesktop.org (Postfix) with ESMTP id 8E6C410EA3C; Thu, 18 Dec 2025 16:21:57 +0000 (UTC) Authentication-Results: gabe.freedesktop.org; dkim=pass (2048-bit key; unprotected) header.d=intel.com header.i=@intel.com header.b="czhqCq4L"; dkim-atps=neutral Received: from mgamail.intel.com (mgamail.intel.com [192.198.163.13]) by gabe.freedesktop.org (Postfix) with ESMTPS id D2BF210EA35; Thu, 18 Dec 2025 16:21:55 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=intel.com; i=@intel.com; q=dns/txt; s=Intel; t=1766074916; x=1797610916; h=from:to:cc:subject:date:message-id:in-reply-to: references:mime-version:content-transfer-encoding; bh=zXrBibmD8dfuuhx1aBEaLGuRlUWI5vMBXQhbM2b+jIw=; b=czhqCq4LqlJB9VrgTfAQs9dQ3svnq8HhlN7NE5YdnWGcHA+SvUC++iJd 9g0WqSyqH6fXXD+Jn/G+x7caKk7EmE2zR6w4LzFfJWhmJuDqJ87/Ezb+9 lrc1eZEiunJAxS88W8hSS5ZkhkMeDKJQB+8yN9+ZYORaaeZ/ADK4Bccsq mnvCYyhNC2q1N6yRR78djckdSdm+KYnsnMfPZXlm8cx3HgybACeFd63YM WE9HQhKwZ1v2gbipMRw9Fc5iuf1giMoeGpmCLr0Ke9n825QYPAj4UUe3d 10Uy2h+TlYzigWfy8IVRDJ7VWgzgJJkU/RyL3pvvQ2CvIlBramnHXR/PG Q==; X-CSE-ConnectionGUID: d6cTPC4jS1mzPAOvBgIXLw== X-CSE-MsgGUID: D3+BEINYQ66+BV6Zk+wEIg== X-IronPort-AV: E=McAfee;i="6800,10657,11646"; a="70607590" X-IronPort-AV: E=Sophos;i="6.21,158,1763452800"; d="scan'208";a="70607590" Received: from orviesa005.jf.intel.com ([10.64.159.145]) by fmvoesa107.fm.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 18 Dec 2025 08:21:56 -0800 X-CSE-ConnectionGUID: ejg8kK/9SNmo6kt6s04LUA== X-CSE-MsgGUID: pVswdQ1WRZC73v3doh5OZA== X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="6.21,158,1763452800"; d="scan'208";a="203705589" Received: from dhhellew-desk2.ger.corp.intel.com (HELO fedora) ([10.245.244.93]) by orviesa005-auth.jf.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 18 Dec 2025 08:21:52 -0800 From: =?UTF-8?q?Thomas=20Hellstr=C3=B6m?= To: intel-xe@lists.freedesktop.org Cc: =?UTF-8?q?Thomas=20Hellstr=C3=B6m?= , Matthew Brost , dri-devel@lists.freedesktop.org, himal.prasad.ghimiray@intel.com, apopple@nvidia.com, airlied@gmail.com, Simona Vetter , felix.kuehling@amd.com, =?UTF-8?q?Christian=20K=C3=B6nig?= , dakr@kernel.org, "Mrozek, Michal" , Joonas Lahtinen Subject: [PATCH v5 06/24] drm/pagemap, drm/xe: Manage drm_pagemap provider lifetimes Date: Thu, 18 Dec 2025 17:20:43 +0100 Message-ID: <20251218162101.605379-7-thomas.hellstrom@linux.intel.com> X-Mailer: git-send-email 2.51.1 In-Reply-To: <20251218162101.605379-1-thomas.hellstrom@linux.intel.com> References: <20251218162101.605379-1-thomas.hellstrom@linux.intel.com> MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit X-BeenThere: intel-xe@lists.freedesktop.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Intel Xe graphics driver List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: intel-xe-bounces@lists.freedesktop.org Sender: "Intel-xe" If a device holds a reference on a foregin device's drm_pagemap, and a device unbind is executed on the foreign device, Typically that foreign device would evict its device-private pages and then continue its device-managed cleanup eventually releasing its drm device and possibly allow for module unload. However, since we're still holding a reference on a drm_pagemap, when that reference is released and the provider module is unloaded we'd execute out of undefined memory. Therefore keep a reference on the provider device and module until the last drm_pagemap reference is gone. Note that in theory, the drm_gpusvm_helper module may be unloaded as soon as the final module_put() of the provider driver module is executed, so we need to add a module_exit() function that waits for the work item executing the module_put() has completed. v2: - Better commit message (Matt Brost) Signed-off-by: Thomas Hellström Reviewed-by: Matthew Brost --- drivers/gpu/drm/drm_pagemap.c | 101 ++++++++++++++++++++++++++++++++-- drivers/gpu/drm/xe/xe_svm.c | 15 ++++- include/drm/drm_pagemap.h | 10 +++- 3 files changed, 117 insertions(+), 9 deletions(-) diff --git a/drivers/gpu/drm/drm_pagemap.c b/drivers/gpu/drm/drm_pagemap.c index 4cc57ced4993..97a2b5561ab5 100644 --- a/drivers/gpu/drm/drm_pagemap.c +++ b/drivers/gpu/drm/drm_pagemap.c @@ -9,6 +9,7 @@ #include #include #include +#include /** * DOC: Overview @@ -549,16 +550,92 @@ static int drm_pagemap_migrate_populate_ram_pfn(struct vm_area_struct *vas, return -ENOMEM; } +static void drm_pagemap_dev_unhold_work(struct work_struct *work); +static LLIST_HEAD(drm_pagemap_unhold_list); +static DECLARE_WORK(drm_pagemap_work, drm_pagemap_dev_unhold_work); + +/** + * struct drm_pagemap_dev_hold - Struct to aid in drm_device release. + * @link: Link into drm_pagemap_unhold_list for deferred reference releases. + * @drm: drm device to put. + * + * When a struct drm_pagemap is released, we also need to release the + * reference it holds on the drm device. However, typically that needs + * to be done separately from a system-wide workqueue. + * Each time a struct drm_pagemap is initialized + * (or re-initialized if cached) therefore allocate a separate + * drm_pagemap_dev_hold item, from which we put the drm device and + * associated module. + */ +struct drm_pagemap_dev_hold { + struct llist_node link; + struct drm_device *drm; +}; + static void drm_pagemap_release(struct kref *ref) { struct drm_pagemap *dpagemap = container_of(ref, typeof(*dpagemap), ref); - + struct drm_pagemap_dev_hold *dev_hold = dpagemap->dev_hold; + + /* + * We know the pagemap provider is alive at this point, since + * the struct drm_pagemap_dev_hold holds a reference to the + * pagemap provider drm_device and its module. + */ + dpagemap->dev_hold = NULL; kfree(dpagemap); + llist_add(&dev_hold->link, &drm_pagemap_unhold_list); + schedule_work(&drm_pagemap_work); + /* + * Here, either the provider device is still alive, since if called from + * page_free(), the caller is holding a reference on the dev_pagemap, + * or if called from drm_pagemap_put(), the direct caller is still alive. + * This ensures we can't race with THIS module unload. + */ +} + +static void drm_pagemap_dev_unhold_work(struct work_struct *work) +{ + struct llist_node *node = llist_del_all(&drm_pagemap_unhold_list); + struct drm_pagemap_dev_hold *dev_hold, *next; + + /* + * Deferred release of drm_pagemap provider device and module. + * THIS module is kept alive during the release by the + * flush_work() in the drm_pagemap_exit() function. + */ + llist_for_each_entry_safe(dev_hold, next, node, link) { + struct drm_device *drm = dev_hold->drm; + struct module *module = drm->driver->fops->owner; + + drm_dbg(drm, "Releasing reference on provider device and module.\n"); + drm_dev_put(drm); + module_put(module); + kfree(dev_hold); + } +} + +static struct drm_pagemap_dev_hold * +drm_pagemap_dev_hold(struct drm_pagemap *dpagemap) +{ + struct drm_pagemap_dev_hold *dev_hold; + struct drm_device *drm = dpagemap->drm; + + dev_hold = kzalloc(sizeof(*dev_hold), GFP_KERNEL); + if (!dev_hold) + return ERR_PTR(-ENOMEM); + + init_llist_node(&dev_hold->link); + dev_hold->drm = drm; + (void)try_module_get(drm->driver->fops->owner); + drm_dev_get(drm); + + return dev_hold; } /** * drm_pagemap_create() - Create a struct drm_pagemap. - * @dev: Pointer to a struct device providing the device-private memory. + * @drm: Pointer to a struct drm_device providing the device-private memory. * @pagemap: Pointer to a pre-setup struct dev_pagemap providing the struct pages. * @ops: Pointer to the struct drm_pagemap_ops. * @@ -568,20 +645,28 @@ static void drm_pagemap_release(struct kref *ref) * Error pointer on error. */ struct drm_pagemap * -drm_pagemap_create(struct device *dev, +drm_pagemap_create(struct drm_device *drm, struct dev_pagemap *pagemap, const struct drm_pagemap_ops *ops) { struct drm_pagemap *dpagemap = kzalloc(sizeof(*dpagemap), GFP_KERNEL); + struct drm_pagemap_dev_hold *dev_hold; if (!dpagemap) return ERR_PTR(-ENOMEM); kref_init(&dpagemap->ref); - dpagemap->dev = dev; + dpagemap->drm = drm; dpagemap->ops = ops; dpagemap->pagemap = pagemap; + dev_hold = drm_pagemap_dev_hold(dpagemap); + if (IS_ERR(dev_hold)) { + kfree(dpagemap); + return ERR_CAST(dev_hold); + } + dpagemap->dev_hold = dev_hold; + return dpagemap; } EXPORT_SYMBOL(drm_pagemap_create); @@ -933,3 +1018,11 @@ int drm_pagemap_populate_mm(struct drm_pagemap *dpagemap, return err; } EXPORT_SYMBOL(drm_pagemap_populate_mm); + +static void drm_pagemap_exit(void) +{ + flush_work(&drm_pagemap_work); + if (WARN_ON(!llist_empty(&drm_pagemap_unhold_list))) + disable_work_sync(&drm_pagemap_work); +} +module_exit(drm_pagemap_exit); diff --git a/drivers/gpu/drm/xe/xe_svm.c b/drivers/gpu/drm/xe/xe_svm.c index b72d523a4416..b72bc989e7ee 100644 --- a/drivers/gpu/drm/xe/xe_svm.c +++ b/drivers/gpu/drm/xe/xe_svm.c @@ -1479,7 +1479,7 @@ xe_drm_pagemap_device_map(struct drm_pagemap *dpagemap, unsigned int order, enum dma_data_direction dir) { - struct device *pgmap_dev = dpagemap->dev; + struct device *pgmap_dev = dpagemap->drm->dev; enum drm_interconnect_protocol prot; dma_addr_t addr; @@ -1499,6 +1499,14 @@ static const struct drm_pagemap_ops xe_drm_pagemap_ops = { .populate_mm = xe_drm_pagemap_populate_mm, }; +static void xe_devm_release(void *data) +{ + struct xe_vram_region *vr = data; + + drm_pagemap_put(vr->dpagemap); + vr->dpagemap = NULL; +} + /** * xe_devm_add: Remap and provide memmap backing for device memory * @tile: tile that the memory region belongs to @@ -1524,7 +1532,7 @@ int xe_devm_add(struct xe_tile *tile, struct xe_vram_region *vr) return ret; } - vr->dpagemap = drm_pagemap_create(dev, &vr->pagemap, + vr->dpagemap = drm_pagemap_create(&xe->drm, &vr->pagemap, &xe_drm_pagemap_ops); if (IS_ERR(vr->dpagemap)) { drm_err(&xe->drm, "Failed to create drm_pagemap tile %d memory: %pe\n", @@ -1532,6 +1540,9 @@ int xe_devm_add(struct xe_tile *tile, struct xe_vram_region *vr) ret = PTR_ERR(vr->dpagemap); goto out_no_dpagemap; } + ret = devm_add_action_or_reset(dev, xe_devm_release, vr); + if (ret) + goto out_no_dpagemap; vr->pagemap.type = MEMORY_DEVICE_PRIVATE; vr->pagemap.range.start = res->start; diff --git a/include/drm/drm_pagemap.h b/include/drm/drm_pagemap.h index 093e7199c44b..b3edcdde4454 100644 --- a/include/drm/drm_pagemap.h +++ b/include/drm/drm_pagemap.h @@ -10,6 +10,7 @@ struct dma_fence; struct drm_pagemap; +struct drm_pagemap_dev_hold; struct drm_pagemap_zdd; struct device; @@ -131,14 +132,17 @@ struct drm_pagemap_ops { * used for device p2p handshaking. * @ops: The struct drm_pagemap_ops. * @ref: Reference count. - * @dev: The struct drevice owning the device-private memory. + * @drm: The struct drm device owning the device-private memory. * @pagemap: Pointer to the underlying dev_pagemap. + * @dev_hold: Pointer to a struct drm_pagemap_dev_hold for + * device referencing. */ struct drm_pagemap { const struct drm_pagemap_ops *ops; struct kref ref; - struct device *dev; + struct drm_device *drm; struct dev_pagemap *pagemap; + struct drm_pagemap_dev_hold *dev_hold; }; struct drm_pagemap_devmem; @@ -213,7 +217,7 @@ struct drm_pagemap_devmem_ops { struct dma_fence *pre_migrate_fence); }; -struct drm_pagemap *drm_pagemap_create(struct device *dev, +struct drm_pagemap *drm_pagemap_create(struct drm_device *drm, struct dev_pagemap *pagemap, const struct drm_pagemap_ops *ops); -- 2.51.1