Re: Spinlocks, intr levels et al

From: Kanoj Sarcar (kanoj@google.engr.sgi.com)
Date: Thu Jan 06 2000 - 15:17:38 EST


>
> Kanoj Sarcar wrote:
> > Okay. Btw, thinking about this more, spin_lock_irqsave_intercpu()
> > might be problematic even for a single lock L1. Assume cpu1 has
> > the lock L1, then cpu2 gets an intr, also tries doing
> > spin_lock_irqsave_intercpu(&L1). cpu1 now tries to do an inter
> > cpu function, but cpu2 will not be able to reply. Unless the
> > inter cpu function uses a suitably higher irq level to interrupt
> > other cpus.
> >
> > Right?
>

Okay, I was able to get around my original problem: basically, I think
it is quite complex to do intercpu interrupts while holding a lock that can
also be acquired from intr level. So, I recoded a little bit to get
the ipi's done without holding the lock.

In case anyone is interested, I can now do kunmap()'s from intr
context, without deadlocking with ipi's needed for flush_tlb_all().

> Yes. I think that ipi's with disabled interrupts lead to madness. Just
> check the tlb flush code: in order to handle crossing invalidates,
> wait_on_irq() must poll the smp_invalidate_needed bitmap...

I am willing to tolerate this, since all the complexity is hidden
inside the flush_tlb* primitives. If the i386 specific version will
hold intrs high while one flush_tlb* is happening, I am willing to
let it handle the complexity of multiple, concurrent flush_tlb*s
by polling bitmaps etc.

>
> Btw, I would propose a change of change the prototype for
> smp_call_function():
>
> currently:
> int smp_call_function(fnc, info, wait, retry)
>
> wait: wait until the ipi functions on all other cores have finished
> retry: schedule() until the required data structures are free
>
> returns -EBUSY on error (IIRC)
>
> new:
> void smp_call_function(fnc,info,wait)
>

I am assuming you mean go from

int smp_call_function (void (*func) (void *info), void *info, int nonatomic,
                        int wait)

to

void smp_call_function (void (*func) (void *info), void *info, int wait)

> * never fails. panic() [on i386] indirectly relies on
> smp_call_function(), and panic() shouldn't fail ;)
> * never schedules, it uses a normal spinlock. [panic() could
> schedule...]

Yes, the above two are real problems. OTOH, have you walked the code
path down under panic() to identify all the scheduling points? Seems
to me sys_sync is another such point, maybe we should not be doing
that too?

Using a spinlock instead of a sleeping lock inside smp_call_function()
is a good idea.

> * the call-back is called with disalbed interrupts, and it must not
> reenable interrupts. [docu clarification]

Shouldn't this already be happening?

> * smp_call_function() itself enables interrupts, otherwise we could lock
> up.[the current code call schedule(), and thus indirectly reenables the
> interrupts]

Umm, I don't quite see the point. Are you sure smp_call_function() is never
called from intr context today? Even if not today, it would be nice to
let this happen in the future. I see the possibility of deadlock with
this though (the sleeping lock in smp_call_function() currently prevents
this I think). To prevent the deadlock, I think smp_call_function() would
need to have a percpu bitmask of possible ipi's that it needs to check
while it can not acquire the new spinlock that will replace ther sleeping
lock.

Or am I just confused?

Kanoj

>
> The "only" problem is that smp_call_function() is a documented
> cross-platform function, ie I doubt that Linus would accept the patch,
> but OTHO the current code it horribly broken.
>
> What do you think?
> --
> Manfred
>

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



This archive was generated by hypermail 2b29 : Fri Jan 07 2000 - 21:00:07 EST