Deduplicate sched stats, sdt probes, and kdtrace hooks helper vars.
At least amd64 GENERIC and LINT build.

One more #ifdef KTR for 4bsd.

Take the DEFINE_SHIM() proposal.
Fix inlined strcmp().
Also hopefully fix compilation issues, but I only started tinderbox.

In D54850#1253853, @asomers wrote:

In D54850#1253849, @kib wrote:

Is this the only place where the update of the cache under the shared vnode lock occurs?
Or even better, can you point me to the single entry point of the cache update code?

I'm auditing now to look for other cases. I think I've found one in fuse_internal_invalidate_inode (which is used by very few real-world FUSE file systems, but has coverage in the test suite).

That said, there is much better strategy to solve the issue than trying to upgrade the lock.
Add one more mutex (or sx, I am not sure about ops that are done for cache update), and lock it around the cache update code if the vnode lock is only share-locked.

I did not coded it myself because I wanted an easy experiment if the exclusive locking would solve the problem. Next, I do not know where the cache update code is.

I could do that. In terms of performance, how does mutex compare to the vnode lock?

Add function-pointers based workaround for risvc and arm.
Hopefully at least riscv would grow ifuncs in some time frame.

Is this the only place where the update of the cache under the shared vnode lock occurs?
Or even better, can you point me to the single entry point of the cache update code?

In D54831#1253740, @jrtc27 wrote:

armv7 and riscv64 are tier 2, breaking them is not permitted. I have an idea for how to make them work though, without being too invasive.

Rename sched_instance_name variable to sched_name.

In D54831#1253627, @olce wrote:

In D54831#1253606, @kib wrote:

I am not sure what do you mean by kern.sched.instance_name, I cannot find such thing.

Indeed, I meant kern.sched.sched_instance_name. Could you rename it to simply kern.sched.name?

Add sched_instance_select() call to all arches.
Rename the tunable to kern.sched.name.
Automatically fall back to some scheduler if the named one is not found, and there is one.

In D54831#1253590, @olce wrote:

In D54831#1253559, @kib wrote:

Make kern.sched sysctls working when 4BSD is selected:

I do not see any new change related to that (compared to the previous diff)?

Handle all arches for cpu_switch.S.

Make kern.sched sysctls working when 4BSD is selected:
sysctl kern.sched.ule.topology_spec: allow to run if ULE is not initialized
sched_shim: restore kern.ccpu sysctl

It is apparently should be considered part of the ABI, and is used by
the base top(1).  But do not declare the ccpu variable in headers, it is
needed only by 4bsd. So put the variable definition into sched_shim.c to
make the kernel buildable without SCHED_4BSD.

In D54831#1253496, @olce wrote:

In D54831#1253408, @kib wrote:

In D54831#1253398, @olce wrote:

I'd like also that we investigate why the loop on the blocked lock was elided for 4BSD.

Simply because threads are never blocked because there is only one global scheduler lock on 4BSD instead of per-runq lock on ULE. So on ULE you are not guaranteed that the sched_switch() finished with removing the thread from CPU while cpu_switch() already tries to switch to the new context. The blocked state makes this race closed without the need to somehow lock several runqs. On 4BSD it is impossible (global lock).

I well know what it is used for in ULE. Was not sure for 4BSD as it uses the block lock as well. After a check, that is just to release sleepqueues et alter's locks earlier (as ULE also does), and indeed the new selected thread cannot have the block lock.

In D54816#1253459, @markj wrote:

In D54816#1253431, @kib wrote:

So I went ahead and implemented the third (?) approach. IMO it is the only safe option there.

We cannot convert ktrace_mtx to a spinlock?

Remove comment.
Add __ktrace_used to 'tv' decls,

Upload the right diff.

So I went ahead and implemented the third (?) approach. IMO it is the only safe option there.

If you really dislike this approach, I think a workable solution that also works for msleep_spin() is the following.

Actually commit the comment before diffing.

In D54816#1253407, @markj wrote:

In D54816#1253403, @kib wrote:

In D54816#1253389, @markj wrote:

The issue is that for ktrcsw() we lock the ktrace_mtx mutex while owning the interlock from a subsystem that called msleep().

Would another solution be to modify _sleep() to release the spinlock later, effectively copying what msleep_spin_sbt() does?

I do not quite follow, sorry? We either own the interlock, or we own the spinlock. ktrcsw() cannot be called under spinlock at all.

msleep_spin_sbt() drops the sleepqueue spinlock in order to call ktrcsw(). So, it is not susceptible to the problem fixed by this diff. Can we use the same strategy elsewhere instead of adding a new field to struct thread?

In D54831#1253408, @kib wrote:

In D54831#1253398, @olce wrote:

I'd like also that we investigate why the loop on the blocked lock was elided for 4BSD.

