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@xxxxxxxxxx>
> ---
> 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@xxxxxxxxxxx
http://oss.sgi.com/mailman/listinfo/xfs
