Patchwork [2/5] alsa-lib: fix noMMU build

login
register
mail settings
Submitter Thomas Petazzoni
Date March 9, 2013, 6:21 p.m.
Message ID <1362853290-15592-3-git-send-email-thomas.petazzoni@free-electrons.com>
Download mbox | patch
Permalink /patch/226370/
State Accepted
Commit d4b074554faa9451630cde47eb8378a8b0803252
Headers show

Comments

Thomas Petazzoni - March 9, 2013, 6:21 p.m.
Add a patch to use vfork() instead of fork().

Signed-off-by: Thomas Petazzoni <thomas.petazzoni@free-electrons.com>
---
 package/alsa-lib/alsa-lib-no-mmu.patch |   44 ++++++++++++++++++++++++++++++++
 package/alsa-lib/alsa-lib.mk           |    1 +
 2 files changed, 45 insertions(+)
 create mode 100644 package/alsa-lib/alsa-lib-no-mmu.patch
Peter Korsgaard - March 9, 2013, 8:15 p.m.
>>>>> "Thomas" == Thomas Petazzoni <thomas.petazzoni@free-electrons.com> writes:

 Thomas> Add a patch to use vfork() instead of fork().
 Thomas> Signed-off-by: Thomas Petazzoni <thomas.petazzoni@free-electrons.com>
 Thomas> ---
 Thomas>  package/alsa-lib/alsa-lib-no-mmu.patch |   44 ++++++++++++++++++++++++++++++++
 Thomas>  package/alsa-lib/alsa-lib.mk           |    1 +
 Thomas>  2 files changed, 45 insertions(+)
 Thomas>  create mode 100644 package/alsa-lib/alsa-lib-no-mmu.patch

 Thomas> diff --git a/package/alsa-lib/alsa-lib-no-mmu.patch b/package/alsa-lib/alsa-lib-no-mmu.patch
 Thomas> new file mode 100644
 Thomas> index 0000000..317676a
 Thomas> --- /dev/null
 Thomas> +++ b/package/alsa-lib/alsa-lib-no-mmu.patch
 Thomas> @@ -0,0 +1,44 @@
 Thomas> +Don't use fork() on noMMU platforms
 Thomas> +
 Thomas> +Signed-off-by: Thomas Petazzoni <thomas.petazzoni@free-electrons.com>
 Thomas> +
 Thomas> +Index: alsa-lib-1.0.26/configure.in
 Thomas> +===================================================================
 Thomas> +--- alsa-lib-1.0.26.orig/configure.in	2012-09-06 10:55:14.000000000 +0200
 Thomas> ++++ alsa-lib-1.0.26/configure.in	2013-03-09 16:22:08.000000000 +0100
 Thomas> +@@ -66,6 +66,8 @@
 Thomas> + AM_CONDITIONAL(ALSA_HSEARCH_R, [test "x$HAVE_HSEARCH_R" != xyes])
 Thomas> + AC_CHECK_FUNCS([uselocale])
 Thomas> + 
 Thomas> ++AC_CHECK_FUNC([fork])
 Thomas> ++
 Thomas> + SAVE_LIBRARY_VERSION
 Thomas> + AC_SUBST(LIBTOOL_VERSION_INFO)
 Thomas> + 
 Thomas> +Index: alsa-lib-1.0.26/src/pcm/pcm_direct.c
 Thomas> +===================================================================
 Thomas> +--- alsa-lib-1.0.26.orig/src/pcm/pcm_direct.c	2012-09-06 10:55:14.000000000 +0200
 Thomas> ++++ alsa-lib-1.0.26/src/pcm/pcm_direct.c	2013-03-09 16:22:51.000000000 +0100
 Thomas> +@@ -424,13 +424,21 @@
 Thomas> + 		close(dmix->server_fd);
 Thomas> + 		return ret;
 Thomas> + 	}
 Thomas> +-	
 Thomas> ++
 Thomas> ++#ifdef HAVE_FORK
 Thomas> + 	ret = fork();
 Thomas> ++#else
 Thomas> ++	ret = vfork();

Are you sending this upstream? What's the reason for the HAVE_FORK
check? Why not just always use vfork() instead? That should work fine on
mmu as well (and be a tiny bit faster).
Thomas Petazzoni - March 10, 2013, 10:48 a.m.
Dear Peter Korsgaard,

On Sat, 09 Mar 2013 21:15:33 +0100, Peter Korsgaard wrote:

> Are you sending this upstream? What's the reason for the HAVE_FORK
> check? Why not just always use vfork() instead? That should work fine on
> mmu as well (and be a tiny bit faster).

