On 28/7/21 4:23 pm, Sebastian Huber wrote: > On 27/07/2021 10:58, chr...@rtems.org wrote: >> From: Chris Johns <chr...@rtems.org> >> >> - See `man lockmgr` >> >> - Implement the lock_object and move the RTEMS mutex to that object >> >> - Add debug support to track the locks with gdb >> >> Update #4475 >> --- >> freebsd/sys/sys/_lock.h | 10 +- >> freebsd/sys/sys/_lockmgr.h | 6 + >> freebsd/sys/sys/_mutex.h | 5 - >> freebsd/sys/sys/_rwlock.h | 5 - >> freebsd/sys/sys/_sx.h | 5 - >> rtemsbsd/include/machine/_kernel_lock.h | 2 +- >> rtemsbsd/include/machine/rtems-bsd-mutex.h | 8 +- >> .../include/machine/rtems-bsd-muteximpl.h | 87 ++- >> rtemsbsd/include/machine/rtems-bsd-thread.h | 1 + >> rtemsbsd/rtems/rtems-kernel-epoch.c | 2 +- >> rtemsbsd/rtems/rtems-kernel-jail.c | 10 +- >> rtemsbsd/rtems/rtems-kernel-lockmgr.c | 578 ++++++++++++++++++ >> rtemsbsd/rtems/rtems-kernel-mutex.c | 21 +- >> rtemsbsd/rtems/rtems-kernel-muteximpl.c | 5 +- >> rtemsbsd/rtems/rtems-kernel-rwlock.c | 28 +- >> rtemsbsd/rtems/rtems-kernel-sx.c | 34 +- >> 16 files changed, 724 insertions(+), 83 deletions(-) >> create mode 100644 rtemsbsd/rtems/rtems-kernel-lockmgr.c >> >> diff --git a/freebsd/sys/sys/_lock.h b/freebsd/sys/sys/_lock.h >> index ae10254c..9e3388d5 100644 >> --- a/freebsd/sys/sys/_lock.h >> +++ b/freebsd/sys/sys/_lock.h >> @@ -32,15 +32,19 @@ >> #ifndef _SYS__LOCK_H_ >> #define _SYS__LOCK_H_ >> +#ifdef __rtems__ >> +#include <machine/rtems-bsd-mutex.h> >> +#endif /* __rtems__ */ >> struct lock_object { >> -#ifndef __rtems__ >> const char *lo_name; /* Individual lock name. */ >> u_int lo_flags; >> u_int lo_data; /* General class specific data. */ >> +#ifndef __rtems__ >> struct witness *lo_witness; /* Data for witness. */ >> -#else /* __rtems__ */ >> - unsigned int lo_flags; >> +#endif /* __rtems__ */ >> +#ifdef __rtems__ >> + rtems_bsd_mutex lo_mtx; >> #endif /* __rtems__ */ >> }; > > This change increases the size by 40% from 20 bytes to 28 bytes.
Thanks for pointing this out. I suspect I added them back early and have not looked at it again. > Why do we need the lo_name? I suspect this could be conditional. It is useful when debugging the locks so being able to get the name in the trace is a good thing. > The thread queues have already a name. Does this mean referencing the internals of the RTEMS mutex to get the name? The references to this field are only a few and in rtemsbsd so it looks like this could change. > For what do we need lo_data? This field is used in various places for different type of locks so it needs to stay. > > [...] >> diff --git a/rtemsbsd/include/machine/rtems-bsd-muteximpl.h >> b/rtemsbsd/include/machine/rtems-bsd-muteximpl.h >> index b362e524..2e180b97 100644 >> --- a/rtemsbsd/include/machine/rtems-bsd-muteximpl.h >> +++ b/rtemsbsd/include/machine/rtems-bsd-muteximpl.h >> @@ -50,6 +50,8 @@ >> #include <inttypes.h> >> +#include <rtems/thread.h> >> +#include <rtems/score/thread.h> >> #include <rtems/score/threadimpl.h> >> #include <rtems/score/threadqimpl.h> >> @@ -60,17 +62,48 @@ extern "C" { >> #define RTEMS_BSD_MUTEX_TQ_OPERATIONS \ >> &_Thread_queue_Operations_priority_inherit >> +#if RTEMS_DEBUG >> +/* >> + * Resource tracking. In GDB you can: >> + * >> + * define mutex-owned >> + * set $m = $arg0 >> + * set $c = 0 >> + * while $m != 0 >> + * set $c = $c + 1 >> + * if $m->queue.Queue.owner != 0 >> + * printf "%08x %-40s\n", $m->queue.Queue.owner, $m->queue.Queue.name >> + * end >> + * set $m = $m->mutex_list.tqe_next >> + * end >> + * printf "Total: %d\n", $c >> + * end >> + * >> + * (gdb) mutex-owned _bsd_bsd_mutexlist->tqh_first >> + */ >> +extern TAILQ_HEAD(bsd_mutex_list, rtems_bsd_mutex) bsd_mutexlist; >> +extern rtems_mutex bsd_mutexlist_lock; >> +#endif /* RTEMS_DEBUG */ > > Global symbols should be prefixed with rtems_bsd or _bsd, but not bsd. > > [...] >> diff --git a/rtemsbsd/rtems/rtems-kernel-lockmgr.c >> b/rtemsbsd/rtems/rtems-kernel-lockmgr.c >> new file mode 100644 >> index 00000000..36e7a82f >> --- /dev/null >> +++ b/rtemsbsd/rtems/rtems-kernel-lockmgr.c >> @@ -0,0 +1,578 @@ >> +/** >> + * @file >> + * >> + * @ingroup rtems_bsd_rtems >> + * >> + * @brief TODO. >> + */ >> + >> +/* >> + * Copyright (c) 2020 Chris Johns. All rights reserved. >> + * >> + * Redistribution and use in source and binary forms, with or without >> + * modification, are permitted provided that the following conditions >> + * are met: >> + * 1. Redistributions of source code must retain the above copyright >> + * notice, this list of conditions and the following disclaimer. >> + * 2. Redistributions in binary form must reproduce the above copyright >> + * notice, this list of conditions and the following disclaimer in the >> + * documentation and/or other materials provided with the distribution. >> + * >> + * THIS SOFTWARE IS PROVIDED BY THE AUTHOR AND CONTRIBUTORS ``AS IS'' AND >> + * ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE >> + * IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR >> PURPOSE >> + * ARE DISCLAIMED. IN NO EVENT SHALL THE AUTHOR OR CONTRIBUTORS BE LIABLE >> + * FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR >> CONSEQUENTIAL >> + * DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS >> + * OR SERVICES; LOSS OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION) >> + * HOWEVER CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, >> STRICT >> + * LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY >> + * OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF >> + * SUCH DAMAGE. >> + */ >> + >> +#include <machine/rtems-bsd-kernel-space.h> >> +#include <machine/rtems-bsd-muteximpl.h> >> +#include <machine/rtems-bsd-thread.h> >> + >> +#include <sys/param.h> >> +#include <sys/types.h> >> +#include <sys/systm.h> >> +#include <sys/lock.h> >> +#include <sys/lockmgr.h> >> +#include <sys/mutex.h> >> +#include <sys/proc.h> >> +#include <sys/conf.h> >> + >> +#ifdef DEBUG_LOCKS >> +#define STACK_PRINT(lk) printf("caller: %p\n", (lk)->lk_stack) >> +#define STACK_SAVE(lk) (lk)->lk_stack = __builtin_return_address(0) >> +#define STACK_ZERO(lk) (lk)->lk_stack = NULL >> +#else >> +#define STACK_PRINT(lk) >> +#define STACK_SAVE(lk) >> +#define STACK_ZERO(lk) >> +#endif >> + >> +static void assert_lockmgr(const struct lock_object *lock, int how); >> +static void lock_lockmgr(struct lock_object *lock, uintptr_t how); >> +static uintptr_t unlock_lockmgr(struct lock_object *lock); >> + >> +#define lockmgr_xlocked(lk) \ >> + rtems_bsd_mutex_owned(RTEMS_DECONST(struct lock_object*, >> &lk->lock_object)) >> +#define lockmgr_disowned(lk) \ >> + !rtems_bsd_mutex_owned(RTEMS_DECONST(struct lock_object*, >> &lk->lock_object)) >> + >> +struct lock_class lock_class_lockmgr = { >> + .lc_name = "lockmgr", >> + .lc_flags = LC_RECURSABLE | LC_SLEEPABLE | LC_SLEEPLOCK | LC_UPGRADABLE, >> + .lc_assert = assert_lockmgr, >> +#ifdef DDB >> + .lc_ddb_show = db_show_lockmgr, >> +#endif >> + .lc_lock = lock_lockmgr, >> + .lc_unlock = unlock_lockmgr, >> +#ifdef KDTRACE_HOOKS >> + .lc_owner = owner_lockmgr, >> +#endif >> +}; >> + >> +static void >> +assert_lockmgr(const struct lock_object *lock, int what) >> +{ >> + panic("lockmgr locks do not support assertions"); >> +} > > This is neither the RTEMS style nor the FreeBSD style. At some point in time > it > would be really nice to have a clang-format for RTEMS-specific code. What about adding https://github.com/freebsd/freebsd-src/blob/main/.clang-format ? [ https://reviews.freebsd.org/D20533 ] I had not noticed there was a style in rtemsbsd until you started to mention this now. Is this the reason for the comment about tabs in the namespace file? Chris _______________________________________________ devel mailing list devel@rtems.org http://lists.rtems.org/mailman/listinfo/devel