Intel-XE Archive on lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] drm/xe/xe_device_sysfs: add cleanup action in xe_device_sysfs_init
@ 2025-09-10 18:32 Zongyao Bai
  2025-09-10 18:38 ` ✗ CI.checkpatch: warning for drm/xe/xe_device_sysfs: add cleanup action in xe_device_sysfs_init (rev2) Patchwork
                   ` (9 more replies)
  0 siblings, 10 replies; 19+ messages in thread
From: Zongyao Bai @ 2025-09-10 18:32 UTC (permalink / raw)
  To: intel-xe; +Cc: Zongyao Bai, Stuart Summers, Shuicheng Lin, Lucas De Marchi

xe_device_sysfs_fini(): switch the order of late_bind_remove_files
and sysfs_remove_files to follow the rule that files create latest
deleted first.(LIFO)

xe_device_sysfs_init(): on partial failure, any sysfs files created
before the failure will be cleaned up via xe_device_sysfs_fini,
ensuring no leaked sysfs entries
Add function description comments for xe_device_sysfs_init().

v2:
- Refine code using "goto cleanup" to reduced code duplication and
  improve clarity and scalability. (Stuart)

- Change remove order in late_bind_remove_files(). (Stuart)

- In xe_device_sysfs_fini() and late_bind_remove_files(), cancle the
  capable, platform and pcode/runtime check. And just remove in reverse
  order of creation(LIFO). sysfs_remove_file is safe even the attribute
  doesn't exist, so it avoids extra overhead or error paths during cleanup.

Fixes: 0e414bf7ad01 ("drm/xe: Expose PCIe link downgrade attributes")
Fixes: cdc36b66cd41 (drm/xe: Expose fan control and voltage regulator version)
Cc: Stuart Summers <stuart.summers@intel.com>
Cc: Shuicheng Lin <shuicheng.lin@intel.com>
Cc: Lucas De Marchi <lucas.demarchi@intel.com>
Signed-off-by: Zongyao Bai <zongyao.bai@intel.com>
---
 drivers/gpu/drm/xe/xe_device_sysfs.c | 61 +++++++++++++++-------------
 1 file changed, 33 insertions(+), 28 deletions(-)

diff --git a/drivers/gpu/drm/xe/xe_device_sysfs.c b/drivers/gpu/drm/xe/xe_device_sysfs.c
index 6ee422594b56..44ee008ea3ce 100644
--- a/drivers/gpu/drm/xe/xe_device_sysfs.c
+++ b/drivers/gpu/drm/xe/xe_device_sysfs.c
@@ -184,25 +184,13 @@ static int late_bind_create_files(struct device *dev)
 
 static void late_bind_remove_files(struct device *dev)
 {
-	struct xe_device *xe = pdev_to_xe_device(to_pci_dev(dev));
-	struct xe_tile *root = xe_device_get_root_tile(xe);
-	u32 cap = 0;
-	int ret;
-
-	xe_pm_runtime_get(xe);
-
-	ret = xe_pcode_read(root, PCODE_MBOX(PCODE_LATE_BINDING, GET_CAPABILITY_STATUS, 0),
-			    &cap, NULL);
-	if (ret)
-		goto out;
-
-	if (REG_FIELD_GET(V1_FAN_SUPPORTED, cap))
-		sysfs_remove_file(&dev->kobj, &dev_attr_lb_fan_control_version.attr);
-
-	if (REG_FIELD_GET(VR_PARAMS_SUPPORTED, cap))
-		sysfs_remove_file(&dev->kobj, &dev_attr_lb_voltage_regulator_version.attr);
-out:
-	xe_pm_runtime_put(xe);
+	/* Remove in reverse order of creation (LIFO).
+	* Avoid querying pcode/runtime pm here — sysfs_remove_file is safe
+	* when the attribute doesn't exist, and this avoids extra overhead
+	* or error paths during cleanup.
+	*/
+	sysfs_remove_file(&dev->kobj, &dev_attr_lb_voltage_regulator_version.attr);
+	sysfs_remove_file(&dev->kobj, &dev_attr_lb_fan_control_version.attr);
 }
 
 /**
@@ -287,16 +275,28 @@ static const struct attribute *auto_link_downgrade_attrs[] = {
 static void xe_device_sysfs_fini(void *arg)
 {
 	struct xe_device *xe = arg;
+	struct device *dev = xe->drm.dev;
 
-	if (xe->d3cold.capable)
-		sysfs_remove_file(&xe->drm.dev->kobj, &dev_attr_vram_d3cold_threshold.attr);
-
-	if (xe->info.platform == XE_BATTLEMAGE) {
-		sysfs_remove_files(&xe->drm.dev->kobj, auto_link_downgrade_attrs);
-		late_bind_remove_files(xe->drm.dev);
-	}
+	/* Remove in reverse order of creation (LIFO).
+	 * Avoid attrs check here — sysfs_remove_file is safe
+	 */
+	late_bind_remove_files(dev);
+	sysfs_remove_files(&dev->kobj, auto_link_downgrade_attrs);
+	sysfs_remove_file(&dev->kobj, &dev_attr_vram_d3cold_threshold.attr);
 }
 
