When is it safe to release connection resources?

When is it safe to release connection resources?

[Flavio Baronti]

Hello,

I'm new to RDMA development and I have a question regarding resource release.
If I understood correctly, when ibv_get_cq_event returns, it holds some sort of lock over the completion queue, which is 
released when I call ibv_req_notify_cq. This lock is checked also in ibv_destroy_cq, so that:
1) When ibv_destroy_cq returns, I am certain that there is no thread running somewhere between ibv_get_cq_event and 
ibv_req_notify_cq
2) When ibv_destroy_cq returns, I am certain that ibv_get_cq_event will not return the destroyed cq any more.

Is all this correct?

Thanks
Flavio
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: When is it safe to release connection resources?

[Bart Van Assche]

On Thu, Dec 29, 2011 at 4:42 PM, Flavio Baronti
<f.baronti-ngIpsMLAhaq41k5uCYKmRQ@public.gmane.org> wrote:
> I'm new to RDMA development and I have a question regarding resource release.
> If I understood correctly, when ibv_get_cq_event returns, it holds some sort
> of lock over the completion queue, which is released when I call
> ibv_req_notify_cq. This lock is checked also in ibv_destroy_cq, so that:
> 1) When ibv_destroy_cq returns, I am certain that there is no thread running
> somewhere between ibv_get_cq_event and ibv_req_notify_cq
> 2) When ibv_destroy_cq returns, I am certain that ibv_get_cq_event will not
> return the destroyed cq any more.

There is a paragraph in the IBTA that warns that the above sequence
can cause WQE and data segment leakage in the HCA for QPs associated
with an SRQ. I don't know though whether this can also occur for QPs
with a private receive queue - I'm not a HCA firmware expert.

Bart.
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

RE: When is it safe to release connection resources?

[Hefty, Sean]

> If I understood correctly, when ibv_get_cq_event returns, it holds some sort
> of lock over the completion queue, which is

ibv_get_cq_event will increment a reference count on the CQ that it is returning.

> released when I call ibv_req_notify_cq. This lock is checked also in

You decrement the count with ibv_ack_cq_events.  ibv_req_notify_cq is used to arm the cq, so that a completion generates an interrupt and a new event.

> ibv_destroy_cq, so that:
> 1) When ibv_destroy_cq returns, I am certain that there is no thread running
> somewhere between ibv_get_cq_event and
> ibv_req_notify_cq

ibv_destroy_cq will block until all outstanding references on the cq have been released.  The intent is to protect the user from ibv_get_cq_event from returning a reference to a cq that is being destroyed from another thread, which could result in a crash in the user's code.

> 2) When ibv_destroy_cq returns, I am certain that ibv_get_cq_event will not
> return the destroyed cq any more.

correct

- Sean
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: When is it safe to release connection resources?

[Flavio Baronti]

So in order to safely call ibv_destroy_cq, I should call ibv_ack_cq_events *after* ibv_poll_cq? The example on the man 
page for ibv_get_cq_event calls it before, is it an error?

Flavio

Il 1/2/2012 17:39 PM, Hefty, Sean ha scritto:
>> If I understood correctly, when ibv_get_cq_event returns, it holds some sort
>> of lock over the completion queue, which is
>
> ibv_get_cq_event will increment a reference count on the CQ that it is returning.
>
>> released when I call ibv_req_notify_cq. This lock is checked also in
>
> You decrement the count with ibv_ack_cq_events.  ibv_req_notify_cq is used to arm the cq, so that a completion generates an interrupt and a new event.
>
>> ibv_destroy_cq, so that:
>> 1) When ibv_destroy_cq returns, I am certain that there is no thread running
>> somewhere between ibv_get_cq_event and
>> ibv_req_notify_cq
>
> ibv_destroy_cq will block until all outstanding references on the cq have been released.  The intent is to protect the user from ibv_get_cq_event from returning a reference to a cq that is being destroyed from another thread, which could result in a crash in the user's code.
>
>> 2) When ibv_destroy_cq returns, I am certain that ibv_get_cq_event will not
>> return the destroyed cq any more.
>
> correct
>
> - Sean
>

--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

RE: When is it safe to release connection resources?

[Hefty, Sean]

> So in order to safely call ibv_destroy_cq, I should call ibv_ack_cq_events
> *after* ibv_poll_cq? The example on the man
> page for ibv_get_cq_event calls it before, is it an error?

You should call ibv_ack_cq_events after ibv_get_cq_events but before ibv_destroy_cq.  That can come before or after calling ibv_poll_cq.  And, although it's probably easiest to call ack after get, note that you don't need a 1:1 call ratio between get/ack.  You can keep a count of the number of times that ibv_get_cq_events returns a specific cq, and then call ibv_ack_cq_events for that amount just before calling ibv_destroy_cq.

