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

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

[billodo]

Hi all-
Here is a first 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 patch provides the beginnings of the
infrastructure for per-fs stats (in addition to global accumulative
stats).

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.

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

[billodo]

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 | 17 ++++++++---
 fs/xfs/xfs_sysfs.c | 78 ++++++++++++++++++++++++++++++++++++++++++++++++++
 fs/xfs/xfs_sysfs.h |  1 +
 5 files changed, 177 insertions(+), 4 deletions(-)

diff --git a/fs/xfs/xfs_stats.c b/fs/xfs/xfs_stats.c
index f224038..856cf57 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;
+}
+
+int xfs_stats_clearall(const char *buf)
+{
+	int		c, n;
+	__uint32_t	vn_active;
+
+	n = sizeof(struct xfsstats);
+	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();
+	}
+	return n;
+}
+
 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..c117afc 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);
+int xfs_stats_clearall(const char *buf);
 
 #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..9a794cf 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,15 +1839,20 @@ 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;
 #endif
+	if (error)
+		goto out_remove_stats_kobj;
 
 	error = xfs_qm_init();
 	if (error)
@@ -1862,8 +1868,10 @@ init_xfs_fs(void)
  out_remove_kobj:
 #ifdef DEBUG
 	xfs_sysfs_del(&xfs_dbg_kobj);
- out_kset_unregister:
 #endif
+ out_remove_stats_kobj:
+	xfs_sysfs_del(&xfs_stats_kobj);
+ out_kset_unregister:
 	kset_unregister(xfs_kset);
  out_sysctl_unregister:
 	xfs_sysctl_unregister();
@@ -1889,6 +1897,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_sysfs.c b/fs/xfs/xfs_sysfs.c
index aa03670..ba8d097 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;
@@ -125,6 +126,83 @@ 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_show(
+	char	*buf,
+	void	*data)
+{
+	return 0;
+}
+
+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;
+	return xfs_stats_clearall(buf);
+}
+XFS_SYSFS_ATTR_RW(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

[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

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

[billodo]

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 ad435f1..9f20d3e 100644
--- a/fs/xfs/xfs_stats.c
+++ b/fs/xfs/xfs_stats.c
@@ -112,80 +112,6 @@ int xfs_stats_clearall(const char *buf)
 	return n;
 }
 
-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

[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

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

[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

[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

[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 1/3] xfs: create global stats and stats_clear in sysfs

[Eric Sandeen]

On 9/3/15 11:36 AM, billodo 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.

Looks pretty good, a few things below.
(FWIW, lots of these came from running scripts/checkpatch.pl against
the patch - not everything it says is mandatory, but worth doing to
catch some things)

> 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 | 17 ++++++++---
>  fs/xfs/xfs_sysfs.c | 78 ++++++++++++++++++++++++++++++++++++++++++++++++++
>  fs/xfs/xfs_sysfs.h |  1 +
>  5 files changed, 177 insertions(+), 4 deletions(-)
> 
> diff --git a/fs/xfs/xfs_stats.c b/fs/xfs/xfs_stats.c
> index f224038..856cf57 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);

This line is > 80 chars, might wrap the last arg onto the next line

> +		/* inner loop does each group */
> +		for (; j < xstats[i].endpoint; j++)
> +			len += snprintf(buf + len, PATH_MAX - len, " %u", counter_val(j));

ditto

> +		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;
> +}
> +
> +int xfs_stats_clearall(const char *buf)

given that *buf isn't used, I'd make this a (void)

And returning the size of the struct isn't correct,
and nothing can really fail, so may as well make return
void too,

void xfs_stats_clearall(void)

And finally, given that the sysctl handler for clearing
stats is still around, and this dupicates that code,
may as well make the sysctl handler just call this function,
and eliminate the duplicate code there.

> +{
> +	int		c, n;
> +	__uint32_t	vn_active;
> +
> +	n = sizeof(struct xfsstats);
> +	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();
> +	}
> +	return n;
> +}
> +
>  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..c117afc 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);
> +int xfs_stats_clearall(const char *buf);
>  
>  #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..9a794cf 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,15 +1839,20 @@ 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");

