[PATCH 6/7] xfs: per-filesystem stats in sysfs.

[PATCH 6/7] xfs: per-filesystem stats in sysfs.

[Bill O'Donnell]

This patch implements per-filesystem stats objects in sysfs. It
depends on the application of the previous patch series that
develops the infrastructure to support both xfs global stats and
xfs per-fs stats in sysfs.

Stats objects are instantiated when an xfs filesystem is mounted
and deleted on unmount. With this patch, the stats directory is
created and populated with the familiar stats and stats_clear files.
Example:
        /sys/fs/xfs/sda9/stats/stats
        /sys/fs/xfs/sda9/stats/stats_clear

With this patch, the individual counts within the new per-fs
stats file(s) remain at zero. Functions that use the the macros
to increment, decrement, and add-to the per-fs stats counts will
be covered in a separate new patch to follow this one. Note that
the counts within the global stats file (/sys/fs/xfs/stats/stats)
advance normally and can be cleared as it was prior to this patch.

Signed-off-by: Bill O'Donnell <billodo@redhat.com>
---
 fs/xfs/xfs_linux.h |  4 ++--
 fs/xfs/xfs_mount.c | 24 +++++++++++++++++++++++-
 fs/xfs/xfs_mount.h |  1 +
 fs/xfs/xfs_stats.c | 34 +++++++++++++++++++---------------
 fs/xfs/xfs_stats.h |  5 ++++-
 fs/xfs/xfs_super.c | 21 ++++++++++++++++-----
 fs/xfs/xfs_sysfs.c |  6 +++---
 7 files changed, 68 insertions(+), 27 deletions(-)

diff --git a/fs/xfs/xfs_linux.h b/fs/xfs/xfs_linux.h
index f1a8505..ec0e239 100644
--- a/fs/xfs/xfs_linux.h
+++ b/fs/xfs/xfs_linux.h
@@ -172,8 +172,8 @@ struct xfs_kobj {
 };
 
 struct xstats {
-	struct xfsstats __percpu *xs_stats;
-	struct xfs_kobj         xs_kobj;
+	struct xfsstats __percpu	*xs_stats;
+	struct xfs_kobj			xs_kobj;
 };
 
 extern struct xstats xfsstats;
diff --git a/fs/xfs/xfs_mount.c b/fs/xfs/xfs_mount.c
index bf92e0c..5921d18 100644
--- a/fs/xfs/xfs_mount.c
+++ b/fs/xfs/xfs_mount.c
@@ -791,11 +791,25 @@ xfs_mountfs(
 		goto out_free_dir;
 	}
 
+	/*
+	 * Allocate stats memory and create stats sysfs object.
+	 */
+	mp->m_stats.xs_stats = alloc_percpu(struct xfsstats);
+	if (IS_ERR(mp->m_stats.xs_stats)) {
+		error = PTR_ERR(mp->m_stats.xs_stats);
+		goto out_free_perag;
+	}
+	error = xfs_sysfs_init(&mp->m_stats.xs_kobj, &xfs_stats_ktype,
+				&mp->m_kobj,
+				"stats");
+	if (error)
+		goto out_free_stats;
+
 	if (!sbp->sb_logblocks) {
 		xfs_warn(mp, "no log defined");
 		XFS_ERROR_REPORT("xfs_mountfs", XFS_ERRLEVEL_LOW, mp);
 		error = -EFSCORRUPTED;
-		goto out_free_perag;
+		goto out_del_stats;
 	}
 
 	/*
@@ -965,6 +979,10 @@ xfs_mountfs(
 	if (mp->m_logdev_targp && mp->m_logdev_targp != mp->m_ddev_targp)
 		xfs_wait_buftarg(mp->m_logdev_targp);
 	xfs_wait_buftarg(mp->m_ddev_targp);
+ out_del_stats:
+	xfs_sysfs_del(&mp->m_stats.xs_kobj);
+ out_free_stats:
+	free_percpu(mp->m_stats.xs_stats);
  out_free_perag:
 	xfs_free_perag(mp);
  out_free_dir:
@@ -1047,6 +1065,10 @@ xfs_unmountfs(
 		xfs_warn(mp, "Unable to update superblock counters. "
 				"Freespace may not be correct on next mount.");
 
+	/* remove the stats kobject and free stats memory */
+	xfs_sysfs_del(&mp->m_stats.xs_kobj);
+	free_percpu(mp->m_stats.xs_stats);
+
 	xfs_log_unmount(mp);
 	xfs_da_unmount(mp);
 	xfs_uuid_unmount(mp);
diff --git a/fs/xfs/xfs_mount.h b/fs/xfs/xfs_mount.h
index 7999e91..8795272 100644
--- a/fs/xfs/xfs_mount.h
+++ b/fs/xfs/xfs_mount.h
@@ -127,6 +127,7 @@ typedef struct xfs_mount {
 	int64_t			m_low_space[XFS_LOWSP_MAX];
 						/* low free space thresholds */
 	struct xfs_kobj		m_kobj;
+	struct xstats		m_stats;	/* per-fs stats */
 
 	struct workqueue_struct *m_buf_workqueue;
 	struct workqueue_struct	*m_data_workqueue;
diff --git a/fs/xfs/xfs_stats.c b/fs/xfs/xfs_stats.c
index ab79973..d5b3704 100644
--- a/fs/xfs/xfs_stats.c
+++ b/fs/xfs/xfs_stats.c
@@ -18,6 +18,11 @@
 #include "xfs.h"
 #include <linux/proc_fs.h>
 
+#include "xfs_format.h"
+#include "xfs_trans_resv.h"
+#include "xfs_mount.h"
+#include "xfs_sysfs.h"
+
 struct xstats xfsstats;
 
 static int counter_val(struct xfsstats __percpu *stats, int idx)
@@ -25,7 +30,7 @@ static int counter_val(struct xfsstats __percpu *stats, int idx)
 	int val = 0, cpu;
 
 	for_each_possible_cpu(cpu)
-		val += *(((__u32 *)&per_cpu(*stats, cpu) + idx));
+		val += *(((__u32 *)per_cpu_ptr(stats, cpu) + idx));
 	return val;
 }
 
@@ -78,9 +83,9 @@ int xfs_stats_format(struct xfsstats __percpu *stats, char *buf)
 	}
 	/* extra precision counters */
 	for_each_possible_cpu(i) {
-		xs_xstrat_bytes += per_cpu(*stats, i).xs_xstrat_bytes;
-		xs_write_bytes += per_cpu(*stats, i).xs_write_bytes;
-		xs_read_bytes += per_cpu(*stats, i).xs_read_bytes;
+		xs_xstrat_bytes += per_cpu_ptr(stats, i)->xs_xstrat_bytes;
+		xs_write_bytes += per_cpu_ptr(stats, i)->xs_write_bytes;
+		xs_read_bytes += per_cpu_ptr(stats, i)->xs_read_bytes;
 	}
 
 	len += snprintf(buf+len, PATH_MAX-len, "xpc %Lu %Lu %Lu\n",
@@ -104,10 +109,9 @@ void xfs_stats_clearall(struct xfsstats __percpu *stats)
 	for_each_possible_cpu(c) {
 		preempt_disable();
 		/* save vn_active, it's a universal truth! */
-		vn_active = per_cpu(*stats, c).vn_active;
-		memset(&per_cpu(*stats, c), 0,
-		       sizeof(*stats));
-		per_cpu(*stats, c).vn_active = vn_active;
+		vn_active = per_cpu_ptr(stats, c)->vn_active;
+		memset(per_cpu_ptr(stats, c), 0, sizeof(*stats));
+		per_cpu_ptr(stats, c)->vn_active = vn_active;
 		preempt_enable();
 	}
 }
