| Message ID | 3380804.uZWydldqLU@positron.chronox.de (mailing list archive) |
|---|---|
| State | Not Applicable |
| Delegated to: | Herbert Xu |
| Headers | show |
On Tue, Dec 27, 2016 at 11:40:23PM +0100, Stephan Müller wrote: > The variable ip is defined to be a __u64 which is always 8 bytes on any > architecture. Thus, the check for sizeof(ip) > 4 will always be true. > > As the check happens in a hot code path, remove the branch. The fact that it's a hot code path means the compiler will optimize it out, so the fact that it's on the hot code path is irrelevant. The main issue is that on platforms with a 32-bit IP's, ip >> 32 will always be zero. It might be that we can just do this via #if BITS_PER_LONG == 32 ... #else ... #endif I'm not sure that works for all platforms, though. More research is needed... - Ted -- To unsubscribe from this list: send the line "unsubscribe linux-crypto" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Hi Ted, (replying to an old thread) On Wed, Jan 18, 2017 at 5:35 AM, Theodore Ts'o <tytso@mit.edu> wrote: > On Tue, Dec 27, 2016 at 11:40:23PM +0100, Stephan Müller wrote: >> The variable ip is defined to be a __u64 which is always 8 bytes on any >> architecture. Thus, the check for sizeof(ip) > 4 will always be true. >> >> As the check happens in a hot code path, remove the branch. > > The fact that it's a hot code path means the compiler will optimize it > out, so the fact that it's on the hot code path is irrelevant. The > main issue is that on platforms with a 32-bit IP's, ip >> 32 will > always be zero. It might be that we can just do this via > > #if BITS_PER_LONG == 32 > ... > #else > ... > #endif > > I'm not sure that works for all platforms, though. More research is > needed... Is the intention for the test "sizeof(ip) > 4" to distinguish between 32-bit and 64-bit platforms? As ip is __u64, while regs is a pointer, shouldn't the test be "sizeof(regs) > 4"? Gr{oetje,eeting}s, Geert -- Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org In personal conversations with technical people, I call myself a hacker. But when I'm talking to journalists I just say "programmer" or something like that. -- Linus Torvalds
diff --git a/drivers/char/random.c b/drivers/char/random.c index 5c26b1c..8d4d720 100644 --- a/drivers/char/random.c +++ b/drivers/char/random.c @@ -1136,8 +1136,7 @@ void add_interrupt_randomness(int irq, int irq_flags) fast_pool->pool[1] ^= now ^ c_high; ip = regs ? instruction_pointer(regs) : _RET_IP_; fast_pool->pool[2] ^= ip; - fast_pool->pool[3] ^= (sizeof(ip) > 4) ? ip >> 32 : - get_reg(fast_pool, regs); + fast_pool->pool[3] ^= ip >> 32; fast_mix(fast_pool); add_interrupt_bench(cycles);
The variable ip is defined to be a __u64 which is always 8 bytes on any architecture. Thus, the check for sizeof(ip) > 4 will always be true. As the check happens in a hot code path, remove the branch. Signed-off-by: Stephan Mueller <smueller@chronox.de> --- drivers/char/random.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-)