On Mon, Aug 19, 2024 at 01:31:40PM +0200, Andi Shyti wrote: > The i915 driver generates sysfs entries for each engine of the > GPU in /sys/class/drm/cardX/engines/. > > The process is straightforward: we loop over the UABI engines and > for each one, we: > > - Create the object. > - Create basic files. > - If the engine supports timeslicing, create timeslice duration files. > - If the engine supports preemption, create preemption-related files. > - Create default value files. > > Currently, if any of these steps fail, the process stops, and no > further sysfs files are created. > > However, it's not necessary to stop the process on failure. > Instead, we can continue creating the remaining sysfs files for > the other engines. Even if some files fail to be created, the > list of engines can still be retrieved by querying i915. > > Signed-off-by: Andi Shyti <andi.shyti@xxxxxxxxxxxxxxx> > --- > Hi, > > It might make sense to create an "inv-<engine_name>" if something > goes wrong, so that the user is aware that the engine exists, but > the sysfs file is not present. well, if the sysfs dir/files creation is failing, then it will probably be unreliable anyway right? > > One further improvement would be to provide more information > about thei failure reason the dev_warn() message. So, perhaps this patch should already go there and remove the dev_err and add individual dev_warn for each failing path? Also it looks something is off with the goto paths... That if (0) is also ugly... probably better to use a kobject_put with continue on every failing point as well... > > Andi > > drivers/gpu/drm/i915/gt/sysfs_engines.c | 5 ++--- > 1 file changed, 2 insertions(+), 3 deletions(-) > > diff --git a/drivers/gpu/drm/i915/gt/sysfs_engines.c b/drivers/gpu/drm/i915/gt/sysfs_engines.c > index 021f51d9b456..aab2759067d2 100644 > --- a/drivers/gpu/drm/i915/gt/sysfs_engines.c > +++ b/drivers/gpu/drm/i915/gt/sysfs_engines.c > @@ -530,9 +530,8 @@ void intel_engines_add_sysfs(struct drm_i915_private *i915) > err_object: > kobject_put(kobj); > err_engine: > - dev_err(kdev, "Failed to add sysfs engine '%s'\n", > - engine->name); > - break; > + dev_warn(kdev, "Failed to add sysfs engine '%s'\n", > + engine->name); > } > } > } > -- > 2.45.2 >
