linux-kernel - Re: [patch 2/2] sched: charge unaccounted run-time on entity re-weight

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <1292589534.2266.182.camel@twins>
Date:	Fri, 17 Dec 2010 13:38:54 +0100
From:	Peter Zijlstra <a.p.zijlstra@...llo.nl>
To:	Paul Turner <pjt@...gle.com>
Cc:	linux-kernel@...r.kernel.org, Ingo Molnar <mingo@...e.hu>,
	Mike Galbraith <efault@....de>,
	Linus Torvalds <torvalds@...ux-foundation.org>
Subject: Re: [patch 2/2] sched: charge unaccounted run-time on entity
 re-weight

On Thu, 2010-12-16 at 14:31 -0800, Paul Turner wrote:

> That doesn't quite work.
> 
> The problem stems from:
> 
> - update_curr() accues time against current cfs_rq's timeline
>   - We always need to do this for entity placement
>   - Manipulation of the current cfs_rq's load affects its weights

> However the current cfs_rq in the problem case is a group entity which
> happens to be the current entity on the parenting se's group_cfs_rq
> (say that 10 times fast).
> 
> When we update that entity's (call it X) weight to reflect the
> interactions on its owned cfs_rq, the update isout of order with the
> subsequent update_curr() on the parent which is what actually accounts
> the accrued vruntime versus X (which was accumulated at old weight)
> 
> We need to either:
> 
> A) Get all of the update_currs() done up front, e.g. at the start of
> enqueue_task_fair add another for_each
> - I don't like this approach because it it becomes a concern that has
> to be implemented by all callers
> - There's also no point in issuing these if the entity in question
> isnt cfs_rq->curr since there's no time to account in that case
> 
> B) Change the reweights in enqueue/dequeue/etc to occur against the
> owned cfs_rq as opposed to the queueing cfs_rq.
> - This is not really clean in my mind since it steps outside of the
> semantic of we are "enqueuing E to T".  Instead of only really
> manipulating T we're adding "oh and we'll finish manipulations
> resulting from prior enqeues against E if it was a tree".

I knew there was a reason I did it like that early on ;-)

> C) Charge unaccounted time versus an entity before re-weighting it
> - I think this ends up being the nicest, we only end up issuing the
> extra update_currs when we need them, and the second becomes a nop
> since rq->clock doesn't move.  Not to mention it also blocks up this
> hole completely since it becomes always safe to reweight_entity().

Hrmm, I see what you mean, its not exactly pretty either, but I must
admit to not seeing a better solution at the moment.

OK, so your patch it is ;-)
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/