Messages in this thread | | | From | John Ogness <> | Subject | Re: [RFC PATCH v2 1/2] printk-rb: add a new printk ringbuffer implementation | Date | Tue, 25 Jun 2019 10:44:19 +0200 |
| |
On 2019-06-25, Sergey Senozhatsky <sergey.senozhatsky.work@gmail.com> wrote: >>>> + struct prb_reserved_entry e; >>>> + char *s; >>>> + >>>> + s = prb_reserve(&e, &rb, 32); >>>> + if (s) { >>>> + sprintf(s, "Hello, world!"); >>>> + prb_commit(&e); >>>> + } >>> >>> A nit: snprintf(). >>> >>> sprintf() is tricky, it may write "slightly more than was >>> anticipated" bytes - all those string_nocheck(" disabled"), >>> error_string("pK-error"), etc. >> >> Agreed. Documentation should show good examples. > > In vprintk_emit(), are we going to always reserve 1024-byte > records, since we don't know the size in advance, e.g. > > printk("%pS %s\n", regs->ip, current->name) > prb_reserve(&e, &rb, ????); > > or are we going to run vscnprintf() on a NULL buffer first, > then reserve the exactly required number of bytes and afterwards > vscnprintf(s) -> prb_commit(&e)?
(As suggested by Petr) I want to use vscnprintf() on a NULL buffer. However, a NULL buffer is not sufficient because things like the loglevel are sometimes added via %s (for example, in /dev/kmsg). So rather than a NULL buffer, I would use a small buffer on the stack (large enough to store loglevel/cont information). This way we can use vscnprintf() to get the exact size _and_ printk_get_level() will see enough of the formatted string to parse what it needs.
> I'm asking this because, well, if the most common usage > pattern (printk->prb_reserve) will always reserve fixed > size records (aka data blocks), then you _probably_ (??) > can drop the 'variable size records' requirement from prb > design and start looking at records (aka data blocks) as > fixed sized chunks of bytes, which are always located at > fixed offsets.
The average printk message size is well under 128 bytes. It would be quite wasteful to always reserve 1K blocks.
John Ogness
| |