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

Aggelos Economopoulos aoiko at cc.ece.ntua.gr
Wed Jan 16 08:26:39 PST 2008


On Monday 07 January 2008, Matthew Dillon wrote:
[...]
>     If we're gonna start throwing around different types of sleeps maybe
>     we should make the names more verbose. spin_sleep(), lockmgr_sleep(),
>     etc. [...]

So, about adding a lockmgr_sleep(): I think it would be useful; open-coding
the whole thing makes it easy to slip in hard to notice bugs. But what should
the prototype be? If we follow the freebsd semantics, the lock should be re-
acquired on return. This means that we need to do a lockstatus() (which implies
a spin_lock_wr()) to get information that the caller probably has available or
add an extra "lock flags" parameter which complicates the interface. The
options seem to be:

* lockmgr_sleep(void *ident, struct lock *lk, int flags, const char *wmesg,
	int timeo)

forces spin_lock_wr(), but is consistent with our (and freebsd's) msleep()
prototype.

* lockmgr_sleep(void *ident, struct lock *lk, int lkflags, int flags,
	const char *wmesg, int timeo)

this sucks. If you accidentally exchange the flags arguments you'll only find
out after one or more debugging sessions.

* lockmgr_sleep(void *ident, int lkflags, struct lock *lk, int flags,
	const char *wmesg, int timeo)

this is ugly and inconsistent.

*  lockmgr_sleep(void *ident, int flags, const char *wmesg, int timeo,
		struct lock *lk, int lkflags)

this is more promising but inconsistent with our msleep(). OTOH, I the
original msleep() prototype seems a bit backward to me, so I'd consider
this an improvement.

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)

The down side of course is having to change a couple of dozen call sites. But
as long as we're breaking people's patches with the name change, we might as
well fix the interface, assuming everyone agrees that's an improvement.

Any thoughts and/or better ideas?

Aggelos





More information about the Submit mailing list