Bug 1384938 - rpms not built with maximum number of cpus available
Summary: rpms not built with maximum number of cpus available
Keywords:
Status: CLOSED RAWHIDE
Alias: None
Product: Fedora
Classification: Fedora
Component: redhat-rpm-config
Version: 25
Hardware: aarch64
OS: Linux
unspecified
high
Target Milestone: ---
Assignee: Jason Tibbitts
QA Contact: Fedora Extras Quality Assurance
URL:
Whiteboard:
Depends On:
Blocks:
TreeView+ depends on / blocked
 
Reported: 2016-10-14 11:49 UTC by Robert Richter (Marvell)
Modified: 2017-08-31 15:06 UTC (History)
9 users (show)

Fixed In Version:
Clone Of:
: 1386840 (view as bug list)
Environment:
Last Closed: 2016-10-18 17:45:36 UTC
Type: Bug
Embargoed:


Attachments (Terms of Use)
Fix: Enable all cpus for rpm builds (1.35 KB, patch)
2016-10-14 11:49 UTC, Robert Richter (Marvell)
no flags Details | Diff

Description Robert Richter (Marvell) 2016-10-14 11:49:19 UTC
Created attachment 1210507 [details]
Fix: Enable all cpus for rpm builds

Description of problem:

When building Fedora kernel packages on ThunderX dual socket systems I
was wondering why the build was limited to 16 cpus only instead of the
maximum numbers of 96 in that system.

Now, I found out that I can use the RPM_BUILD_NCPUS variable for this,
or add '%_smp_ncpus_max 0' to the .rpmmacros file.

Both is a bit inconvince and also takes some time for the user to find
it out. Since there is no particular reason to limit cpus to 16, change
the default to use the maximum number of cpus that is possible.

It looks like there is a problem with memory-consuming tasks on certain systems which originally introduced the limit to 16 cpus. Is this still valid?

An alternative would be to limit the number of build tasks depending on the available memory as suggested in bug 1118734.

Instead of glolbaly limiting max cpus, rather only single affected packages or architectures should be set to that limit. ATM it is unclear which packages or systems are affected, so just let's remove the limit and then later fix those exceptional cases.

Patch enclosed.

Version-Release number of selected component (if applicable):

 redhat-rpm-config-43-1.fc25.noarch

Note: Rawhid is also affected.

How reproducible/Actual results:

 # rpm --eval %_smp_mflags
 -j16

Expected results:

Set to the maximum number of cpus in the system, e.g.:

 # rpm --eval %_smp_mflags
 -j96

Additional info:

Defined in /usr/lib/rpm/redhat/macros:

# Maximum number of CPU's to use when building, 0 for unlimited.
%_smp_ncpus_max 16
%_smp_mflags %([ -z "$RPM_BUILD_NCPUS" ] \\\
        && RPM_BUILD_NCPUS="`/usr/bin/getconf _NPROCESSORS_ONLN`"; \\\
        ncpus_max=%{?_smp_ncpus_max}; \\\
        if [ -n "$ncpus_max" ] && [ "$ncpus_max" -gt 0 ] && [ "$RPM_BUILD_NCPUS" -gt "$ncpus_max" ]; then RPM_BUILD_NCPUS="$ncpus_max"; fi; \\\
        if [ "$RPM_BUILD_NCPUS" -gt 1 ]; then echo "-j$RPM_BUILD_NCPUS"; fi)

Comment 1 Robert Richter (Marvell) 2016-10-14 11:54:19 UTC
Adding the link to the discussion of the issue in the last arm status meeting:

https://meetbot.fedoraproject.org/teams/fedora_arm_and_aarch64_status_meeting/fedora_arm_and_aarch64_status_meeting.2016-10-11-15.00.log.html

Comment 2 Panu Matilainen 2016-10-14 12:02:22 UTC
See bug 669638 for background on the limit.

