diff mbox

[CFT,08/10] sysfs: Add support for permanently empty directories.

Message ID 87fv6zhxkp.fsf@x220.int.ebiederm.org (mailing list archive)
State New, archived
Headers show

Commit Message

Eric W. Biederman May 14, 2015, 5:35 p.m. UTC
Add two functions sysfs_create_empty_dir and sysfs_remove_empty_dir
that hang a permanently empty directory off of a kobject or remove
a permanently emptpy directory hanging from a kobject.  Export
these new functions so modular filesystems can use them.

As all permanently empty directories are, are names and used
for mouting other filesystems this seems like the right abstraction.

Cc: stable@vger.kernel.org
Signed-off-by: "Eric W. Biederman" <ebiederm@xmission.com>
---
 fs/sysfs/dir.c        | 34 ++++++++++++++++++++++++++++++++++
 include/linux/sysfs.h | 16 ++++++++++++++++
 2 files changed, 50 insertions(+)

Comments

Greg Kroah-Hartman May 14, 2015, 8:31 p.m. UTC | #1
On Thu, May 14, 2015 at 12:35:02PM -0500, Eric W. Biederman wrote:
> 
> Add two functions sysfs_create_empty_dir and sysfs_remove_empty_dir
> that hang a permanently empty directory off of a kobject or remove
> a permanently emptpy directory hanging from a kobject.  Export
> these new functions so modular filesystems can use them.
> 
> As all permanently empty directories are, are names and used
> for mouting other filesystems this seems like the right abstraction.

That sentence doesn't make much sense, cut and paste?

> 
> Cc: stable@vger.kernel.org
> Signed-off-by: "Eric W. Biederman" <ebiederm@xmission.com>
> ---
>  fs/sysfs/dir.c        | 34 ++++++++++++++++++++++++++++++++++
>  include/linux/sysfs.h | 16 ++++++++++++++++
>  2 files changed, 50 insertions(+)
> 
> diff --git a/fs/sysfs/dir.c b/fs/sysfs/dir.c
> index 0b45ff42f374..8244741474d7 100644
> --- a/fs/sysfs/dir.c
> +++ b/fs/sysfs/dir.c
> @@ -121,3 +121,37 @@ int sysfs_move_dir_ns(struct kobject *kobj, struct kobject *new_parent_kobj,
>  
>  	return kernfs_rename_ns(kn, new_parent, kn->name, new_ns);
>  }
> +
> +/**
> + * sysfs_create_empty_dir - create an always empty directory
> + * @parent_kobj:  kobject that will contain this always empty directory
> + * @name: The name of the always empty directory to add
> + */
> +int sysfs_create_empty_dir(struct kobject *parent_kobj, const char *name)

As this really is just a mount point, how about we be explicit with
this and call the function:
	sysfs_create_mount_point()
	sysfs_remove_mount_point()
That makes more sense in the long run, otherwise if you just want to
create an empty directory in sysfs, you can do so without making an
"empty" kobject and some people might do that accidentally in the
future.  This makes it more obvious as to what is going on.

thanks,

greg k-h
--
To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Eric W. Biederman May 14, 2015, 9:33 p.m. UTC | #2
Greg Kroah-Hartman <gregkh@linuxfoundation.org> writes:

> On Thu, May 14, 2015 at 12:35:02PM -0500, Eric W. Biederman wrote:
>> 
>> Add two functions sysfs_create_empty_dir and sysfs_remove_empty_dir
>> that hang a permanently empty directory off of a kobject or remove
>> a permanently emptpy directory hanging from a kobject.  Export
>> these new functions so modular filesystems can use them.
>> 
>> As all permanently empty directories are, are names and used
>> for mouting other filesystems this seems like the right abstraction.
>
> That sentence doesn't make much sense, cut and paste?

Probably one edit too many or too few depending on how you look at it.

What I meant is that since the only interesting thing about a
permanently empty directory is it's name, treating them like sysfs files
rather than normal sysfs directories which require a kobject seems like
the right abstraction.

>> Cc: stable@vger.kernel.org
>> Signed-off-by: "Eric W. Biederman" <ebiederm@xmission.com>
>> ---
>>  fs/sysfs/dir.c        | 34 ++++++++++++++++++++++++++++++++++
>>  include/linux/sysfs.h | 16 ++++++++++++++++
>>  2 files changed, 50 insertions(+)
>> 
>> diff --git a/fs/sysfs/dir.c b/fs/sysfs/dir.c
>> index 0b45ff42f374..8244741474d7 100644
>> --- a/fs/sysfs/dir.c
>> +++ b/fs/sysfs/dir.c
>> @@ -121,3 +121,37 @@ int sysfs_move_dir_ns(struct kobject *kobj, struct kobject *new_parent_kobj,
>>  
>>  	return kernfs_rename_ns(kn, new_parent, kn->name, new_ns);
>>  }
>> +
>> +/**
>> + * sysfs_create_empty_dir - create an always empty directory
>> + * @parent_kobj:  kobject that will contain this always empty directory
>> + * @name: The name of the always empty directory to add
>> + */
>> +int sysfs_create_empty_dir(struct kobject *parent_kobj, const char *name)
>
> As this really is just a mount point, how about we be explicit with
> this and call the function:
> 	sysfs_create_mount_point()
> 	sysfs_remove_mount_point()
> That makes more sense in the long run, otherwise if you just want to
> create an empty directory in sysfs, you can do so without making an
> "empty" kobject and some people might do that accidentally in the
> future.  This makes it more obvious as to what is going on.

