[PATCH rtems-libbsd 3/7] sys/kern: Add lockmgr support
Chris Johns
chrisj at rtems.org
Wed Jul 28 07:58:34 UTC 2021
On 28/7/21 4:23 pm, Sebastian Huber wrote:
> On 27/07/2021 10:58, chrisj at rtems.org wrote:
>> From: Chris Johns <chrisj at 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
More information about the devel
mailing list