Issue 13159044: code review 13159044: syscall: add Cloneflags to Linux SysProcAttr. (Closed)
|
Created:
10 years, 8 months ago by remyoudompheng Modified:
10 years, 7 months ago CC:
golang-dev, bradfitz, dfc Visibility:
Public. |
Descriptionsyscall: add Cloneflags to Linux SysProcAttr.
Also use clone(2) syscall instead of fork().
Fixes issue 6214.
Patch Set 1 #Patch Set 2 : diff -r def72122b0b8 https://go.googlecode.com/hg/ #Patch Set 3 : diff -r eb5bdb19aa21 https://go.googlecode.com/hg/ #Patch Set 4 : diff -r df6fd92ec7c1 https://go.googlecode.com/hg/ #MessagesTotal messages: 9
Hello golang-dev@googlegroups.com (cc: golang-dev@googlegroups.com), I'd like you to review this change to https://go.googlecode.com/hg/
Sign in to reply to this message.
On 2013/08/23 20:33:02, remyoudompheng wrote: > Hello mailto:golang-dev@googlegroups.com (cc: mailto:golang-dev@googlegroups.com), > > I'd like you to review this change to > https://go.googlecode.com/hg/ I have mixed feelings about whether the API should allow using arbitrary Cloneflags. For example CLONE_VM and CLONE_THREAD really don't make sense here. We have tested this to implement pseudo containers and it was quite nice.
Sign in to reply to this message.
LGTM On Fri, Aug 23, 2013 at 3:33 PM, <remyoudompheng@gmail.com> wrote: > Reviewers: golang-dev1, > > Message: > Hello golang-dev@googlegroups.com (cc: golang-dev@googlegroups.com), > > I'd like you to review this change to > https://go.googlecode.com/hg/ > > > Description: > syscall: add Cloneflags to Linux SysProcAttr. > > Also use clone(2) syscall instead of fork(). > > Fixes issue 6214. > > Please review this at https://codereview.appspot.**com/13159044/<https://codereview.appspot.com/131... > > Affected files: > M src/pkg/syscall/exec_linux.go > > > Index: src/pkg/syscall/exec_linux.go > ==============================**==============================**======= > --- a/src/pkg/syscall/exec_linux.**go > +++ b/src/pkg/syscall/exec_linux.**go > @@ -20,6 +20,7 @@ > Noctty bool // Detach fd 0 from controlling terminal > Ctty int // Controlling TTY fd (Linux only) > Pdeathsig Signal // Signal that the process will get when > its parent dies (Linux only) > + Cloneflags uintptr // Flags for clone calls (Linux only) > } > > // Implemented in runtime package. > @@ -61,7 +62,7 @@ > // About to call fork. > // No more allocation or calls of non-assembly functions. > runtime_BeforeFork() > - r1, _, err1 = RawSyscall(SYS_FORK, 0, 0, 0) > + r1, _, err1 = RawSyscall6(SYS_CLONE, uintptr(SIGCHLD)|sys.**Cloneflags, > 0, 0, 0, 0, 0) > if err1 != 0 { > runtime_AfterFork() > return 0, err1 > > > -- > > ---You received this message because you are subscribed to the Google > Groups "golang-dev" group. > To unsubscribe from this group and stop receiving emails from it, send an > email to golang-dev+unsubscribe@**googlegroups.com<golang-dev%2Bunsubscribe@googlegrou... > . > For more options, visit https://groups.google.com/**groups/opt_out<https://groups.google.com/groups/o... > . >
Sign in to reply to this message.
Please update the comment which begins "About to call fork" to explain why it calls clone(2). On 24/08/2013, at 6:38, Brad Fitzpatrick <bradfitz@golang.org> wrote: > LGTM > > > > On Fri, Aug 23, 2013 at 3:33 PM, <remyoudompheng@gmail.com> wrote: >> Reviewers: golang-dev1, >> >> Message: >> Hello golang-dev@googlegroups.com (cc: golang-dev@googlegroups.com), >> >> I'd like you to review this change to >> https://go.googlecode.com/hg/ >> >> >> Description: >> syscall: add Cloneflags to Linux SysProcAttr. >> >> Also use clone(2) syscall instead of fork(). >> >> Fixes issue 6214. >> >> Please review this at https://codereview.appspot.com/13159044/ >> >> Affected files: >> M src/pkg/syscall/exec_linux.go >> >> >> Index: src/pkg/syscall/exec_linux.go >> =================================================================== >> --- a/src/pkg/syscall/exec_linux.go >> +++ b/src/pkg/syscall/exec_linux.go >> @@ -20,6 +20,7 @@ >> Noctty bool // Detach fd 0 from controlling terminal >> Ctty int // Controlling TTY fd (Linux only) >> Pdeathsig Signal // Signal that the process will get when its parent dies (Linux only) >> + Cloneflags uintptr // Flags for clone calls (Linux only) >> } >> >> // Implemented in runtime package. >> @@ -61,7 +62,7 @@ >> // About to call fork. >> // No more allocation or calls of non-assembly functions. >> runtime_BeforeFork() >> - r1, _, err1 = RawSyscall(SYS_FORK, 0, 0, 0) >> + r1, _, err1 = RawSyscall6(SYS_CLONE, uintptr(SIGCHLD)|sys.Cloneflags, 0, 0, 0, 0, 0) >> if err1 != 0 { >> runtime_AfterFork() >> return 0, err1 >> >> >> -- >> >> ---You received this message because you are subscribed to the Google Groups "golang-dev" group. >> To unsubscribe from this group and stop receiving emails from it, send an email to golang-dev+unsubscribe@googlegroups.com. >> For more options, visit https://groups.google.com/groups/opt_out. > > -- > > --- > You received this message because you are subscribed to the Google Groups "golang-dev" group. > To unsubscribe from this group and stop receiving emails from it, send an email to golang-dev+unsubscribe@googlegroups.com. > For more options, visit https://groups.google.com/groups/opt_out.
Sign in to reply to this message.
This is package syscall. You get what you deserve if you do something like that. On Aug 23, 2013 3:34 PM, <remyoudompheng@gmail.com> wrote: > On 2013/08/23 20:33:02, remyoudompheng wrote: > >> Hello mailto:golang-dev@**googlegroups.com <golang-dev@googlegroups.com>(cc: >> > mailto:golang-dev@**googlegroups.com <golang-dev@googlegroups.com>), > > I'd like you to review this change to >> https://go.googlecode.com/hg/ >> > > I have mixed feelings about whether the API should allow using arbitrary > Cloneflags. > For example CLONE_VM and CLONE_THREAD really don't make sense here. > > We have tested this to implement pseudo containers and it was quite > nice. > > https://codereview.appspot.**com/13159044/<https://codereview.appspot.com/131... > > -- > > ---You received this message because you are subscribed to the Google > Groups "golang-dev" group. > To unsubscribe from this group and stop receiving emails from it, send an > email to golang-dev+unsubscribe@**googlegroups.com<golang-dev%2Bunsubscribe@googlegrou... > . > For more options, visit https://groups.google.com/**groups/opt_out<https://groups.google.com/groups/o... > . >
Sign in to reply to this message.
*** Submitted as https://code.google.com/p/go/source/detail?r=6b3d572eb22a *** syscall: add Cloneflags to Linux SysProcAttr. Also use clone(2) syscall instead of fork(). Fixes issue 6214. R=golang-dev, bradfitz, dave CC=golang-dev https://codereview.appspot.com/13159044
Sign in to reply to this message.
There was talk about using vfork or posix_spawn or something like that in os/exec. This means we'll have to fall back to clone if cloneflags is set?
Sign in to reply to this message.
2013/9/3 Russ Cox <rsc@golang.org>: > There was talk about using vfork or posix_spawn or something like that in > os/exec. This means we'll have to fall back to clone if cloneflags is set? clone already has a CLONE_VFORK flag, can't we simply use it in combination with extra flags ?
Sign in to reply to this message.
|
