Patchwork Enable tunables by default

login
register
mail settings
Submitter Siddhesh Poyarekar
Date June 14, 2017, 5:36 p.m.
Message ID <1497461763-14802-1-git-send-email-siddhesh@sourceware.org>
Download mbox | patch
Permalink /patch/21010/
State New
Headers show

Comments

Siddhesh Poyarekar - June 14, 2017, 5:36 p.m.
All of the major architectures are adopting tunables as a way to add
tuning to the library, from hwcap_mask for aarch64 to HLE for s390 and
ifunc and cache geometry for x86.  Given this adoption and the fact
that we don't want additional tuning knobs to be added outside of
tunables, it makes sense to enable tunables by default using this
trivial patch.

Smoke tested on x86 to ensure that tunables code was built without
specifying it as a configure flag.  I have kept it as --enabled and
not changed it to --disable since we want to still keep the option of
different kinds of front-ends for tunables.

	* configure.ac(--enable-tunables): Enable by default.
	* configure: Regenerate.
---
 configure    | 2 +-
 configure.ac | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)
H.J. Lu - June 14, 2017, 5:52 p.m.
On Wed, Jun 14, 2017 at 10:36 AM, Siddhesh Poyarekar
<siddhesh@sourceware.org> wrote:
> All of the major architectures are adopting tunables as a way to add
> tuning to the library, from hwcap_mask for aarch64 to HLE for s390 and
> ifunc and cache geometry for x86.  Given this adoption and the fact
> that we don't want additional tuning knobs to be added outside of
> tunables, it makes sense to enable tunables by default using this
> trivial patch.
>
> Smoke tested on x86 to ensure that tunables code was built without
> specifying it as a configure flag.  I have kept it as --enabled and
> not changed it to --disable since we want to still keep the option of
> different kinds of front-ends for tunables.
>
>         * configure.ac(--enable-tunables): Enable by default.
>         * configure: Regenerate.
> ---
>  configure    | 2 +-
>  configure.ac | 2 +-
>  2 files changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/configure b/configure
> index 422482f..f9067e2 100755
> --- a/configure
> +++ b/configure
> @@ -3724,7 +3724,7 @@ fi
>  if test "${enable_tunables+set}" = set; then :
>    enableval=$enable_tunables; have_tunables=$enableval
>  else
> -  have_tunables=no
> +  have_tunables=yes
>  fi
>
>
> diff --git a/configure.ac b/configure.ac
> index 7f43042..fa6a883 100644
> --- a/configure.ac
> +++ b/configure.ac
> @@ -436,7 +436,7 @@ AC_ARG_ENABLE([tunables],
>               [AS_HELP_STRING([--enable-tunables],
>                [Enable tunables support. Known values are 'yes', 'no' and 'valstring'])],
>               [have_tunables=$enableval],
> -             [have_tunables=no])
> +             [have_tunables=yes])
>  AC_SUBST(have_tunables)
>  if test "$have_tunables" = yes; then
>    AC_DEFINE(HAVE_TUNABLES)
> --
> 2.7.4
>

LGTM.

Thanks.
Joseph Myers - June 14, 2017, 5:57 p.m.
This patch is missing an update to install.texi (with INSTALL regenerated) 
to reflect the changed default, and a corresponding NEWS entry.
Siddhesh Poyarekar - June 14, 2017, 6:08 p.m.
On Wednesday 14 June 2017 11:27 PM, Joseph Myers wrote:
> This patch is missing an update to install.texi (with INSTALL regenerated) 
> to reflect the changed default, and a corresponding NEWS entry.

Ah yes, sorry, reposted.

Siddhesh

Patch

diff --git a/configure b/configure
index 422482f..f9067e2 100755
--- a/configure
+++ b/configure
@@ -3724,7 +3724,7 @@  fi
 if test "${enable_tunables+set}" = set; then :
   enableval=$enable_tunables; have_tunables=$enableval
 else
-  have_tunables=no
+  have_tunables=yes
 fi
 
 
diff --git a/configure.ac b/configure.ac
index 7f43042..fa6a883 100644
--- a/configure.ac
+++ b/configure.ac
@@ -436,7 +436,7 @@  AC_ARG_ENABLE([tunables],
 	      [AS_HELP_STRING([--enable-tunables],
 	       [Enable tunables support. Known values are 'yes', 'no' and 'valstring'])],
 	      [have_tunables=$enableval],
-	      [have_tunables=no])
+	      [have_tunables=yes])
 AC_SUBST(have_tunables)
 if test "$have_tunables" = yes; then
   AC_DEFINE(HAVE_TUNABLES)