Re: [Xen-devel] [PATCH v14 11/11] pvqspinlock, x86: Enable PV qspinlock for XEN
flat view
From: David Vrabel <hidden>
Date: 2015-01-21 10:30:31
Also in:
kvm, linux-arch, lkml
On 20/01/15 20:12, Waiman Long wrote:
This patch adds the necessary XEN specific code to allow XEN to support the CPU halting and kicking operations needed by the queue spinlock PV code.
Xen is a word, please don't capitalize it.
quoted hunk
+void xen_lock_stats(int stat_types) +{ + if (stat_types & PV_LOCKSTAT_WAKE_KICKED) + add_smp(&wake_kick_stats, 1); + if (stat_types & PV_LOCKSTAT_WAKE_SPURIOUS) + add_smp(&wake_spur_stats, 1); + if (stat_types & PV_LOCKSTAT_KICK_NOHALT) + add_smp(&kick_nohlt_stats, 1); + if (stat_types & PV_LOCKSTAT_HALT_QHEAD) + add_smp(&halt_qhead_stats, 1); + if (stat_types & PV_LOCKSTAT_HALT_QNODE) + add_smp(&halt_qnode_stats, 1); + if (stat_types & PV_LOCKSTAT_HALT_ABORT) + add_smp(&halt_abort_stats, 1); +} +PV_CALLEE_SAVE_REGS_THUNK(xen_lock_stats);
This is not inlined and the 6 test-and-branch cannot be optimized away.
quoted hunk
+/* + * Halt the current CPU & release it back to the host + * Return 0 if halted, -1 otherwise. + */ +int xen_halt_cpu(u8 *byte, u8 val) +{ + int irq = __this_cpu_read(lock_kicker_irq); + unsigned long flags; + u64 start; + + /* If kicker interrupts not initialized yet, just spin */ + if (irq == -1) + return -1; + + /* + * Make sure an interrupt handler can't upset things in a + * partially setup state. + */ + local_irq_save(flags); + start = spin_time_start(); + + /* clear pending */ + xen_clear_irq_pending(irq); + + /* Allow interrupts while blocked */ + local_irq_restore(flags);
It's not clear what "partially setup state" is being protected here. xen_clear_irq_pending() is an atomic bit clear. I think you can drop the irq save/restore here.
quoted hunk
+ /* + * Don't halt if the content of the given byte address differs from + * the expected value. A read memory barrier is added to make sure that + * the latest value of the byte address is fetched. + */ + smp_rmb();
The atomic bit clear in xen_clear_irq_pending() acts as a full memory barrier. I don't think you need an additional memory barrier here, only a compiler one. I suggest using READ_ONCE().
quoted hunk
+ if (*byte != val) { + xen_lock_stats(PV_LOCKSTAT_HALT_ABORT); + return -1; + } + /* + * If an interrupt happens here, it will leave the wakeup irq + * pending, which will cause xen_poll_irq() to return + * immediately. + */ + + /* Block until irq becomes pending (or perhaps a spurious wakeup) */ + xen_poll_irq(irq); + spin_time_accum_blocked(start); + return 0; +} +PV_CALLEE_SAVE_REGS_THUNK(xen_halt_cpu); + +#endif /* CONFIG_QUEUE_SPINLOCK */ + static irqreturn_t dummy_handler(int irq, void *dev_id) { BUG();
David