Messages in this thread | | | Date | Mon, 1 Aug 2022 16:41:34 +0200 | From | "Jason A. Donenfeld" <> | Subject | Re: [PATCH] random: use raw spinlocks for use on RT |
| |
Hi Sebastian,
On Mon, Aug 01, 2022 at 04:34:13PM +0200, Sebastian Andrzej Siewior wrote: > So I have everything ready for 5.20 (6.0) ready without the RT patch and > then this vsprintf issues comes along…
I already sent my rc1 pull to Linus for 6.0, but I can do a second pull with this in it for next week while the merge window is still open, so that your RT-patch-len-zero objective is still met for 6.0.
> From that point of view I would prefer to either init it upfront in a > way that works for everyone/ loose the first %p since it is probably a > minor inconvenience if nobody complains - instead swapping all locks. > We managed without this for kasan and lockdep which are both not used in > a production environment.
The kfence change was a production change, actually. Lots of people turn that on by default.
If you want to address this within printk itself, just do `if (rt || lockdep)` as the condition, so we don't swallow the first one. When you have to make code worse to satisfy a tool, the tool is the problem. We only would need this first message dropping on rt, not on other kernels. Don't knock other kernels.
However... I suspect these issues will continue to bite us in new subtle ways for some time to come. Who is to say that you can't call get_random_bytes() from a driver's hard IRQ? As RT gets integrated and more widely deployed, I imagine these things will start coming up. random.c was already designed to handle random bytes in irqoff; that's why it uses irqsave/irqrestore all over its spinlock handling. This RT thing is a snag in that original intention. But its an intention trivial to recover with this patch. So if you're okay with it, I think I'd prefer to do this and have our problems go away once and for all.
> I would need to do worst-case measurements and I've been looking at this > just before writting the other email and there was a local_lock_t > somewhere which needs also change…
That would be very interesting to learn about. If your measurements say yes, then maybe we can do this. If your measurements say "yikes", then I guess we can't. Either way, I like having some metric to decide this by.
Jason
| |