On 28/7/21 4:23 pm, Sebastian Huber wrote:
> On 27/07/2021 10:58, [email protected] wrote:
>> From: Chris Johns <[email protected]>
>>
>> - 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
[email protected]
http://lists.rtems.org/mailman/listinfo/devel
