[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <CA+55aFwkqwO8RFoRQZz2DTe-e-jgg2Vw1MjPGp4+i5wmno4vMw@mail.gmail.com>
Date: Fri, 7 Oct 2016 08:58:43 -0700
From: Linus Torvalds <torvalds@...ux-foundation.org>
To: Peter Zijlstra <peterz@...radead.org>
Cc: Waiman Long <waiman.long@....com>, Jason Low <jason.low2@....com>,
Ding Tianhong <dingtianhong@...wei.com>,
Thomas Gleixner <tglx@...utronix.de>,
Will Deacon <Will.Deacon@....com>,
Ingo Molnar <mingo@...hat.com>,
Imre Deak <imre.deak@...el.com>,
Linux Kernel Mailing List <linux-kernel@...r.kernel.org>,
Davidlohr Bueso <dave@...olabs.net>,
Tim Chen <tim.c.chen@...ux.intel.com>,
Terry Rudd <terry.rudd@....com>,
"Paul E. McKenney" <paulmck@...ibm.com>,
Chris Wilson <chris@...is-wilson.co.uk>,
Daniel Vetter <daniel.vetter@...ll.ch>,
Rob Clark <robdclark@...il.com>
Subject: Re: [PATCH -v4 1/8] locking/drm: Kill mutex trickery
On Fri, Oct 7, 2016 at 8:43 AM, Peter Zijlstra <peterz@...radead.org> wrote:
>
> So something like the below would serve as a replacement for your
> previous hacks. Is this API something acceptable to people? Ingo,
> Thomas?
Ugh. I think the concept is fine, but can we place make these enum's
be all upper case or something to make them really stand out visually.
Also, it's a bit subtle how this depends on the success/failure enums
to just have the same values as the return value of the non-try mutex.
So a comment about why that particular numbering is important above
the enum definition, please?
You _kind_ of imply that magic numbering requirementby explicitly
using " = 0" and " = 1", but quite frankly, in many ways that's really
nasty, because I can see somebody looking at that enum definition and
saying "that makes no sense: it sets *two* of the three values
explicitly, and the values it sets explicitly are the same that it
would get implicitly _anyway_, so I'll just clean this up".
So I really think that enum needs a comment on the forced choice of
values, and making them all caps would make the users more obvious, I
think.
The other choice would be to just make the choices be negative (==
recursive), zero (== failed) or positive (== got lock), which allows
for the same value re-use for the non-recursive case, and you could
avoid the enum entirely.
Linus
Powered by blists - more mailing lists