From: Nathan Fontenot <nfont@linux.vnet.ibm.com>
To: Thomas Huth <thuth@redhat.com>,
Benjamin Herrenschmidt <benh@kernel.crashing.org>
Cc: linuxppc-dev@lists.ozlabs.org, anton@samba.org, kvm-ppc@vger.kernel.org
Subject: Re: BUG: sleeping function called from ras_epow_interrupt context
Date: Thu, 16 Jul 2015 17:39:11 +0000 [thread overview]
Message-ID: <55A7EC3F.1080406@linux.vnet.ibm.com> (raw)
In-Reply-To: <55A74DF9.2010100@redhat.com>
On 07/16/2015 01:23 AM, Thomas Huth wrote:
> On 07/15/2015 09:58 PM, Nathan Fontenot wrote:
>> On 07/15/2015 09:35 AM, Thomas Huth wrote:
>>> On 07/14/2015 11:22 PM, Benjamin Herrenschmidt wrote:
>>>> On Tue, 2015-07-14 at 20:43 +0200, Thomas Huth wrote:
>>>>> Any suggestions how to fix this? Simply revert 587f83e8dd50d? Use
>>>>> mdelay() instead of msleep() in rtas_busy_delay()? Something more
>>>>> fancy?
>>>>
>>>> A proper fix would be more fancy, the get_sensor should happen in a
>>>> kernel thread instead.
>>>
>>> I'm not very familiar with this stuff, but isn't the EPOW interrupt
>>> something that is very time-critical? Moving parts of the handler into a
>>> kernel thread then does not sound like a very good idea to me...
>>>
>>> Another question: Can it happen at all that this get-sensor call results
>>> in a sleep condition? Looking at commit ID
>>> 81b73dd92b97423b8f5324a59044da478c04f4c4 ("Fix might-sleep warning on
>>> removing cpus"), which apparently fixed a similar issue for CPU
>>> hot-plugging, indicates that at least some of the rtas calls are never
>>> returning the busy code? In that case we could fix this by introducing a
>>> similar rtas_get_sensor_fast() function? (or simply revert 587f83e8dd50d
>>> which would be quite similar, I think)
>>>
>>
>> Looking at the PAPR, the get-sensor-state rtas call for the EPOW sensor
>> is listed as a fast call and should not return a busy indication.
>
> Great, good to know, thanks for looking that up! So IMHO we should
> either introduce a rtas_get_sensor_fast() function or revert
> 587f83e8dd50d ... any preferences? Shall I come up with a patch?
>
A quick look at the kernel, I only find three places that rtas_get_sensor
is called. The instance you point out here for the EPOW sensor is the
only time I find it called for a sensor that should not return a busy
indication.
Reverting commit 587f83e8dd50d would solve the issue but not fix any future
users of a fast get-sensor call. I don't have an issue with a patch for a
rtas_get_sensor_fast().
-Nathan
>> I'm curious as to why we're getting a busy return indication when
>> making this call.
>
> Looking at the code again, rtas_busy_delay() likely never slept ... it's
> likely just the "might_sleep()" annotation in that function that causes
> the BUG.
>
> Thomas
>
>
WARNING: multiple messages have this Message-ID (diff)
From: Nathan Fontenot <nfont@linux.vnet.ibm.com>
To: Thomas Huth <thuth@redhat.com>,
Benjamin Herrenschmidt <benh@kernel.crashing.org>
Cc: linuxppc-dev@lists.ozlabs.org, anton@samba.org, kvm-ppc@vger.kernel.org
Subject: Re: BUG: sleeping function called from ras_epow_interrupt context
Date: Thu, 16 Jul 2015 12:39:11 -0500 [thread overview]
Message-ID: <55A7EC3F.1080406@linux.vnet.ibm.com> (raw)
In-Reply-To: <55A74DF9.2010100@redhat.com>
On 07/16/2015 01:23 AM, Thomas Huth wrote:
> On 07/15/2015 09:58 PM, Nathan Fontenot wrote:
>> On 07/15/2015 09:35 AM, Thomas Huth wrote:
>>> On 07/14/2015 11:22 PM, Benjamin Herrenschmidt wrote:
>>>> On Tue, 2015-07-14 at 20:43 +0200, Thomas Huth wrote:
>>>>> Any suggestions how to fix this? Simply revert 587f83e8dd50d? Use
>>>>> mdelay() instead of msleep() in rtas_busy_delay()? Something more
>>>>> fancy?
>>>>
>>>> A proper fix would be more fancy, the get_sensor should happen in a
>>>> kernel thread instead.
>>>
>>> I'm not very familiar with this stuff, but isn't the EPOW interrupt
>>> something that is very time-critical? Moving parts of the handler into a
>>> kernel thread then does not sound like a very good idea to me...
>>>
>>> Another question: Can it happen at all that this get-sensor call results
>>> in a sleep condition? Looking at commit ID
>>> 81b73dd92b97423b8f5324a59044da478c04f4c4 ("Fix might-sleep warning on
>>> removing cpus"), which apparently fixed a similar issue for CPU
>>> hot-plugging, indicates that at least some of the rtas calls are never
>>> returning the busy code? In that case we could fix this by introducing a
>>> similar rtas_get_sensor_fast() function? (or simply revert 587f83e8dd50d
>>> which would be quite similar, I think)
>>>
>>
>> Looking at the PAPR, the get-sensor-state rtas call for the EPOW sensor
>> is listed as a fast call and should not return a busy indication.
>
> Great, good to know, thanks for looking that up! So IMHO we should
> either introduce a rtas_get_sensor_fast() function or revert
> 587f83e8dd50d ... any preferences? Shall I come up with a patch?
>
A quick look at the kernel, I only find three places that rtas_get_sensor
is called. The instance you point out here for the EPOW sensor is the
only time I find it called for a sensor that should not return a busy
indication.
Reverting commit 587f83e8dd50d would solve the issue but not fix any future
users of a fast get-sensor call. I don't have an issue with a patch for a
rtas_get_sensor_fast().
-Nathan
>> I'm curious as to why we're getting a busy return indication when
>> making this call.
>
> Looking at the code again, rtas_busy_delay() likely never slept ... it's
> likely just the "might_sleep()" annotation in that function that causes
> the BUG.
>
> Thomas
>
>
next prev parent reply other threads:[~2015-07-16 17:39 UTC|newest]
Thread overview: 12+ messages / expand[flat|nested] mbox.gz Atom feed top
2015-07-14 18:43 BUG: sleeping function called from ras_epow_interrupt context Thomas Huth
2015-07-14 18:43 ` Thomas Huth
2015-07-14 21:22 ` Benjamin Herrenschmidt
2015-07-14 21:22 ` Benjamin Herrenschmidt
2015-07-15 14:35 ` Thomas Huth
2015-07-15 14:35 ` Thomas Huth
2015-07-15 19:58 ` Nathan Fontenot
2015-07-15 19:58 ` Nathan Fontenot
2015-07-16 6:23 ` Thomas Huth
2015-07-16 6:23 ` Thomas Huth
2015-07-16 17:39 ` Nathan Fontenot [this message]
2015-07-16 17:39 ` Nathan Fontenot
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=55A7EC3F.1080406@linux.vnet.ibm.com \
--to=nfont@linux.vnet.ibm.com \
--cc=anton@samba.org \
--cc=benh@kernel.crashing.org \
--cc=kvm-ppc@vger.kernel.org \
--cc=linuxppc-dev@lists.ozlabs.org \
--cc=thuth@redhat.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.