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
