[Buildroot] [PATCH] package/openblas: allow disabling multithreading

[Buildroot] [PATCH] package/openblas: allow disabling multithreading

[Thomas De Schampheleire]

From: Thomas De Schampheleire <thomas.de_schampheleire@nokia.com>

Buildroot would automatically enable multithreading in OpenBLAS if the
architecture supports it. However, one may want to avoid OpenBLAS creating
threads itself and configure single-threaded operation. To accommodate this
use case, add a config option for multithreading.

When multithreading is disabled, but OpenBLAS functions are called in the
same application by multiple threads, then locking is mandatory. The
USE_LOCKING flag was added in version 0.3.7 with following release note:

    a new option USE_LOCKING was added to ensure thread safety when OpenBLAS
    itself is built without multithreading but will be called from multiple
    threads.

However, if one knows that OpenBLAS will only be called from single-threaded
applications, then passing USE_LOCKING is not necessary, so make it a config
option too.

Signed-off-by: Thomas De Schampheleire <thomas.de_schampheleire@nokia.com>
---
 package/openblas/Config.in   | 21 +++++++++++++++++++++
 package/openblas/openblas.mk |  8 +++++++-
 2 files changed, 28 insertions(+), 1 deletion(-)

diff --git a/package/openblas/Config.in b/package/openblas/Config.in
index 3f658c862a..e09f3d0c5a 100644
--- a/package/openblas/Config.in
+++ b/package/openblas/Config.in
@@ -73,4 +73,25 @@ config BR2_PACKAGE_OPENBLAS_TARGET
 	string "OpenBLAS target CPU"
 	default BR2_PACKAGE_OPENBLAS_DEFAULT_TARGET
 
+config BR2_PACKAGE_OPENBLAS_USE_THREAD
+	bool "use multithreading"
+	default y
+	depends on BR2_TOOLCHAIN_HAS_THREADS
+	depends on !BR2_STATIC_LIBS
+	help
+	  Tell OpenBLAS to use multithreading, by passing USE_THREAD=1.
+
+config BR2_PACKAGE_OPENBLAS_USE_LOCKING
+	bool "use locking"
+	default y
+	depends on BR2_TOOLCHAIN_HAS_THREADS
+	help
+	  Tell OpenBLAS to use locking, by passing USE_LOCKING=1.
+	  Locking is implicitly enabled when USE_THREAD=1, in which case
+	  disabling this option will not have an effect.
+	  However, if USE_THREAD=0 (i.e. OpenBLAS itself will run in
+	  single-threaded mode) but an application makes OpenBLAS
+	  function calls from multiple threads, then locking is
+	  mandatory for correct operation.
+
 endif
diff --git a/package/openblas/openblas.mk b/package/openblas/openblas.mk
index 9701df9148..0fd761c89c 100644
--- a/package/openblas/openblas.mk
+++ b/package/openblas/openblas.mk
@@ -25,12 +25,18 @@ OPENBLAS_MAKE_OPTS += ONLY_CBLAS=1
 endif
 
 # Enable/Disable multi-threading (not for static-only since it uses dlfcn.h)
-ifeq ($(BR2_TOOLCHAIN_HAS_THREADS):$(BR2_STATIC_LIBS),y:)
+ifeq ($(BR2_PACKAGE_OPENBLAS_USE_THREAD),y)
 OPENBLAS_MAKE_OPTS += USE_THREAD=1
 else
 OPENBLAS_MAKE_OPTS += USE_THREAD=0
 endif
 
+ifeq ($(BR2_PACKAGE_OPENBLAS_USE_LOCKING),y)
+OPENBLAS_MAKE_OPTS += USE_LOCKING=1
+else
+OPENBLAS_MAKE_OPTS += USE_LOCKING=0
+endif
+
 # We don't know if OpenMP is available or not, so disable
 OPENBLAS_MAKE_OPTS += USE_OPENMP=0
 
-- 
2.26.2

[Buildroot] [PATCH] package/openblas: allow disabling multithreading

[Thomas De Schampheleire]

From: Thomas De Schampheleire <thomas.de_schampheleire@nokia.com>

Buildroot would automatically enable multithreading in OpenBLAS if the
architecture supports it. However, one may want to avoid OpenBLAS creating
threads itself and configure single-threaded operation. To accommodate this
use case, add a config option for multithreading.

