| Message ID | fba13b278df3df6a3fc7.1392224144@argentina |
|---|---|
| State | Superseded |
| Headers | show |
>>>>> "Thomas" == Thomas De Schampheleire <patrickdepinguin@gmail.com> writes: > alsa-utils uses fork, which doesn't work on targets without mmu, such as > Blackfin. Apply a patch by the Blackfin developers to fix this. > Fixes > http://autobuild.buildroot.org/results/9f8/9f8e572c9f1c677155cc7d1828371bcf572ff878 > Signed-off-by: Thomas De Schampheleire <thomas.de.schampheleire@gmail.com> What is the upstream status of this? > --- > package/alsa-utils/alsa-utils-0001-no-mmu.patch | 61 ++++++++++++++++++++ > package/alsa-utils/alsa-utils.mk | 1 + > 2 files changed, 62 insertions(+), 0 deletions(-) > diff --git a/package/alsa-utils/alsa-utils-0001-no-mmu.patch b/package/alsa-utils/alsa-utils-0001-no-mmu.patch > new file mode 100644 > --- /dev/null > +++ b/package/alsa-utils/alsa-utils-0001-no-mmu.patch > @@ -0,0 +1,61 @@ > +Signed-off-by: Aaron Wu <Aaron.wu at analog.com> > + > +Add no-mmu support for alsa-utils mainly for Blackfin > +--- > + alsactl/init_utils_run.c | 4 ++++ > + alsaloop/alsaloop.c | 8 ++++++++ > + configure.in | 1 + > + 3 files changed, 13 insertions(+) > + > +diff --git a/alsactl/init_utils_run.c b/alsactl/init_utils_run.c > +--- a/alsactl/init_utils_run.c > ++++ b/alsactl/init_utils_run.c > +@@ -89,7 +89,11 @@ int run_program0(struct space *space, > + argv[0] = program; > + } > + > ++ #ifdef HAVE_FORK > + pid = fork(); > ++ #else > ++ pid = vfork(); > ++ #endif I would be a lot happier with a patch simply doing s/fork/vfork/. If vfork is safe to use, we might as well use it everywhere - And if not, we shouldn't do it on nommu either. > + switch(pid) { > + case 0: > + /* child closes parent ends of pipes */ I'm far from a vfork expert, but as parent and child shares stack, how does it work with both calling close() on the pipes?
Hi Peter, On Tue, Feb 18, 2014 at 12:40 PM, Peter Korsgaard <jacmet@uclibc.org> wrote: >>>>>> "Thomas" == Thomas De Schampheleire <patrickdepinguin@gmail.com> writes: > > > alsa-utils uses fork, which doesn't work on targets without mmu, such as > > Blackfin. Apply a patch by the Blackfin developers to fix this. > > > Fixes > > http://autobuild.buildroot.org/results/9f8/9f8e572c9f1c677155cc7d1828371bcf572ff878 > > > Signed-off-by: Thomas De Schampheleire <thomas.de.schampheleire@gmail.com> > > What is the upstream status of this? This has also been submitted without response: http://mailman.alsa-project.org/pipermail/alsa-devel/2013-December/069749.html > > > --- > > package/alsa-utils/alsa-utils-0001-no-mmu.patch | 61 ++++++++++++++++++++ > > package/alsa-utils/alsa-utils.mk | 1 + > > 2 files changed, 62 insertions(+), 0 deletions(-) > > > diff --git a/package/alsa-utils/alsa-utils-0001-no-mmu.patch b/package/alsa-utils/alsa-utils-0001-no-mmu.patch > > new file mode 100644 > > --- /dev/null > > +++ b/package/alsa-utils/alsa-utils-0001-no-mmu.patch > > @@ -0,0 +1,61 @@ > > +Signed-off-by: Aaron Wu <Aaron.wu at analog.com> > > + > > +Add no-mmu support for alsa-utils mainly for Blackfin > > +--- > > + alsactl/init_utils_run.c | 4 ++++ > > + alsaloop/alsaloop.c | 8 ++++++++ > > + configure.in | 1 + > > + 3 files changed, 13 insertions(+) > > + > > +diff --git a/alsactl/init_utils_run.c b/alsactl/init_utils_run.c > > +--- a/alsactl/init_utils_run.c > > ++++ b/alsactl/init_utils_run.c > > +@@ -89,7 +89,11 @@ int run_program0(struct space *space, > > + argv[0] = program; > > + } > > + > > ++ #ifdef HAVE_FORK > > + pid = fork(); > > ++ #else > > ++ pid = vfork(); > > ++ #endif > > I would be a lot happier with a patch simply doing s/fork/vfork/. If > vfork is safe to use, we might as well use it everywhere - And if not, > we shouldn't do it on nommu either. It's been done before in commits c2abc61d028b9e9cc602108ce1ad03942092bed2 and d4b074554faa9451630cde47eb8378a8b0803252. But it's far from pretty code, true. > > > > + switch(pid) { > > + case 0: > > + /* child closes parent ends of pipes */ > > I'm far from a vfork expert, but as parent and child shares stack, how > does it work with both calling close() on the pipes? I'm no vfork expert either, but, from the manpage of vfork: vfork() differs from fork(2) in that the calling thread is sus‐ pended until the child terminates (either normally, by calling _exit(2), or abnormally, after delivery of a fatal signal), or it makes a call to execve(2). Until that point, the child shares all memory with its parent, including the stack. The child must not return from the current function or call exit(3), but may call _exit(2). As with fork(2), the child process created by vfork() inherits copies of various of the caller's process attributes (e.g., file descriptors, signal dispositions, and current working directory); the vfork() call differs only in the treatment of the virtual address space, as described above. so the behavior of file descriptors seems the same as in fork. The child basically does: setup input/output (pipes) execv() The parent does: setup input/output (pipes) read output from child and store it As the parent is blocked during the execution of the child, the child will first fill the pipe with its data, then exit, and only then the parent will read it. This is unlike the fork case where parent could start reading data while the child is writing into the pipe. This code (run_program) is executed when a PROGRAM directive is encountered in the alsactl configuration. From 'man alsactl_init': PROGRAM Execute external program. The key is true, if the program returns without exit code zero. The whole event environment is available to the executed program. The program's output printed to stdout is available for the RESULT key. Based on this, and the code described above, it is not a problem that the parent is blocked during execution of the child, in run_program. I therefore think the vfork should work (untested). The other introduction of fork in alsaloop/alsaloop.c is basically: if (daemonize) { daemon(); fork(); if (parent) { exit() }; } So the child is the only one doing actual work, there is no communication between parent and child. Also here, I believe the vfork change is acceptable... What should we do with your comment on the HAVE_FORK check? I'm not sure what upstream would think of either alternative... Thanks, Thomas
Dear Thomas De Schampheleire, On Tue, 18 Feb 2014 14:47:45 +0100, Thomas De Schampheleire wrote: > > I'm far from a vfork expert, but as parent and child shares stack, how > > does it work with both calling close() on the pipes? > > I'm no vfork expert either, but, from the manpage of vfork: > > vfork() differs from fork(2) in that the calling thread is sus‐ > pended until the child terminates (either normally, by calling > _exit(2), or abnormally, after delivery of a fatal signal), or it > makes a call to execve(2). Until that point, the child shares > all memory with its parent, including the stack. The child must > not return from the current function or call exit(3), but may > call _exit(2). > > As with fork(2), the child process created by vfork() inherits > copies of various of the caller's process attributes (e.g., file > descriptors, signal dispositions, and current working directory); > the vfork() call differs only in the treatment of the virtual > address space, as described above. > > > so the behavior of file descriptors seems the same as in fork. > The child basically does: > setup input/output (pipes) > execv() > > The parent does: > setup input/output (pipes) > read output from child and store it > > As the parent is blocked during the execution of the child, the child > will first fill the pipe with its data, then exit, and only then the > parent will read it. This is unlike the fork case where parent could > start reading data while the child is writing into the pipe. > > This code (run_program) is executed when a PROGRAM directive is > encountered in the alsactl configuration. From 'man alsactl_init': > > PROGRAM > Execute external program. The key is true, if the program > returns without exit code zero. The whole event environment > is available to the executed program. The program's output > printed to stdout is available for the RESULT key. > > > Based on this, and the code described above, it is not a problem that > the parent is blocked during execution of the child, in run_program. > I therefore think the vfork should work (untested). I believe that the problem Peter is pointing at is not really that the parent is blocked until the child execs, but rather that the child shares all its data (including global and local variables) with the parent, until the child execs. Example: int foo = 2; pid = fork(); if (pid == 0) { foo = 3; _exit(0); } else { sleep(1); printf("%d\n", foo); } This will show "2" because the data is not shared between the child and parent with fork(). Try the same example after replacing fork() with vfork(). The program will show "3". You can try the example above, it really shows the behavior I'm explaining. This means that if a single variable is modified by the child before it exits or execs, then fork() cannot be replaced by vfork(). Now, I looked again at the alsa-utils code in alsactl/init_utils_run.c, and I indeed don't see any variable being changed. Only file descriptors are changed. Best regards, Thomas
>>>>> "Thomas" == Thomas De Schampheleire <patrickdepinguin@gmail.com> writes: Hi, > Hi Peter, > On Tue, Feb 18, 2014 at 12:40 PM, Peter Korsgaard <jacmet@uclibc.org> wrote: >>>>>>> "Thomas" == Thomas De Schampheleire <patrickdepinguin@gmail.com> writes: >> >> > alsa-utils uses fork, which doesn't work on targets without mmu, such as >> > Blackfin. Apply a patch by the Blackfin developers to fix this. >> >> > Fixes >> > http://autobuild.buildroot.org/results/9f8/9f8e572c9f1c677155cc7d1828371bcf572ff878 >> >> > Signed-off-by: Thomas De Schampheleire <thomas.de.schampheleire@gmail.com> >> >> What is the upstream status of this? > This has also been submitted without response: > http://mailman.alsa-project.org/pipermail/alsa-devel/2013-December/069749.html :/ >> I would be a lot happier with a patch simply doing s/fork/vfork/. If >> vfork is safe to use, we might as well use it everywhere - And if not, >> we shouldn't do it on nommu either. > It's been done before in commits > c2abc61d028b9e9cc602108ce1ad03942092bed2 and > d4b074554faa9451630cde47eb8378a8b0803252. > But it's far from pretty code, true. My point is that if nommu follows a different code path than "normal" Linux (and upstream doesn't test on nommu), then it is bound to break sooner or later. >> > + switch(pid) { >> > + case 0: >> > + /* child closes parent ends of pipes */ >> >> I'm far from a vfork expert, but as parent and child shares stack, how >> does it work with both calling close() on the pipes? > I'm no vfork expert either, but, from the manpage of vfork: > vfork() differs from fork(2) in that the calling thread is sus‐ > pended until the child terminates (either normally, by calling > _exit(2), or abnormally, after delivery of a fatal signal), or it > makes a call to execve(2). Until that point, the child shares > all memory with its parent, including the stack. The child must > not return from the current function or call exit(3), but may > call _exit(2). > As with fork(2), the child process created by vfork() inherits > copies of various of the caller's process attributes (e.g., file > descriptors, signal dispositions, and current working directory); > the vfork() call differs only in the treatment of the virtual > address space, as described above. Ahh yes, I forgot that the parent thread is stalled. Then the only difference is that the devnull variable gets assigned to, but as that is not accessed in the parent it shouldn't matter. > so the behavior of file descriptors seems the same as in fork. > The child basically does: > setup input/output (pipes) > execv() > The parent does: > setup input/output (pipes) > read output from child and store it > As the parent is blocked during the execution of the child, the child > will first fill the pipe with its data, then exit, and only then the > parent will read it. This is unlike the fork case where parent could > start reading data while the child is writing into the pipe. The parent will actually get unblocked as soon as the child calls execv, but yes. > The other introduction of fork in alsaloop/alsaloop.c is basically: > if (daemonize) { > daemon(); > fork(); > if (parent) { exit() }; > } > So the child is the only one doing actual work, there is no > communication between parent and child. > Also here, I believe the vfork change is acceptable... But does the parent ever get to run so it can exit to the shell while the child runs? I guess not. > What should we do with your comment on the HAVE_FORK check? > I'm not sure what upstream would think of either alternative... As I said above, I really dislike having a seperate (largely untested) codepath for !mmu if not absolutely needed.
diff --git a/package/alsa-utils/alsa-utils-0001-no-mmu.patch b/package/alsa-utils/alsa-utils-0001-no-mmu.patch new file mode 100644 --- /dev/null +++ b/package/alsa-utils/alsa-utils-0001-no-mmu.patch @@ -0,0 +1,61 @@ +Signed-off-by: Aaron Wu <Aaron.wu at analog.com> + +Add no-mmu support for alsa-utils mainly for Blackfin +--- + alsactl/init_utils_run.c | 4 ++++ + alsaloop/alsaloop.c | 8 ++++++++ + configure.in | 1 + + 3 files changed, 13 insertions(+) + +diff --git a/alsactl/init_utils_run.c b/alsactl/init_utils_run.c +--- a/alsactl/init_utils_run.c ++++ b/alsactl/init_utils_run.c +@@ -89,7 +89,11 @@ int run_program0(struct space *space, + argv[0] = program; + } + ++ #ifdef HAVE_FORK + pid = fork(); ++ #else ++ pid = vfork(); ++ #endif + switch(pid) { + case 0: + /* child closes parent ends of pipes */ +diff --git a/alsaloop/alsaloop.c b/alsaloop/alsaloop.c +--- a/alsaloop/alsaloop.c ++++ b/alsaloop/alsaloop.c +@@ -850,14 +850,22 @@ int main(int argc, char *argv[]) + logit(LOG_CRIT, "daemon() failed: %s\n", strerror(errno)); + exit(EXIT_FAILURE); + } ++ #ifdef HAVE_FORK + i = fork(); ++ #else ++ i = vfork(); ++ #endif + if (i < 0) { + logit(LOG_CRIT, "fork() failed: %s\n", strerror(errno)); + exit(EXIT_FAILURE); + } + if (i > 0) { + /* wait(&i); */ ++ #ifdef HAVE_FORK + exit(EXIT_SUCCESS); ++ #else ++ _exit(EXIT_SUCCESS); ++ #endif + } + } + +diff --git a/configure.in b/configure.in +--- a/configure.in ++++ b/configure.in +@@ -23,6 +23,7 @@ then + AC_MSG_RESULT($CC) + fi + ++AC_CHECK_FUNC([fork]) + AC_PROG_CC + dnl AC_PROG_CXX + AC_PROG_INSTALL diff --git a/package/alsa-utils/alsa-utils.mk b/package/alsa-utils/alsa-utils.mk --- a/package/alsa-utils/alsa-utils.mk +++ b/package/alsa-utils/alsa-utils.mk @@ -9,6 +9,7 @@ ALSA_UTILS_SOURCE = alsa-utils-$(ALSA_UT ALSA_UTILS_SITE = http://alsa.cybermirror.org/utils ALSA_UTILS_LICENSE = GPLv2 ALSA_UTILS_LICENSE_FILES = COPYING +ALSA_UTILS_AUTORECONF = YES ALSA_UTILS_INSTALL_STAGING = YES ALSA_UTILS_DEPENDENCIES = host-gettext host-pkgconf alsa-lib \ $(if $(BR2_PACKAGE_NCURSES),ncurses)
alsa-utils uses fork, which doesn't work on targets without mmu, such as Blackfin. Apply a patch by the Blackfin developers to fix this. Fixes http://autobuild.buildroot.org/results/9f8/9f8e572c9f1c677155cc7d1828371bcf572ff878 Signed-off-by: Thomas De Schampheleire <thomas.de.schampheleire@gmail.com> --- package/alsa-utils/alsa-utils-0001-no-mmu.patch | 61 ++++++++++++++++++++ package/alsa-utils/alsa-utils.mk | 1 + 2 files changed, 62 insertions(+), 0 deletions(-)