[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <YB5Bsyk4o3Mqr6Li@kroah.com>
Date: Sat, 6 Feb 2021 08:13:55 +0100
From: Greg Kroah-Hartman <gregkh@...uxfoundation.org>
To: Chris Down <chris@...isdown.name>
Cc: Steven Rostedt <rostedt@...dmis.org>,
Petr Mladek <pmladek@...e.com>, linux-kernel@...r.kernel.org,
Sergey Senozhatsky <sergey.senozhatsky@...il.com>,
John Ogness <john.ogness@...utronix.de>,
Johannes Weiner <hannes@...xchg.org>,
Andrew Morton <akpm@...ux-foundation.org>, kernel-team@...com,
Alexey Dobriyan <adobriyan@...il.com>,
Jason Baron <jbaron@...mai.com>,
Kees Cook <keescook@...omium.org>, linux-api@...r.kernel.org
Subject: Re: [PATCH] printk: Userspace format enumeration support
On Fri, Feb 05, 2021 at 10:45:19PM +0000, Chris Down wrote:
> Hi Steven,
>
> Steven Rostedt writes:
> > Interesting, because when I was looking at the original patch (looked at
> > the lore link before reading your reply), I thought to myself "this looks
> > exactly like what I did for trace_printk formats", which the above file is
> > where it is shown. I'm curious if this work was inspired by that?
>
> The double __builtin_constant_p() trick was suggested by Johannes based on
> prior art in trace_puts() just prior to patch submission. Other than that,
> it seems we came up with basically the same solution independently. :-)
>
> > > Anyway, there is something wrong at the moment. The output looks fine
> > > with cat. But "less" says that it is a binary format and the output
> > > is a bit messy:
> >
> > Hmm, that's usually the case when lseek gets messed up. Not sure how that
> > happened.
>
> It looks as intended to me -- none of the newlines, nulls, or other control
> sequences are escaped currently, since I didn't immediately see a reason to
> do that. If that's a blocker though, I'm happy to change it.
>
> > > $> less /proc/printk_formats
> > > "/proc/printk_formats" may be a binary file. See it anyway?
> > > vmlinux,^A3Warning: unable to open an initial console.
> > > ^@...inux,^A3Failed to execute %s (error %d)
> > > ^@...inux,^A6Kernel memory protection disabled.
> > > ^@...inux,^A3Starting init: %s exists but couldn't execute it (error %d)
> > >
> > >
> > > That is for now. I still have to think about it. And I am also curious
> > > about what others thing about this idea.
> > >
> >
> > I'm not against the idea. I don't think it belongs in /proc. Perhaps
> > debugfs is a better place to put it.
>
> Any location is fine with me, as long as it gets to userspace. How does
> <debugfs>/printk/formats or <debugfs>/printk/formats/<module> sound to you?
That's fine with me, but I'd like to see the patch with this in it first
before approving it :)
thanks,
greg k-h
Powered by blists - more mailing lists