get_random_int() is gone

[solardiz]

As seen in our mkosi-mainline builds starting today, with kernel 6.1.0-060100rc1daily20221017-generic:

/root/src/src/modules/notifiers/p_notifiers.c: In function 'p_freq_transition_notifier':
/root/src/src/modules/notifiers/../../modules/notifiers/p_notifiers.h:39:28: error: implicit declaration of function 'get_random_int'; did you mean 'get_random_long'? [-Werror=implicit-function-declaration]
   39 | #define P_CHECK_RANDOM(x) (get_random_int() <= x)
      |                            ^~~~~~~~~~~~~~
/root/src/src/modules/notifiers/../../modules/notifiers/p_notifiers.h:43:33: note: in expansion of macro 'P_CHECK_RANDOM'
   43 |    if (rate == P_ALWAYS_RATE || P_CHECK_RANDOM(rate)) {                    \
      |                                 ^~~~~~~~~~~~~~
/root/src/src/modules/notifiers/p_notifiers.c:153:4: note: in expansion of macro 'P_TRY_OFFLOAD_NOTIFIER'
  153 |    P_TRY_OFFLOAD_NOTIFIER(P_OFTEN_RATE, "p_freq_transition_notifier");
      |    ^~~~~~~~~~~~~~~~~~~~~~

[solardiz]

So @zx2c4 deprecated get_random_int in favor of get_random_u32 in https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=de492c83cae0af72de370b9404aacda93dafcad5

One problem is this breaks out-of-tree uses. It would have been nice to to have to make another release of LKRG just to support Linux 6.1+'s dropping of this function. (We don't yet know whether we'll have enough other reasons to release before or at the same time that Linux 6.1 is released.)

Another minor problem is get_random_u32 isn't in the oldest kernels we still support (3.10+).

One possible fix would be to add conditional compilation checking for 6.1+ here. Another possibility is to switch to using (u32)get_random_long(), since we already have a dependency on and a compatibility wrapper for get_random_long on the oldest kernels - but this is a bit wasteful at runtime, compared to continuing to use get_random_int in this place on those kernels.

[zx2c4]

One problem is this breaks out-of-tree uses.

As a very strong rule, upstream never ever keeps APIs around for out of tree modules. This isn't my rule.

The correct thing to do is use an ifdef. However, get_random_u32() has been around for a very long time and is in all the stable kernels supported by kernel.org.

[zx2c4]

#define P_ALWAYS_RATE             4294967295U   /*  100%     */
#define P_SUPER_RARE_RATE         2147483647    /*   50%     */
#define P_RARE_RATE               429496729     /*   10%     */
#define P_LESS_RARE_RATE          214748364     /*    5%     */
#define P_OFTEN_RATE              42949672      /*    1%     */
#define P_MORE_OFTEN_RATE         21474836      /*    0.5%   */
#define P_M_MORE_OFTEN_RATE       4294967       /*    0.1%   */
#define P_S_MORE_OFTEN_RATE       2147483       /*    0.05%  */
#define P_SS_MORE_OFTEN_RATE      429496        /*    0.01%  */
#define P_M_SS_MORE_OFTEN_RATE    21474         /*    0.005% */
#define P_S_SS_MORE_OFTEN_RATE    42949         /*    0.001% */

#define P_CHECK_RANDOM(x) (get_random_int() <= x)

#define P_TRY_OFFLOAD_NOTIFIER(rate, where)                                \
do {                                                                       \
   if (rate == P_ALWAYS_RATE || P_CHECK_RANDOM(rate)) {                    \
      p_debug_log(P_LOG_DEBUG, "%s: Offloading integrity check", where); \
      p_offload_work(0);                                                   \
   }                                                                       \
} while(0)

Something seems off here, by the way. P_M_SS_MORE_OFTEN_RATE is less than P_S_SS_MORE_OFTEN_RATE. Should that be 214748 instead?

[zx2c4]

Also, get_random_u16() and then scaling all those constants would be even more efficient. Or if you don't care about some loss of precision get_random_u8() might even work. You can also refactor this exercise in terms of maximums. For example:

100%: prandom_u32_max(1) == 0 50%: prandom_u32_max(2) == 0

For powers of 2, prandom_u32_max() is uniformly distributed. For non-powers of two, there'll be some modulo bias. In 6.2, I'll be replacing prandom_u32_max() with get_random_u32_below(), which will always give uniform distribution.

[solardiz]

Thank you, @zx2c4. P_M_SS_MORE_OFTEN_RATE is indeed inconsistent with the comment, and longer-term we probably need to clean this up - those names are too cryptic.

[zx2c4]

#define P_RARE_RATE               429496729     /*   10%     */
#define P_LESS_RARE_RATE          214748364     /*    5%     */
#define P_OFTEN_RATE              42949672      /*    1%     */
#define P_MORE_OFTEN_RATE         21474836      /*    0.5%   */

Those names also just aren't right. "less rare" is more rare than "rare". And "more often" is less often than "often". And "often" is more often than "rare". I think some circuits might have gotten crossed at some point.

[solardiz]

The correct thing to do is use an ifdef.

@zx2c4 Do you think the dropping of get_random_int will be backported to any stable/longterm kernels? Looks like the 4.9.x branch is still maintained, yet 4.9 didn't have get_random_u32 yet. Edit: and ditto for 4.14.x.

Anyway, any suggestion for the cut-off for the #if by kernel version? We could do 6.1+ or we could do e.g. 5.0+ or we could find out the precise version when get_random_u32 was introduced.

I think there's nothing we could literally #ifdef for - no "feature macro", and these symbols themselves aren't macros. Or is there?

And while we're at it, is get_random_long to stay or is it going away soon?

[zx2c4]

You are not correct. get_random_u32() is in current 4.9.y stable kernels, since v4.9.320. It was added to mainline in 4.11.

I don't think the get_random_int() removal will be backported.

get_random_long() is still useful and in active use. I don't anticipate it going away.

You can probably get away with:

#if LINUX_VERSION_CODE < KERNEL_VERSION(4, 11, 0)
#include <linux/random.h>
#define get_random_u32() get_random_int()
#endif

Note, however, that on really ancient kernels, the implementation of get_random_int() was utter garbage -- something amounting to an MD5 of jiffies+rdtsc+pid if I recall the horror correctly.

[solardiz]

Oh right, here's what it was:

/*
 * Get a random word for internal kernel use only. Similar to urandom but
 * with the goal of minimal entropy pool depletion. As a result, the random
 * value is not cryptographically secure but for several uses the cost of
 * depleting entropy is too high
 */
static DEFINE_PER_CPU(__u32 [MD5_DIGEST_WORDS], get_random_int_hash);
unsigned int get_random_int(void)
{
        __u32 *hash;
        unsigned int ret;

        if (arch_get_random_int(&ret))
                return ret;

        hash = get_cpu_var(get_random_int_hash);

        hash[0] += current->pid + jiffies + random_get_entropy();
        md5_transform(hash, random_int_secret);
        ret = hash[0];
        put_cpu_var(get_random_int_hash);

        return ret;
}
EXPORT_SYMBOL(get_random_int);

[zx2c4]

[solardiz]

@zx2c4 Sorry if we hurt your eyes.