Search Linux Wireless

Re: [PATCH 1/4] ath10k: provide firmware crash info via debugfs.

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Ben Greear <greearb@xxxxxxxxxxxxxxx> writes:

> On 06/07/2014 05:55 AM, Kalle Valo wrote:
>> Ben Greear <greearb@xxxxxxxxxxxxxxx> writes:
>>
>>>>> I'm a bit leery of adding spin-locks in the dump routine just for
>>>>> this, but I can add and use a new spin-lock if you prefer.
>>>>
>>>> Why a new spinlock? I didn't review the locking requirements, but I
>>>> would first check ar->data_lock can be used.
>>>
>>> I think it has to be a spin-lock because the crash dump is gathered
>>> in the irq handler, so I can't use a mutex as far as I know...
>>>
>>> I'll work on adding such a lock today.
>>
>> I asked why add a _new_ spinlock as ar->data_lock is already a spinlock.
>
> Surely we do not want to impede traffic flow just to dump debug info?

I don't see data_lock being used anywhere in hotpath, but of course I
might have missed something. Anyway, if for optimisation reasons we need
to introduce a new lock that should happen for a specific case in hot
path. Handling firmware debug log events is not in that category.

> And, it is easier to review a specific spinlock rather than use one big
> global-ish lock and have to review every use of that lock for issues.

Sure, it's easier for you to just add a new lock and then forget :) But
not for me who would have to maintain 20 different locks for years to
come. My view is that a new lock should be added on very exceptional
cases and with good justifications.

-- 
Kalle Valo
--
To unsubscribe from this list: send the line "unsubscribe linux-wireless" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html




[Index of Archives]     [Linux Host AP]     [ATH6KL]     [Linux Wireless Personal Area Network]     [Linux Bluetooth]     [Linux Netdev]     [Kernel Newbies]     [Linux Kernel]     [IDE]     [Git]     [Netfilter]     [Bugtraq]     [Yosemite Hiking]     [MIPS Linux]     [ARM Linux]     [Linux RAID]

  Powered by Linux