Yeah.  That seems fairly reasonable.

My brain is on the edge between the functional description of
creating a permanently empty directory, and the usage based
description (creating a directory to mount filesystems on).

But I agree a name that makes it totally obvious we are creating a
directory to mount something on is going to be more usable and
comprehensible.

My head doesn't like sysfs_create_mount_point() as a mount point can be
a file.  But I will put it on the back burner and see if I can come up
with something better, and if not sysfs_create_mount_point it is.

Brainstorming:

sysfs_create_expected_mount_point()
sysfs_reserve_dir_for_mount()
sysfs_create_dir_mount_point()
sysfs_create_expected_mount_point()

Partly I think I would like to rename the proc, sysctl and
infrastructure bit as well (consistency and clarity is good).

Where I get stuck is how do I ask the question:
I see this directory is a mount point, is it a directory whose sole
purpose in life is to be a mount point?

In the context of that question I like my naming of empty_dir as it
conveys what I am interested in.

But I like the sysfs_create_mount_point for general use.  Maybe I won't
make my names consistent.

I don't know.  I am putting this naming question on the back burner for
a bit.

Eric
--
To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
diff mbox

Patch

diff --git a/fs/sysfs/dir.c b/fs/sysfs/dir.c
index 0b45ff42f374..8244741474d7 100644
--- a/fs/sysfs/dir.c
+++ b/fs/sysfs/dir.c
@@ -121,3 +121,37 @@  int sysfs_move_dir_ns(struct kobject *kobj, struct kobject *new_parent_kobj,
 
 	return kernfs_rename_ns(kn, new_parent, kn->name, new_ns);
 }
+
+/**
+ * sysfs_create_empty_dir - create an always empty directory
+ * @parent_kobj:  kobject that will contain this always empty directory
+ * @name: The name of the always empty directory to add
+ */
+int sysfs_create_empty_dir(struct kobject *parent_kobj, const char *name)
+{
+	struct kernfs_node *kn, *parent = parent_kobj->sd;
+
+	kn = kernfs_create_empty_dir(parent, name, NULL);
+	if (IS_ERR(kn)) {
+		if (PTR_ERR(kn) == -EEXIST)
+			sysfs_warn_dup(parent, name);
+		return PTR_ERR(kn);
+	}
+
+	return 0;
+}
+EXPORT_SYMBOL_GPL(sysfs_create_empty_dir);
+
+/**
+ *	sysfs_remove_empty_dir - remove an always empty directory.
+ *	@parent_kobj: kobject that will contain this always empty directory
+ *	@name: The name of the always empty directory to remove
+ *
+ */
+void sysfs_remove_empty_dir(struct kobject *parent_kobj, const char *name)
+{
+	struct kernfs_node *parent = parent_kobj->sd;
+
+	kernfs_remove_by_name_ns(parent, name, NULL);
+}
+EXPORT_SYMBOL_GPL(sysfs_remove_empty_dir);
diff --git a/include/linux/sysfs.h b/include/linux/sysfs.h
index 99382c0df17e..e156d419de75 100644
--- a/include/linux/sysfs.h
+++ b/include/linux/sysfs.h
@@ -210,6 +210,10 @@  int __must_check sysfs_rename_dir_ns(struct kobject *kobj, const char *new_name,
 int __must_check sysfs_move_dir_ns(struct kobject *kobj,
 				   struct kobject *new_parent_kobj,
 				   const void *new_ns);
+int __must_check sysfs_create_empty_dir(struct kobject *parent_kobj,
+					const char *name);
+void sysfs_remove_empty_dir(struct kobject *parent_kobj,
+			    const char *name);
 
 int __must_check sysfs_create_file_ns(struct kobject *kobj,
 				      const struct attribute *attr,
@@ -298,6 +302,18 @@  static inline int sysfs_move_dir_ns(struct kobject *kobj,
 	return 0;
 }
 
+static inline int sysfs_create_empty_dir(struct kobject *parent_kobj,
+					 const char *name)
+{
+	return 0;
+}
+
+static inline void sysfs_remove_empty_dir(struct kobject *parent_kobj,
+					  const char *name)
+{
+	return 0;
+}
+
 static inline int sysfs_create_file_ns(struct kobject *kobj,
 				       const struct attribute *attr,
 				       const void *ns)