Date: Tue, 16 Aug 2005 16:22:41 -0700 From: Luigi Rizzo <rizzo@icir.org> To: John Baldwin <jhb@FreeBSD.org> Cc: Peter Jeremy <PeterJeremy@optushome.com.au>, freebsd-arch@FreeBSD.org Subject: Re: Special schedulers, one CPU only kernel, one only userland Message-ID: <20050816162241.A74005@xorpc.icir.org> In-Reply-To: <200508161716.44597.jhb@FreeBSD.org>; from jhb@FreeBSD.org on Tue, Aug 16, 2005 at 05:16:43PM -0400 References: <42F9ECF2.8080809@freebsd.org> <20050816051231.D66550@xorpc.icir.org> <20050816200010.GJ13959@cirb503493.alcatel.com.au> <200508161716.44597.jhb@FreeBSD.org>
next in thread | previous in thread | raw e-mail | index | archive | help
ok just for the records, the issue i had in mind is the release/acquire
the mutex around some code that might cause a deadlock, not for the
mutex ops per se, but for the need to make sure that the data
structure is consistent before releasing the lock, and rechecking
the state afterwards. Basically:
1 mtx_lock(&foo)
2 .. work on obj_foo
3 .. make obj_foo consistent
4 mtx_unlock(&foo)
5 f()
6 mtx_lock(&foo)
7 .. revalidate state of obj_foo
8 .. more work
where f() is the call that might sleep or cause a deadlock.
In cases where f() has a low probability of blocking, we could
save a bit of work at runtime by writing the code as below:
1 mtx_lock(&foo)
2 .. work on obj_foo
if (try_f_without_blocking() == EWOULDBLOCK) {
3 .. make obj_foo consistent
4 mtx_unlock(&foo)
5 f()
6 mtx_lock(&foo)
7 .. revalidate state of obj_foo
}
8 .. more work
(where try_f_without_blocking() is a version of f() that returns
EWOULDBLOCK in case the 'other' lock is busy).
Here maybe we would benefit by some support (macros, whatever) that
permits us to specify in a compact way what to do around f() should
it become blocking.
On the other hand, maybe the instances of code as the one above
are so rare that there is hardly any need for that.
cheers
luigi
On Tue, Aug 16, 2005 at 05:16:43PM -0400, John Baldwin wrote:
> On Tuesday 16 August 2005 04:00 pm, Peter Jeremy wrote:
> > On Tue, 2005-Aug-16 05:12:31 -0700, Luigi Rizzo wrote:
> > >reading this thread, and at times looking at some of the kernel code,
> > >with plenty of places where you have to drop a lock that you
> > >already have, do some small thing and then reacquire the lock itself,
> > >makes me wonder if we don't need a better mechanism/abtraction for
> > >this kind of programming.
> >
> > There are two distinct cases where this is done:
> > 1) The thread needs to call another function which can potentially sleep
> > and therefore needs to drop any locks it is holding before calling
> > the function and re-acquire them later.
> > 2) The thread needs to execute for an excessive period whilst holding
> > the lock and explicitly releases it occasionally to allow other
> > threads to run (eg much of the vnode/buffer/page scanning code).
> >
> > For the second case, we could create a function that checked if
> > another thread was waiting on the lock and, if so, released the lock
> > and grabbed it again after sleeping. This is fairly easy for a single
> > lock but would be quite messy to implement for multiple locks - since
> > you need to release/acquire them in the correct order to prevent
> > deadlocks.
> >
> > The first case again seems easy at first glance: Add a "I'm happy
> > to release this lock" flag to each lock. Before calling a function
> > that might block, you set the flag. When the function calls sleep(9),
> > the code releases the locks and re-acquires them before returning.
> > There are three large gotchas:
> > 1) Since sleeping might have changed the state of some structures
> > that the thread is working on, it needs to re-validate its
> > internal state to ensure that it's not working with stale data.
> > This means the thread needs to check if it slept or not after
> > each point where it could potentially sleep - and I suspect
> > this amounts to most of the non-boilerplate code in the existing
> > re-acquire lock section.
> > 2) You have to ensure that either the release/acquire is atomic for
> > all locks or the ordering is correct.
> > 3) You need some way to pass back a permanent failure to re-acquire
> > a requested lock (maybe a piece of hardware went away).
>
> We had a thought about this at USENIX ATC two years ago in Boston. The idea
> would be that each thread had an array of say 4 mutex pointers. You would
> have two new mutex functions:
>
> mtx_unlock_if_sleep();
> mtx_lock_if_slept();
>
> mtx_unlock_if_sleep() would add the mutex to the per-thread array. If the
> array was full, it would unlock the lock. If you ever slept, you would
> unlock the locks in the order that they are in the array. The
> mtx_lock_if_slept() function would lock the mutex if it wasn't in the array
> due to an overflow or if the thread had slept and would remove the mutex from
> the array if it was in there. Note that if you don't carefully balance the
> calls you could create trouble but some assertions could help catch that.
> Programmatically, the programmer would have to think of these just as if it
> was a normal mtx_unlock() and mtx_lock() pair. All it does is avoid dropping
> the lock unless you actually block. Thus, one case might be:
>
> mtx_lock(&foo);
> ...
> mtx_unlock_if_sleep(&foo);
> malloc(, M_WAITOK, );
> mtx_lock_if_slept(&foo);
> ...
> mtx_unlock(&foo);
>
> Note that as far as handling races, etc. though it would be the same as
>
> mtx_lock(&foo);
> ...
> mtx_unlock(&foo);
> malloc(, M_WAITOK, );
> mtx_lock(&foo);
> ...
> mtx_unlock(&foo);
>
> That is, it doesn't simplify handling of races at all or eliminate any races
> and if anything is even more confusing, so I'm not sure if it would really
> buy much and if what it bought would be worth the potential for confusion and
> folks thinking it did eliminate races thus resulting in bugs.
>
> I think one of the biggest things to help make things saner in general is to
> queue up as much work as possible while holding the lock and defer things
> that need to happen outside of the lock so you avoid lots of unlock/lock
> pairs.
>
> --
> John Baldwin <jhb@FreeBSD.org> <>< http://www.FreeBSD.org/~jhb/
> "Power Users Use the Power to Serve" = http://www.FreeBSD.org
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?20050816162241.A74005>