@@ -172,28 +176,28 @@ xfs_init_procfs(void)
 		goto out;
 
 	if (!proc_symlink("fs/xfs/stat", NULL,
-			"/sys/fs/xfs/stats/stats"))
+			  "/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))
+			 &xqmstat_proc_fops))
 		goto out_remove_stat_file;
 	if (!proc_create("fs/xfs/xqm", 0, NULL,
-			&xqm_proc_fops))
+			 &xqm_proc_fops))
 		goto out_remove_xqmstat_file;
 #endif
 	return 0;
 
 #ifdef CONFIG_XFS_QUOTA
-out_remove_xqmstat_file:
+ out_remove_xqmstat_file:
 	remove_proc_entry("fs/xfs/xqmstat", NULL);
-out_remove_stat_file:
+ out_remove_stat_file:
 	remove_proc_entry("fs/xfs/stat", NULL);
 #endif
-out_remove_xfs_dir:
+ out_remove_xfs_dir:
 	remove_proc_entry("fs/xfs", NULL);
-out:
+ out:
 	return -ENOMEM;
 }
 
diff --git a/fs/xfs/xfs_stats.h b/fs/xfs/xfs_stats.h
index 672fe83..00afc13 100644
--- a/fs/xfs/xfs_stats.h
+++ b/fs/xfs/xfs_stats.h
@@ -218,13 +218,16 @@ int xfs_stats_format(struct xfsstats __percpu *stats, char *buf);
 void xfs_stats_clearall(struct xfsstats __percpu *stats);
 extern struct xstats xfsstats;
 
+struct xfs_mount;
+
 /*
  * We don't disable preempt, not too worried about poking the
  * wrong CPU's stat for now (also aggregated before reporting).
  */
 #define XFS_STATS_INC(v)	(per_cpu(*xfsstats.xs_stats, current_cpu()).v++)
 #define XFS_STATS_DEC(v)	(per_cpu(*xfsstats.xs_stats, current_cpu()).v--)
-#define XFS_STATS_ADD(v, inc)	(per_cpu(*xfsstats.xs_stats, current_cpu()).v += (inc))
+#define XFS_STATS_ADD(v, inc)	(per_cpu(*xfsstats.xs_stats, current_cpu()).v \
+				+= (inc))
 
 extern int xfs_init_procfs(void);
 extern void xfs_cleanup_procfs(void);
diff --git a/fs/xfs/xfs_super.c b/fs/xfs/xfs_super.c
index 23e5681..3898643 100644
--- a/fs/xfs/xfs_super.c
+++ b/fs/xfs/xfs_super.c
@@ -1801,7 +1801,6 @@ xfs_destroy_workqueues(void)
 	destroy_workqueue(xfs_alloc_wq);
 }
 
-
 STATIC int __init
 init_xfs_fs(void)
 {
@@ -1843,17 +1842,26 @@ init_xfs_fs(void)
 	}
 
 	xfsstats.xs_stats = alloc_percpu(struct xfsstats);
+	if (!xfsstats.xs_stats) {
+		error = -ENOMEM;
+		goto out_kset_unregister;
+	}
 	xfsstats.xs_kobj.kobject.kset = xfs_kset;
+	if (!xfsstats.xs_kobj.kobject.kset) {
+		error = -ENOMEM;
+		goto out_free_stats;
+	}
+
 	error = xfs_sysfs_init(&xfsstats.xs_kobj, &xfs_stats_ktype, NULL,
-			       "stats");
+				"stats");
 	if (error)
-		goto out_kset_unregister;
+		goto out_free_stats;
 
 #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_remove_stats;
+		goto out_del_stats;
 #endif
 
 	error = xfs_qm_init();
@@ -1870,8 +1878,10 @@ init_xfs_fs(void)
  out_remove_dbg_kobj:
 #ifdef DEBUG
 	xfs_sysfs_del(&xfs_dbg_kobj);
