linux-kernel - Re: [PATCH] sched: fix task and run queue run

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: <20151001063732.GM2881@worktop.programming.kicks-ass.net>
Date:	Thu, 1 Oct 2015 08:37:32 +0200
From:	Peter Zijlstra <peterz@...radead.org>
To:	"Meyer, Mike" <Mike.Meyer@...adata.com>
Cc:	"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
	"mingo@...hat.com" <mingo@...hat.com>
Subject: Re: [PATCH] sched: fix task and run queue run_delay inconsistencies

On Wed, Sep 30, 2015 at 08:28:41PM +0000, Meyer, Mike wrote:
> > From: Peter Zijlstra [mailto:peterz@...radead.org]
> >
> > On Wed, Sep 23, 2015 at 12:37:18AM +0000, Meyer, Mike wrote:
> > >
> > > The proposed patch addresses the issue by calling
> > > sched_info_reset_dequeued(thread) following the call to
> > > enqueue_task(rq, thread) for running threads in situations in which
> > > thread->sched_info.last_queued should remain 0.
> > 
> > Would something like the below; which avoids calling
> > sched_info_{de,}queued() for these sites also work?
> > 
> > It even shrinks the code (due to inlining {en,de}queue_task()):
> > 
> > $ size defconfig-build/kernel/sched/core.o defconfig-
> > build/kernel/sched/core.o.orig
> >    text    data     bss     dec     hex filename
> >   64019   23378    2344   89741   15e8d defconfig-build/kernel/sched/core.o
> >   64149   23378    2344   89871   15f0f defconfig-build/kernel/sched/core.o.orig
> > 
> Yes that will also address the issue.
> 
> The reason I approached the way I did was to avoid adding code path to
> the far more common uses of {en,de}queue_task() but I doubt anyone is
> going to notice a difference with the addition of some register
> save/restores and a compare in that path.  Overall the code does
> shrink with the alternative which is good.

In most cases the flags should be compile time constants, and with the
inline we can determine the branch at compile time, avoiding emitting
that branch instruction entirely.

But let me double check the asm for a few important sites.

> My only comment is I am not sure about the naming of the flag
> ENQUEUE_TEMP which implies (to me) the enqueue is temporary which
> clearly it isn't.    Maybe something like DEQUEUE_MOVE/ENQUEUE_MOVE
> would be a bit more descriptive of the use case.

Yes, I ran out of creative juices, let me attempt a better name once
I've woken up a bit.

--
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/