All of lore.kernel.org
 help / color / mirror / Atom feed
From: Greg KH <gregkh@linuxfoundation.org>
To: Alexander Potapenko <glider@google.com>
Cc: LKML <linux-kernel@vger.kernel.org>,
	Andrew Morton <akpm@linux-foundation.org>,
	Andrey Konovalov <andreyknvl@google.com>,
	Dmitriy Vyukov <dvyukov@google.com>,
	Ingo Molnar <mingo@redhat.com>, Marco Elver <elver@google.com>,
	Petr Mladek <pmladek@suse.com>,
	Steven Rostedt <rostedt@goodmis.org>,
	Sergey Senozhatsky <sergey.senozhatsky@gmail.com>,
	Linux Memory Management List <linux-mm@kvack.org>
Subject: Re: [PATCH v2 3/5] docs: ABI: add /sys/kernel/error_report/ documentation
Date: Fri, 15 Jan 2021 16:45:09 +0100	[thread overview]
Message-ID: <YAG4hf4THWFbn2op@kroah.com> (raw)
In-Reply-To: <CAG_fn=Xen6Nd9qJnW6F4r5vgj7WAUo40BHeN_FXKpJ2jrpT6-g@mail.gmail.com>

On Fri, Jan 15, 2021 at 04:26:21PM +0100, Alexander Potapenko wrote:
> > sysfs is "one value per file"
> What about existing interfaces that even export binary blobs through
> sysfs (e.g. /sys/firmware, /sys/boot_params)?
> What qualifies as a "value" in that case?

binary files are fine as the kernel is just a "pipe" through to the
hardware / firmware.  No translation or parsing happens in the kernel.
And that's NOT trace events :)

> > please put something like this in
> > tracefs, as there is no such rules there.  Or debugfs, but please, not
> > sysfs.
> Does tracefs have anything similar to sysfs_notify() or any other way
> to implement a poll() handler?

Don't know, look and see!

> Our main goal is to let users wait on poll(), so that they don't have
> to check the file for new contents every now and then. Is it possible
> with tracefs or debugfs?

debugfs, yes, you can do whatever you want.  tracefs probably has this,
otherwise how would trace tools work?  :)

> Also, for our goal debugfs isn't a particularly good fit, as Android
> kernels do not enable debugfs.

Do you care about Android kernels?  If so, yes, debugfs is not good.
And have you asked the Android kernel team about this?

> Not sure about tracefs, they seem to have it, need to check.

It should be there.

> Do you think it is viable to keep
> /sys/kernel/error_report/report_count in sysfs and use it for
> notifications, but move last_report somewhere else?

No, not at all, please keep it out of sysfs.

> (I'd probably prefer procfs, but it could be tracefs as well, if you
> find that better).

If it does not have to do with processes, it does not belong in procfs.

Again, this seems to fit in with tracing, so please use tracefs.

> This way it will still be possible to easily notify userspace about
> new reports, but the report itself won't have any atomicity
> guarantees. Users will have to check the report count to ensure it
> didn't change under their feet.

Again, use a file in tracefs.

thanks,

greg k-h


  reply	other threads:[~2021-01-15 15:45 UTC|newest]

Thread overview: 22+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-01-15 13:03 [PATCH v2 0/5] Add sysfs interface to collect reports from debugging tools Alexander Potapenko
2021-01-15 13:03 ` [PATCH v2 1/5] tracing: add error_report trace points Alexander Potapenko
2021-01-15 13:03 ` [PATCH v2 2/5] lib: add error_report_notify to collect debugging tools' reports Alexander Potapenko
2021-01-15 13:50   ` Greg KH
2021-01-15 17:17     ` Alexander Potapenko
2021-01-18 11:38   ` Petr Mladek
2021-01-18 13:08     ` Alexander Potapenko
2021-01-18 13:14       ` Alexander Potapenko
2021-01-18 16:43       ` Petr Mladek
2021-01-21 13:13         ` Alexander Potapenko
2021-01-15 13:03 ` [PATCH v2 3/5] docs: ABI: add /sys/kernel/error_report/ documentation Alexander Potapenko
2021-01-15 13:45   ` Greg KH
2021-01-15 15:26     ` Alexander Potapenko
2021-01-15 15:45       ` Greg KH [this message]
2021-01-15 16:52       ` Steven Rostedt
2021-01-18 10:22         ` Alexander Potapenko
2021-01-18 14:52           ` Steven Rostedt
2021-01-15 13:03 ` [PATCH v2 4/5] kfence: use error_report_start and error_report_end tracepoints Alexander Potapenko
2021-01-15 13:03 ` [PATCH v2 5/5] kasan: " Alexander Potapenko
2021-01-15 13:06 ` [PATCH v2 0/5] Add sysfs interface to collect reports from debugging tools Vlastimil Babka
2021-01-15 13:09   ` Alexander Potapenko
2021-01-21 12:56     ` Alexander Potapenko

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=YAG4hf4THWFbn2op@kroah.com \
    --to=gregkh@linuxfoundation.org \
    --cc=akpm@linux-foundation.org \
    --cc=andreyknvl@google.com \
    --cc=dvyukov@google.com \
    --cc=elver@google.com \
    --cc=glider@google.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-mm@kvack.org \
    --cc=mingo@redhat.com \
    --cc=pmladek@suse.com \
    --cc=rostedt@goodmis.org \
    --cc=sergey.senozhatsky@gmail.com \
    /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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.