- out_remove_stats:
+ out_del_stats:
 #endif
+	xfs_sysfs_del(&xfsstats.xs_kobj);
+ out_free_stats:
 	free_percpu(xfsstats.xs_stats);
  out_kset_unregister:
 	kset_unregister(xfs_kset);
@@ -1899,6 +1909,7 @@ exit_xfs_fs(void)
 #ifdef DEBUG
 	xfs_sysfs_del(&xfs_dbg_kobj);
 #endif
+	xfs_sysfs_del(&xfsstats.xs_kobj);
 	free_percpu(xfsstats.xs_stats);
 	kset_unregister(xfs_kset);
 	xfs_sysctl_unregister();
diff --git a/fs/xfs/xfs_sysfs.c b/fs/xfs/xfs_sysfs.c
index 3275408..a6bc433 100644
--- a/fs/xfs/xfs_sysfs.c
+++ b/fs/xfs/xfs_sysfs.c
@@ -143,9 +143,9 @@ to_xstats(struct kobject *kobject)
 STATIC ssize_t
 stats_show(
 	struct kobject	*kobject,
-	char	*buf)
+	char		*buf)
 {
-	struct xstats *stats = to_xstats(kobject);
+	struct xstats	*stats = to_xstats(kobject);
 
 	return xfs_stats_format(stats->xs_stats, buf);
 }
