LinuxKPI: rework struct timer_list (*function[_415])
Needs ReviewPublic
Actions

Authored by bz on Thu, Aug 13, 9:07 PM.

Details

Reviewers

jhb
kib
np

Group Reviewers

linuxkpi
Restricted Owners Package	(Owns No Changed Paths)

Summary

struct timer_list has a function pointer callback which changed
in Linux v4.15. Since f446b7cab4507 we have a union for both
versions of the function pointer with differently typed arguments.

I was about to cut the 415 version off when I hit the following:
error: incompatible function pointer types assigning to
'void (*)(unsigned long)' from 'void (struct timer_list *)'
[-Wincompatible-function-pointer-types]
only until I relaized how many semi-native drivers are making use
(or abusing) this functionality.

I went through and started to rework the header file to conditionally
compile one version or the other depending on LINUXKPI_VERSION as that
is what we have it for. With that all the semi-native drivers relying
on the pre-4.15 behaviour now have a LINUXKPI_VERSION of 41499 set
with the additional comment.

irdma is special in the regard that it compiles with OFED but is newer
and uses the new timer KPI already, thus we define
_WANT_LKPI_TIMER_CONTAINER_OF as well.

I highly suggest that maintainers of these drivers seek a different
solution (either updated LinuxKPI or fully native) as the LinuxKPI
is out of date since Jan 2018. Once all the drivers are updated
I intend to remove the 415 backward compat hacks.

Note: I am only for consistency using the macro in linux_compat.c,
but that will always compile to the latest version and should be
trated as such.

MFC after: 15 days

Diff Detail

Repository

rG FreeBSD src repository

Lint

Lint Passed

Unit

No Test Coverage

Build Status

Buildable 75578
Build 72461: arc lint + arc unit

Event Timeline

bz created this revision.Thu, Aug 13, 9:07 PM

Owners added a reviewer: Restricted Owners Package.Thu, Aug 13, 9:07 PM

Herald added subscribers: emaste, imp. · View Herald TranscriptThu, Aug 13, 9:07 PM

bz requested review of this revision.Thu, Aug 13, 9:07 PM

Harbormaster completed remote builds in B75578: Diff 184021.Thu, Aug 13, 9:07 PM

I have no idea who's responsible for mthca(4). I have another change for that so any contacts would be good.

kib added inline comments.Mon, Aug 17, 9:15 AM

sys/compat/linuxkpi/common/include/linux/timer.h
48	What about having two function pointers of corresponding types, e.g. `void (function)(struct timer_list )` and `void (*function_415)(unsigned long)` in the structure. Then, compiler will flag all places where incompatible assignment is performed, and since most (ab)users of the old interface are already heavily patched it would be just one more patch for them. Or just change to the new signature. 415 is too old IMO.

bz added inline comments.Mon, Aug 17, 1:19 PM

sys/compat/linuxkpi/common/include/linux/timer.h
48	Well (1) as I said `I highly suggest that maintainers of these drivers seek a different solution (either updated LinuxKPI or fully native) as the LinuxKPI is out of date since Jan 2018. Once all the drivers are updated I intend to remove the 415 backward compat hacks.` and (2) if the drivers get updated I never have to commit this. But someone had to point things out and put them all together :)

emaste added a project: linuxkpi.Mon, Aug 17, 4:57 PM

kib added inline comments.Wed, Aug 19, 1:15 AM

sys/compat/linuxkpi/common/include/linux/timer.h
48	So what do you want from the driver maintainers, for instance lets consider mlx5. There are two calls to setup_timer(), both are trivially convertible to the new KPI. What should I do there? I would gladly test a patch for mlx5 if you have it.

bz added inline comments.Wed, Aug 19, 1:51 AM

sys/compat/linuxkpi/common/include/linux/timer.h
48	Sorry, what should I write a patch for? I am not maintaining mlx5. If you know how to convert mlx5 in main as-is do it and it goes out of the list of changes here too. The newer LinuxKPI bits are there. If you need more of it be our guest and submit the (modern) LinuxKPI changes needed.

kib added inline comments.Wed, Aug 19, 2:17 AM

sys/compat/linuxkpi/common/include/linux/timer.h
48	This is the sweeping change, relatively trivial. Since you are pushing the KPI, it makes sense to just convert everything and be done with it. Anyway, see D58941