From 7ae79a38a58d71ddd7dd8c6b8a648163ca644e70 Mon Sep 17 00:00:00 2001 From: Shuhei Matsumoto Date: Thu, 2 Dec 2021 04:11:46 +0900 Subject: [PATCH] nvme: Limit spdk_nvme_poll_group_remove() to use only for disconnected qpairs Signed-off-by: Shuhei Matsumoto Change-Id: I3c06c41664ee757423641474141439f9c32fc0b6 Reviewed-on: https://review.spdk.io/gerrit/c/spdk/spdk/+/10671 Community-CI: Broadcom CI Community-CI: Mellanox Build Bot Tested-by: SPDK CI Jenkins Reviewed-by: Aleksey Marchuk Reviewed-by: Monica Kenguva Reviewed-by: Jim Harris --- CHANGELOG.md | 3 ++ include/spdk/nvme.h | 7 +++-- lib/nvme/nvme_transport.c | 30 +++++++++---------- .../nvme/nvme_transport.c/nvme_transport_ut.c | 8 ++--- 4 files changed, 26 insertions(+), 22 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 83fc94da5..975b2ec7f 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -16,6 +16,9 @@ Many APIs are now vectored rather than scalar, meaning they take iovecs instead API `spdk_nvme_trtype_is_fabrics` was added to return existing transport type is fabric or not. +API `spdk_nvme_poll_group_remove` was limited to be available only for a +disconnected qpair in the group. + ### bdev_nvme Added `num_io_queues` to `bdev_nvme_attach_controller` RPC to allow specifying amount diff --git a/include/spdk/nvme.h b/include/spdk/nvme.h index ce0e68cfd..e0e2f3998 100644 --- a/include/spdk/nvme.h +++ b/include/spdk/nvme.h @@ -3,7 +3,7 @@ * * Copyright (c) Intel Corporation. All rights reserved. * Copyright (c) 2019-2021 Mellanox Technologies LTD. All rights reserved. - * Copyright (c) 2021 NVIDIA CORPORATION & AFFILIATES. All rights reserved. + * Copyright (c) 2021, 2022 NVIDIA CORPORATION & AFFILIATES. All rights reserved. * * Redistribution and use in source and binary forms, with or without * modification, are permitted provided that the following conditions @@ -2634,12 +2634,13 @@ struct spdk_nvme_poll_group *spdk_nvme_qpair_get_optimal_poll_group(struct spdk_ int spdk_nvme_poll_group_add(struct spdk_nvme_poll_group *group, struct spdk_nvme_qpair *qpair); /** - * Remove an spdk_nvme_qpair from a poll group. + * Remove a disconnected spdk_nvme_qpair from a poll group. * * \param group The group from which to remove the qpair. * \param qpair The qpair to remove from the poll group. * - * return 0 on success, -ENOENT if the qpair is not found in the group, or -EPROTO on a protocol (transport) specific failure. + * return 0 on success, -ENOENT if the qpair is not found in the group, -EINVAL if the qpair is not + * disconnected in the group, or -EPROTO on a protocol (transport) specific failure. */ int spdk_nvme_poll_group_remove(struct spdk_nvme_poll_group *group, struct spdk_nvme_qpair *qpair); diff --git a/lib/nvme/nvme_transport.c b/lib/nvme/nvme_transport.c index 1b48c4fea..d53fa9867 100644 --- a/lib/nvme/nvme_transport.c +++ b/lib/nvme/nvme_transport.c @@ -4,7 +4,7 @@ * Copyright (c) Intel Corporation. * All rights reserved. * Copyright (c) 2021 Mellanox Technologies LTD. All rights reserved. - * Copyright (c) 2021 NVIDIA CORPORATION & AFFILIATES. All rights reserved. + * Copyright (c) 2021, 2022 NVIDIA CORPORATION & AFFILIATES. All rights reserved. * * Redistribution and use in source and binary forms, with or without * modification, are permitted provided that the following conditions @@ -684,23 +684,23 @@ int nvme_transport_poll_group_remove(struct spdk_nvme_transport_poll_group *tgroup, struct spdk_nvme_qpair *qpair) { - int rc; + int rc __attribute__((unused)); - rc = tgroup->transport->ops.poll_group_remove(tgroup, qpair); - if (rc == 0) { - if (qpair->poll_group_tailq_head == &tgroup->connected_qpairs) { - STAILQ_REMOVE(&tgroup->connected_qpairs, qpair, spdk_nvme_qpair, poll_group_stailq); - } else if (qpair->poll_group_tailq_head == &tgroup->disconnected_qpairs) { - STAILQ_REMOVE(&tgroup->disconnected_qpairs, qpair, spdk_nvme_qpair, poll_group_stailq); - } else { - return -ENOENT; - } - - qpair->poll_group = NULL; - qpair->poll_group_tailq_head = NULL; + if (qpair->poll_group_tailq_head == &tgroup->connected_qpairs) { + return -EINVAL; + } else if (qpair->poll_group_tailq_head != &tgroup->disconnected_qpairs) { + return -ENOENT; } - return rc; + rc = tgroup->transport->ops.poll_group_remove(tgroup, qpair); + assert(rc == 0); + + STAILQ_REMOVE(&tgroup->disconnected_qpairs, qpair, spdk_nvme_qpair, poll_group_stailq); + + qpair->poll_group = NULL; + qpair->poll_group_tailq_head = NULL; + + return 0; } int64_t diff --git a/test/unit/lib/nvme/nvme_transport.c/nvme_transport_ut.c b/test/unit/lib/nvme/nvme_transport.c/nvme_transport_ut.c index fe49243cc..50353e956 100644 --- a/test/unit/lib/nvme/nvme_transport.c/nvme_transport_ut.c +++ b/test/unit/lib/nvme/nvme_transport.c/nvme_transport_ut.c @@ -3,7 +3,7 @@ * * Copyright (c) Intel Corporation. * All rights reserved. - * Copyright (c) 2021 NVIDIA CORPORATION & AFFILIATES. All rights reserved. + * Copyright (c) 2021, 2022 NVIDIA CORPORATION & AFFILIATES. All rights reserved. * * Redistribution and use in source and binary forms, with or without * modification, are permitted provided that the following conditions @@ -209,9 +209,9 @@ test_nvme_transport_poll_group_add_remove(void) STAILQ_INSERT_TAIL(&tgroup.connected_qpairs, &qpair, poll_group_stailq); rc = nvme_transport_poll_group_remove(&tgroup, &qpair); - CU_ASSERT(rc == 0); - CU_ASSERT(qpair.poll_group == NULL); - CU_ASSERT(qpair.poll_group_tailq_head == NULL); + CU_ASSERT(rc == -EINVAL); + + STAILQ_REMOVE(&tgroup.connected_qpairs, &qpair, spdk_nvme_qpair, poll_group_stailq); /* Invalid qpair */ qpair.poll_group_tailq_head = NULL;