Paths

Table of Contentst

Fix 13.x KBI for linuxkpi
ClosedPublic
Actions

Authored by imp on Jul 31 2021, 6:11 AM.

Details

Reviewers

• hselasky
bz
wulf

Group Reviewers

x11

Commits

rG0437d10e359e: direct commit: fix KBI for pci_dev

Summary

direct: fix KBI for pci_dev

Move all the new elemenets to the end of the structure for 13. We
allocate this inside the linuxkpi code, so the size isn't enccoded in
client modules. However, the offsets to the different fields are
encoded. Tihs modifies 04456f711853, 40a215e38a4d, and 3a606aadf2e7
and will likely create merge conflicts there (and that's a good thing
since the elements need to be moved to the end of the structure when
merging).

direct commit: tweak irq_ent to be binary compatible

Since this is inlined into the clients, all clients have to agree on the
irq_ent offsets.

Restore visibility to linux_kmem_cache_free_rcu

linux_kmem_cache_free_rcu was made static in 10235ad0567f, however
client drivers depended on calling it directly. Make it visible again
to restore the 13.0-Release KBI for linuxkpi.

Bump FreeBSD_version to 1300513 for restoration of 13.0 KBI

Since the last few commmits to linuxkpi changes the KBI (this time back
to 13.0 release to restore the status quo of a couple weeks ago), you'll
need to recompile everything that uses it. The plus side is that our
packages (built using 13.0) for drm-kmod 5.4 work again on -stable
systems.

Diff Detail

Repository

rS FreeBSD src repository - subversion

Lint

Lint Passed

Unit

No Test Coverage

Build Status

Buildable 40778
Build 37667: arc lint + arc unit

Event Timeline

imp created this revision.Jul 31 2021, 6:11 AM

Herald added subscribers: linuxkpi, emaste. · View Herald TranscriptJul 31 2021, 6:11 AM

imp requested review of this revision.Jul 31 2021, 6:11 AM

Harbormaster completed remote builds in B40772: Diff 93031.Jul 31 2021, 6:11 AM

This is several commits that I've uploaded as one to make it easier to review.

wulf added inline comments.Jul 31 2021, 9:51 AM

sys/compat/linuxkpi/common/src/linux_slab.c
148–149	`lkpi_kmem_cache_free_rcu` function should be renamed back to `linux_kmem_cache_free_rcu` and made visible. `linux_kmem_cache_free_rcu_callback` can be left intact. 10235ad0567f did not change it.

• hselasky added inline comments.Jul 31 2021, 1:11 PM

sys/compat/linuxkpi/common/include/linux/interrupt.h
57	Please lowercase this comment.

• hselasky accepted this revision.Jul 31 2021, 1:11 PM

This revision is now accepted and ready to land.Jul 31 2021, 1:11 PM

I haven't cared so much a binary compatibility for now. People who want to use the LinuxKPI need to rebuild their code from sources.

--HPS

Has anyone tested that 13.0-RELEASE drm-kmod binary package drivers work again after this commit commit?
I'll go and have a look at some other things after lunch but will get back to you in a few hours.

In D31363#706833, @hselasky wrote:

I haven't cared so much a binary compatibility for now. People who want to use the LinuxKPI need to rebuild their code from sources.

I think if we want to garantuee things we need to start thinking a lot harder and write down "rules" as otherwise it'll not work.

If it was me, I'd say build a module for each release on the release and distribute that for 3rd party (external) code and whoever uses stable needs to track source and build modules herself. But with out ports/pkg system that doesn't work either so easily. Winning this is really hard given we still break in-kernel (non-compat) KPIs in stable branches regularly.

sys/compat/linuxkpi/common/include/linux/interrupt.h
57	I am not sure this will work as the allocation is in request_irq() in 13.0-R and that is an inline function from the header file. That means the alloc in 13.0-R binary code will not allocate the full size but we might still (possibly) try to call thread_handler from the implementation file on a newer linuxkpi and that will fail accessing memory beyond the end of the struct unless no code from 13.0-R returns IRQ_WAKE_THREAD from the handler, which wasn't newly added. vmwgfx_irq.c from drm-kmod master seems to catch that case though itself, so we might be blessed enough.

