diff mbox

[RFC,v4+,hot_track,13/19] debugfs: introduce one function

Message ID 1351485061-12297-14-git-send-email-zwu.kernel@gmail.com
State Not Applicable, archived
Headers show

Commit Message

Zhiyong Wu Oct. 29, 2012, 4:30 a.m. UTC 
From: Zhi Yong Wu <wuzhy@linux.vnet.ibm.com>

  The debugfs function is used to get expected dentry.

Signed-off-by: Zhi Yong Wu <wuzhy@linux.vnet.ibm.com>
---
 fs/debugfs/inode.c      |   26 ++++++++++++++++++++++++++
 include/linux/debugfs.h |    9 +++++++++
 2 files changed, 35 insertions(+), 0 deletions(-)

Comments

gregkh@linuxfoundation.org Oct. 29, 2012, 6:11 p.m. UTC | #1 
On Mon, Oct 29, 2012 at 12:30:55PM +0800, zwu.kernel@gmail.com wrote:
> From: Zhi Yong Wu <wuzhy@linux.vnet.ibm.com>
> 
>   The debugfs function is used to get expected dentry.

Huh?  Why do you need this?  Why haven't you added documentation for the
function saying what it does?

confused,

greg k-h
--
To unsubscribe from this list: send the line "unsubscribe linux-ext4" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Zhiyong Wu Oct. 29, 2012, 10:25 p.m. UTC | #2 
On Tue, Oct 30, 2012 at 2:11 AM, Greg KH <gregkh@linuxfoundation.org> wrote:
> On Mon, Oct 29, 2012 at 12:30:55PM +0800, zwu.kernel@gmail.com wrote:
>> From: Zhi Yong Wu <wuzhy@linux.vnet.ibm.com>
>>
>>   The debugfs function is used to get expected dentry.
>
> Huh?  Why do you need this?  Why haven't you added documentation for the
It is used to determine if one sysfs directory has been created. OK, i
will add some doc, thanks for your suggestion.

> function saying what it does?
>
> confused,
>
> greg k-h
gregkh@linuxfoundation.org Oct. 29, 2012, 10:34 p.m. UTC | #3 
On Tue, Oct 30, 2012 at 06:25:50AM +0800, Zhi Yong Wu wrote:
> On Tue, Oct 30, 2012 at 2:11 AM, Greg KH <gregkh@linuxfoundation.org> wrote:
> > On Mon, Oct 29, 2012 at 12:30:55PM +0800, zwu.kernel@gmail.com wrote:
> >> From: Zhi Yong Wu <wuzhy@linux.vnet.ibm.com>
> >>
> >>   The debugfs function is used to get expected dentry.
> >
> > Huh?  Why do you need this?  Why haven't you added documentation for the
> It is used to determine if one sysfs directory has been created. OK, i
> will add some doc, thanks for your suggestion.

You didn't answer the "why" part here.  How come you think you need
this?  Can't you just save off the dentry you created somewhere so you
don't need to look it up again?

greg k-h
--
To unsubscribe from this list: send the line "unsubscribe linux-ext4" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Zhiyong Wu Oct. 29, 2012, 10:45 p.m. UTC | #4 
On Tue, Oct 30, 2012 at 6:34 AM, Greg KH <gregkh@linuxfoundation.org> wrote:
> On Tue, Oct 30, 2012 at 06:25:50AM +0800, Zhi Yong Wu wrote:
>> On Tue, Oct 30, 2012 at 2:11 AM, Greg KH <gregkh@linuxfoundation.org> wrote:
>> > On Mon, Oct 29, 2012 at 12:30:55PM +0800, zwu.kernel@gmail.com wrote:
>> >> From: Zhi Yong Wu <wuzhy@linux.vnet.ibm.com>
>> >>
>> >>   The debugfs function is used to get expected dentry.
>> >
>> > Huh?  Why do you need this?  Why haven't you added documentation for the
>> It is used to determine if one sysfs directory has been created. OK, i
>> will add some doc, thanks for your suggestion.
>
> You didn't answer the "why" part here.  How come you think you need
ah, Let me say its scenario at first. If we do two mount ops as below:
1.) mount -o loop,hot_track image1 /data1
2.) mount -o loop,hot_track image2 /data2

The mount -o hot_track operation will automatically create one sysfs
directory /sys/kernel/debug/hot_track. To prevent this dir being
created again when 2.) is done, we need to know if it has existed at
first. In my patch, i at first get its dentry by this new function,
then determine if its d_inode field is NULL, if no, it means that this
sysfs dir has existed.
This is the reason that i want to add one new function.

> this?  Can't you just save off the dentry you created somewhere so you
> don't need to look it up again?
Because i can't find one appropriate place to save it.
>
> greg k-h
gregkh@linuxfoundation.org Oct. 29, 2012, 10:54 p.m. UTC | #5 
On Tue, Oct 30, 2012 at 06:45:19AM +0800, Zhi Yong Wu wrote:
> On Tue, Oct 30, 2012 at 6:34 AM, Greg KH <gregkh@linuxfoundation.org> wrote:
> > On Tue, Oct 30, 2012 at 06:25:50AM +0800, Zhi Yong Wu wrote:
> >> On Tue, Oct 30, 2012 at 2:11 AM, Greg KH <gregkh@linuxfoundation.org> wrote:
> >> > On Mon, Oct 29, 2012 at 12:30:55PM +0800, zwu.kernel@gmail.com wrote:
> >> >> From: Zhi Yong Wu <wuzhy@linux.vnet.ibm.com>
> >> >>
> >> >>   The debugfs function is used to get expected dentry.
> >> >
> >> > Huh?  Why do you need this?  Why haven't you added documentation for the
> >> It is used to determine if one sysfs directory has been created. OK, i
> >> will add some doc, thanks for your suggestion.
> >
> > You didn't answer the "why" part here.  How come you think you need
> ah, Let me say its scenario at first. If we do two mount ops as below:
> 1.) mount -o loop,hot_track image1 /data1
> 2.) mount -o loop,hot_track image2 /data2
> 
> The mount -o hot_track operation will automatically create one sysfs
> directory /sys/kernel/debug/hot_track. To prevent this dir being
> created again when 2.) is done, we need to know if it has existed at
> first. In my patch, i at first get its dentry by this new function,
> then determine if its d_inode field is NULL, if no, it means that this
> sysfs dir has existed.
> This is the reason that i want to add one new function.

