Thread: Re: Patch for Makefile race against current cvs
On Fri, 9 Nov 2001, Tom Lane wrote: > Klaus Naumann <knaumann@gmx-ag.de> writes: > > It is ... the problem is that with the current Makefile there > > are always two files which need the .y files > > (i.e. preproc.c and preproc.h). If you now do a make -j3, > > make will start things in parallel. > > But they're generated by the *same action*. If make runs the same > action twice in parallel, it's make that is broken. I think your > gripe is directed to the wrong place. I don't have any gripe ! I just wanted to help a good (at least as I thought) project ... About the other thing: This is GNU Make version 3.79.1, by Richard Stallman and Roland McGrath. On Debian/Woody Linux. So well, this make runs these things in parallel. Also even if it would be make's fault I don't see what my patch makes worse. But if you don't want to apply it, you don't apply it. Later, Klaus -- Klaus Naumann Systems Administration GMX Aktiengesellschaft Riesstrasse 17, 80992 München Telefon +49.89.143 39-0 Telefax +49.89.143 39-100 mailto:knaumann@gmx-ag.de http://www.gmx.net/
Klaus Naumann <knaumann@gmx-ag.de> writes: > Also even if it would be make's fault I don't see what my patch makes > worse. But if you don't want to apply it, you don't apply it. Well, as to whether it gets applied or not, I'll defer to Peter Eisentraut who has done most of the work recently on our configure and make support. The reason I'm asking all these questions is that I want to understand what the problem really is. It seems to me that if we have a problem with these bison invocations then we are likely to have similar problems elsewhere. We need to understand why it's unsafe and what the general rule is for avoiding such mistakes in future. What bothers me is that you seem to be saying that *any* construct involving multiple outputs from one rule is unsafe in a parallel make. That strikes me as a huge restriction, and one that would surely be mentioned prominently in the gmake manual if it were real. But I can't find anything that says that. I think what you are looking at here is a gmake bug, and that you should report it to the gmake people. regards, tom lane
On Fri, 9 Nov 2001, Tom Lane wrote: > Klaus Naumann <knaumann@gmx-ag.de> writes: > > Also even if it would be make's fault I don't see what my patch makes > > worse. But if you don't want to apply it, you don't apply it. > > Well, as to whether it gets applied or not, I'll defer to Peter > Eisentraut who has done most of the work recently on our configure and > make support. The reason I'm asking all these questions is that I > want to understand what the problem really is. It seems to me that if > we have a problem with these bison invocations then we are likely to > have similar problems elsewhere. We need to understand why it's > unsafe and what the general rule is for avoiding such mistakes in > future. To me it looks like make is calling the target twice as there are two dependent files. I think this is due to a dependency of a dependency. Lemme explain: What I think is, that we enter a directory and make a parallel build. We want to compile prepare.c , so make calls the rule to build prepare.c . A second make (because it's parallel) sees a file which needs prepare.h, so it calls the rule. At that point the rule is called twice. If it's now doing things which are not supposed to work in parallel (like moving away a file) the build fails. Maybe this is completely wrong - but this is what it looks like to me ... > What bothers me is that you seem to be saying that *any* construct > involving multiple outputs from one rule is unsafe in a parallel make. I didn't say that - I just was refering to this special case. It happened to me - more then one time. Even about 20 times (while creating the patch ...) > That strikes me as a huge restriction, and one that would surely be > mentioned prominently in the gmake manual if it were real. But I can't > find anything that says that. I agree, and I don't think it's a problem which happens always. > I think what you are looking at here is a gmake bug, and that you should > report it to the gmake people. Meybe we should ask them - but somehow I have the impression that it's not a gmake problem ... CU, Klaus -- Klaus Naumann Systems Administration GMX Aktiengesellschaft Riesstrasse 17, 80992 München Telefon +49.89.143 39-0 Telefax +49.89.143 39-100 mailto:knaumann@gmx-ag.de http://www.gmx.net/
Klaus Naumann <knaumann@gmx-ag.de> writes: > To me it looks like make is calling the target twice as there are > two dependent files. > I think this is due to a dependency of a dependency. > Lemme explain: What I think is, that we enter a directory and make > a parallel build. We want to compile prepare.c , so make calls the > rule to build prepare.c . A second make (because it's parallel) > sees a file which needs prepare.h, so it calls the rule. > At that point the rule is called twice. If it's now doing things > which are not supposed to work in parallel (like moving away > a file) the build fails. Hmm. It sounds to me like the issue is not the dual-output rule as such; it's that we have dependencies on one of the output files from elsewhere in the source tree, and so we end up with multiple sub-makes entering the backend/parser directory in parallel to build that header file, and then it's broken because the rule is not safe for parallel execution (quite independently of whether it has one or two output files). But that's supposed to be taken care of by having the parse.h file updated before we start recursing into the rest of the backend --- see src/backend/Makefile. Why is that not stopping the problem? Did you see any actual failures related to the other bison invocations, or just the one in backend/parser? AFAIK the other ones generate files that are only used within their one source directory, and so they should not have this problem anyway. There should only be one sub-make looking at the other ones. regards, tom lane
On Fri, 9 Nov 2001, Tom Lane wrote: > Klaus Naumann <knaumann@gmx-ag.de> writes: > > To me it looks like make is calling the target twice as there are > > two dependent files. > > I think this is due to a dependency of a dependency. > > Lemme explain: What I think is, that we enter a directory and make > > a parallel build. We want to compile prepare.c , so make calls the > > rule to build prepare.c . A second make (because it's parallel) > > sees a file which needs prepare.h, so it calls the rule. > > At that point the rule is called twice. If it's now doing things > > which are not supposed to work in parallel (like moving away > > a file) the build fails. > > Hmm. It sounds to me like the issue is not the dual-output rule as > such; it's that we have dependencies on one of the output files from > elsewhere in the source tree, and so we end up with multiple sub-makes > entering the backend/parser directory in parallel to build that header > file, and then it's broken because the rule is not safe for parallel > execution (quite independently of whether it has one or two output > files). This might be also true. But I that far to say, that this happens within the same directory. What I mean is, the file which generate the dependencies you are talking about are in the same directory as the bison files. > But that's supposed to be taken care of by having the parse.h file > updated before we start recursing into the rest of the backend --- see > src/backend/Makefile. Why is that not stopping the problem? s.a. > Did you see any actual failures related to the other bison invocations, > or just the one in backend/parser? AFAIK the other ones generate files > that are only used within their one source directory, and so they should > not have this problem anyway. There should only be one sub-make looking > at the other ones. Yes, I also have seen races in two other directories (and decided to change all occorences of the two-file target in any Makefile because I'm under the impression that it's to dangerous ...). I have seen problems in src/backend/bootstrap and in src/pl/plpgsql/src too. Bye, Klaus -- Klaus Naumann Systems Administration GMX Aktiengesellschaft Riesstrasse 17, 80992 München Telefon +49.89.143 39-0 Telefax +49.89.143 39-100 mailto:knaumann@gmx-ag.de http://www.gmx.net/
Klaus Naumann <knaumann@gmx-ag.de> writes: > Yes, I also have seen races in two other directories > (and decided to change all occorences of the two-file > target in any Makefile because I'm under the impression > that it's to dangerous ...). Now you're back to saying that any two-target rule is unsafe under parallel make. I find that hard to believe, given the lack of any suggestion in the gmake documents that any care needs to be taken to make things safe for parallel makes. regards, tom lane
On Fri, 9 Nov 2001, Tom Lane wrote: > Klaus Naumann <knaumann@gmx-ag.de> writes: > > Yes, I also have seen races in two other directories > > (and decided to change all occorences of the two-file > > target in any Makefile because I'm under the impression > > that it's to dangerous ...). > > Now you're back to saying that any two-target rule is unsafe under > parallel make. I find that hard to believe, given the lack of any > suggestion in the gmake documents that any care needs to be taken > to make things safe for parallel makes. At least the last sentence is just plain wrong. I have seen and heard of several projects which had to change their Makefiles to make parallel builds work. A good example would be the linux kernel. If you do a make -j3 dep clean bzImage you really f*ck up things as it's all done in parallel. If you would add a rule anything: dep clean bzImage things would get messed up too (I have tried it ...). I don't count this as a bug in make ... And no, I'm not saying that any two-target rule is unsafe, I just said I removed all as I think that it's a good idea to aviod possible problems which might occur in the future. CU, Klaus -- Klaus Naumann Systems Administration GMX Aktiengesellschaft Riesstrasse 17, 80992 München Telefon +49.89.143 39-0 Telefax +49.89.143 39-100 mailto:knaumann@gmx-ag.de http://www.gmx.net/
Tom Lane writes: > What bothers me is that you seem to be saying that *any* construct > involving multiple outputs from one rule is unsafe in a parallel make. > That strikes me as a huge restriction, and one that would surely be > mentioned prominently in the gmake manual if it were real. But I can't > find anything that says that. I think we have been victims of a misunderstanding of how multiple-target rules work. The gmake manual says: "A rule with multiple targets is equivalent to writing many rules, each with one target, and all identical aside from that." So when we write this: | $(srcdir)/preproc.c $(srcdir)/preproc.h: preproc.y | $(YACC) -d $(YFLAGS) $< | mv y.tab.c $(srcdir)/preproc.c | mv y.tab.h $(srcdir)/preproc.h make reads it as this: | $(srcdir)/preproc.c: preproc.y | $(YACC) -d $(YFLAGS) $< | mv y.tab.c $(srcdir)/preproc.c | mv y.tab.h $(srcdir)/preproc.h | | $(srcdir)/preproc.h: preproc.y | $(YACC) -d $(YFLAGS) $< | mv y.tab.c $(srcdir)/preproc.c | mv y.tab.h $(srcdir)/preproc.h So it's completely conceivable that both rules get executed in parallel. (In general, you cannot optimize this away, because the actual commands may differ depending on the value of $@.) This basically means that you cannot correctly write a target that generates more than one file. The only workaround I see is to cheat about the dependencies, as Klaus' patch does: y.tab.c: preproc.y $(YACC) -d $(YFLAGS) $< $(srcdir)/preproc.c: y.tab.c mv y.tab.c $(srcdir)/preproc.c $(srcdir)/preproc.h: y.tab.c mv y.tab.h $(srcdir)/preproc.h It's the latter rule where we're technically cheating and which I was initially regarding as part of the "details I don't like", but now I think it's the best (only?) way to go. -- Peter Eisentraut peter_e@gmx.net
Peter Eisentraut <peter_e@gmx.net> writes: > y.tab.c: preproc.y > $(YACC) -d $(YFLAGS) $< > $(srcdir)/preproc.c: y.tab.c > mv y.tab.c $(srcdir)/preproc.c > $(srcdir)/preproc.h: y.tab.c > mv y.tab.h $(srcdir)/preproc.h If you're going to do that, hadn't those last two better be "cp" not "mv"? What I find particularly distressing about this is that it will mean that y.tab.c and y.tab.h have to become part of the shipped distribution tarball. That's way ugly. I'd personally sooner document the existing restriction: don't use parallel make on CVS sources. As long as cheating is the order of the day, have you thought about | $(srcdir)/preproc.c: preproc.y | $(YACC) -d $(YFLAGS) $< | mv y.tab.c $(srcdir)/preproc.c | mv y.tab.h $(srcdir)/preproc.h | | $(srcdir)/preproc.h: $(srcdir)/preproc.c Also, I'm still feeling that we are missing something fundamental about parallel make. Surely there's got to be *some* interlock in make that prevents multiple subjobs from executing the same rule in parallel. Otherwise there are just too many things that are risky. I'm not even convinced that "cp tmpfile targetfile" is totally safe under such conditions. regards, tom lane
I wrote: > Also, I'm still feeling that we are missing something fundamental about > parallel make. Surely there's got to be *some* interlock in make that > prevents multiple subjobs from executing the same rule in parallel. Sigh, I guess my expectations are too high. Some digging around in gmake 3.79.1 shows that the submakes coordinate only to the extent of limiting the *number* of simultaneous child processes, not to the extent of determining what the children are doing. I don't think that there is any really bulletproof way to invoke bison under this sort of scenario: if there are multiple submakes executing the same build rule then it's entirely likely that we'll see things like one child installing a y.tab.h file that's been truncated and only partially rewritten by another child. Yes, the second child will eventually make it good, but that's little comfort if the first child launches compiles that fail meanwhile. But we could provide some security for multiple children of a single make by changing the rules to be like $(srcdir)/parse.h: gram.y ifdef YACC $(YACC) -d $(YFLAGS) $< mv y.tab.h $(srcdir)/parse.h mv y.tab.c $(srcdir)/gram.c else @$(missing) bison $< $@ endif $(srcdir)/gram.c: $(srcdir)/parse.h Comments? regards, tom lane
Tom Lane writes: > Also, I'm still feeling that we are missing something fundamental about > parallel make. Surely there's got to be *some* interlock in make that > prevents multiple subjobs from executing the same rule in parallel. There is, but remember that the two-target rule we're looking at is actually two separate rules written in abbreviated form. And under that pretext, the two rules are lying: They don't generate exactly the file they have as the target. Unfortunately, there isn't a right way to fix that. Keep in mind that parallel make only executes the rule commands in parallel, it does not cause the dependency analysis to be distributed. In order to satisfy a dependency graph you only need to process each node once, and parallelism doesn't change that. -- Peter Eisentraut peter_e@gmx.net
Tom Lane writes: > But we could provide some security for multiple children of a single > make by changing the rules to be like > > $(srcdir)/parse.h: gram.y > ifdef YACC > $(YACC) -d $(YFLAGS) $< > mv y.tab.h $(srcdir)/parse.h > mv y.tab.c $(srcdir)/gram.c > else > @$(missing) bison $< $@ > endif > > $(srcdir)/gram.c: $(srcdir)/parse.h That seems to be okay, although I think I'd switch gram.c and parse.h for stylistic reasons. -- Peter Eisentraut peter_e@gmx.net
Peter Eisentraut <peter_e@gmx.net> writes: > Keep in mind that parallel make only executes the rule commands in > parallel, it does not cause the dependency analysis to be distributed. > In order to satisfy a dependency graph you only need to process each node > once, and parallelism doesn't change that. Well, the proposed patch eliminates the issue for a single sub-make, by artificially serializing the execution of these rules. But I'm still concerned about what happens when a sub-make running in a different directory sees a dependency on parse.h. There *is* distributed dependency analysis when you think about that scenario. regards, tom lane
Peter Eisentraut <peter_e@gmx.net> writes: > Tom Lane writes: >> But we could provide some security for multiple children of a single >> make by changing the rules to be like >> >> $(srcdir)/parse.h: gram.y >> ifdef YACC >> $(YACC) -d $(YFLAGS) $< >> mv y.tab.h $(srcdir)/parse.h >> mv y.tab.c $(srcdir)/gram.c >> else >> @$(missing) bison $< $@ >> endif >> >> $(srcdir)/gram.c: $(srcdir)/parse.h > That seems to be okay, although I think I'd switch gram.c and parse.h for > stylistic reasons. It's not real important, but I thought it would be a good idea to minimize the dependencies seen by sub-makes entering the directory from other backend directories. Those sub-makes will only be interested in parse.h, not in gram.c. regards, tom lane
I think we have found an acceptable solution. But unless Tom already applied the patches while I write this, I suggest we wait for 7.3devel. -- Peter Eisentraut peter_e@gmx.net
Peter Eisentraut <peter_e@gmx.net> writes: > I think we have found an acceptable solution. But unless Tom already > applied the patches while I write this, I suggest we wait for 7.3devel. Why wait? Do you think there's a risk of breaking something? It seemed a pretty safe change to me. I'll defer to you to apply the patch in any case. But unless you see a risk I would suggest dealing with it now, not later. regards, tom lane
> I think we have found an acceptable solution. But unless Tom already > applied the patches while I write this, I suggest we wait for 7.3devel. OK, that's all I needed to hear. If I don't hear something else, I will throw the entire thread into the patches2 archive and we can sort it out when we hit 7.3 development. -- Bruce Momjian | http://candle.pha.pa.us pgman@candle.pha.pa.us | (610) 853-3000 + If your life is a hard drive, | 830 Blythe Avenue + Christ can be your backup. | Drexel Hill, Pennsylvania 19026
On Thu, 15 Nov 2001, Bruce Momjian wrote: > > I think we have found an acceptable solution. But unless Tom already > > applied the patches while I write this, I suggest we wait for 7.3devel. > > OK, that's all I needed to hear. If I don't hear something else, I will > throw the entire thread into the patches2 archive and we can sort it out > when we hit 7.3 development. Well, it's a bugfix and I don't see what it could break (wow, Tom and me agree :))) ). So why wouldn't we want to commit it in ? CU, Klaus -- Klaus Naumann Systems Administration GMX Aktiengesellschaft Riesstrasse 17, 80992 München Telefon +49.89.143 39-0 Telefax +49.89.143 39-100 mailto:knaumann@gmx-ag.de http://www.gmx.net/
Tom Lane writes: > But we could provide some security for multiple children of a single > make by changing the rules to be like > > $(srcdir)/parse.h: gram.y > ifdef YACC > $(YACC) -d $(YFLAGS) $< > mv y.tab.h $(srcdir)/parse.h > mv y.tab.c $(srcdir)/gram.c > else > @$(missing) bison $< $@ > endif > > $(srcdir)/gram.c: $(srcdir)/parse.h So I did and tried that: | peter ~/pgsql/src/backend/parser$ make gram.c -j | bison -y -d gram.y | mv y.tab.c ./gram.c | mv y.tab.h ./parse.h | bison -y gram.y | mv -f y.tab.c gram.c | peter ~/pgsql/src/backend/parser$ Huh? The problem is that $(srcdir)/gram.c: $(srcdir)/parse.h says, "before trying to make gram.c you must make parse.h", but it does *not* says *how* to make gram.c. So when faced with that question, make runs the built-in %.y => %.c rule that you see. In backend/bootstrap you can see crash and burn as a result of this. The correct version uses the rule $(srcdir)/gram.c: $(srcdir)/parse.h ; which says, "to make gram.c, make parse.h and then run the empty command". This is what I checked in. -- Peter Eisentraut peter_e@gmx.net