All of lore.kernel.org
 help / color / mirror / Atom feed
From: Eric Blake <eblake@redhat.com>
To: qemu-devel@nongnu.org
Cc: Vladimir Sementsov-Ogievskiy <vsementsov@yandex-team.ru>,
	Kevin Wolf <kwolf@redhat.com>, Hanna Reitz <hreitz@redhat.com>,
	qemu-block@nongnu.org (open list:Network Block Dev...)
Subject: [PULL 12/14] nbd/client: Simplify cookie vs. index computation
Date: Wed, 19 Jul 2023 15:27:49 -0500	[thread overview]
Message-ID: <20230719202736.2675295-28-eblake@redhat.com> (raw)
In-Reply-To: <20230719202736.2675295-16-eblake@redhat.com>

Our code relies on a sentinel cookie value of zero for deciding when a
packet has been handled, as well as relying on array indices between 0
and MAX_NBD_REQUESTS-1 for dereferencing purposes.  As long as we can
symmetrically convert between two forms, there is no reason to go with
the odd choice of using XOR with a random pointer, when we can instead
simplify the mappings with a mere offset of 1.

Using ((uint64_t)-1) as the sentinel instead of NULL such that the two
macros could be entirely eliminated might also be possible, but would
require a more careful audit to find places where we currently rely on
zero-initialization to be interpreted as the sentinel value, so I did
not pursue that course.

Signed-off-by: Eric Blake <eblake@redhat.com>
Message-ID: <20230608135653.2918540-7-eblake@redhat.com>
[eblake: enhance commit message]
Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov@yandex-team.ru>
---
 block/nbd.c | 16 ++++++++--------
 1 file changed, 8 insertions(+), 8 deletions(-)

diff --git a/block/nbd.c b/block/nbd.c
index be3c46c6fee..5322e66166c 100644
--- a/block/nbd.c
+++ b/block/nbd.c
@@ -50,8 +50,8 @@
 #define EN_OPTSTR ":exportname="
 #define MAX_NBD_REQUESTS    16

