Page MenuHomeFreeBSD

cpuset: Move userspace declarations out of _cpuset.h
ClosedPublic

Authored by markj on Wed, May 6, 5:27 PM.
Tags
None
Referenced Files
F157300830: D56856.id177319.diff
Wed, May 20, 3:45 AM
F157267162: D56856.id.diff
Tue, May 19, 9:20 PM
F157196545: D56856.id.diff
Tue, May 19, 6:03 AM
Unknown Object (File)
Mon, May 18, 12:51 PM
Unknown Object (File)
Mon, May 18, 12:10 AM
Unknown Object (File)
Mon, May 18, 12:10 AM
Unknown Object (File)
Fri, May 15, 3:39 AM
Unknown Object (File)
Thu, May 14, 6:04 PM
Subscribers

Details

Summary

The _* headers are for structure definitions and should avoid
dependencies on other headers. This convention is violated by using
BEGIN_DECLS/END_DECLS.

Move the declarations to cpuset.h, I see no reason they can't be there.

Diff Detail

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

Event Timeline

markj requested review of this revision.Wed, May 6, 5:27 PM
kib added a subscriber: brooks.
kib added inline comments.
sys/sys/cpuset.h
185

I was sure that __DECLS were extracted from cdefs.h into smaller header, but it seems @brooks did not committed that patch still.

This revision is now accepted and ready to land.Wed, May 6, 6:57 PM
olce added a subscriber: olce.

I suspect the idea was that internal functions (prefixed by __) should be in an "internal" header sys/sys/_cpuset.h, except that's not the meaning of the _-prefixed header.

Not sure why __cpuset_alloc() and __cpuset_free() are even provided. They are not used in the tree.

I suspect the idea was that internal functions (prefixed by __) should be in an "internal" header sys/sys/_cpuset.h, except that's not the meaning of the _-prefixed header.

Not sure why __cpuset_alloc() and __cpuset_free() are even provided. They are not used in the tree.

There are some tests which use the CPU_ALLOC wrapper. glibc provides a similar macro so I guess it's to provide compatibility.

There are some tests which use the CPU_ALLOC wrapper. glibc provides a similar macro so I guess it's to provide compatibility.

Yes, I missed that we have a CPU_ALLOC() wrapper too. (glibc defines this macro as an alias to __sched_cpualloc().)

I'm not sure though this header is standalone, as I see references to size_t. That API is documented in the man page to require sys/param.h, but perhaps it would be better to add sys/types.h in the !KERNEL section.

I'm not sure though this header is standalone, as I see references to size_t. That API is documented in the man page to require sys/param.h, but perhaps it would be better to add sys/types.h in the !KERNEL section.

Sure, I may try to fix that too. My larger goal is to land D56857 and the patch below (which I suspect will break a lot of code in the ports tree, so may take a lot of work).

commit d6a6e4d6a7a7b2a4dc4222ce80850d3052573f56 (HEAD -> main-mount-pollution)
Author: Mark Johnston <markj@FreeBSD.org>
Date:   Wed May 6 19:49:52 2026 +0000

    ucred.h: Do not include audit.h unconditionally
    
    audit.h is included because struct ucred embeds a struct auditinfo_addr,
    which is defined in audit.h.  But struct ucred is not visible to
    userspace by default, so we shouldn't be including audit.h by default
    either, especially since audit.h pulls in param.h, which pollutes the
    namespace.  (In particular it pulls in machine/param.h, which #defines
    "MACHINE", which makes the QEMU build unhappy.)
    
    MFC after:      1 week

diff --git a/sys/sys/ucred.h b/sys/sys/ucred.h
index ba241cf9ff3a..0685e06f3f10 100644
--- a/sys/sys/ucred.h
+++ b/sys/sys/ucred.h
@@ -36,8 +36,8 @@
 #if defined(_KERNEL) || defined(_WANT_UCRED)
 #include <sys/_lock.h>
 #include <sys/_mutex.h>
-#endif
 #include <bsm/audit.h>
+#endif
 
 #if defined(_KERNEL) || defined(_WANT_UCRED)
 /*

I don't see any need to define these in all the headers that need type definitions.

sys/sys/cpuset.h
185

That would likely be incredibly disruptive. What I did to is reduce includes of cdefs.h in some std*.h headers because code kept relying on __DECLS coming from inappropriate places making it difficult to allow compilers to provide their own std*.h.

sys/sys/cpuset.h
185

I mean having sys/_decls.h with just __BEGIN/END_DECLS. Of course sys/cdefs.h needs to include it.