linux-xfs.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Christoph Hellwig <hch@lst.de>
To: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Cc: Christoph Hellwig <hch@lst.de>,
	"Rafael J. Wysocki" <rafael@kernel.org>,
	Alexander Viro <viro@zeniv.linux.org.uk>,
	Jens Axboe <axboe@kernel.dk>, Tejun Heo <tj@kernel.org>,
	linux-block@vger.kernel.org, linux-xfs@vger.kernel.org,
	linux-fsdevel@vger.kernel.org, linux-kernel@vger.kernel.org
Subject: Re: [PATCH 13/13] xfs: convert xfs_sysfs attrs to use ->seq_show
Date: Tue, 14 Sep 2021 09:30:03 +0200	[thread overview]
Message-ID: <20210914073003.GA31077@lst.de> (raw)
In-Reply-To: <YT7vZthsMCM1uKxm@kroah.com>

On Mon, Sep 13, 2021 at 08:27:50AM +0200, Greg Kroah-Hartman wrote:
> 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.

To be honest I've always seen sysfs_emit as at best a horrible band aid
to enforce the PAGE_SIZE bounds checking.  Better than nothing, but
not a solution at all, as you can't force anyone to actually use it.

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

Well, you'd need one page allocated in the seq_file code, and one in
the sysfs code.  At which point we might as well drop using seq_file
at all.  But in general seq_file seems like a very nice helper for
over flow free printing into a buffer.  If sysfs files actually were
all limited to a single print we wouldn't really need it, and could
just have something like sysfs_emit just with the buffer hidden inside
a structure that is opaqueue to the caller.  But looking at various
attributes that is not exactly the case.  While the majority certainly
uses a single value and a single print statement there is plenty where
this is not the case.  Either because they use multiple values, or
often also because they dynamically append to the string to print
things like comma-separated flags.

  parent reply	other threads:[~2021-09-14  7:30 UTC|newest]

Thread overview: 37+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-09-13  5:41 start switching sysfs attributes to expose the seq_file Christoph Hellwig
2021-09-13  5:41 ` [PATCH 01/13] seq_file: mark seq_get_buf as deprecated Christoph Hellwig
2021-09-13 13:19   ` Christian Brauner
2021-09-13 16:22   ` Daniel Wagner
2021-09-13 16:29   ` Tejun Heo
2021-09-13  5:41 ` [PATCH 02/13] kernfs: remove kernfs_create_file and kernfs_create_file_ns Christoph Hellwig
2021-09-13 13:20   ` Christian Brauner
2021-09-13  5:41 ` [PATCH 03/13] kernfs: remove the unused lockdep_key field in struct kernfs_ops Christoph Hellwig
2021-09-13 13:21   ` Christian Brauner
2021-09-13 16:30   ` Tejun Heo
2021-09-13  5:41 ` [PATCH 04/13] sysfs: split out binary attribute handling from sysfs_add_file_mode_ns Christoph Hellwig
2021-09-13 13:26   ` Christian Brauner
2021-09-13  5:41 ` [PATCH 05/13] sysfs: refactor sysfs_add_file_mode_ns Christoph Hellwig
2021-09-13 13:27   ` Christian Brauner
2021-09-13  5:41 ` [PATCH 06/13] sysfs: simplify sysfs_kf_seq_show Christoph Hellwig
2021-09-13  5:41 ` [PATCH 07/13] sysfs: add ->seq_show support to sysfs_ops Christoph Hellwig
2021-09-13 13:30   ` Christian Brauner
2021-09-13  5:41 ` [PATCH 08/13] block: convert the blk_mq_hw_ctx attrs to use ->seq_show Christoph Hellwig
2021-09-13  5:41 ` [PATCH 09/13] block: convert the blk_integrity " Christoph Hellwig
2021-09-13  5:41 ` [PATCH 10/13] block: convert the request_queue " Christoph Hellwig
2021-09-13  5:41 ` [PATCH 11/13] block: convert the elevator_queue " Christoph Hellwig
2021-09-13  5:41 ` [PATCH 12/13] xfs: convert xfs_errortag " Christoph Hellwig
2021-09-13  5:41 ` [PATCH 13/13] xfs: convert xfs_sysfs " Christoph Hellwig
2021-09-13  6:27   ` Greg Kroah-Hartman
2021-09-14  1:20     ` Dave Chinner
2021-09-14  5:12       ` Greg Kroah-Hartman
2021-09-14 10:56         ` Dave Chinner
2021-09-14  7:30     ` Christoph Hellwig [this message]
2021-09-14 15:28       ` Greg Kroah-Hartman
2021-09-14 15:30         ` Christoph Hellwig
2021-09-14 15:41           ` Greg Kroah-Hartman
2021-09-15  7:04             ` Christoph Hellwig
2021-09-15  7:07               ` Greg Kroah-Hartman
2021-09-13 16:39 ` start switching sysfs attributes to expose the seq_file Bart Van Assche
2021-09-13 16:46   ` Greg Kroah-Hartman
2021-09-14  2:53     ` Bart Van Assche
2021-09-13 16:46 ` Tejun Heo

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20210914073003.GA31077@lst.de \
    --to=hch@lst.de \
    --cc=axboe@kernel.dk \
    --cc=gregkh@linuxfoundation.org \
    --cc=linux-block@vger.kernel.org \
    --cc=linux-fsdevel@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-xfs@vger.kernel.org \
    --cc=rafael@kernel.org \
    --cc=tj@kernel.org \
    --cc=viro@zeniv.linux.org.uk \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body. 
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).