linux-kernel - Re: [PATCH 2/2] futex: lockless wakeups

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]

Date:	Mon, 20 Apr 2015 16:57:30 +0200
From:	Peter Zijlstra <peterz@...radead.org>
To:	Davidlohr Bueso <dave@...olabs.net>
Cc:	Ingo Molnar <mingo@...nel.org>,
	Thomas Gleixner <tglx@...utronix.de>,
	Ingo Molnar <mingo@...hat.com>,
	Sebastian Andrzej Siewior <bigeasy@...utronix.de>,
	Linus Torvalds <torvalds@...ux-foundation.org>,
	Chris Mason <clm@...com>, Steven Rostedt <rostedt@...dmis.org>,
	fredrik.markstrom@...driver.com, linux-kernel@...r.kernel.org,
	Andrew Morton <akpm@...ux-foundation.org>
Subject: Re: [PATCH 2/2] futex: lockless wakeups

On Mon, Apr 20, 2015 at 06:55:39AM -0700, Davidlohr Bueso wrote:
> I have been using this from Peter to test against:
> 
> diff --git a/include/linux/sched.h b/include/linux/sched.h
> index 6d77432..fdf1f68 100644
> --- a/include/linux/sched.h
> +++ b/include/linux/sched.h
> @@ -214,9 +214,10 @@ print_cfs_rq(struct seq_file *m, int cpu, struct cfs_rq *cfs_rq);
>  #define TASK_WAKEKILL		128
>  #define TASK_WAKING		256
>  #define TASK_PARKED		512
> -#define TASK_STATE_MAX		1024
> +#define TASK_YIELD              1024
> +#define TASK_STATE_MAX		2048
>  
> -#define TASK_STATE_TO_CHAR_STR "RSDTtXZxKWP"
> +#define TASK_STATE_TO_CHAR_STR "RSDTtXZxKWPY"
>  
>  extern char ___assert_task_state[1 - 2*!!(
>  		sizeof(TASK_STATE_TO_CHAR_STR)-1 != ilog2(TASK_STATE_MAX)+1)];
> diff --git a/kernel/sched/core.c b/kernel/sched/core.c
> index f0f831e..2c938ae 100644
> --- a/kernel/sched/core.c
> +++ b/kernel/sched/core.c
> @@ -1005,7 +1005,7 @@ void set_task_cpu(struct task_struct *p, unsigned int new_cpu)
>  	 * ttwu() will sort out the placement.
>  	 */
>  	WARN_ON_ONCE(p->state != TASK_RUNNING && p->state != TASK_WAKING &&
> -			!p->on_rq);
> +			!p->on_rq && !(p->state & TASK_YIELD));
>  
>  #ifdef CONFIG_LOCKDEP
>  	/*
> @@ -2743,6 +2743,23 @@ static void __sched __schedule(void)
>  		if (unlikely(signal_pending_state(prev->state, prev))) {
>  			prev->state = TASK_RUNNING;
>  		} else {
> +
> +			/*
> +			 * Provide an auto-yield feature on schedule().
> +			 *
> +			 * The thought is to avoid a sleep+wakeup cycle
> +			 * if simply yielding the cpu will suffice to
> +			 * satisfy the required condition.
> +			 *
> +			 * Assumes the calling schedule() site can deal
> +			 * with spurious wakeups.
> +			 */
> +			if (prev->state & TASK_YIELD) {
> +				prev->state &= ~TASK_YIELD;
> +				if (rq->nr_running > 1)
> +					goto no_deactivate;
> +			}
> +
>  			deactivate_task(rq, prev, DEQUEUE_SLEEP);
>  			prev->on_rq = 0;
>  
> @@ -2759,6 +2776,7 @@ static void __sched __schedule(void)
>  					try_to_wake_up_local(to_wakeup);
>  			}
>  		}
> +	no_deactivate:
>  		switch_count = &prev->nvcsw;
>  	}
>  

Haha, you dug that out did you ;-)

Does this patch on its own actually help anything? Back when I wrote
that there was the vague hope it would actually help some of the
client-server ping-pong workloads, where the client does sync requests
to the server.

In that case, if the server can handle the req. while the client yields
we've saved on an expensive wakeup.

Reality never really worked out for that -- even after we fixed a few
boot and runtime issues with it.
--
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/