Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
Date:   Fri, 5 Feb 2021 22:45:19 +0000
From:   Chris Down <chris@chrisdown.name>
To:     Steven Rostedt <rostedt@goodmis.org>
Cc:     Petr Mladek <pmladek@suse.com>, linux-kernel@vger.kernel.org,
        Sergey Senozhatsky <sergey.senozhatsky@gmail.com>,
        John Ogness <john.ogness@linutronix.de>,
        Johannes Weiner <hannes@cmpxchg.org>,
        Andrew Morton <akpm@linux-foundation.org>, kernel-team@fb.com,
        Alexey Dobriyan <adobriyan@gmail.com>,
        Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
        Jason Baron <jbaron@akamai.com>,
        Kees Cook <keescook@chromium.org>, linux-api@vger.kernel.org
Subject: Re: [PATCH] printk: Userspace format enumeration support
Message-ID: <YB3Kf896Zt9O+/Yh@chrisdown.name>
References: <YBwU0G+P0vb9wTwm@chrisdown.name>
 <YB11jybvFCb95S9e@alley>
 <20210205124748.4af2d406@gandalf.local.home>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii; format=flowed
Content-Disposition: inline
In-Reply-To: <20210205124748.4af2d406@gandalf.local.home>
User-Agent: Mutt/2.0.5 (da5e3282) (2021-01-21)
Precedence: bulk

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.
>> ^@vmlinux,^A3Failed to execute %s (error %d)
>> ^@vmlinux,^A6Kernel memory protection disabled.
>> ^@vmlinux,^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?

Thanks,

Chris