Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1450497 > unrolled thread

Re: [patch 61/66] timers: Convert to hotplug state machine

Back to article view | Back to linux.kernel

This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by below is the oldest one visible, not the original post.

Contents

  Re: [patch 61/66] timers: Convert to hotplug state machine Jon Hunter <jonathanh@nvidia.com> - 2016-07-26 11:30 +0200
    Re: [patch 61/66] timers: Convert to hotplug state machine Thomas Gleixner <tglx@linutronix.de> - 2016-07-26 16:20 +0200
      Re: [patch 61/66] timers: Convert to hotplug state machine Jon Hunter <jonathanh@nvidia.com> - 2016-07-26 20:30 +0200
    Re: [patch 61/66] timers: Convert to hotplug state machine rcochran@linutronix.de - 2016-07-26 16:50 +0200
      Re: [patch 61/66] timers: Convert to hotplug state machine Jon Hunter <jonathanh@nvidia.com> - 2016-07-26 20:30 +0200

#1450497 — Re: [patch 61/66] timers: Convert to hotplug state machine

On 25/07/16 16:35, rcochran@linutronix.de wrote:
> On Mon, Jul 25, 2016 at 03:56:48PM +0100, Jon Hunter wrote:
>>> There is a hidden dependency between:
>>>
>>> - timers
>>> - Block multiqueue
>>> - rcutree
>>>
>>> If timers_dead_cpu() comes later than blk_mq_queue_reinit_notify()
>>> that latter function causes a RCU stall.
>>
>> After this change is applied I am seeing RCU stalls during suspend
>> on Tegra. I guess I am hitting the case mentioned above? How should
>> this be avoided?
> 
> The problem that I had found was a hidden dependency.  When I
> initially placed the timers callback into the new HP state list, that
> caused a stall because the dependency was broken.  The old code worked
> by luck, based on the order of the notifier registrations.  The new
> code makes the old implicit ordering explicit, so it should work just
> as well as before (famous last words).

I see.

>> Interestingly I am only seeing the above when using the ARM
>> multi_v7_defconfig kernel configuration and not with the tegra_defconfig.
>> One key difference between these is that the multi_v7_defconfig does not
>> have CONFIG_PREEMPT enabled. Initial testing shows enabling CONFIG_PREEMPT
>> for multi_v7_defconfig makes the problem go away.
> 
> Just to be sure, this problem didn't exist before the HP rework, that
> is, suspend worked fine with and without CONFIG_PREEMPT, right?

Correct. I test suspend on Tegra with both multi_v7_defconfig
(CONFIG_PREEMPT disabled) and tegra_defconfig (CONFIG_PREEMPT enabled).
Looking at the git history for these configs I don't see any changes in
this regard since they were added (unless some underlying Kconfig files
have changed).

> I see if I can find a tegra system to test with...

Thanks. I have not tried another ARM based device, but I would be
curious if another ARM device sees this or not.

Cheers
Jon

-- 
nvpublic

[toc] | [next] | [standalone]

#1450617

Jon,

On Tue, 26 Jul 2016, Jon Hunter wrote:
> On 25/07/16 16:35, rcochran@linutronix.de wrote:
> > Just to be sure, this problem didn't exist before the HP rework, that
> > is, suspend worked fine with and without CONFIG_PREEMPT, right?
> 
> Correct. I test suspend on Tegra with both multi_v7_defconfig
> (CONFIG_PREEMPT disabled) and tegra_defconfig (CONFIG_PREEMPT enabled).
> Looking at the git history for these configs I don't see any changes in
> this regard since they were added (unless some underlying Kconfig files
> have changed).

Is that fully reproducible, i.e on every suspend?

Can you please check whether this issue happens with just cpu offline as
well? I.e. set a cpu (or all non-boot cpus) offline, wait long enough that the
stall detector can trigger, set the cpu(s) online again and repeat.

Thanks

	tglx

[toc] | [prev] | [next] | [standalone]

#1450767

Hi Thomas,

On 26/07/16 15:15, Thomas Gleixner wrote:
> Jon,
> 
> On Tue, 26 Jul 2016, Jon Hunter wrote:
>> On 25/07/16 16:35, rcochran@linutronix.de wrote:
>>> Just to be sure, this problem didn't exist before the HP rework, that
>>> is, suspend worked fine with and without CONFIG_PREEMPT, right?
>>
>> Correct. I test suspend on Tegra with both multi_v7_defconfig
>> (CONFIG_PREEMPT disabled) and tegra_defconfig (CONFIG_PREEMPT enabled).
>> Looking at the git history for these configs I don't see any changes in
>> this regard since they were added (unless some underlying Kconfig files
>> have changed).
> 
> Is that fully reproducible, i.e on every suspend?

No not every suspend. I run 10 suspend cycles on each Tegra board (5
boards total) and typically between 3 and 5 boards would see the stall
in the 10 suspend cycles. So it does appear to be timing sensitive.

> Can you please check whether this issue happens with just cpu offline as
> well? I.e. set a cpu (or all non-boot cpus) offline, wait long enough that the
> stall detector can trigger, set the cpu(s) online again and repeat.

I can, but it appears like Richard's patch has fixed this. I am happy to
do more testing if necessary.

Cheers
Jon

-- 
nvpublic

[toc] | [prev] | [next] | [standalone]

#1450627

Jon,

On Tue, Jul 26, 2016 at 10:20:58AM +0100, Jon Hunter wrote:
> Thanks. I have not tried another ARM based device, but I would be
> curious if another ARM device sees this or not.

I do see this stall on socfpga and on zynq, but in both cases the
suspend mechanism is flakey in other ways, too.  At least I can
reproduce the stall sometimes.

In your other mail you wrote that you test it like this:

   rtcwake -d rtc1 -m mem -s 3

But the stall appears only after 20 seconds.  So the resume event
after three seconds (-s 3) is getting lost, right?

Thanks,
Richard

[toc] | [prev] | [next] | [standalone]

#1450775

Hi Richard,

On 26/07/16 15:40, rcochran@linutronix.de wrote:
> Jon,
> 
> On Tue, Jul 26, 2016 at 10:20:58AM +0100, Jon Hunter wrote:
>> Thanks. I have not tried another ARM based device, but I would be
>> curious if another ARM device sees this or not.
> 
> I do see this stall on socfpga and on zynq, but in both cases the
> suspend mechanism is flakey in other ways, too.  At least I can
> reproduce the stall sometimes.
> 
> In your other mail you wrote that you test it like this:
> 
>    rtcwake -d rtc1 -m mem -s 3
> 
> But the stall appears only after 20 seconds.  So the resume event
> after three seconds (-s 3) is getting lost, right?

I don't think so. I noticed that when the stall occurs, I don't see the
board attempt to transition to the Tegra LP1 power state in suspend and
appears to wake up straight away. So I think the wake-up is seen, but
the stall prevents it from transitioning all the way to LP1.

Cheers
Jon

-- 
nvpublic

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web