| Message ID | 089f4b9d-a29a-5031-a272-e005cb5ce78c@suse.cz |
|---|---|
| State | New |
| Headers | show |
| Series | Properly detect working jobserver in gcc driver. | expand |
On Thu, Aug 01, 2019 at 04:34:09PM +0200, Martin Liška wrote: > Ok, after deeper discussion with Honza, I would like to suggest the original > patch that was about proper detection of jobserver. > > Can you please Jakub test the patch in your environment? Isn't this done too late (as in, doesn't the driver at that moment already have some files newly openend, like e.g. the @ option files? > + = ((sscanf (n, "--jobserver-auth=%d,%d", &rfd, &wfd) == 2) No need to wrap sscanf (...) == 2 into ()s. Also, you've already done a strstr, what is the point in verifying it once again that it starts with --jobserver-auth= string? And in the lto-writer.c code there is no space between sscanf and (. Jakub
On 8/1/19 4:41 PM, Jakub Jelinek wrote: > On Thu, Aug 01, 2019 at 04:34:09PM +0200, Martin Liška wrote: >> Ok, after deeper discussion with Honza, I would like to suggest the original >> patch that was about proper detection of jobserver. >> >> Can you please Jakub test the patch in your environment? > > Isn't this done too late (as in, doesn't the driver at that moment already > have some files newly openend, like e.g. the @ option files? You are right, I've reworked that. Good observation. > >> + = ((sscanf (n, "--jobserver-auth=%d,%d", &rfd, &wfd) == 2) > > No need to wrap sscanf (...) == 2 into ()s. Also, you've already done > a strstr, what is the point in verifying it once again that it starts with > --jobserver-auth= string? > And in the lto-writer.c code there is no space between sscanf and (. Yep, I simplified that. Martin > > Jakub >
On Fri, Aug 02, 2019 at 08:30:47AM +0200, Martin Liška wrote: > On 8/1/19 4:41 PM, Jakub Jelinek wrote: > > On Thu, Aug 01, 2019 at 04:34:09PM +0200, Martin Liška wrote: > >> Ok, after deeper discussion with Honza, I would like to suggest the original > >> patch that was about proper detection of jobserver. > >> > >> Can you please Jakub test the patch in your environment? > > > > Isn't this done too late (as in, doesn't the driver at that moment already > > have some files newly openend, like e.g. the @ option files? > > You are right, I've reworked that. Good observation. I was actually wrong, because while expandargv fopens new file descriptors when processing the options, it fcloses them too before it returns. Sorry for that. Can you strace if other fds are opened and not closed in the spot you had it before? Advantage of doing it there is that it will not be done for all the -E/-S/-c compilations when the linker is not spawned. > > > >> + = ((sscanf (n, "--jobserver-auth=%d,%d", &rfd, &wfd) == 2) > > > > No need to wrap sscanf (...) == 2 into ()s. Also, you've already done > > a strstr, what is the point in verifying it once again that it starts with > > --jobserver-auth= string? > > And in the lto-writer.c code there is no space between sscanf and (. > > Yep, I simplified that. Thanks. Note, your patch from yesterday also passed testing. Jakub
On 8/2/19 9:44 AM, Jakub Jelinek wrote: > On Fri, Aug 02, 2019 at 08:30:47AM +0200, Martin Liška wrote: >> On 8/1/19 4:41 PM, Jakub Jelinek wrote: >>> On Thu, Aug 01, 2019 at 04:34:09PM +0200, Martin Liška wrote: >>>> Ok, after deeper discussion with Honza, I would like to suggest the original >>>> patch that was about proper detection of jobserver. >>>> >>>> Can you please Jakub test the patch in your environment? >>> >>> Isn't this done too late (as in, doesn't the driver at that moment already >>> have some files newly openend, like e.g. the @ option files? >> >> You are right, I've reworked that. Good observation. > > I was actually wrong, because while expandargv fopens new file descriptors > when processing the options, it fcloses them too before it returns. > > Sorry for that. Ah ok. > > Can you strace if other fds are opened and not closed in the spot you had it > before? Advantage of doing it there is that it will not be done for all the > -E/-S/-c compilations when the linker is not spawned. I've used the same trick which you used and I'm attaching the output. I believe it's fine, I can't see any opened fd by GCC. Martin > >>> >>>> + = ((sscanf (n, "--jobserver-auth=%d,%d", &rfd, &wfd) == 2) >>> >>> No need to wrap sscanf (...) == 2 into ()s. Also, you've already done >>> a strstr, what is the point in verifying it once again that it starts with >>> --jobserver-auth= string? >>> And in the lto-writer.c code there is no space between sscanf and (. >> >> Yep, I simplified that. > > Thanks. Note, your patch from yesterday also passed testing. > > Jakub > dropping MAKEFLAGS in kw -j2 --jobserver-auth=3,4 -- total 0 lrwx------ 1 marxin users 64 Aug 2 10:33 0 -> /dev/pts/4 lrwx------ 1 marxin users 64 Aug 2 10:33 1 -> /dev/pts/4 lrwx------ 1 marxin users 64 Aug 2 10:33 2 -> /dev/pts/4 lrwx------ 1 marxin users 64 Aug 2 10:33 3 -> /dev/pts/2 dropping MAKEFLAGS in kw -j2 --jobserver-auth=3,4 -- total 0 lrwx------ 1 marxin users 64 Aug 2 10:33 0 -> /dev/pts/5 lrwx------ 1 marxin users 64 Aug 2 10:33 1 -> /dev/pts/5 lrwx------ 1 marxin users 64 Aug 2 10:33 2 -> /dev/pts/5 lrwx------ 1 marxin users 64 Aug 2 10:33 3 -> /dev/pts/2 dropping MAKEFLAGS in kw -j2 --jobserver-auth=3,4 -- total 0 lrwx------ 1 marxin users 64 Aug 2 10:33 0 -> /dev/pts/4 lrwx------ 1 marxin users 64 Aug 2 10:33 1 -> /dev/pts/4 lrwx------ 1 marxin users 64 Aug 2 10:33 2 -> /dev/pts/4 lrwx------ 1 marxin users 64 Aug 2 10:33 3 -> /dev/pts/2 dropping MAKEFLAGS in kw -j2 --jobserver-auth=3,4 -- total 0 lrwx------ 1 marxin users 64 Aug 2 10:33 0 -> /dev/pts/5 lrwx------ 1 marxin users 64 Aug 2 10:33 1 -> /dev/pts/5 lrwx------ 1 marxin users 64 Aug 2 10:33 2 -> /dev/pts/5 lrwx------ 1 marxin users 64 Aug 2 10:33 3 -> /dev/pts/2 dropping MAKEFLAGS in w -j16 --jobserver-auth=4,5 total 0 lrwx------ 1 marxin users 64 Aug 2 10:33 0 -> /dev/pts/5 l-wx------ 1 marxin users 64 Aug 2 10:33 1 -> /tmp/ccdu6rwA lr-x------ 1 marxin users 64 Aug 2 10:33 14 -> /dev/shm/objdir/gcc/libgcc_s.so lr-x------ 1 marxin users 64 Aug 2 10:33 17 -> /usr/lib64/libc.so l-wx------ 1 marxin users 64 Aug 2 10:33 2 -> /tmp/cc9oWLNw.le lr-x------ 1 marxin users 64 Aug 2 10:33 22 -> /dev/shm/objdir/gcc/libgcc_s.so lrwx------ 1 marxin users 64 Aug 2 10:33 3 -> /dev/pts/2 lr-x------ 1 marxin users 64 Aug 2 10:33 9 -> /usr/lib64/libm.so dropping MAKEFLAGS in w -j16 --jobserver-auth=4,5 total 0 lrwx------ 1 marxin users 64 Aug 2 10:33 0 -> /dev/pts/4 l-wx------ 1 marxin users 64 Aug 2 10:33 1 -> /tmp/ccKEFRGz lr-x------ 1 marxin users 64 Aug 2 10:33 10 -> /usr/lib64/libm.so lr-x------ 1 marxin users 64 Aug 2 10:33 14 -> /dev/shm/objdir/gcc/libgcc_s.so lr-x------ 1 marxin users 64 Aug 2 10:33 18 -> /usr/lib64/libc.so l-wx------ 1 marxin users 64 Aug 2 10:33 2 -> /tmp/ccv4FRky.le lr-x------ 1 marxin users 64 Aug 2 10:33 22 -> /dev/shm/objdir/gcc/libgcc_s.so lrwx------ 1 marxin users 64 Aug 2 10:33 3 -> /dev/pts/2 dropping MAKEFLAGS in kw -j2 --jobserver-auth=3,4 -- total 0 lrwx------ 1 marxin users 64 Aug 2 10:33 0 -> /dev/pts/5 lrwx------ 1 marxin users 64 Aug 2 10:33 1 -> /dev/pts/5 lrwx------ 1 marxin users 64 Aug 2 10:33 2 -> /dev/pts/5 lrwx------ 1 marxin users 64 Aug 2 10:33 3 -> /dev/pts/2 dropping MAKEFLAGS in kw -j2 --jobserver-auth=3,4 -- total 0 lrwx------ 1 marxin users 64 Aug 2 10:33 0 -> /dev/pts/4 lrwx------ 1 marxin users 64 Aug 2 10:33 1 -> /dev/pts/4 lrwx------ 1 marxin users 64 Aug 2 10:33 2 -> /dev/pts/4 lrwx------ 1 marxin users 64 Aug 2 10:33 3 -> /dev/pts/2 dropping MAKEFLAGS in w -j16 --jobserver-auth=4,5 total 0 lrwx------ 1 marxin users 64 Aug 2 10:33 0 -> /dev/pts/5 l-wx------ 1 marxin users 64 Aug 2 10:33 1 -> /tmp/ccrRRAfS lr-x------ 1 marxin users 64 Aug 2 10:33 10 -> /dev/shm/objdir/gcc/libgcc_s.so lr-x------ 1 marxin users 64 Aug 2 10:33 13 -> /usr/lib64/libc.so lr-x------ 1 marxin users 64 Aug 2 10:33 18 -> /dev/shm/objdir/gcc/libgcc_s.so l-wx------ 1 marxin users 64 Aug 2 10:33 2 -> /tmp/ccWJ65uO.le lrwx------ 1 marxin users 64 Aug 2 10:33 3 -> /dev/pts/2 dropping MAKEFLAGS in kw -j2 --jobserver-auth=3,4 -- total 0 lrwx------ 1 marxin users 64 Aug 2 10:33 0 -> /dev/pts/5 lrwx------ 1 marxin users 64 Aug 2 10:33 1 -> /dev/pts/5 lrwx------ 1 marxin users 64 Aug 2 10:33 2 -> /dev/pts/5 lrwx------ 1 marxin users 64 Aug 2 10:33 3 -> /dev/pts/2 dropping MAKEFLAGS in w -j16 --jobserver-auth=4,5 total 0 lrwx------ 1 marxin users 64 Aug 2 10:33 0 -> /dev/pts/4 l-wx------ 1 marxin users 64 Aug 2 10:33 1 -> /tmp/ccXIkUlY lr-x------ 1 marxin users 64 Aug 2 10:33 10 -> /usr/lib64/libm.so lr-x------ 1 marxin users 64 Aug 2 10:33 14 -> /dev/shm/objdir/gcc/libgcc_s.so lr-x------ 1 marxin users 64 Aug 2 10:33 18 -> /usr/lib64/libc.so l-wx------ 1 marxin users 64 Aug 2 10:33 2 -> /tmp/ccvNMFyU.le lr-x------ 1 marxin users 64 Aug 2 10:33 22 -> /dev/shm/objdir/gcc/libgcc_s.so lrwx------ 1 marxin users 64 Aug 2 10:33 3 -> /dev/pts/2 dropping MAKEFLAGS in kw -j2 --jobserver-auth=3,4 -- total 0 lrwx------ 1 marxin users 64 Aug 2 10:33 0 -> /dev/pts/5 lrwx------ 1 marxin users 64 Aug 2 10:33 1 -> /dev/pts/5 lrwx------ 1 marxin users 64 Aug 2 10:33 2 -> /dev/pts/5 lrwx------ 1 marxin users 64 Aug 2 10:33 3 -> /dev/pts/2 dropping MAKEFLAGS in kw -j2 --jobserver-auth=3,4 -- total 0 lrwx------ 1 marxin users 64 Aug 2 10:33 0 -> /dev/pts/5 lrwx------ 1 marxin users 64 Aug 2 10:33 1 -> /dev/pts/5 lrwx------ 1 marxin users 64 Aug 2 10:33 2 -> /dev/pts/5 lrwx------ 1 marxin users 64 Aug 2 10:33 3 -> /dev/pts/2 dropping MAKEFLAGS in kw -j2 --jobserver-auth=3,4 -- total 0 lrwx------ 1 marxin users 64 Aug 2 10:33 0 -> /dev/pts/4 lrwx------ 1 marxin users 64 Aug 2 10:33 1 -> /dev/pts/4 lrwx------ 1 marxin users 64 Aug 2 10:33 2 -> /dev/pts/4 lrwx------ 1 marxin users 64 Aug 2 10:33 3 -> /dev/pts/2 dropping MAKEFLAGS in kw -j2 --jobserver-auth=3,4 -- total 0 lrwx------ 1 marxin users 64 Aug 2 10:33 0 -> /dev/pts/4 lrwx------ 1 marxin users 64 Aug 2 10:33 1 -> /dev/pts/4 lrwx------ 1 marxin users 64 Aug 2 10:33 2 -> /dev/pts/4 lrwx------ 1 marxin users 64 Aug 2 10:33 3 -> /dev/pts/2 dropping MAKEFLAGS in kw -j2 --jobserver-auth=3,4 -- total 0 lrwx------ 1 marxin users 64 Aug 2 10:33 0 -> /dev/pts/5 lrwx------ 1 marxin users 64 Aug 2 10:33 1 -> /dev/pts/5 lrwx------ 1 marxin users 64 Aug 2 10:33 2 -> /dev/pts/5 lrwx------ 1 marxin users 64 Aug 2 10:33 3 -> /dev/pts/2 dropping MAKEFLAGS in kw -j2 --jobserver-auth=3,4 -- total 0 lrwx------ 1 marxin users 64 Aug 2 10:33 0 -> /dev/pts/4 lrwx------ 1 marxin users 64 Aug 2 10:33 1 -> /dev/pts/4 lrwx------ 1 marxin users 64 Aug 2 10:33 2 -> /dev/pts/4 lrwx------ 1 marxin users 64 Aug 2 10:33 3 -> /dev/pts/2 dropping MAKEFLAGS in kw -j2 --jobserver-auth=3,4 -- total 0 lrwx------ 1 marxin users 64 Aug 2 10:33 0 -> /dev/pts/5 lrwx------ 1 marxin users 64 Aug 2 10:33 1 -> /dev/pts/5 lrwx------ 1 marxin users 64 Aug 2 10:33 2 -> /dev/pts/5 lrwx------ 1 marxin users 64 Aug 2 10:33 3 -> /dev/pts/2 dropping MAKEFLAGS in kw -j2 --jobserver-auth=3,4 -- total 0 lrwx------ 1 marxin users 64 Aug 2 10:33 0 -> /dev/pts/5 lrwx------ 1 marxin users 64 Aug 2 10:33 1 -> /dev/pts/5 lrwx------ 1 marxin users 64 Aug 2 10:33 2 -> /dev/pts/5 lrwx------ 1 marxin users 64 Aug 2 10:33 3 -> /dev/pts/2 dropping MAKEFLAGS in kw -j2 --jobserver-auth=3,4 -- total 0 lrwx------ 1 marxin users 64 Aug 2 10:33 0 -> /dev/pts/5 lrwx------ 1 marxin users 64 Aug 2 10:33 1 -> /dev/pts/5 lrwx------ 1 marxin users 64 Aug 2 10:33 2 -> /dev/pts/5 lrwx------ 1 marxin users 64 Aug 2 10:33 3 -> /dev/pts/2 dropping MAKEFLAGS in kw -j2 --jobserver-auth=3,4 -- total 0 lrwx------ 1 marxin users 64 Aug 2 10:33 0 -> /dev/pts/5 lrwx------ 1 marxin users 64 Aug 2 10:33 1 -> /dev/pts/5 lrwx------ 1 marxin users 64 Aug 2 10:33 2 -> /dev/pts/5 lrwx------ 1 marxin users 64 Aug 2 10:33 3 -> /dev/pts/2 dropping MAKEFLAGS in kw -j2 --jobserver-auth=3,4 -- total 0 lrwx------ 1 marxin users 64 Aug 2 10:33 0 -> /dev/pts/5 lrwx------ 1 marxin users 64 Aug 2 10:33 1 -> /dev/pts/5 lrwx------ 1 marxin users 64 Aug 2 10:33 2 -> /dev/pts/5 lrwx------ 1 marxin users 64 Aug 2 10:33 3 -> /dev/pts/2 dropping MAKEFLAGS in w -j16 --jobserver-auth=4,5 total 0 lr-x------ 1 marxin users 64 Aug 2 10:33 0 -> pipe:[579323352] l-wx------ 1 marxin users 64 Aug 2 10:33 1 -> /tmp/ccozzUjb lr-x------ 1 marxin users 64 Aug 2 10:33 10 -> /usr/lib64/libm.so lr-x------ 1 marxin users 64 Aug 2 10:33 14 -> /dev/shm/objdir/gcc/libgcc_s.so lr-x------ 1 marxin users 64 Aug 2 10:33 18 -> /usr/lib64/libc.so l-wx------ 1 marxin users 64 Aug 2 10:33 2 -> /tmp/ccacysX9.le lr-x------ 1 marxin users 64 Aug 2 10:33 22 -> /dev/shm/objdir/gcc/libgcc_s.so lrwx------ 1 marxin users 64 Aug 2 10:33 3 -> /dev/pts/2 dropping MAKEFLAGS in w -j16 --jobserver-auth=4,5 total 0 lrwx------ 1 marxin users 64 Aug 2 10:33 0 -> /dev/pts/4 l-wx------ 1 marxin users 64 Aug 2 10:33 1 -> /tmp/ccozzUjb lr-x------ 1 marxin users 64 Aug 2 10:33 10 -> /usr/lib64/libm.so lr-x------ 1 marxin users 64 Aug 2 10:33 14 -> /dev/shm/objdir/gcc/libgcc_s.so lr-x------ 1 marxin users 64 Aug 2 10:33 18 -> /usr/lib64/libc.so l-wx------ 1 marxin users 64 Aug 2 10:33 2 -> /tmp/ccacysX9.le lr-x------ 1 marxin users 64 Aug 2 10:33 22 -> /dev/shm/objdir/gcc/libgcc_s.so lrwx------ 1 marxin users 64 Aug 2 10:33 3 -> /dev/pts/2 dropping MAKEFLAGS in w -j16 --jobserver-auth=11,12 dropping MAKEFLAGS in w -j16 --jobserver-auth=11,12 total 0 lr-x------ 1 marxin users 64 Aug 2 10:33 0 -> pipe:[579328123] l-wx------ 1 marxin users 64 Aug 2 10:33 1 -> /tmp/ccDCgNyq lr-x------ 1 marxin users 64 Aug 2 10:33 10 -> /usr/lib64/crtn.o lrwx------ 1 marxin users 64 Aug 2 10:33 2 -> /dev/pts/5 lrwx------ 1 marxin users 64 Aug 2 10:33 3 -> /dev/pts/2 lr-x------ 1 marxin users 64 Aug 2 10:33 4 -> /usr/lib64/crt1.o lr-x------ 1 marxin users 64 Aug 2 10:33 5 -> /usr/lib64/crti.o lr-x------ 1 marxin users 64 Aug 2 10:33 6 -> /dev/shm/objdir/gcc/crtbegin.o lr-x------ 1 marxin users 64 Aug 2 10:33 7 -> /dev/shm/objdir/gcc/testsuite/gcc/c_lto_20081125_0.o lr-x------ 1 marxin users 64 Aug 2 10:33 8 -> /dev/shm/objdir/gcc/testsuite/gcc/c_lto_20081125_1.o lr-x------ 1 marxin users 64 Aug 2 10:33 9 -> /dev/shm/objdir/gcc/crtend.o total 0 lrwx------ 1 marxin users 64 Aug 2 10:33 0 -> /dev/pts/5 l-wx------ 1 marxin users 64 Aug 2 10:33 1 -> /tmp/ccDCgNyq lr-x------ 1 marxin users 64 Aug 2 10:33 10 -> /usr/lib64/crtn.o lrwx------ 1 marxin users 64 Aug 2 10:33 2 -> /dev/pts/5 lrwx------ 1 marxin users 64 Aug 2 10:33 3 -> /dev/pts/2 lr-x------ 1 marxin users 64 Aug 2 10:33 4 -> /usr/lib64/crt1.o lr-x------ 1 marxin users 64 Aug 2 10:33 5 -> /usr/lib64/crti.o lr-x------ 1 marxin users 64 Aug 2 10:33 6 -> /dev/shm/objdir/gcc/crtbegin.o lr-x------ 1 marxin users 64 Aug 2 10:33 7 -> /dev/shm/objdir/gcc/testsuite/gcc/c_lto_20081125_0.o lr-x------ 1 marxin users 64 Aug 2 10:33 8 -> /dev/shm/objdir/gcc/testsuite/gcc/c_lto_20081125_1.o lr-x------ 1 marxin users 64 Aug 2 10:33 9 -> /dev/shm/objdir/gcc/crtend.o dropping MAKEFLAGS in kw -j2 --jobserver-auth=3,4 -- total 0 lrwx------ 1 marxin users 64 Aug 2 10:33 0 -> /dev/pts/4 lrwx------ 1 marxin users 64 Aug 2 10:33 1 -> /dev/pts/4 lrwx------ 1 marxin users 64 Aug 2 10:33 2 -> /dev/pts/4 lrwx------ 1 marxin users 64 Aug 2 10:33 3 -> /dev/pts/2 dropping MAKEFLAGS in kw -j2 --jobserver-auth=3,4 -- total 0 lrwx------ 1 marxin users 64 Aug 2 10:33 0 -> /dev/pts/4 lrwx------ 1 marxin users 64 Aug 2 10:33 1 -> /dev/pts/4 lrwx------ 1 marxin users 64 Aug 2 10:33 2 -> /dev/pts/4 lrwx------ 1 marxin users 64 Aug 2 10:33 3 -> /dev/pts/2 dropping MAKEFLAGS in kw -j2 --jobserver-auth=3,4 -- total 0 lrwx------ 1 marxin users 64 Aug 2 10:33 0 -> /dev/pts/4 lrwx------ 1 marxin users 64 Aug 2 10:33 1 -> /dev/pts/4 lrwx------ 1 marxin users 64 Aug 2 10:33 2 -> /dev/pts/4 lrwx------ 1 marxin users 64 Aug 2 10:33 3 -> /dev/pts/2 dropping MAKEFLAGS in w -j16 --jobserver-auth=11,12 dropping MAKEFLAGS in w -j16 --jobserver-auth=11,12 total 0 lrwx------ 1 marxin users 64 Aug 2 10:33 0 -> /dev/pts/4 l-wx------ 1 marxin users 64 Aug 2 10:33 1 -> /tmp/ccG4DfIN lr-x------ 1 marxin users 64 Aug 2 10:33 10 -> /usr/lib64/crtn.o lrwx------ 1 marxin users 64 Aug 2 10:33 2 -> /dev/pts/4 lrwx------ 1 marxin users 64 Aug 2 10:33 3 -> /dev/pts/2 lr-x------ 1 marxin users 64 Aug 2 10:33 4 -> /usr/lib64/crt1.o lr-x------ 1 marxin users 64 Aug 2 10:33 5 -> /usr/lib64/crti.o lr-x------ 1 marxin users 64 Aug 2 10:33 6 -> /dev/shm/objdir/gcc/crtbegin.o lr-x------ 1 marxin users 64 Aug 2 10:33 7 -> /dev/shm/objdir/gcc/testsuite/gcc/c_lto_20081125_0.o lr-x------ 1 marxin users 64 Aug 2 10:33 8 -> /dev/shm/objdir/gcc/testsuite/gcc/c_lto_20081125_1.o lr-x------ 1 marxin users 64 Aug 2 10:33 9 -> /dev/shm/objdir/gcc/crtend.o total 0 lr-x------ 1 marxin users 64 Aug 2 10:33 0 -> pipe:[579322564] l-wx------ 1 marxin users 64 Aug 2 10:33 1 -> /tmp/ccG4DfIN lr-x------ 1 marxin users 64 Aug 2 10:33 10 -> /usr/lib64/crtn.o lrwx------ 1 marxin users 64 Aug 2 10:33 2 -> /dev/pts/4 lrwx------ 1 marxin users 64 Aug 2 10:33 3 -> /dev/pts/2 lr-x------ 1 marxin users 64 Aug 2 10:33 4 -> /usr/lib64/crt1.o lr-x------ 1 marxin users 64 Aug 2 10:33 5 -> /usr/lib64/crti.o lr-x------ 1 marxin users 64 Aug 2 10:33 6 -> /dev/shm/objdir/gcc/crtbegin.o lr-x------ 1 marxin users 64 Aug 2 10:33 7 -> /dev/shm/objdir/gcc/testsuite/gcc/c_lto_20081125_0.o lr-x------ 1 marxin users 64 Aug 2 10:33 8 -> /dev/shm/objdir/gcc/testsuite/gcc/c_lto_20081125_1.o lr-x------ 1 marxin users 64 Aug 2 10:33 9 -> /dev/shm/objdir/gcc/crtend.o dropping MAKEFLAGS in kw -j2 --jobserver-auth=3,4 -- total 0 lrwx------ 1 marxin users 64 Aug 2 10:33 0 -> /dev/pts/4 lrwx------ 1 marxin users 64 Aug 2 10:33 1 -> /dev/pts/4 lrwx------ 1 marxin users 64 Aug 2 10:33 2 -> /dev/pts/4 lrwx------ 1 marxin users 64 Aug 2 10:33 3 -> /dev/pts/2 dropping MAKEFLAGS in kw -j2 --jobserver-auth=3,4 -- total 0 lrwx------ 1 marxin users 64 Aug 2 10:33 0 -> /dev/pts/4 lrwx------ 1 marxin users 64 Aug 2 10:33 1 -> /dev/pts/4 lrwx------ 1 marxin users 64 Aug 2 10:33 2 -> /dev/pts/4 lrwx------ 1 marxin users 64 Aug 2 10:33 3 -> /dev/pts/2 dropping MAKEFLAGS in kw -j2 --jobserver-auth=3,4 -- total 0 lrwx------ 1 marxin users 64 Aug 2 10:33 0 -> /dev/pts/4 lrwx------ 1 marxin users 64 Aug 2 10:33 1 -> /dev/pts/4 lrwx------ 1 marxin users 64 Aug 2 10:33 2 -> /dev/pts/4 lrwx------ 1 marxin users 64 Aug 2 10:33 3 -> /dev/pts/2 dropping MAKEFLAGS in w -j16 --jobserver-auth=4,5 total 0 lrwx------ 1 marxin users 64 Aug 2 10:33 0 -> /dev/pts/4 l-wx------ 1 marxin users 64 Aug 2 10:33 1 -> /tmp/cc6xSPub lr-x------ 1 marxin users 64 Aug 2 10:33 10 -> /dev/shm/objdir/gcc/libgcc_s.so lr-x------ 1 marxin users 64 Aug 2 10:33 13 -> /usr/lib64/libc.so lr-x------ 1 marxin users 64 Aug 2 10:33 18 -> /dev/shm/objdir/gcc/libgcc_s.so l-wx------ 1 marxin users 64 Aug 2 10:33 2 -> /tmp/ccEeSLA7.le lrwx------ 1 marxin users 64 Aug 2 10:33 3 -> /dev/pts/2 dropping MAKEFLAGS in kw -j2 --jobserver-auth=3,4 -- total 0 lrwx------ 1 marxin users 64 Aug 2 10:33 0 -> /dev/pts/4 lrwx------ 1 marxin users 64 Aug 2 10:33 1 -> /dev/pts/4 lrwx------ 1 marxin users 64 Aug 2 10:33 2 -> /dev/pts/4 lrwx------ 1 marxin users 64 Aug 2 10:33 3 -> /dev/pts/2 dropping MAKEFLAGS in kw -j2 --jobserver-auth=3,4 -- total 0 lrwx------ 1 marxin users 64 Aug 2 10:33 0 -> /dev/pts/4 lrwx------ 1 marxin users 64 Aug 2 10:33 1 -> /dev/pts/4 lrwx------ 1 marxin users 64 Aug 2 10:33 2 -> /dev/pts/4 lrwx------ 1 marxin users 64 Aug 2 10:33 3 -> /dev/pts/2 dropping MAKEFLAGS in kw -j2 --jobserver-auth=3,4 -- total 0 lrwx------ 1 marxin users 64 Aug 2 10:33 0 -> /dev/pts/4 lrwx------ 1 marxin users 64 Aug 2 10:33 1 -> /dev/pts/4 lrwx------ 1 marxin users 64 Aug 2 10:33 2 -> /dev/pts/4 lrwx------ 1 marxin users 64 Aug 2 10:33 3 -> /dev/pts/2 dropping MAKEFLAGS in w -j16 --jobserver-auth=4,5 total 0 lrwx------ 1 marxin users 64 Aug 2 10:33 0 -> /dev/pts/4 l-wx------ 1 marxin users 64 Aug 2 10:33 1 -> /tmp/cc91ZMov lr-x------ 1 marxin users 64 Aug 2 10:33 10 -> /dev/shm/objdir/gcc/libgcc_s.so lr-x------ 1 marxin users 64 Aug 2 10:33 13 -> /usr/lib64/libc.so lr-x------ 1 marxin users 64 Aug 2 10:33 18 -> /dev/shm/objdir/gcc/libgcc_s.so l-wx------ 1 marxin users 64 Aug 2 10:33 2 -> /tmp/ccXWaZKr.le lrwx------ 1 marxin users 64 Aug 2 10:33 3 -> /dev/pts/2 dropping MAKEFLAGS in kw -j2 --jobserver-auth=3,4 -- total 0 lrwx------ 1 marxin users 64 Aug 2 10:33 0 -> /dev/pts/4 lrwx------ 1 marxin users 64 Aug 2 10:33 1 -> /dev/pts/4 lrwx------ 1 marxin users 64 Aug 2 10:33 2 -> /dev/pts/4 lrwx------ 1 marxin users 64 Aug 2 10:33 3 -> /dev/pts/2 dropping MAKEFLAGS in kw -j2 --jobserver-auth=3,4 -- total 0 lrwx------ 1 marxin users 64 Aug 2 10:33 0 -> /dev/pts/4 lrwx------ 1 marxin users 64 Aug 2 10:33 1 -> /dev/pts/4 lrwx------ 1 marxin users 64 Aug 2 10:33 2 -> /dev/pts/4 lrwx------ 1 marxin users 64 Aug 2 10:33 3 -> /dev/pts/2 dropping MAKEFLAGS in w -j16 --jobserver-auth=4,5 total 0 lrwx------ 1 marxin users 64 Aug 2 10:33 0 -> /dev/pts/4 l-wx------ 1 marxin users 64 Aug 2 10:33 1 -> /tmp/ccK5b4bR lr-x------ 1 marxin users 64 Aug 2 10:33 10 -> /usr/lib64/libm.so lr-x------ 1 marxin users 64 Aug 2 10:33 15 -> /usr/lib64/libm.so lr-x------ 1 marxin users 64 Aug 2 10:33 19 -> /dev/shm/objdir/gcc/libgcc_s.so l-wx------ 1 marxin users 64 Aug 2 10:33 2 -> /tmp/cc5885XO.le lr-x------ 1 marxin users 64 Aug 2 10:33 24 -> /usr/lib64/libm.so lr-x------ 1 marxin users 64 Aug 2 10:33 28 -> /dev/shm/objdir/gcc/libgcc_s.so lrwx------ 1 marxin users 64 Aug 2 10:33 3 -> /dev/pts/2 lr-x------ 1 marxin users 64 Aug 2 10:33 32 -> /usr/lib64/libc.so lr-x------ 1 marxin users 64 Aug 2 10:33 36 -> /dev/shm/objdir/gcc/libgcc_s.so dropping MAKEFLAGS in kw -j2 --jobserver-auth=3,4 -- total 0 lrwx------ 1 marxin users 64 Aug 2 10:33 0 -> /dev/pts/4 lrwx------ 1 marxin users 64 Aug 2 10:33 1 -> /dev/pts/4 lrwx------ 1 marxin users 64 Aug 2 10:33 2 -> /dev/pts/4 lrwx------ 1 marxin users 64 Aug 2 10:33 3 -> /dev/pts/2 dropping MAKEFLAGS in w -j16 --jobserver-auth=4,5 total 0 lrwx------ 1 marxin users 64 Aug 2 10:33 0 -> /dev/pts/4 l-wx------ 1 marxin users 64 Aug 2 10:33 1 -> /tmp/ccgExLq7 lr-x------ 1 marxin users 64 Aug 2 10:33 10 -> /usr/lib64/libm.so lr-x------ 1 marxin users 64 Aug 2 10:33 14 -> /dev/shm/objdir/gcc/libgcc_s.so lr-x------ 1 marxin users 64 Aug 2 10:33 19 -> /usr/lib64/libm.so l-wx------ 1 marxin users 64 Aug 2 10:33 2 -> /tmp/ccq6akH8.le lr-x------ 1 marxin users 64 Aug 2 10:33 23 -> /dev/shm/objdir/gcc/libgcc_s.so lr-x------ 1 marxin users 64 Aug 2 10:33 27 -> /usr/lib64/libc.so lrwx------ 1 marxin users 64 Aug 2 10:33 3 -> /dev/pts/2 lr-x------ 1 marxin users 64 Aug 2 10:33 31 -> /dev/shm/objdir/gcc/libgcc_s.so
On Fri, Aug 02, 2019 at 10:47:10AM +0200, Martin Liška wrote: > > Can you strace if other fds are opened and not closed in the spot you had it > > before? Advantage of doing it there is that it will not be done for all the > > -E/-S/-c compilations when the linker is not spawned. > > I've used the same trick which you used and I'm attaching the output. > I believe it's fine, I can't see any opened fd by GCC. LGTM. > gcc/ChangeLog: > > 2019-08-02 Martin Liska <mliska@suse.cz> > > * gcc.c (driver::maybe_run_linker): Call detect_jobserver > to detect working job server. > (driver::detect_jobserver): Test whether jobserver > is active from GCC driver. That will prevent situation where > GCC is invoked from a LD plugin and the linker already uses > file descriptors suggested by make. That leads to a wrong > detection. > * gcc.h (driver): Add detect_jobserver. > * lto-wrapper.c (jobserver_active_p): Simplify sscanf by > not scanning for --jobserver-auth prefix. > --- > gcc/gcc.c | 42 ++++++++++++++++++++++++++++++++++++++++++ > gcc/gcc.h | 1 + > gcc/lto-wrapper.c | 2 +- > 3 files changed, 44 insertions(+), 1 deletion(-) > > diff --git a/gcc/gcc.c b/gcc/gcc.c > index a4323eb146e..18a07426290 100644 > --- a/gcc/gcc.c > +++ b/gcc/gcc.c > @@ -8268,6 +8268,8 @@ driver::maybe_run_linker (const char *argv0) const > { > int tmp = execution_count; > > + detect_jobserver (); > + > if (! have_c) > { > #if HAVE_LTO_PLUGIN > 0 > @@ -8357,6 +8359,46 @@ driver::final_actions () const > } > } > > +/* Detect whether jobserver is active and working. If not drop > + --jobserver-auth from MAKEFLAGS. */ > + > +void > +driver::detect_jobserver () const > +{ > + /* Detect jobserver and drop it if it's not working. */ > + const char *makeflags = env.get ("MAKEFLAGS"); > + if (makeflags != NULL) > + { > + const char *needle = "--jobserver-auth="; > + const char *n = strstr (makeflags, needle); > + if (n != NULL) > + { > + int rfd = -1; > + int wfd = -1; > + > + bool jobserver > + = (sscanf (n + strlen (needle), "%d,%d", &rfd, &wfd) == 2 > + && rfd > 0 > + && wfd > 0 > + && fcntl (rfd, F_GETFD) >= 0 > + && fcntl (wfd, F_GETFD) >= 0); > + > + /* Drop the jobserver if it's not working now. */ > + if (!jobserver) > + { > + unsigned offset = n - makeflags; > + char *dup = xstrdup (makeflags); > + dup[offset] = '\0'; > + > + const char *space = strchr (makeflags + offset, ' '); > + if (space != NULL) > + strcpy (dup + offset, space); > + xputenv (concat ("MAKEFLAGS=", dup, NULL)); > + } > + } > + } > +} > + > /* Determine what the exit code of the driver should be. */ > > int > diff --git a/gcc/gcc.h b/gcc/gcc.h > index a0a1d94c6e6..dc77dba67fb 100644 > --- a/gcc/gcc.h > +++ b/gcc/gcc.h > @@ -51,6 +51,7 @@ class driver > void do_spec_on_infiles () const; > void maybe_run_linker (const char *argv0) const; > void final_actions () const; > + void detect_jobserver () const; > int get_exit_code () const; > > private: > diff --git a/gcc/lto-wrapper.c b/gcc/lto-wrapper.c > index 353187c6043..3414adedd26 100644 > --- a/gcc/lto-wrapper.c > +++ b/gcc/lto-wrapper.c > @@ -1234,7 +1234,7 @@ jobserver_active_p (void) > int rfd = -1; > int wfd = -1; > > - return ((sscanf(n, "--jobserver-auth=%d,%d", &rfd, &wfd) == 2) > + return (sscanf (n + strlen (needle), "%d,%d", &rfd, &wfd) == 2 > && rfd > 0 > && wfd > 0 > && fcntl (rfd, F_GETFD) >= 0 > -- > 2.22.0 > Jakub
On Fri, Aug 2, 2019 at 10:50 AM Jakub Jelinek <jakub@redhat.com> wrote: > > On Fri, Aug 02, 2019 at 10:47:10AM +0200, Martin Liška wrote: > > > Can you strace if other fds are opened and not closed in the spot you had it > > > before? Advantage of doing it there is that it will not be done for all the > > > -E/-S/-c compilations when the linker is not spawned. > > > > I've used the same trick which you used and I'm attaching the output. > > I believe it's fine, I can't see any opened fd by GCC. > > LGTM. Btw, we discussed yesterday on the phone and the conclusion was to make -flto auto-detect a job-server (but not fall back to # of threads) and add -flto=auto to auto-detect a job-server and fall back to # of threads. That basically makes -flto=jobserver the default behavior which means we should document -flto=1 as a way to override jobserver detection. We also discussed carrying distribution-local patches to make GNU make expose the jobserver FD to all jobs, not just those marked to make the new default effective. Does anybody have an idea if there's another common enough make that would benefit from support? That is, do other "jobserver" implementations/APIs exist we could support or is another make used in more than 1% of build systems? Richard. > > gcc/ChangeLog: > > > > 2019-08-02 Martin Liska <mliska@suse.cz> > > > > * gcc.c (driver::maybe_run_linker): Call detect_jobserver > > to detect working job server. > > (driver::detect_jobserver): Test whether jobserver > > is active from GCC driver. That will prevent situation where > > GCC is invoked from a LD plugin and the linker already uses > > file descriptors suggested by make. That leads to a wrong > > detection. > > * gcc.h (driver): Add detect_jobserver. > > * lto-wrapper.c (jobserver_active_p): Simplify sscanf by > > not scanning for --jobserver-auth prefix. > > --- > > gcc/gcc.c | 42 ++++++++++++++++++++++++++++++++++++++++++ > > gcc/gcc.h | 1 + > > gcc/lto-wrapper.c | 2 +- > > 3 files changed, 44 insertions(+), 1 deletion(-) > > > > diff --git a/gcc/gcc.c b/gcc/gcc.c > > index a4323eb146e..18a07426290 100644 > > --- a/gcc/gcc.c > > +++ b/gcc/gcc.c > > @@ -8268,6 +8268,8 @@ driver::maybe_run_linker (const char *argv0) const > > { > > int tmp = execution_count; > > > > + detect_jobserver (); > > + > > if (! have_c) > > { > > #if HAVE_LTO_PLUGIN > 0 > > @@ -8357,6 +8359,46 @@ driver::final_actions () const > > } > > } > > > > +/* Detect whether jobserver is active and working. If not drop > > + --jobserver-auth from MAKEFLAGS. */ > > + > > +void > > +driver::detect_jobserver () const > > +{ > > + /* Detect jobserver and drop it if it's not working. */ > > + const char *makeflags = env.get ("MAKEFLAGS"); > > + if (makeflags != NULL) > > + { > > + const char *needle = "--jobserver-auth="; > > + const char *n = strstr (makeflags, needle); > > + if (n != NULL) > > + { > > + int rfd = -1; > > + int wfd = -1; > > + > > + bool jobserver > > + = (sscanf (n + strlen (needle), "%d,%d", &rfd, &wfd) == 2 > > + && rfd > 0 > > + && wfd > 0 > > + && fcntl (rfd, F_GETFD) >= 0 > > + && fcntl (wfd, F_GETFD) >= 0); > > + > > + /* Drop the jobserver if it's not working now. */ > > + if (!jobserver) > > + { > > + unsigned offset = n - makeflags; > > + char *dup = xstrdup (makeflags); > > + dup[offset] = '\0'; > > + > > + const char *space = strchr (makeflags + offset, ' '); > > + if (space != NULL) > > + strcpy (dup + offset, space); > > + xputenv (concat ("MAKEFLAGS=", dup, NULL)); > > + } > > + } > > + } > > +} > > + > > /* Determine what the exit code of the driver should be. */ > > > > int > > diff --git a/gcc/gcc.h b/gcc/gcc.h > > index a0a1d94c6e6..dc77dba67fb 100644 > > --- a/gcc/gcc.h > > +++ b/gcc/gcc.h > > @@ -51,6 +51,7 @@ class driver > > void do_spec_on_infiles () const; > > void maybe_run_linker (const char *argv0) const; > > void final_actions () const; > > + void detect_jobserver () const; > > int get_exit_code () const; > > > > private: > > diff --git a/gcc/lto-wrapper.c b/gcc/lto-wrapper.c > > index 353187c6043..3414adedd26 100644 > > --- a/gcc/lto-wrapper.c > > +++ b/gcc/lto-wrapper.c > > @@ -1234,7 +1234,7 @@ jobserver_active_p (void) > > int rfd = -1; > > int wfd = -1; > > > > - return ((sscanf(n, "--jobserver-auth=%d,%d", &rfd, &wfd) == 2) > > + return (sscanf (n + strlen (needle), "%d,%d", &rfd, &wfd) == 2 > > && rfd > 0 > > && wfd > 0 > > && fcntl (rfd, F_GETFD) >= 0 > > -- > > 2.22.0 > > > > > Jakub
> On Fri, Aug 2, 2019 at 10:50 AM Jakub Jelinek <jakub@redhat.com> wrote: > > > > On Fri, Aug 02, 2019 at 10:47:10AM +0200, Martin Liška wrote: > > > > Can you strace if other fds are opened and not closed in the spot you had it > > > > before? Advantage of doing it there is that it will not be done for all the > > > > -E/-S/-c compilations when the linker is not spawned. > > > > > > I've used the same trick which you used and I'm attaching the output. > > > I believe it's fine, I can't see any opened fd by GCC. > > > > LGTM. > > Btw, we discussed yesterday on the phone and the conclusion was to > make -flto auto-detect a job-server (but not fall back to # of threads) > and add -flto=auto to auto-detect a job-server and fall back to # of threads. > That basically makes -flto=jobserver the default behavior which means > we should document -flto=1 as a way to override jobserver detection. > > We also discussed carrying distribution-local patches to make GNU make > expose the jobserver FD to all jobs, not just those marked to make the > new default effective. Does anybody have an idea if there's another > common enough make that would benefit from support? That is, > do other "jobserver" implementations/APIs exist we could support > or is another make used in more than 1% of build systems? Ideal solution would be to provide a simple jobserver library for multithreaded tools to use (GCC is not the only one - llvm, compression programs etc) and patch make and its replacements to use it. I think Ninja is used by some larger projects now. Honza > > Richard. > > > > gcc/ChangeLog: > > > > > > 2019-08-02 Martin Liska <mliska@suse.cz> > > > > > > * gcc.c (driver::maybe_run_linker): Call detect_jobserver > > > to detect working job server. > > > (driver::detect_jobserver): Test whether jobserver > > > is active from GCC driver. That will prevent situation where > > > GCC is invoked from a LD plugin and the linker already uses > > > file descriptors suggested by make. That leads to a wrong > > > detection. > > > * gcc.h (driver): Add detect_jobserver. > > > * lto-wrapper.c (jobserver_active_p): Simplify sscanf by > > > not scanning for --jobserver-auth prefix. > > > --- > > > gcc/gcc.c | 42 ++++++++++++++++++++++++++++++++++++++++++ > > > gcc/gcc.h | 1 + > > > gcc/lto-wrapper.c | 2 +- > > > 3 files changed, 44 insertions(+), 1 deletion(-) > > > > > > diff --git a/gcc/gcc.c b/gcc/gcc.c > > > index a4323eb146e..18a07426290 100644 > > > --- a/gcc/gcc.c > > > +++ b/gcc/gcc.c > > > @@ -8268,6 +8268,8 @@ driver::maybe_run_linker (const char *argv0) const > > > { > > > int tmp = execution_count; > > > > > > + detect_jobserver (); > > > + > > > if (! have_c) > > > { > > > #if HAVE_LTO_PLUGIN > 0 > > > @@ -8357,6 +8359,46 @@ driver::final_actions () const > > > } > > > } > > > > > > +/* Detect whether jobserver is active and working. If not drop > > > + --jobserver-auth from MAKEFLAGS. */ > > > + > > > +void > > > +driver::detect_jobserver () const > > > +{ > > > + /* Detect jobserver and drop it if it's not working. */ > > > + const char *makeflags = env.get ("MAKEFLAGS"); > > > + if (makeflags != NULL) > > > + { > > > + const char *needle = "--jobserver-auth="; > > > + const char *n = strstr (makeflags, needle); > > > + if (n != NULL) > > > + { > > > + int rfd = -1; > > > + int wfd = -1; > > > + > > > + bool jobserver > > > + = (sscanf (n + strlen (needle), "%d,%d", &rfd, &wfd) == 2 > > > + && rfd > 0 > > > + && wfd > 0 > > > + && fcntl (rfd, F_GETFD) >= 0 > > > + && fcntl (wfd, F_GETFD) >= 0); > > > + > > > + /* Drop the jobserver if it's not working now. */ > > > + if (!jobserver) > > > + { > > > + unsigned offset = n - makeflags; > > > + char *dup = xstrdup (makeflags); > > > + dup[offset] = '\0'; > > > + > > > + const char *space = strchr (makeflags + offset, ' '); > > > + if (space != NULL) > > > + strcpy (dup + offset, space); > > > + xputenv (concat ("MAKEFLAGS=", dup, NULL)); > > > + } > > > + } > > > + } > > > +} > > > + > > > /* Determine what the exit code of the driver should be. */ > > > > > > int > > > diff --git a/gcc/gcc.h b/gcc/gcc.h > > > index a0a1d94c6e6..dc77dba67fb 100644 > > > --- a/gcc/gcc.h > > > +++ b/gcc/gcc.h > > > @@ -51,6 +51,7 @@ class driver > > > void do_spec_on_infiles () const; > > > void maybe_run_linker (const char *argv0) const; > > > void final_actions () const; > > > + void detect_jobserver () const; > > > int get_exit_code () const; > > > > > > private: > > > diff --git a/gcc/lto-wrapper.c b/gcc/lto-wrapper.c > > > index 353187c6043..3414adedd26 100644 > > > --- a/gcc/lto-wrapper.c > > > +++ b/gcc/lto-wrapper.c > > > @@ -1234,7 +1234,7 @@ jobserver_active_p (void) > > > int rfd = -1; > > > int wfd = -1; > > > > > > - return ((sscanf(n, "--jobserver-auth=%d,%d", &rfd, &wfd) == 2) > > > + return (sscanf (n + strlen (needle), "%d,%d", &rfd, &wfd) == 2 > > > && rfd > 0 > > > && wfd > 0 > > > && fcntl (rfd, F_GETFD) >= 0 > > > -- > > > 2.22.0 > > > > > > > > > Jakub
> On Fri, Aug 2, 2019 at 10:50 AM Jakub Jelinek <jakub@redhat.com> wrote: > > > > On Fri, Aug 02, 2019 at 10:47:10AM +0200, Martin Liška wrote: > > > > Can you strace if other fds are opened and not closed in the spot you had it > > > > before? Advantage of doing it there is that it will not be done for all the > > > > -E/-S/-c compilations when the linker is not spawned. > > > > > > I've used the same trick which you used and I'm attaching the output. > > > I believe it's fine, I can't see any opened fd by GCC. > > > > LGTM. > > Btw, we discussed yesterday on the phone and the conclusion was to > make -flto auto-detect a job-server (but not fall back to # of threads) > and add -flto=auto to auto-detect a job-server and fall back to # of threads. > That basically makes -flto=jobserver the default behavior which means > we should document -flto=1 as a way to override jobserver detection. And concerning to the yesterday discussion, my preference would still be -flto to first try presence of jobserver and default to number of threads otherwise. It seems like user friendly default to me and other tools with reasonable parallelism support usually behaves this way, too. Sure one can use -flto=auto everywhere but then one needs to use different options for differnt compilers. It would be nice to make -flto to do right hting for majority of users including those like me who cut&paste command lines from Make output and execute them by hand. The fork bombing is IMO relatively rare and it was not what Jakub ran into (it was broken jobserv detection). After all whole distro built with Martin's orriginal approach of -flto=<nthreads> which forkbombs on every possible occasion. Said that, I could live with more conservative defaults. Honza
On 8/2/19 11:15 AM, Jan Hubicka wrote: >> On Fri, Aug 2, 2019 at 10:50 AM Jakub Jelinek <jakub@redhat.com> wrote: >>> >>> On Fri, Aug 02, 2019 at 10:47:10AM +0200, Martin Liška wrote: >>>>> Can you strace if other fds are opened and not closed in the spot you had it >>>>> before? Advantage of doing it there is that it will not be done for all the >>>>> -E/-S/-c compilations when the linker is not spawned. >>>> >>>> I've used the same trick which you used and I'm attaching the output. >>>> I believe it's fine, I can't see any opened fd by GCC. >>> >>> LGTM. >> Hi. >> Btw, we discussed yesterday on the phone and the conclusion was to >> make -flto auto-detect a job-server (but not fall back to # of threads) >> and add -flto=auto to auto-detect a job-server and fall back to # of threads. >> That basically makes -flto=jobserver the default behavior which means >> we should document -flto=1 as a way to override jobserver detection. > > And concerning to the yesterday discussion, my preference would still be > -flto to first try presence of jobserver and default to number of > threads otherwise. It seems like user friendly default to me and other > tools with reasonable parallelism support usually behaves this way, too. I also like the default as Honza defined. > > Sure one can use -flto=auto everywhere but then one needs to use > different options for differnt compilers. It would be nice to make > -flto to do right hting for majority of users including those like me > who cut&paste command lines from Make output and execute them by hand. Note that our ambition is to ideally backport the patches to our gcc9 package. The auto-detection of job-server with -flto is a behavior change that will happen in the gcc9 package. To be honest I don't like the invention of 'auto' value for -flto command. That would be useful just for gcc9 right now :/ Martin > > The fork bombing is IMO relatively rare and it was not what Jakub ran > into (it was broken jobserv detection). > After all whole distro built with Martin's orriginal approach of > -flto=<nthreads> which forkbombs on every possible occasion. > > Said that, I could live with more conservative defaults. > Honza >
On Fri, Aug 2, 2019 at 11:19 AM Martin Liška <mliska@suse.cz> wrote: > > On 8/2/19 11:15 AM, Jan Hubicka wrote: > >> On Fri, Aug 2, 2019 at 10:50 AM Jakub Jelinek <jakub@redhat.com> wrote: > >>> > >>> On Fri, Aug 02, 2019 at 10:47:10AM +0200, Martin Liška wrote: > >>>>> Can you strace if other fds are opened and not closed in the spot you had it > >>>>> before? Advantage of doing it there is that it will not be done for all the > >>>>> -E/-S/-c compilations when the linker is not spawned. > >>>> > >>>> I've used the same trick which you used and I'm attaching the output. > >>>> I believe it's fine, I can't see any opened fd by GCC. > >>> > >>> LGTM. > >> > > Hi. > > >> Btw, we discussed yesterday on the phone and the conclusion was to > >> make -flto auto-detect a job-server (but not fall back to # of threads) > >> and add -flto=auto to auto-detect a job-server and fall back to # of threads. > >> That basically makes -flto=jobserver the default behavior which means > >> we should document -flto=1 as a way to override jobserver detection. > > > > And concerning to the yesterday discussion, my preference would still be > > -flto to first try presence of jobserver and default to number of > > threads otherwise. It seems like user friendly default to me and other > > tools with reasonable parallelism support usually behaves this way, too. > > I also like the default as Honza defined. > > > > > Sure one can use -flto=auto everywhere but then one needs to use > > different options for differnt compilers. It would be nice to make > > -flto to do right hting for majority of users including those like me > > who cut&paste command lines from Make output and execute them by hand. > > Note that our ambition is to ideally backport the patches to our gcc9 package. > The auto-detection of job-server with -flto is a behavior change that will happen > in the gcc9 package. > > To be honest I don't like the invention of 'auto' value for -flto command. > That would be useful just for gcc9 right now :/ Well. We teached people they need to use -flto=N or -flto=jobserver to get parallelism. Like we teached people they need to use -O2 to get any optimization. Other compilers behave differently. So what. Auto-detecting threads is nice. Suddenly trashing your system because you happen to invoke two links in parallel is not. Which is at least why changing this kind of behavior for 9.2 (or 9.3) vs. 9.1 is out of the question. And I'm not sure it is OK for 10. As with defaulting to use a job-server when present that's more reasonable. For cut&pasting you then can use -flto=auto. But it's just my opinion - I surely can adapt. Richard. > Martin > > > > > The fork bombing is IMO relatively rare and it was not what Jakub ran > > into (it was broken jobserv detection). > > After all whole distro built with Martin's orriginal approach of > > -flto=<nthreads> which forkbombs on every possible occasion. > > > > Said that, I could live with more conservative defaults. > > Honza > > >
On 8/2/19 11:55 AM, Richard Biener wrote: > On Fri, Aug 2, 2019 at 11:19 AM Martin Liška <mliska@suse.cz> wrote: >> >> On 8/2/19 11:15 AM, Jan Hubicka wrote: >>>> On Fri, Aug 2, 2019 at 10:50 AM Jakub Jelinek <jakub@redhat.com> wrote: >>>>> >>>>> On Fri, Aug 02, 2019 at 10:47:10AM +0200, Martin Liška wrote: >>>>>>> Can you strace if other fds are opened and not closed in the spot you had it >>>>>>> before? Advantage of doing it there is that it will not be done for all the >>>>>>> -E/-S/-c compilations when the linker is not spawned. >>>>>> >>>>>> I've used the same trick which you used and I'm attaching the output. >>>>>> I believe it's fine, I can't see any opened fd by GCC. >>>>> >>>>> LGTM. >>>> >> >> Hi. >> >>>> Btw, we discussed yesterday on the phone and the conclusion was to >>>> make -flto auto-detect a job-server (but not fall back to # of threads) >>>> and add -flto=auto to auto-detect a job-server and fall back to # of threads. >>>> That basically makes -flto=jobserver the default behavior which means >>>> we should document -flto=1 as a way to override jobserver detection. >>> >>> And concerning to the yesterday discussion, my preference would still be >>> -flto to first try presence of jobserver and default to number of >>> threads otherwise. It seems like user friendly default to me and other >>> tools with reasonable parallelism support usually behaves this way, too. >> >> I also like the default as Honza defined. >> >>> >>> Sure one can use -flto=auto everywhere but then one needs to use >>> different options for differnt compilers. It would be nice to make >>> -flto to do right hting for majority of users including those like me >>> who cut&paste command lines from Make output and execute them by hand. >> >> Note that our ambition is to ideally backport the patches to our gcc9 package. >> The auto-detection of job-server with -flto is a behavior change that will happen >> in the gcc9 package. >> >> To be honest I don't like the invention of 'auto' value for -flto command. >> That would be useful just for gcc9 right now :/ > > Well. We teached people they need to use -flto=N or -flto=jobserver to > get parallelism. Like we teached people they need to use -O2 to get > any optimization. > > Other compilers behave differently. So what. > > Auto-detecting threads is nice. Suddenly trashing your system > because you happen to invoke two links in parallel is not. > Which is at least why changing this kind of behavior for 9.2 (or 9.3) > vs. 9.1 is out of the question. And I'm not sure it is OK for 10. > > As with defaulting to use a job-server when present that's more > reasonable. > > For cut&pasting you then can use -flto=auto. > > But it's just my opinion - I surely can adapt. Hi. Based on what was written, I feel OK with -flto=auto. I would like to see jobserver detection to be only enabled with the 'auto' option value. That will make it consistent in GCC 9.x branch. Patch can bootstrap on x86_64-linux-gnu and survives regression tests. Ready to be installed? Thanks, Martin > > Richard. > >> Martin >> >>> >>> The fork bombing is IMO relatively rare and it was not what Jakub ran >>> into (it was broken jobserv detection). >>> After all whole distro built with Martin's orriginal approach of >>> -flto=<nthreads> which forkbombs on every possible occasion. >>> >>> Said that, I could live with more conservative defaults. >>> Honza >>> >>
I'm sending slightly updated version of the patch where I allow -flto=auto in common_handle_option. Martin
On Fri, Aug 9, 2019 at 10:11 AM Martin Liška <mliska@suse.cz> wrote: > > I'm sending slightly updated version of the patch > where I allow -flto=auto in common_handle_option. +One can also use @option{-flto=auto} to either use GNU make's +job server mode to determine the number of parallel jobs, if available. +Or the default value for @var{n} is automatically detected based +on number of cores. "Use @option{-flto=auto} to use GNU make's job server, if available, or otherwise fall back to autodetection of the number of CPU threads present in your system." OK with that. I still think that making -flto use a jobserver if detected (but _not_ use the number of CPU cores by default) makes sense as an independent change. > Martin
On 8/9/19 10:19 AM, Richard Biener wrote: > OK with that. I still think that making -flto use a jobserver if detected > (but _not_ use the number of CPU cores by default) makes > sense as an independent change. In order to address that, I'm suggesting following patch that I've been testing. Martin
On 8/9/19 2:38 PM, Martin Liška wrote: > On 8/9/19 10:19 AM, Richard Biener wrote: >> OK with that. I still think that making -flto use a jobserver if detected >> (but _not_ use the number of CPU cores by default) makes >> sense as an independent change. > > In order to address that, I'm suggesting following patch that I've been > testing. > > Martin > Hm, I take back the config changes. Martin
On 8/9/19 7:05 AM, Martin Liška wrote: > On 8/9/19 2:38 PM, Martin Liška wrote: >> On 8/9/19 10:19 AM, Richard Biener wrote: >>> OK with that. I still think that making -flto use a jobserver if detected >>> (but _not_ use the number of CPU cores by default) makes >>> sense as an independent change. >> In order to address that, I'm suggesting following patch that I've been >> testing. >> >> Martin >> > Hm, I take back the config changes. > > Martin > > > 0001-Automatically-detect-GNU-jobserver-with-flto.patch > > From 1f9c9f74a84ec3ca930bbc9525ef2185200e0ce8 Mon Sep 17 00:00:00 2001 > From: Martin Liska <mliska@suse.cz> > Date: Fri, 9 Aug 2019 14:03:11 +0200 > Subject: [PATCH] Automatically detect GNU jobserver with -flto. > > gcc/ChangeLog: > > 2019-08-09 Martin Liska <mliska@suse.cz> > > * doc/invoke.texi: Document automatic detection of jobserver. > * lto-wrapper.c (run_gcc): Detect jobserver always. OK jeff
From 0a7f15566dda99146784eb3f85e8b4547f2ab71c Mon Sep 17 00:00:00 2001 From: Martin Liska <mliska@suse.cz> Date: Thu, 1 Aug 2019 16:30:01 +0200 Subject: [PATCH] Properly detect working jobserver in gcc driver. gcc/ChangeLog: 2019-08-01 Martin Liska <mliska@suse.cz> PR lto/91313 * gcc.c (driver::maybe_run_linker): Test whether jobserver is active from GCC driver. That will prevent situation where GCC is invoked from a LD plugin and the linker already uses file descriptors suggested by make. That leads to a wrong detection. --- gcc/gcc.c | 33 +++++++++++++++++++++++++++++++++ 1 file changed, 33 insertions(+) diff --git a/gcc/gcc.c b/gcc/gcc.c index a4323eb146e..fc11abf1f88 100644 --- a/gcc/gcc.c +++ b/gcc/gcc.c @@ -8268,6 +8268,39 @@ driver::maybe_run_linker (const char *argv0) const { int tmp = execution_count; + /* Detect jobserver and drop it if it's not working. */ + const char *makeflags = env.get ("MAKEFLAGS"); + if (makeflags != NULL) + { + const char *needle = "--jobserver-auth="; + const char *n = strstr (makeflags, needle); + if (n != NULL) + { + int rfd = -1; + int wfd = -1; + + bool jobserver + = ((sscanf (n, "--jobserver-auth=%d,%d", &rfd, &wfd) == 2) + && rfd > 0 + && wfd > 0 + && fcntl (rfd, F_GETFD) >= 0 + && fcntl (wfd, F_GETFD) >= 0); + + /* Drop the jobserver if it's not working now. */ + if (!jobserver) + { + unsigned offset = n - makeflags; + char *dup = xstrdup (makeflags); + dup[offset] = '\0'; + + const char *space = strchr (makeflags + offset, ' '); + if (space != NULL) + strcpy (dup + offset, space); + xputenv (concat ("MAKEFLAGS=", dup, NULL)); + } + } + } + if (! have_c) { #if HAVE_LTO_PLUGIN > 0 -- 2.22.0