Please check __SIZEOF_LONG__ == 4/8; you care about long here, not pointer size, but ILP32 / LP64 conflate the two (on 64-bit CHERI architectures like Morello and CHERI-RISC-V we have 8 byte long, unchanged, but 16 byte pointers)

Missing newlines here and in the other runq_sw_apply() wrappers below.

Why?

Why not check the bitset instead?

Is there some reason we can't use bitset(9) for this?

This change should be committed on its own, IMO.

This was kind of "passively deliberate". I usually put an empty new line at start if there are no declarations (old style), but at the same time style(9) now allows not putting them. I'm fine with both approaches.

The lines you're pointing at are indeed somewhat inconsistent with some other uses in this file, including some I introduced. I've just fixed that and settled for the old style. Given the extent of the changes, I could as well switch all relevant parts to the new style. What do you think?

Because it is then used by sched_ule.c in D45390. I could move it to <sys/runq.h> though.

That's what runq_first_thread() already does. It also fetches the first available thread which is not strictly necessary here, but then the caller (4BSD) needs to fetch it also, so I don't think this will make a practical difference with respect to the data cache and performance. Calling runq_first_thread() allows to save on instruction cache by not duplicating assembly code (which would be the case using bitset macros; but this could most probably be factorized in a separate function also). I'll see if/how I can rework that.

I thought we were only supporting ILP32 or LP64 (see also arch(7)), that's the reason I chose to test for that. I've just changed it with your suggestion instead.

Even if CheriBSD is not FreeBSD, I would suggest that you (or some people working on it) update arch(7) in FreeBSD to reflect what CheriBSD as a downstream consumer does, so that people can generally be aware of such fundamental extensions that need support from upstream.

No special reason other than the original code didn't use it and, although aware of it, somehow I didn't connect the dots.

It seems some of the functionality that this work requires is missing in bitset(9), and I think it's a good opportunity to add it there. Otherwise, the functionality is really the same, so it would indeed be better to avoid code duplication.

I'll change that as a separate commit, for ease of commit/review, which will be grouped with the commits of this differential revision (so the change will appear here when it's done).

Yes. This whole review in fact consists of 12 commits coalesced. This particular change is (currently) part of commit 4 ("API tidy up: 'pri' => 'idx', 'idx' as int, remove runq_remove_idx() (knows some ULE's internals)."). Does that answer your concern, or would like to see, e.g., the content of that specific commit?

It's the newline between } and static inline void not the first line of the fn Mark's talking about here

Checked with Mark, you're right.

This was also deliberate... I wanted to have a single herald comment for both the public function and the callback used to implement it (passed to runq_sw_apply()). Can I keep that like that? Else I'd move the herald comment close to the public function, unless you have some better suggestion?

I finally decided to completely remove RQSW_L2BPW and let the compiler optimize / and % with RQSW_BPW as the dividend to simplify code a bit.

After an attempt to add the missing functionality in bitset(9) (last June), I concluded that doing so wasn't worth it, as it requires a lot of macrology, making it very hard to use, and potentially impacting changes for users (bitset(9)'s model is to have macros define all the required code, leading to unnecessary code and produced assembly duplication; departing from this scheme requires having macros that declare/define common functions which must then be used by consumers).

Additionally, implementing range scanning with predicates efficiently requires knowing the internals of bitset(9), and it seems they are supposed to stay opaque. So, I'm keeping the current implementation for now.

Since runq already means 'run queue', the name is tautological.

Since you changing the return type, the function can be written much more concise now.
Also we have TD_IS_IDLETHREAD().

return (TDQ_LOAD(tdq) > (TD_IS_IDLETHREAD(curthread) ? 0 : 1);

Move this somewhere like sys/_param.h or sys/_howmany.h, and include the _header from there and from sys/param.h.

This is GNU extension, and for all installed headers we do mark it with extension

ffsl() requires strings.h in userspace, and sys/libkern.h in kernel.

And this requires sys/queue.h.
To put it differently, I do not see why include some random headers, but not other, from the header.

constness for the value parameters is not too useful.

Again, there is a lot of stuff that is needed to make the header compilable, and adding a random one there is not useful.

Although unfortunate, "run queue" designates the top structure holding all queues, and the predicate here only applies to a single queue within the run queue. The current naming respects that and is deliberate. Completely changing the terminology was out of scope for this change.

It could have been even with the previous type, as essentially the returned value has not changed.

That's much easier to read indeed.

Done, see D50880.

Done here, but removing the include of strings.h later in this series, as I'm then preventing use of <sys/runq.h> from userspace. There's really nothing in this header that is user-serviceable, especially given that this series switches RQ_PPQ to 1 (and RQ_PPQ should just disappear, at least from userland, as it's an implementation detail irrelevant to the outside work after this work).

Simple: The includes you're talking about were already missing. I just added those related to my own additions.

I've just added them.

Not useful on x86, as these are passed in registers that are not callee-saved, but might be useful on other architectures. Anyway, I prefer that the signature reflects the expected implementation behavior.

It's not a random one, but just the new one needed given modifications here (and that is useful, even if incomplete).

I've added others now. That said, I'll now refrain for doing such changes in this series. This wastes too much time because every following commit has to be rebased, and we can fix these issues afterwards.

I looked at the generated code a while ago and the compiler was completely getting rid of the stub and inlining runq_sw_set_empty(). I've just check now and that hasn't changed (and same for runq_sw_set_not_empty()).