Re: [RFC] New locking primitive for 2.5

Christoph Hellwig
2002-02-07T18:23:18Z

In article <3C629F91.2869CB1F@dlr.de> you wrote:
> The new lock uses a combination of a spinlock and a (mutex-)semaphore.
> You can lock it for short-term issues in a spin-lock mode:
>
> combi_spin_lock(struct combilock *x)
> combi_spin_unlock(struct combilock *x)
>
> and for longer lasting tasks in a sleeping mode by:
>
> combi_mutex_lock(struct combilock *x)
> combi_mutex_unlock(struct combilock *x)

I think this API is really ugly. If both pathes actually do the same,
just with different defaults, one lock function with a flag would be
much nicer. Also why do we need two unlock functions?

What about the following instead:

combi_lock(struct combilock *x, int spin);
combi_unlock(struct combilock *x);

> If a spin_lock request is blocked by a mutex_lock call, the spin_lock
> attempt also sleeps i.e. behaves like a semaphore.
> If you gained ownership of the lock, you can switch between spin-mode
> and mutex-(ie.e sleeping) mode by calling:
>
> combi_to_mutex_mode(struct combilock *x)
> combi_to_spin_mode(struct combilock *x)
>
> without loosing the lock. So you may start with a spin-lock and relax
> to a sleeping lock if for example you need to call a non-atomic kmalloc.

This looks really ugly. I'd really prefer an automatic fallback from
spinning to sleeping after some timeout like e.g. solaris adaptive
mutices.

> * Does it make sense to also provide irq-save versions of the
> locking functions? This means you could use the unlock functions
> from interrupt context. But the main use in this situation is
> completion handling and there are already (new) completion handlers
> available. So I don't think this is a must have.

You are no supposed to sleep in irq context, so irq-save combi-locks
don't make that much sense, IMHO.

Christoph

--
Of course it doesn't work. We've performed a software upgrade.