[RFC 0/4] Introduce drmfs pseudo filesystem for drm subsystem

[RFC 0/4] Introduce drmfs pseudo filesystem for drm subsystem

[swati.dhingra]

From: Swati Dhingra <swati.dhingra@intel.com>

Currently, we don't have a stable ABI which can be used for the purpose of
providing output debug/loggging/crc and other such data from DRM.
The ABI in current use (filesystems, ioctls, et al.) have their own
constraints and are intended to output a particular type of data.
Few cases in point:
sysfs	- stable ABI, but constrained to one textual value per file
debugfs	- unstable ABI, free-for-all
ioctls	- not as suitable to many single purpose continuous data
	  dumping, we would very quickly run out ioctl space; requires more
	  userspace support than "cat"
device nodes -  a real possibilty, kernel instantiation is more tricky,
		requires udev (+udev.rules) or userspace discovery of the
		dynamic major:minor (via sysfs) [mounting a registered
		filesystem is easy in comparison]
netlink	- stream based, therefore involves numerous copies.

Debugfs is the lesser among the evils here, thereby we have grown used to the
convenience and flexibility in presentation that debugfs gives us
(including relayfs inodes) that we want some of that hierachy in stable user
ABI form.

Due to these limitations, there is a need for a new pseudo filesytem, that
would act as a stable 'free-for-all' ABI, with the heirarchial structure and
thus convenience of debugfs. This will be managed by drm, thus named 'drmfs'.
DRM would register this filesystem to manage a canonical mountpoint, but this
wouldn't limit everyone to only using that pseudofs underneath.

This can serve to hold various kinds of output data from Linux DRM subsystems,
for the files which can't truely fit anywhere else with existing ABI's but
present so, for the lack of a better place.

In this patch series, we have introduced a pseudo filesystem named as 'drmfs'
for now. The filesystem is introduced in the first patch, and the subsequent
patches make use of the filesystem interfaces, in drm driver, and making them
available for use by the drm subsystem components, one of which is i915.
We've moved the location of i915 GuC logs from debugfs to drmfs in the last
patch. Subsequently, more such files such as pipe_crc, error states, memory
stats, etc. can be move to this filesystem, if the idea introduced here is
acceptable per se. The filesystem introduced is being used to house the data
generated by i915 driver in this patch series, but will hopefully be generic
enough to provide scope for usage by any other drm subsystem component.

The patch series is being floated as RFC to gather feedback on the idea and
infrastructure proposed here and it's suitability to address the specific
problem statement/use case.

v2: fix the bat failures caused due to missing config check

v3: Changes made:
    - Move the location of drmfs from fs/ to drivers/gpu/drm/ (Chris)
    - Moving config checks to header (Chris,Daniel)

v4: Added the kernel Documentaion (using Sphinx).

Sourab Gupta (4):
  drm: Introduce drmfs pseudo filesystem interfaces
  drm: Register drmfs filesystem from drm init
  drm: Create driver specific root directory inside drmfs
  drm/i915: Creating guc log file in drmfs instead of debugfs

 Documentation/gpu/drm-uapi.rst             |  76 ++++
 drivers/gpu/drm/Kconfig                    |   9 +
 drivers/gpu/drm/Makefile                   |   1 +
 drivers/gpu/drm/drm_drv.c                  |  26 ++
 drivers/gpu/drm/drmfs.c                    | 566 +++++++++++++++++++++++++++++
 drivers/gpu/drm/i915/i915_guc_submission.c |  33 +-
 include/drm/drm_drv.h                      |   3 +
 include/drm/drmfs.h                        |  77 ++++
 include/uapi/linux/magic.h                 |   3 +
 9 files changed, 773 insertions(+), 21 deletions(-)
 create mode 100644 drivers/gpu/drm/drmfs.c
 create mode 100644 include/drm/drmfs.h

-- 
2.7.4

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[RFC 1/4] drm: Introduce drmfs pseudo filesystem interfaces

[swati.dhingra]

From: Sourab Gupta <sourab.gupta@intel.com>

The patch introduces a new pseudo filesystem type, named 'drmfs' which is
intended to house the files for the data generated by drm subsystem that
cannot be accommodated by any of the existing filesystems.
The filesystem is modelled on the lines of existing pseudo-filesystems such
as debugfs/tracefs, and borrows ideas from their implementation.
This filesystem will be appearing at sys/kernel/drm.

A new config 'CONFIG_DRMFS' is introduced to enable/disable the filesystem,
which is dependent on CONFIG_DRM.
The filesystem will not be registered standalone during kernel init time,
instead it is intended to be initialized/registered during drm initialization.

The intent for introduction of the filesystem is to act as a location to hold
various kinds of data output from Linux DRM subsystems, which can't really fit
anywhere else into the existing filesystems such as debugfs/sysfs etc. All these
filesystems have their own constraints and are intended to output a particular
type of data such as attributes and small debug parameter data. Due to these
constraints, there is a need for a new pseudo filesytem, customizable to DRM
specific requirements and catering to the needs to DRM subsystem components

Signed-off-by: Sourab Gupta <sourab.gupta@intel.com>
Signed-off-by: Swati Dhingra <swati.dhingra@intel.com>
---
 Documentation/gpu/drm-uapi.rst |  76 ++++++
 drivers/gpu/drm/Kconfig        |   9 +
 drivers/gpu/drm/Makefile       |   1 +
 drivers/gpu/drm/drmfs.c        | 566 +++++++++++++++++++++++++++++++++++++++++
 include/drm/drmfs.h            |  77 ++++++
 include/uapi/linux/magic.h     |   3 +
 6 files changed, 732 insertions(+)
 create mode 100644 drivers/gpu/drm/drmfs.c
 create mode 100644 include/drm/drmfs.h

diff --git a/Documentation/gpu/drm-uapi.rst b/Documentation/gpu/drm-uapi.rst
index de3ac9f..be9540c 100644
--- a/Documentation/gpu/drm-uapi.rst
+++ b/Documentation/gpu/drm-uapi.rst
@@ -222,3 +222,79 @@ Testing and validation
 
 .. kernel-doc:: drivers/gpu/drm/drm_debugfs_crc.c
    :doc: CRC ABI
+
+DRMFS
+=====
+
+Overview
+--------
+
+Drmfs is stable ABI for housing data generated by drm subsystem components.
+
+The ABI in current use (debugfs/sysfs/ioctls) have their own constraints and
+are designed to output their own particular data types.
+
+Few cases in point::
+
+       sysfs   - stable ABI, but constrained to one textual value per file
+       debugfs - unstable ABI, free-for-all
+       ioctls  - not as suitable to many single purpose continuous data
+                 dumping, we would very quickly run out ioctl space; require
+                 more userspace support than "cat"
+       device nodes -  a real possibilty, kernel instantiation is more tricky,
+                       requires udev (+udev.rules) or userspace discovery of
+                       the dynamic major:minor (via sysfs) [mounting a
+                       registered filesystem is easy in comparison]
+       netlink - stream based, therefore involves numerous copies
+
+Debugfs is the lesser among the evils here, thereby we have grown used to the
+convenience and flexibility in presentation that debugfs gives us (including
+relayfs inodes) that we want some of that hierarchy in stable user ABI form.
+
+Due to these limitations, a new pseudo filesytem is needed that would act as a
+stable ‘free-for-all’ ABI, with the heirarchial structure and thus provide
+convenience of debugfs. This to be managed by drm, thus named **drmfs**. The
+filesystem is modelled on the lines of existing pseudo-filesystems such as
+debugfs/tracefs, and borrows ideas from their implementation. DRM would register
+this filesystem to manage a canonical mountpoint, but this wouldn’t limit
+everyone to only use that pseudofs underneath.
+
+One of the usecase is to hold the GuC (micro-controller) logs.Currently, relayfs
+is being used to provide these logs to userspace. Relayfs needs a parent dentry
+to be associated with log file, and also these logs should be available in 
+production kernels. In absence of any other suitable candidate, debugfs is used 
+currently, since it provides the hierarchial VFS structures available (dentry et al.).
+Instead, drmfs provides a suitable stable ABI for GuC logs, which is modelled on
+the lines of debugfs but controlled fully by drm.
+
+This can serve to hold various kinds of output data from Linux DRM subsystems,
+for the files which can’t truly fit anywhere else with existing ABI’s but present
+so, for the lack of a better place. For e.g. i915 GuC logs, pipe_crc, error
+states, memory stats.
+
+Interfaces
+----------
+
+This section covers the infrastructure of drmfs pseudo filesystem. It introduces
+interfaces to create/remove files and directories.
+
+.. kernel-doc:: drivers/gpu/drm/drmfs.c
+   :functions: drmfs_create_file
+
+.. kernel-doc:: drivers/gpu/drm/drmfs.c
+   :functions: drmfs_create_dir
+
+.. kernel-doc:: drivers/gpu/drm/drmfs.c
+   :functions: drmfs_remove
+
+.. kernel-doc:: drivers/gpu/drm/drmfs.c
+   :functions:: drmfs_remove_recursive
+
+.. kernel-doc:: drivers/gpu/drm/drmfs.c
+   :functions:: drmfs_initialized
+
+.. kernel-doc:: drivers/gpu/drm/drmfs.c
+   :functions:: drmfs_init
+
+.. kernel-doc:: drivers/gpu/drm/drmfs.c
+   :functions:: drmfs_fini
diff --git a/drivers/gpu/drm/Kconfig b/drivers/gpu/drm/Kconfig
index 95fc041..5082840 100644
--- a/drivers/gpu/drm/Kconfig
+++ b/drivers/gpu/drm/Kconfig
@@ -26,6 +26,15 @@ config DRM_MIPI_DSI
 	bool
 	depends on DRM
 
+config DRMFS
+	bool "Drmfs file system support"
+	depends on DRM
+	help
+	  Drmfs is a pseudo file system for drm subsystem output data.
+
+	  drmfs is a filesystem which serves as a stable ABI to hold
+	  miscellaneous output data from drm subsystems.
+
 config DRM_DP_AUX_CHARDEV
 	bool "DRM DP AUX Interface"
 	depends on DRM
diff --git a/drivers/gpu/drm/Makefile b/drivers/gpu/drm/Makefile
index 883f3e7..5021225 100644
--- a/drivers/gpu/drm/Makefile
+++ b/drivers/gpu/drm/Makefile
@@ -42,6 +42,7 @@ CFLAGS_drm_trace_points.o := -I$(src)
 
 obj-$(CONFIG_DRM)	+= drm.o
 obj-$(CONFIG_DRM_MIPI_DSI) += drm_mipi_dsi.o
+obj-$(CONFIG_DRMFS)	+= drmfs.o
 obj-$(CONFIG_DRM_ARM)	+= arm/
 obj-$(CONFIG_DRM_TTM)	+= ttm/
 obj-$(CONFIG_DRM_TDFX)	+= tdfx/
