> Date: Tue, 3 Apr 2018 11:16:45 +0200
> From: Martin Pieuchot <[email protected]>
> 
> Here's a diff to switch hppa to the MI mutex implementation.  I'm
> looking for testers, as I don't own such hardware.
> 
> Note that our MI mutex implementation relies on atomic_cas_ptr(9).  On
> hppa all the calls to this function are serialized on a single lock. I
> don't believe it will introduce contention at this point since the
> KERNEL_LOCK() is still the problem.  However if it becomes a bottleneck
> we could use multiple locks in a small hash table.

I don't have such hardware either, but I'm not in favour of doing
this.  PA-RISC is "challenged" but at least the locking primitive it
supplies is suitable for implementing spin locks.  Emulating CAS with
spin locks to implement a spin lock feels so wrong.  There are also
some concerns about guarantees of forward progress in such an
implementation.

> Index: arch/hppa/conf/files.hppa
> ===================================================================
> RCS file: /cvs/src/sys/arch/hppa/conf/files.hppa,v
> retrieving revision 1.97
> diff -u -p -r1.97 files.hppa
> --- arch/hppa/conf/files.hppa 5 Jun 2017 18:59:07 -0000       1.97
> +++ arch/hppa/conf/files.hppa 12 Feb 2018 10:08:57 -0000
> @@ -298,7 +298,6 @@ file      arch/hppa/hppa/fpu.c
>  file arch/hppa/hppa/ipi.c                    multiprocessor
>  file arch/hppa/hppa/lock_machdep.c           multiprocessor
>  file arch/hppa/hppa/machdep.c
> -file arch/hppa/hppa/mutex.c
>  file arch/hppa/hppa/pmap.c
>  file arch/hppa/hppa/process_machdep.c
>  file arch/hppa/hppa/sys_machdep.c
> Index: arch/hppa/hppa/genassym.cf
> ===================================================================
> RCS file: /cvs/src/sys/arch/hppa/hppa/genassym.cf,v
> retrieving revision 1.47
> diff -u -p -r1.47 genassym.cf
> --- arch/hppa/hppa/genassym.cf        9 Feb 2015 08:20:13 -0000       1.47
> +++ arch/hppa/hppa/genassym.cf        12 Feb 2018 10:15:56 -0000
> @@ -45,7 +45,7 @@ include <uvm/uvm_extern.h>
>  include <machine/cpu.h>
>  include <machine/frame.h>
>  include <machine/iomod.h>
> -include <machine/mutex.h>
> +include <sys/mutex.h>
>  include <machine/pmap.h>
>  include <machine/psl.h>
>  include <machine/pte.h>
> Index: arch/hppa/hppa/ipi.c
> ===================================================================
> RCS file: /cvs/src/sys/arch/hppa/hppa/ipi.c,v
> retrieving revision 1.5
> diff -u -p -r1.5 ipi.c
> --- arch/hppa/hppa/ipi.c      8 Sep 2017 05:36:51 -0000       1.5
> +++ arch/hppa/hppa/ipi.c      12 Feb 2018 10:16:01 -0000
> @@ -25,7 +25,7 @@
>  #include <machine/fpu.h>
>  #include <machine/iomod.h>
>  #include <machine/intr.h>
> -#include <machine/mutex.h>
> +#include <sys/mutex.h>
>  #include <machine/reg.h>
>  
>  void hppa_ipi_nop(void);
> Index: arch/hppa/hppa/mutex.c
> ===================================================================
> RCS file: arch/hppa/hppa/mutex.c
> diff -N arch/hppa/hppa/mutex.c
> --- arch/hppa/hppa/mutex.c    20 Apr 2017 13:57:29 -0000      1.16
> +++ /dev/null 1 Jan 1970 00:00:00 -0000
> @@ -1,156 +0,0 @@
> -/*   $OpenBSD: mutex.c,v 1.16 2017/04/20 13:57:29 visa Exp $ */
> -
> -/*
> - * Copyright (c) 2004 Artur Grabowski <[email protected]>
> - * 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. The name of the author may not be used to endorse or promote products
> - *    derived from this software without specific prior written permission. 
> - *
> - * THIS SOFTWARE IS PROVIDED ``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 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 <sys/param.h>
> -#include <sys/mutex.h>
> -#include <sys/systm.h>
> -#include <sys/atomic.h>
> -
> -#include <machine/intr.h>
> -
> -#include <ddb/db_output.h>
> -
> -int __mtx_enter_try(struct mutex *);
> -
> -#ifdef MULTIPROCESSOR
> -/* Note: lock must be 16-byte aligned. */
> -#define __mtx_lock(mtx) ((int *)(((vaddr_t)mtx->mtx_lock + 0xf) & ~0xf))
> -#endif
> -
> -void
> -__mtx_init(struct mutex *mtx, int wantipl)
> -{
> -#ifdef MULTIPROCESSOR
> -     mtx->mtx_lock[0] = 1;
> -     mtx->mtx_lock[1] = 1;
> -     mtx->mtx_lock[2] = 1;
> -     mtx->mtx_lock[3] = 1;
> -#endif
> -     mtx->mtx_wantipl = wantipl;
> -     mtx->mtx_oldipl = IPL_NONE;
> -     mtx->mtx_owner = NULL;
> -}
> -
> -#ifdef MULTIPROCESSOR
> -void
> -__mtx_enter(struct mutex *mtx)
> -{
> -     while (__mtx_enter_try(mtx) == 0)
> -             ;
> -}
> -
> -int
> -__mtx_enter_try(struct mutex *mtx)
> -{
> -     struct cpu_info *ci = curcpu();
> -     volatile int *lock = __mtx_lock(mtx);
> -     int ret;
> -     int s;
> -
> -     if (mtx->mtx_wantipl != IPL_NONE)
> -             s = splraise(mtx->mtx_wantipl);
> -
> -#ifdef DIAGNOSTIC
> -     if (__predict_false(mtx->mtx_owner == ci))
> -             panic("mtx %p: locking against myself", mtx);
> -#endif
> -
> -     asm volatile (
> -             "ldcws      0(%2), %0"
> -             : "=&r" (ret), "+m" (lock)
> -             : "r" (lock)
> -     );
> -
> -     if (ret) {
> -             membar_enter();
> -             mtx->mtx_owner = ci;
> -             if (mtx->mtx_wantipl != IPL_NONE)
> -                     mtx->mtx_oldipl = s;
> -#ifdef DIAGNOSTIC
> -             ci->ci_mutex_level++;
> -#endif
> -
> -             return (1);
> -     }
> -
> -     if (mtx->mtx_wantipl != IPL_NONE)
> -             splx(s);
> -
> -     return (0);
> -}
> -#else
> -void
> -__mtx_enter(struct mutex *mtx)
> -{
> -     struct cpu_info *ci = curcpu();
> -
> -#ifdef DIAGNOSTIC
> -     if (__predict_false(mtx->mtx_owner == ci))
> -             panic("mtx %p: locking against myself", mtx);
> -#endif
> -
> -     if (mtx->mtx_wantipl != IPL_NONE)
> -             mtx->mtx_oldipl = splraise(mtx->mtx_wantipl);
> -
> -     mtx->mtx_owner = ci;
> -
> -#ifdef DIAGNOSTIC
> -     ci->ci_mutex_level++;
> -#endif
> -}
> -
> -int
> -__mtx_enter_try(struct mutex *mtx)
> -{
> -     __mtx_enter(mtx);
> -     return (1);
> -}
> -#endif
> -
> -void
> -__mtx_leave(struct mutex *mtx)
> -{
> -#ifdef MULTIPROCESSOR
> -     volatile int *lock = __mtx_lock(mtx);
> -#endif
> -     int s;
> -
> -     MUTEX_ASSERT_LOCKED(mtx);
> -
> -#ifdef DIAGNOSTIC
> -     curcpu()->ci_mutex_level--;
> -#endif
> -     s = mtx->mtx_oldipl;
> -     mtx->mtx_owner = NULL;
> -#ifdef MULTIPROCESSOR
> -     membar_exit();
> -     *lock = 1;
> -#endif
> -
> -     if (mtx->mtx_wantipl != IPL_NONE)
> -             splx(s);
> -}
> Index: arch/hppa/include/cpu.h
> ===================================================================
> RCS file: /cvs/src/sys/arch/hppa/include/cpu.h,v
> retrieving revision 1.90
> diff -u -p -r1.90 cpu.h
> --- arch/hppa/include/cpu.h   18 May 2017 15:41:59 -0000      1.90
> +++ arch/hppa/include/cpu.h   12 Feb 2018 10:16:09 -0000
> @@ -70,7 +70,7 @@
>  #include <sys/queue.h>
>  #include <sys/sched.h>
>  
> -#include <machine/mutex.h>
> +#include <sys/mutex.h>
>  
>  /*
>   * Note that the alignment of ci_trap_save is important since we want to keep
> Index: arch/hppa/include/mutex.h
> ===================================================================
> RCS file: /cvs/src/sys/arch/hppa/include/mutex.h,v
> retrieving revision 1.9
> diff -u -p -r1.9 mutex.h
> --- arch/hppa/include/mutex.h 13 Jan 2018 15:18:11 -0000      1.9
> +++ arch/hppa/include/mutex.h 12 Feb 2018 10:11:34 -0000
> @@ -1,93 +1,3 @@
>  /*   $OpenBSD: mutex.h,v 1.9 2018/01/13 15:18:11 mpi Exp $   */
>  
> -/*
> - * Copyright (c) 2004 Artur Grabowski <[email protected]>
> - * 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. The name of the author may not be used to endorse or promote products
> - *    derived from this software without specific prior written permission. 
> - *
> - * THIS SOFTWARE IS PROVIDED ``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 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. 
> - */
> -
> -#ifndef _MACHINE_MUTEX_H_
> -#define _MACHINE_MUTEX_H_
> -
> -#include <sys/_lock.h>
> -
> -#define      MUTEX_UNLOCKED  { 1, 1, 1, 1 }
> -
> -/* Note: mtx_lock must be 16-byte aligned. */
> -struct mutex {
> -#ifdef MULTIPROCESSOR
> -     volatile int mtx_lock[4];
> -#endif
> -     int mtx_wantipl;
> -     int mtx_oldipl;
> -     volatile void *mtx_owner;
> -#ifdef WITNESS
> -     struct lock_object mtx_lock_obj;
> -#endif
> -};
> -
> -/*
> - * To prevent lock ordering problems with the kernel lock, we need to
> - * make sure we block all interrupts that can grab the kernel lock.
> - * The simplest way to achieve this is to make sure mutexes always
> - * raise the interrupt priority level to the highest level that has
> - * interrupts that grab the kernel lock.
> - */
> -#ifdef MULTIPROCESSOR
> -#define __MUTEX_IPL(ipl) \
> -    (((ipl) > IPL_NONE && (ipl) < IPL_MPFLOOR) ? IPL_MPFLOOR : (ipl))
> -#ifdef WITNESS
> -#define MUTEX_INITIALIZER_FLAGS(ipl, name, flags) \
> -     { MUTEX_UNLOCKED, __MUTEX_IPL((ipl)), 0, NULL, \
> -       MTX_LO_INITIALIZER(name, flags) }
> -#else /* WITNESS */
> -#define MUTEX_INITIALIZER_FLAGS(ipl, name, flags) \
> -     { MUTEX_UNLOCKED, __MUTEX_IPL((ipl)), 0, NULL }
> -#endif /* WITNESS */
> -#else /* MULTIPROCESSOR */
> -#define __MUTEX_IPL(ipl) (ipl)
> -#define MUTEX_INITIALIZER_FLAGS(ipl, name, flags) \
> -     { __MUTEX_IPL((ipl)), 0, NULL }
> -#endif /* MULTIPROCESSOR */
> -
> -void __mtx_init(struct mutex *, int);
> -#define _mtx_init(mtx, ipl) __mtx_init((mtx), __MUTEX_IPL((ipl)))
> -
> -#ifdef DIAGNOSTIC
> -#define MUTEX_ASSERT_LOCKED(mtx) do {                                        
> \
> -     if ((mtx)->mtx_owner != curcpu())                               \
> -             panic("mutex %p not held in %s", (mtx), __func__);      \
> -} while (0)
> -
> -#define MUTEX_ASSERT_UNLOCKED(mtx) do {                                      
> \
> -     if ((mtx)->mtx_owner == curcpu())                               \
> -             panic("mutex %p held in %s", (mtx), __func__);          \
> -} while (0)
> -#else
> -#define MUTEX_ASSERT_LOCKED(mtx) do { } while (0)
> -#define MUTEX_ASSERT_UNLOCKED(mtx) do { } while (0)
> -#endif
> -
> -#define MUTEX_LOCK_OBJECT(mtx)       (&(mtx)->mtx_lock_obj)
> -#define MUTEX_OLDIPL(mtx)    (mtx)->mtx_oldipl
> -
> -#endif
> +#define __USE_MI_MUTEX
> 
> 

Reply via email to