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

Groups > linux.kernel > #1570168 > unrolled thread

Re: [Linux v4.10.0-rc1+] Still call-traces after suspend-resume (pm? i915? cpu/hotplug?)

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: [Linux v4.10.0-rc1+] Still call-traces after suspend-resume (pm?  i915? cpu/hotplug?) "Rafael J. Wysocki" <rafael.j.wysocki@intel.com> - 2017-01-30 23:50 +0100
    Re: [Intel-gfx] [Linux v4.10.0-rc1+] Still call-traces after  suspend-resume (pm? i915? cpu/hotplug?) Imre Deak <imre.deak@intel.com> - 2017-01-31 12:10 +0100
      Re: [Intel-gfx] [Linux v4.10.0-rc1+] Still call-traces after  suspend-resume (pm? i915? cpu/hotplug?) "Rafael J. Wysocki" <rafael.j.wysocki@intel.com> - 2017-01-31 12:50 +0100
        Re: [Intel-gfx] [Linux v4.10.0-rc1+] Still call-traces after  suspend-resume (pm? i915? cpu/hotplug?) Imre Deak <imre.deak@intel.com> - 2017-01-31 13:10 +0100
          Re: [Intel-gfx] [Linux v4.10.0-rc1+] Still call-traces after  suspend-resume (pm? i915? cpu/hotplug?) "Rafael J. Wysocki" <rafael.j.wysocki@intel.com> - 2017-01-31 13:20 +0100
            Re: [Intel-gfx] [Linux v4.10.0-rc1+] Still call-traces after  suspend-resume (pm? i915? cpu/hotplug?) Imre Deak <imre.deak@intel.com> - 2017-01-31 14:20 +0100
    Re: [Linux v4.10.0-rc1+] Still call-traces after suspend-resume (pm?  i915? cpu/hotplug?) "Rafael J. Wysocki" <rafael@kernel.org> - 2017-02-01 01:30 +0100
      [PATCH] PM / runtime: Avoid false-positive warnings from might_sleep_if() "Rafael J. Wysocki" <rjw@rjwysocki.net> - 2017-02-04 01:10 +0100
        Re: [PATCH] PM / runtime: Avoid false-positive warnings from might_sleep_if() Sedat Dilek <sedat.dilek@gmail.com> - 2017-02-05 11:00 +0100

#1570168 — Re: [Linux v4.10.0-rc1+] Still call-traces after suspend-resume (pm? i915? cpu/hotplug?)