diff --git a/drivers/gpu/drm/drmfs.c b/drivers/gpu/drm/drmfs.c
new file mode 100644
index 0000000..cbcf761
--- /dev/null
+++ b/drivers/gpu/drm/drmfs.c
@@ -0,0 +1,566 @@
+/*
+ * Copyright © 2014 Intel Corporation
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining a
+ * copy of this software and associated documentation files (the "Software"),
+ * to deal in the Software without restriction, including without limitation
+ * the rights to use, copy, modify, merge, publish, distribute, sublicense,
+ * and/or sell copies of the Software, and to permit persons to whom the
+ * Software is furnished to do so, subject to the following conditions:
+ *
+ * The above copyright notice and this permission notice (including the next
+ * paragraph) shall be included in all copies or substantial portions of the
+ * Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+ * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT.  IN NO EVENT SHALL
+ * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+ * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
+ * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER
+ * DEALINGS IN THE SOFTWARE.
+ *
+ * Authors:
+ *	Swati Dhingra <swati.dhingra@intel.com>
+ *	Sourab Gupta <sourab.gupta@intel.com>
+ *	Akash Goel <akash.goel@intel.com>
+ */
+
+#include <linux/module.h>
+#include <linux/fs.h>
+#include <linux/mount.h>
+#include <linux/kobject.h>
+#include <linux/namei.h>
+#include <drm/drmfs.h>
+#include <linux/fsnotify.h>
+#include <linux/seq_file.h>
+#include <linux/parser.h>
+#include <linux/magic.h>
+#include <linux/slab.h>
+
+#define DRMFS_DEFAULT_MODE	0700
+
+static struct vfsmount *drmfs_mount;
+static int drmfs_mount_count;
+static bool drmfs_registered;
+
+static ssize_t default_read_file(struct file *file, char __user *buf,
+				size_t count, loff_t *ppos)
+{
+	return 0;
+}
+
+static ssize_t default_write_file(struct file *file, const char __user *buf,
+				size_t count, loff_t *ppos)
+{
+	return count;
+}
+
+static const struct file_operations drmfs_default_file_operations = {
+	.read =		default_read_file,
+	.write =	default_write_file,
+	.open =		simple_open,
+	.llseek =	noop_llseek,
+};
+
+static struct inode *drmfs_get_inode(struct super_block *sb)
+{
+	struct inode *inode = new_inode(sb);
+
+	if (inode) {
+		inode->i_ino = get_next_ino();
+		inode->i_atime = inode->i_mtime = inode->i_ctime = CURRENT_TIME;
+	}
+	return inode;
+}
+
+struct drmfs_mount_opts {
+	kuid_t uid;
+	kgid_t gid;
+	umode_t mode;
+};
+
+enum {
+	Opt_uid,
+	Opt_gid,
+	Opt_mode,
+	Opt_err
+};
+
+static const match_table_t tokens = {
+	{Opt_uid, "uid=%u"},
+	{Opt_gid, "gid=%u"},
+	{Opt_mode, "mode=%o"},
+	{Opt_err, NULL}
+};
+
+struct drmfs_fs_info {
+	struct drmfs_mount_opts mount_opts;
+};
+
+static int drmfs_parse_options(char *data, struct drmfs_mount_opts *opts)
+{
+	substring_t args[MAX_OPT_ARGS];
+	int option;
+	int token;
+	kuid_t uid;
+	kgid_t gid;
+	char *p;
+
+	opts->mode = DRMFS_DEFAULT_MODE;
+
+	while ((p = strsep(&data, ",")) != NULL) {
+		if (!*p)
+			continue;
+
+		token = match_token(p, tokens, args);
+		switch (token) {
+		case Opt_uid:
+			if (match_int(&args[0], &option))
+				return -EINVAL;
+			uid = make_kuid(current_user_ns(), option);
+			if (!uid_valid(uid))
+				return -EINVAL;
+			opts->uid = uid;
+			break;
+		case Opt_gid:
+			if (match_int(&args[0], &option))
+				return -EINVAL;
+			gid = make_kgid(current_user_ns(), option);
+			if (!gid_valid(gid))
+				return -EINVAL;
+			opts->gid = gid;
+			break;
+		case Opt_mode:
+			if (match_octal(&args[0], &option))
+				return -EINVAL;
+			opts->mode = option & S_IALLUGO;
+			break;
+		/*
+		 * We might like to report bad mount options here
+		 */
+		}
+	}
+
+	return 0;
+}
+
+static int drmfs_apply_options(struct super_block *sb)
+{
+	struct drmfs_fs_info *fsi = sb->s_fs_info;
+	struct inode *inode = sb->s_root->d_inode;
+	struct drmfs_mount_opts *opts = &fsi->mount_opts;
+
+	inode->i_mode &= ~S_IALLUGO;
+	inode->i_mode |= opts->mode;
+
+	inode->i_uid = opts->uid;
+	inode->i_gid = opts->gid;
+
+	return 0;
+}
+
+static int drmfs_remount(struct super_block *sb, int *flags, char *data)
+{
+	int err;
+	struct drmfs_fs_info *fsi = sb->s_fs_info;
+
+	sync_filesystem(sb);
+	err = drmfs_parse_options(data, &fsi->mount_opts);
+	if (err)
+		goto fail;
+
+	drmfs_apply_options(sb);
+
+fail:
+	return err;
+}
+
+static int drmfs_show_options(struct seq_file *m, struct dentry *root)
+{
+	struct drmfs_fs_info *fsi = root->d_sb->s_fs_info;
+	struct drmfs_mount_opts *opts = &fsi->mount_opts;
+
+	if (!uid_eq(opts->uid, GLOBAL_ROOT_UID))
+		seq_printf(m, ",uid=%u",
+			   from_kuid_munged(&init_user_ns, opts->uid));
+	if (!gid_eq(opts->gid, GLOBAL_ROOT_GID))
+		seq_printf(m, ",gid=%u",
+			   from_kgid_munged(&init_user_ns, opts->gid));
+	if (opts->mode != DRMFS_DEFAULT_MODE)
+		seq_printf(m, ",mode=%o", opts->mode);
+
+	return 0;
+}
+
+static const struct super_operations drmfs_super_operations = {
+	.statfs		= simple_statfs,
+	.remount_fs	= drmfs_remount,
+	.show_options	= drmfs_show_options,
+};
+
+static int drm_fill_super(struct super_block *sb, void *data, int silent)
+{
+	static struct tree_descr drm_files[] = { {""} };
+	struct drmfs_fs_info *fsi;
+	int err;
+
+	save_mount_options(sb, data);
+
+	fsi = kzalloc(sizeof(struct drmfs_fs_info), GFP_KERNEL);
+	sb->s_fs_info = fsi;
+	if (!fsi) {
+		err = -ENOMEM;
+		goto fail;
+	}
+
+	err = drmfs_parse_options(data, &fsi->mount_opts);
+	if (err)
+		goto fail;
+
+	err  =  simple_fill_super(sb, DRMFS_MAGIC, drm_files);
+	if (err)
+		goto fail;
+
+	sb->s_op = &drmfs_super_operations;
+
+	drmfs_apply_options(sb);
+
+	return 0;
+
+fail:
+	kfree(fsi);
+	sb->s_fs_info = NULL;
+	return err;
+}
+
+static struct dentry *drm_mount(struct file_system_type *fs_type,
+			int flags, const char *dev_name,
+			void *data)
+{
+	return mount_single(fs_type, flags, data, drm_fill_super);
+}
+
+static struct file_system_type drm_fs_type = {
+	.owner =	THIS_MODULE,
+	.name =		"drmfs",
+	.mount =	drm_mount,
+	.kill_sb =	kill_litter_super,
+};
+MODULE_ALIAS_FS("drmfs");
+
+static struct dentry *start_creating(const char *name, struct dentry *parent)
+{
+	struct dentry *dentry;
+	int error;
+
+	pr_debug("drmfs: creating file '%s'\n", name);
+
+	error = simple_pin_fs(&drm_fs_type, &drmfs_mount,
+			      &drmfs_mount_count);
+	if (error)
+		return ERR_PTR(error);
+
+	/* If the parent is not specified, we create it in the root.
+	 * We need the root dentry to do this, which is in the super
+	 * block. A pointer to that is in the struct vfsmount that we
+	 * have around.
+	 */
+	if (!parent)
+		parent = drmfs_mount->mnt_root;
+
+	inode_lock(parent->d_inode);
+	dentry = lookup_one_len(name, parent, strlen(name));
+	if (!IS_ERR(dentry) && dentry->d_inode) {
+		dput(dentry);
+		dentry = ERR_PTR(-EEXIST);
+	}
+
+	if (IS_ERR(dentry)) {
+		inode_unlock(parent->d_inode);
+		simple_release_fs(&drmfs_mount, &drmfs_mount_count);
+	}
+
+	return dentry;
+}
+
+static struct dentry *failed_creating(struct dentry *dentry)
+{
+	inode_unlock(dentry->d_parent->d_inode);
+	dput(dentry);
+	simple_release_fs(&drmfs_mount, &drmfs_mount_count);
+	return NULL;
+}
+
+static struct dentry *end_creating(struct dentry *dentry)
+{
+	inode_unlock(dentry->d_parent->d_inode);
+	return dentry;
+}
+
+/**
+ * drmfs_create_file - create a file in the drmfs filesystem
+ * @name: a pointer to a string containing the name of the file to create.
+ * @mode: the permission that the file should have.
+ * @parent: a pointer to the parent dentry for this file.  This should be a
+ *          directory dentry if set.  If this parameter is NULL, then the
+ *          file will be created in the root of the drmfs filesystem.
+ * @data: a pointer to something that the caller will want to get to later
+ *        on.  The inode.i_private pointer will point to this value on
+ *        the open() call.
+ * @fops: a pointer to a struct file_operations that should be used for
+ *        this file.
+ *
+ * This is the basic "create a file" function for drmfs.  It allows for a
+ * wide range of flexibility in creating a file, or a directory (if you want
+ * to create a directory, the drmfs_create_dir() function is
+ * recommended to be used instead.)
+ *
+ * This function will return a pointer to a dentry if it succeeds.  This
+ * pointer must be passed to the drmfs_remove() function when the file is
+ * to be removed (no automatic cleanup happens if your module is unloaded,
+ * you are responsible here.)  If an error occurs, %NULL will be returned.
+ *
+ */
+struct dentry *drmfs_create_file(const char *name, umode_t mode,
+				   struct dentry *parent, void *data,
+				   const struct file_operations *fops)
+{
+	struct dentry *dentry;
+	struct inode *inode;
+
+	if (!(mode & S_IFMT))
+		mode |= S_IFREG;
+
+	if (WARN_ON(!S_ISREG(mode)))
+		return NULL;
+
+	dentry = start_creating(name, parent);
+
+	if (IS_ERR(dentry))
+		return NULL;
+
+	inode = drmfs_get_inode(dentry->d_sb);
+	if (unlikely(!inode))
+		return failed_creating(dentry);
+
+	inode->i_mode = mode;
+	inode->i_fop = fops ? fops : &drmfs_default_file_operations;
+	inode->i_private = data;
+	d_instantiate(dentry, inode);
+	fsnotify_create(dentry->d_parent->d_inode, dentry);
+	return end_creating(dentry);
+}
+EXPORT_SYMBOL(drmfs_create_file);
+
+static struct dentry *__create_dir(const char *name, struct dentry *parent,
+				   const struct inode_operations *ops)
+{
+	struct dentry *dentry = start_creating(name, parent);
+	struct inode *inode;
+
+	if (IS_ERR(dentry))
+		return NULL;
+
+	inode = drmfs_get_inode(dentry->d_sb);
+	if (unlikely(!inode))
+		return failed_creating(dentry);
+
+	inode->i_mode = S_IFDIR | S_IRWXU | S_IRUGO | S_IXUGO;
+	inode->i_op = ops;
+	inode->i_fop = &simple_dir_operations;
+
+	/* directory inodes start off with i_nlink == 2 (for "." entry) */
+	inc_nlink(inode);
+	d_instantiate(dentry, inode);
+	inc_nlink(dentry->d_parent->d_inode);
+	fsnotify_mkdir(dentry->d_parent->d_inode, dentry);
+	return end_creating(dentry);
+}
+
+/**
+ * drmfs_create_dir - create a directory in the drmfs filesystem
+ * @name: a pointer to a string containing the name of the directory to
+ *        create.
+ * @parent: a pointer to the parent dentry for this file.  This should be a
+ *          directory dentry if set.  If this parameter is NULL, then the
+ *          directory will be created in the root of the drmfs filesystem.
+ *
+ * This function creates a directory in drmfs with the given name.
+ *
+ * This function will return a pointer to a dentry if it succeeds.  This
+ * pointer must be passed to the drmfs_remove() function when the file is
+ * to be removed. If an error occurs, %NULL will be returned.
+ *
+ */
+struct dentry *drmfs_create_dir(const char *name, struct dentry *parent)
+{
+	return __create_dir(name, parent, &simple_dir_inode_operations);
+}
+EXPORT_SYMBOL(drmfs_create_dir);
+
+static int __drmfs_remove(struct dentry *dentry, struct dentry *parent)
+{
+	int ret = 0;
+
+	if (simple_positive(dentry)) {
+		if (dentry->d_inode) {
+			dget(dentry);
+			switch (dentry->d_inode->i_mode & S_IFMT) {
+			case S_IFDIR:
+				ret = simple_rmdir(parent->d_inode, dentry);
+				break;
+			default:
+				simple_unlink(parent->d_inode, dentry);
+				break;
+			}
+			if (!ret)
+				d_delete(dentry);
+			dput(dentry);
+		}
+	}
+	return ret;
+}
+
+
+/**
+ * drmfs_remove - removes a file or directory from the drmfs filesystem
+ * @dentry: a pointer to a the dentry of the file or directory to be
+ *          removed.
+ *
+ * This function removes a file or directory in drmfs that was previously
+ * created with a call to another drmfs function (like
+ * drmfs_create_file() or variants thereof.)
+ */
+void drmfs_remove(struct dentry *dentry)
+{
+	struct dentry *parent;
+	int ret;
+
+	if (IS_ERR_OR_NULL(dentry))
+		return;
+
+	parent = dentry->d_parent;
+	inode_lock(parent->d_inode);
+	ret = __drmfs_remove(dentry, parent);
+	inode_unlock(parent->d_inode);
+	if (!ret)
+		simple_release_fs(&drmfs_mount, &drmfs_mount_count);
+}
+EXPORT_SYMBOL(drmfs_remove);
+
+/**
+ * drmfs_remove_recursive - recursively removes a directory
+ * @dentry: a pointer to a the dentry of the directory to be removed.
+ *
+ * This function recursively removes a directory tree in drmfs that
+ * was previously created with a call to another drmfs function
+ * (like drmfs_create_file() or variants thereof.)
+ */
+void drmfs_remove_recursive(struct dentry *dentry)
+{
+	struct dentry *child, *parent;
+
+	if (IS_ERR_OR_NULL(dentry))
+		return;
+
+	parent = dentry;
+ down:
+	inode_lock(parent->d_inode);
+ loop:
+	/*
+	 * The parent->d_subdirs is protected by the d_lock. Outside that
+	 * lock, the child can be unlinked and set to be freed which can
+	 * use the d_u.d_child as the rcu head and corrupt this list.
+	 */
+	spin_lock(&parent->d_lock);
+	list_for_each_entry(child, &parent->d_subdirs, d_child) {
+		if (!simple_positive(child))
+			continue;
+
+		/* perhaps simple_empty(child) makes more sense */
+		if (!list_empty(&child->d_subdirs)) {
+			spin_unlock(&parent->d_lock);
+			inode_unlock(parent->d_inode);
+			parent = child;
+			goto down;
+		}
+
+		spin_unlock(&parent->d_lock);
+
+		if (!__drmfs_remove(child, parent))
+			simple_release_fs(&drmfs_mount, &drmfs_mount_count);
+
+		/*
+		 * The parent->d_lock protects against child from unlinking
+		 * from d_subdirs. When releasing the parent->d_lock we can
+		 * no longer trust that the next pointer is valid.
+		 * Restart the loop. We'll skip this one with the
+		 * simple_positive() check.
+		 */
+		goto loop;
+	}
+	spin_unlock(&parent->d_lock);
+
+	inode_unlock(parent->d_inode);
+	child = parent;
+	parent = parent->d_parent;
+	inode_lock(parent->d_inode);
+
+	if (child != dentry)
+		/* go up */
+		goto loop;
+
+	if (!__drmfs_remove(child, parent))
+		simple_release_fs(&drmfs_mount, &drmfs_mount_count);
+	inode_unlock(parent->d_inode);
+}
+EXPORT_SYMBOL(drmfs_remove_recursive);
+
+/**
+ * drmfs_initialized - Tells whether drmfs has been registered
+ */
+bool drmfs_initialized(void)
+{
+	return drmfs_registered;
+}
+EXPORT_SYMBOL(drmfs_initialized);
+
+/**
+ * drmfs_init - Initializes/Registers drmfs filesystem
+ *
+ * This function initializes drmfs filesytem and registers it with kernel.
+ * Also a canonical mountpoint is created for filesystem. 
+ * 
+ * RETURNS:
+ * 0 on success, negative error code on failure.
+ */
+int drmfs_init(void)
+{
+	int retval;
+
+	retval = sysfs_create_mount_point(kernel_kobj, "drm");
+	if (retval)
+		return -EINVAL;
+
+	retval = register_filesystem(&drm_fs_type);
+	if (!retval)
+		drmfs_registered = true;
+
+	return retval;
+}
+
+/**
+ * drmfs_fini - Unregisters/Deinitializes drmfs filesystem
+ *
+ * This function deregisters drmfs and removes its canonical mountpoint.
+ */
+
+void drmfs_fini(void)
+{
+	unregister_filesystem(&drm_fs_type);
+
+	drmfs_registered = false;
+
+	sysfs_remove_mount_point(kernel_kobj, "drm");
+}
diff --git a/include/drm/drmfs.h b/include/drm/drmfs.h
new file mode 100644
index 0000000..6df16bb
--- /dev/null
+++ b/include/drm/drmfs.h
@@ -0,0 +1,77 @@
+/*
+ * Copyright © 2014 Intel Corporation
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining a
+ * copy of this software and associated documentation files (the "Software"),
+ * to deal in the Software without restriction, including without limitation
+ * the rights to use, copy, modify, merge, publish, distribute, sublicense,
+ * and/or sell copies of the Software, and to permit persons to whom the
+ * Software is furnished to do so, subject to the following conditions:
+ *
+ * The above copyright notice and this permission notice (including the next
+ * paragraph) shall be included in all copies or substantial portions of the
+ * Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+ * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT.  IN NO EVENT SHALL
+ * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+ * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
+ * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER
+ * DEALINGS IN THE SOFTWARE.
+ *
+ * Authors:
+ *	Swati Dhingra <swati.dhingra@intel.com>
+ *	Sourab Gupta <sourab.gupta@intel.com>
+ *	Akash Goel <akash.goel@intel.com>
+ */
+
+#ifndef _DRMFS_H_
+#define _DRMFS_H_
+
+#include <linux/fs.h>
+#include <linux/seq_file.h>
+
+#include <linux/types.h>
+
+struct file_operations;
+
+#ifdef CONFIG_DRMFS
+
+struct dentry *drmfs_create_file(const char *name, umode_t mode,
+				   struct dentry *parent, void *data,
+				   const struct file_operations *fops);
+
+struct dentry *drmfs_create_dir(const char *name, struct dentry *parent);
+
+void drmfs_remove(struct dentry *dentry);
+void drmfs_remove_recursive(struct dentry *dentry);
+
+bool drmfs_initialized(void);
+int drmfs_init(void);
+void drmfs_fini(void);
+
+#else
+
+static inline struct dentry *drmfs_create_file(const char *name, umode_t mode,
+				   struct dentry *parent, void *data,
+				   const struct file_operations *fops)
+{
+	return ERR_PTR(-ENODEV);
+}
+
+static inline struct dentry *drmfs_create_dir(const char *name, struct dentry *parent)
+{
+	return ERR_PTR(-ENODEV);
+}
+
+static inline void drmfs_remove(struct dentry *dentry){}
+static inline void drmfs_remove_recursive(struct dentry *dentry){}
+
+static inline bool drmfs_initialized(void){return false;}
+static inline int drmfs_init(void){return -EINVAL;}
+static inline void drmfs_fini(void){}
+
+#endif /* CONFIG_DRMFS */
+
+#endif
diff --git a/include/uapi/linux/magic.h b/include/uapi/linux/magic.h
index 9bd5594..ef01eb7 100644
--- a/include/uapi/linux/magic.h
+++ b/include/uapi/linux/magic.h
@@ -61,6 +61,9 @@
 #define STACK_END_MAGIC		0x57AC6E9D
 
 #define TRACEFS_MAGIC          0x74726163
