[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <20170624221630.GV1627@brightrain.aerifal.cx>
Date: Sat, 24 Jun 2017 18:16:30 -0400
From: Rich Felker <dalias@...c.org>
To: musl@...ts.openwall.com
Subject: Re: [PATCH] unify the use of FUTEX_PRIVATE
On Sat, Jun 24, 2017 at 11:31:04PM +0200, Jens Gustedt wrote:
> Hello,
>
> On Sat, 24 Jun 2017 19:51:27 +0200 Jens Gustedt <jens.gustedt@...ia.fr>
> wrote:
>
> > Am 24. Juni 2017 18:40:26 MESZ schrieb Szabolcs Nagy <nsz@...t70.net>:
> > >i think some of these are the same flag
> >
> > I don't think so.
> > For mutexes the sense is "shared" so just the opposite meaning, and
> > for rwlock, too, it seems. For sem it is just a screwed Boolean..
> >
> > I would merely tend to have different names for them, but that's a
> > bit invasive.
>
> We could do something like
>
> /* Shared and private flags for different control structures use the
> same bit to favor optimization. */
> /* for mutex, mutexattr and rwlock */
> #define PSHARED FUTEX_PRIVATE
> /* for sem */
> #define PRIVATE FUTEX_PRIVATE
>
> But then it could also be confusing that condattr uses bit 31 for the
> same purpose, and pthread_cond_t uses a pointer type. This could lead
> to
>
> /* Some shared and private flags for different control structures use
> the same bit to favor optimization. */
> #define MTX_SHARED FUTEX_PRIVATE
> #define MATR_SHARED FUTEX_PRIVATE
> #define RWL_SHARED FUTEX_PRIVATE
> #define RWATR_SHARED 1
> #define CND_SHARED (void*)-1
> #define CATR_SHARED (1<<31)
> /* Maybe replace by 1? */
> #define SEM_PRIVATE FUTEX_PRIVATE
>
> To set the bit, in some places we use bit shift, in some
> multiplication with 128 or 128U. To read the bit some do just masking
> and then use the result as a "Boolean", some use bit shift and then
> %2 to filter out the bit.
>
> Probably I still overlooked some usages, and the naming could still be
> improved.
>
> I could try to produce a patch in that sense, if there is consensus to
> go in that direction.
For now let's just use a macro for FUTEX_PRIVATE. My concern was
mainly about whether there are cases where it's assumed that the 128
or ^128 translates to FUTEX_PRIVATE by arithmetic ops. The __wait,
__timedwait, and __wake interfaces just treat their argument as a
boolean now, so I think the only places there might be remaining
assumptions of that sort is where a flag bit from the sync object is
passed directly into a futex __syscall -- places like the condvar
implementation. You've looked at all this code just now so you
probably have a better idea if there are any such places; if so I
think the assumption that the bits match up should just be replaced by
a conditional (e.g. flags & 128 ? 0 : FUTEX_PRIVATE or such).
Rich
Powered by blists - more mailing lists
Confused about mailing lists and their use?
Read about mailing lists on Wikipedia
and check out these
guidelines on proper formatting of your messages.
