RPING: Make sure CQ event thread exits before destroying the CQ.

I applied this, but am waiting on the other patch until the discussion finishes up.

It is possible for the CQ event thread to poll the CQ after it has been destroyed which can result in a seg fault on T3 interfaces.  This patch cancels the thread and waits for it to exit before destroying the CQ.

Signed-off-by Steve Wise <[email protected]>

---

 examples/rping.c |    8 ++++++--

 1 files changed, 6 insertions(+), 2 deletions(-)

examples/rping.c +4 -2

index 91952e7..e603d3b 100644
@@ -800,10 +800,10 @@ static void *rping_persistent_server_thread(void *arg)
 
        rping_test_server(cb);
        rdma_disconnect(cb->child_cm_id);
-       rping_free_buffers(cb);
-       rping_free_qp(cb);
        pthread_cancel(cb->cqthread);
        pthread_join(cb->cqthread, NULL);
+       rping_free_buffers(cb);
+       rping_free_qp(cb);
        rdma_destroy_id(cb->child_cm_id);
        free_cb(cb);
        return NULL;

@@ -888,6 +888,8 @@ static int rping_run_server(struct rping_cb *cb)
 
        rping_test_server(cb);
        rdma_disconnect(cb->child_cm_id);
+       pthread_cancel(cb->cqthread);
+       pthread_join(cb->cqthread, NULL);
        rdma_destroy_id(cb->child_cm_id);

 err2:
       rping_free_buffers(cb);

examples/rping.c +2 -0

@@ -1055,6 +1057,8 @@ static int rping_run_client(struct rping_cb *cb)
 
        rping_test_client(cb);
        rdma_disconnect(cb->cm_id);
+       pthread_cancel(cb->cqthread);
+       pthread_join(cb->cqthread, NULL);

 err2:
       rping_free_buffers(cb);
 err1:

Quoted 38 lines · Steve Wise

It is possible for the CQ event thread to poll the CQ after it has been destroyed which can result in a seg fault on T3 interfaces. This patch cancels the thread and waits for it to exit before destroying the CQ.

Signed-off-by Steve Wise<[email protected]>

---

  examples/rping.c |    8 ++++++--
  1 files changed, 6 insertions(+), 2 deletions(-)

examples/rping.c +4 -2

index 91952e7..e603d3b 100644
@@ -800,10 +800,10 @@ static void *rping_persistent_server_thread(void *arg)
 
        rping_test_server(cb);
        rdma_disconnect(cb->child_cm_id);
-       rping_free_buffers(cb);
-       rping_free_qp(cb);
        pthread_cancel(cb->cqthread);
        pthread_join(cb->cqthread, NULL);
+       rping_free_buffers(cb);
+       rping_free_qp(cb);
        rdma_destroy_id(cb->child_cm_id);
        free_cb(cb);
        return NULL;

@@ -888,6 +888,8 @@ static int rping_run_server(struct rping_cb *cb)
 
        rping_test_server(cb);
        rdma_disconnect(cb->child_cm_id);
+       pthread_cancel(cb->cqthread);
+       pthread_join(cb->cqthread, NULL);
        rdma_destroy_id(cb->child_cm_id);
  err2:
        rping_free_buffers(cb);

examples/rping.c static int rping_run_client(struct rping_cb *cb) · line 1064

Quoted 8 lines

examples/rping.c +2 -0

@@ -1055,6 +1057,8 @@ static int rping_run_client(struct rping_cb *cb)
 
        rping_test_client(cb);
        rdma_disconnect(cb->cm_id);
+       pthread_cancel(cb->cqthread);
+       pthread_join(cb->cqthread, NULL);
  err2:
        rping_free_buffers(cb);
  err1:

Hello Steve,

Are you aware that in general it is easy to trigger a deadlock or other undesired behavior by invoking pthread_cancel() ? If a thread e.g. gets canceled after having obtained a mutex lock and before that mutex is unlocked, this will cause trouble for any other thread that tries to grab the mutex.