+#define DRMFS_MAGIC	       0x84724143      /* some random number.
+						* Is there a better way?
+						*/
 
 #define V9FS_MAGIC		0x01021997
 
-- 
2.7.4

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[RFC 2/4] drm: Register drmfs filesystem from drm init

[swati.dhingra]

From: Sourab Gupta <sourab.gupta@intel.com>

The drmfs filesystem will not be registered standalone during kernel init time,
instead it is intended to be initialized/registered during drm initialization.
This again is dependent on CONFIG_DRMFS being defined.

Signed-off-by: Sourab Gupta <sourab.gupta@intel.com>
Signed-off-by: Swati Dhingra <swati.dhingra@intel.com>
---
 drivers/gpu/drm/drm_drv.c | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/drivers/gpu/drm/drm_drv.c b/drivers/gpu/drm/drm_drv.c
index f74b7d0..298013c 100644
--- a/drivers/gpu/drm/drm_drv.c
+++ b/drivers/gpu/drm/drm_drv.c
@@ -34,6 +34,7 @@
 #include <linux/slab.h>
 
 #include <drm/drm_drv.h>
+#include <drm/drmfs.h>
 #include <drm/drmP.h>
 
 #include "drm_crtc_internal.h"
@@ -828,6 +829,7 @@ static void drm_core_exit(void)
 {
 	unregister_chrdev(DRM_MAJOR, "drm");
 	debugfs_remove(drm_debugfs_root);
+	drmfs_fini();
 	drm_sysfs_destroy();
 	idr_destroy(&drm_minors_idr);
 	drm_connector_ida_destroy();
@@ -848,6 +850,10 @@ static int __init drm_core_init(void)
 		goto error;
 	}
 
