Re: Buggy IPI and MTRR code on low memory

From: Steven Rostedt
Date: Wed Jan 28 2009 - 19:30:15 EST



On Wed, 28 Jan 2009, Andrew Morton wrote:
> >
> > I don't think so. With ticket spinlocks and such, that just looks like it
> > is destine to crash. Also, spinlocks disable preemption, and we would need
> > to enable it again otherwise we have a dangling preempt_disable.
>
> Well that sucks.
>
> I think the model of taking a lock, passing the data across to another
> thread of control and having that thread of control release the lock
> once the data has been consumed is a good one. It's faster and in this
> case it means that we can now (maybe) perform (wait=0) IPI calls with local
> interrupts disabled, which wasn't the case before.

I think that model is nice too. Just not for spinlocks ;-)

>
> It's a shame that irrelevant-other-stuff prevents us from implementing
> such a thing. Of course, we can still do it, via some ad-hoc locking
> thing using raw_spin_lock/atomic_t's/bitops/cpu_relax/etc.

It should not be too hackish. I don't even think we need raw or atomic.

cpu caller:

again:
spin_lock(&data_lock);
if (per_cpu(data, cpu).lock) {
spin_unlock(&data_lock);
cpu_relax();
goto again;
}

per_cpu(data, cpu).lock = 1;
spin_unlock(&data_lock);

call ipi functions


cpu callee:

func();

smp_wmb();
per_cpu(data, cpu).lock = 0;


Here we can only call an ipi on a funcion if the data lock is cleared.
If it is set, we wait for the ipi function to clear it and then we set it.

Thus we synchronize the callers, but the callers do not need to wait on
the callees. And since the callers can only call a ipi on a cpu that is
finished, we do not need to worry about corrupted data.

>
> > Of course this will only work with the single cpu function callers. I
> > think the all cpu function callers still need to alloc.
>
> I haven't really thought about the smp_call_function_many()
> common case.

We can keep the original code that still does the malloc for the many
cpus. But falls back to the single version that does not use malloc if the
malloc fails.


current code for smp_call_function_many():

data = kmalloc(sizeof(*data) + cpumask_size(), GFP_ATOMIC);
if (unlikely(!data)) {
/* Slow path. */
for_each_online_cpu(cpu) {
if (cpu == smp_processor_id())
continue;
if (cpumask_test_cpu(cpu, mask))
smp_call_function_single(cpu, func, info, wait);
}
return;
}

I think this would work.

/me goes and tries it out

-- Steve
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/