svn commit: r324609 - head/sys/sys

[Mateusz Guzik]

On Sat, Oct 14, 2017 at 2:30 AM, Ian Lepore <ian at freebsd.org> wrote:

> On Fri, 2017-10-13 at 23:38 +0200, Svatopluk Kraus wrote:
> > MTX_UNOWNED is a flag. You did not change its value from 4 to 0, you
> > removed it actually. I have very bad feeling about it. But maybe, it's
> > really possible and in that case, a very good explanation should be
> > provided.
> >
> > Svata
> >
>
> The part that scares me is that DESTROYED may have been defined as
> CONTESTED|UNOWNED for subtle and clever reasons lost in the mists of
> time.  Any of the places that are testing the MTX_CONTESTED bit may
> have been relying somehow on the fact that a destroyed mutex has that
> bit set.
>
> Then again, maybe Mateusz has carefully analyzed all this stuff, and we
> should just relax. :)
>
>
I highly doubt there was anything clever going on. Rather, someone wanted
to somehow denote a destroyed mutex, but did not want to introduce an
additional flag - they are not free since the field shares the value with
the
address of the owner. That is, adding flags requires increasing alignment
of struct thread and that adds to memory usage. Thus they combined the
flags in a way which can never happen under normal circumstances.

mtx_destroy explicitly sets the value to MTX_DESTROYED, so there is no
change here.

MTX_UNOWNED is *not* used as a flag. If you grep the tree you will see
it is only used in direct comparisons.

That said, I reviewed all users again and a minor bug was introduced in
owner_mtx (only used by dtrace). For some reason it grabs the owner,
but the instead of checking if it grabbed anything it checks the flag
(fixed in r324613). So that's my bad.

Justification for the change was provided in the commit message: it makes
.text smaller on amd64 and probably all architectures.

-- 
Mateusz Guzik <mjguzik gmail.com>