I think you might be good to go with this @imp . I'd still love someone testing the 13.0-R package before the commit so we know it actually helps as if it doesn't this thing will keep hunting...

In D31363#706833, @hselasky wrote:

I haven't cared so much a binary compatibility for now. People who want to use the LinuxKPI need to rebuild their code from sources.

This causes a lot of grief for the drm-kmod folks, hence my attempt to make things compatible.

In D31363#706834, @bz wrote:

Has anyone tested that 13.0-RELEASE drm-kmod binary package drivers work again after this commit commit?

wulf@ tested this, which is why I made a run at getting it into the tree. We were compatible for 6 months until
the start of July.

In D31363#706863, @bz wrote:

In D31363#706833, @hselasky wrote:

I haven't cared so much a binary compatibility for now. People who want to use the LinuxKPI need to rebuild their code from sources.

I think if we want to garantuee things we need to start thinking a lot harder and write down "rules" as otherwise it'll not work.

Yes. We should, but we've been doing this in other parts of the kernel for a long time, and I just follow the same rules I follow there.

If it was me, I'd say build a module for each release on the release and distribute that for 3rd party (external) code and whoever uses stable needs to track source and build modules herself. But with out ports/pkg system that doesn't work either so easily. Winning this is really hard given we still break in-kernel (non-compat) KPIs in stable branches regularly.

Yes. That's also not a viable solution. We tried that through the 12.x release and it's been a disaster. We've kept other parts of the kernel with a stable ABI in the stable branches, so I thought I'd give this a shot. I even have a script to abstract all the .h files from the kernel so one can build a 13.1 version on a 13.0 (or 14.0) host, but that ran into some head winds so I set it aside before it was complete.

The real solution is to only distribute sources and force people that want to use this to compile everything to identically match their kernel, but that turns out to be a tricker thing to crack.

imp added inline comments.Jul 31 2021, 4:56 PM

sys/compat/linuxkpi/common/include/linux/interrupt.h
57	Yea, I think that's why it is 'working' for wulf. The linuxkpi heavy use of inlines do present problems in this area. In fact, this is the crux of the issue for why this won't work with 5.5 based drm-kmod: it needs to use this functionality, but that doesn't exist on 13.0's sources (among other things, and I know I'm glossing over).

Tweaks, as requested

This revision now requires review to proceed.Jul 31 2021, 4:57 PM

Harbormaster completed remote builds in B40778: Diff 93056.Jul 31 2021, 4:57 PM

I am fine with this.

This revision is now accepted and ready to land.Jul 31 2021, 6:58 PM

I tested it once again. After addition of missing prototype I am able to run XFCE on 13-STABLE with drm-fbsd13-kmod-5.4.92.g20210720_1 installed from pkg.FreeBSD.org packages.

sys/compat/linuxkpi/common/include/linux/slab.h

213

I have to place
void linux_kmem_cache_free_rcu(struct linux_kmem_cache *, void *);
line here to fix build.

/usr/src/sys/compat/linuxkpi/common/src/linux_slab.c:149:1: error: no previous prototype for function 'linux_kmem_cache_free_rcu' [-Werror,-Wmissing-prototypes]
linux_kmem_cache_free_rcu(struct linux_kmem_cache *c, void *m)
^
/usr/src/sys/compat/linuxkpi/common/src/linux_slab.c:148:1: note: declare 'static' if the function is not intended to be used outside of this translation unit
void
^
static 
1 error generated.

make[1]: stopped in /usr/src

make: stopped in /usr/src

Closed by commit rG0437d10e359e: direct commit: fix KBI for pci_dev (authored by imp). · Explain WhySep 22 2021, 3:01 PM

This revision was automatically updated to reflect the committed changes.

imp added a commit: rG0437d10e359e: direct commit: fix KBI for pci_dev.

rebased and committed to the tree... hope we're still compatible :)