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

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

Details

Reviewers

manu
markj
bz
• hselasky

Group Reviewers

linuxkpi

Commits

rG2f382e4469ca: LinuxKPI: Implement linux/hashtable.h for FreeBSD.
rGf9e90c24737f: LinuxKPI: Implement linux/hashtable.h for FreeBSD.

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

rG FreeBSD src repository

Lint

Lint Skipped

Unit

Tests Skipped

Event Timeline

• hselasky created this revision.May 10 2022, 12:56 PM

Herald added subscribers: emaste, imp. · View Herald TranscriptMay 10 2022, 12:56 PM

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

• hselasky added inline comments.May 10 2022, 12:59 PM

sys/compat/linuxkpi/common/include/linux/hashtable.h
163	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.

markj added inline comments.May 10 2022, 1:25 PM

sys/compat/linuxkpi/common/include/linux/hash.h
3	Add an SPDX tag?
sys/compat/linuxkpi/common/include/linux/hashtable.h
3	Here too.
93	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?

emaste added inline comments.May 10 2022, 1:29 PM

sys/compat/linuxkpi/common/include/linux/hash.h
39	Presumably fine for our use cases but really we ought to have a hash_64 that returns up to 64 bits?
54	Not just amd64 for FreeBSD

• hselasky added inline comments.May 10 2022, 2:54 PM

sys/compat/linuxkpi/common/include/linux/hash.h
3	Yes, good point.
54	I'll update that tomorrow. Was so in the original I used from the DRM kmod.
sys/compat/linuxkpi/common/include/linux/hashtable.h
93	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
93	Should we then KASSERT that on add it's NULL to better guard against (upstream not detected) broken code?

• hselasky marked 6 inline comments as done.May 11 2022, 11:30 AM

• hselasky added inline comments.

sys/compat/linuxkpi/common/include/linux/hash.h
39	Fixed.

Implement suggestions from bz@ and markj@ .

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

In D35162#797271, @hselasky wrote:

@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.

• hselasky edited the summary of this revision. (Show Details)May 11 2022, 11:42 AM

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

bz accepted this revision as: bz.May 11 2022, 11:56 AM

This revision is now accepted and ready to land.May 11 2022, 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#797277, @hselasky wrote:

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?