[PoC,2/7] Add kobject to super_block

Commit Message

Goldwyn Rodrigues April 29, 2016, 2:01 a.m. UTC

From: Goldwyn Rodrigues <[e-mail]>

This automatically creates the /sys/fs/<fs-type>/<s_id>

It is not necessary to use super_block_attribute structures.
Filesystems can build their own attribute structs. These are just
helper functions if a filesystem chooses to use a simple way to
show it's variables in sysfs.

Also, the SB_ATTR and SB_ATTR_RO helpers to build the
super_block_attribute.

We can use the kobject to (delay) destroy the super block.

Signed-off-by: Goldwyn Rodrigues <[e-mail]>
---
 fs/super.c         | 57 +++++++++++++++++++++++++++++++++++++++++++++---------
 include/linux/fs.h | 26 +++++++++++++++++++++++--
 2 files changed, 72 insertions(+), 11 deletions(-)

Comments

Al Viro April 29, 2016, 2:26 a.m. UTC | #1

On Thu, Apr 28, 2016 at 09:01:17PM -0500, Goldwyn Rodrigues wrote:
> @@ -167,7 +204,8 @@ static void destroy_super(struct super_block *s)
>  	WARN_ON(!list_empty(&s->s_mounts));
>  	kfree(s->s_subtype);
>  	kfree(s->s_options);
> -	call_rcu(&s->rcu, destroy_super_rcu);
> +	if (s->s_type->fs_flags & FS_CREATE_SYSFS)
> +		kobject_del(&s->s_kobj);

So we have kobject_del() under a spinlock.  Wonderful...  Better yet,
you have the sodding kobjects sitting around well past the point when
the filesystem driver has gone through rmmod.

sysfs: lifetime rules made simple.  So simple that they don't fit a lot of
situations, but hey - it sure *looks* easy to use...

NAK.  If you want a description of super_block life cycle, I can describe it
to you.  But please, don't play with it until you understand it.
--
To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in
the body of a message to [e-mail]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Goldwyn Rodrigues April 29, 2016, 7:09 p.m. UTC | #2

On 04/28/2016 09:26 PM, Al Viro wrote:
> On Thu, Apr 28, 2016 at 09:01:17PM -0500, Goldwyn Rodrigues wrote:
>> @@ -167,7 +204,8 @@ static void destroy_super(struct super_block *s)
>>   	WARN_ON(!list_empty(&s->s_mounts));
>>   	kfree(s->s_subtype);
>>   	kfree(s->s_options);
>> -	call_rcu(&s->rcu, destroy_super_rcu);
>> +	if (s->s_type->fs_flags & FS_CREATE_SYSFS)
>> +		kobject_del(&s->s_kobj);
>
> So we have kobject_del() under a spinlock.  Wonderful...  Better yet,
> you have the sodding kobjects sitting around well past the point when
> the filesystem driver has gone through rmmod.
>
> sysfs: lifetime rules made simple.  So simple that they don't fit a lot of
> situations, but hey - it sure *looks* easy to use...
>
> NAK.  If you want a description of super_block life cycle, I can describe it
> to you.  But please, don't play with it until you understand it.
>


Yes, I understand what you are trying to say. Using kobject to destroy 
super_block does not make sense in the current scenario.

Thanks for your feedback.

Patch

diff --git a/fs/super.c b/fs/super.c
index 74914b1..5f5e25d 100644
--- a/fs/super.c
+++ b/fs/super.c
@@ -135,10 +135,36 @@  static unsigned long super_cache_count(struct shrinker *shrink,
 	return total_objects;
 }
 
-static void destroy_super_work(struct work_struct *work)
+/* kobject super_block handling */
+
+static ssize_t super_block_attribute_show(struct kobject *kobj,
+		struct attribute *attr, char *buf)
+{
+	struct super_block_attribute *sattr =
+		container_of(attr, struct super_block_attribute, attr);
+	struct super_block *sb = container_of(kobj, struct super_block, s_kobj);
+	if (!sattr->show)
+		return -EIO;
+
+	return sattr->show(sb, sattr, buf);
+}
+
+static ssize_t super_block_attribute_store(struct kobject *kobj,
+		struct attribute *attr, const char *buf, size_t count)
+{
+	struct super_block_attribute *sattr =
+		container_of(attr, struct super_block_attribute, attr);
+	struct super_block *sb = container_of(kobj, struct super_block, s_kobj);
+	if (!sattr->show)
+		return -EIO;
+
+	return sattr->store(sb, sattr, buf, count);
+}
+
+static void release_super(struct kobject *kobj)
 {
-	struct super_block *s = container_of(work, struct super_block,
-							destroy_work);
+	struct super_block *s = container_of(kobj, struct super_block,
+							s_kobj);
 	int i;
 
 	for (i = 0; i < SB_FREEZE_LEVELS; i++)
@@ -146,11 +172,22 @@  static void destroy_super_work(struct work_struct *work)
 	kfree(s);
 }
 
