On Thu, Apr 02, 2020 at 02:32:51PM +0200, Uladzislau Rezki (Sony) wrote:
> It is documneted that a headless object can be reclaimed from
> might_sleep() context only. Because of that when a head is
> dynamically attached it makes sense to drop the lock and do
> an allocation with much more permissve flags comparing if it
> is done from atomic context.
>
> That is why use GFP_KERNEL flag plus some extra ones which
> would make an allocation most likely to be succeed. The big
> advantage of doing so is a direct reclaim process.
>
> Tested such approach on my local tiny system with 145MB of
> ram(the minimum amount the KVM system is capable of booting)
> and 4xCPUs. For stressing the rcuperf module was used. During
> tests with difference combinations i did not observe any hit
> of our last emergency case, when synchronize_rcu() is involved.
>
> Please note, the "dynamically attaching" path was enabled only,
> apart of that all types of objects were considered as headless
> variant during testing.
>
> Signed-off-by: Uladzislau Rezki (Sony) <urezki@xxxxxxxxx>
> Suggested-by: Joel Fernandes (Google) <joel@xxxxxxxxxxxxxxxxx>
> ---
> kernel/rcu/tree.c | 23 ++++++++++++++++-------
> 1 file changed, 16 insertions(+), 7 deletions(-)
>
> diff --git a/kernel/rcu/tree.c b/kernel/rcu/tree.c
> index 6172e6296dd7..24f620a06219 100644
> --- a/kernel/rcu/tree.c
> +++ b/kernel/rcu/tree.c
> @@ -3148,13 +3148,10 @@ static inline struct rcu_head *attach_rcu_head_to_object(void *obj)
> {
> unsigned long *ptr;
>
> + /* Try hard to get the memory. */
> ptr = kmalloc(sizeof(unsigned long *) +
> - sizeof(struct rcu_head), GFP_NOWAIT | __GFP_NOWARN);
> -
> - if (!ptr)
> - ptr = kmalloc(sizeof(unsigned long *) +
> - sizeof(struct rcu_head), GFP_ATOMIC | __GFP_NOWARN);
> -
> + sizeof(struct rcu_head), GFP_KERNEL |
> + __GFP_ATOMIC | __GFP_HIGH | __GFP_RETRY_MAYFAIL);
On thing here though, you removed the NOWARN. Was there a reason? It would
now warn even when synchronously waiting right? I will fixup your commit to
add it back for now but let me know if you had some other reason to remove it.
thanks,
- Joel
> if (!ptr)
> return NULL;
>
> @@ -3222,9 +3219,20 @@ void kvfree_call_rcu(struct rcu_head *head, rcu_callback_t func)
> if (!success) {
> /* Is headless object? */
> if (head == NULL) {
> + /* Drop the lock. */
> + if (krcp->initialized)
> + spin_unlock(&krcp->lock);
> + local_irq_restore(flags);
> +
> head = attach_rcu_head_to_object(ptr);
> if (head == NULL)
> - goto unlock_return;
> + goto inline_return;
> +
> + /* Take it back. */
> + local_irq_save(flags);
> + krcp = this_cpu_ptr(&krc);
> + if (krcp->initialized)
> + spin_lock(&krcp->lock);
>
> /*
> * Tag the headless object. Such objects have a back-pointer
> @@ -3263,6 +3271,7 @@ void kvfree_call_rcu(struct rcu_head *head, rcu_callback_t func)
> spin_unlock(&krcp->lock);
> local_irq_restore(flags);
>
> +inline_return:
> /*
> * High memory pressure, so inline kvfree() after
> * synchronize_rcu(). We can do it from might_sleep()
> --
> 2.20.1
>
