Dear Andy Fleming,

In message <1302687759-1649-1-git-send-email-aflem...@freescale.com> you wrote:
> The MAKEALL script cleverly runs make with the appropriate options
> to use all of the cores on the system, but your average U-Boot build
> can't make much use of more than a few cores.  If you happen to have
> a many-core server, your builds will leave most of the system idle.
...
> ${output_prefix}/$target.  We launch up to 8 builds in parallel (with
> each build still being told to use n+1 cores), and wait for all 8 to
> finish before launching 8 more.  It's not ideal, but it is faster.

How did you figure that 8 * (n+1) would be an efficient number of
tasks to use?  Does this not depend on a lot of factors, like number
of cores, relative speed of CPUs versus I/O subsystem, etc. ?

...
> +# Output
> +# It is now possible to save the output to a build directory.  This serves
> +# two purposes.  It allows you to keep the images for testing, and it
> +# allows you to launch the makes in tandem.  Pass in -o <dir>, and the build
> +# will happen in <dir>/$target/

Hm... this conflicts with / duplicates the function of BUILD_DIR,
doesn't it?

>  [ -d ${LOG_DIR} ] || mkdir ${LOG_DIR} || exit 1
>  
> +if [ "${output_prefix}" ] ; then
> +     [ -d ${output_prefix} ] || mkdir -p ${output_prefix} || exit 1
> +     [ -d "${output_prefix}/ERR" ] && rm -rf "${output_prefix}/ERR"
> +     mkdir "${output_prefix}/ERR"
> +fi

Should LOG_DIR not be adjusted, too?

> +     [ "$output_prefix" ] && export BUILD_DIR="${output_prefix}/${target}"

Ouch.  This means you are messing with user settings without warning
or any indication.  I don't like this.

If we have a conflict (and here we have one), there should be at least
errot / warning messages.

> +     if [ "$output_prefix" ] ; then
> +             ${MAKE} clean
> +             find "${output_prefix}/$target" -type f -name '*.depend' | 
> xargs rm

Why remove the .depend files and not any of the other temporary files?

> -     TOTAL_CNT=$((TOTAL_CNT + 1))

Um....

>       ${CROSS_COMPILE}size ${BUILD_DIR}/u-boot \
>                               | tee -a ${LOG_DIR}/$target.MAKELOG
>  }
> @@ -650,7 +681,20 @@ build_targets() {
>               if [ -n "${list}" ] ; then
>                       build_targets ${list}
>               else
> -                     build_target ${t}
> +                     if [ "$output_prefix" ] ; then
> +                             build_target ${t} &
> +                     else
> +                             build_target ${t}
> +                     fi
> +
> +                     TOTAL_CNT=$((TOTAL_CNT + 1))
> +                     CURRENT_COUNT=$((CURRENT_COUNT + 1))
> +             fi

So TOTAL_CNT and CURRENT_COUNT get only set in the "else" case?

> +             # Only run 8 builds concurrently
> +             if [ ${CURRENT_COUNT} -gt 8 ]; then

Magic number...

>       echo "Boards compiled: ${TOTAL_CNT}"

Is this report correct for all use cases, now?

Best regards,

Wolfgang Denk

-- 
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: w...@denx.de
It is dangerous to be sincere unless you are also stupid.
                                                - George Bernard Shaw
_______________________________________________
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

Reply via email to