I could possibly send this upstream, yes. The reason to use HAVE_FORK
is to not change the MMU code, which probably is going to make the
patch easier to get merged upstream, and also because that's the way
the Blackfin people handled the problem.

Best regards,

Thomas
Peter Korsgaard - March 10, 2013, 8:23 p.m.
>>>>> "Thomas" == Thomas Petazzoni <thomas.petazzoni@free-electrons.com> writes:

 Thomas> Dear Peter Korsgaard,
 Thomas> On Sat, 09 Mar 2013 21:15:33 +0100, Peter Korsgaard wrote:

 >> Are you sending this upstream? What's the reason for the HAVE_FORK
 >> check? Why not just always use vfork() instead? That should work fine on
 >> mmu as well (and be a tiny bit faster).

 Thomas> I could possibly send this upstream, yes. The reason to use
 Thomas> HAVE_FORK is to not change the MMU code, which probably is
 Thomas> going to make the patch easier to get merged upstream, and also
 Thomas> because that's the way the Blackfin people handled the problem.

I would say just the opposite. As you know, you have to be careful
(because parent/child is sharing memory) when you convert from fork to
vfork, so as a maintainer I wouldn't like to only do it for a very small
subset of machines (that he probably cannot easily test himself).

Either the conversion is correct, and should be done for everyone so it
keeps working or it shouldn't be done.

Patch

diff --git a/package/alsa-lib/alsa-lib-no-mmu.patch b/package/alsa-lib/alsa-lib-no-mmu.patch
new file mode 100644
index 0000000..317676a
--- /dev/null
+++ b/package/alsa-lib/alsa-lib-no-mmu.patch
@@ -0,0 +1,44 @@ 
+Don't use fork() on noMMU platforms
+
+Signed-off-by: Thomas Petazzoni <thomas.petazzoni@free-electrons.com>
+
+Index: alsa-lib-1.0.26/configure.in
+===================================================================
+--- alsa-lib-1.0.26.orig/configure.in	2012-09-06 10:55:14.000000000 +0200
++++ alsa-lib-1.0.26/configure.in	2013-03-09 16:22:08.000000000 +0100
+@@ -66,6 +66,8 @@
+ AM_CONDITIONAL(ALSA_HSEARCH_R, [test "x$HAVE_HSEARCH_R" != xyes])
+ AC_CHECK_FUNCS([uselocale])
+ 
++AC_CHECK_FUNC([fork])
++
+ SAVE_LIBRARY_VERSION
+ AC_SUBST(LIBTOOL_VERSION_INFO)
+ 
+Index: alsa-lib-1.0.26/src/pcm/pcm_direct.c
+===================================================================
+--- alsa-lib-1.0.26.orig/src/pcm/pcm_direct.c	2012-09-06 10:55:14.000000000 +0200
++++ alsa-lib-1.0.26/src/pcm/pcm_direct.c	2013-03-09 16:22:51.000000000 +0100
+@@ -424,13 +424,21 @@
+ 		close(dmix->server_fd);
+ 		return ret;
+ 	}
+-	
++
++#ifdef HAVE_FORK
+ 	ret = fork();
++#else
++	ret = vfork();
++#endif
+ 	if (ret < 0) {
+ 		close(dmix->server_fd);
+ 		return ret;
+ 	} else if (ret == 0) {
++#ifdef HAVE_FORK
+ 		ret = fork();
++#else
++		ret = vfork();
++#endif
+ 		if (ret == 0)
+ 			server_job(dmix);
+ 		_exit(EXIT_SUCCESS);
diff --git a/package/alsa-lib/alsa-lib.mk b/package/alsa-lib/alsa-lib.mk
index ba222db..2856407 100644
--- a/package/alsa-lib/alsa-lib.mk
+++ b/package/alsa-lib/alsa-lib.mk
@@ -11,6 +11,7 @@  ALSA_LIB_LICENSE = LGPLv2.1+
 ALSA_LIB_LICENSE_FILES = COPYING
 ALSA_LIB_INSTALL_STAGING = YES
 ALSA_LIB_CFLAGS=$(TARGET_CFLAGS)
+ALSA_LIB_AUTORECONF = YES
 ALSA_LIB_CONF_OPT = --with-alsa-devdir=$(call qstrip,$(BR2_PACKAGE_ALSA_LIB_DEVDIR)) \
 		    --with-pcm-plugins="$(call qstrip,$(BR2_PACKAGE_ALSA_LIB_PCM_PLUGINS))" \
 		    --with-ctl-plugins="$(call qstrip,$(BR2_PACKAGE_ALSA_LIB_CTL_PLUGINS))" \