SRU: We could make modprobe less slow by building it with proper CFLAGS

Bug #276096 reported by Loïc Minier
10
Affects Status Importance Assigned to Milestone
module-init-tools (Ubuntu)
Fix Released
Undecided
Unassigned
Hardy
Fix Released
Undecided
Unassigned

Bug Description

modprobe is slow in hardy because it's built with -O2; building it with -Os makes it noticeably faster.

TEST CASE:
time for i in $(seq 100); do modprobe -q foobar; done

Revision history for this message
Loïc Minier (lool) wrote :

Fixed in intrepid in 3.3-pre11-4ubuntu12.

Changed in module-init-tools:
status: New → Fix Released
Revision history for this message
Loïc Minier (lool) wrote :

Regression potential: if we have a serious bug in our toolchain, it could break modprobe; hopefully we'd notice and we'd need to fix our toolchain anyway.

Revision history for this message
Loïc Minier (lool) wrote :

NB: udeb is already built with -Os.

Revision history for this message
Loïc Minier (lool) wrote :

Before the change, idle system, "echo performance >/sys/devices/system/cpu/cpu0/cpufreq/scaling_governor" to both HT logical CPUs:
root@crownbeach:~# time for i in $(seq 100); do modprobe -q foobar; done

real 0m8.720s
user 0m8.357s
sys 0m0.348s

after the change:
root@crownbeach:~# time for i in $(seq 100); do modprobe -q foobar; done

real 0m6.792s
user 0m6.480s
sys 0m0.288s

Revision history for this message
Loïc Minier (lool) wrote :
description: updated
Revision history for this message
Loïc Minier (lool) wrote :

Package available in my ppa (~lool).

Revision history for this message
Loïc Minier (lool) wrote :

Unconclusive bootcharts follow

Revision history for this message
Loïc Minier (lool) wrote :
Revision history for this message
Martin Pitt (pitti) wrote :

Risking to break anything just for a two second boot time improvement? This update would not fix any bug, and thus isn't the obvious candidate for a stable update in my opinion.

If you really need it for e. g. mobile, then I'm ok with uploading it to hardy-proposed and let it sit there for a month or so, and move it over if we don't hear any regression reports.

Revision history for this message
Matt Zimmerman (mdz) wrote :

This change is required for MID and netbook builds, and I would rather we make the change in an SRU than have a different module-init-tools in those builds.

I think putting it into hardy-proposed for an extended period is perfectly reasonable. MID and netbook builds can pull this package in, and it can be thoroughly tested there before being released to hardy-updates.

Revision history for this message
Martin Pitt (pitti) wrote :

Accepted into -proposed, please test and give feedback here. Please see https://wiki.ubuntu.com/Testing/EnableProposed for documentation how to enable and use -proposed. Thank you in advance!

Changed in module-init-tools:
status: New → Fix Committed
Revision history for this message
Loïc Minier (lool) wrote :

On another system (Thinkpad T60p), before the change:
root@bee:~# echo performance > /sys/devices/system/cpu/cpu1/cpufreq/scaling_governor
root@bee:~# time for i in $(seq 100); do modprobe -q foobar; done

real 0m4.472s
user 0m4.312s
sys 0m0.164s

after the change:
real 0m4.045s
user 0m3.840s
sys 0m0.292s

Revision history for this message
Loïc Minier (lool) wrote :

So testing from -proposed is successful, but the difference is again not huge. Still an easy win it seems. :)

Revision history for this message
Paul Elliott (omahn) wrote :

Successfully verified hardy-proposed:

3.3-pre11-4ubuntu5 in hardy
pre500@pristine804:~$ time for i in $(seq 100); do modprobe -q bitblit; done
real 0m0.930s
user 0m0.376s
sys 0m0.544s

3.3-pre11-4ubuntu5.8.04.1 in hardy-proposed
pre500@pristine804:~$ time for i in $(seq 100); do modprobe -q bitblit; done
real 0m0.857s
user 0m0.392s
sys 0m0.464s

Although IMHO the gains offered are not worth the risk of updating such a critical package in LTS.

Revision history for this message
Martin Pitt (pitti) wrote :

This has been sitting in -proposed for very long now, and we didn't hear complaints, and got several verifiers. Considering sufficiently verified now.

Revision history for this message
Martin Pitt (pitti) wrote :

Copied to hardy-updates.

Changed in module-init-tools:
status: Fix Committed → Fix Released
To post a comment you must log in.
This report contains Public information  
Everyone can see this information.

Other bug subscribers

Remote bug watches

Bug watches keep track of this bug in other bug trackers.