public inbox for libc-alpha@sourceware.org
 help / color / mirror / Atom feed
* [PATCH] x86: Remove the default REP MOVSB threshold tunable value [BZ #27061]
@ 2020-12-13 14:51 H.J. Lu
  2020-12-14 14:59 ` Carlos O'Donell
  0 siblings, 1 reply; 3+ messages in thread
From: H.J. Lu @ 2020-12-13 14:51 UTC (permalink / raw)
  To: libc-alpha

Since the default REP MOVSB threshold is 2048 * (vector size / 16),
remove its default tunable value so that the correct default value
will be set correctly by init_cacheinfo ().
---
 sysdeps/x86/dl-tunables.list | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/sysdeps/x86/dl-tunables.list b/sysdeps/x86/dl-tunables.list
index 1a4a93a070..348616fb69 100644
--- a/sysdeps/x86/dl-tunables.list
+++ b/sysdeps/x86/dl-tunables.list
@@ -39,9 +39,10 @@ glibc {
       # REP MOVSB.  Since larger register size can move more data with a
       # single load and store, the threshold is higher with larger register
       # size.  Note: Since the REP MOVSB threshold must be greater than 8
-      # times of vector size, the minium value must be updated at run-time.
+      # times of vector size and the default value is 2048 * (vector size
+      # / 16), the default value and the minium value must be updated at
+      # run-time.
       minval: 1
-      default: 2048
     }
     x86_rep_stosb_threshold {
       type: SIZE_T
-- 
2.29.2


^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [PATCH] x86: Remove the default REP MOVSB threshold tunable value [BZ #27061]
  2020-12-13 14:51 [PATCH] x86: Remove the default REP MOVSB threshold tunable value [BZ #27061] H.J. Lu
@ 2020-12-14 14:59 ` Carlos O'Donell
  2020-12-14 15:14   ` H.J. Lu
  0 siblings, 1 reply; 3+ messages in thread
From: Carlos O'Donell @ 2020-12-14 14:59 UTC (permalink / raw)
  To: H.J. Lu, libc-alpha

On 12/13/20 9:51 AM, H.J. Lu via Libc-alpha wrote:
> Since the default REP MOVSB threshold is 2048 * (vector size / 16),
> remove its default tunable value so that the correct default value
> will be set correctly by init_cacheinfo ().

This is a failure in the tunables framework. We should have internal
APIs to detect:
* Set by user.
* Still set to default.
I've run into this before when we were looking at some of the other
tunables for x86. Please file an enhancement request for this and
then we can look at implementing this and adding back the defaults.

OK with the typo fix and upstream bug reference.

Reviewed-by: Carlos O'Donell <carlos@redhat.com>

> ---
>  sysdeps/x86/dl-tunables.list | 5 +++--
>  1 file changed, 3 insertions(+), 2 deletions(-)
> 
> diff --git a/sysdeps/x86/dl-tunables.list b/sysdeps/x86/dl-tunables.list
> index 1a4a93a070..348616fb69 100644
> --- a/sysdeps/x86/dl-tunables.list
> +++ b/sysdeps/x86/dl-tunables.list
> @@ -39,9 +39,10 @@ glibc {
>        # REP MOVSB.  Since larger register size can move more data with a
>        # single load and store, the threshold is higher with larger register
>        # size.  Note: Since the REP MOVSB threshold must be greater than 8
> -      # times of vector size, the minium value must be updated at run-time.
> +      # times of vector size and the default value is 2048 * (vector size
> +      # / 16), the default value and the minium value must be updated at

s/minium/minimum/g

Please add references to upstream bug.

> +      # run-time.
>        minval: 1
> -      default: 2048
>      }
>      x86_rep_stosb_threshold {
>        type: SIZE_T
> 


-- 
Cheers,
Carlos.


^ permalink raw reply	[flat|nested] 3+ messages in thread

* [PATCH] x86: Remove the default REP MOVSB threshold tunable value [BZ #27061]
  2020-12-14 14:59 ` Carlos O'Donell
@ 2020-12-14 15:14   ` H.J. Lu
  0 siblings, 0 replies; 3+ messages in thread
From: H.J. Lu @ 2020-12-14 15:14 UTC (permalink / raw)
  To: Carlos O'Donell; +Cc: GNU C Library

[-- Attachment #1: Type: text/plain, Size: 1948 bytes --]

On Mon, Dec 14, 2020 at 6:59 AM Carlos O'Donell <carlos@redhat.com> wrote:
>
> On 12/13/20 9:51 AM, H.J. Lu via Libc-alpha wrote:
> > Since the default REP MOVSB threshold is 2048 * (vector size / 16),
> > remove its default tunable value so that the correct default value
> > will be set correctly by init_cacheinfo ().
>
> This is a failure in the tunables framework. We should have internal
> APIs to detect:
> * Set by user.
> * Still set to default.
> I've run into this before when we were looking at some of the other
> tunables for x86. Please file an enhancement request for this and
> then we can look at implementing this and adding back the defaults.

https://sourceware.org/bugzilla/show_bug.cgi?id=27069

> OK with the typo fix and upstream bug reference.

Fixed.

> Reviewed-by: Carlos O'Donell <carlos@redhat.com>
>
> > ---
> >  sysdeps/x86/dl-tunables.list | 5 +++--
> >  1 file changed, 3 insertions(+), 2 deletions(-)
> >
> > diff --git a/sysdeps/x86/dl-tunables.list b/sysdeps/x86/dl-tunables.list
> > index 1a4a93a070..348616fb69 100644
> > --- a/sysdeps/x86/dl-tunables.list
> > +++ b/sysdeps/x86/dl-tunables.list
> > @@ -39,9 +39,10 @@ glibc {
> >        # REP MOVSB.  Since larger register size can move more data with a
> >        # single load and store, the threshold is higher with larger register
> >        # size.  Note: Since the REP MOVSB threshold must be greater than 8
> > -      # times of vector size, the minium value must be updated at run-time.
> > +      # times of vector size and the default value is 2048 * (vector size
> > +      # / 16), the default value and the minium value must be updated at
>
> s/minium/minimum/g
>
> Please add references to upstream bug.

Fixed.

> > +      # run-time.
> >        minval: 1
> > -      default: 2048
> >      }
> >      x86_rep_stosb_threshold {
> >        type: SIZE_T
> >
>
>
> --
> Cheers,
> Carlos.
>

Here is the patch I am checking in.

Thanks.

-- 
H.J.

[-- Attachment #2: 0001-x86-Remove-the-default-REP-MOVSB-threshold-tunable-v.patch --]
[-- Type: text/x-patch, Size: 1536 bytes --]

From 07f5e6c5e2f91854024c6294e436418c504fe66b Mon Sep 17 00:00:00 2001
From: "H.J. Lu" <hjl.tools@gmail.com>
Date: Sun, 13 Dec 2020 04:56:41 -0800
Subject: [PATCH] x86: Remove the default REP MOVSB threshold tunable value [BZ
 #27061]

Since we can't tell if the tunable value is set by user or not:

https://sourceware.org/bugzilla/show_bug.cgi?id=27069

remove the default REP MOVSB threshold tunable value so that the correct
default value will be set correctly by init_cacheinfo ().

Reviewed-by: Carlos O'Donell <carlos@redhat.com>
---
 sysdeps/x86/dl-tunables.list | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/sysdeps/x86/dl-tunables.list b/sysdeps/x86/dl-tunables.list
index 1a4a93a070..bc26e9fa01 100644
--- a/sysdeps/x86/dl-tunables.list
+++ b/sysdeps/x86/dl-tunables.list
@@ -39,9 +39,11 @@ glibc {
       # REP MOVSB.  Since larger register size can move more data with a
       # single load and store, the threshold is higher with larger register
       # size.  Note: Since the REP MOVSB threshold must be greater than 8
-      # times of vector size, the minium value must be updated at run-time.
+      # times of vector size and the default value is 2048 * (vector size
+      # / 16), the default value and the minimum value must be updated at
+      # run-time.  NB: Don't set the default value since we can't tell if
+      # the tunable value is set by user or not [BZ #27069].
       minval: 1
-      default: 2048
     }
     x86_rep_stosb_threshold {
       type: SIZE_T
-- 
2.29.2


^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2020-12-14 15:15 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-12-13 14:51 [PATCH] x86: Remove the default REP MOVSB threshold tunable value [BZ #27061] H.J. Lu
2020-12-14 14:59 ` Carlos O'Donell
2020-12-14 15:14   ` H.J. Lu

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).