On 04/05/21(Tue) 01:10, Scott Cheloha wrote:
> [...]
> I want to run softclock() without the kernel lock. The way to go, I
> think, is to first push the kernel lock down into timeout_run(), and
> then to remove the kernel lock from each timeout, one by one.
Grabbing and releasing the KERNEL_LOCk() on a per-timeout basis creates
more latency than running all timeouts in a batch after having grabbed
the KERNEL_LOCK(). I doubt this is the best way forward.
> Before we can push the kernel lock down into timeout_run() we need to
> remove the kernel lock from timeout_del_barrier(9).
Seems worth it on its own.
> The kernel lock is used in timeout_del_barrier(9) to determine whether
> the given timeout has stopped running. Because the softclock() runs
> with the kernel lock we currently assume that once the calling thread
> has taken the kernel lock any onging softclock() must have returned
> and relinquished the lock, so the timeout in question has returned.
So you want to stop using the KERNEL_LOCK() to do the serialization?
> The simplest replacement I can think of is a volatile pointer to the
> running timeout that we set before leaving the timeout_mutex and clear
> after reentering the same during timeout_run().
Sounds like a condition variable protected by this mutex. Interesting
that cond_wait(9) doesn't work with a mutex.
> So in the non-TIMEOUT_PROC case the timeout_del_barrier(9) caller just
> spins until the timeout function returns and the timeout_running
> pointer is changed. Not every caller can sleep during
> timeout_del_barrier(9). I think spinning is the simplest thing that
> will definitely work here.
This keeps the current semantic indeed.
> -void
> -timeout_barrier(struct timeout *to)
> +int
> +timeout_del_barrier(struct timeout *to)
> {
> + struct timeout barrier;
> + struct cond c = COND_INITIALIZER();
> int needsproc = ISSET(to->to_flags, TIMEOUT_PROC);
>
> timeout_sync_order(needsproc);
>
> - if (!needsproc) {
> - KERNEL_LOCK();
> - splx(splsoftclock());
> - KERNEL_UNLOCK();
> - } else {
> - struct cond c = COND_INITIALIZER();
> - struct timeout barrier;
> + mtx_enter(&timeout_mutex);
> +
> + if (timeout_del_locked(to)) {
> + mtx_leave(&timeout_mutex);
> + return 1;
> + }
>
> + if (needsproc) {
> timeout_set_proc(&barrier, timeout_proc_barrier, &c);
> barrier.to_process = curproc->p_p;
> -
> - mtx_enter(&timeout_mutex);
> SET(barrier.to_flags, TIMEOUT_ONQUEUE);
> CIRCQ_INSERT_TAIL(&timeout_proc, &barrier.to_list);
> mtx_leave(&timeout_mutex);
> -
> wakeup_one(&timeout_proc);
> -
> cond_wait(&c, "tmobar");
> + } else {
> + mtx_leave(&timeout_mutex);
> + /* XXX Is this in the right spot? */
> + splx(splsoftclock());
> + while (timeout_running == to)
> + CPU_BUSY_CYCLE();
Won't splx() will execute the soft-interrupt if there's any pending?
Shouldn't the barrier be before? Could you add `spc->spc_spinning++'
around the spinning loop? What happen if two threads call
timeout_del_barrier(9) with the same argument at the same time? Is
it possible and/or supported?
