Re: [PATCH v3] lib: cpu_rmap: avoid flushing all workqueues

From: Ben Hutchings
Date: Wed Jan 02 2013 - 15:21:33 EST


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@xxxxxxxxxxxx>
[...]
> --- 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@xxxxxxxxxxxxxx>

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.

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/