-static void destroy_super_rcu(struct rcu_head *head)
+static const struct sysfs_ops sb_sysfs_ops = {
+	.show = super_block_attribute_show,
+	.store = super_block_attribute_store,
+};
+
+static int register_kobj(struct super_block *sb)
 {
-	struct super_block *s = container_of(head, struct super_block, rcu);
-	INIT_WORK(&s->destroy_work, destroy_super_work);
-	schedule_work(&s->destroy_work);
+	struct kobj_type *ktype;
+       	ktype = kzalloc(sizeof(struct kobj_type), GFP_USER);
+	if (IS_ERR(ktype))
+		return PTR_ERR(ktype);
+	ktype->sysfs_ops = &sb_sysfs_ops;
+	ktype->release = release_super;
+	sb->s_kobj.kset = sb->s_type->kset;
+	return kobject_init_and_add(&sb->s_kobj, ktype, NULL,
+			"%s", sb->s_id);
 }
 
 /**
@@ -167,7 +204,8 @@  static void destroy_super(struct super_block *s)
 	WARN_ON(!list_empty(&s->s_mounts));
 	kfree(s->s_subtype);
 	kfree(s->s_options);
-	call_rcu(&s->rcu, destroy_super_rcu);
+	if (s->s_type->fs_flags & FS_CREATE_SYSFS)
+		kobject_del(&s->s_kobj);
 }
 
 /**
@@ -1021,7 +1059,8 @@  struct dentry *mount_bdev(struct file_system_type *fs_type,
 			deactivate_locked_super(s);
 			goto error;
 		}
-
+		if (s->s_type->fs_flags & FS_CREATE_SYSFS)
+			register_kobj(s);
 		s->s_flags |= MS_ACTIVE;
 		bdev->bd_super = s;
 	}
diff --git a/include/linux/fs.h b/include/linux/fs.h
index cfea7b4..e6b9d89 100644
--- a/include/linux/fs.h
+++ b/include/linux/fs.h
@@ -1317,6 +1317,29 @@  struct sb_writers {
 	struct percpu_rw_semaphore	rw_sem[SB_FREEZE_LEVELS];
 };
 
+struct super_block_attribute {
+	struct attribute attr;
+	ssize_t (*show)(struct super_block *sb, struct super_block_attribute *,
+			char *buf);
+	ssize_t (*store)(struct super_block *sb, struct super_block_attribute *,
+			const char *buf, size_t count);
+};
+
+#define SB_ATTR(_name, _mode) 			\
+struct super_block_attribute sb_attr_##_name = {\
+	.attr = {.name = __stringify(_name),	\
+		.mode = _mode},			\
+	.show = _name##_show,			\
+	.store = _name##_store,			\
+}
+
+#define SB_ATTR_RO(_name) 	 		\
+struct super_block_attribute sb_attr_##_name = {\
+	.attr = {.name = __stringify(_name),	\
+		.mode = S_IRUGO},		\
+	.show = _name##_show,			\
+}
+
 struct super_block {
 	struct list_head	s_list;		/* Keep this first */
 	dev_t			s_dev;		/* search index; _not_ kdev_t */
@@ -1406,8 +1429,6 @@  struct super_block {
 	 */
 	struct list_lru		s_dentry_lru ____cacheline_aligned_in_smp;
 	struct list_lru		s_inode_lru ____cacheline_aligned_in_smp;
-	struct rcu_head		rcu;
-	struct work_struct	destroy_work;
 
 	struct mutex		s_sync_lock;	/* sync serialisation lock */
 
@@ -1419,6 +1440,7 @@  struct super_block {
 	/* s_inode_list_lock protects s_inodes */
 	spinlock_t		s_inode_list_lock ____cacheline_aligned_in_smp;
 	struct list_head	s_inodes;	/* all inodes */
+	struct kobject		s_kobj;
 };
 
 extern struct timespec current_fs_time(struct super_block *sb);