+/**
+ * xe_device_sysfs_init - Initialize sysfs attributes for the Xe device
+ * @xe: Pointer to the Xe device structure
+ *
+ * Creates and registers sysfs attributes for device-specific controls and status,
+ * including VRAM D3cold threshold and PCIe link downgrade capabilities, depending
+ * on device capabilities and platform type.
+ *
+ * On partial failure, any sysfs files created before the failure will be cleaned up
+ * via xe_device_sysfs_fini.
+ */
+
 int xe_device_sysfs_init(struct xe_device *xe)
 {
 	struct device *dev = xe->drm.dev;
@@ -311,12 +311,17 @@ int xe_device_sysfs_init(struct xe_device *xe)
 	if (xe->info.platform == XE_BATTLEMAGE) {
 		ret = sysfs_create_files(&dev->kobj, auto_link_downgrade_attrs);
 		if (ret)
-			return ret;
+			goto cleanup;
 
 		ret = late_bind_create_files(dev);
 		if (ret)
-			return ret;
+			goto cleanup;
 	}
 
 	return devm_add_action_or_reset(dev, xe_device_sysfs_fini, xe);
+
+cleanup:
+	xe_device_sysfs_fini(xe);
+	return ret;
+
 }
-- 
2.34.1


^ permalink raw reply related	[flat|nested] 19+ messages in thread
end of thread, other threads:[~2025-09-16 15:07 UTC | newest]

Thread overview: 19+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-09-10 18:32 [PATCH] drm/xe/xe_device_sysfs: add cleanup action in xe_device_sysfs_init Zongyao Bai
2025-09-10 18:38 ` ✗ CI.checkpatch: warning for drm/xe/xe_device_sysfs: add cleanup action in xe_device_sysfs_init (rev2) Patchwork
2025-09-10 18:39 ` ✓ CI.KUnit: success " Patchwork
2025-09-10 19:12 ` ✓ Xe.CI.BAT: " Patchwork
2025-09-11  2:24 ` ✗ Xe.CI.Full: failure " Patchwork
2025-09-11  4:11 ` [PATCH] drm/xe/xe_device_sysfs: add cleanup action in xe_device_sysfs_init Lucas De Marchi
2025-09-15 21:47 ` [PATCH v3 0/2] drm/xe/xe_device_sysfs: modify sysfs clean up process Zongyao Bai
2025-09-15 21:47   ` [PATCH v3 1/2] drm/xe/xe_device_sysfs: Add cleanup action in xe_device_sysfs_init Zongyao Bai
2025-09-15 22:34     ` Lin, Shuicheng
2025-09-16 14:29     ` Michal Wajdeczko
2025-09-16 15:06     ` (subset) " Lucas De Marchi
2025-09-15 21:47   ` [PATCH v3 2/2] drm/xe/xe_device_sysfs: Remove sysfs unconditionally while clean sysfs entries Zongyao Bai
2025-09-15 22:46     ` Lin, Shuicheng
2025-09-16 14:39     ` Michal Wajdeczko
2025-09-16 15:03       ` Lucas De Marchi
2025-09-15 21:55 ` ✗ CI.checkpatch: warning for drm/xe/xe_device_sysfs: add cleanup action in xe_device_sysfs_init (rev3) Patchwork
2025-09-15 21:56 ` ✓ CI.KUnit: success " Patchwork
2025-09-15 22:34 ` ✓ Xe.CI.BAT: " Patchwork
2025-09-16  3:29 ` ✗ Xe.CI.Full: failure " Patchwork

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox