Re: [PATCH v4] printk: Userspace format enumeration support

2021-02-18 Thread Chris Down
Petr Mladek writes: On Thu 2021-02-18 12:41:39, Chris Down wrote: Petr Mladek writes: > > - See if it's safe to pass a printk_fmt_sec to seq_file instead of a module > > Also it might be needed to store the pointer to struct module. You mean, have a `struct module` entry for this? I somewhat su

Re: [PATCH v4] printk: Userspace format enumeration support

2021-02-18 Thread Petr Mladek
On Thu 2021-02-18 12:41:39, Chris Down wrote: > Petr Mladek writes: > > > - See if it's safe to pass a printk_fmt_sec to seq_file instead of a > > > module > > > > Also it might be needed to store the pointer to struct module. > > You mean, have a `struct module` entry for this? I somewhat suspe

Re: [PATCH v4] printk: Userspace format enumeration support

2021-02-18 Thread Chris Down
Thanks for all your feedback, Petr and Steven. :-) Petr, I believe this is a comprehensive checklist of everything we discussed for v5 -- any chance you could double check I'm not missing anything you folks wanted? Thanks! - Use seq_file iterator again instead of simple_open + size - Remove d

Re: [PATCH v4] printk: Userspace format enumeration support

2021-02-18 Thread Petr Mladek
On Thu 2021-02-18 12:21:55, Chris Down wrote: > Thanks for all your feedback, Petr and Steven. :-) > > Petr, I believe this is a comprehensive checklist of everything we discussed > for v5 -- any chance you could double check I'm not missing anything you > folks wanted? Thanks! > > - Use seq_file

Re: [PATCH v4] printk: Userspace format enumeration support

2021-02-18 Thread Chris Down
Petr Mladek writes: - Move to another file, kernel/printk/debug_formats.c or similar Just to be sure. The filename should be ideally based on the configure option and API names, e.g. formats_index.c or so. The printk_ prefix is not strictly necessary. The file is in printk/ directory. IMHO, we

Re: output: was: Re: [PATCH v4] printk: Userspace format enumeration support

2021-02-18 Thread Petr Mladek
On Wed 2021-02-17 15:28:24, Chris Down wrote: > Petr Mladek writes: > > I guess that you already use it internally and have its own tooling > > around it. > > Hmm, we're actually not using it yet widely because I don't want to backport > it to our prod kernel until we're reasonably agreed on the f

Re: code style: Re: [PATCH v4] printk: Userspace format enumeration support

2021-02-18 Thread Petr Mladek
On Wed 2021-02-17 15:56:38, Chris Down wrote: > Petr Mladek writes: > > > > How about config PRINTK_INDEX? > > > > > > Ah yes, I also like that. PRINTK_INDEX is fine from my perspective and is > > > more straightforward than "enumeration", thanks. > > > > It is better than enumeration. But there

Re: code style: Re: [PATCH v4] printk: Userspace format enumeration support

2021-02-18 Thread Petr Mladek
On Wed 2021-02-17 16:32:21, Chris Down wrote: > Chris Down writes: > > open(f); > > debugfs_file_get(f); > > fops->open(); > >inode->private = ps; > > debugfs_file_put(f); > > > > remove_printk_fmt_sec(); /* kfree ps */ > > > > read(f); > > debugfs_file_get(f); > > fops->read(); > >p

Re: output: was: Re: [PATCH v4] printk: Userspace format enumeration support

2021-02-17 Thread Chris Down
Steven Rostedt writes: OK, now do the same in C. "%q" "and I guess that "f" in the print statement in python (but I don't know for sure) does some magic with converting the "\n" and such. I agree with Petr on this. Print the format itself, and not what is converted. It's much easier to convert "

Re: output: was: Re: [PATCH v4] printk: Userspace format enumeration support

2021-02-17 Thread Steven Rostedt
On Wed, 17 Feb 2021 15:28:24 + Chris Down wrote: > Petr Mladek writes: > >I guess that you already use it internally and have its own tooling > >around it. > > Hmm, we're actually not using it yet widely because I don't want to backport > it > to our prod kernel until we're reasonably ag

Re: code style: Re: [PATCH v4] printk: Userspace format enumeration support

2021-02-17 Thread Chris Down
Chris Down writes: open(f); debugfs_file_get(f); fops->open(); inode->private = ps; debugfs_file_put(f); remove_printk_fmt_sec(); /* kfree ps */ read(f); debugfs_file_get(f); fops->read(); ps = inode->private; /* invalid */ debugfs_file_put(f); Er, sorry, inode->private is popula

Re: code style: Re: [PATCH v4] printk: Userspace format enumeration support

2021-02-17 Thread Chris Down
Petr Mladek writes: What about storing the pointer to struct pf_object into struct printk_fmt_sec *ps into the s->file->f_inode->i_private? Then we would not need any global list/table at all. Unless I'm misreading the debugfs code, I think the following is possible: open(f); debugfs_file_ge

Re: code style: Re: [PATCH v4] printk: Userspace format enumeration support

2021-02-17 Thread Petr Mladek
On Tue 2021-02-16 17:27:08, Chris Down wrote: > Petr Mladek writes: > > > +/* > > > + * Stores .printk_fmt section boundaries for vmlinux and all loaded > > > modules. > > > + * Add entries with store_printk_fmt_sec, remove entries with > > > + * remove_printk_fmt_sec. > > > + */ > > > +static DEF

Re: code style: Re: [PATCH v4] printk: Userspace format enumeration support

2021-02-17 Thread Johannes Weiner
On Wed, Feb 17, 2021 at 04:45:51PM +0100, Petr Mladek wrote: > On Tue 2021-02-16 21:05:48, Chris Down wrote: > > Johannes Weiner writes: > > > On Tue, Feb 16, 2021 at 05:27:08PM +, Chris Down wrote: > > > > Petr Mladek writes: > > > > > I wonder if we could find a better name for the configure

Re: code style: Re: [PATCH v4] printk: Userspace format enumeration support

2021-02-17 Thread Chris Down
Petr Mladek writes: > How about config PRINTK_INDEX? Ah yes, I also like that. PRINTK_INDEX is fine from my perspective and is more straightforward than "enumeration", thanks. It is better than enumeration. But there is still the same problem. The word "index" is used neither in the code nor i

Re: debugfs: was: Re: [PATCH v4] printk: Userspace format enumeration support

2021-02-17 Thread Chris Down
Petr Mladek writes: > > + debugfs_remove(ps->file); > > IMHO, we should remove the file before we remove the way how > to read it. This should be done in the opposite order > than in store_printk_fmt_sec(). There is a subtle issue with doing this as-is: debugfs_remove(ps->file) cannot be cal

Re: code style: Re: [PATCH v4] printk: Userspace format enumeration support

2021-02-17 Thread Petr Mladek
On Tue 2021-02-16 21:05:48, Chris Down wrote: > Johannes Weiner writes: > > On Tue, Feb 16, 2021 at 05:27:08PM +, Chris Down wrote: > > > Petr Mladek writes: > > > > I wonder if we could find a better name for the configure switch. > > > > I have troubles to imagine what printk enumeration migh

Re: debugfs: was: Re: [PATCH v4] printk: Userspace format enumeration support

