[PATCH 0/3 V2] xfs: new global stats in sysfs

[PATCH 0/3 V2] xfs: new global stats in sysfs

[Bill O'Donnell]

Hi all-

Here is the second pass at a series to add new global stats to sysfs.
As a part of that, the /proc/fs/xfs/stat file becomes a symlink to
the sysfs stats entry. The series provides the beginnings of the
infrastructure for per-fs stats (in addition to global accumulative
stats).

We already have per-fs information in /sys, so it makes sense to
have per-fs stats there too.  As a first step, moving existing
global stats infrastructure to /sys will allow us to re-use that
sysfs code for per-fs stats as well.

Patch 1 handles the bring-up and tear down of xfs/stats directory
structure in sysfs when an fs is mounted. The directory contains
the stats file and the stats_clear file. The stats file contents mimic
those of /proc/fs/xfs/stat. The stats_clear file is empty, and much
like the current stat_clear command, handles the zeroing of the stats
file when a "1" is echoed to the stats_clear file.

Patch 2 creates the symlink for stats from procfs to sysfs.

Patch 3 removes the now unused portions of procfs for stat.

Again, comments and questions are welcome.

Thanks-
Bill

_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs

[PATCH 1/3] xfs: create global stats and stats_clear in sysfs

[Bill O'Donnell]

Currently, xfs global stats are in procfs. This patch introduces
(replicates) the global stats in sysfs. Additionally a stats_clear file
is introduced in sysfs.

Signed-off-by: Bill O'Donnell <billodo@redhat.com>
---
 fs/xfs/xfs_stats.c  | 83 +++++++++++++++++++++++++++++++++++++++++++++++++++++
 fs/xfs/xfs_stats.h  |  2 ++
 fs/xfs/xfs_super.c  | 20 +++++++++----
 fs/xfs/xfs_sysctl.c | 15 ++--------
 fs/xfs/xfs_sysfs.c  | 75 +++++++++++++++++++++++++++++++++++++++++++++++
 fs/xfs/xfs_sysfs.h  |  1 +
 6 files changed, 179 insertions(+), 17 deletions(-)

diff --git a/fs/xfs/xfs_stats.c b/fs/xfs/xfs_stats.c
index f224038..6008e25 100644
--- a/fs/xfs/xfs_stats.c
+++ b/fs/xfs/xfs_stats.c
@@ -29,6 +29,89 @@ static int counter_val(int idx)
 	return val;
 }
 
+int xfs_stats_format(char *buf)
+{
+	int		i, j;
+	int		len = 0;
+	__uint64_t	xs_xstrat_bytes = 0;
+	__uint64_t	xs_write_bytes = 0;
+	__uint64_t	xs_read_bytes = 0;
+
+	static const struct xstats_entry {
+		char	*desc;
+		int	endpoint;
+	} xstats[] = {
+		{ "extent_alloc",	XFSSTAT_END_EXTENT_ALLOC	},
+		{ "abt",		XFSSTAT_END_ALLOC_BTREE		},
+		{ "blk_map",		XFSSTAT_END_BLOCK_MAPPING	},
+		{ "bmbt",		XFSSTAT_END_BLOCK_MAP_BTREE	},
+		{ "dir",		XFSSTAT_END_DIRECTORY_OPS	},
+		{ "trans",		XFSSTAT_END_TRANSACTIONS	},
+		{ "ig",			XFSSTAT_END_INODE_OPS		},
+		{ "log",		XFSSTAT_END_LOG_OPS		},
+		{ "push_ail",		XFSSTAT_END_TAIL_PUSHING	},
+		{ "xstrat",		XFSSTAT_END_WRITE_CONVERT	},
+		{ "rw",			XFSSTAT_END_READ_WRITE_OPS	},
+		{ "attr",		XFSSTAT_END_ATTRIBUTE_OPS	},
+		{ "icluster",		XFSSTAT_END_INODE_CLUSTER	},
+		{ "vnodes",		XFSSTAT_END_VNODE_OPS		},
+		{ "buf",		XFSSTAT_END_BUF			},
+		{ "abtb2",		XFSSTAT_END_ABTB_V2		},
+		{ "abtc2",		XFSSTAT_END_ABTC_V2		},
+		{ "bmbt2",		XFSSTAT_END_BMBT_V2		},
+		{ "ibt2",		XFSSTAT_END_IBT_V2		},
+		{ "fibt2",		XFSSTAT_END_FIBT_V2		},
+		/* we print both series of quota information together */
+		{ "qm",			XFSSTAT_END_QM			},
+	};
+
+	/* Loop over all stats groups */
+
+	for (i = j = 0; i < ARRAY_SIZE(xstats); i++) {
+		len += snprintf(buf + len, PATH_MAX - len, "%s",
+				xstats[i].desc);
+		/* inner loop does each group */
+		for (; j < xstats[i].endpoint; j++)
+			len += snprintf(buf + len, PATH_MAX - len, " %u",
+					counter_val(j));
+		len += snprintf(buf + len, PATH_MAX - len, "\n");
+	}
+	/* extra precision counters */
+	for_each_possible_cpu(i) {
+		xs_xstrat_bytes += per_cpu(xfsstats, i).xs_xstrat_bytes;
+		xs_write_bytes += per_cpu(xfsstats, i).xs_write_bytes;
+		xs_read_bytes += per_cpu(xfsstats, i).xs_read_bytes;
+	}
+
+	len += snprintf(buf+len, PATH_MAX-len, "xpc %Lu %Lu %Lu\n",
+			xs_xstrat_bytes, xs_write_bytes, xs_read_bytes);
+	len += snprintf(buf+len, PATH_MAX-len, "debug %u\n",
+#if defined(DEBUG)
+		1);
+#else
+		0);
+#endif
+
+return len;
+}
+
+void xfs_stats_clearall(void)
+{
+	int		c;
+	__uint32_t	vn_active;
+
+	xfs_notice(NULL, "Clearing xfsstats");
+	for_each_possible_cpu(c) {
+		preempt_disable();
+		/* save vn_active, it's a universal truth! */
+		vn_active = per_cpu(xfsstats, c).vn_active;
+		memset(&per_cpu(xfsstats, c), 0,
+		       sizeof(struct xfsstats));
+		per_cpu(xfsstats, c).vn_active = vn_active;
+		preempt_enable();
+	}
+}
+
 static int xfs_stat_proc_show(struct seq_file *m, void *v)
 {
 	int		i, j;
diff --git a/fs/xfs/xfs_stats.h b/fs/xfs/xfs_stats.h
index c8f238b..18807b5 100644
--- a/fs/xfs/xfs_stats.h
+++ b/fs/xfs/xfs_stats.h
@@ -18,6 +18,8 @@
 #ifndef __XFS_STATS_H__
 #define __XFS_STATS_H__
 
+int xfs_stats_format(char *buf);
+void xfs_stats_clearall(void);
 
 #if defined(CONFIG_PROC_FS) && !defined(XFS_STATS_OFF)
 
diff --git a/fs/xfs/xfs_super.c b/fs/xfs/xfs_super.c
index 3bf503a..31ad281 100644
--- a/fs/xfs/xfs_super.c
+++ b/fs/xfs/xfs_super.c
@@ -61,6 +61,7 @@ static kmem_zone_t *xfs_ioend_zone;
 mempool_t *xfs_ioend_pool;
 
 static struct kset *xfs_kset;		/* top-level xfs sysfs dir */
+static struct xfs_kobj xfs_stats_kobj;	/* global stats sysfs attrs */
 #ifdef DEBUG
 static struct xfs_kobj xfs_dbg_kobj;	/* global debug sysfs attrs */
 #endif
@@ -1838,19 +1839,25 @@ init_xfs_fs(void)
 	xfs_kset = kset_create_and_add("xfs", NULL, fs_kobj);
 	if (!xfs_kset) {
 		error = -ENOMEM;
-		goto out_sysctl_unregister;;
+		goto out_sysctl_unregister;
 	}
 
+	xfs_stats_kobj.kobject.kset = xfs_kset;
+	error = xfs_sysfs_init(&xfs_stats_kobj, &xfs_stats_ktype, NULL,
+			       "stats");
+	if (error)
+		goto out_kset_unregister;
+
 #ifdef DEBUG
 	xfs_dbg_kobj.kobject.kset = xfs_kset;
 	error = xfs_sysfs_init(&xfs_dbg_kobj, &xfs_dbg_ktype, NULL, "debug");
 	if (error)
-		goto out_kset_unregister;
+		goto out_remove_stats_kobj;
 #endif
 
 	error = xfs_qm_init();
 	if (error)
-		goto out_remove_kobj;
+		goto out_remove_dbg_kobj;
 
 	error = register_filesystem(&xfs_fs_type);
 	if (error)
@@ -1859,11 +1866,13 @@ init_xfs_fs(void)
 
  out_qm_exit:
 	xfs_qm_exit();
- out_remove_kobj:
+ out_remove_dbg_kobj:
 #ifdef DEBUG
 	xfs_sysfs_del(&xfs_dbg_kobj);
- out_kset_unregister:
+ out_remove_stats_kobj:
 #endif
+	xfs_sysfs_del(&xfs_stats_kobj);
+ out_kset_unregister:
 	kset_unregister(xfs_kset);
  out_sysctl_unregister:
 	xfs_sysctl_unregister();
@@ -1889,6 +1898,7 @@ exit_xfs_fs(void)
 #ifdef DEBUG
 	xfs_sysfs_del(&xfs_dbg_kobj);
 #endif
+	xfs_sysfs_del(&xfs_stats_kobj);
 	kset_unregister(xfs_kset);
 	xfs_sysctl_unregister();
 	xfs_cleanup_procfs();
diff --git a/fs/xfs/xfs_sysctl.c b/fs/xfs/xfs_sysctl.c
index a0c8067..5defabb 100644
--- a/fs/xfs/xfs_sysctl.c
+++ b/fs/xfs/xfs_sysctl.c
@@ -19,6 +19,7 @@
 #include <linux/sysctl.h>
 #include <linux/proc_fs.h>
 #include "xfs_error.h"
+#include "xfs_stats.h"
 
 static struct ctl_table_header *xfs_table_header;
 
@@ -31,22 +32,12 @@ xfs_stats_clear_proc_handler(
 	size_t			*lenp,
 	loff_t			*ppos)
 {
-	int		c, ret, *valp = ctl->data;
-	__uint32_t	vn_active;
+	int		ret, *valp = ctl->data;
 
 	ret = proc_dointvec_minmax(ctl, write, buffer, lenp, ppos);
 
 	if (!ret && write && *valp) {
-		xfs_notice(NULL, "Clearing xfsstats");
-		for_each_possible_cpu(c) {
-			preempt_disable();
-			/* save vn_active, it's a universal truth! */
-			vn_active = per_cpu(xfsstats, c).vn_active;
-			memset(&per_cpu(xfsstats, c), 0,
-			       sizeof(struct xfsstats));
-			per_cpu(xfsstats, c).vn_active = vn_active;
-			preempt_enable();
-		}
+		xfs_stats_clearall();
 		xfs_stats_clear = 0;
 	}
 
diff --git a/fs/xfs/xfs_sysfs.c b/fs/xfs/xfs_sysfs.c
index aa03670..3704873 100644
--- a/fs/xfs/xfs_sysfs.c
+++ b/fs/xfs/xfs_sysfs.c
@@ -21,6 +21,7 @@
 #include "xfs_log_format.h"
 #include "xfs_log.h"
 #include "xfs_log_priv.h"
+#include "xfs_stats.h"
 
 struct xfs_sysfs_attr {
 	struct attribute attr;
@@ -38,6 +39,8 @@ to_attr(struct attribute *attr)
 	static struct xfs_sysfs_attr xfs_sysfs_attr_##name = __ATTR_RW(name)
 #define XFS_SYSFS_ATTR_RO(name) \
 	static struct xfs_sysfs_attr xfs_sysfs_attr_##name = __ATTR_RO(name)
+#define XFS_SYSFS_ATTR_WO(name) \
+	static struct xfs_sysfs_attr xfs_sysfs_attr_##name = __ATTR_WO(name)
 
 #define ATTR_LIST(name) &xfs_sysfs_attr_##name.attr
 
@@ -125,6 +128,78 @@ struct kobj_type xfs_dbg_ktype = {
 
 #endif /* DEBUG */
 
+
+/* stats */
+
+STATIC ssize_t
+stats_show(
+	char	*buf,
+	void	*data)
+{
+	return xfs_stats_format(buf);
+}
+XFS_SYSFS_ATTR_RO(stats);
+
+STATIC ssize_t
+stats_clear_store(
+	const char	*buf,
+	size_t		count,
+	void		*data)
+{
+	int		ret;
+	int		val;
+
+	ret = kstrtoint(buf, 0, &val);
+	if (ret)
+		return ret;
+
+	if (val != 1)
+		return -EINVAL;
+	xfs_stats_clearall();
+	return count;
+}
+XFS_SYSFS_ATTR_WO(stats_clear);
+
+static struct attribute *xfs_stats_attrs[] = {
+	ATTR_LIST(stats),
+	ATTR_LIST(stats_clear),
+	NULL,
+};
+
+STATIC ssize_t
+xfs_stats_show(
+	struct kobject		*kobject,
+	struct attribute	*attr,
+	char			*buf)
+{
+	struct xfs_sysfs_attr *xfs_attr = to_attr(attr);
+
+	return xfs_attr->show ? xfs_attr->show(buf, NULL) : 0;
+}
+
+STATIC ssize_t
+xfs_stats_store(
+	struct kobject          *kobject,
+	struct attribute        *attr,
+	const char              *buf,
+	size_t                  count)
+{
+	struct xfs_sysfs_attr *xfs_attr = to_attr(attr);
+
+	return xfs_attr->store ? xfs_attr->store(buf, count, NULL) : 0;
+}
+
+static struct sysfs_ops xfs_stats_ops = {
+	.show = xfs_stats_show,
+	.store = xfs_stats_store,
+};
+
+struct kobj_type xfs_stats_ktype = {
+	.release = xfs_sysfs_release,
+	.sysfs_ops = &xfs_stats_ops,
+	.default_attrs = xfs_stats_attrs,
+};
+
 /* xlog */
 
 STATIC ssize_t
diff --git a/fs/xfs/xfs_sysfs.h b/fs/xfs/xfs_sysfs.h
index 240eee3..be692e5 100644
--- a/fs/xfs/xfs_sysfs.h
+++ b/fs/xfs/xfs_sysfs.h
@@ -22,6 +22,7 @@
 extern struct kobj_type xfs_mp_ktype;	/* xfs_mount */
 extern struct kobj_type xfs_dbg_ktype;	/* debug */
 extern struct kobj_type xfs_log_ktype;	/* xlog */
+extern struct kobj_type xfs_stats_ktype;	/* stats */
 
 static inline struct xfs_kobj *
 to_kobj(struct kobject *kobject)
-- 
2.4.3

_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs

[PATCH 2/3] xfs: create symlink proc/fs/xfs/stat to sys/fs/xfs/stats

[Bill O'Donnell]

As a part of the work to move xfs global stats from procfs to sysfs,
this patch creates the symlink from proc/fs/xfs/stat to sys/fs/xfs/stats.

Signed-off-by: Bill O'Donnell <billodo@redhat.com>
---
 fs/xfs/xfs_stats.c | 7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/fs/xfs/xfs_stats.c b/fs/xfs/xfs_stats.c
index 6008e25..734b867 100644
--- a/fs/xfs/xfs_stats.c
+++ b/fs/xfs/xfs_stats.c
@@ -244,9 +244,12 @@ xfs_init_procfs(void)
 	if (!proc_mkdir("fs/xfs", NULL))
 		goto out;
 
-	if (!proc_create("fs/xfs/stat", 0, NULL,
-			 &xfs_stat_proc_fops))
+	if (!proc_symlink("fs/xfs/stat", NULL,
+			  "/sys/fs/xfs/stats/stats"))
+	{
 		goto out_remove_xfs_dir;
+	}
+
 #ifdef CONFIG_XFS_QUOTA
 	if (!proc_create("fs/xfs/xqmstat", 0, NULL,
 			 &xqmstat_proc_fops))
-- 
2.4.3

_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs

[PATCH 3/3] xfs: remove unused procfs code

[Bill O'Donnell]

As a part of the work to move xfs global stats from procfs to sysfs,
this patch removes the now unused procfs code that was xfs stat specific.

Signed-off-by: Bill O'Donnell <billodo@redhat.com>
---
 fs/xfs/xfs_stats.c | 74 ------------------------------------------------------
 1 file changed, 74 deletions(-)

diff --git a/fs/xfs/xfs_stats.c b/fs/xfs/xfs_stats.c
index 734b867..4bcac05 100644
--- a/fs/xfs/xfs_stats.c
+++ b/fs/xfs/xfs_stats.c
@@ -112,80 +112,6 @@ void xfs_stats_clearall(void)
 	}
 }
 
-static int xfs_stat_proc_show(struct seq_file *m, void *v)
-{
-	int		i, j;
-	__uint64_t	xs_xstrat_bytes = 0;
-	__uint64_t	xs_write_bytes = 0;
-	__uint64_t	xs_read_bytes = 0;
-
-	static const struct xstats_entry {
-		char	*desc;
-		int	endpoint;
-	} xstats[] = {
-		{ "extent_alloc",	XFSSTAT_END_EXTENT_ALLOC	},
-		{ "abt",		XFSSTAT_END_ALLOC_BTREE		},
-		{ "blk_map",		XFSSTAT_END_BLOCK_MAPPING	},
-		{ "bmbt",		XFSSTAT_END_BLOCK_MAP_BTREE	},
-		{ "dir",		XFSSTAT_END_DIRECTORY_OPS	},
-		{ "trans",		XFSSTAT_END_TRANSACTIONS	},
-		{ "ig",			XFSSTAT_END_INODE_OPS		},
-		{ "log",		XFSSTAT_END_LOG_OPS		},
-		{ "push_ail",		XFSSTAT_END_TAIL_PUSHING	},
-		{ "xstrat",		XFSSTAT_END_WRITE_CONVERT	},
-		{ "rw",			XFSSTAT_END_READ_WRITE_OPS	},
-		{ "attr",		XFSSTAT_END_ATTRIBUTE_OPS	},
-		{ "icluster",		XFSSTAT_END_INODE_CLUSTER	},
-		{ "vnodes",		XFSSTAT_END_VNODE_OPS		},
-		{ "buf",		XFSSTAT_END_BUF			},
-		{ "abtb2",		XFSSTAT_END_ABTB_V2		},
-		{ "abtc2",		XFSSTAT_END_ABTC_V2		},
-		{ "bmbt2",		XFSSTAT_END_BMBT_V2		},
-		{ "ibt2",		XFSSTAT_END_IBT_V2		},
-		{ "fibt2",		XFSSTAT_END_FIBT_V2		},
-		/* we print both series of quota information together */
-		{ "qm",			XFSSTAT_END_QM			},
-	};
-
-	/* Loop over all stats groups */
-	for (i = j = 0; i < ARRAY_SIZE(xstats); i++) {
-		seq_printf(m, "%s", xstats[i].desc);
-		/* inner loop does each group */
-		for (; j < xstats[i].endpoint; j++)
-			seq_printf(m, " %u", counter_val(j));
-		seq_putc(m, '\n');
-	}
-	/* extra precision counters */
-	for_each_possible_cpu(i) {
-		xs_xstrat_bytes += per_cpu(xfsstats, i).xs_xstrat_bytes;
-		xs_write_bytes += per_cpu(xfsstats, i).xs_write_bytes;
-		xs_read_bytes += per_cpu(xfsstats, i).xs_read_bytes;
-	}
-
-	seq_printf(m, "xpc %Lu %Lu %Lu\n",
-			xs_xstrat_bytes, xs_write_bytes, xs_read_bytes);
-	seq_printf(m, "debug %u\n",
-#if defined(DEBUG)
-		1);
-#else
-		0);
-#endif
-	return 0;
-}
-
-static int xfs_stat_proc_open(struct inode *inode, struct file *file)
-{
-	return single_open(file, xfs_stat_proc_show, NULL);
-}
-
-static const struct file_operations xfs_stat_proc_fops = {
-	.owner		= THIS_MODULE,
-	.open		= xfs_stat_proc_open,
-	.read		= seq_read,
-	.llseek		= seq_lseek,
-	.release	= single_release,
-};
-
 /* legacy quota interfaces */
 #ifdef CONFIG_XFS_QUOTA
 static int xqm_proc_show(struct seq_file *m, void *v)
-- 
2.4.3

_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs

Re: [PATCH 2/3] xfs: create symlink proc/fs/xfs/stat to sys/fs/xfs/stats

[Eric Sandeen]

On 9/4/15 7:55 AM, Bill O'Donnell wrote:
> As a part of the work to move xfs global stats from procfs to sysfs,
> this patch creates the symlink from proc/fs/xfs/stat to sys/fs/xfs/stats.
> 
> Signed-off-by: Bill O'Donnell <billodo@redhat.com>
> ---
>  fs/xfs/xfs_stats.c | 7 +++++--
>  1 file changed, 5 insertions(+), 2 deletions(-)
> 
> diff --git a/fs/xfs/xfs_stats.c b/fs/xfs/xfs_stats.c
> index 6008e25..734b867 100644
> --- a/fs/xfs/xfs_stats.c
> +++ b/fs/xfs/xfs_stats.c
> @@ -244,9 +244,12 @@ xfs_init_procfs(void)
>  	if (!proc_mkdir("fs/xfs", NULL))
>  		goto out;
>  
> -	if (!proc_create("fs/xfs/stat", 0, NULL,
> -			 &xfs_stat_proc_fops))
> +	if (!proc_symlink("fs/xfs/stat", NULL,
> +			  "/sys/fs/xfs/stats/stats"))
> +	{
>  		goto out_remove_xfs_dir;
> +	}

Ok, without the printk, now there's no need for the braces :)

Also, as an aside, the kernel/xfs style is:

	if (foo) {
		...
	}

not:

	if (foo)
	{
		...
	}

just FYI...

Thanks,
-Eric
> +
>  #ifdef CONFIG_XFS_QUOTA
>  	if (!proc_create("fs/xfs/xqmstat", 0, NULL,
>  			 &xqmstat_proc_fops))
> 

_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs

Re: [PATCH 1/3] xfs: create global stats and stats_clear in sysfs

[Eric Sandeen]

On 9/4/15 7:55 AM, Bill O'Donnell wrote:
> Currently, xfs global stats are in procfs. This patch introduces
> (replicates) the global stats in sysfs. Additionally a stats_clear file
> is introduced in sysfs.
> 
> Signed-off-by: Bill O'Donnell <billodo@redhat.com>


> +STATIC ssize_t
> +xfs_stats_store(
> +	struct kobject          *kobject,
> +	struct attribute        *attr,
> +	const char              *buf,
> +	size_t                  count)

still have some spaces after the types, before the vars...

Otherwise, this looks good to me; it's not the only place we have
missing tabs... *shrug* you could resend, or maybe Dave's willing
to fix up on commit.

Reviewed-by: Eric Sandeen <sandeen@redhat.com>

Thanks,
-Eric

_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs

Re: [PATCH 1/3] xfs: create global stats and stats_clear in sysfs

[Bill O'Donnell]

On Fri, Sep 04, 2015 at 03:13:49PM -0500, Eric Sandeen wrote:
> On 9/4/15 7:55 AM, Bill O'Donnell wrote:
> > Currently, xfs global stats are in procfs. This patch introduces
> > (replicates) the global stats in sysfs. Additionally a stats_clear file
> > is introduced in sysfs.
> > 
> > Signed-off-by: Bill O'Donnell <billodo@redhat.com>
> 
> 
> > +STATIC ssize_t
> > +xfs_stats_store(
> > +	struct kobject          *kobject,
> > +	struct attribute        *attr,
> > +	const char              *buf,
> > +	size_t                  count)
> 
> still have some spaces after the types, before the vars...
> 
> Otherwise, this looks good to me; it's not the only place we have
> missing tabs... *shrug* you could resend, or maybe Dave's willing
> to fix up on commit.

Nah. I'll fix it and resubmit.
Thanks again for the review ;)
-Bill

> 
> Reviewed-by: Eric Sandeen <sandeen@redhat.com>
> 
> Thanks,
> -Eric

_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs

[PATCH 2/3] xfs: create symlink proc/fs/xfs/stat to sys/fs/xfs/stats

[Bill O'Donnell]

As a part of the work to move xfs global stats from procfs to sysfs,
this patch creates the symlink from proc/fs/xfs/stat to sys/fs/xfs/stats.

Signed-off-by: Bill O'Donnell <billodo@redhat.com>
---
 fs/xfs/xfs_stats.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/fs/xfs/xfs_stats.c b/fs/xfs/xfs_stats.c
index 6008e25..a9f05de 100644
--- a/fs/xfs/xfs_stats.c
+++ b/fs/xfs/xfs_stats.c
@@ -244,9 +244,10 @@ xfs_init_procfs(void)
 	if (!proc_mkdir("fs/xfs", NULL))
 		goto out;
 
-	if (!proc_create("fs/xfs/stat", 0, NULL,
-			 &xfs_stat_proc_fops))
+	if (!proc_symlink("fs/xfs/stat", NULL,
+			  "/sys/fs/xfs/stats/stats"))
 		goto out_remove_xfs_dir;
+
 #ifdef CONFIG_XFS_QUOTA
 	if (!proc_create("fs/xfs/xqmstat", 0, NULL,
 			 &xqmstat_proc_fops))
-- 
2.4.3

_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs

[PATCH 2/3] xfs: create symlink proc/fs/xfs/stat to sys/fs/xfs/stats

[Bill O'Donnell]

As a part of the work to move xfs global stats from procfs to sysfs,
this patch creates the symlink from proc/fs/xfs/stat to sys/fs/xfs/stats.

Signed-off-by: Bill O'Donnell <billodo@redhat.com>
---
 fs/xfs/xfs_stats.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/fs/xfs/xfs_stats.c b/fs/xfs/xfs_stats.c
index 6008e25..a9f05de 100644
--- a/fs/xfs/xfs_stats.c
+++ b/fs/xfs/xfs_stats.c
@@ -244,9 +244,10 @@ xfs_init_procfs(void)
 	if (!proc_mkdir("fs/xfs", NULL))
 		goto out;
 
-	if (!proc_create("fs/xfs/stat", 0, NULL,
-			 &xfs_stat_proc_fops))
+	if (!proc_symlink("fs/xfs/stat", NULL,
+			  "/sys/fs/xfs/stats/stats"))
 		goto out_remove_xfs_dir;
+
 #ifdef CONFIG_XFS_QUOTA
 	if (!proc_create("fs/xfs/xqmstat", 0, NULL,
 			 &xqmstat_proc_fops))
-- 
2.4.3

_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs

Re: [PATCH 2/3] xfs: create symlink proc/fs/xfs/stat to sys/fs/xfs/stats

[Eric Sandeen]

On 9/3/15 11:36 AM, billodo wrote:
> As a part of the work to move xfs global stats from procfs to sysfs,
> this patch creates the symlink from proc/fs/xfs/stat to sys/fs/xfs/stats.
> 
> Signed-off-by: Bill O'Donnell <billodo@redhat.com>
> ---
>  fs/xfs/xfs_stats.c | 8 ++++++--
>  1 file changed, 6 insertions(+), 2 deletions(-)
> 
> diff --git a/fs/xfs/xfs_stats.c b/fs/xfs/xfs_stats.c
> index 856cf57..ad435f1 100644
> --- a/fs/xfs/xfs_stats.c
> +++ b/fs/xfs/xfs_stats.c
> @@ -244,9 +244,13 @@ xfs_init_procfs(void)
>  	if (!proc_mkdir("fs/xfs", NULL))
>  		goto out;
>  
> -	if (!proc_create("fs/xfs/stat", 0, NULL,
> -			 &xfs_stat_proc_fops))
> +	if (!proc_symlink("fs/xfs/stat", NULL,
> +			  "/sys/fs/xfs/stats/stats"))
> +	{
> +		printk(KERN_INFO "failed to created fs/xfs/stat symlink\n");
>  		goto out_remove_xfs_dir;
> +	}
> +

I'm not sure we need the printk, we didn't say anything if proc_create(fs/xfs/stat) failed...

I guess the cleanup is fine as it is, right, it can remove a symlink too AFAIK.

-Eric


>  #ifdef CONFIG_XFS_QUOTA
>  	if (!proc_create("fs/xfs/xqmstat", 0, NULL,
>  			 &xqmstat_proc_fops))
> 

_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs

Re: [PATCH 2/3] xfs: create symlink proc/fs/xfs/stat to sys/fs/xfs/stats

[Darrick J. Wong]

On Thu, Sep 03, 2015 at 02:15:01PM -0500, Bill O'Donnell wrote:
> On Thu, Sep 03, 2015 at 01:39:53PM -0500, Bill O'Donnell wrote:
> > On Thu, Sep 03, 2015 at 01:32:25PM -0500, Eric Sandeen wrote:
> > > On 9/3/15 12:57 PM, Darrick J. Wong wrote:
> > > > On Thu, Sep 03, 2015 at 11:36:26AM -0500, billodo wrote:
> > > >> As a part of the work to move xfs global stats from procfs to sysfs,
> > > >> this patch creates the symlink from proc/fs/xfs/stat to sys/fs/xfs/stats.
> > > >>
> > > >> Signed-off-by: Bill O'Donnell <billodo@redhat.com>
> > > >> ---
> > > >>  fs/xfs/xfs_stats.c | 8 ++++++--
> > > >>  1 file changed, 6 insertions(+), 2 deletions(-)
> > > >>
> > > >> diff --git a/fs/xfs/xfs_stats.c b/fs/xfs/xfs_stats.c
> > > >> index 856cf57..ad435f1 100644
> > > >> --- a/fs/xfs/xfs_stats.c
> > > >> +++ b/fs/xfs/xfs_stats.c
> > > >> @@ -244,9 +244,13 @@ xfs_init_procfs(void)
> > > >>  	if (!proc_mkdir("fs/xfs", NULL))
> > > >>  		goto out;
> > > >>  
> > > >> -	if (!proc_create("fs/xfs/stat", 0, NULL,
> > > >> -			 &xfs_stat_proc_fops))
> > > >> +	if (!proc_symlink("fs/xfs/stat", NULL,
> > > >> +			  "/sys/fs/xfs/stats/stats"))
> > > > 
> > > > Uh.... is it actually guaranteed that sysfs is mounted on /sys now?
> > > > 
> > > > I sort of recall gregkh grumbling years ago that sysfs can be mounted anywhere,
> > > > and that /proc shouldn't hardcode links to it.  But that's just handwaving on
> > > > my part.
> > > 
> > > You can blame me for that idea.  At least one other driver does
> > > do it, though; of_core_init():
> > > 
> > > proc_symlink("device-tree", NULL, "/sys/firmware/devicetree/base");
> > > 
> > > worst-case scenario, your "legacy" stats file will be a broken symlink...
> > > 
> > > -Eric
> > > 
> > 
> > I'm still looking for something in documentation that dictates such a requirement
> > regarding symlinks to sysfs elements.
> > -Bill
> 
> I did find this in: https://www.kernel.org/doc/Documentation/sysfs-rules.txt
> --------------snip--------------------------
> - sysfs is always at /sys
>   Parsing /proc/mounts is a waste of time. Other mount points are a
>   system configuration bug you should not try to solve. For test cases,
>   possibly support a SYSFS_PATH environment variable to overwrite the
>   application's behavior, but never try to search for sysfs. Never try
>   to mount it, if you are not an early boot script.
> -------------snip---------------------------

Ah, ok, sorry for the noise then.

--D

> > 
> > > > --D
> > > > 
> > > >> +	{
> > > >> +		printk(KERN_INFO "failed to created fs/xfs/stat symlink\n");
> > > >>  		goto out_remove_xfs_dir;
> > > >> +	}
> > > >> +
> > > >>  #ifdef CONFIG_XFS_QUOTA
> > > >>  	if (!proc_create("fs/xfs/xqmstat", 0, NULL,
> > > >>  			 &xqmstat_proc_fops))
> > > >> -- 
> > > >> 2.4.3
> > > 
> > 
> > _______________________________________________
> > xfs mailing list
> > xfs@oss.sgi.com
> > http://oss.sgi.com/mailman/listinfo/xfs
> 
> _______________________________________________
> xfs mailing list
> xfs@oss.sgi.com
> http://oss.sgi.com/mailman/listinfo/xfs

_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs

Re: [PATCH 2/3] xfs: create symlink proc/fs/xfs/stat to sys/fs/xfs/stats

[Bill O'Donnell]

On Thu, Sep 03, 2015 at 01:39:53PM -0500, Bill O'Donnell wrote:
> On Thu, Sep 03, 2015 at 01:32:25PM -0500, Eric Sandeen wrote:
> > On 9/3/15 12:57 PM, Darrick J. Wong wrote:
> > > On Thu, Sep 03, 2015 at 11:36:26AM -0500, billodo wrote:
> > >> As a part of the work to move xfs global stats from procfs to sysfs,
> > >> this patch creates the symlink from proc/fs/xfs/stat to sys/fs/xfs/stats.
> > >>
> > >> Signed-off-by: Bill O'Donnell <billodo@redhat.com>
> > >> ---
> > >>  fs/xfs/xfs_stats.c | 8 ++++++--
> > >>  1 file changed, 6 insertions(+), 2 deletions(-)
> > >>
> > >> diff --git a/fs/xfs/xfs_stats.c b/fs/xfs/xfs_stats.c
> > >> index 856cf57..ad435f1 100644
> > >> --- a/fs/xfs/xfs_stats.c
> > >> +++ b/fs/xfs/xfs_stats.c
> > >> @@ -244,9 +244,13 @@ xfs_init_procfs(void)
> > >>  	if (!proc_mkdir("fs/xfs", NULL))
> > >>  		goto out;
> > >>  
> > >> -	if (!proc_create("fs/xfs/stat", 0, NULL,
> > >> -			 &xfs_stat_proc_fops))
> > >> +	if (!proc_symlink("fs/xfs/stat", NULL,
> > >> +			  "/sys/fs/xfs/stats/stats"))
> > > 
> > > Uh.... is it actually guaranteed that sysfs is mounted on /sys now?
> > > 
> > > I sort of recall gregkh grumbling years ago that sysfs can be mounted anywhere,
> > > and that /proc shouldn't hardcode links to it.  But that's just handwaving on
> > > my part.
> > 
> > You can blame me for that idea.  At least one other driver does
> > do it, though; of_core_init():
> > 
> > proc_symlink("device-tree", NULL, "/sys/firmware/devicetree/base");
> > 
> > worst-case scenario, your "legacy" stats file will be a broken symlink...
> > 
> > -Eric
> > 
> 
> I'm still looking for something in documentation that dictates such a requirement
> regarding symlinks to sysfs elements.
> -Bill

I did find this in: https://www.kernel.org/doc/Documentation/sysfs-rules.txt
--------------snip--------------------------
- sysfs is always at /sys
  Parsing /proc/mounts is a waste of time. Other mount points are a
  system configuration bug you should not try to solve. For test cases,
  possibly support a SYSFS_PATH environment variable to overwrite the
  application's behavior, but never try to search for sysfs. Never try
  to mount it, if you are not an early boot script.
-------------snip---------------------------
> 
> > > --D
> > > 
> > >> +	{
> > >> +		printk(KERN_INFO "failed to created fs/xfs/stat symlink\n");
> > >>  		goto out_remove_xfs_dir;
> > >> +	}
> > >> +
> > >>  #ifdef CONFIG_XFS_QUOTA
> > >>  	if (!proc_create("fs/xfs/xqmstat", 0, NULL,
> > >>  			 &xqmstat_proc_fops))
> > >> -- 
> > >> 2.4.3
> > 
> 
> _______________________________________________
> xfs mailing list
> xfs@oss.sgi.com
> http://oss.sgi.com/mailman/listinfo/xfs

_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs

Re: [PATCH 2/3] xfs: create symlink proc/fs/xfs/stat to sys/fs/xfs/stats

[Bill O'Donnell]

On Thu, Sep 03, 2015 at 01:32:25PM -0500, Eric Sandeen wrote:
> On 9/3/15 12:57 PM, Darrick J. Wong wrote:
> > On Thu, Sep 03, 2015 at 11:36:26AM -0500, billodo wrote:
> >> As a part of the work to move xfs global stats from procfs to sysfs,
> >> this patch creates the symlink from proc/fs/xfs/stat to sys/fs/xfs/stats.
> >>
> >> Signed-off-by: Bill O'Donnell <billodo@redhat.com>
> >> ---
> >>  fs/xfs/xfs_stats.c | 8 ++++++--
> >>  1 file changed, 6 insertions(+), 2 deletions(-)
> >>
> >> diff --git a/fs/xfs/xfs_stats.c b/fs/xfs/xfs_stats.c
> >> index 856cf57..ad435f1 100644
> >> --- a/fs/xfs/xfs_stats.c
> >> +++ b/fs/xfs/xfs_stats.c
> >> @@ -244,9 +244,13 @@ xfs_init_procfs(void)
> >>  	if (!proc_mkdir("fs/xfs", NULL))
> >>  		goto out;
> >>  
> >> -	if (!proc_create("fs/xfs/stat", 0, NULL,
> >> -			 &xfs_stat_proc_fops))
> >> +	if (!proc_symlink("fs/xfs/stat", NULL,
> >> +			  "/sys/fs/xfs/stats/stats"))
> > 
> > Uh.... is it actually guaranteed that sysfs is mounted on /sys now?
> > 
> > I sort of recall gregkh grumbling years ago that sysfs can be mounted anywhere,
> > and that /proc shouldn't hardcode links to it.  But that's just handwaving on
> > my part.
> 
> You can blame me for that idea.  At least one other driver does
> do it, though; of_core_init():
> 
> proc_symlink("device-tree", NULL, "/sys/firmware/devicetree/base");
> 
> worst-case scenario, your "legacy" stats file will be a broken symlink...
> 
> -Eric
> 

I'm still looking for something in documentation that dictates such a requirement
regarding symlinks to sysfs elements.
-Bill

> > --D
> > 
> >> +	{
> >> +		printk(KERN_INFO "failed to created fs/xfs/stat symlink\n");
> >>  		goto out_remove_xfs_dir;
> >> +	}
> >> +
> >>  #ifdef CONFIG_XFS_QUOTA
> >>  	if (!proc_create("fs/xfs/xqmstat", 0, NULL,
> >>  			 &xqmstat_proc_fops))
> >> -- 
> >> 2.4.3
> 

_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs

Re: [PATCH 2/3] xfs: create symlink proc/fs/xfs/stat to sys/fs/xfs/stats

[Eric Sandeen]

On 9/3/15 12:57 PM, Darrick J. Wong wrote:
> On Thu, Sep 03, 2015 at 11:36:26AM -0500, billodo wrote:
>> As a part of the work to move xfs global stats from procfs to sysfs,
>> this patch creates the symlink from proc/fs/xfs/stat to sys/fs/xfs/stats.
>>
>> Signed-off-by: Bill O'Donnell <billodo@redhat.com>
>> ---
>>  fs/xfs/xfs_stats.c | 8 ++++++--
>>  1 file changed, 6 insertions(+), 2 deletions(-)
>>
>> diff --git a/fs/xfs/xfs_stats.c b/fs/xfs/xfs_stats.c
>> index 856cf57..ad435f1 100644
>> --- a/fs/xfs/xfs_stats.c
>> +++ b/fs/xfs/xfs_stats.c
>> @@ -244,9 +244,13 @@ xfs_init_procfs(void)
>>  	if (!proc_mkdir("fs/xfs", NULL))
>>  		goto out;
>>  
>> -	if (!proc_create("fs/xfs/stat", 0, NULL,
>> -			 &xfs_stat_proc_fops))
>> +	if (!proc_symlink("fs/xfs/stat", NULL,
>> +			  "/sys/fs/xfs/stats/stats"))
> 
> Uh.... is it actually guaranteed that sysfs is mounted on /sys now?
> 
> I sort of recall gregkh grumbling years ago that sysfs can be mounted anywhere,
> and that /proc shouldn't hardcode links to it.  But that's just handwaving on
> my part.

You can blame me for that idea.  At least one other driver does
do it, though; of_core_init():

proc_symlink("device-tree", NULL, "/sys/firmware/devicetree/base");

worst-case scenario, your "legacy" stats file will be a broken symlink...

-Eric

> --D
> 
>> +	{
>> +		printk(KERN_INFO "failed to created fs/xfs/stat symlink\n");
>>  		goto out_remove_xfs_dir;
>> +	}
>> +
>>  #ifdef CONFIG_XFS_QUOTA
>>  	if (!proc_create("fs/xfs/xqmstat", 0, NULL,
>>  			 &xqmstat_proc_fops))
>> -- 
>> 2.4.3

_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs

Re: [PATCH 2/3] xfs: create symlink proc/fs/xfs/stat to sys/fs/xfs/stats

[Darrick J. Wong]

On Thu, Sep 03, 2015 at 11:36:26AM -0500, billodo wrote:
> As a part of the work to move xfs global stats from procfs to sysfs,
> this patch creates the symlink from proc/fs/xfs/stat to sys/fs/xfs/stats.
> 
> Signed-off-by: Bill O'Donnell <billodo@redhat.com>
> ---
>  fs/xfs/xfs_stats.c | 8 ++++++--
>  1 file changed, 6 insertions(+), 2 deletions(-)
> 
> diff --git a/fs/xfs/xfs_stats.c b/fs/xfs/xfs_stats.c
> index 856cf57..ad435f1 100644
> --- a/fs/xfs/xfs_stats.c
> +++ b/fs/xfs/xfs_stats.c
> @@ -244,9 +244,13 @@ xfs_init_procfs(void)
>  	if (!proc_mkdir("fs/xfs", NULL))
>  		goto out;
>  
> -	if (!proc_create("fs/xfs/stat", 0, NULL,
> -			 &xfs_stat_proc_fops))
> +	if (!proc_symlink("fs/xfs/stat", NULL,
> +			  "/sys/fs/xfs/stats/stats"))

Uh.... is it actually guaranteed that sysfs is mounted on /sys now?

I sort of recall gregkh grumbling years ago that sysfs can be mounted anywhere,
and that /proc shouldn't hardcode links to it.  But that's just handwaving on
my part.

--D

> +	{
> +		printk(KERN_INFO "failed to created fs/xfs/stat symlink\n");
>  		goto out_remove_xfs_dir;
> +	}
> +
>  #ifdef CONFIG_XFS_QUOTA
>  	if (!proc_create("fs/xfs/xqmstat", 0, NULL,
>  			 &xqmstat_proc_fops))
> -- 
> 2.4.3
> 
> _______________________________________________
> xfs mailing list
> xfs@oss.sgi.com
> http://oss.sgi.com/mailman/listinfo/xfs

_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs

[PATCH 2/3] xfs: create symlink proc/fs/xfs/stat to sys/fs/xfs/stats

[billodo]

As a part of the work to move xfs global stats from procfs to sysfs,
this patch creates the symlink from proc/fs/xfs/stat to sys/fs/xfs/stats.

Signed-off-by: Bill O'Donnell <billodo@redhat.com>
---
 fs/xfs/xfs_stats.c | 8 ++++++--
 1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/fs/xfs/xfs_stats.c b/fs/xfs/xfs_stats.c
index 856cf57..ad435f1 100644
--- a/fs/xfs/xfs_stats.c
+++ b/fs/xfs/xfs_stats.c
@@ -244,9 +244,13 @@ xfs_init_procfs(void)
 	if (!proc_mkdir("fs/xfs", NULL))
 		goto out;
 
-	if (!proc_create("fs/xfs/stat", 0, NULL,
-			 &xfs_stat_proc_fops))
+	if (!proc_symlink("fs/xfs/stat", NULL,
+			  "/sys/fs/xfs/stats/stats"))
+	{
+		printk(KERN_INFO "failed to created fs/xfs/stat symlink\n");
 		goto out_remove_xfs_dir;
+	}
+
 #ifdef CONFIG_XFS_QUOTA
 	if (!proc_create("fs/xfs/xqmstat", 0, NULL,
 			 &xqmstat_proc_fops))
-- 
2.4.3

_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs