On Thu, 2008-05-01 at 20:14 -0400, Parag Warudkar wrote:So you're saying reverting e22ecef1 fixes the issue? - as have others. The initial rationale for the change was: vruntime and walltime are related by a factor of rq->weight, hence compensate for that. However that in itself has a flaw, we're comparing vruntime to process runtime time, _NOT_ walltime. And vruntime and process runtime do not have that factor so the compensation is unneeded. However that still leaves me puzzled on why it would cause such bad regression. The current code reads like: /* * delta *= w / rw */ static inline unsigned long calc_delta_weight(unsigned long delta, struct sched_entity *se); /* sleeps upto a single latency don't count. */ if (sched_feat(NEW_FAIR_SLEEPERS)) { if (sched_feat(NORMALIZED_SLEEPER)) vruntime -= calc_delta_weight(sysctl_sched_latency, se); else vruntime -= sysctl_sched_latency; } So what we do is _shrink_ the bonus the busier we get (the more tasks we get, the higher the total runqueue weight (rw) hence delta will get smaller) The thing is, I once asked Frans to test !NEW_FAIR_SLEEPERS and he reported that that also solves the problem. So no bonus is good, and a fixed bonus is good, but a variable bonus that is between these two values is not. Parag, would you also test with !NEW_FAIR_SLEEPERS to see if that solves your problem? The easiest way to disable it is (assumes you have debugfs mounted at /debug): # echo NO_NEW_FAIR_SLEEPERS > /debug/sched_features You can also disable NORMALIZED_SLEEPER that way (of course you would first have to enable NEW_FAIR_SLEEPERS again): # echo NO_NORMALIZED_SLEEPER > /debug/sched_features You can get a current status of all the flags using: # cat /debug/sched_features So by default we have both enabled; could you report if either NO_NEW_FAIR_SLEEPERS NEW_FAIR_SLEEPERS + NO_NORMALIZED_SLEEPERS works for you? Also, could you apply this patch, and report the bonus_max value for your music player under all three scenarios? I use amarok and use the following line, something similar should also work for PA. # grep bonus_max `grep -l amarokapp /proc/*/task/*/sched` All of the above assumes you have: CONFIG_SCHED_DEBUG=y CONFIG_SCHEDSTATS=y CONFIG_DEBUG_FS=y --- Index: linux-2.6-2/include/linux/sched.h =================================================================== --- linux-2.6-2.orig/include/linux/sched.h +++ linux-2.6-2/include/linux/sched.h @@ -977,6 +977,7 @@ struct sched_entity { u64 block_max; u64 exec_max; u64 slice_max; + u64 bonus_max; u64 nr_migrations; u64 nr_migrations_cold; Index: linux-2.6-2/kernel/sched.c =================================================================== --- linux-2.6-2.orig/kernel/sched.c +++ linux-2.6-2/kernel/sched.c @@ -2587,6 +2587,7 @@ static void __sched_fork(struct task_str p->se.block_max = 0; p->se.exec_max = 0; p->se.slice_max = 0; + p->se.bonus_max = 0; p->se.wait_max = 0; #endif Index: linux-2.6-2/kernel/sched_debug.c =================================================================== --- linux-2.6-2.orig/kernel/sched_debug.c +++ linux-2.6-2/kernel/sched_debug.c @@ -325,6 +325,7 @@ void proc_sched_show_task(struct task_st PN(se.block_max); PN(se.exec_max); PN(se.slice_max); + PN(se.bonus_max); PN(se.wait_max); PN(se.wait_sum); P(se.wait_count); @@ -402,6 +403,7 @@ void proc_sched_set_task(struct task_str p->se.block_max = 0; p->se.exec_max = 0; p->se.slice_max = 0; + p->se.bonus_max = 0; p->se.nr_migrations = 0; p->se.nr_migrations_cold = 0; p->se.nr_failed_migrations_affine = 0; Index: linux-2.6-2/kernel/sched_fair.c =================================================================== --- linux-2.6-2.orig/kernel/sched_fair.c +++ linux-2.6-2/kernel/sched_fair.c @@ -662,9 +662,12 @@ place_entity(struct cfs_rq *cfs_rq, stru if (!initial) { /* sleeps upto a single latency don't count. */ if (sched_feat(NEW_FAIR_SLEEPERS)) { - if (sched_feat(NORMALIZED_SLEEPER)) - vruntime -= calc_delta_weight(sysctl_sched_latency, se); - else + if (sched_feat(NORMALIZED_SLEEPER)) { + unsigned long bonus; + bonus = calc_delta_weight(sysctl_sched_latency, se); + schedstat_set(se->bonus_max, max_t(u64, se->bonus_max, bonus)); + vruntime -= bonus; + } else vruntime -= sysctl_sched_latency; } --
| Greg Kroah-Hartman | [PATCH 001/196] Chinese: Add the known_regression URI to the HOWTO |
| Yinghai Lu | Re: [GIT *] Allow request_firmware() to be satisfied from in-kernel, use it in mor... |
| Trent Piepho | [PATCH] [POWERPC] Improve (in|out)_beXX() asm code |
| Alan Cox | [PATCH 00/76] Queued TTY Patches |
git: | |
| Junio C Hamano | Re: More precise tag following |
| Yossi Leybovich | corrupt object on git-gc |
| bain | [Announce] teamGit v0.0.3 |
| Aaron Bentley | Re: VCS comparison table |
| Nick Guenther | Re: Real men don't attack straw men |
| Brandon Lee | DELL PERC 5iR slow performance |
| Stefan Beke | mail dovecot: pipe() failed: Too many open files |
| Paul Greidanus | Re: [Fwd: Open-Hardware] |
| David Miller | [GIT]: Networking |
| Jarek Poplawski | [PATCH] pkt_sched: Destroy gen estimators under rtnl_lock(). |
| KOSAKI Motohiro | [bug?] tg3: Failed to load firmware "tigon/tg3_tso.bin" |
| Gerrit Renker | [PATCH 27/37] dccp: Integration of dynamic feature activation - part 2 (server side) |