-#define COOKIE_TO_INDEX(bs, cookie) ((cookie) ^ (uint64_t)(intptr_t)(bs))
-#define INDEX_TO_COOKIE(bs, index)  ((index)  ^ (uint64_t)(intptr_t)(bs))
+#define COOKIE_TO_INDEX(cookie) ((cookie) - 1)
+#define INDEX_TO_COOKIE(index)  ((index) + 1)

 typedef struct {
     Coroutine *coroutine;
@@ -420,7 +420,7 @@ static void coroutine_fn GRAPH_RDLOCK nbd_reconnect_attempt(BDRVNBDState *s)
 static coroutine_fn int nbd_receive_replies(BDRVNBDState *s, uint64_t cookie)
 {
     int ret;
-    uint64_t ind = COOKIE_TO_INDEX(s, cookie), ind2;
+    uint64_t ind = COOKIE_TO_INDEX(cookie), ind2;
     QEMU_LOCK_GUARD(&s->receive_mutex);

     while (true) {
@@ -435,7 +435,7 @@ static coroutine_fn int nbd_receive_replies(BDRVNBDState *s, uint64_t cookie)
              * woken by whoever set s->reply.cookie (or never wait in this
              * yield). So, we should not wake it here.
              */
-            ind2 = COOKIE_TO_INDEX(s, s->reply.cookie);
+            ind2 = COOKIE_TO_INDEX(s->reply.cookie);
             assert(!s->requests[ind2].receiving);

             s->requests[ind].receiving = true;
@@ -468,7 +468,7 @@ static coroutine_fn int nbd_receive_replies(BDRVNBDState *s, uint64_t cookie)
             nbd_channel_error(s, -EINVAL);
             return -EINVAL;
         }
-        ind2 = COOKIE_TO_INDEX(s, s->reply.cookie);
+        ind2 = COOKIE_TO_INDEX(s->reply.cookie);
         if (ind2 >= MAX_NBD_REQUESTS || !s->requests[ind2].coroutine) {
             nbd_channel_error(s, -EINVAL);
             return -EINVAL;
@@ -519,7 +519,7 @@ nbd_co_send_request(BlockDriverState *bs, NBDRequest *request,
     qemu_mutex_unlock(&s->requests_lock);

     qemu_co_mutex_lock(&s->send_mutex);
-    request->cookie = INDEX_TO_COOKIE(s, i);
+    request->cookie = INDEX_TO_COOKIE(i);

     assert(s->ioc);

@@ -832,7 +832,7 @@ static coroutine_fn int nbd_co_do_receive_one_chunk(
         int *request_ret, QEMUIOVector *qiov, void **payload, Error **errp)
 {
     int ret;
-    int i = COOKIE_TO_INDEX(s, cookie);
+    int i = COOKIE_TO_INDEX(cookie);
     void *local_payload = NULL;
     NBDStructuredReplyChunk *chunk;

@@ -1038,7 +1038,7 @@ static bool coroutine_fn nbd_reply_chunk_iter_receive(BDRVNBDState *s,

 break_loop:
     qemu_mutex_lock(&s->requests_lock);
-    s->requests[COOKIE_TO_INDEX(s, cookie)].coroutine = NULL;
+    s->requests[COOKIE_TO_INDEX(cookie)].coroutine = NULL;
     s->in_flight--;
     qemu_co_queue_next(&s->free_sema);
     qemu_mutex_unlock(&s->requests_lock);
-- 
2.41.0



  parent reply	other threads:[~2023-07-19 20:39 UTC|newest]

Thread overview: 16+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-07-19 20:27 [PULL 00/14] NBD patches for 2023-07-19 Eric Blake
2023-07-19 20:27 ` [PULL 01/14] qemu-nbd: pass structure into nbd_client_thread instead of plain char* Eric Blake
2023-07-19 20:27 ` [PULL 02/14] qemu-nbd: fix regression with qemu-nbd --fork run over ssh Eric Blake
2023-07-19 20:27 ` [PULL 03/14] qemu-nbd: properly report error if qemu_daemon() is failed Eric Blake
2023-07-19 20:27 ` [PULL 04/14] qemu-nbd: properly report error on error in dup2() after qemu_daemon() Eric Blake
2023-07-19 20:27 ` [PULL 05/14] qemu-nbd: handle dup2() error when qemu-nbd finished setup process Eric Blake
2023-07-19 20:27 ` [PULL 06/14] qemu-nbd: make verbose bool and local variable in main() Eric Blake
2023-07-19 20:27 ` [PULL 07/14] nbd/client: Use smarter assert Eric Blake
2023-07-19 20:27 ` [PULL 08/14] nbd: Consistent typedef usage in header Eric Blake
2023-07-19 20:27 ` [PULL 09/14] nbd/server: Prepare for alternate-size headers Eric Blake
2023-07-19 20:27 ` [PULL 10/14] nbd/server: Refactor to pass full request around Eric Blake
2023-07-19 20:27 ` [PULL 11/14] nbd: s/handle/cookie/ to match NBD spec Eric Blake
2023-07-19 20:27 ` Eric Blake [this message]
2023-07-19 20:27 ` [PULL 13/14] nbd/client: Add safety check on chunk payload length Eric Blake
2023-07-19 20:27 ` [PULL 14/14] nbd: Use enum for various negotiation modes Eric Blake
2023-07-21  9:52 ` [PULL 00/14] NBD patches for 2023-07-19 Peter Maydell

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20230719202736.2675295-28-eblake@redhat.com \
    --to=eblake@redhat.com \
    --cc=hreitz@redhat.com \
    --cc=kwolf@redhat.com \
    --cc=qemu-block@nongnu.org \
    --cc=qemu-devel@nongnu.org \
    --cc=vsementsov@yandex-team.ru \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.