When multithreading is disabled, but OpenBLAS functions are called in the
same application by multiple threads, then locking is mandatory. The
USE_LOCKING flag was added in version 0.3.7 with following release note:

    a new option USE_LOCKING was added to ensure thread safety when OpenBLAS
    itself is built without multithreading but will be called from multiple
    threads.

However, if one knows that OpenBLAS will only be called from single-threaded
applications, then passing USE_LOCKING is not necessary, so make it a config
option too.

Signed-off-by: Thomas De Schampheleire <thomas.de_schampheleire@nokia.com>
---

This patch was sent earlier but had no response.


 package/openblas/Config.in   | 21 +++++++++++++++++++++
 package/openblas/openblas.mk |  8 +++++++-
 2 files changed, 28 insertions(+), 1 deletion(-)

diff --git a/package/openblas/Config.in b/package/openblas/Config.in
index 3f658c862a..e09f3d0c5a 100644
--- a/package/openblas/Config.in
+++ b/package/openblas/Config.in
@@ -73,4 +73,25 @@ config BR2_PACKAGE_OPENBLAS_TARGET
 	string "OpenBLAS target CPU"
 	default BR2_PACKAGE_OPENBLAS_DEFAULT_TARGET
 
+config BR2_PACKAGE_OPENBLAS_USE_THREAD
+	bool "use multithreading"
+	default y
+	depends on BR2_TOOLCHAIN_HAS_THREADS
+	depends on !BR2_STATIC_LIBS
+	help
+	  Tell OpenBLAS to use multithreading, by passing USE_THREAD=1.
+
+config BR2_PACKAGE_OPENBLAS_USE_LOCKING
+	bool "use locking"
+	default y
+	depends on BR2_TOOLCHAIN_HAS_THREADS
+	help
+	  Tell OpenBLAS to use locking, by passing USE_LOCKING=1.
+	  Locking is implicitly enabled when USE_THREAD=1, in which case
+	  disabling this option will not have an effect.
+	  However, if USE_THREAD=0 (i.e. OpenBLAS itself will run in
+	  single-threaded mode) but an application makes OpenBLAS
+	  function calls from multiple threads, then locking is
+	  mandatory for correct operation.
+
 endif
diff --git a/package/openblas/openblas.mk b/package/openblas/openblas.mk
index 9701df9148..0fd761c89c 100644
--- a/package/openblas/openblas.mk
+++ b/package/openblas/openblas.mk
@@ -25,12 +25,18 @@ OPENBLAS_MAKE_OPTS += ONLY_CBLAS=1
 endif
 
 # Enable/Disable multi-threading (not for static-only since it uses dlfcn.h)
-ifeq ($(BR2_TOOLCHAIN_HAS_THREADS):$(BR2_STATIC_LIBS),y:)
+ifeq ($(BR2_PACKAGE_OPENBLAS_USE_THREAD),y)
 OPENBLAS_MAKE_OPTS += USE_THREAD=1
 else
 OPENBLAS_MAKE_OPTS += USE_THREAD=0
 endif
 
+ifeq ($(BR2_PACKAGE_OPENBLAS_USE_LOCKING),y)
+OPENBLAS_MAKE_OPTS += USE_LOCKING=1
+else
+OPENBLAS_MAKE_OPTS += USE_LOCKING=0
+endif
+
 # We don't know if OpenMP is available or not, so disable
 OPENBLAS_MAKE_OPTS += USE_OPENMP=0
 
-- 
2.26.2

[Buildroot] [PATCH] package/openblas: allow disabling multithreading

[Thomas Petazzoni]

Hello Thomas,

On Wed,  2 Dec 2020 15:50:24 +0100
Thomas De Schampheleire <patrickdepinguin@gmail.com> wrote:

> +config BR2_PACKAGE_OPENBLAS_USE_THREAD
> +	bool "use multithreading"
> +	default y
> +	depends on BR2_TOOLCHAIN_HAS_THREADS
> +	depends on !BR2_STATIC_LIBS
> +	help
> +	  Tell OpenBLAS to use multithreading, by passing USE_THREAD=1.
> +
> +config BR2_PACKAGE_OPENBLAS_USE_LOCKING
> +	bool "use locking"
> +	default y
> +	depends on BR2_TOOLCHAIN_HAS_THREADS

Should this instead be:

	depends on !BR2_PACKAGE_OPENBLAS_USE_THREAD

indeed, as you're explaining in the help text, when USE_THREAD=1, the
USE_LOCKING option has no effect, as it is forcefully enabled.

And perhaps drop the "default y" ?

