Page MenuHomeFreeBSD

LinuxKPI: Implement linux/hashtable.h for FreeBSD.
ClosedPublic

Authored by hselasky on Tue, May 10, 12:56 PM.

Details

Summary

This implementation uses the concurrency kit, CK, API directly which is
suitable for use with EPOCH(9) and RCU under FreeBSD.

No functional change intended.

The initial "linux/hash.h" code was obtained from DragonFlyBSD via FreeBSD's drm-kmod in ports.

MFC after: 1 week
Sponsored by: NVIDIA Networking

Test Plan

cd /usr/ports/graphics/*fbsd13
make extract
rm ./work/drm-kmod-drm_*/linuxkpi/gplv2/include/linux/hashtable.h
rm ./work/drm-kmod-drm_*/linuxkpi/bsd/include/linux/hash.h
make all

Diff Detail

Repository
R10 FreeBSD src repository
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.

Event Timeline

manu@ : Could you do a quick test with graphics drivers ?

sys/compat/linuxkpi/common/include/linux/hashtable.h
162
NOTE: This container_of() could have been skipped, if it wasn't for how Linux defined the hashtable API!

I'll give this a try tonight as well and have a closer look.
It's one of two missing pieces I need; in theory kfifo is the other one but I think that is used so little that I might simply change the code?

I'd probably would help to mention where the hash.h with a 2013 copyright was obtained from.

sys/compat/linuxkpi/common/include/linux/hash.h
2

Add an SPDX tag?

sys/compat/linuxkpi/common/include/linux/hashtable.h
2

Here too.

92

So hash_del_rcu() clears this pointer after removing the element, but what if the element was never inserted to begin with? In other words, how does a node get initialized?

sys/compat/linuxkpi/common/include/linux/hash.h
38

Presumably fine for our use cases but really we ought to have a hash_64 that returns up to 64 bits?

53

Not just amd64 for FreeBSD

sys/compat/linuxkpi/common/include/linux/hash.h
2

Yes, good point.

53

I'll update that tomorrow. Was so in the original I used from the DRM kmod.

sys/compat/linuxkpi/common/include/linux/hashtable.h
92

There is a hidden assumption in Linux that if this feature is used, that memory is allocated with kzalloc or M_ZERO .

Makes more code compile for me too. Thank you!

sys/compat/linuxkpi/common/include/linux/hashtable.h
92

Should we then KASSERT that on add it's NULL to better guard against (upstream not detected) broken code?

hselasky added inline comments.
sys/compat/linuxkpi/common/include/linux/hash.h
38

Fixed.

hselasky marked an inline comment as done.

Implement suggestions from bz@ and markj@ .

@manu: Where was "include/linux/hash.h" obtained from?

@manu: Where was "include/linux/hash.h" obtained from?

This predates my involvment with drm-kmod but looking at the header I'd say DragonFlyBSD.

I'd probably say "Dragonfly via drm-kmod" as you obtained it from drm-kmod?

This revision is now accepted and ready to land.Wed, May 11, 11:56 AM
In D35162#797274, @bz wrote:

I'd probably say "Dragonfly via drm-kmod" as you obtained it from drm-kmod?

Yes, that's correct.

Do we need to bump the FreeBSD version for this - no?

Do we need to bump the FreeBSD version for this - no?

I'll leave that to @manu on how to deal with possible conflicts or if it is a problem with drm-kmod ports/packages.

In D35162#797274, @bz wrote:

I'd probably say "Dragonfly via drm-kmod" as you obtained it from drm-kmod?

Yes, that's correct.

Do we need to bump the FreeBSD version for this - no?

It's not needed, drm-kmod will still use the version there and we can switch that whenever we want.

The loop macros look right to me after staring at them for a while, just some style nits.

sys/compat/linuxkpi/common/include/linux/hashtable.h
89

Style, lines are too long.

92

Looking at Linux, they do explicit zeroing in INIT_HLIST_NODE.

165

Lines are too long here as well.

hselasky added inline comments.
sys/compat/linuxkpi/common/include/linux/hashtable.h
92

Nice to know.

hselasky marked an inline comment as done.
hselasky marked an inline comment as done.

Address some long lines and improve some type asserts.

This revision now requires review to proceed.Thu, May 12, 2:35 PM
This revision is now accepted and ready to land.Thu, May 12, 2:36 PM