[Differential] [Commented On] D1711: Changes to the callout code to restore active semantics and also add a test-framework and test to validate thecallout code (and potentially for use by other tests).

[hselasky (Hans Petter Selasky)]

hselasky added a comment.

randall: You are right I confused the two c_cpu values.

Let my try to shoot again:

static struct callout_cpu *
callout_cpu_switch(struct callout *c, struct callout_cpu *cc, int new_cpu)
{
        struct callout_cpu *new_cc;

        MPASS(c != NULL && cc != NULL);
        CC_LOCK_ASSERT(cc);

        /*
         * Avoid interrupts and preemption firing after the callout cpu
         * is blocked in order to avoid deadlocks as the new thread
         * may be willing to acquire the callout cpu lock.
         */
        c->c_cpu = CPUBLOCK;
        spinlock_enter();
        CC_UNLOCK(cc);
        new_cc = CC_CPU(new_cpu);
        CC_LOCK(new_cc);
        spinlock_exit();
        c->c_cpu = new_cpu;
        return (new_cc);
}

spinlock_enter() is not an SMP lock. It only protects the executing CPU from interrupts and task switching. Basically that means two threads can be stuck at:
new_cc = CC_CPU(new_cpu), meaning a single callout can be switched to multiple different CPU's at the same time? Or am I wrong?

--HPS

REVISION DETAIL
  https://reviews.freebsd.org/D1711

To: rrs, gnn, rwatson, lstewart, jhb, kostikbel, sbruno, imp, adrian, hselasky
Cc: julian, hiren, jhb, kostikbel, emaste, delphij, neel, erj, freebsd-net