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>