mtx_init/lock_init and uninitialized struct mtx

[Dmitry Krivenok]

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 at 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.  I've considered adding a MTX_NEW flag to
> disable this check for places where the developer knows it is safe.  Most
> mutexes are allocated in an already-zero'd structure or BSS as Patrick noted,
> so they are already correct.  It is a trade off between catching double
> initializations and requiring extra M_ZERO flags or bzero() calls in various
> places.
>
>> static int foo()
>> {
>>    struct mtx m;  // Uninitialized auto variable, so it's value is
> undefined.
>>    mtx_init(&m, "my_mutex", NULL, MTX_DEF);
>>    …
>>    // Do something
>>    ...
>>    mtx_destroy(&m);
>>    return 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:
>>
>>  79         /* Check for double-init and zero object. */
>>  80         KASSERT(!lock_initalized(lock), ("lock \"%s\" %p already
>> initialized",
>>  81             name, lock));
>>
>> lock_initialized() just checks that a bit is set in lo_flags field of
>> struct lock_object:
>>
>> 178 #define lock_initalized(lo)     ((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:
>>
>>  37 struct mtx {
>>  38         struct lock_object      lock_object;    /* Common lock
>> properties. */
>>  39         volatile uintptr_t      mtx_lock;       /* Owner and flags. */
>>  40 };
>>
>> 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 at gmail.com
>> skype: krivenok_dmitry
>> jabber: krivenok_dmitry at jabber.ru
>> icq: 242-526-443
>> _______________________________________________
>> freebsd-hackers at freebsd.org mailing list
>> http://lists.freebsd.org/mailman/listinfo/freebsd-hackers
>> To unsubscribe, send any mail to "freebsd-hackers-unsubscribe at freebsd.org"
>>
>
> --
> John Baldwin
>



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