@@ -159,7 +159,7 @@ stats_clear_store(
 {
 	int		ret;
 	int		val;
-	struct xstats *stats = to_xstats(kobject);
+	struct xstats	*stats = to_xstats(kobject);
 
 	ret = kstrtoint(buf, 0, &val);
 	if (ret)
-- 
2.4.3

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

Re: [PATCH 6/7] xfs: per-filesystem stats in sysfs.

[Eric Sandeen]

On 9/25/15 6:22 PM, Bill O'Donnell wrote:
> This patch implements per-filesystem stats objects in sysfs. It
> depends on the application of the previous patch series that
> develops the infrastructure to support both xfs global stats and
> xfs per-fs stats in sysfs.
> 
> Stats objects are instantiated when an xfs filesystem is mounted
> and deleted on unmount. With this patch, the stats directory is
> created and populated with the familiar stats and stats_clear files.
> Example:
>         /sys/fs/xfs/sda9/stats/stats
>         /sys/fs/xfs/sda9/stats/stats_clear
> 
> With this patch, the individual counts within the new per-fs
> stats file(s) remain at zero. Functions that use the the macros
> to increment, decrement, and add-to the per-fs stats counts will
> be covered in a separate new patch to follow this one. Note that
> the counts within the global stats file (/sys/fs/xfs/stats/stats)
> advance normally and can be cleared as it was prior to this patch.
> 
> Signed-off-by: Bill O'Donnell <billodo@redhat.com>
> ---
>  fs/xfs/xfs_linux.h |  4 ++--
>  fs/xfs/xfs_mount.c | 24 +++++++++++++++++++++++-
>  fs/xfs/xfs_mount.h |  1 +
>  fs/xfs/xfs_stats.c | 34 +++++++++++++++++++---------------
>  fs/xfs/xfs_stats.h |  5 ++++-
>  fs/xfs/xfs_super.c | 21 ++++++++++++++++-----
>  fs/xfs/xfs_sysfs.c |  6 +++---
>  7 files changed, 68 insertions(+), 27 deletions(-)

> diff --git a/fs/xfs/xfs_mount.c b/fs/xfs/xfs_mount.c
> index bf92e0c..5921d18 100644
> --- a/fs/xfs/xfs_mount.c
> +++ b/fs/xfs/xfs_mount.c
> @@ -791,11 +791,25 @@ xfs_mountfs(
>  		goto out_free_dir;
>  	}
>  
> +	/*
> +	 * Allocate stats memory and create stats sysfs object.
> +	 */
> +	mp->m_stats.xs_stats = alloc_percpu(struct xfsstats);
> +	if (IS_ERR(mp->m_stats.xs_stats)) {
> +		error = PTR_ERR(mp->m_stats.xs_stats);
> +		goto out_free_perag;
> +	}

alloc_percpu simply returns NULL on failure AFAIK, so looking for IS_ERR
doesn't seem right.

Also, I think the placement of this in the routine is a little odd;
I'd move it up right under where we create the per-fs sysfs entry, i.e.:

        error = xfs_sysfs_init(&mp->m_kobj, &xfs_mp_ktype, NULL, mp->m_fsname);
        if (error)
                goto out;

+	/*
+	 * Allocate stats memory and create stats sysfs object.
+	 */
+	mp->m_stats.xs_stats = alloc_percpu(struct xfsstats);
+	if (!mp->m_stats.xs_stats) {
+		error = -ENOMEM;
+		goto out_free_perag;
+	}

... etc ...

so that all the sysfs gunk is in the same area.


> +	error = xfs_sysfs_init(&mp->m_stats.xs_kobj, &xfs_stats_ktype,
> +				&mp->m_kobj,
> +				"stats");
> +	if (error)
> +		goto out_free_stats;
> +
>  	if (!sbp->sb_logblocks) {
>  		xfs_warn(mp, "no log defined");
>  		XFS_ERROR_REPORT("xfs_mountfs", XFS_ERRLEVEL_LOW, mp);
>  		error = -EFSCORRUPTED;
> -		goto out_free_perag;
> +		goto out_del_stats;
>  	}
>  
>  	/*
> @@ -965,6 +979,10 @@ xfs_mountfs(
>  	if (mp->m_logdev_targp && mp->m_logdev_targp != mp->m_ddev_targp)
>  		xfs_wait_buftarg(mp->m_logdev_targp);
>  	xfs_wait_buftarg(mp->m_ddev_targp);
> + out_del_stats:
> +	xfs_sysfs_del(&mp->m_stats.xs_kobj);
> + out_free_stats:
> +	free_percpu(mp->m_stats.xs_stats);
>   out_free_perag:
>  	xfs_free_perag(mp);
>   out_free_dir:
> @@ -1047,6 +1065,10 @@ xfs_unmountfs(
>  		xfs_warn(mp, "Unable to update superblock counters. "
>  				"Freespace may not be correct on next mount.");
>  
> +	/* remove the stats kobject and free stats memory */
> +	xfs_sysfs_del(&mp->m_stats.xs_kobj);
> +	free_percpu(mp->m_stats.xs_stats);
> +
>  	xfs_log_unmount(mp);
>  	xfs_da_unmount(mp);
>  	xfs_uuid_unmount(mp);
> diff --git a/fs/xfs/xfs_mount.h b/fs/xfs/xfs_mount.h
> index 7999e91..8795272 100644
> --- a/fs/xfs/xfs_mount.h
> +++ b/fs/xfs/xfs_mount.h
> @@ -127,6 +127,7 @@ typedef struct xfs_mount {
>  	int64_t			m_low_space[XFS_LOWSP_MAX];
>  						/* low free space thresholds */
>  	struct xfs_kobj		m_kobj;
> +	struct xstats		m_stats;	/* per-fs stats */
>  
>  	struct workqueue_struct *m_buf_workqueue;
>  	struct workqueue_struct	*m_data_workqueue;
> diff --git a/fs/xfs/xfs_stats.c b/fs/xfs/xfs_stats.c
> index ab79973..d5b3704 100644
> --- a/fs/xfs/xfs_stats.c
> +++ b/fs/xfs/xfs_stats.c
> @@ -18,6 +18,11 @@
>  #include "xfs.h"
>  #include <linux/proc_fs.h>
>  
> +#include "xfs_format.h"
> +#include "xfs_trans_resv.h"
> +#include "xfs_mount.h"
> +#include "xfs_sysfs.h"

AFAICT, none of these includes are needed

...

> @@ -1843,17 +1842,26 @@ init_xfs_fs(void)
>  	}
>  
>  	xfsstats.xs_stats = alloc_percpu(struct xfsstats);
> +	if (!xfsstats.xs_stats) {
> +		error = -ENOMEM;
> +		goto out_kset_unregister;
> +	}

This error checking should be added when xs_stats is introduced
in patch 5.

>  	xfsstats.xs_kobj.kobject.kset = xfs_kset;
> +	if (!xfsstats.xs_kobj.kobject.kset) {
> +		error = -ENOMEM;
> +		goto out_free_stats;
> +	}
> +

this can't fail, you're just assigning one thing to another, no need
for this test.  We already tested for failure to allocate xfs_kset.

>  	error = xfs_sysfs_init(&xfsstats.xs_kobj, &xfs_stats_ktype, NULL,
> -			       "stats");
> +				"stats");

if this is the alignment you want, do it in the patch that introduced it...

>  	if (error)
> -		goto out_kset_unregister;
> +		goto out_free_stats;
>  
>  #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_remove_stats;
> +		goto out_del_stats;
>  #endif
>  
>  	error = xfs_qm_init();
> @@ -1870,8 +1878,10 @@ init_xfs_fs(void)
>   out_remove_dbg_kobj:
>  #ifdef DEBUG
>  	xfs_sysfs_del(&xfs_dbg_kobj);
> - out_remove_stats:
> + out_del_stats:

hm, why that name?  If we have:

 out_remove_dbg_kobj:
 	xfs_sysfs_del(&xfs_dbg_kobj);

then we should have:

 out_remove_stats_kobj:
	xfs_sysfs_del(&xfsstats.xs_kobj);

for consistency, I think.


> diff --git a/fs/xfs/xfs_stats.h b/fs/xfs/xfs_stats.h
> index 672fe83..00afc13 100644
> --- a/fs/xfs/xfs_stats.h
> +++ b/fs/xfs/xfs_stats.h
> @@ -218,13 +218,16 @@ int xfs_stats_format(struct xfsstats __percpu *stats, char *buf);
>  void xfs_stats_clearall(struct xfsstats __percpu *stats);
>  extern struct xstats xfsstats;
>  
> +struct xfs_mount;

I don't think that's needed.

-Eric

> +
>  /*
>   * We don't disable preempt, not too worried about poking the
>   * wrong CPU's stat for now (also aggregated before reporting).
>   */



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

[PATCH 6/7] xfs: per-filesystem stats in sysfs.

[Bill O'Donnell]

This patch implements per-filesystem stats objects in sysfs. It
depends on the application of the previous patch series that
develops the infrastructure to support both xfs global stats and
xfs per-fs stats in sysfs.

Stats objects are instantiated when an xfs filesystem is mounted
and deleted on unmount. With this patch, the stats directory is
created and populated with the familiar stats and stats_clear files.
Example:
        /sys/fs/xfs/sda9/stats/stats
        /sys/fs/xfs/sda9/stats/stats_clear

With this patch, the individual counts within the new per-fs
stats file(s) remain at zero. Functions that use the the macros
to increment, decrement, and add-to the per-fs stats counts will
be covered in a separate new patch to follow this one. Note that
the counts within the global stats file (/sys/fs/xfs/stats/stats)
advance normally and can be cleared as it was prior to this patch.

Signed-off-by: Bill O'Donnell <billodo@redhat.com>
---
 fs/xfs/xfs_mount.c | 24 +++++++++++++++++++++++-
 fs/xfs/xfs_mount.h |  1 +
 fs/xfs/xfs_stats.c |  5 +++++
 fs/xfs/xfs_super.c | 12 ++++--------
 4 files changed, 33 insertions(+), 9 deletions(-)

diff --git a/fs/xfs/xfs_mount.c b/fs/xfs/xfs_mount.c
index bf92e0c..89ac1bd 100644
--- a/fs/xfs/xfs_mount.c
+++ b/fs/xfs/xfs_mount.c
@@ -693,9 +693,23 @@ xfs_mountfs(
 	if (error)
 		goto out;
 
+	/*
+	 * Allocate stats memory and create stats sysfs object.
+	 */
+	mp->m_stats.xs_stats = alloc_percpu(struct xfsstats);
+	if (!mp->m_stats.xs_stats) {
+		error = PTR_ERR(mp->m_stats.xs_stats);
+		goto out_remove_sysfs;
+	}
+	error = xfs_sysfs_init(&mp->m_stats.xs_kobj, &xfs_stats_ktype,
+				&mp->m_kobj,
+				"stats");
+	if (error)
+		goto out_free_stats;
+
 	error = xfs_uuid_mount(mp);
 	if (error)
-		goto out_remove_sysfs;
+		goto out_del_stats;
 
 	/*
 	 * Set the minimum read and write sizes
@@ -971,6 +985,10 @@ xfs_mountfs(
 	xfs_da_unmount(mp);
  out_remove_uuid:
 	xfs_uuid_unmount(mp);
+ out_del_stats:
+	xfs_sysfs_del(&mp->m_stats.xs_kobj);
+ out_free_stats:
+	free_percpu(mp->m_stats.xs_stats);
  out_remove_sysfs:
 	xfs_sysfs_del(&mp->m_kobj);
  out:
@@ -1047,6 +1065,10 @@ xfs_unmountfs(
 		xfs_warn(mp, "Unable to update superblock counters. "
 				"Freespace may not be correct on next mount.");
 
+	/* remove the stats kobject and free stats memory */
+	xfs_sysfs_del(&mp->m_stats.xs_kobj);
+	free_percpu(mp->m_stats.xs_stats);
+
 	xfs_log_unmount(mp);
 	xfs_da_unmount(mp);
 	xfs_uuid_unmount(mp);
diff --git a/fs/xfs/xfs_mount.h b/fs/xfs/xfs_mount.h
index 7999e91..8795272 100644
--- a/fs/xfs/xfs_mount.h
+++ b/fs/xfs/xfs_mount.h
@@ -127,6 +127,7 @@ typedef struct xfs_mount {
 	int64_t			m_low_space[XFS_LOWSP_MAX];
 						/* low free space thresholds */
 	struct xfs_kobj		m_kobj;
+	struct xstats		m_stats;	/* per-fs stats */
 
 	struct workqueue_struct *m_buf_workqueue;
 	struct workqueue_struct	*m_data_workqueue;
diff --git a/fs/xfs/xfs_stats.c b/fs/xfs/xfs_stats.c
index 3f9fc05..ef888bb 100644
--- a/fs/xfs/xfs_stats.c
+++ b/fs/xfs/xfs_stats.c
@@ -18,6 +18,11 @@
 #include "xfs.h"
 #include <linux/proc_fs.h>
 
+//#include "xfs_format.h"
+//#include "xfs_trans_resv.h"
+//#include "xfs_mount.h"
+//#include "xfs_sysfs.h"
+
 struct xstats xfsstats;
 
 static int counter_val(struct xfsstats __percpu *stats, int idx)
diff --git a/fs/xfs/xfs_super.c b/fs/xfs/xfs_super.c
index 5901336..68d4976 100644
--- a/fs/xfs/xfs_super.c
+++ b/fs/xfs/xfs_super.c
@@ -1841,18 +1841,14 @@ init_xfs_fs(void)
 		goto out_sysctl_unregister;
 	}
 
+	xfsstats.xs_kobj.kobject.kset = xfs_kset;
+
 	xfsstats.xs_stats = alloc_percpu(struct xfsstats);
 	if (!xfsstats.xs_stats) {
 		error = -ENOMEM;
 		goto out_kset_unregister;
 	}
 
-	xfsstats.xs_kobj.kobject.kset = xfs_kset;
-	if (!xfsstats.xs_kobj.kobject.kset) {
-		error = -ENOMEM;
-		goto out_free_stats;
-	}
-
 	error = xfs_sysfs_init(&xfsstats.xs_kobj, &xfs_stats_ktype, NULL,
 			       "stats");
 	if (error)
@@ -1862,7 +1858,7 @@ init_xfs_fs(void)
 	xfs_dbg_kobj.kobject.kset = xfs_kset;
 	error = xfs_sysfs_init(&xfs_dbg_kobj, &xfs_dbg_ktype, NULL, "debug");
 	if (error)
-		goto out_del_stats;
+		goto out_remove_stats_obj;
 #endif
 
 	error = xfs_qm_init();
@@ -1879,7 +1875,7 @@ init_xfs_fs(void)
  out_remove_dbg_kobj:
 #ifdef DEBUG
 	xfs_sysfs_del(&xfs_dbg_kobj);
- out_del_stats:
+ out_remove_stats_obj:
 #endif
 	xfs_sysfs_del(&xfsstats.xs_kobj);
  out_free_stats:
-- 
2.4.3

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

[PATCH 6/7] xfs: per-filesystem stats in sysfs.

[Bill O'Donnell]

This patch implements per-filesystem stats objects in sysfs. It
depends on the application of the previous patch series that
develops the infrastructure to support both xfs global stats and
xfs per-fs stats in sysfs.

Stats objects are instantiated when an xfs filesystem is mounted
and deleted on unmount. With this patch, the stats directory is
created and populated with the familiar stats and stats_clear files.
Example:
        /sys/fs/xfs/sda9/stats/stats
        /sys/fs/xfs/sda9/stats/stats_clear

With this patch, the individual counts within the new per-fs
stats file(s) remain at zero. Functions that use the the macros
to increment, decrement, and add-to the per-fs stats counts will
be covered in a separate new patch to follow this one. Note that
the counts within the global stats file (/sys/fs/xfs/stats/stats)
advance normally and can be cleared as it was prior to this patch.

Signed-off-by: Bill O'Donnell <billodo@redhat.com>
---
 fs/xfs/xfs_mount.c | 24 +++++++++++++++++++++++-
 fs/xfs/xfs_mount.h |  1 +
 fs/xfs/xfs_super.c | 12 ++++--------
 3 files changed, 28 insertions(+), 9 deletions(-)

diff --git a/fs/xfs/xfs_mount.c b/fs/xfs/xfs_mount.c
index bf92e0c..89ac1bd 100644
--- a/fs/xfs/xfs_mount.c
+++ b/fs/xfs/xfs_mount.c
@@ -693,9 +693,23 @@ xfs_mountfs(
 	if (error)
 		goto out;
 
+	/*
+	 * Allocate stats memory and create stats sysfs object.
+	 */
+	mp->m_stats.xs_stats = alloc_percpu(struct xfsstats);
+	if (!mp->m_stats.xs_stats) {
+		error = PTR_ERR(mp->m_stats.xs_stats);
+		goto out_remove_sysfs;
+	}
+	error = xfs_sysfs_init(&mp->m_stats.xs_kobj, &xfs_stats_ktype,
+				&mp->m_kobj,
+				"stats");
+	if (error)
+		goto out_free_stats;
+
 	error = xfs_uuid_mount(mp);
 	if (error)
-		goto out_remove_sysfs;
+		goto out_del_stats;
 
 	/*
 	 * Set the minimum read and write sizes
@@ -971,6 +985,10 @@ xfs_mountfs(
 	xfs_da_unmount(mp);
  out_remove_uuid:
 	xfs_uuid_unmount(mp);
+ out_del_stats:
+	xfs_sysfs_del(&mp->m_stats.xs_kobj);
+ out_free_stats:
+	free_percpu(mp->m_stats.xs_stats);
  out_remove_sysfs:
 	xfs_sysfs_del(&mp->m_kobj);
  out:
@@ -1047,6 +1065,10 @@ xfs_unmountfs(
 		xfs_warn(mp, "Unable to update superblock counters. "
 				"Freespace may not be correct on next mount.");
 
+	/* remove the stats kobject and free stats memory */
+	xfs_sysfs_del(&mp->m_stats.xs_kobj);
+	free_percpu(mp->m_stats.xs_stats);
+
 	xfs_log_unmount(mp);
 	xfs_da_unmount(mp);
 	xfs_uuid_unmount(mp);
diff --git a/fs/xfs/xfs_mount.h b/fs/xfs/xfs_mount.h
index 7999e91..8795272 100644
--- a/fs/xfs/xfs_mount.h
+++ b/fs/xfs/xfs_mount.h
@@ -127,6 +127,7 @@ typedef struct xfs_mount {
 	int64_t			m_low_space[XFS_LOWSP_MAX];
 						/* low free space thresholds */
 	struct xfs_kobj		m_kobj;
+	struct xstats		m_stats;	/* per-fs stats */
 
 	struct workqueue_struct *m_buf_workqueue;
 	struct workqueue_struct	*m_data_workqueue;
diff --git a/fs/xfs/xfs_super.c b/fs/xfs/xfs_super.c
index 5901336..68d4976 100644
--- a/fs/xfs/xfs_super.c
+++ b/fs/xfs/xfs_super.c
@@ -1841,18 +1841,14 @@ init_xfs_fs(void)
 		goto out_sysctl_unregister;
 	}
 
+	xfsstats.xs_kobj.kobject.kset = xfs_kset;
+
 	xfsstats.xs_stats = alloc_percpu(struct xfsstats);
 	if (!xfsstats.xs_stats) {
 		error = -ENOMEM;
 		goto out_kset_unregister;
 	}
 
-	xfsstats.xs_kobj.kobject.kset = xfs_kset;
-	if (!xfsstats.xs_kobj.kobject.kset) {
-		error = -ENOMEM;
-		goto out_free_stats;
-	}
-
 	error = xfs_sysfs_init(&xfsstats.xs_kobj, &xfs_stats_ktype, NULL,
 			       "stats");
 	if (error)
@@ -1862,7 +1858,7 @@ init_xfs_fs(void)
 	xfs_dbg_kobj.kobject.kset = xfs_kset;
 	error = xfs_sysfs_init(&xfs_dbg_kobj, &xfs_dbg_ktype, NULL, "debug");
 	if (error)
-		goto out_del_stats;
+		goto out_remove_stats_obj;
 #endif
 
 	error = xfs_qm_init();
@@ -1879,7 +1875,7 @@ init_xfs_fs(void)
  out_remove_dbg_kobj:
 #ifdef DEBUG
 	xfs_sysfs_del(&xfs_dbg_kobj);
- out_del_stats:
+ out_remove_stats_obj:
 #endif
 	xfs_sysfs_del(&xfsstats.xs_kobj);
  out_free_stats:
-- 
2.4.3

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

[PATCH 6/7] xfs: per-filesystem stats in sysfs.

[Bill O'Donnell]

This patch implements per-filesystem stats objects in sysfs. It
depends on the application of the previous patch series that
develops the infrastructure to support both xfs global stats and
xfs per-fs stats in sysfs.

Stats objects are instantiated when an xfs filesystem is mounted
and deleted on unmount. With this patch, the stats directory is
created and populated with the familiar stats and stats_clear files.
Example:
        /sys/fs/xfs/sda9/stats/stats
        /sys/fs/xfs/sda9/stats/stats_clear

With this patch, the individual counts within the new per-fs
stats file(s) remain at zero. Functions that use the the macros
to increment, decrement, and add-to the per-fs stats counts will
be covered in a separate new patch to follow this one. Note that
the counts within the global stats file (/sys/fs/xfs/stats/stats)
advance normally and can be cleared as it was prior to this patch.

Signed-off-by: Bill O'Donnell <billodo@redhat.com>
---
 fs/xfs/xfs_mount.c | 24 +++++++++++++++++++++++-
 fs/xfs/xfs_mount.h |  1 +
 2 files changed, 24 insertions(+), 1 deletion(-)

diff --git a/fs/xfs/xfs_mount.c b/fs/xfs/xfs_mount.c
index bf92e0c..89ac1bd 100644
--- a/fs/xfs/xfs_mount.c
+++ b/fs/xfs/xfs_mount.c
@@ -693,9 +693,23 @@ xfs_mountfs(
 	if (error)
 		goto out;
 
+	/*
+	 * Allocate stats memory and create stats sysfs object.
+	 */
+	mp->m_stats.xs_stats = alloc_percpu(struct xfsstats);
+	if (!mp->m_stats.xs_stats) {
+		error = PTR_ERR(mp->m_stats.xs_stats);
+		goto out_remove_sysfs;
+	}
+	error = xfs_sysfs_init(&mp->m_stats.xs_kobj, &xfs_stats_ktype,
+				&mp->m_kobj,
+				"stats");
+	if (error)
+		goto out_free_stats;
+
 	error = xfs_uuid_mount(mp);
 	if (error)
-		goto out_remove_sysfs;
+		goto out_del_stats;
 
 	/*
 	 * Set the minimum read and write sizes
@@ -971,6 +985,10 @@ xfs_mountfs(
 	xfs_da_unmount(mp);
  out_remove_uuid:
 	xfs_uuid_unmount(mp);
+ out_del_stats:
+	xfs_sysfs_del(&mp->m_stats.xs_kobj);
+ out_free_stats:
+	free_percpu(mp->m_stats.xs_stats);
  out_remove_sysfs:
 	xfs_sysfs_del(&mp->m_kobj);
  out:
@@ -1047,6 +1065,10 @@ xfs_unmountfs(
 		xfs_warn(mp, "Unable to update superblock counters. "
 				"Freespace may not be correct on next mount.");
 
+	/* remove the stats kobject and free stats memory */
+	xfs_sysfs_del(&mp->m_stats.xs_kobj);
+	free_percpu(mp->m_stats.xs_stats);
+
 	xfs_log_unmount(mp);
 	xfs_da_unmount(mp);
 	xfs_uuid_unmount(mp);
diff --git a/fs/xfs/xfs_mount.h b/fs/xfs/xfs_mount.h
index 7999e91..8795272 100644
--- a/fs/xfs/xfs_mount.h
+++ b/fs/xfs/xfs_mount.h
@@ -127,6 +127,7 @@ typedef struct xfs_mount {
 	int64_t			m_low_space[XFS_LOWSP_MAX];
 						/* low free space thresholds */
 	struct xfs_kobj		m_kobj;
+	struct xstats		m_stats;	/* per-fs stats */
 
 	struct workqueue_struct *m_buf_workqueue;
 	struct workqueue_struct	*m_data_workqueue;
-- 
2.4.3

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

Re: [PATCH 6/7] xfs: per-filesystem stats in sysfs.

[Dave Chinner]

On Fri, Oct 02, 2015 at 11:22:39AM -0500, Bill O'Donnell wrote:
> This patch implements per-filesystem stats objects in sysfs. It
> depends on the application of the previous patch series that
> develops the infrastructure to support both xfs global stats and
> xfs per-fs stats in sysfs.
> 
> Stats objects are instantiated when an xfs filesystem is mounted
> and deleted on unmount. With this patch, the stats directory is
> created and populated with the familiar stats and stats_clear files.
> Example:
>         /sys/fs/xfs/sda9/stats/stats
>         /sys/fs/xfs/sda9/stats/stats_clear
> 
> With this patch, the individual counts within the new per-fs
> stats file(s) remain at zero. Functions that use the the macros
> to increment, decrement, and add-to the per-fs stats counts will
> be covered in a separate new patch to follow this one. Note that
> the counts within the global stats file (/sys/fs/xfs/stats/stats)
> advance normally and can be cleared as it was prior to this patch.
> 
> Signed-off-by: Bill O'Donnell <billodo@redhat.com>

.....

> @@ -1047,6 +1065,10 @@ xfs_unmountfs(
>  		xfs_warn(mp, "Unable to update superblock counters. "
>  				"Freespace may not be correct on next mount.");
>  
> +	/* remove the stats kobject and free stats memory */
> +	xfs_sysfs_del(&mp->m_stats.xs_kobj);
> +	free_percpu(mp->m_stats.xs_stats);
> +
>  	xfs_log_unmount(mp);

Ok, there's an obvious (to me!) problem here. xfs_log_unmount() is
called while there are still active objects in the AIL, which means
there is still AIL operations that need to be done. The AIL
operations have stats in them, so this will result in use-after-free
of the mp->m_stats structure here.

Actually, looking at the alloc side of this patch, we're doing a
XFS_STATS_INC(xb_create) through:

xfs_fs_fill_super
  xfs_readsb
    xfs_buf_read_uncached
      xfs_buf_get_uncached
        _xfs_buf_alloc
	  XFS_STATS_INC(target->bt_mount, xb_create)

at mount time before we've set up mp->m_stats. Ok, so the
initialisation and teardown of the per-mount stats memory is being
done at the wrong layer - it needs to be done in xfs_fs_fill_super()
before we do any IO at all and dropped in xfs_fs_put_super() after
we've torn down all the IO structures. The sysfs object
initialisation is fine where it is, but the actual stats storage is
needed earlier/later than the sysfs object is.

/me now wonders why this doesn't go kaboom at mount time...

Reworked patch is inline below.

Cheers,

Dave.
-- 
Dave Chinner
david@fromorbit.com

xfs: per-filesystem stats in sysfs

From: Bill O'Donnell <billodo@redhat.com>

This patch implements per-filesystem stats objects in sysfs. It
depends on the application of the previous patch series that
develops the infrastructure to support both xfs global stats and
xfs per-fs stats in sysfs.

Stats objects are instantiated when an xfs filesystem is mounted
and deleted on unmount. With this patch, the stats directory is
created and populated with the familiar stats and stats_clear files.
Example:
        /sys/fs/xfs/sda9/stats/stats
        /sys/fs/xfs/sda9/stats/stats_clear

With this patch, the individual counts within the new per-fs
stats file(s) remain at zero. Functions that use the the macros
to increment, decrement, and add-to the per-fs stats counts will
be covered in a separate new patch to follow this one. Note that
the counts within the global stats file (/sys/fs/xfs/stats/stats)
advance normally and can be cleared as it was prior to this patch.

[dchinner: move setup/teardown to xfs_fs_{fill|put}_super() so
it is down before/after any path that uses the per-mount stats. ]

Signed-off-by: Bill O'Donnell <billodo@redhat.com>
Reviewed-by: Eric Sandeen <sandeen@redhat.com>
Signed-off-by: Dave Chinner <david@fromorbit.com>

---
 fs/xfs/xfs_mount.c | 11 ++++++++++-
 fs/xfs/xfs_mount.h |  1 +
 fs/xfs/xfs_super.c | 14 ++++++++++++--
 3 files changed, 23 insertions(+), 3 deletions(-)

diff --git a/fs/xfs/xfs_mount.c b/fs/xfs/xfs_mount.c
index bf92e0c..eb498ce 100644
--- a/fs/xfs/xfs_mount.c
+++ b/fs/xfs/xfs_mount.c
@@ -693,10 +693,15 @@ xfs_mountfs(
 	if (error)
 		goto out;
 
-	error = xfs_uuid_mount(mp);
+	error = xfs_sysfs_init(&mp->m_stats.xs_kobj, &xfs_stats_ktype,
+			       &mp->m_kobj, "stats");
 	if (error)
 		goto out_remove_sysfs;
 
+	error = xfs_uuid_mount(mp);
+	if (error)
+		goto out_del_stats;
+
 	/*
 	 * Set the minimum read and write sizes
 	 */
@@ -971,6 +976,8 @@ xfs_mountfs(
 	xfs_da_unmount(mp);
  out_remove_uuid:
 	xfs_uuid_unmount(mp);
+ out_del_stats:
+	xfs_sysfs_del(&mp->m_stats.xs_kobj);
  out_remove_sysfs:
 	xfs_sysfs_del(&mp->m_kobj);
  out:
@@ -1047,6 +1054,7 @@ xfs_unmountfs(
 		xfs_warn(mp, "Unable to update superblock counters. "
 				"Freespace may not be correct on next mount.");
 
+
 	xfs_log_unmount(mp);
 	xfs_da_unmount(mp);
 	xfs_uuid_unmount(mp);
@@ -1056,6 +1064,7 @@ xfs_unmountfs(
 #endif
 	xfs_free_perag(mp);
 
+	xfs_sysfs_del(&mp->m_stats.xs_kobj);
 	xfs_sysfs_del(&mp->m_kobj);
 }
 
diff --git a/fs/xfs/xfs_mount.h b/fs/xfs/xfs_mount.h
index 404bfa5..f20e5de 100644
--- a/fs/xfs/xfs_mount.h
+++ b/fs/xfs/xfs_mount.h
@@ -127,6 +127,7 @@ typedef struct xfs_mount {
 	int64_t			m_low_space[XFS_LOWSP_MAX];
 						/* low free space thresholds */
 	struct xfs_kobj		m_kobj;
+	struct xstats		m_stats;	/* per-fs stats */
 
 	struct workqueue_struct *m_buf_workqueue;
 	struct workqueue_struct	*m_data_workqueue;
diff --git a/fs/xfs/xfs_super.c b/fs/xfs/xfs_super.c
index e1a35a5..9d11d3e 100644
--- a/fs/xfs/xfs_super.c
+++ b/fs/xfs/xfs_super.c
@@ -1474,9 +1474,16 @@ xfs_fs_fill_super(
 	if (error)
 		goto out_destroy_workqueues;
 
+	/* Allocate stats memory before we do operations that might use it */
+	mp->m_stats.xs_stats = alloc_percpu(struct xfsstats);
+	if (!mp->m_stats.xs_stats) {
+		error = PTR_ERR(mp->m_stats.xs_stats);
+		goto out_destroy_counters;
+	}
+
 	error = xfs_readsb(mp, flags);
 	if (error)
-		goto out_destroy_counters;
+		goto out_free_stats;
 
 	error = xfs_finish_flags(mp);
 	if (error)
@@ -1545,9 +1552,11 @@ xfs_fs_fill_super(
 	xfs_filestream_unmount(mp);
  out_free_sb:
 	xfs_freesb(mp);
+ out_free_stats:
+	free_percpu(mp->m_stats.xs_stats);
  out_destroy_counters:
 	xfs_destroy_percpu_counters(mp);
-out_destroy_workqueues:
+ out_destroy_workqueues:
 	xfs_destroy_mount_workqueues(mp);
  out_close_devices:
 	xfs_close_devices(mp);
@@ -1574,6 +1583,7 @@ xfs_fs_put_super(
 	xfs_unmountfs(mp);
 
 	xfs_freesb(mp);
+	free_percpu(mp->m_stats.xs_stats);
 	xfs_destroy_percpu_counters(mp);
 	xfs_destroy_mount_workqueues(mp);
 	xfs_close_devices(mp);

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