linux-kernel - Re: [PATCH 3/6] drm/gem: use new ww_mutex_(un)lock_for

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Fri, 14 Jun 2019 15:21:53 +0200
From:   Peter Zijlstra <peterz@...radead.org>
To:     christian.koenig@....com
Cc:     daniel@...ll.ch, l.stach@...gutronix.de,
        linux+etnaviv@...linux.org.uk, christian.gmeiner@...il.com,
        yuq825@...il.com, eric@...olt.net, thellstrom@...are.com,
        dri-devel@...ts.freedesktop.org, linux-kernel@...r.kernel.org,
        etnaviv@...ts.freedesktop.org, lima@...ts.freedesktop.org
Subject: Re: [PATCH 3/6] drm/gem: use new ww_mutex_(un)lock_for_each macros

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.