Applied, thanks!
Samuel
David Bidner, le lun. 05 oct. 2026 07:25:42 +0200, a ecrit:
> MACH_LOCK_MON's locate_lock_info() guessed the caller of a lock with
>
> li->caller = *((vm_offset_t *)lock - 1);
>
> which reads an unrelated stack slot and is compiler/codegen dependent.
> Under MACH_LOCK_MON the lock entry points are real functions when
> NCPUS > 1, so the stored value is frequently not a code address and
> `show all slocks` / `lip` print bare or bogus hex such as 0x315(...)
> instead of a caller function name.
>
> Capture the actual return address at the three lock entry points with
> __builtin_return_address(0) and pass it to locate_lock_info(). Resolve
> the stored address with db_printsym(..., DB_STGY_PROC) so the
> LOCK/CALLER column prints name+offset, and always show the lock address
> in parentheses. This only bites the MACH_LOCK_MON diagnostic path:
> locking semantics, statistics and the millisecond timing are unchanged,
> and MACH_LOCK_MON remains off by default.
>
> Based on upstream commit 5e25bd3c ("Add timing info to MACH_LOCK_MON
> lock monitoring").
> ---
> kern/lock_mon.c | 24 +++++++++++-------------
> 1 file changed, 11 insertions(+), 13 deletions(-)
>
> diff --git a/kern/lock_mon.c b/kern/lock_mon.c
> index edc8ae55..5a7e72fa 100644
> --- a/kern/lock_mon.c
> +++ b/kern/lock_mon.c
> @@ -91,8 +91,9 @@ extern spl_t curr_ipl[];
>
>
> struct lock_info *
> -locate_lock_info(lock)
> +locate_lock_info(lock, caller)
> decl_simple_lock_data(, **lock)
> +vm_offset_t caller;
> {
> struct lock_info *li = &(lock_info[HASH_LOCK(*lock)].info[0]);
> int i;
> @@ -103,7 +104,7 @@ decl_simple_lock_data(, **lock)
> return(li);
> } else {
> li->lock = *lock;
> - li->caller = *((vm_offset_t *)lock - 1);
> + li->caller = caller;
> return(li);
> }
> db_printf("out of lock_info slots\n");
> @@ -115,7 +116,8 @@ decl_simple_lock_data(, **lock)
> void simple_lock(lock)
> decl_simple_lock_data(, *lock)
> {
> - struct lock_info *li = locate_lock_info(&lock);
> + struct lock_info *li = locate_lock_info(&lock,
> + (vm_offset_t)
> __builtin_return_address(0));
> int my_cpu = cpu_number();
>
> if (current_thread())
> @@ -134,7 +136,8 @@ decl_simple_lock_data(, *lock)
> int simple_lock_try(lock)
> decl_simple_lock_data(, *lock)
> {
> - struct lock_info *li = locate_lock_info(&lock);
> + struct lock_info *li = locate_lock_info(&lock,
> + (vm_offset_t)
> __builtin_return_address(0));
> int my_cpu = cpu_number();
>
> if (curr_ipl[my_cpu])
> @@ -155,7 +158,8 @@ void simple_unlock(lock)
> decl_simple_lock_data(, *lock)
> {
> time_stamp_t stamp = time_stamp;
> - time_stamp_t *time = &locate_lock_info(&lock)->time;
> + time_stamp_t *time = &locate_lock_info(&lock,
> + (vm_offset_t)
> __builtin_return_address(0))->time;
> unsigned *lock_stack;
>
> *time = stamp - *time;
> @@ -265,20 +269,14 @@ void lock_info_clear(void)
>
> static void print_lock_info(struct lock_info *li)
> {
> - db_addr_t off;
> int sum = li->success + li->fail;
> db_printf("%d %d/%d %d/%d %d/%d %d/%d ", li->success,
> li->fail, (li->fail*100)/sum,
> li->masked, (li->masked*100)/sum,
> li->stack, li->stack/sum,
> li->time, li->time/sum);
> - db_free_symbol(db_search_symbol((db_addr_t) li->lock, 0, &off));
> - if (off < 1024)
> - db_printsym((db_addr_t) li->lock, 0);
> - else {
> - db_printsym(li->caller, 0);
> - db_printf("(%X)", li->lock);
> - }
> + db_printsym(li->caller, DB_STGY_PROC);
> + db_printf("(%X)", li->lock);
> db_printf("\n");
> }
>
> --
> 2.47.3
>
>
--
Samuel
X-Favorit-Cartoon: Calvin and Hobbes
-+- Mail header of Wim van Dorst -+-
