| Message ID | 20211013183601.16514-1-nirmoy.das@amd.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [v2,1/4] dri: do not check for NULL debugfs dentry | expand |
Ah there are three typos :/ s/deference/dereference for this one and for the 2nd patch as well. Regards, Nirmoy On 10/13/2021 8:35 PM, Nirmoy Das wrote: > Debugfs APIs returns encoded error on failure instead of NULL > and for drm primary/minor debugfs directories, we save the > returned value in the dentry pointer and pass it on to drm > drivers to further create debugfs files/directories. Error > conditions are handled by debugfs APIs, so no need to check > for NULL, as saved dentry pointers will either contain a > valid pointer or an error code. > > Also document this for future reference. > > CC: Maarten Lankhorst <maarten.lankhorst@linux.intel.com> > CC: Maxime Ripard <mripard@kernel.org> > CC: Thomas Zimmermann <tzimmermann@suse.de> > CC: David Airlie <airlied@linux.ie> > CC: Daniel Vetter <daniel@ffwll.ch> > > Signed-off-by: Nirmoy Das <nirmoy.das@amd.com> > --- > drivers/gpu/drm/drm_debugfs.c | 9 --------- > drivers/gpu/drm/drm_drv.c | 1 + > include/drm/drm_file.h | 28 ++++++++++++++++++++++++---- > 3 files changed, 25 insertions(+), 13 deletions(-) > > diff --git a/drivers/gpu/drm/drm_debugfs.c b/drivers/gpu/drm/drm_debugfs.c > index b0a826489488..0073854a4383 100644 > --- a/drivers/gpu/drm/drm_debugfs.c > +++ b/drivers/gpu/drm/drm_debugfs.c > @@ -272,9 +272,6 @@ static void drm_debugfs_remove_all_files(struct drm_minor *minor) > > void drm_debugfs_cleanup(struct drm_minor *minor) > { > - if (!minor->debugfs_root) > - return; > - > drm_debugfs_remove_all_files(minor); > > debugfs_remove_recursive(minor->debugfs_root); > @@ -419,9 +416,6 @@ void drm_debugfs_connector_add(struct drm_connector *connector) > struct drm_minor *minor = connector->dev->primary; > struct dentry *root; > > - if (!minor->debugfs_root) > - return; > - > root = debugfs_create_dir(connector->name, minor->debugfs_root); > connector->debugfs_entry = root; > > @@ -440,9 +434,6 @@ void drm_debugfs_connector_add(struct drm_connector *connector) > > void drm_debugfs_connector_remove(struct drm_connector *connector) > { > - if (!connector->debugfs_entry) > - return; > - > debugfs_remove_recursive(connector->debugfs_entry); > > connector->debugfs_entry = NULL; > diff --git a/drivers/gpu/drm/drm_drv.c b/drivers/gpu/drm/drm_drv.c > index 7a5097467ba5..918f302d9c43 100644 > --- a/drivers/gpu/drm/drm_drv.c > +++ b/drivers/gpu/drm/drm_drv.c > @@ -64,6 +64,7 @@ static struct idr drm_minors_idr; > */ > static bool drm_core_init_complete; > > +/* Do not deference this pointer as it will contain ERR_PTR on error. */ > static struct dentry *drm_debugfs_root; > > DEFINE_STATIC_SRCU(drm_unplug_srcu); > diff --git a/include/drm/drm_file.h b/include/drm/drm_file.h > index a3acb7ac3550..3a30fc4d8905 100644 > --- a/include/drm/drm_file.h > +++ b/include/drm/drm_file.h > @@ -69,15 +69,35 @@ enum drm_minor_type { > */ > struct drm_minor { > /* private: */ > - int index; /* Minor device number */ > - int type; /* Control or render */ > - struct device *kdev; /* Linux device */ > + /** @index: minor device number. */ > + int index; > + > + /** @type: minor device type: primary, control, render. */ > + int type; > + > + /** @kdev: Linux device pointer. */ > + struct device *kdev; > + > + /** @dev: drm device pointer. */ > struct drm_device *dev; > > + > + /** @debugfs_root: > + * > + * Dentry for /sys/kernel/debug/dri/@index debugfs dir. Do not > + * deference this pointer as it will contain ERR_PTR on error. > + */ > struct dentry *debugfs_root; > > + /** @debugfs_list: > + * > + * A list to keep track of debugfs dentries created using > + * drm_debugfs_create_files() by drm drivers. > + */ > struct list_head debugfs_list; > - struct mutex debugfs_lock; /* Protects debugfs_list. */ > + > + /** @debugfs_lock: Protects debugfs_list. */ > + struct mutex debugfs_lock; > }; > > /** > -- > 2.32.0 >
diff --git a/drivers/gpu/drm/drm_debugfs.c b/drivers/gpu/drm/drm_debugfs.c index b0a826489488..0073854a4383 100644 --- a/drivers/gpu/drm/drm_debugfs.c +++ b/drivers/gpu/drm/drm_debugfs.c @@ -272,9 +272,6 @@ static void drm_debugfs_remove_all_files(struct drm_minor *minor) void drm_debugfs_cleanup(struct drm_minor *minor) { - if (!minor->debugfs_root) - return; - drm_debugfs_remove_all_files(minor); debugfs_remove_recursive(minor->debugfs_root); @@ -419,9 +416,6 @@ void drm_debugfs_connector_add(struct drm_connector *connector) struct drm_minor *minor = connector->dev->primary; struct dentry *root; - if (!minor->debugfs_root) - return; - root = debugfs_create_dir(connector->name, minor->debugfs_root); connector->debugfs_entry = root; @@ -440,9 +434,6 @@ void drm_debugfs_connector_add(struct drm_connector *connector) void drm_debugfs_connector_remove(struct drm_connector *connector) { - if (!connector->debugfs_entry) - return; - debugfs_remove_recursive(connector->debugfs_entry); connector->debugfs_entry = NULL; diff --git a/drivers/gpu/drm/drm_drv.c b/drivers/gpu/drm/drm_drv.c index 7a5097467ba5..918f302d9c43 100644 --- a/drivers/gpu/drm/drm_drv.c +++ b/drivers/gpu/drm/drm_drv.c @@ -64,6 +64,7 @@ static struct idr drm_minors_idr; */ static bool drm_core_init_complete; +/* Do not deference this pointer as it will contain ERR_PTR on error. */ static struct dentry *drm_debugfs_root; DEFINE_STATIC_SRCU(drm_unplug_srcu); diff --git a/include/drm/drm_file.h b/include/drm/drm_file.h index a3acb7ac3550..3a30fc4d8905 100644 --- a/include/drm/drm_file.h +++ b/include/drm/drm_file.h @@ -69,15 +69,35 @@ enum drm_minor_type { */ struct drm_minor { /* private: */ - int index; /* Minor device number */ - int type; /* Control or render */ - struct device *kdev; /* Linux device */ + /** @index: minor device number. */ + int index; + + /** @type: minor device type: primary, control, render. */ + int type; + + /** @kdev: Linux device pointer. */ + struct device *kdev; + + /** @dev: drm device pointer. */ struct drm_device *dev; + + /** @debugfs_root: + * + * Dentry for /sys/kernel/debug/dri/@index debugfs dir. Do not + * deference this pointer as it will contain ERR_PTR on error. + */ struct dentry *debugfs_root; + /** @debugfs_list: + * + * A list to keep track of debugfs dentries created using + * drm_debugfs_create_files() by drm drivers. + */ struct list_head debugfs_list; - struct mutex debugfs_lock; /* Protects debugfs_list. */ + + /** @debugfs_lock: Protects debugfs_list. */ + struct mutex debugfs_lock; }; /**
Debugfs APIs returns encoded error on failure instead of NULL and for drm primary/minor debugfs directories, we save the returned value in the dentry pointer and pass it on to drm drivers to further create debugfs files/directories. Error conditions are handled by debugfs APIs, so no need to check for NULL, as saved dentry pointers will either contain a valid pointer or an error code. Also document this for future reference. CC: Maarten Lankhorst <maarten.lankhorst@linux.intel.com> CC: Maxime Ripard <mripard@kernel.org> CC: Thomas Zimmermann <tzimmermann@suse.de> CC: David Airlie <airlied@linux.ie> CC: Daniel Vetter <daniel@ffwll.ch> Signed-off-by: Nirmoy Das <nirmoy.das@amd.com> --- drivers/gpu/drm/drm_debugfs.c | 9 --------- drivers/gpu/drm/drm_drv.c | 1 + include/drm/drm_file.h | 28 ++++++++++++++++++++++++---- 3 files changed, 25 insertions(+), 13 deletions(-) -- 2.32.0