On Tue, Feb 19, 2019 at 11:08:54AM +0100, Bernard Metzler wrote:
> +
> +static inline void siw_pd_put(struct siw_pd *pd)
> +{
> + siw_dbg_obj(pd, "old refcount: %d\n", kref_read(&pd->hdr.ref));
> + kref_put(&pd->hdr.ref, siw_free_pd);
> +}
> +
> +/*
> + * siw_mem_id2obj()
> + *
> + * resolves memory from stag given by id. might be called from:
> + * o process context before sending out of sgl, or
> + * o in softirq when resolving target memory
> + */
> +static inline struct siw_mem *siw_mem_id2obj(struct siw_device *sdev, int id)
> +{
> + struct siw_objhdr *obj;
> + struct siw_mem *mem = NULL;
> +
> + rcu_read_lock();
> + obj = siw_get_obj(&sdev->mem_idr, id);
> + rcu_read_unlock();
There is something wrong with this use of rcu:
rcu_read_lock();
struct siw_objhdr *obj = idr_find(idr, id);
if (obj)
kref_get(&obj->ref);
1) Not a good idea to split locking and lookup, one function should
handle both:
rcu_read_lock()
obj = __xa_load(xa, id);
2) It is racy, obj may have have gone to siw_free_mem -> kfree_rcu and
now has a 0 ref, this will caus kref_get to warn_on.
if (obj)
if (!kref_get_unless_zero(&obj->ref))
obj = NULL
Jason
