linuxkpi v5.0 updates

[johalun]

Summary

First I was considering splitting it up in smaller chunks for the reviews but that would be at least 7 different reviews. I put it all here for starters. If needed I can create smaller reviews before committing. I intend to commit in smaller chunks.

Included changes (roughly the chunks I intend to commit in):
cdev: Make modules unload without crash or memory leak. Works for drm.ko so far but graphic drivers still leak memory and doesn't clean up properly at unload.
timers: Update del_timer() according to upstream changes.
[committed] list: Add a couple of list functions.
[committed] sysinfo: Add linux sysinfo struct and helper method to populate it. NOTE: not sure what to put in si->totalhigh here...
tasklet: Use counter for enable/disable to match upstream. This requires moving the state out of the queue member.
[committed] pci: drm_pci functions has moved to kernel internals.
other: Various other minor updates.

This patch includes code and suggested improvements from https://reviews.freebsd.org/D18041 and will make D18041 obsolete.

Test Plan

Can be tested with drm drivers here https://github.com/FreeBSDDesktop/kms-drm/tree/drm-v5.0

[• hselasky]

I'll have a look at these patches tomorrow. Most of them look trivial.

[• hselasky]

Hi,

Did you build LINT with these changes and nothing breaks?

--HPS

[johalun]

Did you build LINT with these changes and nothing breaks?

No. What is the correct procedure to do this?

[• hselasky]

make -C sys/amd64/conf LINT
make KERNCONF=LINT -jX buildkernel

[• hselasky]

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

147 ↗

(On Diff #54998)

This patch is not correct. I get a double free during module unload "mlx4ib" and "mlx5ib" with this patch. I suspect there is an FD still hanging around that you need to close. Can you give some more background on this one?

[• hselasky]

Hi Johan,

Can you rebase your patchset on top of r345109?

--HPS

[• hselasky]

You need to explain why nanouptime() is replaced by nanotime().

[johalun]

In D19565#418990, @hselasky wrote:

You need to explain why nanouptime() is replaced by nanotime().

I think this had me confused https://github.com/freebsd/freebsd/blame/master/sys/compat/linuxkpi/common/include/linux/ktime.h#L232 (ktime_get_raw and ktime_get_raw_ns differ)
It probably should be uptime for all that's not *_real*.
As for ktime_get_raw_ts(), I will remove that since not used anywhere.

[johalun]

• Rebase with upstream r345127.
• Clean up ktime.h
• Patch two drivers that are affected by change to access_ok()

Left to do:

• cdev related: Investigate why mlx crash on unload with double free - perhaps fixing drm kldunload requires changes to drm/linuxkpi_gplv2 instead of lkpi_common?
• pci: Make the code use native KPI instead of LinuxKPI - probably best in a separate review since it's already committed.

[bwidawsk]

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

95 ↗

(On Diff #55055)

This should go in linux/mm_types.h

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

38

I don't think td_critnest is quite right here, but I can't figure out anything obviously better. I think you might be able to do something with td_priority >= PI_SOFT or some such, but I don't know.

[johalun]

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

38

Yeah, I'm not sure at all about this one so any feedback is welcome...

[johalun]

Revert cdev patch that caused double free for mlx* driver.
Added: Don't call cdev_init() when cdev_alloc() is called. This fixes memory leaks that occur when unloading drm.

[johalun]

Add linux_pci_unregister_drm_driver()

[• hselasky]

sys/compat/linuxkpi/common/include/asm/uaccess.h

55 ↗

(On Diff #55166)

Can you give an example where user_access_begin() is already defined?

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

64 ↗

(On Diff #55166)

You will need to bump the __FreeBSD_version when changing this structure.

sys/compat/linuxkpi/common/include/linux/dma-mapping.h

179 ↗

(On Diff #55166)

Commit style changes separately.

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

192

Spelling:

impl->implementation :
Our implementation is different. Avoid same name as Linux.

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

63

Can you given an example where access_ok is already defined?

sys/compat/linuxkpi/common/src/linux_tasklet.c

72

struct tasklet_struct *ts;
struct tasklet_struct *last;

77

last = TAILQ_LAST(&tw->head);

98

if (ts == last)
    break;

98

Your assumption that TAILQ_FIRST() will always go into the TAILQ_INSERT_TAIL() case does not hold. Please update the code like suggested.

153

atomic_set(&ts->_state, TASKLET_ST_IDLE);

221

atomic already has a barrier. I think this one can be removed.

237

Ditto.

[johalun]

sys/compat/linuxkpi/common/include/asm/uaccess.h

55 ↗

(On Diff #55166)

In drm 4.16 code. linuxkpi_gplv2 headers are included before base.

https://github.com/FreeBSDDesktop/kms-drm/blob/drm-v4.16-fbsd13.0/linuxkpi/gplv2/include/linux/compiler.h#L41

What we actually could do is putting back the LINUXKPI_VERSION macro and define all versions here... This solution was also recommended by Warner.

#define LINUXKPI_VERSION XXX   <- users declare this

#if LINUXKPI_VERSION < 5000 (or however many zeros we want)
#define user_access_begin()
#else 
#define user_access_begin(ptr, len) access_ok(ptr, len)
#endif

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

64 ↗

(On Diff #55166)

Yes we intend to bump the version anyways.

sys/compat/linuxkpi/common/include/linux/dma-mapping.h

179 ↗

(On Diff #55166)

Ok. Will do when committing.

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

192

Ok. I shortened it to avoid exceeding 80 chars.

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

63

See user_access_begin above

sys/compat/linuxkpi/common/src/linux_tasklet.c

98

Oh yes, I see now what you mean.

[johalun]

Implement recommended tasklet changes.

[johalun]

Use LINUXKPI_VERSION macro to check what KPI version the client expects, as per discussion. Any code that uses LinuxKPI will define this. We will support at most 2 versions concurrently and it is recommended that that any code that depends on LINUXKPI_VERSION < 50000 is updated to most recent version.

The version is defined as XXXYY, where XXX is Linux version (for example 4.16 = 416, 5.0 = 500) and YY is for our minor version.

[johalun]

Rebase

[• hselasky]

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

38 ↗

(On Diff #56626)

Please use function macro.

[• hselasky]

Hi, Can you commit the register chardev fixes first and rebase?

[johalun]

Re-open.

[johalun]

Rebase r346712

[johalun]

Rebase after committing r347470

[johalun]

Check LINUXKPI_VERSION before declaring IS_ALIGNED (declared in gplv2 code for < 5.0) macro to avoid build error with older out of tree code.
Use function macro for get_random_u32 as suggested by reviewer.

[johalun]

Rebase at r347620

[johalun]

Rebase r347628

[johalun]

Clean up diff

[johalun]

Everything committed!