Re: [PATCH printk v1] printk: fix illegal pbufs access for !CONFIG_PRINTK

From: Sergey Senozhatsky
Date: Thu Sep 21 2023 - 13:49:17 EST


On (23/09/21 09:19), John Ogness wrote:
> On 2023-09-21, Sergey Senozhatsky <senozhatsky@xxxxxxxxxxxx> wrote:
> > I wonder if anyone really use !PRINTK kernels. Can we get rid
> > of CONFIG_PRINTK?
>
> It is used. It is one of the big annoyances during the last several
> years of the rework. I get bug reports relatively quickly after breaking
> !CONFIG_PRINTK. The reports come mostly from the kbuild robots, but also
> from real people.

Right, that has happened to almost everyone who has ever submitted
patches to printk, even dramatically simpler than yours and Petr's
patches (e.g. my printk patches).

> If someone has limited space/memory requirements and does not care about
> dmesg, they can save a considerable amount of kernel size and memory by
> turning all that off. The problem right now is that !CONFIG_PRINTNK is
> horribly hacked together with dummy implementations and useless real
> functions that pretend to do stuff.

I was somehow thinking that prb is the biggest memory consumer on such
systems, but now that I looked at it, even on my very trimmed .config
the difference between PRINTK and !PRINTK is pretty huge:

./scripts/bloat-o-meter vmlinux.o.printk vmlinux.o.noprintk
add/remove: 79/643 grow/shrink: 235/3169 up/down: 30532/-1488150 (-1457618)
...
Total: Before=31118934, After=29661316, chg -4.68%

And !PRINTK doesn't even completely eliminate printk-s footprint.
All those temp buffers that are used for sprintf/printk are still
there. For example:

void mem_cgroup_print_oom_meminfo(struct mem_cgroup *memcg)
{
/* Use static buffer, for the caller is holding oom_lock. */
static char buf[PAGE_SIZE];
struct seq_buf s;

...

memory_stat_format(memcg, &s);
seq_buf_do_printk(&s, KERN_INFO);
}

in !PRINTK builds seq_buf_do_printk() doesn't do anything useful, yet
the buffer (and the code that fills that buffer) is (are) still there.

> After the rework we can work on splitting out the code based on
> functionality. If done right, it will be trivial to "implement"
> !CONFIG_PRINTK in such a way that changes to real code don't explode
> every time on !CONFIG_PRINTK.

Sounds good.
And sorry for the noise.