Re: [PATCH 3/6] drm/gem: use new ww_mutex_(un)lock_for_each macros

From: Peter Zijlstra <peterz@infradead.org>
To: christian.koenig@amd.com
Cc: daniel@ffwll.ch, l.stach@pengutronix.de,
	linux+etnaviv@armlinux.org.uk, christian.gmeiner@gmail.com,
	yuq825@gmail.com, eric@anholt.net, thellstrom@vmware.com,
	dri-devel@lists.freedesktop.org, linux-kernel@vger.kernel.org,
	etnaviv@lists.freedesktop.org, lima@lists.freedesktop.org
Subject: Re: [PATCH 3/6] drm/gem: use new ww_mutex_(un)lock_for_each macros
Date: Fri, 14 Jun 2019 15:21:53 +0200	[thread overview]
Message-ID: <20190614132153.GR3436@hirez.programming.kicks-ass.net> (raw)
In-Reply-To: <6f2084ae-61d5-8cb9-c975-901456eed7e3@gmail.com>

On Fri, Jun 14, 2019 at 03:06:10PM +0200, Christian König wrote:
> Am 14.06.19 um 14:59 schrieb Peter Zijlstra:
> > +#define ww_mutex_lock_for_each(loop, pos, contended, ret, intr, ctx)   \
> > +       for (contended = ERR_PTR(-ENOENT); ({                           \
> > +               __label__ relock, next;                                 \
> > +               ret = -ENOENT;                                          \
> > +               if (contended == ERR_PTR(-ENOENT))                      \
> > +                       contended = NULL;                               \
> > +               else if (contended == ERR_PTR(-EDEADLK))                \
> > +                       contended = (pos);                              \
> > +               else                                                    \
> > +                       goto next;                                      \
> > +               loop {                                                  \
> > +                       if (contended == (pos)) {                       \
> > +                               contended = NULL;                       \
> > +                               continue;                               \
> > +                       }                                               \
> > +relock:                                                                        \
> > +                       ret = !(intr) ? ww_mutex_lock(pos, ctx) :       \
> > +                               ww_mutex_lock_interruptible(pos, ctx);  \
> > +                       if (ret == -EDEADLK) {                          \
> > +                               ww_mutex_unlock_for_each(loop, pos,     \
> > +                                                        contended);    \
> > +                               contended = ERR_PTR(-EDEADLK);          \
> > +                               goto relock;                            \
> > 
> > while relock here continues where it left of and doesn't restart @loop.
> > Or am I reading this monstrosity the wrong way?
> 
> contended = ERR_PTR(-EDEADLK) makes sure that the whole loop is restarted
> after retrying to acquire the lock.
> 
> See the "if" above "loop".

ARGH, the loop inside the loop... Yeah, maybe, brain hurts.