[ofa-general] hotplug event handle question

[Sean Hefty]

>I think the problem is that cma_remove_id_dev overrides the current state,
>losing state information in the process. Why do we need CMA_DEVICE_REMOVAL
>at all? Everything seems to work fine just by forwarding
>RDMA_CM_EVENT_DEVICE_REMOVAL
>to user, without touching state.

I need to read back over the code.  The problem is that device removal can come
at anytime.  The user could have called rdma_destroy_id, be about to call it, or
be destroying the id by returning a non-zero value from a callback.  We need to
synchronize with all cases, and in the later case, we cannot perform the
callback to notify the user of the device removal.  Similarly, if the user
destroys the id from a device removal event callback, then callbacks for others
event should not be called.

If we can do this by removing the device removal state, that would seem to be
the simplest approach, but I need to verify that we can cover all corner cases.

>@@ -2689,20 +2688,13 @@ static void cma_add_one(struct ib_device *device)
> static int cma_remove_id_dev(struct rdma_id_private *id_priv)
> {
> 	struct rdma_cm_event event;
>-	enum cma_state state;
>
>-	/* Record that we want to remove the device */
>-	state = cma_exch(id_priv, CMA_DEVICE_REMOVAL);

Since we're not changing the state, callbacks to the same id may be invoked from
another thread.  The cma guarantees that all callbacks to a single id are
serialized.

>-	if (state == CMA_DESTROYING)
>+	if (cma_comp(id_priv, CMA_DESTROYING))
> 		return 0;
>
>-	cma_cancel_operation(id_priv, state);
>+	cma_cancel_operation(id_priv, id_priv->state);
> 	wait_event(id_priv->wait_remove, !atomic_read(&id_priv->dev_remove));
>
>-	/* Check for destruction from another callback. */
>-	if (!cma_comp(id_priv, CMA_DEVICE_REMOVAL))
>-		return 0;

At the very least we need to repeat the check:

	if (!cma_comp(id_priv, CMA_DESTROYING))
		return 0;

here to avoid calling the user after they've tried to destroy their id from
another callback.  See comment above.

- Sean