On Fri, Oct 19, 2018 at 02:49:05AM +0200, KarimAllah Ahmed wrote:
> When expedited grace-period is set, both synchronize_sched
> synchronize_rcu_bh can be optimized to have a significantly lower latency.
>
> Improve wait_rcu_gp handling to also account for expedited grace-period.
> The downside is that wait_rcu_gp will not wait anymore for all RCU variants
> concurrently when an expedited grace-period is set, however, given the
> improved latency it does not really matter.
>
> Cc: Paul E. McKenney <paul...@linux.vnet.ibm.com>
> Cc: Josh Triplett <j...@joshtriplett.org>
> Cc: Steven Rostedt <rost...@goodmis.org>
> Cc: Mathieu Desnoyers <mathieu.desnoy...@efficios.com>
> Cc: Lai Jiangshan <jiangshan...@gmail.com>
> Cc: linux-kernel@vger.kernel.org
> Signed-off-by: KarimAllah Ahmed <karah...@amazon.de>
Cute!
Unfortunately, there are a few problems with this patch:
1. I will be eliminating synchronize_rcu_mult() due to the fact that
the upcoming RCU flavor consolidation eliminates its sole caller.
See 5fc9d4e000b1 ("rcu: Eliminate synchronize_rcu_mult()")
in my -rcu tree. This would of course also eliminate the effects
of this patch.
2. The real-time guys' users are not going to be at all happy
with the IPIs resulting from the _expedited() API members.
Yes, they can boot with rcupdate.rcu_normal=1, but they don't
always need that big a hammer, and use of this kernel parameter
can slow down boot, hibernation, suspend, network configuration,
and much else besides. We therefore don't want them to have to
use rcupdate.rcu_normal=1 unless absolutely necessary.
3. If the real-time guys' users were to have booted with
rcupdate.rcu_normal=1, then synchronize_sched_expedited()
would invoke _synchronize_rcu_expedited, which would invoke
wait_rcu_gp(), which would invoke _wait_rcu_gp() which would
invoke __wait_rcu_gp(), which, given your patch, would in turn
invoke synchronize_sched_expedited(). This situation could
well prevent their systems from meeting their response-time
requirements.
So I cannot accept this patch nor for that matter any similar patch.
But what were you really trying to get done here? If you were thinking
of adding another synchronize_rcu_mult(), the flavor consolidation will
make that unnecessary in most cases. If you are trying to speed up
CPU-hotplug operations, I suggest using the rcu_expedited sysctl variable
when taking a CPU offline. If something else, please let me know what
it is so that we can work out how the problem might best be solved.
Thanx, Paul
> ---
> kernel/rcu/update.c | 34 ++++++++++++++++++++++++++++------
> 1 file changed, 28 insertions(+), 6 deletions(-)
>
> diff --git a/kernel/rcu/update.c b/kernel/rcu/update.c
> index 68fa19a..44b8817 100644
> --- a/kernel/rcu/update.c
> +++ b/kernel/rcu/update.c
> @@ -392,13 +392,27 @@ void __wait_rcu_gp(bool checktiny, int n,
> call_rcu_func_t *crcu_array,
> might_sleep();
> continue;
> }
> - init_rcu_head_on_stack(&rs_array[i].head);
> - init_completion(&rs_array[i].completion);
> +
> for (j = 0; j < i; j++)
> if (crcu_array[j] == crcu_array[i])
> break;
> - if (j == i)
> - (crcu_array[i])(&rs_array[i].head, wakeme_after_rcu);
> + if (j != i)
> + continue;
> +
> + if ((crcu_array[i] == call_rcu_sched ||
> + crcu_array[i] == call_rcu_bh)
> + && rcu_gp_is_expedited()) {
> + if (crcu_array[i] == call_rcu_sched)
> + synchronize_sched_expedited();
> + else
> + synchronize_rcu_bh_expedited();
> +
> + continue;
> + }
> +
> + init_rcu_head_on_stack(&rs_array[i].head);
> + init_completion(&rs_array[i].completion);
> + (crcu_array[i])(&rs_array[i].head, wakeme_after_rcu);
> }
>
> /* Wait for all callbacks to be invoked. */
> @@ -407,11 +421,19 @@ void __wait_rcu_gp(bool checktiny, int n,
> call_rcu_func_t *crcu_array,
> (crcu_array[i] == call_rcu ||
> crcu_array[i] == call_rcu_bh))
> continue;
> +
> + if ((crcu_array[i] == call_rcu_sched ||
> + crcu_array[i] == call_rcu_bh)
> + && rcu_gp_is_expedited())
> + continue;
> +
> for (j = 0; j < i; j++)
> if (crcu_array[j] == crcu_array[i])
> break;
> - if (j == i)
> - wait_for_completion(&rs_array[i].completion);
> + if (j != i)
> + continue;
> +
> + wait_for_completion(&rs_array[i].completion);
> destroy_rcu_head_on_stack(&rs_array[i].head);
> }
> }
> --
> 2.7.4
>
