Date: Sat, 14 Oct 2017 02:48:49 +0200 From: Mateusz Guzik <mjguzik@gmail.com> To: Ian Lepore <ian@freebsd.org> Cc: Svatopluk Kraus <skra@freebsd.org>, Mateusz Guzik <mjg@freebsd.org>, "src-committers@freebsd.org" <src-committers@freebsd.org>, "svn-src-all@freebsd.org" <svn-src-all@freebsd.org>, "svn-src-head@freebsd.org" <svn-src-head@freebsd.org> Subject: Re: svn commit: r324609 - head/sys/sys Message-ID: <CAGudoHEZ5hdoaPF-PnPW2D397nkOMa4NX7jDB47EzfN4m9AvPA@mail.gmail.com> In-Reply-To: <1507941050.8386.88.camel@freebsd.org> References: <201710132031.v9DKVueS089009@repo.freebsd.org> <CAFHCsPUzYewo1AaaKFE9LKVdUEw3Tr7c=Qpb=ah9XpwZ-OEEtg@mail.gmail.com> <1507941050.8386.88.camel@freebsd.org>
next in thread | previous in thread | raw e-mail | index | archive | help
On Sat, Oct 14, 2017 at 2:30 AM, Ian Lepore <ian@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>
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?CAGudoHEZ5hdoaPF-PnPW2D397nkOMa4NX7jDB47EzfN4m9AvPA>