Simply because threads are never blocked because there is only one global scheduler lock on 4BSD instead of per-runq lock on ULE. So on ULE you are not guaranteed that the sched_switch() finished with removing the thread from CPU while cpu_switch() already tries to switch to the new context. The blocked state makes this race closed without the need to somehow lock several runqs. On 4BSD it is impossible (global lock).

Checking the blocked state on 4BSD would give 4 instructions overhead which I dismiss with prejudice.

In D54831#1253398, @olce wrote:

I'd like also that we investigate why the loop on the blocked lock was elided for 4BSD.

Rename to ktrace_mtx_unlock(), add explanation.

In D54816#1253389, @markj wrote:

The issue is that for ktrcsw() we lock the ktrace_mtx mutex while owning the interlock from a subsystem that called msleep().

Would another solution be to modify _sleep() to release the spinlock later, effectively copying what msleep_spin_sbt() does?

In D54831#1253391, @minsoochoo0122_proton.me wrote:

In D54831#1253386, @kib wrote:

In D54831#1253383, @minsoochoo0122_proton.me wrote:

@olce and I are working for the same goal but with different approach. Instead of putting them both in kernel binary, our approach is to make sched_4bsd as a kernel module which overrides sched_ule's weak symbol on boot. This gives us two advantages:

1. Kernel binary size stays the same (but letting them coexisting won't have significant impact on kernel size so this is not a huge advantage)
2. We can let users to create third-party scheduler as kernel modules. This way, they can run their own scheduler by just building their kernel module without buildkernel and installworld .

The first step of this approach is D54830 which I opened a few hours ago.

Weak symbols do not work exactly this way generally, and esp. in kernel. Anyway, you would not get what you want without significantly hacking either very early linker or even the loader.
I am slightly curious how far this would lead you.

I think I was trying to tackle that idea without deeper understanding in kernel linker. I'll do more research on this. But at least I want hear opinions on loading scheduler as kernel module.

In D54831#1253383, @minsoochoo0122_proton.me wrote:

@olce and I are working for the same goal but with different approach. Instead of putting them both in kernel binary, our approach is to make sched_4bsd as a kernel module which overrides sched_ule's weak symbol on boot. This gives us two advantages:

1. Kernel binary size stays the same (but letting them coexisting won't have significant impact on kernel size so this is not a huge advantage)
2. We can let users to create third-party scheduler as kernel modules. This way, they can run their own scheduler by just building their kernel module without buildkernel and installworld .

The first step of this approach is D54830 which I opened a few hours ago.

I only tried to boot into ULE so far.
There are two MD places that need to be handled for each arch, but I only did that for amd64 right now:

• cpu_switch() made unconditionally wait for blocked thread lock unblock
• sched_instance_select() must be called before ifuncs are resolved.

Ok, my reluctance to this documentation change is because such promise, of never returning specific error code, is very hard to fulfill. For instance, on intr NFS mount, VOP_GETATTR() can return EINTR, and so on.

In summary: defined in the comments as a function tWo arguments I believe.

There is a call to kern_writev() on the socket, is it true that it never returns EINTR for non-blocking socket?

i386 adjustments for kern_thread.c KBI asserts will be done after the review

Assert that PDF_CLOSED is not set for fd operated by pdwait().
Make pdwait() cancellable.

Avoid racy access to the p_pid member of possibly reclaimed struct proc.
Other review' comments are handled.

In D54785#1252692, @jtl wrote:

In D54785#1252666, @kib wrote:

I do not think that trying to schedule a task is very robust with a LOR detected. We are already in the situation risking the deadlock.

Fair point!

If you refuse to make this a default, I am asking to add a simple knob which would make this additional info to be printed in all situations.

I don't "refuse"; I was just giving my thinking.

Ok, refuse was probably too strong word, sorry about this.

I do not think that trying to schedule a task is very robust with a LOR detected. We are already in the situation risking the deadlock.
Also I do not think that the doubts of the stack overflow is very grounded, since WITNESS typically has spinlock recording disabled II believe there are not solved issues with console spinlock which in reality cause hard hang). So there is definitely the space for interrupt frame, and mode, when WITNESS running.

Correct macro names.

Grammar and wording of the man page update.

Add note about the pid reuse.

In D54592#1252227, @asomers wrote:

This looks great, @kib! If you commit it, I'll follow up with the test suite. A few questions:

• Are you planning to write the man page yourself, or would you like help with that?

Add documentation.
Fix pdwait() prototype.

This should be extremely useful, I already anticipate it. But, why not make this information available by default? I am not even sure that the knob is needed to hide it.

Renumber AUE_PDRFORK.
Drop contrib/openbsm patch, similar to how other new AUE_ constants are not imported there.

In D54323#1252001, @cy wrote:

DSO bump.

Use lapic_maxlvt() everywhere.

Save file before diffing.

Add comment in vm_init.c