Re: [PATCH] PM / wakeirq: Add wakeup name to dedicated wake irqs

[Rafael J. Wysocki]

On Fri, Feb 16, 2018 at 11:39 AM, Andy Shevchenko
<andy.shevchenko@xxxxxxxxx> wrote:
> On Fri, Feb 16, 2018 at 11:14 AM, Rafael J. Wysocki <rafael@xxxxxxxxxx> wrote:
>> On Wed, Feb 14, 2018 at 12:23 PM, Andy Shevchenko
>> <andy.shevchenko@xxxxxxxxx> wrote:
>>> On Fri, Feb 9, 2018 at 6:14 PM, Tony Lindgren <tony@xxxxxxxxxxx> wrote:
>>>> This makes it easy to grep :wakeup /proc/interrupts.
>>>
>>> I used to have another patch (not published) to provide this
>>> information via /sys/kernel/irq.
>>>
>>> OK, here we are:
>
>> It's a bit hard to comment patches sent as attachments, but I'll try anyway. :-)
>
> Sorry, I was thinking that Tony may test it first to see if it does
> what he wants.
> Of course I would send normally in case it's an expected approach.
>
>> IMO it is somewhat excessive to put the entire sprintf() under a raw
>> spinlock and it's not even necessary.
>
> It's a copy'n'paste of from the rest of functions there.

Fair enough. :-)

>> The value can change any time after you've dropped the lock and in
>> particular before the function returns, so why bother with locking?
>> desc will not go away from under you at that point anyway.
>
> IIRC descriptor's content might be changed, or descriptor itself might
> be gone (potential crash).

No, desc cannot go away at this point AFAICS due to the kernfs
refcounting.  And the lock is *inside* of the desc object anyway, so
it doesn't help really against that.

The contents may change, but so what?

Effectively, you read an int and reading an int is atomic.  It may
change after that, but the lock doesn't prevent it from changing.  It
only prevents the change from being applied to it before you drop the
lock, but why do you care?