Messages in this thread Patch in this message |  | | Date | Thu, 3 Jan 2013 13:47:49 -0800 | From | Andrew Morton <> | Subject | Re: [PATCH v4] lib: cpu_rmap: avoid flushing all workqueues |
| |
On Wed, 2 Jan 2013 18:35:00 -0800 David Decotigny <decot@googlers.com> wrote: >
(please don't top-post)
> Thanks. It is not too late to review this code. But I'd prefer not to > address refactoring issues with this patch, which is supposed to fix a > deadlock with some drivers. So I'd rather not to add function > renaming, suppressions, etc. that have an effect outside cpu_rmap's > code. Instead, I'd like to propose another patch later, which will be > a little more intrusive in that respect, if that's ok with you. > > I believe Ben answered your other concerns, I consider him as the > expert as to whether there should be finer-grained locking implemented > in this subsystem. Let me just add that I second him in saying that > the deadlock risk was clearly identified and mentioned in the doc. > Unfortunately, initial implementation makes this risk hard to > work-around for some drivers, which is what this patch proposes to > address.
^^ all this pertains to the existing code.
> So, for now, I'd like to keep v4 as the current version. And some > refactoring will be done in a later patch.
^^ this pertains to the current patch. And no reason for ignoring my review comments was provided.
So I did it myself. It's very simple, as free_cpu_rmap() has no callers.
Also, free_irq_cpu_rmap() is now distinctly fishy - it runs all the notifiers every time it is called. Surely it should only do that when the refcount falls to zero?
From: Andrew Morton <akpm@linux-foundation.org> Subject: lib-cpu_rmap-avoid-flushing-all-workqueues-fix
eliminate free_cpu_rmap, rename cpu_rmap_reclaim() to cpu_rmap_release(), propagate kref_put() retval from cpu_rmap_put()
Cc: "David S. Miller" <davem@davemloft.net> Cc: Amir Vadai <amirv@mellanox.com> Cc: Ben Hutchings <bhutchings@solarflare.com> Cc: David Decotigny <decot@googlers.com> Cc: Eric Dumazet <edumazet@google.com> Cc: Josh Triplett <josh@joshtriplett.org> Cc: Or Gerlitz <ogerlitz@mellanox.com> Signed-off-by: Andrew Morton <akpm@linux-foundation.org> ---
include/linux/cpu_rmap.h | 2 +- lib/cpu_rmap.c | 21 ++++++--------------- 2 files changed, 7 insertions(+), 16 deletions(-)
diff -puN include/linux/cpu_rmap.h~lib-cpu_rmap-avoid-flushing-all-workqueues-fix include/linux/cpu_rmap.h --- a/include/linux/cpu_rmap.h~lib-cpu_rmap-avoid-flushing-all-workqueues-fix +++ a/include/linux/cpu_rmap.h @@ -36,7 +36,7 @@ struct cpu_rmap { #define CPU_RMAP_DIST_INF 0xffff extern struct cpu_rmap *alloc_cpu_rmap(unsigned int size, gfp_t flags); -extern void free_cpu_rmap(struct cpu_rmap *rmap); +extern int cpu_rmap_put(struct cpu_rmap *rmap); extern int cpu_rmap_add(struct cpu_rmap *rmap, void *obj); extern int cpu_rmap_update(struct cpu_rmap *rmap, u16 index, diff -puN include/linux/interrupt.h~lib-cpu_rmap-avoid-flushing-all-workqueues-fix include/linux/interrupt.h diff -puN lib/cpu_rmap.c~lib-cpu_rmap-avoid-flushing-all-workqueues-fix lib/cpu_rmap.c --- a/lib/cpu_rmap.c~lib-cpu_rmap-avoid-flushing-all-workqueues-fix +++ a/lib/cpu_rmap.c @@ -65,10 +65,10 @@ struct cpu_rmap *alloc_cpu_rmap(unsigned EXPORT_SYMBOL(alloc_cpu_rmap); /** - * cpu_rmap_reclaim - internal reclaiming helper called from kref_put + * cpu_rmap_release - internal reclaiming helper called from kref_put * @ref: kref to struct cpu_rmap */ -static void cpu_rmap_reclaim(struct kref *ref) +static void cpu_rmap_release(struct kref *ref) { struct cpu_rmap *rmap = container_of(ref, struct cpu_rmap, refcount); kfree(rmap); @@ -84,23 +84,14 @@ static inline void cpu_rmap_get(struct c } /** - * cpu_rmap_put - internal helper to release ref on a cpu_rmap + * cpu_rmap_put - release ref on a cpu_rmap * @rmap: reverse-map allocated with alloc_cpu_rmap() */ -static inline void cpu_rmap_put(struct cpu_rmap *rmap) +int cpu_rmap_put(struct cpu_rmap *rmap) { - kref_put(&rmap->refcount, cpu_rmap_reclaim); + return kref_put(&rmap->refcount, cpu_rmap_release); } - -/** - * free_cpu_rmap - free CPU affinity reverse-map - * @rmap: Reverse-map allocated with alloc_cpu_rmap() - */ -void free_cpu_rmap(struct cpu_rmap *rmap) -{ - cpu_rmap_put(rmap); -} -EXPORT_SYMBOL(free_cpu_rmap); +EXPORT_SYMBOL(cpu_rmap_put); /* Reevaluate nearest object for given CPU, comparing with the given * neighbours at the given distance. _
|  |