- Sean
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: When is it safe to release connection resources?

[Bart Van Assche]

On Mon, Jan 2, 2012 at 4:39 PM, Hefty, Sean <sean.hefty-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org> wrote:
>> If I understood correctly, when ibv_get_cq_event returns, it holds some sort
>> of lock over the completion queue, which is
>> ibv_destroy_cq, so that:
>> 1) When ibv_destroy_cq returns, I am certain that there is no thread running
>> somewhere between ibv_get_cq_event and
>> ibv_req_notify_cq
>
> ibv_destroy_cq will block until all outstanding references on the cq have been released.
> The intent is to protect the user from ibv_get_cq_event from returning a reference to a cq
> that is being destroyed from another thread, which could result in a crash in the user's code.

I've just had a look at the kernel code that implements all this
(uverbs_cmd.c and uverbs_main.c). I haven't found any precautions
against ib_uverbs_comp_handler() accessing *uobj after
ib_uverbs_destroy_cq() has invoked put_uobj(uobj). Did I miss
something ?

Bart.
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

RE: When is it safe to release connection resources?

[Hefty, Sean]

> I've just had a look at the kernel code that implements all this
> (uverbs_cmd.c and uverbs_main.c). I haven't found any precautions
> against ib_uverbs_comp_handler() accessing *uobj after
> ib_uverbs_destroy_cq() has invoked put_uobj(uobj). Did I miss
> something ?

The kernel operation is different, since it relies on callbacks.  When the kernel ib_destroy_cq() returns, we are guaranteed that the completion handler (ib_uverbs_comp_handler) is not executing and will not be called.  After destroying the kernel cq, ib_uverbs_destroy_cq() will remove all references to the destroyed cq from the event list.

The issue is that another thread could have retrieved an event for this cq before the cleanup occurs.  When ib_uverbs_destroy_cq unwinds back to user space, it returns the total number of events that were retrieved from the kernel.  ibv_destroy_cq blocks until the application has processed all cq events, which are indicated by the app calling ibv_ack_cq_events().

- Sean
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: When is it safe to release connection resources?

[Bart Van Assche]

On Wed, Jan 4, 2012 at 9:05 PM, Hefty, Sean <sean.hefty-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org> wrote:
>> I've just had a look at the kernel code that implements all this
>> (uverbs_cmd.c and uverbs_main.c). I haven't found any precautions
>> against ib_uverbs_comp_handler() accessing *uobj after
>> ib_uverbs_destroy_cq() has invoked put_uobj(uobj). Did I miss
>> something ?
>
> The kernel operation is different, since it relies on callbacks.  When the kernel ib_destroy_cq()
> returns, we are guaranteed that the completion handler (ib_uverbs_comp_handler) is not executing
> and will not be called.  After destroying the kernel cq, ib_uverbs_destroy_cq() will remove all
> references to the destroyed cq from the event list.

Sorry if I wasn't clear enough, but I was referring to the case where
the kernel completion handler gets invoked after
ib_uverbs_destroy_cq() has started but before that function has
invoked ib_destroy_cq().

More in general - assuming that completion notifications have been
enabled - I'm wondering whether it is possible to shut down a queue
pair without triggering a race condition if that queue pair hasn't
been transitioned to the reset or error state first.

Bart.
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: When is it safe to release connection resources?

[Roland Dreier]

On Thu, Jan 5, 2012 at 3:23 AM, Bart Van Assche <bvanassche-HInyCGIudOg@public.gmane.org> wrote:
> Sorry if I wasn't clear enough, but I was referring to the case where
> the kernel completion handler gets invoked after
> ib_uverbs_destroy_cq() has started but before that function has
> invoked ib_destroy_cq().

That should be OK.  ib_uverbs_destroy_cq() doesn't really do anything
until ib_destroy_cq() has returned, and at that point it is guaranteed
that the completion handler for the CQ is done.

> More in general - assuming that completion notifications have been
> enabled - I'm wondering whether it is possible to shut down a queue
> pair without triggering a race condition if that queue pair hasn't
> been transitioned to the reset or error state first.

destroying a QP is pretty much equivalent to transitioning it to the
reset state.

 - R.
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: When is it safe to release connection resources?

[Bart Van Assche]

On Thu, Jan 5, 2012 at 5:29 PM, Roland Dreier <roland-BHEL68pLQRGGvPXPguhicg@public.gmane.org> wrote:
> That should be OK.  ib_uverbs_destroy_cq() doesn't really do anything
> until ib_destroy_cq() has returned, and at that point it is guaranteed
> that the completion handler for the CQ is done.

That sounds like a requirement for low-level IB drivers. Has it
already been considered to document this requirement in
Documentation/infiniband/core_locking.txt ?

Thanks,

Bart.
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html