log2: move log2 related functions from kpi to libkern

[dougm]

Summary

The linuxkpi log2.h file includes functions besides ilog2 that are generally useful. This change moves some of them into libkern.h.

[dougm]

sys/dev/drm2/drm_os_freebsd.h

236–237

Is this line needed? I don't know. None of my test builds seemed to care about the files I'm removing here.

sys/dev/qlnx/qlnxe/bcm_osal.h

104–105

Is this line needed? I don't know. None of the test builds I did seemed to care whether or not the functions below were defined.

sys/sys/libkern.h

39

Should this go before line 36? No, not if I want KASSERT to be defined.

sys/sys/log2.h

35 ↗

(On Diff #139650)

Why are these defined? Well, this fixes the failure of buildworld on i386. To be clear, it fails already. This is just added so that I can compete that buildworld. Also, it might let someone include log2.h without libkern.h.

[markj]

sys/dev/drm2/drm_os_freebsd.h

236–237

The one kernel config which uses this code is arm/conf/TEGRA124. Does your test build cover it?

sys/sys/log2.h

149 ↗

(On Diff #139650)

Aside from order_base_2(), these are aliases of rounddown2(), roundup2(), and powerof2(). Which versions are kernel developers supposed to use? These aliases are generally used in drivers which are ported from Linux; can we not keep them in the linuxkpi?

[dougm]

Move is_power_of_2 back to the linuxkpi version.

Restore the orignal 2013 behavior of the constant-handling code for invalid constant log parameters, which might stop coverity from complaining so much.

sys/dev/drm2/drm_os_freebsd.h

236–237

make buildkernel TARGET=arm KERNCONF=TEGRA124 completes successfully.

sys/sys/log2.h

149 ↗

(On Diff #139650)

I agree that is_power_of_two is the same as powerof2 in param.h, and that this definiton could be left in linuxkpi.

However rounddown_pow_of_two is not the same as rounddown2. They take a different number of arguments. One rounds down to the next power of two, while the other rounds down to the next multiple of a given power of two. Same thing for the up versions.

[dougm]

Change is_power_of_2 to powerof2 where the former is no longer defined.

[markj]

sys/sys/log2.h

149 ↗

(On Diff #139650)

If we keep them here, I'd prefer to use names that complement the existing param.h macros. It's not really ideal to have, e.g., roundup2() and roundup_pow_of_2() side-by-side. Drivers which use Linux interfaces, like mana, can pull in LinuxKPI includes if they need to.

I'm not sure what the names should be, but this patch also doesn't introduce any non-LinuxKPI consumers of these macros.

[dougm]

sys/sys/log2.h

149 ↗

(On Diff #139650)

There are three places where this patch removes definitions of roundup_pow_of_two so that this new, common definition can be used. Those three drivers would be non-LinuxKPI consumers of that macro.

This patch is a response to this @alc comment in D45494:
"I would add a new #define/inline with a self-documenting name. If Linux already has such a #define/inline, I would borrow its name."

It's not clear to me I can make both of you happy here.

[markj]

sys/sys/log2.h

149 ↗

(On Diff #139650)

I don't have any better names to suggest, so can't really argue against using the Linux names. :)

[alc]

I'm not convinced that we should be creating an ilog2.h header file. I would leave the definitions in the existing libkern.h header file.

sys/sys/log2.h

149 ↗

(On Diff #139650)

I think that rounddown_powerof2() would be more consistent with param.h. However, in the end, I also think that being consistent with Linux will probably save time for more people. Thoughts?

[dougm]

Drop sys/log2.h. Move its functions back to libkern.h.

[markj]

sys/sys/log2.h

149 ↗

(On Diff #139650)

If someone is familiar with Linux, they can quickly look up the LinuxKPI definition and see the "native" name for it (presumably we'd have #define roundup_pow_of_two roundup_powerof2 or similar), so it's not obvious to me that they'd waste a lot of time.

I'm not doing the work here so don't want to argue very much. I'll just say that I tend to find code easier to read if it's consistently following one of native FreeBSD style or Linux style, and code which mixes the two is a bit jarring. This is just about a couple of macros, though, so either approach is fine with me.

[dougm]

Apply the linux function names to the changes made in f0a0420dfd36ae90f86c

[dougm]

Use order_base_2 in place of fls in the code modified in 9ff1462976fce4f4389be9a3357eadd22d04d30