Thanks,

Thomas
-- 
Thomas Petazzoni, CTO, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com

[Buildroot] [PATCH] package/openblas: allow disabling multithreading

[Thomas De Schampheleire]

Hi Thomas,

First of all, best wishes for 2021 and thanks for applying my patches recently!


El s?b, 2 ene 2021 a las 17:34, Thomas Petazzoni
(<thomas.petazzoni@bootlin.com>) escribi?:
>
> Hello Thomas,
>
> On Wed,  2 Dec 2020 15:50:24 +0100
> Thomas De Schampheleire <patrickdepinguin@gmail.com> wrote:
>
> > +config BR2_PACKAGE_OPENBLAS_USE_THREAD
> > +     bool "use multithreading"
> > +     default y
> > +     depends on BR2_TOOLCHAIN_HAS_THREADS
> > +     depends on !BR2_STATIC_LIBS
> > +     help
> > +       Tell OpenBLAS to use multithreading, by passing USE_THREAD=1.
> > +
> > +config BR2_PACKAGE_OPENBLAS_USE_LOCKING
> > +     bool "use locking"
> > +     default y
> > +     depends on BR2_TOOLCHAIN_HAS_THREADS
>
> Should this instead be:
>
>         depends on !BR2_PACKAGE_OPENBLAS_USE_THREAD
>
> indeed, as you're explaining in the help text, when USE_THREAD=1, the
> USE_LOCKING option has no effect, as it is forcefully enabled.

We can indeed add this inverse dependency on
BR2_PACKAGE_OPENBLAS_USE_THREAD, this is more clear.
However, I think we also have to retain the dependency on
BR2_TOOLCHAIN_HAS_THREADS, right? Because if there is no thread
support, it does not make sense to lock.
If only depending on !BR2_PACKAGE_OPENBLAS_USE_THREAD, then the
USE_LOCKING option becomes visible if !BR2_TOOLCHAIN_HAS_THREADS .
Right?


>
> And perhaps drop the "default y" ?

I added 'default y' because the introduction of USE_LOCKING in
openblas actually lead to a regression if you were using it in
single-threaded mode from a multi-threaded application. The regression
is only at run-time and causes incorrect math results in some cases
(ugh!).
So, the 'default y' would help existing users in this situation when
upgrading Buildroot and its contained openblas.

We could remove the 'default y' too. In my own repo I can make sure to
enable USE_LOCKING explicitly. This may be fine if we assume there are
few users that use openblas, and even fewer that use it in
single-threaded mode from a multithreaded application.

Thanks,
Thomas

[Buildroot] [PATCH] package/openblas: allow disabling multithreading

[Thomas Petazzoni]

On Tue, 5 Jan 2021 15:19:22 +0100
Thomas De Schampheleire <patrickdepinguin@gmail.com> wrote:

> First of all, best wishes for 2021 and thanks for applying my patches recently!

Best wishes to you as well!

> > Should this instead be:
> >
> >         depends on !BR2_PACKAGE_OPENBLAS_USE_THREAD
> >
> > indeed, as you're explaining in the help text, when USE_THREAD=1, the
> > USE_LOCKING option has no effect, as it is forcefully enabled.  
> 
> We can indeed add this inverse dependency on
> BR2_PACKAGE_OPENBLAS_USE_THREAD, this is more clear.
> However, I think we also have to retain the dependency on
> BR2_TOOLCHAIN_HAS_THREADS, right? Because if there is no thread
> support, it does not make sense to lock.
> If only depending on !BR2_PACKAGE_OPENBLAS_USE_THREAD, then the
> USE_LOCKING option becomes visible if !BR2_TOOLCHAIN_HAS_THREADS .
> Right?

Right, seems a good idea.

> > And perhaps drop the "default y" ?  
> 
> I added 'default y' because the introduction of USE_LOCKING in
> openblas actually lead to a regression if you were using it in
> single-threaded mode from a multi-threaded application. The regression
> is only at run-time and causes incorrect math results in some cases
> (ugh!).
> So, the 'default y' would help existing users in this situation when
> upgrading Buildroot and its contained openblas.
> 
> We could remove the 'default y' too. In my own repo I can make sure to
> enable USE_LOCKING explicitly. This may be fine if we assume there are
> few users that use openblas, and even fewer that use it in
> single-threaded mode from a multithreaded application.

Right, fair enough with "default y" then.

Thomas
-- 
Thomas Petazzoni, CTO, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com