lkml.org 
[lkml]   [2022]   [Aug]   [17]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    Patch in this message
    /
    From
    Subject[PATCH v2] nvmet-tcp: Don't kmap() pages which can't come from HIGHMEM
    Date
    kmap() is being deprecated in favor of kmap_local_page().[1]

    There are two main problems with kmap(): (1) It comes with an overhead as
    mapping space is restricted and protected by a global lock for
    synchronization and (2) it also requires global TLB invalidation when the
    kmap’s pool wraps and it might block when the mapping space is fully
    utilized until a slot becomes available.

    The pages which will be mapped are allocated in nvmet_tcp_map_data(),
    using the GFP_KERNEL flag. This assures that they cannot come from
    HIGHMEM. This imply that a straight page_address() can replace the kmap()
    of sg_page(sg) in nvmet_tcp_map_pdu_iovec(). As a side effect, we might
    also delete the field "nr_mapped" from struct "nvmet_tcp_cmd" because,
    after removing the kmap() calls, there would be no longer any need of it.

    Therefore, replace the kmap() of sg_page(sg) with a page_address() and
    delete the "nr_mapped" field from "nvmet_tcp_cmd" and instead pass a
    local variable to iov_iter_kvec() from the call site in
    nvmet_tcp_map_pdu_iovec().

    [1] "[PATCH] checkpatch: Add kmap and kmap_atomic to the deprecated
    list" https://lore.kernel.org/all/20220813220034.806698-1-ira.weiny@intel.com/

    Cc: Chaitanya Kulkarni <chaitanyak@nvidia.com>
    Cc: Keith Busch <kbusch@kernel.org>
    Cc: Sagi Grimberg <sagi@grimberg.me>
    Suggested-by: Ira Weiny <ira.weiny@intel.com>
    Signed-off-by: Fabio M. De Francesco <fmdefrancesco@gmail.com>
    ---

    v1->v2: Use a local variable as argument of iov_iter_kvec() instead of
    the removed "nr_mapped" field from struct "nvmet_tcp_cmd". Thanks to
    Sagi and Keith who noticed my mistake.

    Many thanks to Chaitanya, Keith, Sagi, for the answers and the comments on
    the RFC which led to this patch. The RFC is at:
    https://lore.kernel.org/all/20220816091808.23236-1-fmdefrancesco@gmail.com/

    drivers/nvme/target/tcp.c | 28 ++++------------------------
    1 file changed, 4 insertions(+), 24 deletions(-)

    diff --git a/drivers/nvme/target/tcp.c b/drivers/nvme/target/tcp.c
    index dc3b4dc8fe08..5b839f842623 100644
    --- a/drivers/nvme/target/tcp.c
    +++ b/drivers/nvme/target/tcp.c
    @@ -77,7 +77,6 @@ struct nvmet_tcp_cmd {
    u32 pdu_len;
    u32 pdu_recv;
    int sg_idx;
    - int nr_mapped;
    struct msghdr recv_msg;
    struct kvec *iov;
    u32 flags;
    @@ -167,7 +166,6 @@ static const struct nvmet_fabrics_ops nvmet_tcp_ops;
    static void nvmet_tcp_free_cmd(struct nvmet_tcp_cmd *c);
    static void nvmet_tcp_finish_cmd(struct nvmet_tcp_cmd *cmd);
    static void nvmet_tcp_free_cmd_buffers(struct nvmet_tcp_cmd *cmd);
    -static void nvmet_tcp_unmap_pdu_iovec(struct nvmet_tcp_cmd *cmd);

    static inline u16 nvmet_tcp_cmd_tag(struct nvmet_tcp_queue *queue,
    struct nvmet_tcp_cmd *cmd)
    @@ -301,35 +299,21 @@ static int nvmet_tcp_check_ddgst(struct nvmet_tcp_queue *queue, void *pdu)

    static void nvmet_tcp_free_cmd_buffers(struct nvmet_tcp_cmd *cmd)
    {
    - WARN_ON(unlikely(cmd->nr_mapped > 0));
    -
    kfree(cmd->iov);
    sgl_free(cmd->req.sg);
    cmd->iov = NULL;
    cmd->req.sg = NULL;
    }

    -static void nvmet_tcp_unmap_pdu_iovec(struct nvmet_tcp_cmd *cmd)
    -{
    - struct scatterlist *sg;
    - int i;
    -
    - sg = &cmd->req.sg[cmd->sg_idx];
    -
    - for (i = 0; i < cmd->nr_mapped; i++)
    - kunmap(sg_page(&sg[i]));
    -
    - cmd->nr_mapped = 0;
    -}
    -
    static void nvmet_tcp_map_pdu_iovec(struct nvmet_tcp_cmd *cmd)
    {
    struct kvec *iov = cmd->iov;
    struct scatterlist *sg;
    u32 length, offset, sg_offset;
    + int nr_mapped;

    length = cmd->pdu_len;
    - cmd->nr_mapped = DIV_ROUND_UP(length, PAGE_SIZE);
    + nr_mapped = DIV_ROUND_UP(length, PAGE_SIZE);
    offset = cmd->rbytes_done;
    cmd->sg_idx = offset / PAGE_SIZE;
    sg_offset = offset % PAGE_SIZE;
    @@ -338,7 +322,7 @@ static void nvmet_tcp_map_pdu_iovec(struct nvmet_tcp_cmd *cmd)
    while (length) {
    u32 iov_len = min_t(u32, length, sg->length - sg_offset);

    - iov->iov_base = kmap(sg_page(sg)) + sg->offset + sg_offset;
    + iov->iov_base = page_address(sg_page(sg)) + sg->offset + sg_offset;
    iov->iov_len = iov_len;

    length -= iov_len;
    @@ -347,8 +331,7 @@ static void nvmet_tcp_map_pdu_iovec(struct nvmet_tcp_cmd *cmd)
    sg_offset = 0;
    }

    - iov_iter_kvec(&cmd->recv_msg.msg_iter, READ, cmd->iov,
    - cmd->nr_mapped, cmd->pdu_len);
    + iov_iter_kvec(&cmd->recv_msg.msg_iter, READ, cmd->iov, nr_mapped, cmd->pdu_len);
    }

    static void nvmet_tcp_fatal_error(struct nvmet_tcp_queue *queue)
    @@ -1141,7 +1124,6 @@ static int nvmet_tcp_try_recv_data(struct nvmet_tcp_queue *queue)
    cmd->rbytes_done += ret;
    }

    - nvmet_tcp_unmap_pdu_iovec(cmd);
    if (queue->data_digest) {
    nvmet_tcp_prep_recv_ddgst(cmd);
    return 0;
    @@ -1411,7 +1393,6 @@ static void nvmet_tcp_restore_socket_callbacks(struct nvmet_tcp_queue *queue)
    static void nvmet_tcp_finish_cmd(struct nvmet_tcp_cmd *cmd)
    {
    nvmet_req_uninit(&cmd->req);
    - nvmet_tcp_unmap_pdu_iovec(cmd);
    nvmet_tcp_free_cmd_buffers(cmd);
    }

    @@ -1424,7 +1405,6 @@ static void nvmet_tcp_uninit_data_in_cmds(struct nvmet_tcp_queue *queue)
    if (nvmet_tcp_need_data_in(cmd))
    nvmet_req_uninit(&cmd->req);

    - nvmet_tcp_unmap_pdu_iovec(cmd);
    nvmet_tcp_free_cmd_buffers(cmd);
    }

    --
    2.37.1
    \
     
     \ /
      Last update: 2022-08-17 20:47    [W:2.361 / U:0.004 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site