over 80 chars	

> +	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;
>  #endif
> +	if (error)
> +		goto out_remove_stats_kobj;

this needs to stay inside the #ifdef, otherwise you have two

if (error)
      goto BLAH;'s 

in a row if DEBUG is off...

>  
>  	error = xfs_qm_init();
>  	if (error)
> @@ -1862,8 +1868,10 @@ init_xfs_fs(void)
>   out_remove_kobj:

I'd make this "out_remove_dbg_kobj" now that there are 2.

>  #ifdef DEBUG
>  	xfs_sysfs_del(&xfs_dbg_kobj);
> - out_kset_unregister:
>  #endif
> + out_remove_stats_kobj:

pretty sure this still needs to be inside the #ifdef too;
think it through with and without DEBUG defined, make sure
it does the right thing ...

> +	xfs_sysfs_del(&xfs_stats_kobj);
> + out_kset_unregister:
>  	kset_unregister(xfs_kset);
>   out_sysctl_unregister:
>  	xfs_sysctl_unregister();
> @@ -1889,6 +1897,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_sysfs.c b/fs/xfs/xfs_sysfs.c
> index aa03670..ba8d097 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;
> @@ -125,6 +126,83 @@ 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_show(
> +	char	*buf,
> +	void	*data)
> +{
> +	return 0;
> +}

Ok, this means that if we try to read it we get nothing:

# cat /sys/fs/xfs/stats/stats_clear 
#

I think that's ok.  Oh, but see below; we can declare it
write-only, then it disappears!

> +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;
> +	return xfs_stats_clearall(buf);

Hm, I think this is supposed to return the number of bytes
stored in the file, which is not what the clearall function
does.  Given that you sanitize the input and return an error
if not sane, then by the time we get to something sane, just
return "count" - the amt of data we got in.  That will make
writes to this file return the proper bytes written.  i.e.

	xfs_stats_clearall();

	return count;

(and yeah, it doesn't need "buf")

> +}
> +XFS_SYSFS_ATTR_RW(stats_clear);

Ooh, I just realized that there is an __ATTR_WO in core kernel code;
using that in a new XFS_SYSFS_ATTR_WO would make sense here.
Then you can drop the stats_clear_show I think.

> +
> +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);

Add a blank line after the var declaration

> +	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);

Add a blank line after the var declaration

> +       return xfs_attr->store ? xfs_attr->store(buf, count, NULL) : 0;
> +}

covert all those 8-spaces above to tabs, please

> +
> +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)
> 

_______________________________________________
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 1/3] xfs: create global stats and stats_clear in sysfs

[Eric Sandeen]

On 9/3/15 11:36 AM, billodo 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>
> ---
>  fs/xfs/xfs_stats.c | 83 ++++++++++++++++++++++++++++++++++++++++++++++++++++++
>  fs/xfs/xfs_stats.h |  2 ++
>  fs/xfs/xfs_super.c | 17 ++++++++---
>  fs/xfs/xfs_sysfs.c | 78 ++++++++++++++++++++++++++++++++++++++++++++++++++
>  fs/xfs/xfs_sysfs.h |  1 +
>  5 files changed, 177 insertions(+), 4 deletions(-)
> 
> diff --git a/fs/xfs/xfs_stats.c b/fs/xfs/xfs_stats.c
> index f224038..856cf57 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;

Oh, one other little nitpick, tab over "len" to match the rest.

> +	__uint64_t	xs_xstrat_bytes = 0;
> +	__uint64_t	xs_write_bytes = 0;
> +	__uint64_t	xs_read_bytes = 0;

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

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

[Eric Sandeen]

On 9/3/15 11:36 AM, billodo wrote:
> 
> Hi all-
> Here is a first 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 patch provides the beginnings of the
> infrastructure for per-fs stats (in addition to global accumulative
> stats).

To flesh this out a little more:

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.

-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/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

[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