Messages in this thread | | | From | "Matthias Behr" <> | Subject | AW: [PATCH RT RFC v4 1/8] add generalized priority-inheritance interface | Date | Sat, 16 Aug 2008 17:32:55 +0200 |
| |
Hi Greg,
I got a few review comments/questions. Pls see below.
Best Regards, Matthias
P.S. I'm a kernel newbie so don't hesitate to tell me if I'm wrong ;-)
> +/** > + * pi_sink_init - initialize a pi_sink before use > + * @sink: a sink context > + * @ops: pointer to an pi_sink_ops structure > + */ > +static inline void > +pi_sink_init(struct pi_sink *sink, struct pi_sink_ops *ops) > +{ > + atomic_set(&sink->refs, 0); > + sink->ops = ops; > +}
Shouldn't ops be tested for 0 here? (ASSERT/BUG_ON/...) (get's dereferenced later quite often in the form "if (sink->ops->...)".
> +/** > + * pi_sink_put - down the reference count, freeing the sink if 0 > + * @node: the node context > + * @flags: optional flags to modify behavior. Reserved, must be 0. > + * > + * Returns: none > + */ > +static inline void > +pi_sink_put(struct pi_sink *sink, unsigned int flags) > +{ > + if (atomic_dec_and_test(&sink->refs)) { > + if (sink->ops->free) > + sink->ops->free(sink, flags); > + } > +}
Shouldn't the atomic/locked part cover the ...->free(...) as well? A pi_get right after the atomic_dec_and_test but before the free() could lead to a free() with refs>0?
| |