On Thu, Jun 11, 2015 at 03:56:50PM +0300, Yishai Hadas wrote:
>
> - file->async_file = filp->private_data;
> -
> - INIT_IB_EVENT_HANDLER(&file->event_handler, file->device->ib_dev,
> - ib_uverbs_event_handler);
> - ret = ib_register_event_handler(&file->event_handler);
> - if (ret)
> - goto err_file;
> -
> kref_get(&file->async_file->ref);
This kref_get should be placed next to the assignment:
> + if (is_async) {
> + uverbs_file->async_file = ev_file;
Here
Also, I'd say it should really be:
if (is_async) {
BUG_ON(uverbs_file->async_file);
uverbs_file->async_file = ev_file;
kref_get(&uverbs_file->async_file->ref);
and can you prove the BUG_ON never hits?
It looks to me like a little more error unwind is necessary to
guarantee that. ie ib_uverbs_get_context uses ucontext to create that
invariant, but it sets ucontext last, so async file must be left null
if ib_uverbs_alloc_event_file fails.
Otherwise it looks OK to me.
Jason
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html