Comment 3 Jason Tibbitts 2016-10-14 19:59:08 UTC
So basically four years ago, on the Sparc builders (which I don't believe we even still have) there was an issue and so the limit was set to 16 on all architectures.  I suspect it's not really a Sparc issue but more an issue with various package build processes simply breaking when run with extreme parallelism and the fact that those Sparc CPUs had a really large number of threads.

If we were to drop the limit I suspect we would turn up a few packages that work most of the time with 16-way parallelism but may fail to build when cranked up further.  Still, those package are broken regardless, and I don't think it's productive to work around them by limiting parallelism for every single package.

Is there any real reason _not_ to just set the limit to 0 in rawhide now?  If there really is some Sparc-specific problem, we can just conditionalize the limit for that one architecture.

Comment 4 Pavel Raiskup 2016-10-14 21:53:45 UTC
> If there really is some Sparc-specific problem, we can just conditionalize
> the limit for that one architecture.

This sounds like correct thing to do, but I'm not sure who should we CC about
Sparc issues.  Anyway, the macro condition for Sparc (with proper link to
relevant bugzilla) should be safe..  If there was further work on Sparc,
that condition can be removed later.

Comment 5 Jason Tibbitts 2016-10-15 01:08:28 UTC
It's really only the correct thing to do if the issue actually has anything to do with Sparc.  Sparc CPUs had a rather large number of threads (128 per socket back in 2010) and so issues with makefiles being sensitive to very high parallelism would have shown up there a good bit before most people would have seen them.

Even then, though, surely we shouldn't be artificially limiting makefile parallelism for every package.  If a package is broken by removing the limit, then it's a bug in that package which should preferably be fixed, but if not then easily worked around with a single %define and some comments documenting the brokenness.  These really need to be in there in any case.  And it would be really nice if we had some idea of what was actually failing back in 2012 so that it can be tested today.  Intel machines with 32+ real cores aren't all that hard to come by these days.

The only reason not to just set the limit to 0 right now is the unknown quantity of things which may be broken.  If we want to be really careful, why not increase the limit a bit right now (to, say... 24?) and then let that soak for a while.  Then bump it to 32 or remove it altogether after the next release branches.

As an aside, I would think that the aarch64 team would probably prefer not to have to sort out makefile parallelism issues (which, at -j96 most developers will not really have seen) from other issues related to the porting of things over to the new arch.  So maybe a more cautious approach would be desired here.  But then again, maybe they're perfectly willing to deal with that in exchange for having builds actually use all of their hardware.

Comment 6 Pavel Raiskup 2016-10-15 01:59:22 UTC
> The only reason not to just set the limit to 0 right now is the unknown
> quantity of things which may be broken.

I think the issue is that we don't know _when_ people will actually look
at possible issues.  If there is active initiative, they should be CCed
now.  Otherwise flip the settings for everybody except for Sparc (there's
nothing worse than debugging FTBFS without notice in advance...).

Comment 7 Peter Robinson 2016-10-15 10:12:56 UTC
(In reply to Pavel Raiskup from comment #4)
> > If there really is some Sparc-specific problem, we can just conditionalize
> > the limit for that one architecture.
> 
> This sounds like correct thing to do, but I'm not sure who should we CC about
> Sparc issues.  Anyway, the macro condition for Sparc (with proper link to
> relevant bugzilla) should be safe..  If there was further work on Sparc,
> that condition can be removed later.

There is no active Fedora SPARC port although I do know there are Oracle people looking at it.

IBM Power 64 can have a large core count with 8 threads a core.

Comment 8 Robert Richter (Marvell) 2016-10-17 17:39:31 UTC
Original change was (redhat-rpm-config):

commit 3a2d93f3119168b4b46d3eb37fed9355f51dd8c2
Author: Panu Matilainen <pmatilai>
Date:   Fri Jan 22 15:02:51 2010 +0200

    Limit _smp_mflags to max 16
    - patch from Dennis Gilmore

This dates back to 2010. I think we should get rid of the limit. It was made with the assumption that there are no (Fedora) build systems with more than 16 cpus, this is not valid anymore. Also, since then there were a lot of improvements in GNU Make. I checked the bug-make mailing list and the bug tracker and haven't found parallel build bugs that prevents us from using max number of cpus.

If the parallel build of a package is broken, then the Makefiles of that package should be fixed instead or the limit could be set in the rpm spec file.

Comment 9 Jason Tibbitts 2016-10-17 18:11:08 UTC
The easiest way to change this is to just change the definition in /usr/lib/rpm/redhat/macros.

If we have to per-arch-conditionalize this for whatever bizarre reason, the rpm package can be changed to patch one or more of the /usr/lib/rpm/platform/*/macros files.  Or you can use this unpleasantness, if you really wanted....

%_smp_ncpus_max %{lua:
    local arch = rpm.expand('%_arch')
    if arch == 'sparc' then
        print "16\\n"
    else
        print "0\\n"
    end
}

I'm ready to commit the simple removal of the limit from redhat-rpm-config but will certainly wait for an ACK.

And yes, this will certainly break a small number of packages, because any change breaks something.  It's definitely worth an announcement to devel-announce, which I will happily make.  I don't think it will change anything at all for plain old Fedora x86_64, though.

Comment 10 Peter Robinson 2016-10-17 18:19:53 UTC
I would just do a blanket removal, if it's an issue we'll see it, if it's arch specific we can review that with the arch or the arch will need to fix it.

Comment 11 Jason Tibbitts 2016-10-17 20:43:00 UTC
OK, I went ahead and pushed (to rawhide) a change which just comments out the line which set the value and adds a bit of explanation.  I also sent an explanatory note to devel-announce just to make sure nothing was done in secret.

I also verified that this will have no effect at all for the x86 and ppc portions of the Fedora buildsystem, since those machines present at most 16 threads to the build process.  So really this will only make any difference at all for aarch64, someone who hauls some SPARC stuff out of their garage, or someone with sufficiently beefy Intel kit who is doing their own package builds.

Also note that I didn't touch F25.  I know this was filed against F25, but I'm not going to push anything there unless I see a couple of additional ACKs.

Comment 12 Panu Matilainen 2016-10-18 06:37:12 UTC
I dont disagree with the change at all, I too find it mildly hysterical that we limit every Fedora system in the world to building with 16 cpus max because rel-eng saw "some failures" (but no bugs to refer to that I know of) related to massively parallel builds on an obscure secondary platform in 2010. 

Rawhide is the place for experimenting with such things, and I see you also dropped a note to fedora-devel(-announce)@, so I think we're good. But lets leave F25 out of the picture, this is not a critical fix anyway so there's little point risking it.

Comment 13 Peter Robinson 2016-10-18 08:38:12 UTC
We're also not likely to see this in the standard build infrastructure as even with the Power8 hardware where we have HW with 20 cores / 160 threads we run all builders as VMs not bare metal

Comment 14 Jason Tibbitts 2016-10-18 15:48:30 UTC
Right, I checked for all of the architectures including Power when I was writing up that notice to make sure that outside of aarch64 would even see this.  And even the aarch64 we currently have in the buildsystem (just 8 cores) won't see any difference.  So, really, from the standpoint of the Fedora buildsystem, this is pretty much a null change.  It will get me something in my own builds, though

I do wonder if there are packages which would benefit measurably for increased build parallelism, but this probably isn't the place to talk about it.

@rrichter, you originally filed this against F25.  Are you OK with just doing this in rawhide, or is that going cause issues for you?  I have to agree with Panu that it's probably not worth poking at F25 this close to release.

Comment 15 Robert Richter (Marvell) 2016-10-18 16:16:53 UTC
(In reply to Jason Tibbitts from comment #14)
> @rrichter, you originally filed this against F25.  Are you OK with just
> doing this in rawhide, or is that going cause issues for you?  I have to
> agree with Panu that it's probably not worth poking at F25 this close to
> release.

Thanks Jason, yes, Rawhide is ok to me.

Posting here a workaround for reference:

 # grep _smp_ncpus_max ~/.rpmmacros
 %_smp_ncpus_max 0

Comment 16 Jason Tibbitts 2016-10-18 17:45:36 UTC
OK, thanks.  Rawhide hasn't broken completely since this was pushed so I think we're good here.

Comment 17 Dennis Gilmore 2016-10-21 15:48:27 UTC
There was massive number of failure in make using -j20 and higher. even when building only two objects things would build out of order. I replicated on x86_64 at the time. I think blanket remove is only appropriate if someone is on the hook to do the work to fix the issues.

Comment 18 Dennis Gilmore 2016-10-21 15:54:16 UTC
I will also add that none of the current builds in Fedora have  anywhere near enough cores to hit the bug. There was a workaround added that you could define %{_smp_ncpus_max} macro which will let you choose a higher number.some packages such as gcc and kernel could benefit from having it set to enable the full use outside of the buildsystems where no hosts have more

Comment 19 Robert Richter (Marvell) 2016-10-24 14:49:14 UTC
(In reply to Dennis Gilmore from comment #18)
> I will also add that none of the current builds in Fedora have  anywhere
> near enough cores to hit the bug.

ThunderX systems are probably not the only systems with a much higher number of cores than 16. This arbitrary limit slows down rpm builds by a factor of 6. There are no direct bug reports of packages, that are unable to handle more than 16, nor is gnu make known to fail here. Why it does not fail with 16 but with more than 20? Is there a reason? After more than 6 years now, packages should be capable of being built in parallel.

> There was a workaround added that you
> could define %{_smp_ncpus_max} macro which will let you choose a higher
> number.some packages such as gcc and kernel could benefit from having it set
> to enable the full use outside of the buildsystems where no hosts have more

It should be vice versa, limit all packages that are broken.

Comment 20 Peter Robinson 2016-10-24 14:52:21 UTC
> 6. There are no direct bug reports of packages, that are unable to handle
> more than 16, nor is gnu make known to fail here. Why it does not fail with
> 16 but with more than 20? Is there a reason? After more than 6 years now,
> packages should be capable of being built in parallel.

Biggest issue I've seen is xz compression of small amounts of data fails when massively parallel and you end up with zero byte output

Comment 21 Robert Richter (Marvell) 2016-10-24 15:15:29 UTC
(In reply to Peter Robinson from comment #20)
> Biggest issue I've seen is xz compression of small amounts of data fails
> when massively parallel and you end up with zero byte output

But is this controlled by make's -j option?

Comment 22 Jason Tibbitts 2016-10-24 15:24:05 UTC
Well rpm itself doesn't do any parallel xz compression or decompression (which I'd love to see changed, but that requires low level work on RPM since it doesn't even call the parallel-capable entry points).  So this would have to be related to something which explicitly calls xz -T0.  Which:

* has absolutely no relation whatsoever to the one thing which was just changed in redhat-rpm-config.  None at all; it would have been broken well before the change.

* would be a bug in xz that should be fixed in xz, not worked around in Fedora's RPM macros even if that were possible.

So... I guess I still just don't get it.  Unless maybe the aarch64 folks are going to be seeing this, in which case I'm sure they'll file the appropriate bugs against xz or whatever.

As for being someone on the hook to fix issues that arise.... well, sure, find a package that's failing due to this change and I'll add the one line to it which necessary to limit the CPU count back to 16.  Not that I didn't already say exactly what needed to be done in my announcement, but if someone just really doesn't want to add the one needed line, then I have no problem doing that myself.

Comment 23 Robert Richter (Marvell) 2017-08-31 15:06:43 UTC
FTR:

I found this fixed in redhat-rpm-config-63-1.fc26.noarch:

[root@crb2spass2rric ~]# grep -4 _smp_mflags /usr/lib/rpm/redhat/macros
# https://bugzilla.redhat.com/show_bug.cgi?id=669638 and
# https://bugzilla.redhat.com/show_bug.cgi?id=1384938 for the situation
# surrounding this.
#%_smp_ncpus_max 0
%_smp_mflags %([ -z "$RPM_BUILD_NCPUS" ] \\\
	&& RPM_BUILD_NCPUS="`/usr/bin/getconf _NPROCESSORS_ONLN`"; \\\
        ncpus_max=%{?_smp_ncpus_max}; \\\
        if [ -n "$ncpus_max" ] && [ "$ncpus_max" -gt 0 ] && [ "$RPM_BUILD_NCPUS" -gt "$ncpus_max" ]; then RPM_BUILD_NCPUS="$ncpus_max"; fi; \\\
        if [ "$RPM_BUILD_NCPUS" -gt 1 ]; then echo "-j$RPM_BUILD_NCPUS"; fi)
[root@crb2spass2rric ~]# rpm -qf /usr/lib/rpm/redhat/macros
redhat-rpm-config-63-1.fc26.noarch


Note You need to log in before you can comment on or make changes to this bug.