2021-02-17 Thread Petr Mladek
On Tue 2021-02-16 17:18:58, Chris Down wrote: > Petr Mladek writes: > > > +static size_t printk_fmt_size(const char *fmt) > > > +{ > > > + size_t sz = strlen(fmt) + 1; > > > + > > > + /* > > > + * Some printk formats don't start with KERN_SOH + level. We will add > > > + * it later when rendering

Re: output: was: Re: [PATCH v4] printk: Userspace format enumeration support

2021-02-17 Thread Chris Down
Petr Mladek writes: I guess that you already use it internally and have its own tooling around it. Hmm, we're actually not using it yet widely because I don't want to backport it to our prod kernel until we're reasonably agreed on the format :-) My main concern is making sure that parsing is

Re: output: was: Re: [PATCH v4] printk: Userspace format enumeration support

2021-02-17 Thread Petr Mladek
On Tue 2021-02-16 16:52:39, Chris Down wrote: > Hey Petr, > > Petr Mladek writes: > > This produces something like: > > > > 3Warning: unable to open an initial console. > > 3Failed to execute %s (error %d) > > 6Kernel memory protection disabled. > > 3Starting init: %s exists but couldn't execute

Re: code style: Re: [PATCH v4] printk: Userspace format enumeration support

2021-02-16 Thread Chris Down
Johannes Weiner writes: On Tue, Feb 16, 2021 at 05:27:08PM +, Chris Down wrote: Petr Mladek writes: > I wonder if we could find a better name for the configure switch. > I have troubles to imagine what printk enumeration might mean. > Well, it might be because I am not a native speaker. > >

Re: code style: Re: [PATCH v4] printk: Userspace format enumeration support

2021-02-16 Thread Johannes Weiner
On Tue, Feb 16, 2021 at 05:27:08PM +, Chris Down wrote: > Petr Mladek writes: > > I wonder if we could find a better name for the configure switch. > > I have troubles to imagine what printk enumeration might mean. > > Well, it might be because I am not a native speaker. > > > > Anyway, the wo

Re: code style: Re: [PATCH v4] printk: Userspace format enumeration support

2021-02-16 Thread Chris Down
Petr Mladek writes: I wonder if we could find a better name for the configure switch. I have troubles to imagine what printk enumeration might mean. Well, it might be because I am not a native speaker. Anyway, the word "enumeration" is used only in the configure option. Everything else is "print

Re: debugfs: was: Re: [PATCH v4] printk: Userspace format enumeration support

2021-02-16 Thread Chris Down
Petr Mladek writes: +static size_t printk_fmt_size(const char *fmt) +{ + size_t sz = strlen(fmt) + 1; + + /* +* Some printk formats don't start with KERN_SOH + level. We will add +* it later when rendering the output. +*/ + if (unlikely(fmt[0] != KERN_SOH

code style: Re: [PATCH v4] printk: Userspace format enumeration support

2021-02-16 Thread Petr Mladek
Hi, this is from Nitpicker's department. On Fri 2021-02-12 15:30:16, Chris Down wrote: > We have a number of systems industry-wide that have a subset of their > functionality that works as follows: > > 1. Receive a message from local kmsg, serial console, or netconsole; > 2. Apply a set of rules

Re: output: was: Re: [PATCH v4] printk: Userspace format enumeration support

2021-02-16 Thread Chris Down
Hey Petr, Petr Mladek writes: This produces something like: 3Warning: unable to open an initial console. 3Failed to execute %s (error %d) 6Kernel memory protection disabled. 3Starting init: %s exists but couldn't execute it (error %d) 6Run %s as init process 7initcall %pS returned %d after %lld

debugfs: was: Re: [PATCH v4] printk: Userspace format enumeration support

2021-02-16 Thread Petr Mladek
On Fri 2021-02-12 15:30:16, Chris Down wrote: > We have a number of systems industry-wide that have a subset of their > functionality that works as follows: > > 1. Receive a message from local kmsg, serial console, or netconsole; > 2. Apply a set of rules to classify the message; > 3. Do something

output: was: Re: [PATCH v4] printk: Userspace format enumeration support

2021-02-16 Thread Petr Mladek
On Fri 2021-02-12 15:30:16, Chris Down wrote: > We have a number of systems industry-wide that have a subset of their > functionality that works as follows: > > 1. Receive a message from local kmsg, serial console, or netconsole; > 2. Apply a set of rules to classify the message; > 3. Do something

Re: [PATCH v4] printk: Userspace format enumeration support

2021-02-13 Thread Chris Down
Chris Down writes: kernel test robot writes: Hi Chris, Thank you for the patch! Yet something to improve: I'm pretty sure !CONFIG_PRINTK && CONFIG_IA64_DEBUG_CMPXCHG has been broken like this long before this change. I sent "ia64: Depend on non-static printk for cmpxchg debug"[0] to ia64 f

Re: [PATCH v4] printk: Userspace format enumeration support

2021-02-13 Thread Chris Down
kernel test robot writes: Hi Chris, Thank you for the patch! Yet something to improve: I'm pretty sure !CONFIG_PRINTK && CONFIG_IA64_DEBUG_CMPXCHG has been broken like this long before this change. With !CONFIG_PRINTK, printk() is static in the header, but ia64's cmpxchg.h with CONFIG_IA64

Re: [PATCH v4] printk: Userspace format enumeration support

2021-02-12 Thread kernel test robot
Hi Chris, Thank you for the patch! Yet something to improve: [auto build test ERROR on jeyu/modules-next] [also build test ERROR on linux/master soc/for-next openrisc/for-next powerpc/next uml/linux-next asm-generic/master linus/master v5.11-rc7 next-20210211] [cannot apply to tip/x86/core pmla

[PATCH v4] printk: Userspace format enumeration support

2021-02-12 Thread Chris Down
We have a number of systems industry-wide that have a subset of their functionality that works as follows: 1. Receive a message from local kmsg, serial console, or netconsole; 2. Apply a set of rules to classify the message; 3. Do something based on this classification (like scheduling a remedi