+	ret = drmfs_init();
+	if (ret < 0)
+		DRM_ERROR("Cannot create DRM FS: %d\n", ret);
+
 	drm_debugfs_root = debugfs_create_dir("dri", NULL);
 	if (!drm_debugfs_root) {
 		ret = -ENOMEM;
-- 
2.7.4

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[RFC 3/4] drm: Create driver specific root directory inside drmfs

[swati.dhingra]

From: Sourab Gupta <sourab.gupta@intel.com>

A driver specific directory is created inside drmfs root, and dentry of
this directory is saved for subsequent use by the driver (e.g. i915).
The driver can then create files/directories inside this root directory
directly.

In case of i915 driver, the top directory is created at '/sys/kernel/drm/i915'.

Signed-off-by: Sourab Gupta <sourab.gupta@intel.com>
Signed-off-by: Swati Dhingra <swati.dhingra@intel.com>
---
 drivers/gpu/drm/drm_drv.c | 6 ++++++
 include/drm/drm_drv.h     | 3 +++
 2 files changed, 9 insertions(+)

diff --git a/drivers/gpu/drm/drm_drv.c b/drivers/gpu/drm/drm_drv.c
index 298013c..4c32cf8 100644
--- a/drivers/gpu/drm/drm_drv.c
+++ b/drivers/gpu/drm/drm_drv.c
@@ -671,6 +671,11 @@ EXPORT_SYMBOL(drm_dev_unref);
 int drm_dev_register(struct drm_device *dev, unsigned long flags)
 {
 	int ret;
+	struct dentry *drmfs_root;
+
+	drmfs_root = drmfs_create_dir(dev->driver->name, NULL);
+	if (!IS_ERR(drmfs_root))
+		dev->driver->drmfs_root = drmfs_root;
 
 	mutex_lock(&drm_global_mutex);
 
@@ -742,6 +747,7 @@ void drm_dev_unregister(struct drm_device *dev)
 	drm_minor_unregister(dev, DRM_MINOR_PRIMARY);
 	drm_minor_unregister(dev, DRM_MINOR_RENDER);
 	drm_minor_unregister(dev, DRM_MINOR_CONTROL);
+	drmfs_remove(dev->driver->drmfs_root);
 }
 EXPORT_SYMBOL(drm_dev_unregister);
 
diff --git a/include/drm/drm_drv.h b/include/drm/drm_drv.h
index c4fc495..d646296 100644
--- a/include/drm/drm_drv.h
+++ b/include/drm/drm_drv.h
@@ -405,6 +405,9 @@ struct drm_driver {
 
 	/* List of devices hanging off this driver with stealth attach. */
 	struct list_head legacy_dev_list;
+
+	/* drmfs parent directory dentry for this driver */
+	struct dentry *drmfs_root;
 };
 
 extern __printf(6, 7)
-- 
2.7.4

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[RFC 4/4] drm/i915: Creating guc log file in drmfs instead of debugfs

[swati.dhingra]

From: Sourab Gupta <sourab.gupta@intel.com>

In the current scenario, the relay API fit well only with debugfs, due to
availability of parent dentry. Any other existing filesystem was not feasible for
holding guc logs, due to incompatibility with relay. But this makes the  guc_log
file unavailable on the production kernels.

GuC log file can therefore be one of candidates for movement to the drmfs
filesystem, which can satisfy all the requirements needed by relay API, and can
house any relayfs based output file.

The patch moves the parent directory of guc 'log_dir' from debugfs_root to
drmfs_root, while using the drmfs api's to create the requisite files.

Signed-off-by: Sourab Gupta <sourab.gupta@intel.com>
Signed-off-by: Swati Dhingra <swati.dhingra@intel.com>
---
 drivers/gpu/drm/i915/i915_guc_submission.c | 33 +++++++++++-------------------
 1 file changed, 12 insertions(+), 21 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_guc_submission.c b/drivers/gpu/drm/i915/i915_guc_submission.c
index 5841380..3f5978a 100644
--- a/drivers/gpu/drm/i915/i915_guc_submission.c
+++ b/drivers/gpu/drm/i915/i915_guc_submission.c
@@ -22,8 +22,8 @@
  *
  */
 #include <linux/circ_buf.h>
-#include <linux/debugfs.h>
 #include <linux/relay.h>
+#include <drm/drmfs.h>
 #include "i915_drv.h"
 #include "intel_uc.h"
 
@@ -812,7 +812,7 @@ static int subbuf_start_callback(struct rchan_buf *buf,
 }
 
 /*
- * file_create() callback. Creates relay file in debugfs.
+ * file_create() callback. Creates relay file.
  */
 static struct dentry *create_buf_file_callback(const char *filename,
 					       struct dentry *parent,
@@ -838,17 +838,19 @@ static struct dentry *create_buf_file_callback(const char *filename,
 	 * dentry of the file associated with the channel buffer and that file's
 	 * name need not be same as the filename passed as an argument.
 	 */
-	buf_file = debugfs_create_file("guc_log", mode,
+
+	buf_file = drmfs_create_file("guc_log", mode,
 				       parent, buf, &relay_file_operations);
+
 	return buf_file;
 }
 
 /*
- * file_remove() default callback. Removes relay file in debugfs.
+ * file_remove() default callback. Removes relay file.
  */
 static int remove_buf_file_callback(struct dentry *dentry)
 {
-	debugfs_remove(dentry);
+	drmfs_remove(dentry);
 	return 0;
 }
 
@@ -898,22 +900,11 @@ static int guc_log_create_relay_file(struct intel_guc *guc)
 	struct dentry *log_dir;
 	int ret;
 
-	/* For now create the log file in /sys/kernel/debug/dri/0 dir */
-	log_dir = dev_priv->drm.primary->debugfs_root;
-
-	/* If /sys/kernel/debug/dri/0 location do not exist, then debugfs is
-	 * not mounted and so can't create the relay file.
-	 * The relay API seems to fit well with debugfs only, for availing relay
-	 * there are 3 requirements which can be met for debugfs file only in a
-	 * straightforward/clean manner :-
-	 * i)   Need the associated dentry pointer of the file, while opening the
-	 *      relay channel.
-	 * ii)  Should be able to use 'relay_file_operations' fops for the file.
-	 * iii) Set the 'i_private' field of file's inode to the pointer of
-	 *	relay channel buffer.
-	 */
+	/* Create the log file in drmfs dir: /sys/kernel/drm/i915/ */
+	log_dir = dev_priv->drm.driver->drmfs_root;
+
 	if (!log_dir) {
-		DRM_ERROR("Debugfs dir not available yet for GuC log file\n");
+		DRM_ERROR("Root dir not available yet for GuC log file\n");
 		return -ENODEV;
 	}
 
@@ -1154,7 +1145,7 @@ static int guc_log_create_extras(struct intel_guc *guc)
 	if (!guc->log.relay_chan) {
 		/* Create a relay channel, so that we have buffers for storing
 		 * the GuC firmware logs, the channel will be linked with a file
-		 * later on when debugfs is registered.
+		 * later on when drmfs is registered.
 		 */
 		ret = guc_log_create_relay_channel(guc);
 		if (ret)
-- 
2.7.4

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

✗ Fi.CI.BAT: warning for Introduce drmfs pseudo filesystem for drm subsystem (rev4)

[Patchwork]

== Series Details ==

Series: Introduce drmfs pseudo filesystem for drm subsystem (rev4)
URL   : https://patchwork.freedesktop.org/series/16203/
State : warning

== Summary ==

Series 16203v4 Introduce drmfs pseudo filesystem for drm subsystem
https://patchwork.freedesktop.org/api/1.0/series/16203/revisions/4/mbox/

Test gem_sync:
        Subgroup basic-store-all:
                fail       -> PASS       (fi-ivb-3520m)
Test kms_force_connector_basic:
        Subgroup force-load-detect:
                pass       -> DMESG-WARN (fi-snb-2520m)

fi-bdw-5557u     total:247  pass:233  dwarn:0   dfail:0   fail:0   skip:14 
fi-bsw-n3050     total:247  pass:208  dwarn:0   dfail:0   fail:0   skip:39 
fi-bxt-j4205     total:247  pass:225  dwarn:1   dfail:0   fail:0   skip:21 
fi-bxt-t5700     total:247  pass:220  dwarn:0   dfail:0   fail:0   skip:27 
fi-byt-j1900     total:247  pass:220  dwarn:0   dfail:0   fail:0   skip:27 
fi-byt-n2820     total:247  pass:216  dwarn:0   dfail:0   fail:0   skip:31 
fi-hsw-4770      total:247  pass:228  dwarn:0   dfail:0   fail:0   skip:19 
fi-ilk-650       total:247  pass:195  dwarn:0   dfail:0   fail:0   skip:52 
fi-ivb-3520m     total:247  pass:226  dwarn:0   dfail:0   fail:0   skip:21 
fi-ivb-3770      total:247  pass:226  dwarn:0   dfail:0   fail:0   skip:21 
fi-kbl-7500u     total:247  pass:226  dwarn:0   dfail:0   fail:0   skip:21 
fi-skl-6260u     total:247  pass:234  dwarn:0   dfail:0   fail:0   skip:13 
fi-skl-6700hq    total:247  pass:227  dwarn:0   dfail:0   fail:0   skip:20 
fi-skl-6700k     total:247  pass:224  dwarn:3   dfail:0   fail:0   skip:20 
fi-skl-6770hq    total:247  pass:234  dwarn:0   dfail:0   fail:0   skip:13 
fi-snb-2520m     total:247  pass:215  dwarn:1   dfail:0   fail:0   skip:31 
fi-snb-2600      total:247  pass:215  dwarn:0   dfail:0   fail:0   skip:32 

0f918489ce78af8bc606c24b62d12dc869fb8844 drm-tip: 2016y-12m-19d-11h-45m-08s UTC integration manifest
1a09aad drm/i915: Creating guc log file in drmfs instead of debugfs
dd3f5ba drm: Create driver specific root directory inside drmfs
bb8d42c drm: Register drmfs filesystem from drm init
c468cc0 drm: Introduce drmfs pseudo filesystem interfaces

== Logs ==

For more details see: https://intel-gfx-ci.01.org/CI/Patchwork_3326/
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [RFC 0/4] Introduce drmfs pseudo filesystem for drm subsystem

[Laurent Pinchart]

Hi Swati,

On Monday 19 Dec 2016 16:12:22 swati.dhingra@intel.com wrote:
> From: Swati Dhingra <swati.dhingra@intel.com>
> 
> Currently, we don't have a stable ABI which can be used for the purpose of
> providing output debug/loggging/crc and other such data from DRM.
> The ABI in current use (filesystems, ioctls, et al.) have their own
> constraints and are intended to output a particular type of data.
> Few cases in point:
> sysfs	- stable ABI, but constrained to one textual value per file
> debugfs	- unstable ABI, free-for-all
> ioctls	- not as suitable to many single purpose continuous data
> 	  dumping, we would very quickly run out ioctl space; requires more
> 	  userspace support than "cat"
> device nodes -  a real possibilty, kernel instantiation is more tricky,
> 		requires udev (+udev.rules) or userspace discovery of the
> 		dynamic major:minor (via sysfs) [mounting a registered
> 		filesystem is easy in comparison]
> netlink	- stream based, therefore involves numerous copies.
> 
> Debugfs is the lesser among the evils here, thereby we have grown used to
> the convenience and flexibility in presentation that debugfs gives us
> (including relayfs inodes) that we want some of that hierachy in stable user
> ABI form.

Seriously, why ? A subsystem growing its own file system sounds so wrong. It 
seems that you want to have all the benefits of a stable ABI without going 
through the standardization effort that this requires. I can see so many ways 
that drmfs could be abused, with drivers throwing in new data with little or 
no review. You'll need very compelling arguments to convince me.

> Due to these limitations, there is a need for a new pseudo filesytem, that
> would act as a stable 'free-for-all' ABI, with the heirarchial structure and
> thus convenience of debugfs. This will be managed by drm, thus named
> 'drmfs'. DRM would register this filesystem to manage a canonical
> mountpoint, but this wouldn't limit everyone to only using that pseudofs
> underneath.
> 
> This can serve to hold various kinds of output data from Linux DRM
> subsystems, for the files which can't truely fit anywhere else with
> existing ABI's but present so, for the lack of a better place.
> 
> In this patch series, we have introduced a pseudo filesystem named as
> 'drmfs' for now. The filesystem is introduced in the first patch, and the
> subsequent patches make use of the filesystem interfaces, in drm driver,
> and making them available for use by the drm subsystem components, one of
> which is i915. We've moved the location of i915 GuC logs from debugfs to
> drmfs in the last patch. Subsequently, more such files such as pipe_crc,
> error states, memory stats, etc. can be move to this filesystem, if the
> idea introduced here is acceptable per se. The filesystem introduced is
> being used to house the data generated by i915 driver in this patch series,
> but will hopefully be generic enough to provide scope for usage by any
> other drm subsystem component.
> 
> The patch series is being floated as RFC to gather feedback on the idea and
> infrastructure proposed here and it's suitability to address the specific
> problem statement/use case.
> 
> v2: fix the bat failures caused due to missing config check
> 
> v3: Changes made:
>     - Move the location of drmfs from fs/ to drivers/gpu/drm/ (Chris)
>     - Moving config checks to header (Chris,Daniel)
> 
> v4: Added the kernel Documentaion (using Sphinx).
> 
> Sourab Gupta (4):
>   drm: Introduce drmfs pseudo filesystem interfaces
>   drm: Register drmfs filesystem from drm init
>   drm: Create driver specific root directory inside drmfs
>   drm/i915: Creating guc log file in drmfs instead of debugfs
> 
>  Documentation/gpu/drm-uapi.rst             |  76 ++++
>  drivers/gpu/drm/Kconfig                    |   9 +
>  drivers/gpu/drm/Makefile                   |   1 +
>  drivers/gpu/drm/drm_drv.c                  |  26 ++
>  drivers/gpu/drm/drmfs.c                    | 566 ++++++++++++++++++++++++++
>  drivers/gpu/drm/i915/i915_guc_submission.c |  33 +-
>  include/drm/drm_drv.h                      |   3 +
>  include/drm/drmfs.h                        |  77 ++++
>  include/uapi/linux/magic.h                 |   3 +
>  9 files changed, 773 insertions(+), 21 deletions(-)
>  create mode 100644 drivers/gpu/drm/drmfs.c
>  create mode 100644 include/drm/drmfs.h

-- 
Regards,

Laurent Pinchart

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [RFC 0/4] Introduce drmfs pseudo filesystem for drm subsystem

[Jani Nikula]

On Tue, 20 Dec 2016, Laurent Pinchart <laurent.pinchart@ideasonboard.com> wrote:
> Hi Swati,
>
> On Monday 19 Dec 2016 16:12:22 swati.dhingra@intel.com wrote:
>> From: Swati Dhingra <swati.dhingra@intel.com>
>> 
>> Currently, we don't have a stable ABI which can be used for the purpose of
>> providing output debug/loggging/crc and other such data from DRM.
>> The ABI in current use (filesystems, ioctls, et al.) have their own
>> constraints and are intended to output a particular type of data.
>> Few cases in point:
>> sysfs	- stable ABI, but constrained to one textual value per file
>> debugfs	- unstable ABI, free-for-all
>> ioctls	- not as suitable to many single purpose continuous data
>> 	  dumping, we would very quickly run out ioctl space; requires more
>> 	  userspace support than "cat"
>> device nodes -  a real possibilty, kernel instantiation is more tricky,
>> 		requires udev (+udev.rules) or userspace discovery of the
>> 		dynamic major:minor (via sysfs) [mounting a registered
>> 		filesystem is easy in comparison]
>> netlink	- stream based, therefore involves numerous copies.
>> 
>> Debugfs is the lesser among the evils here, thereby we have grown used to
>> the convenience and flexibility in presentation that debugfs gives us
>> (including relayfs inodes) that we want some of that hierachy in stable user
>> ABI form.
>
> Seriously, why ? A subsystem growing its own file system sounds so wrong. It 
> seems that you want to have all the benefits of a stable ABI without going 
> through the standardization effort that this requires. I can see so many ways 
> that drmfs could be abused, with drivers throwing in new data with little or 
> no review. You'll need very compelling arguments to convince me.

This is not unlike my sentiments on the first version posted
[1]. There's also the distinct feeling of [2]. Suffice it to say at this
time that I'm dubious, not convinced enough to defend this.

Swati, please refrain from posting new versions of the patches until
there's some consensus one way or the other; it's counter-productive to
keep splitting off the discussion into several patch series threads at
this stage. Let's have the discussion here.


BR,
Jani.


[1] http://mid.mail-archive.com/87lgw0xcf4.fsf@intel.com
[2] https://xkcd.com/927/

>
>> Due to these limitations, there is a need for a new pseudo filesytem, that
>> would act as a stable 'free-for-all' ABI, with the heirarchial structure and
>> thus convenience of debugfs. This will be managed by drm, thus named
>> 'drmfs'. DRM would register this filesystem to manage a canonical
>> mountpoint, but this wouldn't limit everyone to only using that pseudofs
>> underneath.
>> 
>> This can serve to hold various kinds of output data from Linux DRM
>> subsystems, for the files which can't truely fit anywhere else with
>> existing ABI's but present so, for the lack of a better place.
>> 
>> In this patch series, we have introduced a pseudo filesystem named as
>> 'drmfs' for now. The filesystem is introduced in the first patch, and the
>> subsequent patches make use of the filesystem interfaces, in drm driver,
>> and making them available for use by the drm subsystem components, one of
>> which is i915. We've moved the location of i915 GuC logs from debugfs to
>> drmfs in the last patch. Subsequently, more such files such as pipe_crc,
>> error states, memory stats, etc. can be move to this filesystem, if the
>> idea introduced here is acceptable per se. The filesystem introduced is
>> being used to house the data generated by i915 driver in this patch series,
>> but will hopefully be generic enough to provide scope for usage by any
>> other drm subsystem component.
>> 
>> The patch series is being floated as RFC to gather feedback on the idea and
>> infrastructure proposed here and it's suitability to address the specific
>> problem statement/use case.
>> 
>> v2: fix the bat failures caused due to missing config check
>> 
>> v3: Changes made:
>>     - Move the location of drmfs from fs/ to drivers/gpu/drm/ (Chris)
>>     - Moving config checks to header (Chris,Daniel)
>> 
>> v4: Added the kernel Documentaion (using Sphinx).
>> 
>> Sourab Gupta (4):
>>   drm: Introduce drmfs pseudo filesystem interfaces
>>   drm: Register drmfs filesystem from drm init
>>   drm: Create driver specific root directory inside drmfs
>>   drm/i915: Creating guc log file in drmfs instead of debugfs
>> 
>>  Documentation/gpu/drm-uapi.rst             |  76 ++++
>>  drivers/gpu/drm/Kconfig                    |   9 +
>>  drivers/gpu/drm/Makefile                   |   1 +
>>  drivers/gpu/drm/drm_drv.c                  |  26 ++
>>  drivers/gpu/drm/drmfs.c                    | 566 ++++++++++++++++++++++++++
>>  drivers/gpu/drm/i915/i915_guc_submission.c |  33 +-
>>  include/drm/drm_drv.h                      |   3 +
>>  include/drm/drmfs.h                        |  77 ++++
>>  include/uapi/linux/magic.h                 |   3 +
>>  9 files changed, 773 insertions(+), 21 deletions(-)
>>  create mode 100644 drivers/gpu/drm/drmfs.c
>>  create mode 100644 include/drm/drmfs.h

-- 
Jani Nikula, Intel Open Source Technology Center
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [RFC 0/4] Introduce drmfs pseudo filesystem for drm subsystem

[Sean Paul]

On Tue, Dec 20, 2016 at 4:44 AM, Jani Nikula
<jani.nikula@linux.intel.com> wrote:
> On Tue, 20 Dec 2016, Laurent Pinchart <laurent.pinchart@ideasonboard.com> wrote:
>> Hi Swati,
>>
>> On Monday 19 Dec 2016 16:12:22 swati.dhingra@intel.com wrote:
>>> From: Swati Dhingra <swati.dhingra@intel.com>
>>>
>>> Currently, we don't have a stable ABI which can be used for the purpose of
>>> providing output debug/loggging/crc and other such data from DRM.
>>> The ABI in current use (filesystems, ioctls, et al.) have their own
>>> constraints and are intended to output a particular type of data.
>>> Few cases in point:
>>> sysfs        - stable ABI, but constrained to one textual value per file
>>> debugfs      - unstable ABI, free-for-all
>>> ioctls       - not as suitable to many single purpose continuous data
>>>        dumping, we would very quickly run out ioctl space; requires more
>>>        userspace support than "cat"
>>> device nodes -  a real possibilty, kernel instantiation is more tricky,
>>>              requires udev (+udev.rules) or userspace discovery of the
>>>              dynamic major:minor (via sysfs) [mounting a registered
>>>              filesystem is easy in comparison]
>>> netlink      - stream based, therefore involves numerous copies.
>>>
>>> Debugfs is the lesser among the evils here, thereby we have grown used to
>>> the convenience and flexibility in presentation that debugfs gives us
>>> (including relayfs inodes) that we want some of that hierachy in stable user
>>> ABI form.
>>
>> Seriously, why ? A subsystem growing its own file system sounds so wrong. It
>> seems that you want to have all the benefits of a stable ABI without going
>> through the standardization effort that this requires. I can see so many ways
>> that drmfs could be abused, with drivers throwing in new data with little or
>> no review. You'll need very compelling arguments to convince me.
>
> This is not unlike my sentiments on the first version posted
> [1]. There's also the distinct feeling of [2]. Suffice it to say at this
> time that I'm dubious, not convinced enough to defend this.
>
> Swati, please refrain from posting new versions of the patches until
> there's some consensus one way or the other; it's counter-productive to
> keep splitting off the discussion into several patch series threads at
> this stage. Let's have the discussion here.
>

I'll echo Laurent's concerns here, seems like the goal is easy to
merge, hard to change. I think the problem with this is that easy to
merge usually leads to designs which need to change :)

Secondly, I'm not sure there's value outside of i915, perhaps I'm
missing use cases for other drivers.

Sean


>
> BR,
> Jani.
>
>
> [1] http://mid.mail-archive.com/87lgw0xcf4.fsf@intel.com
> [2] https://xkcd.com/927/
>
>>
>>> Due to these limitations, there is a need for a new pseudo filesytem, that
>>> would act as a stable 'free-for-all' ABI, with the heirarchial structure and
>>> thus convenience of debugfs. This will be managed by drm, thus named
>>> 'drmfs'. DRM would register this filesystem to manage a canonical
>>> mountpoint, but this wouldn't limit everyone to only using that pseudofs
>>> underneath.
>>>
>>> This can serve to hold various kinds of output data from Linux DRM
>>> subsystems, for the files which can't truely fit anywhere else with
>>> existing ABI's but present so, for the lack of a better place.
>>>
>>> In this patch series, we have introduced a pseudo filesystem named as
>>> 'drmfs' for now. The filesystem is introduced in the first patch, and the
>>> subsequent patches make use of the filesystem interfaces, in drm driver,
>>> and making them available for use by the drm subsystem components, one of
>>> which is i915. We've moved the location of i915 GuC logs from debugfs to
>>> drmfs in the last patch. Subsequently, more such files such as pipe_crc,
>>> error states, memory stats, etc. can be move to this filesystem, if the
>>> idea introduced here is acceptable per se. The filesystem introduced is
>>> being used to house the data generated by i915 driver in this patch series,
>>> but will hopefully be generic enough to provide scope for usage by any
>>> other drm subsystem component.
>>>
>>> The patch series is being floated as RFC to gather feedback on the idea and
>>> infrastructure proposed here and it's suitability to address the specific
>>> problem statement/use case.
>>>
>>> v2: fix the bat failures caused due to missing config check
>>>
>>> v3: Changes made:
>>>     - Move the location of drmfs from fs/ to drivers/gpu/drm/ (Chris)
>>>     - Moving config checks to header (Chris,Daniel)
>>>
>>> v4: Added the kernel Documentaion (using Sphinx).
>>>
>>> Sourab Gupta (4):
>>>   drm: Introduce drmfs pseudo filesystem interfaces
>>>   drm: Register drmfs filesystem from drm init
>>>   drm: Create driver specific root directory inside drmfs
>>>   drm/i915: Creating guc log file in drmfs instead of debugfs
>>>
>>>  Documentation/gpu/drm-uapi.rst             |  76 ++++
>>>  drivers/gpu/drm/Kconfig                    |   9 +
>>>  drivers/gpu/drm/Makefile                   |   1 +
>>>  drivers/gpu/drm/drm_drv.c                  |  26 ++
>>>  drivers/gpu/drm/drmfs.c                    | 566 ++++++++++++++++++++++++++
>>>  drivers/gpu/drm/i915/i915_guc_submission.c |  33 +-
>>>  include/drm/drm_drv.h                      |   3 +
>>>  include/drm/drmfs.h                        |  77 ++++
>>>  include/uapi/linux/magic.h                 |   3 +
>>>  9 files changed, 773 insertions(+), 21 deletions(-)
>>>  create mode 100644 drivers/gpu/drm/drmfs.c
>>>  create mode 100644 include/drm/drmfs.h
>
> --
> Jani Nikula, Intel Open Source Technology Center
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel



-- 
Sean Paul, Software Engineer, Google / Chromium OS
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [RFC 0/4] Introduce drmfs pseudo filesystem for drm subsystem

[sourab gupta]

On Tue, 2016-12-20 at 01:44 -0800, Jani Nikula wrote:
> On Tue, 20 Dec 2016, Laurent Pinchart <laurent.pinchart@ideasonboard.com> wrote:
> > Hi Swati,
> >
> > On Monday 19 Dec 2016 16:12:22 swati.dhingra@intel.com wrote:
> >> From: Swati Dhingra <swati.dhingra@intel.com>
> >>
> >> Currently, we don't have a stable ABI which can be used for the purpose of
> >> providing output debug/loggging/crc and other such data from DRM.
> >> The ABI in current use (filesystems, ioctls, et al.) have their own
> >> constraints and are intended to output a particular type of data.
> >> Few cases in point:
> >> sysfs        - stable ABI, but constrained to one textual value per file
> >> debugfs      - unstable ABI, free-for-all
> >> ioctls       - not as suitable to many single purpose continuous data
> >>        dumping, we would very quickly run out ioctl space; requires more
> >>        userspace support than "cat"
> >> device nodes -  a real possibilty, kernel instantiation is more tricky,
> >>              requires udev (+udev.rules) or userspace discovery of the
> >>              dynamic major:minor (via sysfs) [mounting a registered
> >>              filesystem is easy in comparison]
> >> netlink      - stream based, therefore involves numerous copies.
> >>
> >> Debugfs is the lesser among the evils here, thereby we have grown used to
> >> the convenience and flexibility in presentation that debugfs gives us
> >> (including relayfs inodes) that we want some of that hierachy in stable user
> >> ABI form.
> >
> > Seriously, why ? A subsystem growing its own file system sounds so wrong. It
> > seems that you want to have all the benefits of a stable ABI without going
> > through the standardization effort that this requires. I can see so many ways
> > that drmfs could be abused, with drivers throwing in new data with little or
> > no review. You'll need very compelling arguments to convince me.
> 
> This is not unlike my sentiments on the first version posted
> [1]. There's also the distinct feeling of [2]. Suffice it to say at this
> time that I'm dubious, not convinced enough to defend this.
> 
> Swati, please refrain from posting new versions of the patches until
> there's some consensus one way or the other; it's counter-productive to
> keep splitting off the discussion into several patch series threads at
> this stage. Let's have the discussion here.
> 
> 
> BR,
> Jani.
> 

Hi Jani,

One of the usecases we're exploring here is for pipe_crc to be
implemented over drmfs, instead of debugfs. But we're not sure whether
that'll also assuage the standardization questions/concerns.

Can you let us know how to proceed (since you've mentioned not to post
new version of patches). How do we address the standardization concerns
put forth on drmfs? Does simply having more *suitable* usecases suffice?
If not, What else should the arguments be in favor of drmfs?

Hi Chris,
Can you pitch in with your inputs in the discussion, and help address
the standardization concerns?

Regards,
Sourab

> 
> [1] http://mid.mail-archive.com/87lgw0xcf4.fsf@intel.com
> [2] https://xkcd.com/927/
> 
> >
> >> Due to these limitations, there is a need for a new pseudo filesytem, that
> >> would act as a stable 'free-for-all' ABI, with the heirarchial structure and
> >> thus convenience of debugfs. This will be managed by drm, thus named
> >> 'drmfs'. DRM would register this filesystem to manage a canonical
> >> mountpoint, but this wouldn't limit everyone to only using that pseudofs
> >> underneath.
> >>
> >> This can serve to hold various kinds of output data from Linux DRM
> >> subsystems, for the files which can't truely fit anywhere else with
> >> existing ABI's but present so, for the lack of a better place.
> >>
> >> In this patch series, we have introduced a pseudo filesystem named as
> >> 'drmfs' for now. The filesystem is introduced in the first patch, and the
> >> subsequent patches make use of the filesystem interfaces, in drm driver,
> >> and making them available for use by the drm subsystem components, one of
> >> which is i915. We've moved the location of i915 GuC logs from debugfs to
> >> drmfs in the last patch. Subsequently, more such files such as pipe_crc,
> >> error states, memory stats, etc. can be move to this filesystem, if the
> >> idea introduced here is acceptable per se. The filesystem introduced is
> >> being used to house the data generated by i915 driver in this patch series,
> >> but will hopefully be generic enough to provide scope for usage by any
> >> other drm subsystem component.
> >>
> >> The patch series is being floated as RFC to gather feedback on the idea and
> >> infrastructure proposed here and it's suitability to address the specific
> >> problem statement/use case.
> >>
> >> v2: fix the bat failures caused due to missing config check
> >>
> >> v3: Changes made:
> >>     - Move the location of drmfs from fs/ to drivers/gpu/drm/ (Chris)
> >>     - Moving config checks to header (Chris,Daniel)
> >>
> >> v4: Added the kernel Documentaion (using Sphinx).
> >>
> >> Sourab Gupta (4):
> >>   drm: Introduce drmfs pseudo filesystem interfaces
> >>   drm: Register drmfs filesystem from drm init
> >>   drm: Create driver specific root directory inside drmfs
> >>   drm/i915: Creating guc log file in drmfs instead of debugfs
> >>
> >>  Documentation/gpu/drm-uapi.rst             |  76 ++++
> >>  drivers/gpu/drm/Kconfig                    |   9 +
> >>  drivers/gpu/drm/Makefile                   |   1 +
> >>  drivers/gpu/drm/drm_drv.c                  |  26 ++
> >>  drivers/gpu/drm/drmfs.c                    | 566 ++++++++++++++++++++++++++
> >>  drivers/gpu/drm/i915/i915_guc_submission.c |  33 +-
> >>  include/drm/drm_drv.h                      |   3 +
> >>  include/drm/drmfs.h                        |  77 ++++
> >>  include/uapi/linux/magic.h                 |   3 +
> >>  9 files changed, 773 insertions(+), 21 deletions(-)
> >>  create mode 100644 drivers/gpu/drm/drmfs.c
> >>  create mode 100644 include/drm/drmfs.h
> 
> --
> Jani Nikula, Intel Open Source Technology Center


_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [RFC 0/4] Introduce drmfs pseudo filesystem for drm subsystem

[Jani Nikula]

On Thu, 09 Feb 2017, sourab gupta <sourab.gupta@intel.com> wrote:
> On Tue, 2016-12-20 at 01:44 -0800, Jani Nikula wrote:
>> On Tue, 20 Dec 2016, Laurent Pinchart <laurent.pinchart@ideasonboard.com> wrote:
>> > Hi Swati,
>> >
>> > On Monday 19 Dec 2016 16:12:22 swati.dhingra@intel.com wrote:
>> >> From: Swati Dhingra <swati.dhingra@intel.com>
>> >>
>> >> Currently, we don't have a stable ABI which can be used for the purpose of
>> >> providing output debug/loggging/crc and other such data from DRM.
>> >> The ABI in current use (filesystems, ioctls, et al.) have their own
>> >> constraints and are intended to output a particular type of data.
>> >> Few cases in point:
>> >> sysfs        - stable ABI, but constrained to one textual value per file
>> >> debugfs      - unstable ABI, free-for-all
>> >> ioctls       - not as suitable to many single purpose continuous data
>> >>        dumping, we would very quickly run out ioctl space; requires more
>> >>        userspace support than "cat"
>> >> device nodes -  a real possibilty, kernel instantiation is more tricky,
>> >>              requires udev (+udev.rules) or userspace discovery of the
>> >>              dynamic major:minor (via sysfs) [mounting a registered
>> >>              filesystem is easy in comparison]
>> >> netlink      - stream based, therefore involves numerous copies.
>> >>
>> >> Debugfs is the lesser among the evils here, thereby we have grown used to
>> >> the convenience and flexibility in presentation that debugfs gives us
>> >> (including relayfs inodes) that we want some of that hierachy in stable user
>> >> ABI form.
>> >
>> > Seriously, why ? A subsystem growing its own file system sounds so wrong. It
>> > seems that you want to have all the benefits of a stable ABI without going
>> > through the standardization effort that this requires. I can see so many ways
>> > that drmfs could be abused, with drivers throwing in new data with little or
>> > no review. You'll need very compelling arguments to convince me.
>> 
>> This is not unlike my sentiments on the first version posted
>> [1]. There's also the distinct feeling of [2]. Suffice it to say at this
>> time that I'm dubious, not convinced enough to defend this.
>> 
>> Swati, please refrain from posting new versions of the patches until
>> there's some consensus one way or the other; it's counter-productive to
>> keep splitting off the discussion into several patch series threads at
>> this stage. Let's have the discussion here.
>> 
>> 
>> BR,
>> Jani.
>> 
>
> Hi Jani,
>
> One of the usecases we're exploring here is for pipe_crc to be
> implemented over drmfs, instead of debugfs. But we're not sure whether
> that'll also assuage the standardization questions/concerns.
>
> Can you let us know how to proceed (since you've mentioned not to post
> new version of patches). How do we address the standardization concerns
> put forth on drmfs? Does simply having more *suitable* usecases suffice?
> If not, What else should the arguments be in favor of drmfs?

I've asked not to post new versions of the patches, because the order of
business is 1) see if the idea makes sense, 2) see if the implementation
of the idea makes sense, 3) see if the implementation is polished
enough. We're still at 1), and doing more of 2) or 3) is wasted effort
if we never get past 1).

