Re: [RFC PATCH] sched: wakeup buddy

[Namhyung Kim <namhyung@kernel.org>]

Hi Michael,

On Thu, 28 Feb 2013 14:38:03 +0800, Michael Wang wrote:
> wake_affine() stuff is trying to bind related tasks closely, but it doesn't
> work well according to the test on 'perf bench sched pipe' (thanks to Peter).
>
> Besides, pgbench show that blindly using wake_affine() will eat a lot of
> performance.
>
> Thus, we need a new solution, it should detect the tasks related to each
> other, bind them closely, take care the balance, latency and performance
> at the same time.
>
> Feature wakeup buddy seems like a good solution (thanks to Mike for the hint).
>
> The feature introduced waker, wakee pointer and their ref count, along with
> the new knob sysctl_sched_wakeup_buddy_ref.
>
> Now in select_task_rq_fair(), when current (task B) try to wakeup p (task A),
> if match:
>
> 	1. A->waker == B && A->wakee == B
> 	2. A->waker_ref > sysctl_sched_wakeup_buddy_ref
> 	3. A->wakee_ref > sysctl_sched_wakeup_buddy_ref
>
> then A is the wakeup buddy of B, which means A and B is likely to utilize
> the memory of each other.
>
> Thus, if B is also the wakeup buddy of A, which means no other task has
> destroyed their relationship, then A is likely to benefit from the cached
> data of B, make them running closely is likely to gain benefit.

Not sure if it should require bidirectional relationship.  Looks like
just for benchmarks.  Isn't there a one-way relationship that could get
a benefit from this?  I don't know ;-)

Few nitpicks below..

>
> This patch add the feature wakeup buddy, reorganized the logical of
> wake_affine() stuff with the new feature, by doing these, pgbench and
> 'perf bench sched pipe' perform better.
>
> Highlight:
> 	Default value of sysctl_sched_wakeup_buddy_ref is 8 temporarily,
> 	please let me know if some number perform better on your system,
> 	I'd like to make it bigger to make the decision more carefully,
> 	so we could provide the solution when it is really needed.
>
> 	Comments are very welcomed.
>
> Test:
> 	Test with a 12 cpu X86 server and tip 3.8.0-rc7.
>
> 	'perf bench sched pipe' show nearly double improvement.
>
> 	pgbench result:
> 					prev	post
>
>                 | db_size | clients |  tps  |   |  tps  |
>                 +---------+---------+-------+   +-------+
>                 | 22 MB   |       1 | 10794 |   | 10820 |
>                 | 22 MB   |       2 | 21567 |   | 21915 |
>                 | 22 MB   |       4 | 41621 |   | 42766 |
>                 | 22 MB   |       8 | 53883 |   | 60511 |       +12.30%
>                 | 22 MB   |      12 | 50818 |   | 57129 |       +12.42%
>                 | 22 MB   |      16 | 50463 |   | 59345 |       +17.60%
>                 | 22 MB   |      24 | 46698 |   | 63787 |       +36.59%
>                 | 22 MB   |      32 | 43404 |   | 62643 |       +44.33%
>
>                 | 7484 MB |       1 |  7974 |   |  8014 |
>                 | 7484 MB |       2 | 19341 |   | 19534 |
>                 | 7484 MB |       4 | 36808 |   | 38092 |
>                 | 7484 MB |       8 | 47821 |   | 51968 |       +8.67%
>                 | 7484 MB |      12 | 45913 |   | 52284 |       +13.88%
>                 | 7484 MB |      16 | 46478 |   | 54418 |       +17.08%
>                 | 7484 MB |      24 | 42793 |   | 56375 |       +31.74%
>                 | 7484 MB |      32 | 36329 |   | 55783 |       +53.55%
>                 
>                 | 15 GB   |       1 |  7636 |   |  7880 |       
>                 | 15 GB   |       2 | 19195 |   | 19477 |
>                 | 15 GB   |       4 | 35975 |   | 37962 |
>                 | 15 GB   |       8 | 47919 |   | 51558 |       +7.59%
>                 | 15 GB   |      12 | 45397 |   | 51163 |       +12.70%
>                 | 15 GB   |      16 | 45926 |   | 53912 |       +17.39%
>                 | 15 GB   |      24 | 42184 |   | 55343 |       +31.19%
>                 | 15 GB   |      32 | 35983 |   | 55358 |       +53.84%
>
> Signed-off-by: Michael Wang <wangyun@linux.vnet.ibm.com>
> ---
[SNIP]
> diff --git a/kernel/sched/fair.c b/kernel/sched/fair.c
> index 81fa536..d5acfd8 100644
> --- a/kernel/sched/fair.c
> +++ b/kernel/sched/fair.c
> @@ -3173,6 +3173,75 @@ static int wake_affine(struct sched_domain *sd, struct task_struct *p, int sync)
>  }
>  
>  /*
> + * Reduce sysctl_sched_wakeup_buddy_ref will reduce the preparation time
> + * to active the wakeup buddy feature, and make it agile, however, this
> + * will increase the risk of misidentify.
> + *
> + * Check wakeup_buddy() for the usage.
> + */
> +unsigned int sysctl_sched_wakeup_buddy_ref = 8UL;

It seems that just 8U (or even 8) is enough.

> +
> +/*
> + * wakeup_buddy() help to check whether p1 is the wakeup buddy of p2.
> + *
> + * Return 1 for yes, 0 for no.
> +*/
> +static inline int wakeup_buddy(struct task_struct *p1, struct task_struct *p2)
> +{
> +	if (p1->waker != p2 || p1->wakee != p2)
> +		return 0;
> +
> +	if (p1->waker_ref < sysctl_sched_wakeup_buddy_ref)
> +		return 0;
> +
> +	if (p1->wakee_ref < sysctl_sched_wakeup_buddy_ref)
> +		return 0;
> +
> +	return 1;
> +}
[SNIP]
> @@ -3399,6 +3490,8 @@ select_task_rq_fair(struct task_struct *p, int sd_flag, int wake_flags)
>  unlock:
>  	rcu_read_unlock();
>  
> +	wakeup_ref(p);
> +

Why did you call it here?  Shouldn't it be on somewhere in the ttwu?


>  	return new_cpu;
>  }
>  
> diff --git a/kernel/sysctl.c b/kernel/sysctl.c
> index c88878d..6845d24 100644
> --- a/kernel/sysctl.c
> +++ b/kernel/sysctl.c
> @@ -424,6 +424,16 @@ static struct ctl_table kern_table[] = {
>  		.extra1		= &one,
>  	},
>  #endif
> +#ifdef CONFIG_SMP
> +	{
> +		.procname	= "sched_wakeup_buddy_ref",
> +		.data		= &sysctl_sched_wakeup_buddy_ref,
> +		.maxlen		= sizeof(unsigned int),
> +		.mode		= 0644,
> +		.proc_handler	= proc_dointvec_minmax,
> +		.extra1		= &one,
> +	},
> +#endif
>  #ifdef CONFIG_PROVE_LOCKING
>  	{
>  		.procname	= "prove_locking",