* Re: [PATCH v6 4/4] Avoid extra load with CAS in __pthread_mutex_clocklock_common [BZ #28537] [not found] ` <87v90xryvi.fsf@igel.home> @ 2022-09-11 20:12 ` Sunil Pandey 2022-09-11 20:15 ` Arjan van de Ven 2022-09-29 0:09 ` Noah Goldstein 0 siblings, 2 replies; 8+ messages in thread From: Sunil Pandey @ 2022-09-11 20:12 UTC (permalink / raw) To: Andreas Schwab, Libc-stable Mailing List Cc: H.J. Lu, Florian Weimer, GNU C Library, Arjan van de Ven, Paul A . Clarke On Fri, Nov 12, 2021 at 10:53 AM Andreas Schwab <schwab@linux-m68k.org> wrote: > > On Nov 11 2021, H.J. Lu wrote: > > > diff --git a/nptl/pthread_mutex_timedlock.c b/nptl/pthread_mutex_timedlock.c > > index 57f3f28869..f763cfc7fa 100644 > > --- a/nptl/pthread_mutex_timedlock.c > > +++ b/nptl/pthread_mutex_timedlock.c > > @@ -233,12 +233,12 @@ __pthread_mutex_clocklock_common (pthread_mutex_t *mutex, > > meantime. */ > > if ((oldval & FUTEX_WAITERS) == 0) > > { > > - if (atomic_compare_and_exchange_bool_acq (&mutex->__data.__lock, > > - oldval | FUTEX_WAITERS, > > - oldval) > > - != 0) > > + int val; > > + if ((val = atomic_compare_and_exchange_val_acq > > + (&mutex->__data.__lock, oldval | FUTEX_WAITERS, > > + oldval)) != oldval) > > Please move the assignment out of the condition. > > Andreas. > > -- > Andreas Schwab, schwab@linux-m68k.org > GPG Key fingerprint = 7578 EB47 D4E5 4D69 2510 2552 DF73 E780 A9DA AEC1 > "And now for something completely different." I would like to backport this patch to release branch 2.33 and 2.34 Any comments/suggestions or objections on this. commit 0b82747dc48d5bf0871bdc6da8cb6eec1256355f Author: H.J. Lu <hjl.tools@gmail.com> Date: Thu Nov 11 06:31:51 2021 -0800 Avoid extra load with CAS in __pthread_mutex_lock_full [BZ #28537] Replace boolean CAS with value CAS to avoid the extra load. Reviewed-by: Szabolcs Nagy <szabolcs.nagy@arm.com> ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH v6 4/4] Avoid extra load with CAS in __pthread_mutex_clocklock_common [BZ #28537] 2022-09-11 20:12 ` [PATCH v6 4/4] Avoid extra load with CAS in __pthread_mutex_clocklock_common [BZ #28537] Sunil Pandey @ 2022-09-11 20:15 ` Arjan van de Ven 2022-09-11 21:26 ` Florian Weimer 2022-09-29 0:09 ` Noah Goldstein 1 sibling, 1 reply; 8+ messages in thread From: Arjan van de Ven @ 2022-09-11 20:15 UTC (permalink / raw) To: Sunil Pandey, Andreas Schwab, Libc-stable Mailing List Cc: H.J. Lu, Florian Weimer, GNU C Library, Paul A . Clarke On 9/11/2022 1:12 PM, Sunil Pandey wrote: > On Fri, Nov 12, 2021 at 10:53 AM Andreas Schwab <schwab@linux-m68k.org> wrote: >> >> On Nov 11 2021, H.J. Lu wrote: >> >>> diff --git a/nptl/pthread_mutex_timedlock.c b/nptl/pthread_mutex_timedlock.c >>> index 57f3f28869..f763cfc7fa 100644 >>> --- a/nptl/pthread_mutex_timedlock.c >>> +++ b/nptl/pthread_mutex_timedlock.c >>> @@ -233,12 +233,12 @@ __pthread_mutex_clocklock_common (pthread_mutex_t *mutex, >>> meantime. */ >>> if ((oldval & FUTEX_WAITERS) == 0) >>> { >>> - if (atomic_compare_and_exchange_bool_acq (&mutex->__data.__lock, >>> - oldval | FUTEX_WAITERS, >>> - oldval) >>> - != 0) >>> + int val; >>> + if ((val = atomic_compare_and_exchange_val_acq >>> + (&mutex->__data.__lock, oldval | FUTEX_WAITERS, >>> + oldval)) != oldval) >> >> Please move the assignment out of the condition. >> >> Andreas. >> >> -- >> Andreas Schwab, schwab@linux-m68k.org >> GPG Key fingerprint = 7578 EB47 D4E5 4D69 2510 2552 DF73 E780 A9DA AEC1 >> "And now for something completely different." > > I would like to backport this patch to release branch 2.33 and 2.34 > what exactly is the stable branch policy that would suggest to backport performance improvements like this ? (most projects are sticking to "strict bugfixes and other gross oversights" as much as possible) ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH v6 4/4] Avoid extra load with CAS in __pthread_mutex_clocklock_common [BZ #28537] 2022-09-11 20:15 ` Arjan van de Ven @ 2022-09-11 21:26 ` Florian Weimer 0 siblings, 0 replies; 8+ messages in thread From: Florian Weimer @ 2022-09-11 21:26 UTC (permalink / raw) To: Arjan van de Ven via Libc-alpha Cc: Sunil Pandey, Andreas Schwab, Libc-stable Mailing List, Arjan van de Ven, Paul A . Clarke * Arjan van de Ven via Libc-alpha: > what exactly is the stable branch policy that would suggest to > backport performance improvements like this ? (most projects are > sticking to "strict bugfixes and other gross oversights" as much as > possible) We occasionally backport safe (and not-so-safe) performance optimizations. We draw the line at ABI changes, those aren't possible. It's also very desirable that all future releases have a superset of the backports. But anything else depends on who is willing to do the work. Thanks, Florian ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH v6 4/4] Avoid extra load with CAS in __pthread_mutex_clocklock_common [BZ #28537] 2022-09-11 20:12 ` [PATCH v6 4/4] Avoid extra load with CAS in __pthread_mutex_clocklock_common [BZ #28537] Sunil Pandey 2022-09-11 20:15 ` Arjan van de Ven @ 2022-09-29 0:09 ` Noah Goldstein 1 sibling, 0 replies; 8+ messages in thread From: Noah Goldstein @ 2022-09-29 0:09 UTC (permalink / raw) To: Sunil Pandey Cc: Andreas Schwab, Libc-stable Mailing List, Florian Weimer, Arjan van de Ven, GNU C Library, Paul A . Clarke On Sun, Sep 11, 2022 at 4:13 PM Sunil Pandey via Libc-stable <libc-stable@sourceware.org> wrote: > > On Fri, Nov 12, 2021 at 10:53 AM Andreas Schwab <schwab@linux-m68k.org> wrote: > > > > On Nov 11 2021, H.J. Lu wrote: > > > > > diff --git a/nptl/pthread_mutex_timedlock.c b/nptl/pthread_mutex_timedlock.c > > > index 57f3f28869..f763cfc7fa 100644 > > > --- a/nptl/pthread_mutex_timedlock.c > > > +++ b/nptl/pthread_mutex_timedlock.c > > > @@ -233,12 +233,12 @@ __pthread_mutex_clocklock_common (pthread_mutex_t *mutex, > > > meantime. */ > > > if ((oldval & FUTEX_WAITERS) == 0) > > > { > > > - if (atomic_compare_and_exchange_bool_acq (&mutex->__data.__lock, > > > - oldval | FUTEX_WAITERS, > > > - oldval) > > > - != 0) > > > + int val; > > > + if ((val = atomic_compare_and_exchange_val_acq > > > + (&mutex->__data.__lock, oldval | FUTEX_WAITERS, > > > + oldval)) != oldval) > > > > Please move the assignment out of the condition. > > > > Andreas. > > > > -- > > Andreas Schwab, schwab@linux-m68k.org > > GPG Key fingerprint = 7578 EB47 D4E5 4D69 2510 2552 DF73 E780 A9DA AEC1 > > "And now for something completely different." > > I would like to backport this patch to release branch 2.33 and 2.34 > > Any comments/suggestions or objections on this. > > commit 0b82747dc48d5bf0871bdc6da8cb6eec1256355f > Author: H.J. Lu <hjl.tools@gmail.com> > Date: Thu Nov 11 06:31:51 2021 -0800 > > Avoid extra load with CAS in __pthread_mutex_lock_full [BZ #28537] > > Replace boolean CAS with value CAS to avoid the extra load. > > Reviewed-by: Szabolcs Nagy <szabolcs.nagy@arm.com> Fine by me. ^ permalink raw reply [flat|nested] 8+ messages in thread
[parent not found: <20211111162428.2286605-3-hjl.tools@gmail.com>]
[parent not found: <87zgq9rywl.fsf@igel.home>]
* Re: [PATCH v6 2/4] Avoid extra load with CAS in __pthread_mutex_lock_full [BZ #28537] [not found] ` <87zgq9rywl.fsf@igel.home> @ 2022-09-11 20:16 ` Sunil Pandey 2022-09-29 0:10 ` Noah Goldstein 0 siblings, 1 reply; 8+ messages in thread From: Sunil Pandey @ 2022-09-11 20:16 UTC (permalink / raw) To: Andreas Schwab, Libc-stable Mailing List Cc: H.J. Lu, Florian Weimer, GNU C Library, Arjan van de Ven, Paul A . Clarke On Fri, Nov 12, 2021 at 10:52 AM Andreas Schwab <schwab@linux-m68k.org> wrote: > > On Nov 11 2021, H.J. Lu wrote: > > > diff --git a/nptl/pthread_mutex_lock.c b/nptl/pthread_mutex_lock.c > > index 72058c719c..762059b230 100644 > > --- a/nptl/pthread_mutex_lock.c > > +++ b/nptl/pthread_mutex_lock.c > > @@ -304,12 +304,12 @@ __pthread_mutex_lock_full (pthread_mutex_t *mutex) > > meantime. */ > > if ((oldval & FUTEX_WAITERS) == 0) > > { > > - if (atomic_compare_and_exchange_bool_acq (&mutex->__data.__lock, > > - oldval | FUTEX_WAITERS, > > - oldval) > > - != 0) > > + int val; > > + if ((val = atomic_compare_and_exchange_val_acq > > + (&mutex->__data.__lock, oldval | FUTEX_WAITERS, > > + oldval)) != oldval) > > Please move the assignment out of the condition. > > Andreas. > > -- > Andreas Schwab, schwab@linux-m68k.org > GPG Key fingerprint = 7578 EB47 D4E5 4D69 2510 2552 DF73 E780 A9DA AEC1 > "And now for something completely different." I would like to backport this patch to release branch 2.33 and 2.34 Any comments/suggestions or objections on this. commit 49302b8fdf9103b6fc0a398678668a22fa19574c Author: H.J. Lu <hjl.tools@gmail.com> Date: Thu Nov 11 06:54:01 2021 -0800 Avoid extra load with CAS in __pthread_mutex_clocklock_common [BZ #28537] Replace boolean CAS with value CAS to avoid the extra load. Reviewed-by: Szabolcs Nagy <szabolcs.nagy@arm.com> ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH v6 2/4] Avoid extra load with CAS in __pthread_mutex_lock_full [BZ #28537] 2022-09-11 20:16 ` [PATCH v6 2/4] Avoid extra load with CAS in __pthread_mutex_lock_full " Sunil Pandey @ 2022-09-29 0:10 ` Noah Goldstein 0 siblings, 0 replies; 8+ messages in thread From: Noah Goldstein @ 2022-09-29 0:10 UTC (permalink / raw) To: Sunil Pandey Cc: Andreas Schwab, Libc-stable Mailing List, Florian Weimer, Arjan van de Ven, GNU C Library, Paul A . Clarke On Sun, Sep 11, 2022 at 4:17 PM Sunil Pandey via Libc-stable <libc-stable@sourceware.org> wrote: > > On Fri, Nov 12, 2021 at 10:52 AM Andreas Schwab <schwab@linux-m68k.org> wrote: > > > > On Nov 11 2021, H.J. Lu wrote: > > > > > diff --git a/nptl/pthread_mutex_lock.c b/nptl/pthread_mutex_lock.c > > > index 72058c719c..762059b230 100644 > > > --- a/nptl/pthread_mutex_lock.c > > > +++ b/nptl/pthread_mutex_lock.c > > > @@ -304,12 +304,12 @@ __pthread_mutex_lock_full (pthread_mutex_t *mutex) > > > meantime. */ > > > if ((oldval & FUTEX_WAITERS) == 0) > > > { > > > - if (atomic_compare_and_exchange_bool_acq (&mutex->__data.__lock, > > > - oldval | FUTEX_WAITERS, > > > - oldval) > > > - != 0) > > > + int val; > > > + if ((val = atomic_compare_and_exchange_val_acq > > > + (&mutex->__data.__lock, oldval | FUTEX_WAITERS, > > > + oldval)) != oldval) > > > > Please move the assignment out of the condition. > > > > Andreas. > > > > -- > > Andreas Schwab, schwab@linux-m68k.org > > GPG Key fingerprint = 7578 EB47 D4E5 4D69 2510 2552 DF73 E780 A9DA AEC1 > > "And now for something completely different." > > I would like to backport this patch to release branch 2.33 and 2.34 > > Any comments/suggestions or objections on this. > > commit 49302b8fdf9103b6fc0a398678668a22fa19574c > Author: H.J. Lu <hjl.tools@gmail.com> > Date: Thu Nov 11 06:54:01 2021 -0800 > > Avoid extra load with CAS in __pthread_mutex_clocklock_common [BZ #28537] > > Replace boolean CAS with value CAS to avoid the extra load. > > Reviewed-by: Szabolcs Nagy <szabolcs.nagy@arm.com> Fine by me. ^ permalink raw reply [flat|nested] 8+ messages in thread
[parent not found: <20211111162428.2286605-2-hjl.tools@gmail.com>]
[parent not found: <CAFUsyfJH45MshHkFjohENvhJzt1bC=PrUH_3xRn1KubGy9X1HA@mail.gmail.com>]
[parent not found: <CAMe9rOqOv3WC9rWwtFyzc7rJFDDbNTAfTio-FZC4oW5y751FWA@mail.gmail.com>]
[parent not found: <CAFUsyf+Yf=QM-0Zj31=k6fANCdXa6XnzVn=Kj6im3xDYbanbnA@mail.gmail.com>]
[parent not found: <CAMe9rOpE1+1zqtA4xqJNy-RK03jXNzv-JnYSJFtGmtqSa2QqQQ@mail.gmail.com>]
[parent not found: <924a80cd-202c-99e9-a2d9-1aeda2235b83@linux.intel.com>]
* Re: [PATCH v6 1/4] Add LLL_MUTEX_READ_LOCK [BZ #28537] [not found] ` <924a80cd-202c-99e9-a2d9-1aeda2235b83@linux.intel.com> @ 2022-09-11 20:19 ` Sunil Pandey 2022-09-29 0:10 ` Noah Goldstein 0 siblings, 1 reply; 8+ messages in thread From: Sunil Pandey @ 2022-09-11 20:19 UTC (permalink / raw) To: Arjan van de Ven, Libc-stable Mailing List Cc: H.J. Lu, Noah Goldstein, Florian Weimer, Andreas Schwab, GNU C Library, Paul A . Clarke On Wed, Nov 17, 2021 at 5:17 PM Arjan van de Ven via Libc-alpha <libc-alpha@sourceware.org> wrote: > > On 11/17/2021 4:31 PM, H.J. Lu wrote: > >> Yes, but the loop will be able to run `max_cnt` iterations much faster now. > >> Just wondering if the value needs to be re-tuned. Not that is necessarily needs > >> to be. > > Maybe if we can find some data to show for. > > > > wondering when this was last tuned.. I assume todays CPUs and CPUs from, say, 5 or 10 years ago > have an order of magnitude different performance in this regard.... > if there wasn't a need to retune during that, maybe this value is so robust that it doesn't need > retuning. > > or maybe it's time to retune this in general sometime soon after this patch goes in ;) I would like to backport this patch to release branch 2.33 and 2.34 Any comments/suggestions or objections on this. commit d672a98a1af106bd68deb15576710cd61363f7a6 Author: H.J. Lu <hjl.tools@gmail.com> Date: Tue Nov 2 18:33:07 2021 -0700 Add LLL_MUTEX_READ_LOCK [BZ #28537] CAS instruction is expensive. From the x86 CPU's point of view, getting a cache line for writing is more expensive than reading. See Appendix A.2 Spinlock in: https://www.intel.com/content/dam/www/public/us/en/documents/white-papers/xeon-lock-scaling-analysis-paper.pdf ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH v6 1/4] Add LLL_MUTEX_READ_LOCK [BZ #28537] 2022-09-11 20:19 ` [PATCH v6 1/4] Add LLL_MUTEX_READ_LOCK " Sunil Pandey @ 2022-09-29 0:10 ` Noah Goldstein 0 siblings, 0 replies; 8+ messages in thread From: Noah Goldstein @ 2022-09-29 0:10 UTC (permalink / raw) To: Sunil Pandey Cc: Arjan van de Ven, Libc-stable Mailing List, H.J. Lu, Florian Weimer, Andreas Schwab, GNU C Library, Paul A . Clarke On Sun, Sep 11, 2022 at 4:19 PM Sunil Pandey <skpgkp2@gmail.com> wrote: > > On Wed, Nov 17, 2021 at 5:17 PM Arjan van de Ven via Libc-alpha > <libc-alpha@sourceware.org> wrote: > > > > On 11/17/2021 4:31 PM, H.J. Lu wrote: > > >> Yes, but the loop will be able to run `max_cnt` iterations much faster now. > > >> Just wondering if the value needs to be re-tuned. Not that is necessarily needs > > >> to be. > > > Maybe if we can find some data to show for. > > > > > > > wondering when this was last tuned.. I assume todays CPUs and CPUs from, say, 5 or 10 years ago > > have an order of magnitude different performance in this regard.... > > if there wasn't a need to retune during that, maybe this value is so robust that it doesn't need > > retuning. > > > > or maybe it's time to retune this in general sometime soon after this patch goes in ;) > > I would like to backport this patch to release branch 2.33 and 2.34 Fine by me. > > Any comments/suggestions or objections on this. > > commit d672a98a1af106bd68deb15576710cd61363f7a6 > Author: H.J. Lu <hjl.tools@gmail.com> > Date: Tue Nov 2 18:33:07 2021 -0700 > > Add LLL_MUTEX_READ_LOCK [BZ #28537] > > CAS instruction is expensive. From the x86 CPU's point of view, getting > a cache line for writing is more expensive than reading. See Appendix > A.2 Spinlock in: > > https://www.intel.com/content/dam/www/public/us/en/documents/white-papers/xeon-lock-scaling-analysis-paper.pdf ^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2022-09-29 0:11 UTC | newest] Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- [not found] <20211111162428.2286605-1-hjl.tools@gmail.com> [not found] ` <20211111162428.2286605-5-hjl.tools@gmail.com> [not found] ` <87v90xryvi.fsf@igel.home> 2022-09-11 20:12 ` [PATCH v6 4/4] Avoid extra load with CAS in __pthread_mutex_clocklock_common [BZ #28537] Sunil Pandey 2022-09-11 20:15 ` Arjan van de Ven 2022-09-11 21:26 ` Florian Weimer 2022-09-29 0:09 ` Noah Goldstein [not found] ` <20211111162428.2286605-3-hjl.tools@gmail.com> [not found] ` <87zgq9rywl.fsf@igel.home> 2022-09-11 20:16 ` [PATCH v6 2/4] Avoid extra load with CAS in __pthread_mutex_lock_full " Sunil Pandey 2022-09-29 0:10 ` Noah Goldstein [not found] ` <20211111162428.2286605-2-hjl.tools@gmail.com> [not found] ` <CAFUsyfJH45MshHkFjohENvhJzt1bC=PrUH_3xRn1KubGy9X1HA@mail.gmail.com> [not found] ` <CAMe9rOqOv3WC9rWwtFyzc7rJFDDbNTAfTio-FZC4oW5y751FWA@mail.gmail.com> [not found] ` <CAFUsyf+Yf=QM-0Zj31=k6fANCdXa6XnzVn=Kj6im3xDYbanbnA@mail.gmail.com> [not found] ` <CAMe9rOpE1+1zqtA4xqJNy-RK03jXNzv-JnYSJFtGmtqSa2QqQQ@mail.gmail.com> [not found] ` <924a80cd-202c-99e9-a2d9-1aeda2235b83@linux.intel.com> 2022-09-11 20:19 ` [PATCH v6 1/4] Add LLL_MUTEX_READ_LOCK " Sunil Pandey 2022-09-29 0:10 ` Noah Goldstein
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).