Optimize spa_get_random().

[amotin]

In all places except two spa_get_random() is used for small values,
and the consumers do not need well seeded high quality values.
Switch those two exceptions directly to random_get_pseudo_bytes()
and optimize spa_get_random(), renaming it to random_in_range(),
since it is not related to SPA or ZFS in general.

On FreeBSD directly map random_in_range() to new prng32_bounded() KPI
added in FreeBSD 13. On Linux and in user-space just reduce the type
used to uint32_t to avoid more expensive 64bit division.

How Has This Been Tested?

On 40-thread FreeBSD system heavy uncached 4KB reads from 8 ZVOLs show reduction of time spent inside dbuf_evict_one() by ~10%. Previously it was consumed by ChaCha encryption, backing random_get_pseudo_bytes(), producing excessively strong random numbers for this purpose.

Types of changes

• Bug fix (non-breaking change which fixes an issue)
• New feature (non-breaking change which adds functionality)
• Performance enhancement (non-breaking change which improves efficiency)
• Code cleanup (non-breaking change which makes code smaller or more readable)
• Breaking change (fix or feature that would cause existing functionality to change)
• Library ABI change (libzfs, libzfs_core, libnvpair, libuutil and libzfsbootenv)
• Documentation (a change to man pages or other documentation)

Checklist:

• My code follows the OpenZFS code style requirements.
• I have updated the documentation accordingly.
• I have read the contributing document.
• I have added tests to cover my changes.
• I have run the ZFS Test Suite with this change applied.
• All commit messages are properly formatted and contain Signed-off-by.

[behlendorf]

Makes good sense to me.

[mmaybee]

I think it's a little awkward to move spa_get_random() into the os-specific random.h header file. Would you consider refactoring to introduce something like random_in_range() which, for freebsd would look like:

static inline uint32_t
random_in_range(uint32_t range)
{
#if  __FreeBSD_version >= 1300108
	return (prng32_bounded(range));
#else
	uint32_t r;

	(void) random_get_pseudo_bytes((void *)&r, sizeof (r));

	return (r % range);
#endif
}

and then spa_get_random could be left in spa_misc.c and look like:

uint32_t
spa_get_random(uint32_t range)
{
	ASSERT(range != 0);

	if (range == 1)
		return (0);

	return (random_in_range(range));
}

[amotin]

@mmaybee I suppose it is about "spa_" in the function name? Otherwise I am not sure why to split one trivial function with two.

[mmaybee]

Correct. spa_get_random() should be declared in spa.h, not in random.h

[amotin]

I think it should not be spa_* then. It has nothing to do with spa or ZFS at all. I'll think how to rename it.

[mmaybee]

I think that is a reasonable alternative.

[amotin]

I've renamed spa_get_random() to random_in_range().

[amotin]

Any more comments here?

[mmatuska]

@amotin shouldn't this be #if defined(_KERNEL) && __FreeBSD_version >= 1300108

[amotin]

I don't think this file is used in user-space. It has separate implementation in include/sys/zfs_context.h. Am I wrong?

[mmatuska]

I am currently doing the merge to main multiple things break. I will send you an e-mail.