Quoted 55 lines · Bart Van Assche

Quoted 44 lines · Steve Wise

It is possible for the CQ event thread to poll the CQ after it has been destroyed which can result in a seg fault on T3 interfaces.  This patch cancels the thread and waits for it to exit before destroying the CQ.

Signed-off-by Steve Wise<[email protected]>

---

 examples/rping.c |    8 ++++++--

 1 files changed, 6 insertions(+), 2 deletions(-)

examples/rping.c +2 -2

index 91952e7..e603d3b 100644
@@ -800,10 +800,10 @@ static void *rping_persistent_server_thread(void
 *arg)
 
        rping_test_server(cb);
        rdma_disconnect(cb->child_cm_id);
-       rping_free_buffers(cb);
-       rping_free_qp(cb);
        pthread_cancel(cb->cqthread);
        pthread_join(cb->cqthread, NULL);
+       rping_free_buffers(cb);
+       rping_free_qp(cb);
        rdma_destroy_id(cb->child_cm_id);
        free_cb(cb);

       return NULL;

examples/rping.c +2 -0

@@ -888,6 +888,8 @@ static int rping_run_server(struct rping_cb *cb)
 
        rping_test_server(cb);
        rdma_disconnect(cb->child_cm_id);
+       pthread_cancel(cb->cqthread);
+       pthread_join(cb->cqthread, NULL);
        rdma_destroy_id(cb->child_cm_id);

 err2:
       rping_free_buffers(cb);

examples/rping.c +2 -0

@@ -1055,6 +1057,8 @@ static int rping_run_client(struct rping_cb *cb)
 
        rping_test_client(cb);
        rdma_disconnect(cb->cm_id);
+       pthread_cancel(cb->cqthread);
+       pthread_join(cb->cqthread, NULL);

Quoted 4 lines

 err2:
       rping_free_buffers(cb);
 err1:

Hello Steve,

Quoted 5 lines

Are you aware that in general it is easy to trigger a deadlock or other undesired behavior by invoking pthread_cancel() ? If a thread e.g. gets canceled after having obtained a mutex lock and before that mutex is unlocked, this will cause trouble for any other thread that tries to grab the mutex.

I was under the impression that the thread would only be canceled at precise cancellation points.  Like in system calls, or if the thread calls pthread_testcancel(), which is what rping does.   There are no mutexes held by the thread being canceled either.  I think its ok.  Do you see some other issue with the rping CQ event thread that would cause these problems?

Quoted 62 lines · Steve Wise

Quoted 54 lines · Bart Van Assche

Quoted 40 lines · Steve Wise

It is possible for the CQ event thread to poll the CQ after it has been destroyed which can result in a seg fault on T3 interfaces. This patch cancels the thread and waits for it to exit before destroying the CQ.

Signed-off-by Steve Wise<[email protected]>

---

  examples/rping.c |    8 ++++++--
  1 files changed, 6 insertions(+), 2 deletions(-)

examples/rping.c +2 -2

index 91952e7..e603d3b 100644
@@ -800,10 +800,10 @@ static void *rping_persistent_server_thread(void
 *arg)
 
        rping_test_server(cb);
        rdma_disconnect(cb->child_cm_id);
-       rping_free_buffers(cb);
-       rping_free_qp(cb);
        pthread_cancel(cb->cqthread);
        pthread_join(cb->cqthread, NULL);
+       rping_free_buffers(cb);
+       rping_free_qp(cb);
        rdma_destroy_id(cb->child_cm_id);
        free_cb(cb);

        return NULL;

examples/rping.c +2 -0

@@ -888,6 +888,8 @@ static int rping_run_server(struct rping_cb *cb)
 
        rping_test_server(cb);
        rdma_disconnect(cb->child_cm_id);
+       pthread_cancel(cb->cqthread);
+       pthread_join(cb->cqthread, NULL);
        rdma_destroy_id(cb->child_cm_id);
  err2:
        rping_free_buffers(cb);

