Skip to content

Commit 5bfaba2

Browse files
xp4ns3Christoph Hellwig
authored andcommitted
nvmet-tcp: don't map pages which can't come from HIGHMEM
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. In addition, there is no reason to use a kvec for the command receive data buffers iovec, use a bio_vec instead and let iov_iter handle the buffer mapping and data copy. Test with blktests on a QEMU/KVM x86_32 VM, 6GB RAM, booting a kernel with HIGHMEM64GB enabled. [1] "[PATCH] checkpatch: Add kmap and kmap_atomic to the deprecated list" https://lore.kernel.org/all/[email protected]/ Cc: Chaitanya Kulkarni <[email protected]> Cc: Keith Busch <[email protected]> Suggested-by: Ira Weiny <[email protected]> Signed-off-by: Fabio M. De Francesco <[email protected]> Suggested-by: Christoph Hellwig <[email protected]> Suggested-by: Al Viro <[email protected]> [sagi: added bio_vec plus minor naming changes] Signed-off-by: Sagi Grimberg <[email protected]> Signed-off-by: Christoph Hellwig <[email protected]>
1 parent c4c22c5 commit 5bfaba2

File tree

1 file changed

+13
-31
lines changed

1 file changed

+13
-31
lines changed

drivers/nvme/target/tcp.c

Lines changed: 13 additions & 31 deletions
Original file line numberDiff line numberDiff line change
@@ -77,9 +77,8 @@ struct nvmet_tcp_cmd {
7777
u32 pdu_len;
7878
u32 pdu_recv;
7979
int sg_idx;
80-
int nr_mapped;
8180
struct msghdr recv_msg;
82-
struct kvec *iov;
81+
struct bio_vec *iov;
8382
u32 flags;
8483

8584
struct list_head entry;
@@ -167,7 +166,6 @@ static const struct nvmet_fabrics_ops nvmet_tcp_ops;
167166
static void nvmet_tcp_free_cmd(struct nvmet_tcp_cmd *c);
168167
static void nvmet_tcp_finish_cmd(struct nvmet_tcp_cmd *cmd);
169168
static void nvmet_tcp_free_cmd_buffers(struct nvmet_tcp_cmd *cmd);
170-
static void nvmet_tcp_unmap_pdu_iovec(struct nvmet_tcp_cmd *cmd);
171169

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

302300
static void nvmet_tcp_free_cmd_buffers(struct nvmet_tcp_cmd *cmd)
303301
{
304-
WARN_ON(unlikely(cmd->nr_mapped > 0));
305-
306302
kfree(cmd->iov);
307303
sgl_free(cmd->req.sg);
308304
cmd->iov = NULL;
309305
cmd->req.sg = NULL;
310306
}
311307

312-
static void nvmet_tcp_unmap_pdu_iovec(struct nvmet_tcp_cmd *cmd)
313-
{
314-
struct scatterlist *sg;
315-
int i;
316-
317-
sg = &cmd->req.sg[cmd->sg_idx];
318-
319-
for (i = 0; i < cmd->nr_mapped; i++)
320-
kunmap(sg_page(&sg[i]));
321-
322-
cmd->nr_mapped = 0;
323-
}
324-
325-
static void nvmet_tcp_map_pdu_iovec(struct nvmet_tcp_cmd *cmd)
308+
static void nvmet_tcp_build_pdu_iovec(struct nvmet_tcp_cmd *cmd)
326309
{
327-
struct kvec *iov = cmd->iov;
310+
struct bio_vec *iov = cmd->iov;
328311
struct scatterlist *sg;
329312
u32 length, offset, sg_offset;
313+
int nr_pages;
330314

331315
length = cmd->pdu_len;
332-
cmd->nr_mapped = DIV_ROUND_UP(length, PAGE_SIZE);
316+
nr_pages = DIV_ROUND_UP(length, PAGE_SIZE);
333317
offset = cmd->rbytes_done;
334318
cmd->sg_idx = offset / PAGE_SIZE;
335319
sg_offset = offset % PAGE_SIZE;
@@ -338,17 +322,18 @@ static void nvmet_tcp_map_pdu_iovec(struct nvmet_tcp_cmd *cmd)
338322
while (length) {
339323
u32 iov_len = min_t(u32, length, sg->length - sg_offset);
340324

341-
iov->iov_base = kmap(sg_page(sg)) + sg->offset + sg_offset;
342-
iov->iov_len = iov_len;
325+
iov->bv_page = sg_page(sg);
326+
iov->bv_len = sg->length;
327+
iov->bv_offset = sg->offset + sg_offset;
343328

344329
length -= iov_len;
345330
sg = sg_next(sg);
346331
iov++;
347332
sg_offset = 0;
348333
}
349334

350-
iov_iter_kvec(&cmd->recv_msg.msg_iter, READ, cmd->iov,
351-
cmd->nr_mapped, cmd->pdu_len);
335+
iov_iter_bvec(&cmd->recv_msg.msg_iter, READ, cmd->iov,
336+
nr_pages, cmd->pdu_len);
352337
}
353338

354339
static void nvmet_tcp_fatal_error(struct nvmet_tcp_queue *queue)
@@ -926,7 +911,7 @@ static void nvmet_tcp_handle_req_failure(struct nvmet_tcp_queue *queue,
926911
}
927912

928913
queue->rcv_state = NVMET_TCP_RECV_DATA;
929-
nvmet_tcp_map_pdu_iovec(cmd);
914+
nvmet_tcp_build_pdu_iovec(cmd);
930915
cmd->flags |= NVMET_TCP_F_INIT_FAILED;
931916
}
932917

