Re: lockmgr_sleep() (was Re: ssleep() (was Re: mention msleep() in porting_drivers.txt))

[Aggelos Economopoulos <aoiko@xxxxxxxxxxxxxx>]

On Wednesday 16 January 2008, Matthew Dillon wrote:
>     lockmgr_sleep(ident, lock, slpflags, wmesg, timeo)
> 
>     lockmgr_sleep can also figure out what kind of lock is held internally
>     and deal with it, or it can just assume an exclusive lock.  Either way
>     lkflags do not have to be passed to it.

Assuming an exclusive lock is not intuitive IMO and differs from the FreeBSD
semantics (even though it matches our msleep()/spin_sleep()). What do you
think of the lockmgr change suggested in another mail in this thread?

> 
> :If we're going to rename msleep() to spin_sleep() anyway, I suggest changing
> :the prototype to
> :
> :spin_sleep(void *ident, int flags, const char *wmesg, int timo,
> :	struct spinlock *spin)
> 
>     We want all the *sleep() procedures to use a similar prototype,
>     and in this case being compatible with our own history as well
>     as FreeBSD's will reduce confusion to a minimum.  That means putting
>     the lock as the second argument.
> 
>     spin_sleep(ident, spin, slpflags, wmesg, timeo)

Nobody can disagree with the "compatible" part, but for a whatever_sleep()
that does a "drop, tsleep() and reacquire whatever lock" it seems more
natural to require the tsleep() args followed by the whatever args. It may
be unlikely, but imagine having to add another flag. Would you put it after
"spin" seperating the tsleep() args further?

Also, I don't think FreeBSD compatibility is an issue, unless the two choices
are otherwise on par.

Aggelos