* [PATCH 2/3] PowerPC: Add adaptive elision to rwlocks
@ 2014-11-07 17:13 Adhemerval Zanella
2014-11-27 17:37 ` Adhemerval Zanella
0 siblings, 1 reply; 4+ messages in thread
From: Adhemerval Zanella @ 2014-11-07 17:13 UTC (permalink / raw)
To: GNU C. Library
This patch adds support for lock elision using ISA 2.07 hardware
transactional memory for rwlocks. The logic is similar to the
one presented in pthread_mutex lock elision.
--
* sysdeps/powerpc/nptl/elide.h: New file: generic lock elision support
for powerpc.
* sysdeps/unix/sysv/linux/powerpc/bits/pthreadtypes.h
[pthread_rwlock_t] (__pad1): Change size to 7 bytes in 64 bits case
and remove it for 32 bits case.
[pthread_rwlock_t] (__rwelision): New field for lock elision.
(__PTHREAD_RWLOCK_ELISION_EXTRA): Adjust for new lock elision field
initialization.
* sysdeps/unix/sysv/linux/powerpc/elision-conf.c (elision_init):
Disable lock elision with rdlocks if elision is not available.
---
diff --git a/sysdeps/powerpc/nptl/elide.h b/sysdeps/powerpc/nptl/elide.h
new file mode 100644
index 0000000..5cc47a3
--- /dev/null
+++ b/sysdeps/powerpc/nptl/elide.h
@@ -0,0 +1,111 @@
+/* elide.h: Generic lock elision support for powerpc.
+ Copyright (C) 2014 Free Software Foundation, Inc.
+ This file is part of the GNU C Library.
+
+ The GNU C Library is free software; you can redistribute it and/or
+ modify it under the terms of the GNU Lesser General Public
+ License as published by the Free Software Foundation; either
+ version 2.1 of the License, or (at your option) any later version.
+
+ The GNU C Library is distributed in the hope that it will be useful,
+ but WITHOUT ANY WARRANTY; without even the implied warranty of
+ MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
+ Lesser General Public License for more details.
+
+ You should have received a copy of the GNU Lesser General Public
+ License along with the GNU C Library; if not, see
+ <http://www.gnu.org/licenses/>. */
+
+#ifndef ELIDE_PPC_H
+# define ELIDE_PPC_H
+
+#ifdef ENABLE_LOCK_ELISION
+# include <htm.h>
+# include <elision-conf.h>
+
+/* Returns true if lock defined by IS_LOCK_FREE was elided.
+ ADAPT_COUNT is a pointer to per-lock state variable. */
+
+static inline bool
+__elide_lock (uint8_t *adapt_count, int is_lock_free)
+{
+ if (*adapt_count > 0)
+ {
+ (*adapt_count)--;
+ return false;
+ }
+
+ for (int i = __elision_aconf.try_tbegin; i > 0; i--)
+ {
+ if (__builtin_tbegin (0))
+ {
+ if (is_lock_free)
+ return true;
+ /* Lock was busy. */
+ __builtin_tabort (_ABORT_LOCK_BUSY);
+ }
+ else
+ {
+ /* A persistent failure indicates that a retry will probably
+ result in another failure. Use normal locking now and
+ for the next couple of calls. */
+ if (_TEXASRU_FAILURE_PERSISTENT (__builtin_get_texasru ()))
+ {
+ if (__elision_aconf.skip_lock_internal_abort > 0)
+ *adapt_count = __elision_aconf.skip_lock_internal_abort;
+ break;
+ }
+ /* Same logic as above, but for for a number of temporary failures in a
+ a row. */
+ else if (__elision_aconf.skip_lock_out_of_tbegin_retries > 0
+ && __elision_aconf.try_tbegin > 0)
+ *adapt_count = __elision_aconf.skip_lock_out_of_tbegin_retries;
+ }
+ }
+
+ return false;
+}
+
+# define ELIDE_LOCK(adapt_count, is_lock_free) \
+ __elide_lock (&(adapt_count), is_lock_free)
+
+
+static inline bool
+__elide_trylock (uint8_t *adapt_count, int is_lock_free, int write)
+{
+ if (__elision_aconf.try_tbegin > 0)
+ {
+ if (write)
+ __builtin_tabort (_ABORT_NESTED_TRYLOCK);
+ return __elide_lock (adapt_count, is_lock_free);
+ }
+ return false;
+}
+
+# define ELIDE_TRYLOCK(adapt_count, is_lock_free, write) \
+ __elide_trylock (&(adapt_count), is_lock_free, write)
+
+
+static inline bool
+__elide_unlock (int is_lock_free)
+{
+ if (is_lock_free)
+ {
+ __builtin_tend (0);
+ return true;
+ }
+ return false;
+}
+
+# define ELIDE_UNLOCK(is_lock_free) \
+ __elide_unlock (is_lock_free)
+
+# else
+
+# define ELIDE_LOCK(adapt_count, is_lock_free) 0
+# define ELIDE_TRYLOCK(adapt_count, is_lock_free, write) 0
+# define ELIDE_UNLOCK(is_lock_free) 0
+
+#endif /* ENABLE_LOCK_ELISION */
+
+#endif
diff --git a/sysdeps/unix/sysv/linux/powerpc/bits/pthreadtypes.h b/sysdeps/unix/sysv/linux/powerpc/bits/pthreadtypes.h
index 396a3d9..998f6d4 100644
--- a/sysdeps/unix/sysv/linux/powerpc/bits/pthreadtypes.h
+++ b/sysdeps/unix/sysv/linux/powerpc/bits/pthreadtypes.h
@@ -172,11 +172,13 @@ typedef union
unsigned int __nr_writers_queued;
int __writer;
int __shared;
- unsigned long int __pad1;
+ unsigned char __rwelision;
+ unsigned char __pad1[7];
unsigned long int __pad2;
/* FLAGS must stay at this position in the structure to maintain
binary compatibility. */
unsigned int __flags;
+# define __PTHREAD_RWLOCK_ELISION_EXTRA 0, {0, 0, 0, 0, 0, 0, 0 }
} __data;
# else
struct
@@ -187,20 +189,20 @@ typedef union
unsigned int __writer_wakeup;
unsigned int __nr_readers_queued;
unsigned int __nr_writers_queued;
- unsigned char __pad1;
+ unsigned char __rwelision;
unsigned char __pad2;
unsigned char __shared;
/* FLAGS must stay at this position in the structure to maintain
binary compatibility. */
unsigned char __flags;
int __writer;
+#define __PTHREAD_RWLOCK_ELISION_EXTRA 0
} __data;
# endif
char __size[__SIZEOF_PTHREAD_RWLOCK_T];
long int __align;
} pthread_rwlock_t;
-#define __PTHREAD_RWLOCK_ELISION_EXTRA 0
typedef union
{
diff --git a/sysdeps/unix/sysv/linux/powerpc/elision-conf.c b/sysdeps/unix/sysv/linux/powerpc/elision-conf.c
index 60a7900..44833f1 100644
--- a/sysdeps/unix/sysv/linux/powerpc/elision-conf.c
+++ b/sysdeps/unix/sysv/linux/powerpc/elision-conf.c
@@ -64,6 +64,9 @@ elision_init (int argc __attribute__ ((unused)),
#ifdef ENABLE_LOCK_ELISION
__pthread_force_elision = __libc_enable_secure ? 0 : elision_available;
#endif
+ if (!__pthread_force_elision)
+ /* Disable elision on rwlocks. */
+ __elision_aconf.try_tbegin = 0;
}
#ifdef SHARED
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH 2/3] PowerPC: Add adaptive elision to rwlocks
2014-11-07 17:13 [PATCH 2/3] PowerPC: Add adaptive elision to rwlocks Adhemerval Zanella
@ 2014-11-27 17:37 ` Adhemerval Zanella
2014-12-10 19:40 ` Will Schmidt
0 siblings, 1 reply; 4+ messages in thread
From: Adhemerval Zanella @ 2014-11-27 17:37 UTC (permalink / raw)
To: libc-alpha
On 07-11-2014 15:13, Adhemerval Zanella wrote:
> This patch adds support for lock elision using ISA 2.07 hardware
> transactional memory for rwlocks. The logic is similar to the
> one presented in pthread_mutex lock elision.
>
> --
>
> * sysdeps/powerpc/nptl/elide.h: New file: generic lock elision support
> for powerpc.
> * sysdeps/unix/sysv/linux/powerpc/bits/pthreadtypes.h
> [pthread_rwlock_t] (__pad1): Change size to 7 bytes in 64 bits case
> and remove it for 32 bits case.
> [pthread_rwlock_t] (__rwelision): New field for lock elision.
> (__PTHREAD_RWLOCK_ELISION_EXTRA): Adjust for new lock elision field
> initialization.
> * sysdeps/unix/sysv/linux/powerpc/elision-conf.c (elision_init):
> Disable lock elision with rdlocks if elision is not available.
>
> ---
>
> diff --git a/sysdeps/powerpc/nptl/elide.h b/sysdeps/powerpc/nptl/elide.h
> new file mode 100644
> index 0000000..5cc47a3
> --- /dev/null
> +++ b/sysdeps/powerpc/nptl/elide.h
> @@ -0,0 +1,111 @@
> +/* elide.h: Generic lock elision support for powerpc.
> + Copyright (C) 2014 Free Software Foundation, Inc.
> + This file is part of the GNU C Library.
> +
> + The GNU C Library is free software; you can redistribute it and/or
> + modify it under the terms of the GNU Lesser General Public
> + License as published by the Free Software Foundation; either
> + version 2.1 of the License, or (at your option) any later version.
> +
> + The GNU C Library is distributed in the hope that it will be useful,
> + but WITHOUT ANY WARRANTY; without even the implied warranty of
> + MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
> + Lesser General Public License for more details.
> +
> + You should have received a copy of the GNU Lesser General Public
> + License along with the GNU C Library; if not, see
> + <http://www.gnu.org/licenses/>. */
> +
> +#ifndef ELIDE_PPC_H
> +# define ELIDE_PPC_H
> +
> +#ifdef ENABLE_LOCK_ELISION
> +# include <htm.h>
> +# include <elision-conf.h>
> +
> +/* Returns true if lock defined by IS_LOCK_FREE was elided.
> + ADAPT_COUNT is a pointer to per-lock state variable. */
> +
> +static inline bool
> +__elide_lock (uint8_t *adapt_count, int is_lock_free)
> +{
> + if (*adapt_count > 0)
> + {
> + (*adapt_count)--;
> + return false;
> + }
> +
> + for (int i = __elision_aconf.try_tbegin; i > 0; i--)
> + {
> + if (__builtin_tbegin (0))
> + {
> + if (is_lock_free)
> + return true;
> + /* Lock was busy. */
> + __builtin_tabort (_ABORT_LOCK_BUSY);
> + }
> + else
> + {
> + /* A persistent failure indicates that a retry will probably
> + result in another failure. Use normal locking now and
> + for the next couple of calls. */
> + if (_TEXASRU_FAILURE_PERSISTENT (__builtin_get_texasru ()))
> + {
> + if (__elision_aconf.skip_lock_internal_abort > 0)
> + *adapt_count = __elision_aconf.skip_lock_internal_abort;
> + break;
> + }
> + /* Same logic as above, but for for a number of temporary failures in a
> + a row. */
> + else if (__elision_aconf.skip_lock_out_of_tbegin_retries > 0
> + && __elision_aconf.try_tbegin > 0)
> + *adapt_count = __elision_aconf.skip_lock_out_of_tbegin_retries;
> + }
> + }
> +
> + return false;
> +}
> +
> +# define ELIDE_LOCK(adapt_count, is_lock_free) \
> + __elide_lock (&(adapt_count), is_lock_free)
> +
> +
> +static inline bool
> +__elide_trylock (uint8_t *adapt_count, int is_lock_free, int write)
> +{
> + if (__elision_aconf.try_tbegin > 0)
> + {
> + if (write)
> + __builtin_tabort (_ABORT_NESTED_TRYLOCK);
> + return __elide_lock (adapt_count, is_lock_free);
> + }
> + return false;
> +}
> +
> +# define ELIDE_TRYLOCK(adapt_count, is_lock_free, write) \
> + __elide_trylock (&(adapt_count), is_lock_free, write)
> +
> +
> +static inline bool
> +__elide_unlock (int is_lock_free)
> +{
> + if (is_lock_free)
> + {
> + __builtin_tend (0);
> + return true;
> + }
> + return false;
> +}
> +
> +# define ELIDE_UNLOCK(is_lock_free) \
> + __elide_unlock (is_lock_free)
> +
> +# else
> +
> +# define ELIDE_LOCK(adapt_count, is_lock_free) 0
> +# define ELIDE_TRYLOCK(adapt_count, is_lock_free, write) 0
> +# define ELIDE_UNLOCK(is_lock_free) 0
> +
> +#endif /* ENABLE_LOCK_ELISION */
> +
> +#endif
> diff --git a/sysdeps/unix/sysv/linux/powerpc/bits/pthreadtypes.h b/sysdeps/unix/sysv/linux/powerpc/bits/pthreadtypes.h
> index 396a3d9..998f6d4 100644
> --- a/sysdeps/unix/sysv/linux/powerpc/bits/pthreadtypes.h
> +++ b/sysdeps/unix/sysv/linux/powerpc/bits/pthreadtypes.h
> @@ -172,11 +172,13 @@ typedef union
> unsigned int __nr_writers_queued;
> int __writer;
> int __shared;
> - unsigned long int __pad1;
> + unsigned char __rwelision;
> + unsigned char __pad1[7];
> unsigned long int __pad2;
> /* FLAGS must stay at this position in the structure to maintain
> binary compatibility. */
> unsigned int __flags;
> +# define __PTHREAD_RWLOCK_ELISION_EXTRA 0, {0, 0, 0, 0, 0, 0, 0 }
> } __data;
> # else
> struct
> @@ -187,20 +189,20 @@ typedef union
> unsigned int __writer_wakeup;
> unsigned int __nr_readers_queued;
> unsigned int __nr_writers_queued;
> - unsigned char __pad1;
> + unsigned char __rwelision;
> unsigned char __pad2;
> unsigned char __shared;
> /* FLAGS must stay at this position in the structure to maintain
> binary compatibility. */
> unsigned char __flags;
> int __writer;
> +#define __PTHREAD_RWLOCK_ELISION_EXTRA 0
> } __data;
> # endif
> char __size[__SIZEOF_PTHREAD_RWLOCK_T];
> long int __align;
> } pthread_rwlock_t;
>
> -#define __PTHREAD_RWLOCK_ELISION_EXTRA 0
>
> typedef union
> {
> diff --git a/sysdeps/unix/sysv/linux/powerpc/elision-conf.c b/sysdeps/unix/sysv/linux/powerpc/elision-conf.c
> index 60a7900..44833f1 100644
> --- a/sysdeps/unix/sysv/linux/powerpc/elision-conf.c
> +++ b/sysdeps/unix/sysv/linux/powerpc/elision-conf.c
> @@ -64,6 +64,9 @@ elision_init (int argc __attribute__ ((unused)),
> #ifdef ENABLE_LOCK_ELISION
> __pthread_force_elision = __libc_enable_secure ? 0 : elision_available;
> #endif
> + if (!__pthread_force_elision)
> + /* Disable elision on rwlocks. */
> + __elision_aconf.try_tbegin = 0;
> }
>
> #ifdef SHARED
>
Ping.
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH 2/3] PowerPC: Add adaptive elision to rwlocks
2014-11-27 17:37 ` Adhemerval Zanella
@ 2014-12-10 19:40 ` Will Schmidt
2014-12-17 16:32 ` Adhemerval Zanella
0 siblings, 1 reply; 4+ messages in thread
From: Will Schmidt @ 2014-12-10 19:40 UTC (permalink / raw)
To: Adhemerval Zanella; +Cc: libc-alpha
On Thu, 2014-11-27 at 15:36 -0200, Adhemerval Zanella wrote:
> On 07-11-2014 15:13, Adhemerval Zanella wrote:
> > This patch adds support for lock elision using ISA 2.07 hardware
> > transactional memory for rwlocks. The logic is similar to the
> > one presented in pthread_mutex lock elision.
Cosmetic comments below. no code/functional concerns.
> >
> > --
> >
> > * sysdeps/powerpc/nptl/elide.h: New file: generic lock elision support
> > for powerpc.
> > * sysdeps/unix/sysv/linux/powerpc/bits/pthreadtypes.h
> > [pthread_rwlock_t] (__pad1): Change size to 7 bytes in 64 bits case
> > and remove it for 32 bits case.
I would first ask why the change, but this is actually answered on the
next line. Since __pad1 is simply padding, and it's being shrunk or
replaced to make room for the new lock elision field, probably not worth
being called out here.
> > [pthread_rwlock_t] (__rwelision): New field for lock elision.
> > (__PTHREAD_RWLOCK_ELISION_EXTRA): Adjust for new lock elision field
> > initialization.
> > * sysdeps/unix/sysv/linux/powerpc/elision-conf.c (elision_init):
> > Disable lock elision with rdlocks if elision is not available.
> >
> > ---
> >
> > diff --git a/sysdeps/powerpc/nptl/elide.h b/sysdeps/powerpc/nptl/elide.h
> > new file mode 100644
> > index 0000000..5cc47a3
> > --- /dev/null
> > +++ b/sysdeps/powerpc/nptl/elide.h
> > @@ -0,0 +1,111 @@
> > +/* elide.h: Generic lock elision support for powerpc.
> > + Copyright (C) 2014 Free Software Foundation, Inc.
> > + This file is part of the GNU C Library.
> > +
> > + The GNU C Library is free software; you can redistribute it and/or
> > + modify it under the terms of the GNU Lesser General Public
> > + License as published by the Free Software Foundation; either
> > + version 2.1 of the License, or (at your option) any later version.
> > +
> > + The GNU C Library is distributed in the hope that it will be useful,
> > + but WITHOUT ANY WARRANTY; without even the implied warranty of
> > + MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
> > + Lesser General Public License for more details.
> > +
> > + You should have received a copy of the GNU Lesser General Public
> > + License along with the GNU C Library; if not, see
> > + <http://www.gnu.org/licenses/>. */
> > +
> > +#ifndef ELIDE_PPC_H
> > +# define ELIDE_PPC_H
> > +
> > +#ifdef ENABLE_LOCK_ELISION
> > +# include <htm.h>
> > +# include <elision-conf.h>
> > +
> > +/* Returns true if lock defined by IS_LOCK_FREE was elided.
s/if lock/if the lock/
s/IS_LOCK_FREE/is_lock_free/
> > + ADAPT_COUNT is a pointer to per-lock state variable. */
> > +
> > +static inline bool
> > +__elide_lock (uint8_t *adapt_count, int is_lock_free)
> > +{
> > + if (*adapt_count > 0)
> > + {
> > + (*adapt_count)--;
> > + return false;
> > + }
> > +
> > + for (int i = __elision_aconf.try_tbegin; i > 0; i--)
> > + {
> > + if (__builtin_tbegin (0))
> > + {
> > + if (is_lock_free)
> > + return true;
> > + /* Lock was busy. */
> > + __builtin_tabort (_ABORT_LOCK_BUSY);
> > + }
> > + else
> > + {
> > + /* A persistent failure indicates that a retry will probably
> > + result in another failure. Use normal locking now and
> > + for the next couple of calls. */
> > + if (_TEXASRU_FAILURE_PERSISTENT (__builtin_get_texasru ()))
> > + {
> > + if (__elision_aconf.skip_lock_internal_abort > 0)
> > + *adapt_count = __elision_aconf.skip_lock_internal_abort;
> > + break;
> > + }
> > + /* Same logic as above, but for for a number of temporary failures in a
> > + a row. */
s/for for/for/
(looking back, I also see this at least once in [1/3] )
> > + else if (__elision_aconf.skip_lock_out_of_tbegin_retries > 0
> > + && __elision_aconf.try_tbegin > 0)
> > + *adapt_count = __elision_aconf.skip_lock_out_of_tbegin_retries;
> > + }
> > + }
tab or whitespace indentation glitch here?
> > +
> > + return false;
> > +}
> > +
> > +# define ELIDE_LOCK(adapt_count, is_lock_free) \
> > + __elide_lock (&(adapt_count), is_lock_free)
> > +
> > +
> > +static inline bool
> > +__elide_trylock (uint8_t *adapt_count, int is_lock_free, int write)
> > +{
> > + if (__elision_aconf.try_tbegin > 0)
> > + {
> > + if (write)
> > + __builtin_tabort (_ABORT_NESTED_TRYLOCK);
> > + return __elide_lock (adapt_count, is_lock_free);
> > + }
> > + return false;
> > +}
> > +
> > +# define ELIDE_TRYLOCK(adapt_count, is_lock_free, write) \
> > + __elide_trylock (&(adapt_count), is_lock_free, write)
> > +
> > +
> > +static inline bool
> > +__elide_unlock (int is_lock_free)
> > +{
> > + if (is_lock_free)
> > + {
> > + __builtin_tend (0);
> > + return true;
> > + }
> > + return false;
> > +}
> > +
> > +# define ELIDE_UNLOCK(is_lock_free) \
> > + __elide_unlock (is_lock_free)
> > +
> > +# else
> > +
> > +# define ELIDE_LOCK(adapt_count, is_lock_free) 0
> > +# define ELIDE_TRYLOCK(adapt_count, is_lock_free, write) 0
> > +# define ELIDE_UNLOCK(is_lock_free) 0
> > +
> > +#endif /* ENABLE_LOCK_ELISION */
> > +
> > +#endif
Ok.
> > diff --git a/sysdeps/unix/sysv/linux/powerpc/bits/pthreadtypes.h b/sysdeps/unix/sysv/linux/powerpc/bits/pthreadtypes.h
> > index 396a3d9..998f6d4 100644
> > --- a/sysdeps/unix/sysv/linux/powerpc/bits/pthreadtypes.h
> > +++ b/sysdeps/unix/sysv/linux/powerpc/bits/pthreadtypes.h
> > @@ -172,11 +172,13 @@ typedef union
> > unsigned int __nr_writers_queued;
> > int __writer;
> > int __shared;
> > - unsigned long int __pad1;
> > + unsigned char __rwelision;
> > + unsigned char __pad1[7];
> > unsigned long int __pad2;
> > /* FLAGS must stay at this position in the structure to maintain
> > binary compatibility. */
> > unsigned int __flags;
> > +# define __PTHREAD_RWLOCK_ELISION_EXTRA 0, {0, 0, 0, 0, 0, 0, 0 }
> > } __data;
> > # else
> > struct
> > @@ -187,20 +189,20 @@ typedef union
> > unsigned int __writer_wakeup;
> > unsigned int __nr_readers_queued;
> > unsigned int __nr_writers_queued;
> > - unsigned char __pad1;
> > + unsigned char __rwelision;
> > unsigned char __pad2;
> > unsigned char __shared;
> > /* FLAGS must stay at this position in the structure to maintain
> > binary compatibility. */
> > unsigned char __flags;
> > int __writer;
> > +#define __PTHREAD_RWLOCK_ELISION_EXTRA 0
> > } __data;
> > # endif
> > char __size[__SIZEOF_PTHREAD_RWLOCK_T];
> > long int __align;
> > } pthread_rwlock_t;
> >
> > -#define __PTHREAD_RWLOCK_ELISION_EXTRA 0
> >
> > typedef union
> > {
Ok.
> > diff --git a/sysdeps/unix/sysv/linux/powerpc/elision-conf.c b/sysdeps/unix/sysv/linux/powerpc/elision-conf.c
> > index 60a7900..44833f1 100644
> > --- a/sysdeps/unix/sysv/linux/powerpc/elision-conf.c
> > +++ b/sysdeps/unix/sysv/linux/powerpc/elision-conf.c
> > @@ -64,6 +64,9 @@ elision_init (int argc __attribute__ ((unused)),
> > #ifdef ENABLE_LOCK_ELISION
> > __pthread_force_elision = __libc_enable_secure ? 0 : elision_available;
> > #endif
> > + if (!__pthread_force_elision)
> > + /* Disable elision on rwlocks. */
> > + __elision_aconf.try_tbegin = 0;
> > }
Ok.
> >
> > #ifdef SHARED
> >
> Ping.
>
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH 2/3] PowerPC: Add adaptive elision to rwlocks
2014-12-10 19:40 ` Will Schmidt
@ 2014-12-17 16:32 ` Adhemerval Zanella
0 siblings, 0 replies; 4+ messages in thread
From: Adhemerval Zanella @ 2014-12-17 16:32 UTC (permalink / raw)
To: will_schmidt; +Cc: libc-alpha
Hi Will,
Thanks for the review, I will send a patchset update soon. Below some
comments about your points.
On 10-12-2014 17:40, Will Schmidt wrote:
>
>> ---
>>
>> diff --git a/sysdeps/powerpc/nptl/elide.h b/sysdeps/powerpc/nptl/elide.h
>> new file mode 100644
>> index 0000000..5cc47a3
>> --- /dev/null
>> +++ b/sysdeps/powerpc/nptl/elide.h
>> @@ -0,0 +1,111 @@
>> +/* elide.h: Generic lock elision support for powerpc.
>> + Copyright (C) 2014 Free Software Foundation, Inc.
>> + This file is part of the GNU C Library.
>> +
>> + The GNU C Library is free software; you can redistribute it and/or
>> + modify it under the terms of the GNU Lesser General Public
>> + License as published by the Free Software Foundation; either
>> + version 2.1 of the License, or (at your option) any later version.
>> +
>> + The GNU C Library is distributed in the hope that it will be useful,
>> + but WITHOUT ANY WARRANTY; without even the implied warranty of
>> + MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
>> + Lesser General Public License for more details.
>> +
>> + You should have received a copy of the GNU Lesser General Public
>> + License along with the GNU C Library; if not, see
>> + <http://www.gnu.org/licenses/>. */
>> +
>> +#ifndef ELIDE_PPC_H
>> +# define ELIDE_PPC_H
>> +
>> +#ifdef ENABLE_LOCK_ELISION
>> +# include <htm.h>
>> +# include <elision-conf.h>
>> +
>> +/* Returns true if lock defined by IS_LOCK_FREE was elided.
> s/if lock/if the lock/
> s/IS_LOCK_FREE/is_lock_free/
Fixed
>
>>> + ADAPT_COUNT is a pointer to per-lock state variable. */
>
>>> +
>>> +static inline bool
>>> +__elide_lock (uint8_t *adapt_count, int is_lock_free)
>>> +{
>>> + if (*adapt_count > 0)
>>> + {
>>> + (*adapt_count)--;
>>> + return false;
>>> + }
>>> +
>>> + for (int i = __elision_aconf.try_tbegin; i > 0; i--)
>>> + {
>>> + if (__builtin_tbegin (0))
>>> + {
>>> + if (is_lock_free)
>>> + return true;
>>> + /* Lock was busy. */
>>> + __builtin_tabort (_ABORT_LOCK_BUSY);
>>> + }
>>> + else
>>> + {
>>> + /* A persistent failure indicates that a retry will probably
>>> + result in another failure. Use normal locking now and
>>> + for the next couple of calls. */
>>> + if (_TEXASRU_FAILURE_PERSISTENT (__builtin_get_texasru ()))
>>> + {
>>> + if (__elision_aconf.skip_lock_internal_abort > 0)
>>> + *adapt_count = __elision_aconf.skip_lock_internal_abort;
>>> + break;
>>> + }
>>> + /* Same logic as above, but for for a number of temporary failures in a
>>> + a row. */
> s/for for/for/
> (looking back, I also see this at least once in [1/3] )
Fixed.
>
>
>>> + else if (__elision_aconf.skip_lock_out_of_tbegin_retries > 0
>>> + && __elision_aconf.try_tbegin > 0)
>>> + *adapt_count = __elision_aconf.skip_lock_out_of_tbegin_retries;
>>> + }
>>> + }
> tab or whitespace indentation glitch here?
I will check, but I think it using tab correctly here.
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2014-12-17 16:32 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2014-11-07 17:13 [PATCH 2/3] PowerPC: Add adaptive elision to rwlocks Adhemerval Zanella
2014-11-27 17:37 ` Adhemerval Zanella
2014-12-10 19:40 ` Will Schmidt
2014-12-17 16:32 ` Adhemerval Zanella
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).