Page MenuHomeFreeBSD

Fix new users of MAXPHYS and hide it from the kernel namespace
ClosedPublic

Authored by gallatin on Apr 28 2024, 1:54 AM.
Tags
None
Referenced Files
Unknown Object (File)
Wed, Nov 6, 2:03 PM
Unknown Object (File)
Wed, Nov 6, 2:03 PM
Unknown Object (File)
Oct 10 2024, 7:19 AM
Unknown Object (File)
Oct 10 2024, 7:19 AM
Unknown Object (File)
Oct 10 2024, 7:19 AM
Unknown Object (File)
Oct 10 2024, 7:18 AM
Unknown Object (File)
Oct 10 2024, 7:18 AM
Unknown Object (File)
Oct 10 2024, 6:47 AM
Subscribers

Details

Summary

In cd8537910406, @kib made maxphys a load-time tunable. The #define MAXPHYS in sys/param.h became almost entirely obsolete, as it could now be overridden by kern.maxphys at boot time, or by opt_maxphys.h. After this change, MAXPHYS really only had meaning in subr_param.c, where it is used to initialize maxphys.

However, decades of tradition have led to several new, incorrect, uses of MAXPHYS in other parts of the kernel, mostly by seasoned developers. I've corrected those uses here in a mechanical fashion, and verified that it fixes a bug in the md driver that I was experiencing.

Since using MAXPHYS is such an easy mistake to make, it is best to hide it from the kernel namespace. So I've moved its definition to subr_param.c. Note that lots of userspace programs use it for different reasons, most of them probably wrong. But that's outside the scope of this change.

Diff Detail

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

Event Timeline

gallatin created this revision.
kib added inline comments.
sys/kern/subr_param.c
85

To avoid copy/paste, you could move this fragment into a new header sys/_maxphys.h, then include it from subr_param.h and sys/param.h for !_KERNEL.

This revision is now accepted and ready to land.Apr 28 2024, 2:16 AM

Please add Fixes: 30038a8b4efc ("md: Get rid of the pbuf zone") to the commit log message.

  • Update diff to avoid cutting/pasting MAXPHYS definition as per @kib's suggestion
This revision now requires review to proceed.Apr 28 2024, 4:41 PM
This revision is now accepted and ready to land.Apr 28 2024, 6:04 PM
kib added inline comments.
sys/sys/_maxphys.h
2

I suspect we do need a copyright boilerplate for the file, even if a single SPDX-line kind.

I consulted with @imp, and after a trip down the rabbit hole, we concluded that a header file consisting only of the definition of MAXPHYS is not creative (as this is the only way to express this in C) so it can't have copyright protection, and should simply be public domain.

This revision now requires review to proceed.Apr 29 2024, 11:24 PM
This revision is now accepted and ready to land.Apr 29 2024, 11:54 PM
sys/sys/_maxphys.h
8

The one i propsed had shifts and no comments

sys/sys/_maxphys.h
8

Whoops, sorry. I missed that in our convo.. I'm happy to remove the comment, but I'd prefer to keep the #defines the same because:

  • to me at least, the multiplications are more clear than the shifts
  • I don't want to do another tinderbox run