Messages in this thread | | | Subject | Re: [PATCH v3] lib: cpu_rmap: avoid flushing all workqueues | From | Ben Hutchings <> | Date | Wed, 2 Jan 2013 20:21:37 +0000 |
| |
On Sat, 2012-12-29 at 11:57 -0800, David Decotigny wrote: > In some cases, free_irq_cpu_rmap() is called while holding a lock > (eg. rtnl).
I made fairly sure that it didn't get called while holding the RTNL lock. However it looks like some mlx4_en ethtool ops now call it (indirectly).
> This can lead to deadlocks, because it invokes > flush_scheduled_work() which ends up waiting for whole system > workqueue to flush, but some pending works might try to acquire the > lock we are already holding.
Which is why the kernel-doc says not to do that. But realistically a device driver may not easily be able to avoid holding either its underlying device's lock or a susbsytem global lock... and presumably there may sometimes be a work item that needs the device lock.
> This commit uses reference-counting to replace > irq_run_affinity_notifiers(). It also removes > irq_run_affinity_notifiers() altogether. > > Signed-off-by: David Decotigny <decot@googlers.com> [...] > --- a/lib/cpu_rmap.c > +++ b/lib/cpu_rmap.c [...] > /** > * irq_cpu_rmap_add - add an IRQ to a CPU affinity reverse-map > - * @rmap: The reverse-map > + * @rmap: The per-IRQ reverse-map [...]
Please drop this comment change; the only 'per-IRQ' thing is the IRQ notifier structure which is private.
With that, you can add:
Reviewed-by: Ben Hutchings <bhutchings@solarflare.com>
Ben.
-- Ben Hutchings, Staff Engineer, Solarflare Not speaking for my employer; that's the marketing department's job. They asked us to note that Solarflare product names are trademarked.
| |