linuxkpi: Fix the definition of BITS_PER_LONG
ClosedPublic
Actions

Authored by markj on Oct 16 2025, 1:14 PM.

Details

Reviewers

emaste
dumbbell
bz

Group Reviewers

linuxkpi
cheri

Commits

rGd779d823dafb: linuxkpi: Fix the definition of BITS_PER_LONG
rG3e9ad70a30d9: linuxkpi: Fix the definition of BITS_PER_LONG

Summary

Don't use LP64 to decide the value, as it gives the wrong result on
CHERI platforms. Just define it in terms of sizeof(long).

Diff Detail

Repository

rG FreeBSD src repository

Lint

Lint Not Applicable

Unit

Tests Not Applicable

Event Timeline

markj created this revision.Oct 16 2025, 1:14 PM

Herald added subscribers: emaste, imp. · View Herald TranscriptOct 16 2025, 1:14 PM

markj requested review of this revision.Oct 16 2025, 1:14 PM

Harbormaster completed remote builds in B67822: Diff 164287.Oct 16 2025, 1:14 PM

emaste accepted this revision.Oct 16 2025, 1:18 PM

This revision is now accepted and ready to land.Oct 16 2025, 1:18 PM

Note, there are quite a few other definitions of BITS_PER_LONG that have similar problems, but they're mostly in obsolete or unmaintained drivers. I'm not sure whether to sweep through and fix them too.

dumbbell accepted this revision.Oct 16 2025, 1:54 PM

I'm not sure whether to sweep through and fix them too.

I'd be inclined to just leave them unless they come up in a CHERI context.

Use SIZEOF_LONG instead. Fix up BITS_PER_LONG_LONG too.

This revision now requires review to proceed.Oct 16 2025, 3:50 PM

Harbormaster completed remote builds in B67828: Diff 164298.Oct 16 2025, 3:50 PM

This is fine too. We do have some existing cases using sizeof, e.g.

sys/dev/drm2/drm_atomic.h:#define       NB_BITS_PER_LONG                (sizeof(long) * NBBY)
sys/dev/axgbe/xgbe_osdep.h:#define      BITS_PER_LONG                   (sizeof(long) * CHAR_BIT)

This revision is now accepted and ready to land.Thu, Oct 16, 4:14 PM

bz added a subscriber: bz.Thu, Oct 16, 10:38 PM

bz added inline comments.

sys/compat/linuxkpi/common/include/linux/bitops.h
41	I have a rather bad feeling about CHAR_BITS but maybe that stems from the fact that I used to write code for a 7bbit system before. I think the general FreeBSD usage was * NBBY. The question is if we should fix that to be defined as well to CHAR_BITS then? If they start to disagree, we'll be in trouble.

Typo.

This revision now requires review to proceed.Thu, Oct 16, 10:49 PM

Harbormaster completed remote builds in B67840: Diff 164352.Thu, Oct 16, 10:49 PM

In D53135#1213843, @emaste wrote:
This is fine too. We do have some existing cases using sizeof, e.g.
sys/dev/drm2/drm_atomic.h:#define       NB_BITS_PER_LONG                (sizeof(long) * NBBY)
sys/dev/axgbe/xgbe_osdep.h:#define      BITS_PER_LONG                   (sizeof(long) * CHAR_BIT)

The point was made elsewhere that we might want the value to be an int rather than a size_t, which would have been the case with the old version.

sys/compat/linuxkpi/common/include/linux/bitops.h
41	I think it's safe to assume that CHAR_BIT (I made a typo before) and NBBY agree with each other. Probably NBBY should be defined in terms of the former, but that's not within the scope of this change.

bz accepted this revision.Thu, Oct 16, 11:00 PM

bz added inline comments.

sys/compat/linuxkpi/common/include/linux/bitops.h
41	We have one use of CHAR_BIT in a tree from a few days ago in sys/ ; more than 240 of NBBY (incl. in cddl); I am happy here either way and if NBBY is a FreeBSD-ism (I don't know) maybe we should clean it up. I just prefer to have one way and not multiple.