Re: [PATCH 26/27] random: factor out a __limit_random_u32_below helper
On Wed, Mar 11, 2026 at 08:03:58AM +0100, Christoph Hellwig wrote: > Factor out the guts of __get_random_u32_below into a new helper, > so that callers with their own prng state can reuse this code. What Eric said. random.c is not "some library code" meant to be pulled apart like this. If you think there are some good general purpose arithmetic functions, by all means develop shared infrastructure in the right place. But I think for this super simple/trivial _below function, you can probably just place it additionally where you're using it, without needing to touch random.c.
Re: [PATCH 26/27] random: factor out a __limit_random_u32_below helper
On Wed, 11 Mar 2026 15:29:35 -0700 Eric Biggers wrote: > On Wed, Mar 11, 2026 at 08:03:58AM +0100, Christoph Hellwig wrote: > > Factor out the guts of __get_random_u32_below into a new helper, > > so that callers with their own prng state can reuse this code. > > > > Signed-off-by: Christoph Hellwig > > I think I'd prefer that the test just uses the mod operation instead, > like many of the existing tests do: > > prandom_u32_state(&rng) % ceil Or possibly what the old code used: (prandom_u32_state(&rnd) * (u64)ceil) >> 32 Which distributes the values evenly across the range although some values happen 1 more time than others. I suspect that is good enough for a lot of the users of the cryptographic random number generator as well. David > > Yes, when ceil isn't a power of 2 the result isn't uniformly > distributed. But that's perfectly fine for these tests, especially with > the values of ceil being used being far smaller than U32_MAX. > > There's been an effort to keep the cryptographic random number generator > (drivers/char/random.c and include/linux/random.h) separate from the > non-cryptographic random number generator (lib/random32.c and > include/linux/prandom.h). This patch feels like it's going in a > slightly wrong direction, where random.c gains a function that's used > with both cryptographic and non-cryptographic random numbers. > > And if someone actually needs a fully unform distribution, then they'd > probably want cryptographic random numbers as well. > > So I'm not sure the proposed combination of "fully uniform > non-cryptographic random numbers" makes much sense. > > Plus the '% ceil' implementation is much easier to understand. > > - Eric >
Re: [PATCH 26/27] random: factor out a __limit_random_u32_below helper
On Wed, Mar 11, 2026 at 08:03:58AM +0100, Christoph Hellwig wrote: > Factor out the guts of __get_random_u32_below into a new helper, > so that callers with their own prng state can reuse this code. > > Signed-off-by: Christoph Hellwig I think I'd prefer that the test just uses the mod operation instead, like many of the existing tests do: prandom_u32_state(&rng) % ceil Yes, when ceil isn't a power of 2 the result isn't uniformly distributed. But that's perfectly fine for these tests, especially with the values of ceil being used being far smaller than U32_MAX. There's been an effort to keep the cryptographic random number generator (drivers/char/random.c and include/linux/random.h) separate from the non-cryptographic random number generator (lib/random32.c and include/linux/prandom.h). This patch feels like it's going in a slightly wrong direction, where random.c gains a function that's used with both cryptographic and non-cryptographic random numbers. And if someone actually needs a fully unform distribution, then they'd probably want cryptographic random numbers as well. So I'm not sure the proposed combination of "fully uniform non-cryptographic random numbers" makes much sense. Plus the '% ceil' implementation is much easier to understand. - Eric