I think it's better that you try to convince *other* drm folks than me
at this point. I'm not convinced enough myself that I'd start advocating
this to others on your behalf, but I'm also not opposed enough that I'd
keep arguing after you've convinced others. I don't have the final say
on merging this or not, and even if I did I wouldn't do so without acks
from non-Intel drm folks that you need to convince anyway.

So I'm kind of saying trying convince me is a wasted effort too. ;)


BR,
Jani.


>
> Hi Chris,
> Can you pitch in with your inputs in the discussion, and help address
> the standardization concerns?
>
> Regards,
> Sourab
>
>> 
>> [1] http://mid.mail-archive.com/87lgw0xcf4.fsf@intel.com
>> [2] https://xkcd.com/927/
>> 
>> >
>> >> Due to these limitations, there is a need for a new pseudo filesytem, that
>> >> would act as a stable 'free-for-all' ABI, with the heirarchial structure and
>> >> thus convenience of debugfs. This will be managed by drm, thus named
>> >> 'drmfs'. DRM would register this filesystem to manage a canonical
>> >> mountpoint, but this wouldn't limit everyone to only using that pseudofs
>> >> underneath.
>> >>
>> >> This can serve to hold various kinds of output data from Linux DRM
>> >> subsystems, for the files which can't truely fit anywhere else with
>> >> existing ABI's but present so, for the lack of a better place.
>> >>
>> >> In this patch series, we have introduced a pseudo filesystem named as
>> >> 'drmfs' for now. The filesystem is introduced in the first patch, and the
>> >> subsequent patches make use of the filesystem interfaces, in drm driver,
>> >> and making them available for use by the drm subsystem components, one of
>> >> which is i915. We've moved the location of i915 GuC logs from debugfs to
>> >> drmfs in the last patch. Subsequently, more such files such as pipe_crc,
>> >> error states, memory stats, etc. can be move to this filesystem, if the
>> >> idea introduced here is acceptable per se. The filesystem introduced is
>> >> being used to house the data generated by i915 driver in this patch series,
>> >> but will hopefully be generic enough to provide scope for usage by any
>> >> other drm subsystem component.
>> >>
>> >> The patch series is being floated as RFC to gather feedback on the idea and
>> >> infrastructure proposed here and it's suitability to address the specific
>> >> problem statement/use case.
>> >>
>> >> v2: fix the bat failures caused due to missing config check
>> >>
>> >> v3: Changes made:
>> >>     - Move the location of drmfs from fs/ to drivers/gpu/drm/ (Chris)
>> >>     - Moving config checks to header (Chris,Daniel)
>> >>
>> >> v4: Added the kernel Documentaion (using Sphinx).
>> >>
>> >> Sourab Gupta (4):
>> >>   drm: Introduce drmfs pseudo filesystem interfaces
>> >>   drm: Register drmfs filesystem from drm init
>> >>   drm: Create driver specific root directory inside drmfs
>> >>   drm/i915: Creating guc log file in drmfs instead of debugfs
>> >>
>> >>  Documentation/gpu/drm-uapi.rst             |  76 ++++
>> >>  drivers/gpu/drm/Kconfig                    |   9 +
>> >>  drivers/gpu/drm/Makefile                   |   1 +
>> >>  drivers/gpu/drm/drm_drv.c                  |  26 ++
>> >>  drivers/gpu/drm/drmfs.c                    | 566 ++++++++++++++++++++++++++
>> >>  drivers/gpu/drm/i915/i915_guc_submission.c |  33 +-
>> >>  include/drm/drm_drv.h                      |   3 +
>> >>  include/drm/drmfs.h                        |  77 ++++
>> >>  include/uapi/linux/magic.h                 |   3 +
>> >>  9 files changed, 773 insertions(+), 21 deletions(-)
>> >>  create mode 100644 drivers/gpu/drm/drmfs.c
>> >>  create mode 100644 include/drm/drmfs.h
>> 
>> --
>> Jani Nikula, Intel Open Source Technology Center
>
>