examples/rping.c static int rping_run_client(struct rping_cb *cb) · line 1064

Quoted 8 lines

examples/rping.c +2 -0

@@ -1055,6 +1057,8 @@ static int rping_run_client(struct rping_cb *cb)
 
        rping_test_client(cb);
        rdma_disconnect(cb->cm_id);
+       pthread_cancel(cb->cqthread);
+       pthread_join(cb->cqthread, NULL);
  err2:
        rping_free_buffers(cb);
  err1:

Hello Steve,

Quoted 6 lines

Are you aware that in general it is easy to trigger a deadlock or other undesired behavior by invoking pthread_cancel() ? If a thread e.g. gets canceled after having obtained a mutex lock and before that mutex is unlocked, this will cause trouble for any other thread that tries to grab the mutex.

Quoted 6 lines

I was under the impression that the thread would only be canceled at precise cancellation points. Like in system calls, or if the thread calls
pthread_testcancel(), which is what rping does. There are no mutexes held
by the thread being canceled either. I think its ok. Do you see some other issue with the rping CQ event thread that would cause these problems?

Please keep in mind that glibc uses locking internally for many file I/O functions, e.g. printf() and fprintf().

Bart.

Please keep in mind that glibc uses locking internally for many file I/O functions, e.g. printf() and fprintf().

Quoted 3 lines · Bart Van Assche

Please keep in mind that glibc uses locking internally for many file I/O functions, e.g. printf() and fprintf().

POSIX strictly defines what functions are, aren't and could be cancellation points, yes it is a surprising list. But, glibc won't break internally if you cancel any of its function calls. It has cleanup handlers and what not to protect itself.

rping doesn't hold locks, or allocate memory in the cq_thread, so it seems OK to me. My main question would be what happens internally to ibverbs, and does it call any possible cancellation point while holding locks - is it missing cleanup handlers? Etc.

Jason

Quoted 11 lines · Jason Gunthorpe

Quoted 3 lines · Bart Van Assche

Please keep in mind that glibc uses locking internally for many file I/O functions, e.g. printf() and fprintf().

POSIX strictly defines what functions are, aren't and could be cancellation points, yes it is a surprising list. But, glibc won't break internally if you cancel any of its function calls. It has cleanup handlers and what not to protect itself.

Quoted 7 lines

rping doesn't hold locks, or allocate memory in the cq_thread, so it seems OK to me. My main question would be what happens internally to ibverbs, and does it call any possible cancellation point while holding locks - is it missing cleanup handlers? Etc.

Jason

Hey Jason, do you have a pointer to the list of pthread cancellation points for Linux?

Steve.
--
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

Hey Jason, do you have a pointer to the list of pthread cancellation points for Linux?

Quoted 2 lines · Steve Wise

Hey Jason, do you have a pointer to the list of pthread cancellation points for Linux?

Sure do

http://www.kernel.org/doc/man-pages/online/pages/man7/pthreads.7.html

POSIX:2008 is a more authoritative source, the above is cribbed from there.
[ ... ]

Quoted 7 lines · Jason Gunthorpe

Quoted 2 lines · Steve Wise

Hey Jason, do you have a pointer to the list of pthread cancellation points for Linux?

Sure do

Quoted 5 lines

http://www.kernel.org/doc/man-pages/online/pages/man7/pthreads.7.html

POSIX:2008 is a more authoritative source, the above is cribbed from there.
[ ... ]

The IEEE Std 1003.1, 2004 Edition list of cancellation points is available here:
http://www.opengroup.org/onlinepubs/000095399/functions/xsh_chap02_09.html

Are you aware that in general it is easy to trigger a deadlock or other undesired behavior by invoking pthread_cancel() ? If a thread e.g. gets canceled after having obtained a mutex lock and before that mutex is unlocked, this will cause trouble for any other thread that tries to grab the mutex.