Skip site navigation (1)Skip section navigation (2)
Date:      Thu, 24 Feb 2011 23:43:54 +0300
From:      Dmitry Krivenok <krivenok.dmitry@gmail.com>
To:        John Baldwin <jhb@freebsd.org>
Cc:        freebsd-hackers@freebsd.org
Subject:   Re: mtx_init/lock_init and uninitialized struct mtx
Message-ID:  <AANLkTimnzh7E6xGzoqfkuwa%2B-4u0ccfv%2B3vkWiMsxkVC@mail.gmail.com>
In-Reply-To: <201102241402.21556.jhb@freebsd.org>
References:  <AANLkTimGkjDLO7LCgPMKyDGeWTqKZzzFk=bPzkBCfUn6@mail.gmail.com> <201102241402.21556.jhb@freebsd.org>

next in thread | previous in thread | raw e-mail | index | archive | help
Thanks a lot for your answers!

I'll always explicitly zero stack variables before calling actual
*_init() functions.
Also, it would be great to document this "zeroing" requirement in a
man page for mtx_init()
or simply add a comment in the source.

Dmitry


On Thu, Feb 24, 2011 at 10:02 PM, John Baldwin <jhb@freebsd.org> wrote:
> On Thursday, February 24, 2011 10:47:27 am Dmitry Krivenok wrote:
>> Hello Hackers,
>>
>> Is it allowed to call mtx_init on a mutex defined as an auto variable
>> and not initialized explicitly, i.e.:
>
> It does expect you to zero it first. =A0I've considered adding a MTX_NEW =
flag to
> disable this check for places where the developer knows it is safe. =A0Mo=
st
> mutexes are allocated in an already-zero'd structure or BSS as Patrick no=
ted,
> so they are already correct. =A0It is a trade off between catching double
> initializations and requiring extra M_ZERO flags or bzero() calls in vari=
ous
> places.
>
>> static int foo()
>> {
>> =A0 =A0struct mtx m; =A0// Uninitialized auto variable, so it's value is
> undefined.
>> =A0 =A0mtx_init(&m, "my_mutex", NULL, MTX_DEF);
>> =A0 =A0=85
>> =A0 =A0// Do something
>> =A0 =A0...
>> =A0 =A0mtx_destroy(&m);
>> =A0 =A0return 0;
>> }
>>
>> I encountered a problem with such code on a kernel compiled with
>> INVARIANTS option.
>> The problem is that mtx_init calls lock_init(&m->lock_object) and
>> lock_init, in turn, calls:
>>
>> =A079 =A0 =A0 =A0 =A0 /* Check for double-init and zero object. */
>> =A080 =A0 =A0 =A0 =A0 KASSERT(!lock_initalized(lock), ("lock \"%s\" %p a=
lready
>> initialized",
>> =A081 =A0 =A0 =A0 =A0 =A0 =A0 name, lock));
>>
>> lock_initialized() just checks that a bit is set in lo_flags field of
>> struct lock_object:
>>
>> 178 #define lock_initalized(lo) =A0 =A0 ((lo)->lo_flags & LO_INITIALIZED=
)
>>
>> However, the structure containing this field is never initialized
>> (neither in mtx_init nor in lock_init).
>> So, assuming that the mutex was defined as auto variable, the content
>> of lock_object field of struct mtx
>> is also undefined:
>>
>> =A037 struct mtx {
>> =A038 =A0 =A0 =A0 =A0 struct lock_object =A0 =A0 =A0lock_object; =A0 =A0=
/* Common lock
>> properties. */
>> =A039 =A0 =A0 =A0 =A0 volatile uintptr_t =A0 =A0 =A0mtx_lock; =A0 =A0 =
=A0 /* Owner and flags. */
>> =A040 };
>>
>> In some cases, the initial value of lo_flags _may_ have the
>> "initialized" bit set and KASSERT will call panic.
>>
>> Is it user's responsibility to properly (how exactly?) initialize
>> struct mtx, e.g.
>> memset(&m, '\0', sizeof(struct mtx));
>>
>> Or should mtx_init() explicitly initialize all fields of struct mtx?
>>
>> Thanks in advance!
>>
>> --
>> Sincerely yours, Dmitry V. Krivenok
>> e-mail: krivenok.dmitry@gmail.com
>> skype: krivenok_dmitry
>> jabber: krivenok_dmitry@jabber.ru
>> icq: 242-526-443
>> _______________________________________________
>> freebsd-hackers@freebsd.org mailing list
>> http://lists.freebsd.org/mailman/listinfo/freebsd-hackers
>> To unsubscribe, send any mail to "freebsd-hackers-unsubscribe@freebsd.or=
g"
>>
>
> --
> John Baldwin
>



--=20
Sincerely yours, Dmitry V. Krivenok
e-mail: krivenok.dmitry@gmail.com
skype: krivenok_dmitry
jabber: krivenok_dmitry@jabber.ru
icq: 242-526-443



Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?AANLkTimnzh7E6xGzoqfkuwa%2B-4u0ccfv%2B3vkWiMsxkVC>