kdbus: conn_call() cleanup

diff --git a/ipc/kdbus/connection.c b/ipc/kdbus/connection.c
index 0d0df0ba6ffb4b10d1a340403f101ce5d3004f53..1a2d9fe7fb4796ae5f477335003f3941f1b52ed2 100644 (file)
--- a/ipc/kdbus/connection.c
+++ b/ipc/kdbus/connection.c
@@ -1070,8 +1070,7 @@ static int kdbus_conn_call(struct kdbus_conn *src,
        struct kdbus_conn *dst;
        struct kdbus_bus *bus;
        int ret;
-       bool has_sync_reply;
-       bool free_wait;
+       typeof(wait->waiting) waiting;
        kdbus_assert(staging);
        kdbus_assert(*staging);
        kdbus_assert(src);
@@ -1090,8 +1089,6 @@ static int kdbus_conn_call(struct kdbus_conn *src,
 
 restart:
        wait = NULL;
-       has_sync_reply = false;
-       free_wait = false;
 
        /* name-registry must be locked for lookup *and* collecting data */
        down_read(&bus->name_registry.rwlock);
@@ -1158,6 +1155,7 @@ read_unlock:
                 */
                if (!kdbus_conn_active(src)) {
                        ret = -ECONNRESET;
+                       waiting = 1;
                        break;
                }
 
@@ -1165,10 +1163,8 @@ read_unlock:
                 * After the replying peer unset the waiting variable
                 * it will wake us up.
                 */
-               if (wait->waiting <= 0) {
-                       has_sync_reply = true;
+               if ((waiting = wait->waiting) <= 0) /* reply present (receive) or sync moved (reissue call) */
                        break;
-               }
 
                if (cancel_fd && POLLIN & cancel_fd->f_op->poll(cancel_fd, &pwq.pt)) {
                        ret = -ECANCELED;
@@ -1196,46 +1192,40 @@ read_unlock:
        poll_freewait(&pwq);
 
        /* reap */
-       if (!has_sync_reply) {
+       if (waiting > 0) { /* no reply yet - recheck under lock */
+               bool free_wait = true;
                mutex_lock(&dst->lock);
-               if (wait->waiting <= 0) { /* reply is now present */
-                       kdbus_assert(wait->queue_entry.reply_state < 0); /* wait is now completely divorced from dst */
-                       has_sync_reply = true;
-               } else { /* reply is not present yet - abort the call */
+               if (0 < (waiting = wait->waiting)) { /* reply is not present yet - abort the call */
                        var(reply_state, wait->queue_entry.reply_state);
                        if (reply_state >= 0) { /* still linked - unlink and free the message so that replier will never see it */
                                if (reply_state > 0) /* still enqueued in dst */
                                        kdbus_queue_entry_destroy(&wait->queue_entry, dst);
                                else /* only dequeued sync replies are actually linked */
                                        kdbus_reply_unlink(wait);
-                               free_wait = true;
-                       } else /* unlinked but still in the process of answering */
+                       } else { /* unlinked but in the process of being answered */
                                wait->waiting = 0; /* pity, inform replier we don't care anymore */
-               }
+                               free_wait = false;
+                       }
+               } else if (!waiting) /* reply present */
+                       kdbus_assert(wait->queue_entry.reply_state < 0); /* wait is now completely divorced from dst */
                mutex_unlock(&dst->lock);
+               if (unlikely(!free_wait)) /* will be freed by replier who has already been informed */
+                       goto end;
        }
 
-       if (has_sync_reply) {
-               if (unlikely(wait->waiting < 0)) { /* the object got moved - need to reissue the entire call */
-                       kdbus_reply_free(wait);
-                       goto restart;
-               }
-               ret = wait->err;
-               free_wait = true;
-       }
-
-       if (!ret) { /* we got an actual reply: let's install */
+       if (!waiting && !(ret = wait->err)) { /* we got an actual reply: let's install */
                struct kdbus_queue_entry *entry = &wait->queue_entry;
                mutex_lock(&src->lock);
-               ret = kdbus_queue_entry_install(entry, src, &cmd_send->reply.return_flags, true);
-               kdbus_pool_slice_publish(&src->pool, entry->slice, &cmd_send->reply.offset, &cmd_send->reply.msg_size);
+               if (!(ret = kdbus_queue_entry_install(entry, src, &cmd_send->reply.return_flags, true)))
+                       kdbus_pool_slice_publish(&src->pool, entry->slice, &cmd_send->reply.offset, &cmd_send->reply.msg_size);
                kdbus_queue_entry_destroy_nounlink(entry, src);
                mutex_unlock(&src->lock);
        }
 
-       if (free_wait)
-               kdbus_reply_free(wait);
-
+       kdbus_reply_free(wait);
+       if (unlikely(waiting < 0)) /* the object got moved - need to reissue the entire call */
+               goto restart;
+end:
        return -EINTR == ret ? -ERESTARTSYS : ret;
 }
 

diff --git a/ipc/kdbus/reply.h b/ipc/kdbus/reply.h
index cf16f6337fd600a4b69ef5ca941af360b1aa30ca..f05d27e55385c0a67bbcc9090d3403a3a20d28c1 100644 (file)
--- a/ipc/kdbus/reply.h
+++ b/ipc/kdbus/reply.h
@@ -40,7 +40,7 @@ struct kdbus_reply {
        u64 cookie;
        union {
                u64 deadline_ns; /* async */
-               int waiting; /* sync; 1 == sync caller waiting, 0 == sync caller gone or reply present, -1 == sync moved (need to reissue call t) */
+               int waiting; /* sync; 1 == sync caller waiting, 0 == sync caller gone or reply present, -1 == sync moved (need to reissue call) */
        };
        struct kdbus_conn *reply_dst;
        int err;