-- 
Jani Nikula, Intel Open Source Technology Center
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [RFC 0/4] Introduce drmfs pseudo filesystem for drm subsystem

[sourab gupta]

On Mon, 2016-12-19 at 17:15 -0800, Laurent Pinchart wrote:
> Hi Swati,
> 
> On Monday 19 Dec 2016 16:12:22 swati.dhingra@intel.com wrote:
> > From: Swati Dhingra <swati.dhingra@intel.com>
> >
> > Currently, we don't have a stable ABI which can be used for the purpose of
> > providing output debug/loggging/crc and other such data from DRM.
> > The ABI in current use (filesystems, ioctls, et al.) have their own
> > constraints and are intended to output a particular type of data.
> > Few cases in point:
> > sysfs - stable ABI, but constrained to one textual value per file
> > debugfs       - unstable ABI, free-for-all
> > ioctls        - not as suitable to many single purpose continuous data
> >         dumping, we would very quickly run out ioctl space; requires more
> >         userspace support than "cat"
> > device nodes -  a real possibilty, kernel instantiation is more tricky,
> >               requires udev (+udev.rules) or userspace discovery of the
> >               dynamic major:minor (via sysfs) [mounting a registered
> >               filesystem is easy in comparison]
> > netlink       - stream based, therefore involves numerous copies.
> >
> > Debugfs is the lesser among the evils here, thereby we have grown used to
> > the convenience and flexibility in presentation that debugfs gives us
> > (including relayfs inodes) that we want some of that hierachy in stable user
> > ABI form.
> 
> Seriously, why ? A subsystem growing its own file system sounds so wrong. It
> seems that you want to have all the benefits of a stable ABI without going
> through the standardization effort that this requires. I can see so many ways
> that drmfs could be abused, with drivers throwing in new data with little or
> no review. You'll need very compelling arguments to convince me.
> 

