Page MenuHomeFreeBSD

linuxkpi: `GFP_KERNEL` equals `M_NOWAIT` now instead of `M_WAITOK`
ClosedPublic

Authored by dumbbell on Oct 2 2023, 10:15 PM.
Tags
None
Referenced Files
Unknown Object (File)
Sat, May 4, 10:40 AM
Unknown Object (File)
Sat, Apr 27, 1:40 AM
Unknown Object (File)
Fri, Apr 26, 4:54 PM
Unknown Object (File)
Fri, Apr 26, 4:54 PM
Unknown Object (File)
Fri, Apr 26, 4:54 PM
Unknown Object (File)
Fri, Apr 26, 3:22 PM
Unknown Object (File)
Fri, Apr 26, 11:57 AM
Unknown Object (File)
Sun, Apr 21, 1:58 AM

Details

Summary

Why

The reason is that in some places in the DRM drivers (in particular, the framebuffer management code), kmalloc() is called from a non-sleepable context, such as after a call to mtx_lock(8) with an MTX_DEF mutex.

If GFP_KERNEL is defined as M_WAITOK, we hit an assertion from witness(4).

How

The definition of GFP_KERNEL is changed to M_NOWAIT. This means that callers should verify the return value of kmalloc(). Fortunately, this is always the case in Linux.

This is required by the proposed new DRM/vt(4) integration (https://github.com/freebsd/drm-kmod/pull/243).

Diff Detail

Repository
rG FreeBSD src repository
Lint
Lint Not Applicable
Unit
Tests Not Applicable

Event Timeline

bz requested changes to this revision.Oct 2 2023, 10:31 PM
bz added a subscriber: bz.

There are other users which do NOT check results in LinuxKPI.
vmmap_add() is at least one of them; I would make sure there's a full audit and would then explicitly mention that case in the proposed commit message as well.

That said I have no clue if this really is a good idea or if the quoted code really requires this or, if FreeBSD code, could be fixed.

This revision now requires changes to proceed.Oct 2 2023, 10:31 PM
sys/compat/linuxkpi/common/src/linux_shmemfs.c
51–54

Should we just delete this instead? Or do we need a note about a potential future need to handle GFP_NOWAIT differently from GFP_KERNEL?

Update patch to:

  • Add "ret == NULL" checks in a few places where GFP_KERNEL is used to handle allocation failures.
  • Use M_WAITOK explicitly instead of GFP_KERNEL in places where we can't really check the return value or we are not sure it is checked by the caller.

This should address @bz's concerns.

dumbbell added inline comments.
sys/compat/linuxkpi/common/src/linux_shmemfs.c
51–54

I followed your advice by deleting the previous check and adding a comment to describe a bit the history.

To tell a bit more about the updated patch: I tracked all uses of GFP_* and verified that the return value was verified. In a few places where the code can't return an error, I replaced GFP_KERNEL with M_WAITOK to make it explicit. vmmap_add() is an example. Another one is the vmalloc*() macros: some of their uses don't check for failures on purpose.

This revision was not accepted when it landed; it landed in state Needs Review.Nov 24 2023, 5:32 PM
This revision was automatically updated to reflect the committed changes.