@@ -952,7 +937,7 @@ static int nvmet_tcp_handle_h2c_data_pdu(struct nvmet_tcp_queue *queue)
952937

953938
cmd->pdu_len = le32_to_cpu(data->data_length);
954939
cmd->pdu_recv = 0;
955-
nvmet_tcp_map_pdu_iovec(cmd);
940+
nvmet_tcp_build_pdu_iovec(cmd);
956941
queue->cmd = cmd;
957942
queue->rcv_state = NVMET_TCP_RECV_DATA;
958943

@@ -1021,7 +1006,7 @@ static int nvmet_tcp_done_recv_pdu(struct nvmet_tcp_queue *queue)
10211006
if (nvmet_tcp_need_data_in(queue->cmd)) {
10221007
if (nvmet_tcp_has_inline_data(queue->cmd)) {
10231008
queue->rcv_state = NVMET_TCP_RECV_DATA;
1024-
nvmet_tcp_map_pdu_iovec(queue->cmd);
1009+
nvmet_tcp_build_pdu_iovec(queue->cmd);
10251010
return 0;
10261011
}
10271012
/* send back R2T */
@@ -1141,7 +1126,6 @@ static int nvmet_tcp_try_recv_data(struct nvmet_tcp_queue *queue)
11411126
cmd->rbytes_done += ret;
11421127
}
11431128

1144-
nvmet_tcp_unmap_pdu_iovec(cmd);
11451129
if (queue->data_digest) {
11461130
nvmet_tcp_prep_recv_ddgst(cmd);
11471131
return 0;
@@ -1411,7 +1395,6 @@ static void nvmet_tcp_restore_socket_callbacks(struct nvmet_tcp_queue *queue)
14111395
static void nvmet_tcp_finish_cmd(struct nvmet_tcp_cmd *cmd)
14121396
{
14131397
nvmet_req_uninit(&cmd->req);
1414-
nvmet_tcp_unmap_pdu_iovec(cmd);
14151398
nvmet_tcp_free_cmd_buffers(cmd);
14161399
}
14171400

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

1427-
nvmet_tcp_unmap_pdu_iovec(cmd);
14281410
nvmet_tcp_free_cmd_buffers(cmd);
14291411
}
14301412

0 commit comments

Comments
 (0)