hppa mutex ipl diff

classic Classic list List threaded Threaded
4 messages Options
Reply | Threaded
Open this post in threaded view
|

hppa mutex ipl diff

Mark Kettenis
Here is a diff that makes mutexes raise their ipl to the highest level
that has interrupts that take the kernel lock.  This is necessary for
the work dlg@ has been doing on making subsystems run without the
kernel lock.

This needs to be tested on an MP system, and unfortunately that's
something I cannot do.


Index: include/mutex.h
===================================================================
RCS file: /cvs/src/sys/arch/hppa/include/mutex.h,v
retrieving revision 1.4
diff -u -p -r1.4 mutex.h
--- include/mutex.h 10 Jan 2010 04:07:18 -0000 1.4
+++ include/mutex.h 29 Jan 2014 10:01:52 -0000
@@ -39,9 +39,24 @@ struct mutex {
  void *mtx_owner;
 };
 
-void mtx_init(struct mutex *, int);
+/*
+ * 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_AUDIO) ? IPL_AUDIO : (ipl))
+#else
+#define __MUTEX_IPL(ipl) (ipl)
+#endif
 
-#define MUTEX_INITIALIZER(ipl) { MUTEX_UNLOCKED, (ipl), 0, NULL }
+#define MUTEX_INITIALIZER(ipl) { MUTEX_UNLOCKED, __MUTEX_IPL((ipl)), 0, NULL }
+
+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 { \
Index: hppa/mutex.c
===================================================================
RCS file: /cvs/src/sys/arch/hppa/hppa/mutex.c,v
retrieving revision 1.11
diff -u -p -r1.11 mutex.c
--- hppa/mutex.c 20 Apr 2011 16:10:53 -0000 1.11
+++ hppa/mutex.c 29 Jan 2014 10:01:52 -0000
@@ -50,7 +50,7 @@ try_lock(struct mutex *mtx)
 }
 
 void
-mtx_init(struct mutex *mtx, int wantipl)
+__mtx_init(struct mutex *mtx, int wantipl)
 {
  mtx->mtx_lock[0] = 1;
  mtx->mtx_lock[1] = 1;

Reply | Threaded
Open this post in threaded view
|

Re: hppa mutex ipl diff

Juan Francisco Cantero Hurtado
I've been running a dpb build for hours and I've not seen problems.

On Wed, Jan 29, 2014 at 11:06:40AM +0100, Mark Kettenis wrote:

> Here is a diff that makes mutexes raise their ipl to the highest level
> that has interrupts that take the kernel lock.  This is necessary for
> the work dlg@ has been doing on making subsystems run without the
> kernel lock.
>
> This needs to be tested on an MP system, and unfortunately that's
> something I cannot do.
>
>
> Index: include/mutex.h
> ===================================================================
> RCS file: /cvs/src/sys/arch/hppa/include/mutex.h,v
> retrieving revision 1.4
> diff -u -p -r1.4 mutex.h
> --- include/mutex.h 10 Jan 2010 04:07:18 -0000 1.4
> +++ include/mutex.h 29 Jan 2014 10:01:52 -0000
> @@ -39,9 +39,24 @@ struct mutex {
>   void *mtx_owner;
>  };
>  
> -void mtx_init(struct mutex *, int);
> +/*
> + * 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_AUDIO) ? IPL_AUDIO : (ipl))
> +#else
> +#define __MUTEX_IPL(ipl) (ipl)
> +#endif
>  
> -#define MUTEX_INITIALIZER(ipl) { MUTEX_UNLOCKED, (ipl), 0, NULL }
> +#define MUTEX_INITIALIZER(ipl) { MUTEX_UNLOCKED, __MUTEX_IPL((ipl)), 0, NULL }
> +
> +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 { \
> Index: hppa/mutex.c
> ===================================================================
> RCS file: /cvs/src/sys/arch/hppa/hppa/mutex.c,v
> retrieving revision 1.11
> diff -u -p -r1.11 mutex.c
> --- hppa/mutex.c 20 Apr 2011 16:10:53 -0000 1.11
> +++ hppa/mutex.c 29 Jan 2014 10:01:52 -0000
> @@ -50,7 +50,7 @@ try_lock(struct mutex *mtx)
>  }
>  
>  void
> -mtx_init(struct mutex *mtx, int wantipl)
> +__mtx_init(struct mutex *mtx, int wantipl)
>  {
>   mtx->mtx_lock[0] = 1;
>   mtx->mtx_lock[1] = 1;
>

--
Juan Francisco Cantero Hurtado http://juanfra.info

Reply | Threaded
Open this post in threaded view
|

Re: hppa mutex ipl diff

David Gwynne-5
In reply to this post by Mark Kettenis
is there anything in particular you're supposed to look for to see if it is or isnt working?

either it locks up the machine (deadlock) or doesnt?

On 29 Jan 2014, at 8:06 pm, Mark Kettenis <[hidden email]> wrote:

> Here is a diff that makes mutexes raise their ipl to the highest level
> that has interrupts that take the kernel lock.  This is necessary for
> the work dlg@ has been doing on making subsystems run without the
> kernel lock.
>
> This needs to be tested on an MP system, and unfortunately that's
> something I cannot do.
>
>
> Index: include/mutex.h
> ===================================================================
> RCS file: /cvs/src/sys/arch/hppa/include/mutex.h,v
> retrieving revision 1.4
> diff -u -p -r1.4 mutex.h
> --- include/mutex.h 10 Jan 2010 04:07:18 -0000 1.4
> +++ include/mutex.h 29 Jan 2014 10:01:52 -0000
> @@ -39,9 +39,24 @@ struct mutex {
> void *mtx_owner;
> };
>
> -void mtx_init(struct mutex *, int);
> +/*
> + * 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_AUDIO) ? IPL_AUDIO : (ipl))
> +#else
> +#define __MUTEX_IPL(ipl) (ipl)
> +#endif
>
> -#define MUTEX_INITIALIZER(ipl) { MUTEX_UNLOCKED, (ipl), 0, NULL }
> +#define MUTEX_INITIALIZER(ipl) { MUTEX_UNLOCKED, __MUTEX_IPL((ipl)), 0, NULL }
> +
> +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 { \
> Index: hppa/mutex.c
> ===================================================================
> RCS file: /cvs/src/sys/arch/hppa/hppa/mutex.c,v
> retrieving revision 1.11
> diff -u -p -r1.11 mutex.c
> --- hppa/mutex.c 20 Apr 2011 16:10:53 -0000 1.11
> +++ hppa/mutex.c 29 Jan 2014 10:01:52 -0000
> @@ -50,7 +50,7 @@ try_lock(struct mutex *mtx)
> }
>
> void
> -mtx_init(struct mutex *mtx, int wantipl)
> +__mtx_init(struct mutex *mtx, int wantipl)
> {
> mtx->mtx_lock[0] = 1;
> mtx->mtx_lock[1] = 1;
>


Reply | Threaded
Open this post in threaded view
|

Re: hppa mutex ipl diff

Mark Kettenis
> From: David Gwynne <[hidden email]>
> Date: Thu, 30 Jan 2014 09:59:15 +1000
>
> is there anything in particular you're supposed to look for to see if it is or isnt working?
>
> either it locks up the machine (deadlock) or doesnt?

Something like that.

> On 29 Jan 2014, at 8:06 pm, Mark Kettenis <[hidden email]> wrote:
>
> > Here is a diff that makes mutexes raise their ipl to the highest level
> > that has interrupts that take the kernel lock.  This is necessary for
> > the work dlg@ has been doing on making subsystems run without the
> > kernel lock.
> >
> > This needs to be tested on an MP system, and unfortunately that's
> > something I cannot do.
> >
> >
> > Index: include/mutex.h
> > ===================================================================
> > RCS file: /cvs/src/sys/arch/hppa/include/mutex.h,v
> > retrieving revision 1.4
> > diff -u -p -r1.4 mutex.h
> > --- include/mutex.h 10 Jan 2010 04:07:18 -0000 1.4
> > +++ include/mutex.h 29 Jan 2014 10:01:52 -0000
> > @@ -39,9 +39,24 @@ struct mutex {
> > void *mtx_owner;
> > };
> >
> > -void mtx_init(struct mutex *, int);
> > +/*
> > + * 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_AUDIO) ? IPL_AUDIO : (ipl))
> > +#else
> > +#define __MUTEX_IPL(ipl) (ipl)
> > +#endif
> >
> > -#define MUTEX_INITIALIZER(ipl) { MUTEX_UNLOCKED, (ipl), 0, NULL }
> > +#define MUTEX_INITIALIZER(ipl) { MUTEX_UNLOCKED, __MUTEX_IPL((ipl)), 0, NULL }
> > +
> > +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 { \
> > Index: hppa/mutex.c
> > ===================================================================
> > RCS file: /cvs/src/sys/arch/hppa/hppa/mutex.c,v
> > retrieving revision 1.11
> > diff -u -p -r1.11 mutex.c
> > --- hppa/mutex.c 20 Apr 2011 16:10:53 -0000 1.11
> > +++ hppa/mutex.c 29 Jan 2014 10:01:52 -0000
> > @@ -50,7 +50,7 @@ try_lock(struct mutex *mtx)
> > }
> >
> > void
> > -mtx_init(struct mutex *mtx, int wantipl)
> > +__mtx_init(struct mutex *mtx, int wantipl)
> > {
> > mtx->mtx_lock[0] = 1;
> > mtx->mtx_lock[1] = 1;
> >
>
>
>