livelock with hrtimer cpu_base->lock

livelock with hrtimer cpu_base->lock

[Sodagudi Prasad]

Hi Will,

This is regarding - thread "try to fix contention between expire_timers 
and try_to_del_timer_sync".
https://lkml.org/lkml/2017/7/28/172

I think this live lockup issue was discussed earlier but the final set 
of changes were not concluded.
I would like to check whether you have new updates on this issue or not. 
  This problem is observed with 4.14 .64 stable kernel too.
We see this problem 2 times in overnight testing.

I have to add the following code to avoid live lock. I am thinking that 
fixing this at the cpu_relax() level.

+++ b/kernel/time/hrtimer.c
@@ -52,6 +52,7 @@
  #include <linux/timer.h>
  #include <linux/freezer.h>
  #include <linux/compat.h>
+#include <linux/delay.h>

  #include <linux/uaccess.h>

@@ -152,6 +153,7 @@ struct hrtimer_clock_base *lock_hrtimer_base(const 
struct hrtimer *timer,
                         
raw_spin_unlock_irqrestore(&base->cpu_base->lock, *flags);
                 }
                 cpu_relax();
+               udelay(1);
         }
  }

@@ -1067,6 +1069,7 @@ int hrtimer_cancel(struct hrtimer *timer)
                 if (ret >= 0)
                         return ret;
                 cpu_relax();
+               udelay(1);
         }
  }
  EXPORT_SYMBOL_GPL(hrtimer_cancel);

Note:- Timer event streaming is enabled and still live lock observed.

-Thanks, Prasad

-- 
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora 
Forum,
Linux Foundation Collaborative Project

Re: livelock with hrtimer cpu_base->lock

[Will Deacon]

Hi Prasad,

On Tue, Oct 09, 2018 at 01:56:14PM -0700, Sodagudi Prasad wrote:
> This is regarding - thread "try to fix contention between expire_timers and
> try_to_del_timer_sync".
> https://lkml.org/lkml/2017/7/28/172
> 
> I think this live lockup issue was discussed earlier but the final set of
> changes were not concluded.

Well we basically need a way to pick a value for CPU_RELAX_WFE_THRESHOLD.
Do you have any ideas? It could be determined at runtime if necessary.

> I would like to check whether you have new updates on this issue or not.
> This problem is observed with 4.14 .64 stable kernel too.
> We see this problem 2 times in overnight testing.
> 
> I have to add the following code to avoid live lock. I am thinking that
> fixing this at the cpu_relax() level.
> 
> +++ b/kernel/time/hrtimer.c
> @@ -52,6 +52,7 @@
>  #include <linux/timer.h>
>  #include <linux/freezer.h>
>  #include <linux/compat.h>
> +#include <linux/delay.h>
> 
>  #include <linux/uaccess.h>
> 
> @@ -152,6 +153,7 @@ struct hrtimer_clock_base *lock_hrtimer_base(const
> struct hrtimer *timer,
> raw_spin_unlock_irqrestore(&base->cpu_base->lock, *flags);
>                 }
>                 cpu_relax();
> +               udelay(1);
>         }
>  }
> 
> @@ -1067,6 +1069,7 @@ int hrtimer_cancel(struct hrtimer *timer)
>                 if (ret >= 0)
>                         return ret;
>                 cpu_relax();
> +               udelay(1);
>         }
>  }
>  EXPORT_SYMBOL_GPL(hrtimer_cancel);

This is just another bodge and likely to hurt in places where 1us is
excessive because there isn't contention.

Will

Re: livelock with hrtimer cpu_base->lock

[Sodagudi Prasad]

On 2018-10-10 09:49, Will Deacon wrote:
> Hi Prasad,
> 
> On Tue, Oct 09, 2018 at 01:56:14PM -0700, Sodagudi Prasad wrote:
>> This is regarding - thread "try to fix contention between 
>> expire_timers and
>> try_to_del_timer_sync".
>> https://lkml.org/lkml/2017/7/28/172
>> 
>> I think this live lockup issue was discussed earlier but the final set 
>> of
>> changes were not concluded.
> 
> Well we basically need a way to pick a value for 
> CPU_RELAX_WFE_THRESHOLD.
> Do you have any ideas? It could be determined at runtime if necessary.
> 
Hi Will,

Please share what are values need to be tried for 
CPU_RELAX_WFE_THRESHOLD.

  It would be great if it can be determined from runtime.  Please let me 
know
if any testing need to be done with dynamic detection patch.

-Thanks, Prasad

>> I would like to check whether you have new updates on this issue or 
>> not.
>> This problem is observed with 4.14 .64 stable kernel too.
>> We see this problem 2 times in overnight testing.
>> 
>> I have to add the following code to avoid live lock. I am thinking 
>> that
>> fixing this at the cpu_relax() level.
>> 
>> +++ b/kernel/time/hrtimer.c
>> @@ -52,6 +52,7 @@
>>  #include <linux/timer.h>
>>  #include <linux/freezer.h>
>>  #include <linux/compat.h>
>> +#include <linux/delay.h>
>> 
>>  #include <linux/uaccess.h>
>> 
>> @@ -152,6 +153,7 @@ struct hrtimer_clock_base *lock_hrtimer_base(const
>> struct hrtimer *timer,
>> raw_spin_unlock_irqrestore(&base->cpu_base->lock, *flags);
>>                 }
>>                 cpu_relax();
>> +               udelay(1);
>>         }
>>  }
>> 
>> @@ -1067,6 +1069,7 @@ int hrtimer_cancel(struct hrtimer *timer)
>>                 if (ret >= 0)
>>                         return ret;
>>                 cpu_relax();
>> +               udelay(1);
>>         }
>>  }
>>  EXPORT_SYMBOL_GPL(hrtimer_cancel);
> 
> This is just another bodge and likely to hurt in places where 1us is
> excessive because there isn't contention.
> 
> Will

-- 
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora 
Forum,
Linux Foundation Collaborative Project

Re: livelock with hrtimer cpu_base->lock

[Will Deacon]

On Fri, Oct 12, 2018 at 05:55:28AM -0700, Sodagudi Prasad wrote:
> On 2018-10-10 09:49, Will Deacon wrote:
> >On Tue, Oct 09, 2018 at 01:56:14PM -0700, Sodagudi Prasad wrote:
> >>This is regarding - thread "try to fix contention between expire_timers
> >>and
> >>try_to_del_timer_sync".
> >>https://lkml.org/lkml/2017/7/28/172
> >>
> >>I think this live lockup issue was discussed earlier but the final set
> >>of
> >>changes were not concluded.
> >
> >Well we basically need a way to pick a value for CPU_RELAX_WFE_THRESHOLD.
> >Do you have any ideas? It could be determined at runtime if necessary.
> >
> 
> Please share what are values need to be tried for CPU_RELAX_WFE_THRESHOLD.
> 
>  It would be great if it can be determined from runtime.  Please let me know
> if any testing need to be done with dynamic detection patch.

I was actually hoping you'd have some ideas about how to write the patch. I
don't have a good feel for how to determine this threshold, and you have
hardware that needs it...

One avenue of exploration might be to tune the fallback to WFE based on
what percentage of the eventstream period we've been polling, e.g. 50%.

Will