Re: [PATCH v7] printk: hash addresses printed with %p

[Jason A. Donenfeld]

On Thu, Oct 26, 2017 at 12:27 AM, Tobin C. Harding <me@xxxxxxxx> wrote:
> How good is unlikely()?

It places that branch way at the bottom of the function so that it's
less likely to pollute the icache.

> It doesn't _feel_ right adding a check on every call to printk just to
> check for a condition that was only true for the briefest time when the
> kernel booted. But if unlikely() is good then I guess it doesn't hurt.
>
> I'm leaning towards the option 1, but then all those text books I read
> are telling me to implement the simplest solution first then if we need
> to go faster implement the more complex solution.
>
> This is a pretty airy fairy discussion now, but if you have an opinion
> I'd love to hear it.

I don't think adding a single branch there really matters that much,
considering how many random other branches there are all over the
printk code. However, if you really want to optimize on the little
bits, and sensibly don't want to go with the overcomplex
workqueue-to-statickey thing, you could consider using a plain vanilla
`bool has_gotten_random_ptr_secret` instead of using the atomic_t. The
reason is that there's only ever one single writer, changing from a 0
to a 1. Basically the only thing doing the atomic_t got you was a
cache flush surrounding the read (and the write) so that assigning
has_gotten_random_ptr_secret=true would take effect _immediately_.
However, since you might not necessarily about that, going with a bool
instead will save you an expensive cache flush, while potentially
being a microsecond out of date the first time it's used. Seems like
an okay trade off to me. (That kind of cache latency, also, is a few
orders of magnitude better than using a work queue for the statickey
stuff.)