Hi Laurent,
Can you please let us know how to address the standardization issues? As
per our (limited) knowledge, drmfs seemed to be the most suitable
solution for exposing usecases such as microprocessor logs in i915, and
possibly other such usecases, which ideally can't fit in with
sysfs/debugfs/ioctls due to reasons mentioned above.

But having said that, standardization requires a lot more effort
(defining the constraints etc.), which we're not familiar with, frankly.
Can you please provide your views on how to proceed as such, since the
idea seemed worth pursuing to us (It's a drm based filesystem, whose
existence depends on drm, and directory contents solely controlled by
the corresponding drm driver - as such the contents shouldn't be
controllable by an external driver).
Or, should this be dropped, if the idea of a subsystem having its own
filesystem is fundamentally wrong to its core?

Regards,
Sourab

> > Due to these limitations, there is a need for a new pseudo filesytem, that
> > would act as a stable 'free-for-all' ABI, with the heirarchial structure and
> > thus convenience of debugfs. This will be managed by drm, thus named
> > 'drmfs'. DRM would register this filesystem to manage a canonical
> > mountpoint, but this wouldn't limit everyone to only using that pseudofs
> > underneath.
> >
> > This can serve to hold various kinds of output data from Linux DRM
> > subsystems, for the files which can't truely fit anywhere else with
> > existing ABI's but present so, for the lack of a better place.
> >
> > In this patch series, we have introduced a pseudo filesystem named as
> > 'drmfs' for now. The filesystem is introduced in the first patch, and the
> > subsequent patches make use of the filesystem interfaces, in drm driver,
> > and making them available for use by the drm subsystem components, one of
> > which is i915. We've moved the location of i915 GuC logs from debugfs to
> > drmfs in the last patch. Subsequently, more such files such as pipe_crc,
> > error states, memory stats, etc. can be move to this filesystem, if the
> > idea introduced here is acceptable per se. The filesystem introduced is
> > being used to house the data generated by i915 driver in this patch series,
> > but will hopefully be generic enough to provide scope for usage by any
> > other drm subsystem component.
> >
> > The patch series is being floated as RFC to gather feedback on the idea and
> > infrastructure proposed here and it's suitability to address the specific
> > problem statement/use case.
> >
> > v2: fix the bat failures caused due to missing config check
> >
> > v3: Changes made:
> >     - Move the location of drmfs from fs/ to drivers/gpu/drm/ (Chris)
> >     - Moving config checks to header (Chris,Daniel)
> >
> > v4: Added the kernel Documentaion (using Sphinx).
> >
> > Sourab Gupta (4):
> >   drm: Introduce drmfs pseudo filesystem interfaces
> >   drm: Register drmfs filesystem from drm init
> >   drm: Create driver specific root directory inside drmfs
> >   drm/i915: Creating guc log file in drmfs instead of debugfs
> >
> >  Documentation/gpu/drm-uapi.rst             |  76 ++++
> >  drivers/gpu/drm/Kconfig                    |   9 +
> >  drivers/gpu/drm/Makefile                   |   1 +
> >  drivers/gpu/drm/drm_drv.c                  |  26 ++
> >  drivers/gpu/drm/drmfs.c                    | 566 ++++++++++++++++++++++++++
> >  drivers/gpu/drm/i915/i915_guc_submission.c |  33 +-
> >  include/drm/drm_drv.h                      |   3 +
> >  include/drm/drmfs.h                        |  77 ++++
> >  include/uapi/linux/magic.h                 |   3 +
> >  9 files changed, 773 insertions(+), 21 deletions(-)
> >  create mode 100644 drivers/gpu/drm/drmfs.c
> >  create mode 100644 include/drm/drmfs.h
> 
> --
> Regards,
> 
> Laurent Pinchart
> 


_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [RFC 0/4] Introduce drmfs pseudo filesystem for drm subsystem

[sourab gupta]

On Mon, 2016-12-12 at 07:33 -0800, Alex Deucher wrote:
> On Mon, Dec 12, 2016 at 1:14 AM, sourab gupta <sourab.gupta@intel.com> wrote:
> > On Mon, 2016-12-05 at 03:06 -0800, Dhingra, Swati wrote:
> >> From: Swati Dhingra <swati.dhingra@intel.com>
> >>
> >> Currently, we don't have a stable ABI which can be used for the purpose of
> >> providing output debug/loggging/crc and other such data from DRM.
> >> The ABI in current use (filesystems, ioctls, et al.) have their own
> >> constraints and are intended to output a particular type of data.
> >> Few cases in point:
> >> sysfs   - stable ABI, but constrained to one textual value per file
> >> debugfs - unstable ABI, free-for-all
> >> ioctls  - not as suitable to many single purpose continuous data
> >>           dumping, we would very quickly run out ioctl space; requires more
> >>           userspace support than "cat"
> >> device nodes -  a real possibilty, kernel instantiation is more tricky,
> >>                 requires udev (+udev.rules) or userspace discovery of the
> >>                 dynamic major:minor (via sysfs) [mounting a registered
> >>                 filesystem is easy in comparison]
> >> netlink - stream based, therefore involves numerous copies.
> >>
> >> Debugfs is the lesser among the evils here, thereby we have grown used to the
> >> convenience and flexibility in presentation that debugfs gives us
> >> (including relayfs inodes) that we want some of that hierachy in stable user
> >> ABI form.
> >>
> >> Due to these limitations, there is a need for a new pseudo filesytem, that
> >> would act as a stable 'free-for-all' ABI, with the heirarchial structure and
> >> thus convenience of debugfs. This will be managed by drm, thus named 'drmfs'.
> >> DRM would register this filesystem to manage a canonical mountpoint, but this
> >> wouldn't limit everyone to only using that pseudofs underneath.
> >>
> >> This can serve to hold various kinds of output data from Linux DRM subsystems,
> >> for the files which can't truely fit anywhere else with existing ABI's but
> >> present so, for the lack of a better place.
> >>
> >> In this patch series, we have introduced a pseudo filesystem named as 'drmfs'
> >> for now. The filesystem is introduced in the first patch, and the subsequent
> >> patches make use of the filesystem interfaces, in drm driver, and making them
> >> available for use by the drm subsystem components, one of which is i915.
> >> We've moved the location of i915 GuC logs from debugfs to drmfs in the last
> >> patch. Subsequently, more such files such as pipe_crc, error states, memory
> >> stats, etc. can be move to this filesystem, if the idea introduced here is
> >> acceptable per se. The filesystem introduced is being used to house the data
> >> generated by i915 driver in this patch series, but will hopefully be generic
> >> enough to provide scope for usage by any other drm subsystem component.
> >>
> >> The patch series is being floated as RFC to gather feedback on the idea and
> >> infrastructure proposed here and it's suitability to address the specific
> >> problem statement/use case.
> >>
> >> TODO: Create documentation. Will do so in next version.
> >>
> >> v2: fix the bat failures caused due to missing config check
> >>
> >> v3: Changes made:
> >>     - Move the location of drmfs from fs/ to drivers/gpu/drm/ (Chris)
> >>     - Moving config checks to header (Chris,Daniel)
> >>
> >>
> >> Sourab Gupta (4):
> >>   drm: Introduce drmfs pseudo filesystem interfaces
> >>   drm: Register drmfs filesystem from drm init
> >>   drm: Create driver specific root directory inside drmfs
> >>   drm/i915: Creating guc log file in drmfs instead of debugfs
> >>
> >>  drivers/gpu/drm/Kconfig                    |   9 +
> >>  drivers/gpu/drm/Makefile                   |   1 +
> >>  drivers/gpu/drm/drm_drv.c                  |  12 +
> >>  drivers/gpu/drm/drmfs.c                    | 555 +++++++++++++++++++++++++++++
> >>  drivers/gpu/drm/i915/i915_guc_submission.c |  33 +-
> >>  include/drm/drm_drv.h                      |   3 +
> >>  include/drm/drmfs.h                        |  77 ++++
> >>  include/uapi/linux/magic.h                 |   3 +
> >>  8 files changed, 672 insertions(+), 21 deletions(-)
> >>  create mode 100644 drivers/gpu/drm/drmfs.c
> >>  create mode 100644 include/drm/drmfs.h
> >>
> >> --
> >> 2.7.4
> >>
> >
> > Hi dri-devel folks,
> >
> > Any feedback on the proposed drmfs infrastructure being proposed here?
> 
> I haven't looked too closely, but so far we haven't run into anything
> that we haven't been able to support via some combination of sysfs and
> debugfs.
> 
> Alex

Hi Alex,

The intent here is to introduce a stable free-for-all ABI which has the
hierarchical structure and convenience of debugfs. One of the possible
candidate/usecase here is for holding the GuC (micro-controller) logs.
Currently, we're using relayfs to provide these logs to userspace.
Relayfs needs a parent dentry to be associated with log file, and also
we need these logs to be available in production kernels. In absence of
any other suitable candidate, debugfs is used currently, since it
provides the heirarchial VFS structures available (dentry et al.). But
still we felt the need for a stable ABI, which is modeled on lines of
debugfs but which we can control fully.

Chris,
Can you add on if I'm not fully clear.

Regards,
Sourab


_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [RFC 0/4] Introduce drmfs pseudo filesystem for drm subsystem

[Alex Deucher]

