On Wed, Apr 08, 2020 at 03:33:52PM +0200, Jürgen Groß wrote: > On 08.04.20 14:08, Peter Zijlstra wrote: > > On Tue, Apr 07, 2020 at 10:02:57PM -0700, Ankur Arora wrote: > > > Mechanism: the patching itself is done using stop_machine(). That is > > > not ideal -- text_poke_stop_machine() was replaced with INT3+emulation > > > via text_poke_bp(), but I'm using this to address two issues: > > > 1) emulation in text_poke() can only easily handle a small set > > > of instructions and this is problematic for inlined pv-ops (and see > > > a possible alternatives use-case below.) > > > 2) paravirt patching might have inter-dependendent ops (ex. > > > lock.queued_lock_slowpath, lock.queued_lock_unlock are paired and > > > need to be updated atomically.) > > > > And then you hope that the spinlock state transfers.. That is that both > > implementations agree what an unlocked spinlock looks like. > > > > Suppose the native one was a ticket spinlock, where unlocked means 'head > > == tail' while the paravirt one is a test-and-set spinlock, where > > unlocked means 'val == 0'. > > > > That just happens to not be the case now, but it was for a fair while. > > Sure? This would mean that before spinlock-pvops are being set no lock > is allowed to be used in the kernel, because this would block the boot > time transition of the lock variant to use. Hurm.. true. I suppose I completely forgot how paravirt spinlocks looked before it got rewritten. > Another problem I'm seeing is that runtime pvops patching would rely on > the fact that stop_machine() isn't guarded by a spinlock. It can't be, stop_machine() relies on scheduling. But yes, that another variation of 'stuff uses spinlocks'.
