Add __debugfs_create_dir(), which takes data passed from caller.
Signed-off-by: Hiroshi Doyu <[email protected]>
---
fs/debugfs/inode.c | 7 ++++---
include/linux/debugfs.h | 9 ++++++++-
2 files changed, 12 insertions(+), 4 deletions(-)
diff --git a/fs/debugfs/inode.c b/fs/debugfs/inode.c
index 4733eab..423df9f 100644
--- a/fs/debugfs/inode.c
+++ b/fs/debugfs/inode.c
@@ -387,7 +387,7 @@ struct dentry *debugfs_create_file(const char *name, umode_t mode,
EXPORT_SYMBOL_GPL(debugfs_create_file);
/**
- * debugfs_create_dir - create a directory in the debugfs filesystem
+ * __debugfs_create_dir - create a directory in the debugfs 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
@@ -404,10 +404,11 @@ EXPORT_SYMBOL_GPL(debugfs_create_file);
* If debugfs is not enabled in the kernel, the value -%ENODEV will be
* returned.
*/
-struct dentry *debugfs_create_dir(const char *name, struct dentry *parent)
+struct dentry *__debugfs_create_dir(const char *name, struct dentry *parent,
+ void *data)
{
return __create_file(name, S_IFDIR | S_IRWXU | S_IRUGO | S_IXUGO,
- parent, NULL, NULL);
+ parent, data, NULL);
}
EXPORT_SYMBOL_GPL(debugfs_create_dir);
diff --git a/include/linux/debugfs.h b/include/linux/debugfs.h
index 66c434f..4ba9f0a 100644
--- a/include/linux/debugfs.h
+++ b/include/linux/debugfs.h
@@ -50,7 +50,14 @@ struct dentry *debugfs_create_file(const char *name, umode_t mode,
struct dentry *parent, void *data,
const struct file_operations *fops);
-struct dentry *debugfs_create_dir(const char *name, struct dentry *parent);
+struct dentry *__debugfs_create_dir(const char *name, struct dentry *parent,
+ void *data);
+
+static inline struct dentry *debugfs_create_dir(const char *name,
+ struct dentry *parent)
+{
+ return __debugfs_create_dir(name, parent, NULL);
+}
struct dentry *debugfs_create_symlink(const char *name, struct dentry *parent,
const char *dest);
--
1.7.5.4
On Wed, Aug 08, 2012 at 09:24:32AM +0300, Hiroshi Doyu wrote:
> Add __debugfs_create_dir(), which takes data passed from caller.
Why?
> Signed-off-by: Hiroshi Doyu <[email protected]>
> ---
> fs/debugfs/inode.c | 7 ++++---
> include/linux/debugfs.h | 9 ++++++++-
> 2 files changed, 12 insertions(+), 4 deletions(-)
>
> diff --git a/fs/debugfs/inode.c b/fs/debugfs/inode.c
> index 4733eab..423df9f 100644
> --- a/fs/debugfs/inode.c
> +++ b/fs/debugfs/inode.c
> @@ -387,7 +387,7 @@ struct dentry *debugfs_create_file(const char *name, umode_t mode,
> EXPORT_SYMBOL_GPL(debugfs_create_file);
>
> /**
> - * debugfs_create_dir - create a directory in the debugfs filesystem
> + * __debugfs_create_dir - create a directory in the debugfs 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
> @@ -404,10 +404,11 @@ EXPORT_SYMBOL_GPL(debugfs_create_file);
> * If debugfs is not enabled in the kernel, the value -%ENODEV will be
> * returned.
> */
> -struct dentry *debugfs_create_dir(const char *name, struct dentry *parent)
> +struct dentry *__debugfs_create_dir(const char *name, struct dentry *parent,
> + void *data)
> {
> return __create_file(name, S_IFDIR | S_IRWXU | S_IRUGO | S_IXUGO,
> - parent, NULL, NULL);
> + parent, data, NULL);
> }
> EXPORT_SYMBOL_GPL(debugfs_create_dir);
You can't export a symbol that doesn't exist anymore.
What are you trying to do here? This patch doesn't look right at all.
greg k-h
Hi Greg, Felipe,
On Wed, 8 Aug 2012 15:34:27 +0200
Greg Kroah-Hartman <[email protected]> wrote:
> On Wed, Aug 08, 2012 at 09:24:32AM +0300, Hiroshi Doyu wrote:
> > Add __debugfs_create_dir(), which takes data passed from caller.
>
> Why?
>
> > Signed-off-by: Hiroshi Doyu <[email protected]>
> > ---
> > fs/debugfs/inode.c | 7 ++++---
> > include/linux/debugfs.h | 9 ++++++++-
> > 2 files changed, 12 insertions(+), 4 deletions(-)
.....
> What are you trying to do here? This patch doesn't look right at all.
I missed to send the cover letter of this patch series to LKML, which
explained the background. I attached that cover letter below. Please
read the following explanation too.
The point was that, since directory hierarchy itself already has
the necessary info, like parent <- child relationships, among each
entities(here, smmu?, mc?, cache{tlb,ptc}), I just wanted to avoid to
have those *residual* info embedded in normal data hierarchy, because
cache{tlb,ptc} is used only in debugfs, not in the normal path. With
directory/file hierarchy, we could get necessary data at any time
on-the-fly.
OTOH, if debugfs has the assumption that it has to be always projected
from the existing data hierarchy, I should fix to have the same
hierarchy in entities(here, smmu?, mc?, cache{tlb,ptc}), as Felipe
pointed out in another thread.
It might not be so bad that debugfs has the ability to manage its own
hierarchy with i_private, apart from the original data hierarchy
too. In our case, cache{tlb,ptc} data is used only for debugfs. They
don't have to be in the normal data hierarchy. This depends on the
above assumption, does debugfs have to be always projected from the
normal data hierarchy?
The original tegra smmu debugfs patch is:
http://lists.linuxfoundation.org/pipermail/iommu/2012-August/004507.html
Here's the original missed cover letter:
Subject:[RFC][PATCH 0/2] debugfs: Allow debugfs_create_dir() to take data from caller
From: Hiroshi Doyu <[email protected]>
To: Hiroshi Doyu <[email protected]>
CC: "[email protected]" <[email protected]>,
"[email protected]" <[email protected]>, Al Viro <[email protected]>
Date: Wed, 8 Aug 2012 08:24:31 +0200
Thread-Index: Ac11LoTHb+fK/MOMRX2RFTvtP89jeg==
Accept-Language: en-US, en-CA, fi-FI
The commit c3b1a35 "debugfs: make sure that debugfs_create_file() gets
used only for regulars" doesn't allow to use debugfs_create_file() for
creating directory, and the current debugfs_create_dir() cannot take
the private data from caller. There are some cases that we want to pass some
client data to dir, especially when dir is nested deeply. We can work
around to pass all necessary data with some invented data structure to
the end files, but if directory itself had private data, we could
avoid to introduce new structures just to pass data to end files.
For example, tegra iommu(smmu) case, debugfs structure could be as
below.
sys/
└── kernel
└── debug
├── smmu.0
│ ├── mc
│ │ ├── ptc
│ │ └── tlb
│ └── mc0
│ ├── ptc
│ └── tlb
└── smmu.1
├── mc
│ ├── ptc
│ └── tlb
├── mc0
│ ├── ptc
│ └── tlb
└── mc1
├── ptc
└── tlb
The end files, {ptc,tlb} depend on which mc? to belong to, and
which smmu.? to belong to. The parent data can be accessed from those
end files if necessary.
dent = d_find_alias(s->private);
cache = (int)dent->d_inode->i_private;
mc = (int)dent->d_parent->d_inode->i_private;
smmu = dent->d_parent->d_parent->d_inode->i_private;
The original tegra smmu debugfs patch is:
http://lists.linuxfoundation.org/pipermail/iommu/2012-August/004507.html
Hiroshi Doyu (2):
debugfs: Allow debugfs_create_dir() to take data
iommu/tegra: smmu: Use __debugfs_create_dir
drivers/iommu/tegra-smmu.c | 4 +---
fs/debugfs/inode.c | 7 ++++---
include/linux/debugfs.h | 9 ++++++++-
3 files changed, 13 insertions(+), 7 deletions(-)
--
1.7.5.4
On Thu, 9 Aug 2012 14:56:24 +0200
Hiroshi Doyu <[email protected]> wrote:
> Hi Greg, Felipe,
>
> On Wed, 8 Aug 2012 15:34:27 +0200
> Greg Kroah-Hartman <[email protected]> wrote:
>
> > On Wed, Aug 08, 2012 at 09:24:32AM +0300, Hiroshi Doyu wrote:
> > > Add __debugfs_create_dir(), which takes data passed from caller.
> >
> > Why?
> >
> > > Signed-off-by: Hiroshi Doyu <[email protected]>
> > > ---
> > > fs/debugfs/inode.c | 7 ++++---
> > > include/linux/debugfs.h | 9 ++++++++-
> > > 2 files changed, 12 insertions(+), 4 deletions(-)
> .....
> > What are you trying to do here? This patch doesn't look right at all.
>
> I missed to send the cover letter of this patch series to LKML, which
> explained the background. I attached that cover letter below. Please
> read the following explanation too.
Any chance to get some feedback on this?
I'm also sending another version of patch, which just uses
debgufs_create_dir() as Felipe suggested, in-reply-to this mail.
On Wed, Aug 15, 2012 at 08:40:08AM +0300, Hiroshi Doyu wrote:
> On Thu, 9 Aug 2012 14:56:24 +0200
> Hiroshi Doyu <[email protected]> wrote:
>
> > Hi Greg, Felipe,
> >
> > On Wed, 8 Aug 2012 15:34:27 +0200
> > Greg Kroah-Hartman <[email protected]> wrote:
> >
> > > On Wed, Aug 08, 2012 at 09:24:32AM +0300, Hiroshi Doyu wrote:
> > > > Add __debugfs_create_dir(), which takes data passed from caller.
> > >
> > > Why?
> > >
> > > > Signed-off-by: Hiroshi Doyu <[email protected]>
> > > > ---
> > > > fs/debugfs/inode.c | 7 ++++---
> > > > include/linux/debugfs.h | 9 ++++++++-
> > > > 2 files changed, 12 insertions(+), 4 deletions(-)
> > .....
> > > What are you trying to do here? This patch doesn't look right at all.
> >
> > I missed to send the cover letter of this patch series to LKML, which
> > explained the background. I attached that cover letter below. Please
> > read the following explanation too.
>
> Any chance to get some feedback on this?
I still don't like it, and then there's the basic fact that I'm pretty
sure you broke the build with the patch, but who's noticing little
things like that :)
> I'm also sending another version of patch, which just uses
> debgufs_create_dir() as Felipe suggested, in-reply-to this mail.
I'd prefer not to change debugfs for this, so if you don't do that, I
don't object to your driver-specific patch.
greg k-h