Re: [Qemu-devel] [PATCH v2 2/2] QEMUBH: make AioContext's bh re-entrant

[Stefan Hajnoczi]

On Wed, Jun 19, 2013 at 11:27:25AM +0200, Paolo Bonzini wrote:
> Il 19/06/2013 00:26, mdroth ha scritto:
> > On Tue, Jun 18, 2013 at 09:20:26PM +0200, Paolo Bonzini wrote:
> >> Il 18/06/2013 17:14, mdroth ha scritto:
> >>> Could we possibly simplify this by introducing a recursive mutex that we
> >>> could use to protect the whole list loop and hold even during the cb?
> >>
> >> If it is possible, we should avoid recursive locks.  It makes impossible
> >> to establish a lock hierarchy.  For example:
> >>
> >>> I assume we can't hold the lock during the cb currently since we might
> >>> try to reschedule, but if it's a recursive mutex would that simplify
> >>> things?
> >>
> >> If you have two callbacks in two different AioContexts, both of which do
> >> bdrv_drain_all(), you get an AB-BA deadlock
> > 
> > I think I see what you mean. That problem exists regardless of whether we
> > introduce a recursive mutex though right?
> 
> Without a recursive mutex, you only hold one lock at a time in each thread.
> 
> > I guess the main issue is the
> > fact that we'd be encouraging sloppy locking practices without
> > addressing the root problem?
> 
> Yeah.  We're basically standing where the Linux kernel stood 10 years
> ago (let's say 2.2 timeframe).  If Linux got this far without recursive
> mutexes, we can at least try. :)

FWIW I was also looking into recursive mutexes for the block layer.
What scared me a little is that they make it tempting to stop thinking
about locks since you know you'll be able to reacquire locks you already
hold.

Especially when converting existing code, I think we need to be rigorous
about exploring every function and thinking about the locks it needs and
which child functions it calls.

Otherwise we'll have code paths hidden away somewhere that were never
truly thought through.

Stefan