Why not do like the rest of the kernel does and just have a:
	static dentry *hot_track_root;
and use that as your root debugfs directory dentry:

	if (!hot_track_root) {
		/* Create root directory */
		hot_track_root = debugfs_create(...);
	}

No need to look anything up :)

thanks,

greg k-h
--
To unsubscribe from this list: send the line "unsubscribe linux-ext4" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Zhiyong Wu Oct. 29, 2012, 10:58 p.m. UTC | #6 
On Tue, Oct 30, 2012 at 6:54 AM, Greg KH <gregkh@linuxfoundation.org> wrote:
> On Tue, Oct 30, 2012 at 06:45:19AM +0800, Zhi Yong Wu wrote:
>> On Tue, Oct 30, 2012 at 6:34 AM, Greg KH <gregkh@linuxfoundation.org> wrote:
>> > On Tue, Oct 30, 2012 at 06:25:50AM +0800, Zhi Yong Wu wrote:
>> >> On Tue, Oct 30, 2012 at 2:11 AM, Greg KH <gregkh@linuxfoundation.org> wrote:
>> >> > On Mon, Oct 29, 2012 at 12:30:55PM +0800, zwu.kernel@gmail.com wrote:
>> >> >> From: Zhi Yong Wu <wuzhy@linux.vnet.ibm.com>
>> >> >>
>> >> >>   The debugfs function is used to get expected dentry.
>> >> >
>> >> > Huh?  Why do you need this?  Why haven't you added documentation for the
>> >> It is used to determine if one sysfs directory has been created. OK, i
>> >> will add some doc, thanks for your suggestion.
>> >
>> > You didn't answer the "why" part here.  How come you think you need
>> ah, Let me say its scenario at first. If we do two mount ops as below:
>> 1.) mount -o loop,hot_track image1 /data1
>> 2.) mount -o loop,hot_track image2 /data2
>>
>> The mount -o hot_track operation will automatically create one sysfs
>> directory /sys/kernel/debug/hot_track. To prevent this dir being
>> created again when 2.) is done, we need to know if it has existed at
>> first. In my patch, i at first get its dentry by this new function,
>> then determine if its d_inode field is NULL, if no, it means that this
>> sysfs dir has existed.
>> This is the reason that i want to add one new function.
>
> Why not do like the rest of the kernel does and just have a:
>         static dentry *hot_track_root;
> and use that as your root debugfs directory dentry:
ah, i'm one newbie, don't get familar with other kernel part, but this
is one good point, i will apply it, thanks.
>
>         if (!hot_track_root) {
>                 /* Create root directory */
>                 hot_track_root = debugfs_create(...);
>         }
>
> No need to look anything up :)
>
> thanks,
>
> greg k-h
diff mbox

Patch

diff --git a/fs/debugfs/inode.c b/fs/debugfs/inode.c
index b607d92..c6291bc 100644
--- a/fs/debugfs/inode.c
+++ b/fs/debugfs/inode.c
@@ -354,6 +354,32 @@  exit:
 	return dentry;
 }
 
+struct dentry *debugfs_get_dentry(const char *name,
+			struct dentry *parent, int len)
+{
+	struct dentry *dentry = NULL;
+	int error = 0;
+
+	error = simple_pin_fs(&debug_fs_type, &debugfs_mount,
+				&debugfs_mount_count);
+	if (error)
+		return NULL;
+
+	if (!parent)
+		parent = debugfs_mount->mnt_root;
+
+	mutex_lock(&parent->d_inode->i_mutex);
+	dentry = lookup_one_len(name, parent, strlen(name));
+	if (!IS_ERR(dentry)) {
+		mutex_unlock(&parent->d_inode->i_mutex);
+		return dentry;
+	}
+	mutex_unlock(&parent->d_inode->i_mutex);
+
+	return NULL;
+}
+EXPORT_SYMBOL_GPL(debugfs_get_dentry);
+
 /**
  * debugfs_create_file - create a file in the debugfs filesystem
  * @name: a pointer to a string containing the name of the file to create.
diff --git a/include/linux/debugfs.h b/include/linux/debugfs.h
index 66c434f..8913a4d 100644
--- a/include/linux/debugfs.h
+++ b/include/linux/debugfs.h
@@ -46,6 +46,9 @@  extern struct dentry *arch_debugfs_dir;
 extern const struct file_operations debugfs_file_operations;
 extern const struct inode_operations debugfs_link_operations;
 
+struct dentry *debugfs_get_dentry(const char *name,
+				struct dentry *parent, int len);
+
 struct dentry *debugfs_create_file(const char *name, umode_t mode,
 				   struct dentry *parent, void *data,
 				   const struct file_operations *fops);
@@ -103,6 +106,12 @@  bool debugfs_initialized(void);
 
 #include <linux/err.h>
 
+static inline struct dentry *debugfs_get_dentry(const char *name,
+					struct dentry *parent, int len)
+{
+	return ERR_PTR(-ENODEV);
+}
+
 /* 
  * We do not return NULL from these functions if CONFIG_DEBUG_FS is not enabled
  * so users have a chance to detect if there was a real error or not.  We don't