On 1/24/2017 2:33 AM, Sedat Dilek wrote:
> On Fri, Dec 30, 2016 at 3:02 PM, Rafael J. Wysocki <rafael@kernel.org> wrote:
>> On Fri, Dec 30, 2016 at 12:40 PM, Sedat Dilek <sedat.dilek@gmail.com> wrote:
>>> Hi,
>>>
>>> I have already reported this issue in [1].
>>> One of the issue was solved.
>>> Unfortunately, it looks like there is still a different problem here
>>> (Ubuntu/precise AMD64).
>>>
>>> I tried v4.10-rc1 and latest Linus tree up to...
>>>
>>> commit 98473f9f3f9bd404873cd1178c8be7d6d619f0d1
>>> "mm/filemap: fix parameters to test_bit()"
>>>
>>> Here we go...
>>>
>>> [   29.636047] BUG: sleeping function called from invalid context at
>>> drivers/base/power/runtime.c:1032
>>> [   29.636055] in_atomic(): 1, irqs_disabled(): 0, pid: 1500, name: Xorg
>>> [   29.636058] 1 lock held by Xorg/1500:
>>> [   29.636060]  #0:  (&dev->struct_mutex){+.+.+.}, at:
>>> [<ffffffffa0680c13>] i915_mutex_lock_interruptible+0x43/0x140 [i915]
>>> [   29.636107] CPU: 0 PID: 1500 Comm: Xorg Not tainted
>>> 4.10.0-rc1-6-iniza-amd64 #1
>>> [   29.636109] Hardware name: SAMSUNG ELECTRONICS CO., LTD.
>>> 530U3BI/530U4BI/530U4BH/530U3BI/530U4BI/530U4BH, BIOS 13XK 03/28/2013
>>> [   29.636111] Call Trace:
>>> [   29.636120]  dump_stack+0x85/0xc2
>>> [   29.636124]  ___might_sleep+0x196/0x260
>>> [   29.636127]  __might_sleep+0x53/0xb0
>>> [   29.636131]  __pm_runtime_resume+0x7a/0x90
>>> [   29.636159]  intel_runtime_pm_get+0x25/0x90 [i915]
>>> [   29.636189]  aliasing_gtt_bind_vma+0xaa/0xf0 [i915]
>>> [   29.636220]  i915_vma_bind+0xaf/0x1e0 [i915]
>>> [   29.636248]  i915_gem_execbuffer_relocate_entry+0x513/0x6f0 [i915]
>>> [   29.636272]  i915_gem_execbuffer_relocate_vma.isra.34+0x188/0x250 [i915]
>>> [   29.636275]  ? trace_hardirqs_on+0xd/0x10
>>> [   29.636294]  ? i915_gem_execbuffer_reserve_vma.isra.31+0x152/0x1f0 [i915]
>>> [   29.636316]  ? i915_gem_execbuffer_reserve.isra.32+0x372/0x3a0 [i915]
>>> [   29.636342]  i915_gem_do_execbuffer.isra.38+0xa70/0x1a40 [i915]
>>> [   29.636347]  ? __might_fault+0x4e/0xb0
>>> [   29.636373]  i915_gem_execbuffer2+0xc5/0x260 [i915]
>>> [   29.636376]  ? __might_fault+0x4e/0xb0
>>> [   29.636395]  drm_ioctl+0x206/0x450 [drm]
>>> [   29.636420]  ? i915_gem_execbuffer+0x340/0x340 [i915]
>>> [   29.636425]  ? __fget+0x5/0x200
>>> [   29.636429]  do_vfs_ioctl+0x91/0x6f0
>>> [   29.636431]  ? __fget+0x111/0x200
>>> [   29.636433]  ? __fget+0x5/0x200
>>> [   29.636436]  SyS_ioctl+0x79/0x90
>>> [   29.636441]  entry_SYSCALL_64_fastpath+0x23/0xc6
>>>
>>> On suspend/resume I see the same call trace.
>>> [2] points to the "BUG" line.
>> Well, this appears to be an i915 issue, but not a serious one.
>>
>> Clearly, a function that may sleep (pm_runtime_get_sync() in
>> intel_runtime_pm_get()) is called with disabled interrupts.  If I
>> understand the code correctly, though, it actually is not going to
>> sleep in this particular case, because pm_runtime_get_sync() has
>> already been called once for this device in the same code path which
>> means that this particular instance will return immediately, so this
>> is a false-positive (most likely).
>>
>> Let me see if I the might_sleep_if() assertion in
>> __pm_runtime_resume(() can be moved to a better place.
>>
> Hi Rafael,
>
> did you had a chance to look at this?
> The problem still remains in Linux v4.10-rc5.

No, I didn't.

As I said, this is not a serious issue.

Thanks,
Rafael

[toc] | [next] | [standalone]

#1570608 — Re: [Intel-gfx] [Linux v4.10.0-rc1+] Still call-traces after suspend-resume (pm? i915? cpu/hotplug?)

Hi Rafael,

On Mon, Jan 30, 2017 at 11:44:37PM +0100, Rafael J. Wysocki wrote:
> On 1/24/2017 2:33 AM, Sedat Dilek wrote:
> >On Fri, Dec 30, 2016 at 3:02 PM, Rafael J. Wysocki <rafael@kernel.org> wrote:
> >>On Fri, Dec 30, 2016 at 12:40 PM, Sedat Dilek <sedat.dilek@gmail.com> wrote:
> >>>Hi,
> >>>
> >>>I have already reported this issue in [1].
> >>>One of the issue was solved.
> >>>Unfortunately, it looks like there is still a different problem here
> >>>(Ubuntu/precise AMD64).
> >>>
> >>>I tried v4.10-rc1 and latest Linus tree up to...
> >>>
> >>>commit 98473f9f3f9bd404873cd1178c8be7d6d619f0d1
> >>>"mm/filemap: fix parameters to test_bit()"
> >>>
> >>>Here we go...
> >>>
> >>>[   29.636047] BUG: sleeping function called from invalid context at
> >>>drivers/base/power/runtime.c:1032
> >>>[   29.636055] in_atomic(): 1, irqs_disabled(): 0, pid: 1500, name: Xorg
> >>>[   29.636058] 1 lock held by Xorg/1500:
> >>>[   29.636060]  #0:  (&dev->struct_mutex){+.+.+.}, at:
> >>>[<ffffffffa0680c13>] i915_mutex_lock_interruptible+0x43/0x140 [i915]
> >>>[   29.636107] CPU: 0 PID: 1500 Comm: Xorg Not tainted
> >>>4.10.0-rc1-6-iniza-amd64 #1
> >>>[   29.636109] Hardware name: SAMSUNG ELECTRONICS CO., LTD.
> >>>530U3BI/530U4BI/530U4BH/530U3BI/530U4BI/530U4BH, BIOS 13XK 03/28/2013
> >>>[   29.636111] Call Trace:
> >>>[   29.636120]  dump_stack+0x85/0xc2
> >>>[   29.636124]  ___might_sleep+0x196/0x260
> >>>[   29.636127]  __might_sleep+0x53/0xb0
> >>>[   29.636131]  __pm_runtime_resume+0x7a/0x90
> >>>[   29.636159]  intel_runtime_pm_get+0x25/0x90 [i915]
> >>>[   29.636189]  aliasing_gtt_bind_vma+0xaa/0xf0 [i915]
> >>>[   29.636220]  i915_vma_bind+0xaf/0x1e0 [i915]
> >>>[   29.636248]  i915_gem_execbuffer_relocate_entry+0x513/0x6f0 [i915]
> >>>[   29.636272]  i915_gem_execbuffer_relocate_vma.isra.34+0x188/0x250 [i915]
> >>>[   29.636275]  ? trace_hardirqs_on+0xd/0x10
> >>>[   29.636294]  ? i915_gem_execbuffer_reserve_vma.isra.31+0x152/0x1f0 [i915]
> >>>[   29.636316]  ? i915_gem_execbuffer_reserve.isra.32+0x372/0x3a0 [i915]
> >>>[   29.636342]  i915_gem_do_execbuffer.isra.38+0xa70/0x1a40 [i915]
> >>>[   29.636347]  ? __might_fault+0x4e/0xb0
> >>>[   29.636373]  i915_gem_execbuffer2+0xc5/0x260 [i915]
> >>>[   29.636376]  ? __might_fault+0x4e/0xb0
> >>>[   29.636395]  drm_ioctl+0x206/0x450 [drm]
> >>>[   29.636420]  ? i915_gem_execbuffer+0x340/0x340 [i915]
> >>>[   29.636425]  ? __fget+0x5/0x200
> >>>[   29.636429]  do_vfs_ioctl+0x91/0x6f0
> >>>[   29.636431]  ? __fget+0x111/0x200
> >>>[   29.636433]  ? __fget+0x5/0x200
> >>>[   29.636436]  SyS_ioctl+0x79/0x90
> >>>[   29.636441]  entry_SYSCALL_64_fastpath+0x23/0xc6
> >>>
> >>>On suspend/resume I see the same call trace.
> >>>[2] points to the "BUG" line.
> >>Well, this appears to be an i915 issue, but not a serious one.
> >>
> >>Clearly, a function that may sleep (pm_runtime_get_sync() in
> >>intel_runtime_pm_get()) is called with disabled interrupts.  If I
> >>understand the code correctly, though, it actually is not going to
> >>sleep in this particular case, because pm_runtime_get_sync() has
> >>already been called once for this device in the same code path which
> >>means that this particular instance will return immediately, so this
> >>is a false-positive (most likely).

Not sure what's the root cause, but thought to clarify the above:

Yes, i915_gem_do_execbuffer() does take an RPM reference to optimize
things, so the RPM get in aliasing_gtt_bind_vma() won't need to resume
the device on this path. This isn't a guarantee though,
aliasing_gtt_bind_vma() could be called from other places without an RPM
reference. So preemption being disabled at that point is not
intentional. I also can't see where on the above path it would get
disabled due to a bug or otherwise.

--Imre

> >>
> >>Let me see if I the might_sleep_if() assertion in
> >>__pm_runtime_resume(() can be moved to a better place.
> >>
> >Hi Rafael,
> >
> >did you had a chance to look at this?
> >The problem still remains in Linux v4.10-rc5.
> 
> No, I didn't.

> 
> As I said, this is not a serious issue.
> 
> Thanks,
> Rafael
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

#1570643 — Re: [Intel-gfx] [Linux v4.10.0-rc1+] Still call-traces after suspend-resume (pm? i915? cpu/hotplug?)

On 1/31/2017 11:58 AM, Imre Deak wrote:
> Hi Rafael,

Hi,

> On Mon, Jan 30, 2017 at 11:44:37PM +0100, Rafael J. Wysocki wrote:
>> On 1/24/2017 2:33 AM, Sedat Dilek wrote:
>>> On Fri, Dec 30, 2016 at 3:02 PM, Rafael J. Wysocki <rafael@kernel.org> wrote:
>>>> On Fri, Dec 30, 2016 at 12:40 PM, Sedat Dilek <sedat.dilek@gmail.com> wrote:
>>>>> Hi,
>>>>>
>>>>> I have already reported this issue in [1].
>>>>> One of the issue was solved.
>>>>> Unfortunately, it looks like there is still a different problem here
>>>>> (Ubuntu/precise AMD64).
>>>>>
>>>>> I tried v4.10-rc1 and latest Linus tree up to...
>>>>>
>>>>> commit 98473f9f3f9bd404873cd1178c8be7d6d619f0d1
>>>>> "mm/filemap: fix parameters to test_bit()"
>>>>>
>>>>> Here we go...
>>>>>
>>>>> [   29.636047] BUG: sleeping function called from invalid context at
>>>>> drivers/base/power/runtime.c:1032
>>>>> [   29.636055] in_atomic(): 1, irqs_disabled(): 0, pid: 1500, name: Xorg
>>>>> [   29.636058] 1 lock held by Xorg/1500:
>>>>> [   29.636060]  #0:  (&dev->struct_mutex){+.+.+.}, at:
>>>>> [<ffffffffa0680c13>] i915_mutex_lock_interruptible+0x43/0x140 [i915]
>>>>> [   29.636107] CPU: 0 PID: 1500 Comm: Xorg Not tainted
>>>>> 4.10.0-rc1-6-iniza-amd64 #1
>>>>> [   29.636109] Hardware name: SAMSUNG ELECTRONICS CO., LTD.
>>>>> 530U3BI/530U4BI/530U4BH/530U3BI/530U4BI/530U4BH, BIOS 13XK 03/28/2013
>>>>> [   29.636111] Call Trace:
>>>>> [   29.636120]  dump_stack+0x85/0xc2
>>>>> [   29.636124]  ___might_sleep+0x196/0x260
>>>>> [   29.636127]  __might_sleep+0x53/0xb0
>>>>> [   29.636131]  __pm_runtime_resume+0x7a/0x90
>>>>> [   29.636159]  intel_runtime_pm_get+0x25/0x90 [i915]
>>>>> [   29.636189]  aliasing_gtt_bind_vma+0xaa/0xf0 [i915]
>>>>> [   29.636220]  i915_vma_bind+0xaf/0x1e0 [i915]
>>>>> [   29.636248]  i915_gem_execbuffer_relocate_entry+0x513/0x6f0 [i915]
>>>>> [   29.636272]  i915_gem_execbuffer_relocate_vma.isra.34+0x188/0x250 [i915]
>>>>> [   29.636275]  ? trace_hardirqs_on+0xd/0x10
>>>>> [   29.636294]  ? i915_gem_execbuffer_reserve_vma.isra.31+0x152/0x1f0 [i915]
>>>>> [   29.636316]  ? i915_gem_execbuffer_reserve.isra.32+0x372/0x3a0 [i915]
>>>>> [   29.636342]  i915_gem_do_execbuffer.isra.38+0xa70/0x1a40 [i915]
>>>>> [   29.636347]  ? __might_fault+0x4e/0xb0
>>>>> [   29.636373]  i915_gem_execbuffer2+0xc5/0x260 [i915]
>>>>> [   29.636376]  ? __might_fault+0x4e/0xb0
>>>>> [   29.636395]  drm_ioctl+0x206/0x450 [drm]
>>>>> [   29.636420]  ? i915_gem_execbuffer+0x340/0x340 [i915]
>>>>> [   29.636425]  ? __fget+0x5/0x200
>>>>> [   29.636429]  do_vfs_ioctl+0x91/0x6f0
>>>>> [   29.636431]  ? __fget+0x111/0x200
>>>>> [   29.636433]  ? __fget+0x5/0x200
>>>>> [   29.636436]  SyS_ioctl+0x79/0x90
>>>>> [   29.636441]  entry_SYSCALL_64_fastpath+0x23/0xc6
>>>>>
>>>>> On suspend/resume I see the same call trace.
>>>>> [2] points to the "BUG" line.
>>>> Well, this appears to be an i915 issue, but not a serious one.
>>>>
>>>> Clearly, a function that may sleep (pm_runtime_get_sync() in
>>>> intel_runtime_pm_get()) is called with disabled interrupts.  If I
>>>> understand the code correctly, though, it actually is not going to
>>>> sleep in this particular case, because pm_runtime_get_sync() has
>>>> already been called once for this device in the same code path which
>>>> means that this particular instance will return immediately, so this
>>>> is a false-positive (most likely).
> Not sure what's the root cause, but thought to clarify the above:
>
> Yes, i915_gem_do_execbuffer() does take an RPM reference to optimize
> things, so the RPM get in aliasing_gtt_bind_vma() won't need to resume
> the device on this path. This isn't a guarantee though,
> aliasing_gtt_bind_vma() could be called from other places without an RPM
> reference. So preemption being disabled at that point is not
> intentional. I also can't see where on the above path it would get
> disabled due to a bug or otherwise.

The i915 code is correct AFAICS, but the core complains about a nested 
invocation of pm_runtime_get_sync() with disabled interrupts (which 
looks OK though), so the complaint is a false positive.

As I said, the way to go here appears to be to tweak the core, which I'm 
going to do.

Thanks,
Rafael

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

#1570658 — Re: [Intel-gfx] [Linux v4.10.0-rc1+] Still call-traces after suspend-resume (pm? i915? cpu/hotplug?)

On Tue, Jan 31, 2017 at 12:39:35PM +0100, Rafael J. Wysocki wrote:
> On 1/31/2017 11:58 AM, Imre Deak wrote:
> >Hi Rafael,
> 
> Hi,
> 
> >On Mon, Jan 30, 2017 at 11:44:37PM +0100, Rafael J. Wysocki wrote:
> >>On 1/24/2017 2:33 AM, Sedat Dilek wrote:
> >>>On Fri, Dec 30, 2016 at 3:02 PM, Rafael J. Wysocki <rafael@kernel.org> wrote:
> >>>>On Fri, Dec 30, 2016 at 12:40 PM, Sedat Dilek <sedat.dilek@gmail.com> wrote:
> >>>>>Hi,
> >>>>>
> >>>>>I have already reported this issue in [1].
> >>>>>One of the issue was solved.
> >>>>>Unfortunately, it looks like there is still a different problem here
> >>>>>(Ubuntu/precise AMD64).
> >>>>>
> >>>>>I tried v4.10-rc1 and latest Linus tree up to...
> >>>>>
> >>>>>commit 98473f9f3f9bd404873cd1178c8be7d6d619f0d1
> >>>>>"mm/filemap: fix parameters to test_bit()"
> >>>>>
> >>>>>Here we go...
> >>>>>
> >>>>>[   29.636047] BUG: sleeping function called from invalid context at
> >>>>>drivers/base/power/runtime.c:1032
> >>>>>[   29.636055] in_atomic(): 1, irqs_disabled(): 0, pid: 1500, name: Xorg
> >>>>>[   29.636058] 1 lock held by Xorg/1500:
> >>>>>[   29.636060]  #0:  (&dev->struct_mutex){+.+.+.}, at:
> >>>>>[<ffffffffa0680c13>] i915_mutex_lock_interruptible+0x43/0x140 [i915]
> >>>>>[   29.636107] CPU: 0 PID: 1500 Comm: Xorg Not tainted
> >>>>>4.10.0-rc1-6-iniza-amd64 #1
> >>>>>[   29.636109] Hardware name: SAMSUNG ELECTRONICS CO., LTD.
> >>>>>530U3BI/530U4BI/530U4BH/530U3BI/530U4BI/530U4BH, BIOS 13XK 03/28/2013
> >>>>>[   29.636111] Call Trace:
> >>>>>[   29.636120]  dump_stack+0x85/0xc2
> >>>>>[   29.636124]  ___might_sleep+0x196/0x260
> >>>>>[   29.636127]  __might_sleep+0x53/0xb0
> >>>>>[   29.636131]  __pm_runtime_resume+0x7a/0x90
> >>>>>[   29.636159]  intel_runtime_pm_get+0x25/0x90 [i915]
> >>>>>[   29.636189]  aliasing_gtt_bind_vma+0xaa/0xf0 [i915]
> >>>>>[   29.636220]  i915_vma_bind+0xaf/0x1e0 [i915]
> >>>>>[   29.636248]  i915_gem_execbuffer_relocate_entry+0x513/0x6f0 [i915]
> >>>>>[   29.636272]  i915_gem_execbuffer_relocate_vma.isra.34+0x188/0x250 [i915]
> >>>>>[   29.636275]  ? trace_hardirqs_on+0xd/0x10
> >>>>>[   29.636294]  ? i915_gem_execbuffer_reserve_vma.isra.31+0x152/0x1f0 [i915]
> >>>>>[   29.636316]  ? i915_gem_execbuffer_reserve.isra.32+0x372/0x3a0 [i915]
> >>>>>[   29.636342]  i915_gem_do_execbuffer.isra.38+0xa70/0x1a40 [i915]
> >>>>>[   29.636347]  ? __might_fault+0x4e/0xb0
> >>>>>[   29.636373]  i915_gem_execbuffer2+0xc5/0x260 [i915]
> >>>>>[   29.636376]  ? __might_fault+0x4e/0xb0
> >>>>>[   29.636395]  drm_ioctl+0x206/0x450 [drm]
> >>>>>[   29.636420]  ? i915_gem_execbuffer+0x340/0x340 [i915]
> >>>>>[   29.636425]  ? __fget+0x5/0x200
> >>>>>[   29.636429]  do_vfs_ioctl+0x91/0x6f0
> >>>>>[   29.636431]  ? __fget+0x111/0x200
> >>>>>[   29.636433]  ? __fget+0x5/0x200
> >>>>>[   29.636436]  SyS_ioctl+0x79/0x90
> >>>>>[   29.636441]  entry_SYSCALL_64_fastpath+0x23/0xc6
> >>>>>
> >>>>>On suspend/resume I see the same call trace.
> >>>>>[2] points to the "BUG" line.
> >>>>Well, this appears to be an i915 issue, but not a serious one.
> >>>>
> >>>>Clearly, a function that may sleep (pm_runtime_get_sync() in
> >>>>intel_runtime_pm_get()) is called with disabled interrupts.  If I
> >>>>understand the code correctly, though, it actually is not going to
> >>>>sleep in this particular case, because pm_runtime_get_sync() has
> >>>>already been called once for this device in the same code path which
> >>>>means that this particular instance will return immediately, so this
> >>>>is a false-positive (most likely).
> >Not sure what's the root cause, but thought to clarify the above:
> >
> >Yes, i915_gem_do_execbuffer() does take an RPM reference to optimize
> >things, so the RPM get in aliasing_gtt_bind_vma() won't need to resume
> >the device on this path. This isn't a guarantee though,
> >aliasing_gtt_bind_vma() could be called from other places without an RPM
> >reference. So preemption being disabled at that point is not
> >intentional. I also can't see where on the above path it would get
> >disabled due to a bug or otherwise.
> 
> The i915 code is correct AFAICS, but the core complains about a nested
> invocation of pm_runtime_get_sync() with disabled interrupts (which looks OK
> though), so the complaint is a false positive.

Well, my point was that interrupts (or preemption) doesn't seem to get
disabled anywhere on that path.

--Imre

> 
> As I said, the way to go here appears to be to tweak the core, which I'm
> going to do.
> 
> Thanks,
> Rafael
> 

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

#1570660 — Re: [Intel-gfx] [Linux v4.10.0-rc1+] Still call-traces after suspend-resume (pm? i915? cpu/hotplug?)

On 1/31/2017 1:02 PM, Imre Deak wrote:
> On Tue, Jan 31, 2017 at 12:39:35PM +0100, Rafael J. Wysocki wrote:
>> On 1/31/2017 11:58 AM, Imre Deak wrote:
>>> Hi Rafael,
>> Hi,
>>
>>> On Mon, Jan 30, 2017 at 11:44:37PM +0100, Rafael J. Wysocki wrote:
>>>> On 1/24/2017 2:33 AM, Sedat Dilek wrote:
>>>>> On Fri, Dec 30, 2016 at 3:02 PM, Rafael J. Wysocki <rafael@kernel.org> wrote:
>>>>>> On Fri, Dec 30, 2016 at 12:40 PM, Sedat Dilek <sedat.dilek@gmail.com> wrote:
>>>>>>> Hi,
>>>>>>>
>>>>>>> I have already reported this issue in [1].
>>>>>>> One of the issue was solved.
>>>>>>> Unfortunately, it looks like there is still a different problem here
>>>>>>> (Ubuntu/precise AMD64).
>>>>>>>
>>>>>>> I tried v4.10-rc1 and latest Linus tree up to...
>>>>>>>
>>>>>>> commit 98473f9f3f9bd404873cd1178c8be7d6d619f0d1
>>>>>>> "mm/filemap: fix parameters to test_bit()"
>>>>>>>
>>>>>>> Here we go...
>>>>>>>
>>>>>>> [   29.636047] BUG: sleeping function called from invalid context at
>>>>>>> drivers/base/power/runtime.c:1032
>>>>>>> [   29.636055] in_atomic(): 1, irqs_disabled(): 0, pid: 1500, name: Xorg
>>>>>>> [   29.636058] 1 lock held by Xorg/1500:
>>>>>>> [   29.636060]  #0:  (&dev->struct_mutex){+.+.+.}, at:
>>>>>>> [<ffffffffa0680c13>] i915_mutex_lock_interruptible+0x43/0x140 [i915]
>>>>>>> [   29.636107] CPU: 0 PID: 1500 Comm: Xorg Not tainted
>>>>>>> 4.10.0-rc1-6-iniza-amd64 #1
>>>>>>> [   29.636109] Hardware name: SAMSUNG ELECTRONICS CO., LTD.
>>>>>>> 530U3BI/530U4BI/530U4BH/530U3BI/530U4BI/530U4BH, BIOS 13XK 03/28/2013
>>>>>>> [   29.636111] Call Trace:
>>>>>>> [   29.636120]  dump_stack+0x85/0xc2
>>>>>>> [   29.636124]  ___might_sleep+0x196/0x260
>>>>>>> [   29.636127]  __might_sleep+0x53/0xb0
>>>>>>> [   29.636131]  __pm_runtime_resume+0x7a/0x90
>>>>>>> [   29.636159]  intel_runtime_pm_get+0x25/0x90 [i915]
>>>>>>> [   29.636189]  aliasing_gtt_bind_vma+0xaa/0xf0 [i915]
>>>>>>> [   29.636220]  i915_vma_bind+0xaf/0x1e0 [i915]
>>>>>>> [   29.636248]  i915_gem_execbuffer_relocate_entry+0x513/0x6f0 [i915]
>>>>>>> [   29.636272]  i915_gem_execbuffer_relocate_vma.isra.34+0x188/0x250 [i915]
>>>>>>> [   29.636275]  ? trace_hardirqs_on+0xd/0x10
>>>>>>> [   29.636294]  ? i915_gem_execbuffer_reserve_vma.isra.31+0x152/0x1f0 [i915]
>>>>>>> [   29.636316]  ? i915_gem_execbuffer_reserve.isra.32+0x372/0x3a0 [i915]
>>>>>>> [   29.636342]  i915_gem_do_execbuffer.isra.38+0xa70/0x1a40 [i915]
>>>>>>> [   29.636347]  ? __might_fault+0x4e/0xb0
>>>>>>> [   29.636373]  i915_gem_execbuffer2+0xc5/0x260 [i915]
>>>>>>> [   29.636376]  ? __might_fault+0x4e/0xb0
>>>>>>> [   29.636395]  drm_ioctl+0x206/0x450 [drm]
>>>>>>> [   29.636420]  ? i915_gem_execbuffer+0x340/0x340 [i915]
>>>>>>> [   29.636425]  ? __fget+0x5/0x200
>>>>>>> [   29.636429]  do_vfs_ioctl+0x91/0x6f0
>>>>>>> [   29.636431]  ? __fget+0x111/0x200
>>>>>>> [   29.636433]  ? __fget+0x5/0x200
>>>>>>> [   29.636436]  SyS_ioctl+0x79/0x90
>>>>>>> [   29.636441]  entry_SYSCALL_64_fastpath+0x23/0xc6
>>>>>>>
>>>>>>> On suspend/resume I see the same call trace.
>>>>>>> [2] points to the "BUG" line.
>>>>>> Well, this appears to be an i915 issue, but not a serious one.
>>>>>>
>>>>>> Clearly, a function that may sleep (pm_runtime_get_sync() in
>>>>>> intel_runtime_pm_get()) is called with disabled interrupts.  If I
>>>>>> understand the code correctly, though, it actually is not going to
>>>>>> sleep in this particular case, because pm_runtime_get_sync() has
>>>>>> already been called once for this device in the same code path which
>>>>>> means that this particular instance will return immediately, so this
>>>>>> is a false-positive (most likely).
>>> Not sure what's the root cause, but thought to clarify the above:
>>>
>>> Yes, i915_gem_do_execbuffer() does take an RPM reference to optimize
>>> things, so the RPM get in aliasing_gtt_bind_vma() won't need to resume
>>> the device on this path. This isn't a guarantee though,
>>> aliasing_gtt_bind_vma() could be called from other places without an RPM
>>> reference. So preemption being disabled at that point is not
>>> intentional. I also can't see where on the above path it would get
>>> disabled due to a bug or otherwise.
>> The i915 code is correct AFAICS, but the core complains about a nested
>> invocation of pm_runtime_get_sync() with disabled interrupts (which looks OK
>> though), so the complaint is a false positive.
> Well, my point was that interrupts (or preemption) doesn't seem to get
> disabled anywhere on that path.

But the might_sleep_if() assertion in __pm_runtime_resume() triggers for 
some reason.  I wonder why then?

Of course, I may be overlooking something in the i915 code.

In any case, if you do

pm_runtime_get_sync(dev)
local_irq_disable()
pm_runtime_get_sync(dev)
<do something>
pm_runtime_put(dev)
local_irq_enable()
pm_runtime_put(dev)

that is technically correct, but it will cause the core to complain.

Thanks,
Rafael

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

#1570727 — Re: [Intel-gfx] [Linux v4.10.0-rc1+] Still call-traces after suspend-resume (pm? i915? cpu/hotplug?)

On Tue, Jan 31, 2017 at 01:12:05PM +0100, Rafael J. Wysocki wrote:
> On 1/31/2017 1:02 PM, Imre Deak wrote:
> >On Tue, Jan 31, 2017 at 12:39:35PM +0100, Rafael J. Wysocki wrote:
> >>On 1/31/2017 11:58 AM, Imre Deak wrote:
> >>>Hi Rafael,
> >>Hi,
> >>
> >>>On Mon, Jan 30, 2017 at 11:44:37PM +0100, Rafael J. Wysocki wrote:
> >>>>On 1/24/2017 2:33 AM, Sedat Dilek wrote:
> >>>>>On Fri, Dec 30, 2016 at 3:02 PM, Rafael J. Wysocki <rafael@kernel.org> wrote:
> >>>>>>On Fri, Dec 30, 2016 at 12:40 PM, Sedat Dilek <sedat.dilek@gmail.com> wrote:
> >>>>>>>Hi,
> >>>>>>>
> >>>>>>>I have already reported this issue in [1].
> >>>>>>>One of the issue was solved.
> >>>>>>>Unfortunately, it looks like there is still a different problem here
> >>>>>>>(Ubuntu/precise AMD64).
> >>>>>>>
> >>>>>>>I tried v4.10-rc1 and latest Linus tree up to...
> >>>>>>>
> >>>>>>>commit 98473f9f3f9bd404873cd1178c8be7d6d619f0d1
> >>>>>>>"mm/filemap: fix parameters to test_bit()"
> >>>>>>>
> >>>>>>>Here we go...
> >>>>>>>
> >>>>>>>[   29.636047] BUG: sleeping function called from invalid context at
> >>>>>>>drivers/base/power/runtime.c:1032
> >>>>>>>[   29.636055] in_atomic(): 1, irqs_disabled(): 0, pid: 1500, name: Xorg
> >>>>>>>[   29.636058] 1 lock held by Xorg/1500:
> >>>>>>>[   29.636060]  #0:  (&dev->struct_mutex){+.+.+.}, at:
> >>>>>>>[<ffffffffa0680c13>] i915_mutex_lock_interruptible+0x43/0x140 [i915]
> >>>>>>>[   29.636107] CPU: 0 PID: 1500 Comm: Xorg Not tainted
> >>>>>>>4.10.0-rc1-6-iniza-amd64 #1
> >>>>>>>[   29.636109] Hardware name: SAMSUNG ELECTRONICS CO., LTD.
> >>>>>>>530U3BI/530U4BI/530U4BH/530U3BI/530U4BI/530U4BH, BIOS 13XK 03/28/2013
> >>>>>>>[   29.636111] Call Trace:
> >>>>>>>[   29.636120]  dump_stack+0x85/0xc2
> >>>>>>>[   29.636124]  ___might_sleep+0x196/0x260
> >>>>>>>[   29.636127]  __might_sleep+0x53/0xb0
> >>>>>>>[   29.636131]  __pm_runtime_resume+0x7a/0x90
> >>>>>>>[   29.636159]  intel_runtime_pm_get+0x25/0x90 [i915]
> >>>>>>>[   29.636189]  aliasing_gtt_bind_vma+0xaa/0xf0 [i915]
> >>>>>>>[   29.636220]  i915_vma_bind+0xaf/0x1e0 [i915]
> >>>>>>>[   29.636248]  i915_gem_execbuffer_relocate_entry+0x513/0x6f0 [i915]
> >>>>>>>[   29.636272]  i915_gem_execbuffer_relocate_vma.isra.34+0x188/0x250 [i915]
> >>>>>>>[   29.636275]  ? trace_hardirqs_on+0xd/0x10
> >>>>>>>[   29.636294]  ? i915_gem_execbuffer_reserve_vma.isra.31+0x152/0x1f0 [i915]
> >>>>>>>[   29.636316]  ? i915_gem_execbuffer_reserve.isra.32+0x372/0x3a0 [i915]
> >>>>>>>[   29.636342]  i915_gem_do_execbuffer.isra.38+0xa70/0x1a40 [i915]
> >>>>>>>[   29.636347]  ? __might_fault+0x4e/0xb0
> >>>>>>>[   29.636373]  i915_gem_execbuffer2+0xc5/0x260 [i915]
> >>>>>>>[   29.636376]  ? __might_fault+0x4e/0xb0
> >>>>>>>[   29.636395]  drm_ioctl+0x206/0x450 [drm]
> >>>>>>>[   29.636420]  ? i915_gem_execbuffer+0x340/0x340 [i915]
> >>>>>>>[   29.636425]  ? __fget+0x5/0x200
> >>>>>>>[   29.636429]  do_vfs_ioctl+0x91/0x6f0
> >>>>>>>[   29.636431]  ? __fget+0x111/0x200
> >>>>>>>[   29.636433]  ? __fget+0x5/0x200
> >>>>>>>[   29.636436]  SyS_ioctl+0x79/0x90
> >>>>>>>[   29.636441]  entry_SYSCALL_64_fastpath+0x23/0xc6
> >>>>>>>
> >>>>>>>On suspend/resume I see the same call trace.
> >>>>>>>[2] points to the "BUG" line.
> >>>>>>Well, this appears to be an i915 issue, but not a serious one.
> >>>>>>
> >>>>>>Clearly, a function that may sleep (pm_runtime_get_sync() in
> >>>>>>intel_runtime_pm_get()) is called with disabled interrupts.  If I
> >>>>>>understand the code correctly, though, it actually is not going to
> >>>>>>sleep in this particular case, because pm_runtime_get_sync() has
> >>>>>>already been called once for this device in the same code path which
> >>>>>>means that this particular instance will return immediately, so this
> >>>>>>is a false-positive (most likely).
> >>>Not sure what's the root cause, but thought to clarify the above:
> >>>
> >>>Yes, i915_gem_do_execbuffer() does take an RPM reference to optimize
> >>>things, so the RPM get in aliasing_gtt_bind_vma() won't need to resume
> >>>the device on this path. This isn't a guarantee though,
> >>>aliasing_gtt_bind_vma() could be called from other places without an RPM
> >>>reference. So preemption being disabled at that point is not
> >>>intentional. I also can't see where on the above path it would get
> >>>disabled due to a bug or otherwise.
> >>The i915 code is correct AFAICS, but the core complains about a nested
> >>invocation of pm_runtime_get_sync() with disabled interrupts (which looks OK
> >>though), so the complaint is a false positive.
> >Well, my point was that interrupts (or preemption) doesn't seem to get
> >disabled anywhere on that path.
> 
> But the might_sleep_if() assertion in __pm_runtime_resume() triggers for
> some reason.  I wonder why then?
> 
> Of course, I may be overlooking something in the i915 code.
> 
> In any case, if you do
> 
> pm_runtime_get_sync(dev)
> local_irq_disable()
> pm_runtime_get_sync(dev)
> <do something>
> pm_runtime_put(dev)
> local_irq_enable()
> pm_runtime_put(dev)
> 
> that is technically correct, but it will cause the core to complain.

Ah, found it it's the i915 reloc_cache keeping some atomic mappings
around. So moving the warn as you said looks ok, sorry for the noise.

--Imre

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

#1571218

[Multipart message — attachments visible in raw view] — view raw

On Mon, Jan 30, 2017 at 11:44 PM, Rafael J. Wysocki
<rafael.j.wysocki@intel.com> wrote:
> On 1/24/2017 2:33 AM, Sedat Dilek wrote:
>>
>> On Fri, Dec 30, 2016 at 3:02 PM, Rafael J. Wysocki <rafael@kernel.org>
>> wrote:
>>>
>>> On Fri, Dec 30, 2016 at 12:40 PM, Sedat Dilek <sedat.dilek@gmail.com>
>>> wrote:
>>>>
>>>> Hi,
>>>>
>>>> I have already reported this issue in [1].
>>>> One of the issue was solved.
>>>> Unfortunately, it looks like there is still a different problem here
>>>> (Ubuntu/precise AMD64).
>>>>
>>>> I tried v4.10-rc1 and latest Linus tree up to...
>>>>
>>>> commit 98473f9f3f9bd404873cd1178c8be7d6d619f0d1
>>>> "mm/filemap: fix parameters to test_bit()"
>>>>
>>>> Here we go...
>>>>
>>>> [   29.636047] BUG: sleeping function called from invalid context at
>>>> drivers/base/power/runtime.c:1032
>>>> [   29.636055] in_atomic(): 1, irqs_disabled(): 0, pid: 1500, name: Xorg
>>>> [   29.636058] 1 lock held by Xorg/1500:
>>>> [   29.636060]  #0:  (&dev->struct_mutex){+.+.+.}, at:
>>>> [<ffffffffa0680c13>] i915_mutex_lock_interruptible+0x43/0x140 [i915]
>>>> [   29.636107] CPU: 0 PID: 1500 Comm: Xorg Not tainted
>>>> 4.10.0-rc1-6-iniza-amd64 #1
>>>> [   29.636109] Hardware name: SAMSUNG ELECTRONICS CO., LTD.
>>>> 530U3BI/530U4BI/530U4BH/530U3BI/530U4BI/530U4BH, BIOS 13XK 03/28/2013
>>>> [   29.636111] Call Trace:
>>>> [   29.636120]  dump_stack+0x85/0xc2
>>>> [   29.636124]  ___might_sleep+0x196/0x260
>>>> [   29.636127]  __might_sleep+0x53/0xb0
>>>> [   29.636131]  __pm_runtime_resume+0x7a/0x90
>>>> [   29.636159]  intel_runtime_pm_get+0x25/0x90 [i915]
>>>> [   29.636189]  aliasing_gtt_bind_vma+0xaa/0xf0 [i915]
>>>> [   29.636220]  i915_vma_bind+0xaf/0x1e0 [i915]
>>>> [   29.636248]  i915_gem_execbuffer_relocate_entry+0x513/0x6f0 [i915]
>>>> [   29.636272]  i915_gem_execbuffer_relocate_vma.isra.34+0x188/0x250
>>>> [i915]
>>>> [   29.636275]  ? trace_hardirqs_on+0xd/0x10
>>>> [   29.636294]  ? i915_gem_execbuffer_reserve_vma.isra.31+0x152/0x1f0
>>>> [i915]
>>>> [   29.636316]  ? i915_gem_execbuffer_reserve.isra.32+0x372/0x3a0 [i915]
>>>> [   29.636342]  i915_gem_do_execbuffer.isra.38+0xa70/0x1a40 [i915]
>>>> [   29.636347]  ? __might_fault+0x4e/0xb0
>>>> [   29.636373]  i915_gem_execbuffer2+0xc5/0x260 [i915]
>>>> [   29.636376]  ? __might_fault+0x4e/0xb0
>>>> [   29.636395]  drm_ioctl+0x206/0x450 [drm]
>>>> [   29.636420]  ? i915_gem_execbuffer+0x340/0x340 [i915]
>>>> [   29.636425]  ? __fget+0x5/0x200
>>>> [   29.636429]  do_vfs_ioctl+0x91/0x6f0
>>>> [   29.636431]  ? __fget+0x111/0x200
>>>> [   29.636433]  ? __fget+0x5/0x200
>>>> [   29.636436]  SyS_ioctl+0x79/0x90
>>>> [   29.636441]  entry_SYSCALL_64_fastpath+0x23/0xc6
>>>>
>>>> On suspend/resume I see the same call trace.
>>>> [2] points to the "BUG" line.
>>>
>>> Well, this appears to be an i915 issue, but not a serious one.
>>>
>>> Clearly, a function that may sleep (pm_runtime_get_sync() in
>>> intel_runtime_pm_get()) is called with disabled interrupts.  If I
>>> understand the code correctly, though, it actually is not going to
>>> sleep in this particular case, because pm_runtime_get_sync() has
>>> already been called once for this device in the same code path which
>>> means that this particular instance will return immediately, so this
>>> is a false-positive (most likely).
>>>
>>> Let me see if I the might_sleep_if() assertion in
>>> __pm_runtime_resume(() can be moved to a better place.
>>>
>> Hi Rafael,
>>
>> did you had a chance to look at this?
>> The problem still remains in Linux v4.10-rc5.
>
>
> No, I didn't.
>
> As I said, this is not a serious issue.

Something like the attached (untested).

Please try it and let me know if it makes the splat go away.

Thanks,
Rafael

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

#1573546 — [PATCH] PM / runtime: Avoid false-positive warnings from might_sleep_if()

On Thursday, February 02, 2017 02:34:42 PM Sedat Dilek wrote:
> On Wed, Feb 1, 2017 at 1:22 AM, Rafael J. Wysocki <rafael@kernel.org> wrote:
> > On Mon, Jan 30, 2017 at 11:44 PM, Rafael J. Wysocki
> > <rafael.j.wysocki@intel.com> wrote:
> >> On 1/24/2017 2:33 AM, Sedat Dilek wrote:
> >>>
> >>> On Fri, Dec 30, 2016 at 3:02 PM, Rafael J. Wysocki <rafael@kernel.org>
> >>> wrote:
> >>>>
> >>>> On Fri, Dec 30, 2016 at 12:40 PM, Sedat Dilek <sedat.dilek@gmail.com>
> >>>> wrote:
> >>>>>
> >>>>> Hi,
> >>>>>
> >>>>> I have already reported this issue in [1].
> >>>>> One of the issue was solved.
> >>>>> Unfortunately, it looks like there is still a different problem here
> >>>>> (Ubuntu/precise AMD64).
> >>>>>
> >>>>> I tried v4.10-rc1 and latest Linus tree up to...
> >>>>>
> >>>>> commit 98473f9f3f9bd404873cd1178c8be7d6d619f0d1
> >>>>> "mm/filemap: fix parameters to test_bit()"
> >>>>>
> >>>>> Here we go...
> >>>>>
> >>>>> [   29.636047] BUG: sleeping function called from invalid context at
> >>>>> drivers/base/power/runtime.c:1032
> >>>>> [   29.636055] in_atomic(): 1, irqs_disabled(): 0, pid: 1500, name: Xorg
> >>>>> [   29.636058] 1 lock held by Xorg/1500:
> >>>>> [   29.636060]  #0:  (&dev->struct_mutex){+.+.+.}, at:
> >>>>> [<ffffffffa0680c13>] i915_mutex_lock_interruptible+0x43/0x140 [i915]
> >>>>> [   29.636107] CPU: 0 PID: 1500 Comm: Xorg Not tainted
> >>>>> 4.10.0-rc1-6-iniza-amd64 #1
> >>>>> [   29.636109] Hardware name: SAMSUNG ELECTRONICS CO., LTD.
> >>>>> 530U3BI/530U4BI/530U4BH/530U3BI/530U4BI/530U4BH, BIOS 13XK 03/28/2013
> >>>>> [   29.636111] Call Trace:
> >>>>> [   29.636120]  dump_stack+0x85/0xc2
> >>>>> [   29.636124]  ___might_sleep+0x196/0x260
> >>>>> [   29.636127]  __might_sleep+0x53/0xb0
> >>>>> [   29.636131]  __pm_runtime_resume+0x7a/0x90
> >>>>> [   29.636159]  intel_runtime_pm_get+0x25/0x90 [i915]
> >>>>> [   29.636189]  aliasing_gtt_bind_vma+0xaa/0xf0 [i915]
> >>>>> [   29.636220]  i915_vma_bind+0xaf/0x1e0 [i915]
> >>>>> [   29.636248]  i915_gem_execbuffer_relocate_entry+0x513/0x6f0 [i915]
> >>>>> [   29.636272]  i915_gem_execbuffer_relocate_vma.isra.34+0x188/0x250
> >>>>> [i915]
> >>>>> [   29.636275]  ? trace_hardirqs_on+0xd/0x10
> >>>>> [   29.636294]  ? i915_gem_execbuffer_reserve_vma.isra.31+0x152/0x1f0
> >>>>> [i915]
> >>>>> [   29.636316]  ? i915_gem_execbuffer_reserve.isra.32+0x372/0x3a0 [i915]
> >>>>> [   29.636342]  i915_gem_do_execbuffer.isra.38+0xa70/0x1a40 [i915]
> >>>>> [   29.636347]  ? __might_fault+0x4e/0xb0
> >>>>> [   29.636373]  i915_gem_execbuffer2+0xc5/0x260 [i915]
> >>>>> [   29.636376]  ? __might_fault+0x4e/0xb0
> >>>>> [   29.636395]  drm_ioctl+0x206/0x450 [drm]
> >>>>> [   29.636420]  ? i915_gem_execbuffer+0x340/0x340 [i915]
> >>>>> [   29.636425]  ? __fget+0x5/0x200
> >>>>> [   29.636429]  do_vfs_ioctl+0x91/0x6f0
> >>>>> [   29.636431]  ? __fget+0x111/0x200
> >>>>> [   29.636433]  ? __fget+0x5/0x200
> >>>>> [   29.636436]  SyS_ioctl+0x79/0x90
> >>>>> [   29.636441]  entry_SYSCALL_64_fastpath+0x23/0xc6
> >>>>>
> >>>>> On suspend/resume I see the same call trace.
> >>>>> [2] points to the "BUG" line.
> >>>>
> >>>> Well, this appears to be an i915 issue, but not a serious one.
> >>>>
> >>>> Clearly, a function that may sleep (pm_runtime_get_sync() in
> >>>> intel_runtime_pm_get()) is called with disabled interrupts.  If I
> >>>> understand the code correctly, though, it actually is not going to
> >>>> sleep in this particular case, because pm_runtime_get_sync() has
> >>>> already been called once for this device in the same code path which
> >>>> means that this particular instance will return immediately, so this
> >>>> is a false-positive (most likely).
> >>>>
> >>>> Let me see if I the might_sleep_if() assertion in
> >>>> __pm_runtime_resume(() can be moved to a better place.
> >>>>
> >>> Hi Rafael,
> >>>
> >>> did you had a chance to look at this?
> >>> The problem still remains in Linux v4.10-rc5.
> >>
> >>
> >> No, I didn't.
> >>
> >> As I said, this is not a serious issue.
> >
> > Something like the attached (untested).
> >
> > Please try it and let me know if it makes the splat go away.
> >
> 
> Your patch fixes the issue here.
> I tested against vanilla Linux v4.10-rc5.
> 
> Feel free to give appropriate credits.

OK, thanks!

Below is a full version with a changelog & tags.

Thanks,
Rafael

---
From: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
Subject: [PATCH] PM / runtime: Avoid false-positive warnings from might_sleep_if()

The might_sleep_if() assertions in __pm_runtime_idle(),
__pm_runtime_suspend() and __pm_runtime_resume() may generate
false-positive warnings in some situations.  For example, that
happens if a nested pm_runtime_get_sync()/pm_runtime_put() pair
is executed with disabled interrupts within an outer
pm_runtime_get_sync()/pm_runtime_put() section for the same device.
[Generally, pm_runtime_get_sync() may sleep, so it should not be
called with disabled interrupts, but in this particular case the
previous pm_runtime_get_sync() guarantees that the device will not
be suspended, so the inner pm_runtime_get_sync() will return
immediately after incrementing the device's usage counter.]

That started to happen in the i915 driver in 4.10-rc, leading to
the following splat:

 BUG: sleeping function called from invalid context at drivers/base/power/runtime.c:1032
 in_atomic(): 1, irqs_disabled(): 0, pid: 1500, name: Xorg
 1 lock held by Xorg/1500:
  #0:  (&dev->struct_mutex){+.+.+.}, at:
  [<ffffffffa0680c13>] i915_mutex_lock_interruptible+0x43/0x140 [i915]
 CPU: 0 PID: 1500 Comm: Xorg Not tainted
 Call Trace:
  dump_stack+0x85/0xc2
  ___might_sleep+0x196/0x260
  __might_sleep+0x53/0xb0
  __pm_runtime_resume+0x7a/0x90
  intel_runtime_pm_get+0x25/0x90 [i915]
  aliasing_gtt_bind_vma+0xaa/0xf0 [i915]
  i915_vma_bind+0xaf/0x1e0 [i915]
  i915_gem_execbuffer_relocate_entry+0x513/0x6f0 [i915]
  i915_gem_execbuffer_relocate_vma.isra.34+0x188/0x250 [i915]
  ? trace_hardirqs_on+0xd/0x10
  ? i915_gem_execbuffer_reserve_vma.isra.31+0x152/0x1f0 [i915]
  ? i915_gem_execbuffer_reserve.isra.32+0x372/0x3a0 [i915]
  i915_gem_do_execbuffer.isra.38+0xa70/0x1a40 [i915]
  ? __might_fault+0x4e/0xb0
  i915_gem_execbuffer2+0xc5/0x260 [i915]
  ? __might_fault+0x4e/0xb0
  drm_ioctl+0x206/0x450 [drm]
  ? i915_gem_execbuffer+0x340/0x340 [i915]
  ? __fget+0x5/0x200
  do_vfs_ioctl+0x91/0x6f0
  ? __fget+0x111/0x200
  ? __fget+0x5/0x200
  SyS_ioctl+0x79/0x90
  entry_SYSCALL_64_fastpath+0x23/0xc6

even though the code triggering it is correct.

Unfortunately, the might_sleep_if() assertions in question are
too coarse-grained to cover such cases correctly, so make them
a bit less sensitive in order to avoid the false-positives.

Reported-and-tested-by: Sedat Dilek <sedat.dilek@gmail.com> 
Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
---
 drivers/base/power/runtime.c |   11 ++++++-----
 1 file changed, 6 insertions(+), 5 deletions(-)

Index: linux-pm/drivers/base/power/runtime.c
===================================================================
--- linux-pm.orig/drivers/base/power/runtime.c
+++ linux-pm/drivers/base/power/runtime.c
@@ -966,13 +966,13 @@ int __pm_runtime_idle(struct device *dev
 	unsigned long flags;
 	int retval;
 
-	might_sleep_if(!(rpmflags & RPM_ASYNC) && !dev->power.irq_safe);
-
 	if (rpmflags & RPM_GET_PUT) {
 		if (!atomic_dec_and_test(&dev->power.usage_count))
 			return 0;
 	}
 
+	might_sleep_if(!(rpmflags & RPM_ASYNC) && !dev->power.irq_safe);
+
 	spin_lock_irqsave(&dev->power.lock, flags);
 	retval = rpm_idle(dev, rpmflags);
 	spin_unlock_irqrestore(&dev->power.lock, flags);
@@ -998,13 +998,13 @@ int __pm_runtime_suspend(struct device *
 	unsigned long flags;
 	int retval;
 
-	might_sleep_if(!(rpmflags & RPM_ASYNC) && !dev->power.irq_safe);
-
 	if (rpmflags & RPM_GET_PUT) {
 		if (!atomic_dec_and_test(&dev->power.usage_count))
 			return 0;
 	}
 
+	might_sleep_if(!(rpmflags & RPM_ASYNC) && !dev->power.irq_safe);
+
 	spin_lock_irqsave(&dev->power.lock, flags);
 	retval = rpm_suspend(dev, rpmflags);
 	spin_unlock_irqrestore(&dev->power.lock, flags);
@@ -1029,7 +1029,8 @@ int __pm_runtime_resume(struct device *d
 	unsigned long flags;
 	int retval;
 
-	might_sleep_if(!(rpmflags & RPM_ASYNC) && !dev->power.irq_safe);
+	might_sleep_if(!(rpmflags & RPM_ASYNC) && !dev->power.irq_safe &&
+			dev->power.runtime_status != RPM_ACTIVE);
 
 	if (rpmflags & RPM_GET_PUT)
 		atomic_inc(&dev->power.usage_count);

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

#1573851 — Re: [PATCH] PM / runtime: Avoid false-positive warnings from might_sleep_if()

On Sat, Feb 4, 2017 at 12:58 AM, Rafael J. Wysocki <rjw@rjwysocki.net> wrote:
> On Thursday, February 02, 2017 02:34:42 PM Sedat Dilek wrote:
>> On Wed, Feb 1, 2017 at 1:22 AM, Rafael J. Wysocki <rafael@kernel.org> wrote:
>> > On Mon, Jan 30, 2017 at 11:44 PM, Rafael J. Wysocki
>> > <rafael.j.wysocki@intel.com> wrote:
>> >> On 1/24/2017 2:33 AM, Sedat Dilek wrote:
>> >>>
>> >>> On Fri, Dec 30, 2016 at 3:02 PM, Rafael J. Wysocki <rafael@kernel.org>
>> >>> wrote:
>> >>>>
>> >>>> On Fri, Dec 30, 2016 at 12:40 PM, Sedat Dilek <sedat.dilek@gmail.com>
>> >>>> wrote:
>> >>>>>
>> >>>>> Hi,
>> >>>>>
>> >>>>> I have already reported this issue in [1].
>> >>>>> One of the issue was solved.
>> >>>>> Unfortunately, it looks like there is still a different problem here
>> >>>>> (Ubuntu/precise AMD64).
>> >>>>>
>> >>>>> I tried v4.10-rc1 and latest Linus tree up to...
>> >>>>>
>> >>>>> commit 98473f9f3f9bd404873cd1178c8be7d6d619f0d1
>> >>>>> "mm/filemap: fix parameters to test_bit()"
>> >>>>>
>> >>>>> Here we go...
>> >>>>>
>> >>>>> [   29.636047] BUG: sleeping function called from invalid context at
>> >>>>> drivers/base/power/runtime.c:1032
>> >>>>> [   29.636055] in_atomic(): 1, irqs_disabled(): 0, pid: 1500, name: Xorg
>> >>>>> [   29.636058] 1 lock held by Xorg/1500:
>> >>>>> [   29.636060]  #0:  (&dev->struct_mutex){+.+.+.}, at:
>> >>>>> [<ffffffffa0680c13>] i915_mutex_lock_interruptible+0x43/0x140 [i915]
>> >>>>> [   29.636107] CPU: 0 PID: 1500 Comm: Xorg Not tainted
>> >>>>> 4.10.0-rc1-6-iniza-amd64 #1
>> >>>>> [   29.636109] Hardware name: SAMSUNG ELECTRONICS CO., LTD.
>> >>>>> 530U3BI/530U4BI/530U4BH/530U3BI/530U4BI/530U4BH, BIOS 13XK 03/28/2013
>> >>>>> [   29.636111] Call Trace:
>> >>>>> [   29.636120]  dump_stack+0x85/0xc2
>> >>>>> [   29.636124]  ___might_sleep+0x196/0x260
>> >>>>> [   29.636127]  __might_sleep+0x53/0xb0
>> >>>>> [   29.636131]  __pm_runtime_resume+0x7a/0x90
>> >>>>> [   29.636159]  intel_runtime_pm_get+0x25/0x90 [i915]
>> >>>>> [   29.636189]  aliasing_gtt_bind_vma+0xaa/0xf0 [i915]
>> >>>>> [   29.636220]  i915_vma_bind+0xaf/0x1e0 [i915]
>> >>>>> [   29.636248]  i915_gem_execbuffer_relocate_entry+0x513/0x6f0 [i915]
>> >>>>> [   29.636272]  i915_gem_execbuffer_relocate_vma.isra.34+0x188/0x250
>> >>>>> [i915]
>> >>>>> [   29.636275]  ? trace_hardirqs_on+0xd/0x10
>> >>>>> [   29.636294]  ? i915_gem_execbuffer_reserve_vma.isra.31+0x152/0x1f0
>> >>>>> [i915]
>> >>>>> [   29.636316]  ? i915_gem_execbuffer_reserve.isra.32+0x372/0x3a0 [i915]
>> >>>>> [   29.636342]  i915_gem_do_execbuffer.isra.38+0xa70/0x1a40 [i915]
>> >>>>> [   29.636347]  ? __might_fault+0x4e/0xb0
>> >>>>> [   29.636373]  i915_gem_execbuffer2+0xc5/0x260 [i915]
>> >>>>> [   29.636376]  ? __might_fault+0x4e/0xb0
>> >>>>> [   29.636395]  drm_ioctl+0x206/0x450 [drm]
>> >>>>> [   29.636420]  ? i915_gem_execbuffer+0x340/0x340 [i915]
>> >>>>> [   29.636425]  ? __fget+0x5/0x200
>> >>>>> [   29.636429]  do_vfs_ioctl+0x91/0x6f0
>> >>>>> [   29.636431]  ? __fget+0x111/0x200
>> >>>>> [   29.636433]  ? __fget+0x5/0x200
>> >>>>> [   29.636436]  SyS_ioctl+0x79/0x90
>> >>>>> [   29.636441]  entry_SYSCALL_64_fastpath+0x23/0xc6
>> >>>>>
>> >>>>> On suspend/resume I see the same call trace.
>> >>>>> [2] points to the "BUG" line.
>> >>>>
>> >>>> Well, this appears to be an i915 issue, but not a serious one.
>> >>>>
>> >>>> Clearly, a function that may sleep (pm_runtime_get_sync() in
>> >>>> intel_runtime_pm_get()) is called with disabled interrupts.  If I
>> >>>> understand the code correctly, though, it actually is not going to
>> >>>> sleep in this particular case, because pm_runtime_get_sync() has
>> >>>> already been called once for this device in the same code path which
>> >>>> means that this particular instance will return immediately, so this
>> >>>> is a false-positive (most likely).
>> >>>>
>> >>>> Let me see if I the might_sleep_if() assertion in
>> >>>> __pm_runtime_resume(() can be moved to a better place.
>> >>>>
>> >>> Hi Rafael,
>> >>>
>> >>> did you had a chance to look at this?
>> >>> The problem still remains in Linux v4.10-rc5.
>> >>
>> >>
>> >> No, I didn't.
>> >>
>> >> As I said, this is not a serious issue.
>> >
>> > Something like the attached (untested).
>> >
>> > Please try it and let me know if it makes the splat go away.
>> >
>>
>> Your patch fixes the issue here.
>> I tested against vanilla Linux v4.10-rc5.
>>
>> Feel free to give appropriate credits.
>
> OK, thanks!
>
> Below is a full version with a changelog & tags.
>
> Thanks,
> Rafael
>
> ---
> From: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
> Subject: [PATCH] PM / runtime: Avoid false-positive warnings from might_sleep_if()
>
> The might_sleep_if() assertions in __pm_runtime_idle(),
> __pm_runtime_suspend() and __pm_runtime_resume() may generate
> false-positive warnings in some situations.  For example, that
> happens if a nested pm_runtime_get_sync()/pm_runtime_put() pair
> is executed with disabled interrupts within an outer
> pm_runtime_get_sync()/pm_runtime_put() section for the same device.
> [Generally, pm_runtime_get_sync() may sleep, so it should not be
> called with disabled interrupts, but in this particular case the
> previous pm_runtime_get_sync() guarantees that the device will not
> be suspended, so the inner pm_runtime_get_sync() will return
> immediately after incrementing the device's usage counter.]
>
> That started to happen in the i915 driver in 4.10-rc, leading to
> the following splat:
>
>  BUG: sleeping function called from invalid context at drivers/base/power/runtime.c:1032
>  in_atomic(): 1, irqs_disabled(): 0, pid: 1500, name: Xorg
>  1 lock held by Xorg/1500:
>   #0:  (&dev->struct_mutex){+.+.+.}, at:
>   [<ffffffffa0680c13>] i915_mutex_lock_interruptible+0x43/0x140 [i915]
>  CPU: 0 PID: 1500 Comm: Xorg Not tainted
>  Call Trace:
>   dump_stack+0x85/0xc2
>   ___might_sleep+0x196/0x260
>   __might_sleep+0x53/0xb0
>   __pm_runtime_resume+0x7a/0x90
>   intel_runtime_pm_get+0x25/0x90 [i915]
>   aliasing_gtt_bind_vma+0xaa/0xf0 [i915]
>   i915_vma_bind+0xaf/0x1e0 [i915]
>   i915_gem_execbuffer_relocate_entry+0x513/0x6f0 [i915]
>   i915_gem_execbuffer_relocate_vma.isra.34+0x188/0x250 [i915]
>   ? trace_hardirqs_on+0xd/0x10
>   ? i915_gem_execbuffer_reserve_vma.isra.31+0x152/0x1f0 [i915]
>   ? i915_gem_execbuffer_reserve.isra.32+0x372/0x3a0 [i915]
>   i915_gem_do_execbuffer.isra.38+0xa70/0x1a40 [i915]
>   ? __might_fault+0x4e/0xb0
>   i915_gem_execbuffer2+0xc5/0x260 [i915]
>   ? __might_fault+0x4e/0xb0
>   drm_ioctl+0x206/0x450 [drm]
>   ? i915_gem_execbuffer+0x340/0x340 [i915]
>   ? __fget+0x5/0x200
>   do_vfs_ioctl+0x91/0x6f0
>   ? __fget+0x111/0x200
>   ? __fget+0x5/0x200
>   SyS_ioctl+0x79/0x90
>   entry_SYSCALL_64_fastpath+0x23/0xc6
>
> even though the code triggering it is correct.
>
> Unfortunately, the might_sleep_if() assertions in question are
> too coarse-grained to cover such cases correctly, so make them
> a bit less sensitive in order to avoid the false-positives.
>
> Reported-and-tested-by: Sedat Dilek <sedat.dilek@gmail.com>
> Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
> ---
>  drivers/base/power/runtime.c |   11 ++++++-----
>  1 file changed, 6 insertions(+), 5 deletions(-)
>
> Index: linux-pm/drivers/base/power/runtime.c
> ===================================================================
> --- linux-pm.orig/drivers/base/power/runtime.c
> +++ linux-pm/drivers/base/power/runtime.c
> @@ -966,13 +966,13 @@ int __pm_runtime_idle(struct device *dev
>         unsigned long flags;
>         int retval;
>
> -       might_sleep_if(!(rpmflags & RPM_ASYNC) && !dev->power.irq_safe);
> -
>         if (rpmflags & RPM_GET_PUT) {
>                 if (!atomic_dec_and_test(&dev->power.usage_count))
>                         return 0;
>         }
>
> +       might_sleep_if(!(rpmflags & RPM_ASYNC) && !dev->power.irq_safe);
> +
>         spin_lock_irqsave(&dev->power.lock, flags);
>         retval = rpm_idle(dev, rpmflags);
>         spin_unlock_irqrestore(&dev->power.lock, flags);
> @@ -998,13 +998,13 @@ int __pm_runtime_suspend(struct device *
>         unsigned long flags;
>         int retval;
>
> -       might_sleep_if(!(rpmflags & RPM_ASYNC) && !dev->power.irq_safe);
> -
>         if (rpmflags & RPM_GET_PUT) {
>                 if (!atomic_dec_and_test(&dev->power.usage_count))
>                         return 0;
>         }
>
> +       might_sleep_if(!(rpmflags & RPM_ASYNC) && !dev->power.irq_safe);
> +
>         spin_lock_irqsave(&dev->power.lock, flags);
>         retval = rpm_suspend(dev, rpmflags);
>         spin_unlock_irqrestore(&dev->power.lock, flags);
> @@ -1029,7 +1029,8 @@ int __pm_runtime_resume(struct device *d
>         unsigned long flags;
>         int retval;
>
> -       might_sleep_if(!(rpmflags & RPM_ASYNC) && !dev->power.irq_safe);
> +       might_sleep_if(!(rpmflags & RPM_ASYNC) && !dev->power.irq_safe &&
> +                       dev->power.runtime_status != RPM_ACTIVE);
>
>         if (rpmflags & RPM_GET_PUT)
>                 atomic_inc(&dev->power.usage_count);
>

Thanks, Rafael.

Just for the records: The patch is available in [1].

- Sedat -

[1] http://git.kernel.org/cgit/linux/kernel/git/rafael/linux-pm.git/patch/?id=a9306a63631493afc75893a4ac405d4e1cbae6aa

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web