Re: [PATCH v3 1/6] drm/debugfs: Introduce wrapper for debugfs list

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 2/8/23 15:06, Daniel Vetter wrote:
On Tue, Jan 31, 2023 at 04:58:21PM -0300, Maíra Canal wrote:
Introduce a struct wrapper for all the debugfs-related stuff: the list
of debugfs files and the mutex that protects it. This will make it
easier to initialize all the debugfs list in a DRM object and will
create a good abstraction for a possible implementation of the debugfs
infrastructure for KMS objects.

Signed-off-by: Maíra Canal <mcanal@xxxxxxxxxx>
---
  drivers/gpu/drm/drm_debugfs.c  | 18 ++++++++++++++++++
  drivers/gpu/drm/drm_internal.h | 12 ++++++++++++
  include/drm/drm_debugfs.h      | 16 ++++++++++++++++
  3 files changed, 46 insertions(+)

diff --git a/drivers/gpu/drm/drm_debugfs.c b/drivers/gpu/drm/drm_debugfs.c
index 4f643a490dc3..8658d3929ea5 100644
--- a/drivers/gpu/drm/drm_debugfs.c
+++ b/drivers/gpu/drm/drm_debugfs.c
@@ -218,6 +218,24 @@ void drm_debugfs_create_files(const struct drm_info_list *files, int count,
  }
  EXPORT_SYMBOL(drm_debugfs_create_files);
+struct drm_debugfs_files *drm_debugfs_files_init(void)
+{
+	struct drm_debugfs_files *debugfs_files;
+
+	debugfs_files = kzalloc(sizeof(*debugfs_files), GFP_KERNEL);
+
+	INIT_LIST_HEAD(&debugfs_files->list);
+	mutex_init(&debugfs_files->mutex);
+
+	return debugfs_files;
+}
+
+void drm_debugfs_files_destroy(struct drm_debugfs_files *debugfs_files)
+{
+	mutex_destroy(&debugfs_files->mutex);
+	kfree(debugfs_files);
+}
+
  int drm_debugfs_init(struct drm_minor *minor, int minor_id,
  		     struct dentry *root)
  {
diff --git a/drivers/gpu/drm/drm_internal.h b/drivers/gpu/drm/drm_internal.h
index ed2103ee272c..f1c8766ed828 100644
--- a/drivers/gpu/drm/drm_internal.h
+++ b/drivers/gpu/drm/drm_internal.h
@@ -23,6 +23,7 @@
#include <linux/kthread.h> +#include <drm/drm_debugfs.h>
  #include <drm/drm_ioctl.h>
  #include <drm/drm_vblank.h>
@@ -183,6 +184,8 @@ int drm_gem_dumb_destroy(struct drm_file *file, struct drm_device *dev, /* drm_debugfs.c drm_debugfs_crc.c */
  #if defined(CONFIG_DEBUG_FS)
+struct drm_debugfs_files *drm_debugfs_files_init(void);
+void drm_debugfs_files_destroy(struct drm_debugfs_files *debugfs_files);
  int drm_debugfs_init(struct drm_minor *minor, int minor_id,
  		     struct dentry *root);
  void drm_debugfs_cleanup(struct drm_minor *minor);
@@ -193,6 +196,15 @@ void drm_debugfs_crtc_add(struct drm_crtc *crtc);
  void drm_debugfs_crtc_remove(struct drm_crtc *crtc);
  void drm_debugfs_crtc_crc_add(struct drm_crtc *crtc);
  #else
+static inline struct drm_debugfs_files *drm_debugfs_files_init(void)
+{
+	return NULL;
+}
+
+static inline void drm_debugfs_files_destroy(struct drm_debugfs_files *debugfs_files)
+{
+}
+
  static inline int drm_debugfs_init(struct drm_minor *minor, int minor_id,
  				   struct dentry *root)
  {
diff --git a/include/drm/drm_debugfs.h b/include/drm/drm_debugfs.h
index 7616f457ce70..423aa3de506a 100644
--- a/include/drm/drm_debugfs.h
+++ b/include/drm/drm_debugfs.h
@@ -32,6 +32,8 @@
  #ifndef _DRM_DEBUGFS_H_
  #define _DRM_DEBUGFS_H_
+#include <linux/list.h>
+#include <linux/mutex.h>
  #include <linux/types.h>
  #include <linux/seq_file.h>
  /**
@@ -79,6 +81,20 @@ struct drm_info_node {
  	struct dentry *dent;
  };
+/**
+ * struct drm_debugfs_files - Encapsulates the debugfs list and its mutex
+ *
+ * This structure represents the debugfs list of files and is encapsulated
+ * with a mutex to protect the access of the list.
+ */
+struct drm_debugfs_files {
+	/** @list: List of debugfs files to be created by the DRM object. */
+	struct list_head list;
+
+	/** @mutex: Protects &list access. */
+	struct mutex mutex;

I'm not seeing any use for the mutex here? Also unless you also plan to
put like the debugfs directory pointers in this struct, I'm not sure we
need this abstraction since it's purely internal to debugfs code (so also
should really be in the headers where drivers could perhaps come up with
funny ideas).

Isn't this mutex needed to guarantee race-conditions safety when adding new
files to the list, as drm_debugfs_add_file() is called by the drivers? [1]

[1] https://cgit.freedesktop.org/drm/drm-misc/tree/drivers/gpu/drm/drm_debugfs.c#n343

Best Regards,
- Maíra Canal

-Daniel

+};
+
  /**
   * struct drm_debugfs_info - debugfs info list entry
   *
--
2.39.1





[Index of Archives]     [Linux DRI Users]     [Linux Intel Graphics]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]     [XFree86]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [XFree86]
  Powered by Linux