lkml.org 
[lkml]   [2021]   [Sep]   [13]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    Date
    From
    SubjectRe: [PATCH 13/13] xfs: convert xfs_sysfs attrs to use ->seq_show
    On Mon, Sep 13, 2021 at 07:41:21AM +0200, Christoph Hellwig wrote:
    > Trivial conversion to the seq_file based sysfs attributes.
    >
    > Signed-off-by: Christoph Hellwig <hch@lst.de>
    > ---
    > fs/xfs/xfs_stats.c | 24 +++++-------
    > fs/xfs/xfs_stats.h | 2 +-
    > fs/xfs/xfs_sysfs.c | 96 +++++++++++++++++++++++-----------------------
    > 3 files changed, 58 insertions(+), 64 deletions(-)
    >
    > diff --git a/fs/xfs/xfs_stats.c b/fs/xfs/xfs_stats.c
    > index 20e0534a772c9..71e7a84ba0403 100644
    > --- a/fs/xfs/xfs_stats.c
    > +++ b/fs/xfs/xfs_stats.c
    > @@ -16,10 +16,9 @@ static int counter_val(struct xfsstats __percpu *stats, int idx)
    > return val;
    > }
    >
    > -int xfs_stats_format(struct xfsstats __percpu *stats, char *buf)
    > +void xfs_stats_format(struct xfsstats __percpu *stats, struct seq_file *sf)
    > {
    > int i, j;
    > - int len = 0;
    > uint64_t xs_xstrat_bytes = 0;
    > uint64_t xs_write_bytes = 0;
    > uint64_t xs_read_bytes = 0;
    > @@ -58,13 +57,12 @@ int xfs_stats_format(struct xfsstats __percpu *stats, char *buf)
    > /* Loop over all stats groups */
    >
    > for (i = j = 0; i < ARRAY_SIZE(xstats); i++) {
    > - len += scnprintf(buf + len, PATH_MAX - len, "%s",
    > - xstats[i].desc);
    > + seq_printf(sf, "%s", xstats[i].desc);
    > +
    > /* inner loop does each group */
    > for (; j < xstats[i].endpoint; j++)
    > - len += scnprintf(buf + len, PATH_MAX - len, " %u",
    > - counter_val(stats, j));
    > - len += scnprintf(buf + len, PATH_MAX - len, "\n");
    > + seq_printf(sf, " %u", counter_val(stats, j));
    > + seq_printf(sf, "\n");
    > }
    > /* extra precision counters */
    > for_each_possible_cpu(i) {
    > @@ -74,18 +72,14 @@ int xfs_stats_format(struct xfsstats __percpu *stats, char *buf)
    > defer_relog += per_cpu_ptr(stats, i)->s.defer_relog;
    > }
    >
    > - len += scnprintf(buf + len, PATH_MAX-len, "xpc %Lu %Lu %Lu\n",
    > + seq_printf(sf, "xpc %Lu %Lu %Lu\n",
    > xs_xstrat_bytes, xs_write_bytes, xs_read_bytes);
    > - len += scnprintf(buf + len, PATH_MAX-len, "defer_relog %llu\n",
    > - defer_relog);
    > - len += scnprintf(buf + len, PATH_MAX-len, "debug %u\n",
    > + seq_printf(sf, "defer_relog %llu\n", defer_relog);
    > #if defined(DEBUG)
    > - 1);
    > + seq_printf(sf, "debug 1\n");
    > #else
    > - 0);
    > + seq_printf(sf, "debug 0\n");
    > #endif
    > -
    > - return len;
    > }

    That is a sysfs file? What happened to the "one value per file" rule
    here?

    Ugh.

    Anyway, I like the idea, but as you can see here, it could lead to even
    more abuse of sysfs files. We are just now getting people to use
    sysfs_emit() and that is showing us where people have been abusing the
    api in bad ways.

    Is there any way that sysfs can keep the existing show functionality and
    just do the seq_printf() for the buffer returned by the attribute file
    inside of the sysfs core? That would allow us to keep all of the
    existing attribute file functions as-is, and still get rid of the sysfs
    core usage here?

    thanks,

    greg k-h

    \
     
     \ /
      Last update: 2021-09-13 08:28    [W:2.853 / U:0.256 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site