On 9/25/15 6:22 PM, Bill O'Donnell wrote:
> This patch is the next step toward per-fs xfs stats. Allocate &
> deallocate per-fs stats structures and set up the sysfs entries for them.
>
> Instead of a single global xfsstats structure, add kobject and a pointer
> to a per-cpu struct xfsstats. Modify the macros that manipulate the stats
> accordingly: XFS_STATS_INC, XFS_STATS_DEC, and XFS_STATS_ADD now access
> xfsstats->xs_stats.
>
> The sysfs functions need to get from the kobject back to the xfsstats
> structure which contains it, and pass the pointer to the ->xs_stats
> percpu structure into the show & clear routines.
>
>
> Signed-off-by: Bill O'Donnell <billodo@xxxxxxxxxx>
<snip>
> diff --git a/fs/xfs/xfs_super.c b/fs/xfs/xfs_super.c
> index 0dfc53b..23e5681 100644
> --- a/fs/xfs/xfs_super.c
> +++ b/fs/xfs/xfs_super.c
> @@ -61,7 +61,6 @@ 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
> @@ -1802,6 +1801,7 @@ xfs_destroy_workqueues(void)
> destroy_workqueue(xfs_alloc_wq);
> }
>
> +
> STATIC int __init
> init_xfs_fs(void)
> {
> @@ -1842,8 +1842,9 @@ init_xfs_fs(void)
> goto out_sysctl_unregister;
> }
>
> - xfs_stats_kobj.kobject.kset = xfs_kset;
> - error = xfs_sysfs_init(&xfs_stats_kobj, &xfs_stats_ktype, NULL,
> + xfsstats.xs_stats = alloc_percpu(struct xfsstats);
alloc_percpu can fail, so:
if (!xfsstats.xs_stats) {
error = -ENOMEM;
goto out_kset_unregister ...
}
> + xfsstats.xs_kobj.kobject.kset = xfs_kset;
> + error = xfs_sysfs_init(&xfsstats.xs_kobj, &xfs_stats_ktype, NULL,
> "stats");
> if (error)
> goto out_kset_unregister;
If this fails, you need to free the percpu allocation, so this would
be "goto out_free_stats" I think
> @@ -1852,7 +1853,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_remove_stats_kobj;
> + goto out_remove_stats;
And this remains as out_remove_stats_kobj, I think.
> #endif
>
> error = xfs_qm_init();
> @@ -1869,9 +1870,9 @@ init_xfs_fs(void)
> out_remove_dbg_kobj:
> #ifdef DEBUG
> xfs_sysfs_del(&xfs_dbg_kobj);
> - out_remove_stats_kobj:
> + out_remove_stats:
> #endif
> - xfs_sysfs_del(&xfs_stats_kobj);
> + free_percpu(xfsstats.xs_stats);
hm, now nothing deletes the stats kobject?
xfs_sysfs_del(&xfsstats.xs_kobj);
but there should be another goto target ...
there's one more thing to unwind (the percpu allocation) so you should
end up with more unwind goto target (i.e. out_free_stats), but you still
need the unwind target for the stats kobject sysfs deletion.
something like:
out_remove_dbg_kobj:
#ifdef DEBUG
xfs_sysfs_del(&xfs_dbg_kobj);
out_remove_stats_kobj:
#endif
xfs_sysfs_del(&xfs_stats_kobj);
out_free_stats:
free_percpu(xfsstats.xs_stats);
out_kset_unregister:
kset_unregister(xfs_kset);
out_sysctl_unregister:
but double-check me ;)
> out_kset_unregister:
> kset_unregister(xfs_kset);
> out_sysctl_unregister:
> @@ -1898,7 +1899,7 @@ exit_xfs_fs(void)
> #ifdef DEBUG
> xfs_sysfs_del(&xfs_dbg_kobj);
> #endif
> - xfs_sysfs_del(&xfs_stats_kobj);
> + free_percpu(xfsstats.xs_stats);
here as well, you still need to delete the stats kobject:
xfs_sysfs_del(&xfsstats.xs_kobj);
> kset_unregister(xfs_kset);
> xfs_sysctl_unregister();
> xfs_cleanup_procfs();
-Eric
|