[U-Boot] [PATCH 1/2] MAKEALL: Fix kill_children

When building in parallel, make sure that we look up the children based on the the actual process group id instead of just assuming that the MAKEALL pid is the process group id.
Also ensure that logs from incomplete builds are deleted in the process.
Signed-off-by: Joe Hershberger joe.hershberger@ni.com --- MAKEALL | 26 ++++++++++++++++++++++++-- 1 file changed, 24 insertions(+), 2 deletions(-)
diff --git a/MAKEALL b/MAKEALL index 84a5c92..1f88dc5 100755 --- a/MAKEALL +++ b/MAKEALL @@ -610,6 +610,13 @@ list_target() { donep="${LOG_DIR}/._done_" skipp="${LOG_DIR}/._skip_"
+build_target_killed() { + echo "Aborted $target build." + # Remove the logs for this board since it was aborted + rm -f ${LOG_DIR}/$target.MAKELOG ${LOG_DIR}/$target.ERR + exit +} + build_target() { target=$1 build_idx=$2 @@ -622,6 +629,7 @@ build_target() { if [ $BUILD_MANY == 1 ] ; then output_dir="${OUTPUT_PREFIX}/${target}" mkdir -p "${output_dir}" + trap build_target_killed TERM else output_dir="${OUTPUT_PREFIX}" fi @@ -640,6 +648,8 @@ build_target() { fi
if [ $BUILD_MANY == 1 ] ; then + trap - TERM + ${MAKE} -s tidy
if [ -s ${LOG_DIR}/${target}.ERR ] ; then @@ -721,7 +731,9 @@ build_targets() { if [ $BUILD_MANY == 1 ] ; then build_target ${t} ${TOTAL_CNT} & else + CUR_TGT="${t}" build_target ${t} ${TOTAL_CNT} + CUR_TGT='' fi fi
@@ -745,7 +757,11 @@ build_targets() { #-----------------------------------------------------------------------
kill_children() { - kill -- "-$1" + local pgid=`ps -p $$ --no-headers -o "%r" | tr -d ' '` + local children=`pgrep -g $pgid | grep -v $$ | grep -v $pgid` + + kill $children 2> /dev/null + wait $children 2> /dev/null
exit } @@ -753,6 +769,9 @@ kill_children() { print_stats() { if [ "$ONLY_LIST" == 'y' ] ; then return ; fi
+ # Only count boards that completed + : $((TOTAL_CNT = `find ${skipp}* 2> /dev/null | wc -l`)) + rm -f ${donep}* ${skipp}*
if [ $BUILD_MANY == 1 ] && [ -e "${OUTPUT_PREFIX}/ERR" ] ; then @@ -762,6 +781,9 @@ print_stats() { WRN_LIST=`grep -riwL error ${OUTPUT_PREFIX}/ERR/` WRN_LIST=`for f in $WRN_LIST ; do echo -n " $(basename $f)" ; done` WRN_CNT=`echo $WRN_LIST | wc -w | awk '{print $1}'` + else + # Remove the logs for any board that was interrupted + rm -f ${LOG_DIR}/${CUR_TGT}.MAKELOG ${LOG_DIR}/${CUR_TGT}.ERR fi
echo "" @@ -776,7 +798,7 @@ print_stats() { echo "----------------------------------------------------------"
if [ $BUILD_MANY == 1 ] ; then - kill_children $$ & + kill_children fi
exit $RC

--continue will allow you to <ctrl-c> the MAKEALL and pick up where you left off.
--rebuild-errors will allow you to rebuild only those boards which had trouble on the last run of MAKEALL, allowing you to quickly test a simple fix on just those boards.
Signed-off-by: Joe Hershberger joe.hershberger@ni.com --- MAKEALL | 41 +++++++++++++++++++++++++++++++++-------- 1 file changed, 33 insertions(+), 8 deletions(-)
diff --git a/MAKEALL b/MAKEALL index 1f88dc5..c7ae27d 100755 --- a/MAKEALL +++ b/MAKEALL @@ -20,6 +20,8 @@ usage() -m, --maintainers List all targets and maintainer email -M, --mails List all targets and all affilated emails -C, --check Enable build checking + -n, --continue Continue (skip boards already built) + -r, --rebuild-errors Rebuild any boards that errored -h, --help This help output
Selections by these options are logically ANDed; if the same option @@ -52,8 +54,8 @@ usage() exit ${ret} }
-SHORT_OPTS="ha:c:v:s:lmMC" -LONG_OPTS="help,arch:,cpu:,vendor:,soc:,list,maintainers,mails,check" +SHORT_OPTS="ha:c:v:s:lmMCnr" +LONG_OPTS="help,arch:,cpu:,vendor:,soc:,list,maintainers,mails,check,continue,rebuild-errors"
# Option processing based on util-linux-2.13/getopt-parse.bash
@@ -73,6 +75,8 @@ SELECTED='' ONLY_LIST='' PRINT_MAINTS='' MAINTAINERS_ONLY='' +CONTINUE='' +REBUILD_ERRORS=''
while true ; do case "$1" in @@ -115,6 +119,12 @@ while true ; do -C|--check) CHECK='C=1' shift ;; + -n|--continue) + CONTINUE='y' + shift ;; + -r|--rebuild-errors) + REBUILD_ERRORS='y' + shift ;; -l|--list) ONLY_LIST='y' shift ;; @@ -198,7 +208,9 @@ fi OUTPUT_PREFIX="${BUILD_DIR}"
[ -d ${LOG_DIR} ] || mkdir "${LOG_DIR}" || exit 1 -find "${LOG_DIR}/" -type f -exec rm -f {} + +if [ "$CONTINUE" != 'y' -a "$REBUILD_ERRORS" != 'y' ] ; then + find "${LOG_DIR}/" -type f -exec rm -f {} + +fi
LIST=""
@@ -208,6 +220,7 @@ ERR_LIST="" WRN_CNT=0 WRN_LIST="" TOTAL_CNT=0 +SKIP_CNT=0 CURRENT_CNT=0 OLDEST_IDX=1 RC=0 @@ -728,12 +741,20 @@ build_targets() { : $((CURRENT_CNT += 1)) rm -f "${donep}${TOTAL_CNT}" rm -f "${skipp}${TOTAL_CNT}" - if [ $BUILD_MANY == 1 ] ; then - build_target ${t} ${TOTAL_CNT} & + if [ "$CONTINUE" = 'y' -a -e ${LOG_DIR}/$t.MAKELOG ] ; then + : $((SKIP_CNT += 1)) + touch "${donep}${TOTAL_CNT}" + elif [ "$REBUILD_ERRORS" = 'y' -a ! -e ${LOG_DIR}/$t.ERR ] ; then + : $((SKIP_CNT += 1)) + touch "${donep}${TOTAL_CNT}" else - CUR_TGT="${t}" - build_target ${t} ${TOTAL_CNT} - CUR_TGT='' + if [ $BUILD_MANY == 1 ] ; then + build_target ${t} ${TOTAL_CNT} & + else + CUR_TGT="${t}" + build_target ${t} ${TOTAL_CNT} + CUR_TGT='' + fi fi fi
@@ -786,8 +807,12 @@ print_stats() { rm -f ${LOG_DIR}/${CUR_TGT}.MAKELOG ${LOG_DIR}/${CUR_TGT}.ERR fi
+ : $((TOTAL_CNT -= ${SKIP_CNT})) echo "" echo "--------------------- SUMMARY ----------------------------" + if [ "$CONTINUE" = 'y' -o "$REBUILD_ERRORS" = 'y' ] ; then + echo "Boards skipped: ${SKIP_CNT}" + fi echo "Boards compiled: ${TOTAL_CNT}" if [ ${ERR_CNT} -gt 0 ] ; then echo "Boards with errors: ${ERR_CNT} (${ERR_LIST} )"

On Tue, Oct 30, 2012 at 6:55 PM, Joe Hershberger joe.hershberger@ni.com wrote:
--continue will allow you to <ctrl-c> the MAKEALL and pick up where you left off.
--rebuild-errors will allow you to rebuild only those boards which had trouble on the last run of MAKEALL, allowing you to quickly test a simple fix on just those boards.
Signed-off-by: Joe Hershberger joe.hershberger@ni.com
Acked-by: Simon Glass sjg@chromium.org Tested-by: Simon Glass sjg@chromium.org
MAKEALL | 41 +++++++++++++++++++++++++++++++++-------- 1 file changed, 33 insertions(+), 8 deletions(-)

On Tue, Oct 30, 2012 at 08:55:21PM -0500, Joe Hershberger wrote:
--continue will allow you to <ctrl-c> the MAKEALL and pick up where you left off.
--rebuild-errors will allow you to rebuild only those boards which had trouble on the last run of MAKEALL, allowing you to quickly test a simple fix on just those boards.
Signed-off-by: Joe Hershberger joe.hershberger@ni.com
Applied to u-boot/master, thanks!

On Tue, Oct 30, 2012 at 6:55 PM, Joe Hershberger joe.hershberger@ni.com wrote:
When building in parallel, make sure that we look up the children based on the the actual process group id instead of just assuming that the MAKEALL pid is the process group id.
Also ensure that logs from incomplete builds are deleted in the process.
Signed-off-by: Joe Hershberger joe.hershberger@ni.com
Acked-by: Simon Glass sjg@chromium.org Tested-by: Simon Glass sjg@chromium.org
MAKEALL | 26 ++++++++++++++++++++++++-- 1 file changed, 24 insertions(+), 2 deletions(-)
diff --git a/MAKEALL b/MAKEALL index 84a5c92..1f88dc5 100755 --- a/MAKEALL +++ b/MAKEALL @@ -610,6 +610,13 @@ list_target() { donep="${LOG_DIR}/._done_" skipp="${LOG_DIR}/._skip_"
+build_target_killed() {
echo "Aborted $target build."
# Remove the logs for this board since it was aborted
rm -f ${LOG_DIR}/$target.MAKELOG ${LOG_DIR}/$target.ERR
exit
+}
build_target() { target=$1 build_idx=$2 @@ -622,6 +629,7 @@ build_target() { if [ $BUILD_MANY == 1 ] ; then output_dir="${OUTPUT_PREFIX}/${target}" mkdir -p "${output_dir}"
trap build_target_killed TERM else output_dir="${OUTPUT_PREFIX}" fi
@@ -640,6 +648,8 @@ build_target() { fi
if [ $BUILD_MANY == 1 ] ; then
trap - TERM
${MAKE} -s tidy if [ -s ${LOG_DIR}/${target}.ERR ] ; then
@@ -721,7 +731,9 @@ build_targets() { if [ $BUILD_MANY == 1 ] ; then build_target ${t} ${TOTAL_CNT} & else
CUR_TGT="${t}" build_target ${t} ${TOTAL_CNT}
CUR_TGT='' fi fi
@@ -745,7 +757,11 @@ build_targets() { #-----------------------------------------------------------------------
kill_children() {
kill -- "-$1"
local pgid=`ps -p $$ --no-headers -o "%r" | tr -d ' '`
local children=`pgrep -g $pgid | grep -v $$ | grep -v $pgid`
kill $children 2> /dev/null
wait $children 2> /dev/null exit
} @@ -753,6 +769,9 @@ kill_children() { print_stats() { if [ "$ONLY_LIST" == 'y' ] ; then return ; fi
# Only count boards that completed
: $((TOTAL_CNT = `find ${skipp}* 2> /dev/null | wc -l`))
rm -f ${donep}* ${skipp}* if [ $BUILD_MANY == 1 ] && [ -e "${OUTPUT_PREFIX}/ERR" ] ; then
@@ -762,6 +781,9 @@ print_stats() { WRN_LIST=`grep -riwL error ${OUTPUT_PREFIX}/ERR/` WRN_LIST=`for f in $WRN_LIST ; do echo -n " $(basename $f)" ; done` WRN_CNT=`echo $WRN_LIST | wc -w | awk '{print $1}'`
else
# Remove the logs for any board that was interrupted
rm -f ${LOG_DIR}/${CUR_TGT}.MAKELOG ${LOG_DIR}/${CUR_TGT}.ERR fi echo ""
@@ -776,7 +798,7 @@ print_stats() { echo "----------------------------------------------------------"
if [ $BUILD_MANY == 1 ] ; then
kill_children $$ &
kill_children fi exit $RC
-- 1.7.11.5

Dear Joe Hershberger,
When building in parallel, make sure that we look up the children based on the the actual process group id instead of just assuming that the MAKEALL pid is the process group id.
Also ensure that logs from incomplete builds are deleted in the process.
Signed-off-by: Joe Hershberger joe.hershberger@ni.com
[...]
Nice $subject.
btw. is it possible to improve the u-boot build process parallelization?
Best regards, Marek Vasut

On Thu, Nov 01, 2012 at 03:32:30AM +0100, Marek Vasut wrote:
Dear Joe Hershberger,
When building in parallel, make sure that we look up the children based on the the actual process group id instead of just assuming that the MAKEALL pid is the process group id.
Also ensure that logs from incomplete builds are deleted in the process.
Signed-off-by: Joe Hershberger joe.hershberger@ni.com
[...]
Nice $subject.
btw. is it possible to improve the u-boot build process parallelization?
Probably a little, but not a lot? It's be interesting to profile and see where a single machine build has bottlenecks, if any.

On Wed, Oct 31, 2012 at 9:32 PM, Marek Vasut marek.vasut@gmail.com wrote:
Dear Joe Hershberger,
When building in parallel, make sure that we look up the children based on the the actual process group id instead of just assuming that the MAKEALL pid is the process group id.
Also ensure that logs from incomplete builds are deleted in the process.
Signed-off-by: Joe Hershberger joe.hershberger@ni.com
[...]
Nice $subject.
btw. is it possible to improve the u-boot build process parallelization?
In what way?

Dear Andy Fleming,
On Wed, Oct 31, 2012 at 9:32 PM, Marek Vasut marek.vasut@gmail.com wrote:
Dear Joe Hershberger,
When building in parallel, make sure that we look up the children based on the the actual process group id instead of just assuming that the MAKEALL pid is the process group id.
Also ensure that logs from incomplete builds are deleted in the process.
Signed-off-by: Joe Hershberger joe.hershberger@ni.com
[...]
Nice $subject.
btw. is it possible to improve the u-boot build process parallelization?
In what way?
I recall someone complained the build process wasn't parallelizable enough and thus we need this stuff to run multiple builds at once in MAKEALL.
Best regards, Marek Vasut

-----BEGIN PGP SIGNED MESSAGE----- Hash: SHA1
On 11/01/12 07:35, Marek Vasut wrote:
Dear Andy Fleming,
On Wed, Oct 31, 2012 at 9:32 PM, Marek Vasut marek.vasut@gmail.com wrote:
[snip]
btw. is it possible to improve the u-boot build process parallelization?
In what way?
I recall someone complained the build process wasn't parallelizable enough and thus we need this stuff to run multiple builds at once in MAKEALL.
I don't know if the first half of that statement is strictly true. It'd be interesting to profile a build on one of the very large boxes but I think the issue is we only have ~300 objects being built. It's hard to run up a load of 32 when you only have that many things to build. But multiplying that by the number of boards in a arch or some cpu or soc combinations is easier to run up the load, except around the tails. But, profiling, please!
- -- Tom

On Tue, Oct 30, 2012 at 08:55:20PM -0500, Joe Hershberger wrote:
When building in parallel, make sure that we look up the children based on the the actual process group id instead of just assuming that the MAKEALL pid is the process group id.
Also ensure that logs from incomplete builds are deleted in the process.
Signed-off-by: Joe Hershberger joe.hershberger@ni.com
Now I see: /home/trini/bin/uboot-build.sh: line 110: 2024 Terminated /usr/bin/time -o $TIMEFILE -f %e ./MAKEALL $SOC $MACHINE > $LOG 2>&1
With my MAKEALL wrapper. Anything we can do about that?

Hi Tom,
On Sat, Nov 3, 2012 at 5:54 PM, Tom Rini trini@ti.com wrote:
On Tue, Oct 30, 2012 at 08:55:20PM -0500, Joe Hershberger wrote:
When building in parallel, make sure that we look up the children based on the the actual process group id instead of just assuming that the MAKEALL pid is the process group id.
Also ensure that logs from incomplete builds are deleted in the process.
Signed-off-by: Joe Hershberger joe.hershberger@ni.com
Now I see: /home/trini/bin/uboot-build.sh: line 110: 2024 Terminated /usr/bin/time -o $TIMEFILE -f %e ./MAKEALL $SOC $MACHINE > $LOG 2>&1
With my MAKEALL wrapper. Anything we can do about that?
I see that as well... I did a little research and it seems like you have to jump through some pretty serious hoops to get rid of that message. I don't think it's worth the effort.
Does it have an impact on something in your wrapper?
-Joe

On Sun, Nov 04, 2012 at 06:13:03PM -0600, Joe Hershberger wrote:
Hi Tom,
On Sat, Nov 3, 2012 at 5:54 PM, Tom Rini trini@ti.com wrote:
On Tue, Oct 30, 2012 at 08:55:20PM -0500, Joe Hershberger wrote:
When building in parallel, make sure that we look up the children based on the the actual process group id instead of just assuming that the MAKEALL pid is the process group id.
Also ensure that logs from incomplete builds are deleted in the process.
Signed-off-by: Joe Hershberger joe.hershberger@ni.com
Now I see: /home/trini/bin/uboot-build.sh: line 110: 2024 Terminated /usr/bin/time -o $TIMEFILE -f %e ./MAKEALL $SOC $MACHINE > $LOG 2>&1
With my MAKEALL wrapper. Anything we can do about that?
I see that as well... I did a little research and it seems like you have to jump through some pretty serious hoops to get rid of that message. I don't think it's worth the effort.
Does it have an impact on something in your wrapper?
Purely cosemetic so I can live with the bugfixes as-is, thanks!

On Tue, Oct 30, 2012 at 08:55:20PM -0500, Joe Hershberger wrote:
When building in parallel, make sure that we look up the children based on the the actual process group id instead of just assuming that the MAKEALL pid is the process group id.
Also ensure that logs from incomplete builds are deleted in the process.
Signed-off-by: Joe Hershberger joe.hershberger@ni.com
Applied to u-boot/master, thanks!
participants (7)
-
Andy Fleming
-
Joe Hershberger
-
Joe Hershberger
-
Marek Vasut
-
Marek Vasut
-
Simon Glass
-
Tom Rini