Re: [PATCH] kprobes for s390 architecture

[Heiko Carstens <heiko.carstens@de.ibm.com>]

On Tue, Jun 27, 2006 at 05:23:09PM +0200, Martin Schwidefsky wrote:
> On Sat, 2006-06-24 at 13:36 +0200, Heiko Carstens wrote:
> > > At least this is something that could work... completely untested and might
> > > have some problems that I didn't think of ;)
> > > 
> > > struct capture_data {
> > > 	atomic_t cpus;
> > > 	atomic_t done;
> > > };
> > > 
> > > void capture_wait(void *data)
> > > { 
> > > 	struct capture_data *cap = data;
> > > 
> > > 	atomic_inc(&cap->cpus);
> > > 	while(!atomic_read(&cap->done))
> > > 		cpu_relax();
> > > 	atomic_dec(&cap->cpus);
> > > }
> > > 
> > > void replace_instr(int *a)
> > > {
> > > 	struct capture_data cap;
> > > 
> > > 	preempt_disable();
> > > 	atomic_set(&cap.cpus, 0);
> > > 	atomic_set(&cap.done, 0);
> > > 	smp_call_function(capture_wait, (void *)&cap, 0, 0);
> > > 	while (atomic_read(&cap.cpus) != num_online_cpus() - 1)
> > > 		cpu_relax();
> > > 	*a = 0x42;
> > > 	atomic_inc(&cap.done);
> > > 	while (atomic_read(&cap.cpus))
> > > 		cpu_relax();
> > > 	preempt_enable();
> > > }
> > 
> > Forget this crap. It can easily cause deadlocks with more than two cpus.
> 
> It is not that bad. Instead of preempt_disable/preempt_enable we need a
> spinlock. Then only one cpu can do this particular smp_call_function
> which will "stop" all other cpus until cap->done has been set.

CPU0: smp_call_function() -> loops and waits for other cpus
CPU1: [irqs_enabled] - spin_lock(somelock) -> irq -> capture_wait() -> loop
CPU2: [irqs_enabled] ----- spin_lock_irqsave(a,..) -> toasted

CPU2 ends up trying to grab the same lock that CPU1 holds, but has interrupts
disabled and a pending external interrupt because of the smp_call_function()..

> > Just do a compare and swap operation on the instruction you want to replace,
> > then do an smp_call_function() with the wait parameter set to 1 and passing
> > a pointer to a function that does nothing but return.
> Not good enough. An instruction can be fetched multiple times for a
> single execution (see the other mail). So you have a half executed
> instruction, the cache line is invalidated, a new instruction is written
> and the cache line is recreated to finished the half executed
> instruction. That can easiliy happen on millicoded instructions.

Yes, looks like I was too optimistic. Seems like we really have to go for
stop_machine_run() unless somebody comes up with a better idea...