On Mon, Dec 12, 2016 at 1:14 AM, sourab gupta <sourab.gupta@intel.com> wrote:
> On Mon, 2016-12-05 at 03:06 -0800, Dhingra, Swati wrote:
>> From: Swati Dhingra <swati.dhingra@intel.com>
>>
>> Currently, we don't have a stable ABI which can be used for the purpose of
>> providing output debug/loggging/crc and other such data from DRM.
>> The ABI in current use (filesystems, ioctls, et al.) have their own
>> constraints and are intended to output a particular type of data.
>> Few cases in point:
>> sysfs   - stable ABI, but constrained to one textual value per file
>> debugfs - unstable ABI, free-for-all
>> ioctls  - not as suitable to many single purpose continuous data
>>           dumping, we would very quickly run out ioctl space; requires more
>>           userspace support than "cat"
>> device nodes -  a real possibilty, kernel instantiation is more tricky,
>>                 requires udev (+udev.rules) or userspace discovery of the
>>                 dynamic major:minor (via sysfs) [mounting a registered
>>                 filesystem is easy in comparison]
>> netlink - stream based, therefore involves numerous copies.
>>
>> Debugfs is the lesser among the evils here, thereby we have grown used to the
>> convenience and flexibility in presentation that debugfs gives us
>> (including relayfs inodes) that we want some of that hierachy in stable user
>> ABI form.
>>
>> Due to these limitations, there is a need for a new pseudo filesytem, that
>> would act as a stable 'free-for-all' ABI, with the heirarchial structure and
>> thus convenience of debugfs. This will be managed by drm, thus named 'drmfs'.
>> DRM would register this filesystem to manage a canonical mountpoint, but this
>> wouldn't limit everyone to only using that pseudofs underneath.
>>
>> This can serve to hold various kinds of output data from Linux DRM subsystems,
>> for the files which can't truely fit anywhere else with existing ABI's but
>> present so, for the lack of a better place.
>>
>> In this patch series, we have introduced a pseudo filesystem named as 'drmfs'
>> for now. The filesystem is introduced in the first patch, and the subsequent
>> patches make use of the filesystem interfaces, in drm driver, and making them
>> available for use by the drm subsystem components, one of which is i915.
>> We've moved the location of i915 GuC logs from debugfs to drmfs in the last
>> patch. Subsequently, more such files such as pipe_crc, error states, memory
>> stats, etc. can be move to this filesystem, if the idea introduced here is
>> acceptable per se. The filesystem introduced is being used to house the data
>> generated by i915 driver in this patch series, but will hopefully be generic
>> enough to provide scope for usage by any other drm subsystem component.
>>
>> The patch series is being floated as RFC to gather feedback on the idea and
>> infrastructure proposed here and it's suitability to address the specific
>> problem statement/use case.
>>
>> TODO: Create documentation. Will do so in next version.
>>
>> v2: fix the bat failures caused due to missing config check
>>
>> v3: Changes made:
>>     - Move the location of drmfs from fs/ to drivers/gpu/drm/ (Chris)
>>     - Moving config checks to header (Chris,Daniel)
>>
>>
>> Sourab Gupta (4):
>>   drm: Introduce drmfs pseudo filesystem interfaces
>>   drm: Register drmfs filesystem from drm init
>>   drm: Create driver specific root directory inside drmfs
>>   drm/i915: Creating guc log file in drmfs instead of debugfs
>>
>>  drivers/gpu/drm/Kconfig                    |   9 +
>>  drivers/gpu/drm/Makefile                   |   1 +
>>  drivers/gpu/drm/drm_drv.c                  |  12 +
>>  drivers/gpu/drm/drmfs.c                    | 555 +++++++++++++++++++++++++++++
>>  drivers/gpu/drm/i915/i915_guc_submission.c |  33 +-
>>  include/drm/drm_drv.h                      |   3 +
>>  include/drm/drmfs.h                        |  77 ++++
>>  include/uapi/linux/magic.h                 |   3 +
>>  8 files changed, 672 insertions(+), 21 deletions(-)
>>  create mode 100644 drivers/gpu/drm/drmfs.c
>>  create mode 100644 include/drm/drmfs.h
>>
>> --
>> 2.7.4
>>
>
> Hi dri-devel folks,
>
> Any feedback on the proposed drmfs infrastructure being proposed here?

I haven't looked too closely, but so far we haven't run into anything
that we haven't been able to support via some combination of sysfs and
debugfs.

Alex
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [RFC 0/4] Introduce drmfs pseudo filesystem for drm subsystem

[sourab gupta]

On Mon, 2016-12-05 at 03:06 -0800, Dhingra, Swati wrote:
> From: Swati Dhingra <swati.dhingra@intel.com>
> 
> Currently, we don't have a stable ABI which can be used for the purpose of
> providing output debug/loggging/crc and other such data from DRM.
> The ABI in current use (filesystems, ioctls, et al.) have their own
> constraints and are intended to output a particular type of data.
> Few cases in point:
> sysfs   - stable ABI, but constrained to one textual value per file
> debugfs - unstable ABI, free-for-all
> ioctls  - not as suitable to many single purpose continuous data
>           dumping, we would very quickly run out ioctl space; requires more
>           userspace support than "cat"
> device nodes -  a real possibilty, kernel instantiation is more tricky,
>                 requires udev (+udev.rules) or userspace discovery of the
>                 dynamic major:minor (via sysfs) [mounting a registered
>                 filesystem is easy in comparison]
> netlink - stream based, therefore involves numerous copies.
> 
> Debugfs is the lesser among the evils here, thereby we have grown used to the
> convenience and flexibility in presentation that debugfs gives us
> (including relayfs inodes) that we want some of that hierachy in stable user
> ABI form.
> 
> Due to these limitations, there is a need for a new pseudo filesytem, that
> would act as a stable 'free-for-all' ABI, with the heirarchial structure and
> thus convenience of debugfs. This will be managed by drm, thus named 'drmfs'.
> DRM would register this filesystem to manage a canonical mountpoint, but this
> wouldn't limit everyone to only using that pseudofs underneath.
> 
> This can serve to hold various kinds of output data from Linux DRM subsystems,
> for the files which can't truely fit anywhere else with existing ABI's but
> present so, for the lack of a better place.
> 
> In this patch series, we have introduced a pseudo filesystem named as 'drmfs'
> for now. The filesystem is introduced in the first patch, and the subsequent
> patches make use of the filesystem interfaces, in drm driver, and making them
> available for use by the drm subsystem components, one of which is i915.
> We've moved the location of i915 GuC logs from debugfs to drmfs in the last
> patch. Subsequently, more such files such as pipe_crc, error states, memory
> stats, etc. can be move to this filesystem, if the idea introduced here is
> acceptable per se. The filesystem introduced is being used to house the data
> generated by i915 driver in this patch series, but will hopefully be generic
> enough to provide scope for usage by any other drm subsystem component.
> 
> The patch series is being floated as RFC to gather feedback on the idea and
> infrastructure proposed here and it's suitability to address the specific
> problem statement/use case.
> 
> TODO: Create documentation. Will do so in next version.
> 
> v2: fix the bat failures caused due to missing config check
> 
> v3: Changes made:
>     - Move the location of drmfs from fs/ to drivers/gpu/drm/ (Chris)
>     - Moving config checks to header (Chris,Daniel)
> 
> 
> Sourab Gupta (4):
>   drm: Introduce drmfs pseudo filesystem interfaces
>   drm: Register drmfs filesystem from drm init
>   drm: Create driver specific root directory inside drmfs
>   drm/i915: Creating guc log file in drmfs instead of debugfs
> 
>  drivers/gpu/drm/Kconfig                    |   9 +
>  drivers/gpu/drm/Makefile                   |   1 +
>  drivers/gpu/drm/drm_drv.c                  |  12 +
>  drivers/gpu/drm/drmfs.c                    | 555 +++++++++++++++++++++++++++++
>  drivers/gpu/drm/i915/i915_guc_submission.c |  33 +-
>  include/drm/drm_drv.h                      |   3 +
>  include/drm/drmfs.h                        |  77 ++++
>  include/uapi/linux/magic.h                 |   3 +
>  8 files changed, 672 insertions(+), 21 deletions(-)
>  create mode 100644 drivers/gpu/drm/drmfs.c
>  create mode 100644 include/drm/drmfs.h
> 
> --
> 2.7.4
> 

Hi dri-devel folks,

Any feedback on the proposed drmfs infrastructure being proposed here?

Regards,
Sourab


_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[RFC 0/4] Introduce drmfs pseudo filesystem for drm subsystem

[swati.dhingra]

From: Swati Dhingra <swati.dhingra@intel.com>

Currently, we don't have a stable ABI which can be used for the purpose of
providing output debug/loggging/crc and other such data from DRM.
The ABI in current use (filesystems, ioctls, et al.) have their own
constraints and are intended to output a particular type of data.
Few cases in point:
sysfs	- stable ABI, but constrained to one textual value per file
debugfs	- unstable ABI, free-for-all
ioctls	- not as suitable to many single purpose continuous data
	  dumping, we would very quickly run out ioctl space; requires more
	  userspace support than "cat"
device nodes -  a real possibilty, kernel instantiation is more tricky,
		requires udev (+udev.rules) or userspace discovery of the
		dynamic major:minor (via sysfs) [mounting a registered
		filesystem is easy in comparison]
netlink	- stream based, therefore involves numerous copies.

Debugfs is the lesser among the evils here, thereby we have grown used to the
convenience and flexibility in presentation that debugfs gives us
(including relayfs inodes) that we want some of that hierachy in stable user
ABI form.

Due to these limitations, there is a need for a new pseudo filesytem, that
would act as a stable 'free-for-all' ABI, with the heirarchial structure and
thus convenience of debugfs. This will be managed by drm, thus named 'drmfs'.
DRM would register this filesystem to manage a canonical mountpoint, but this
wouldn't limit everyone to only using that pseudofs underneath.

This can serve to hold various kinds of output data from Linux DRM subsystems,
for the files which can't truely fit anywhere else with existing ABI's but
present so, for the lack of a better place.

In this patch series, we have introduced a pseudo filesystem named as 'drmfs'
for now. The filesystem is introduced in the first patch, and the subsequent
patches make use of the filesystem interfaces, in drm driver, and making them
available for use by the drm subsystem components, one of which is i915.
We've moved the location of i915 GuC logs from debugfs to drmfs in the last
patch. Subsequently, more such files such as pipe_crc, error states, memory
stats, etc. can be move to this filesystem, if the idea introduced here is
acceptable per se. The filesystem introduced is being used to house the data
generated by i915 driver in this patch series, but will hopefully be generic
enough to provide scope for usage by any other drm subsystem component.

The patch series is being floated as RFC to gather feedback on the idea and
infrastructure proposed here and it's suitability to address the specific
problem statement/use case.

TODO: Create documentation. Will do so in next version.

v2: fix the bat failures caused due to missing config check

v3: Changes made:
    - Move the location of drmfs from fs/ to drivers/gpu/drm/ (Chris)
    - Moving config checks to header (Chris,Daniel)


Sourab Gupta (4):
  drm: Introduce drmfs pseudo filesystem interfaces
  drm: Register drmfs filesystem from drm init
  drm: Create driver specific root directory inside drmfs
  drm/i915: Creating guc log file in drmfs instead of debugfs

 drivers/gpu/drm/Kconfig                    |   9 +
 drivers/gpu/drm/Makefile                   |   1 +
 drivers/gpu/drm/drm_drv.c                  |  12 +
 drivers/gpu/drm/drmfs.c                    | 555 +++++++++++++++++++++++++++++
 drivers/gpu/drm/i915/i915_guc_submission.c |  33 +-
 include/drm/drm_drv.h                      |   3 +
 include/drm/drmfs.h                        |  77 ++++
 include/uapi/linux/magic.h                 |   3 +
 8 files changed, 672 insertions(+), 21 deletions(-)
 create mode 100644 drivers/gpu/drm/drmfs.c
 create mode 100644 include/drm/drmfs.h

-- 
2.7.4

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx