Subject: Re: [PATCH v3 3/3] powerpc/powernv: remove opal_sensor_mutex

From: Cedric Le Goater <[email-masked]>
Date: Mon, 30 Mar 2015 12:05:59 +0200

On 03/30/2015 08:59 AM, Michael Ellerman wrote:
> On Mon, 2015-03-30 at 08:51 +0200, Cedric Le Goater wrote:
>> On 03/30/2015 04:09 AM, Michael Ellerman wrote:
>>> On Fri, 2015-03-27 at 17:39 +0100, Cédric Le Goater wrote:
>>>> The opal sensor mutex protects the opal_sensor_read call which
>>>> can return a OPAL_BUSY code on IBM Power systems if a previous 
>>>> request is in progress.
>>>>
>>>> This can be handled at user level with a retry.
>>>
>>> It can, but how does it actually look in practice?
>>>
>>> It looks like the only use of opal_get_sensor_data() is show_sensor() in
>>> drivers/hwmon/ibmpowernv.c.
>>>
>>> Because that's a sysfs attribute folks will be generally just dumping 
>>> that with cat, or reading it in a shell script, neither of which will 
>>> cope nicely with EBUSY I think?
>>
>> It won't, I agree but it should only happen when running concurrent cat 
>> commands on the hwmon sysfs files. The event should be rare enough.
> 
> Rare enough maybe, but a real pain in the .. to cope with in a shell script if
> you're trying to automate something.
> 
>> Anyhow, this is not a big issue. We can drop that patch. The real "issue"
>> is the time it takes to get some values back from the FSP. This is what
>> user space has been most surprised about.
> 
> OK. The other option would be to move the mutex into the sysfs show routine, so
> only that is synchronous. That would give you nice behaviour from cat, ie. it
> would sleep on contention but still be killable with ctrl-c.

Let's keep it how it is and see if it is possible to the improve OPAL 
side first. 

I will send you an updated patchset shortly. 

Thanks for the review. 

C.