On 4/24/2020 10:09 AM, Christoph Hellwig wrote:
On Thu, Apr 23, 2020 at 12:22:27PM +0300, Max Gurtovoy wrote:
diff --git a/drivers/nvme/host/rdma.c b/drivers/nvme/host/rdma.c
index e38f8f7..23cc77e 100644
--- a/drivers/nvme/host/rdma.c
+++ b/drivers/nvme/host/rdma.c
@@ -67,6 +67,9 @@ struct nvme_rdma_request {
struct ib_cqe reg_cqe;
struct nvme_rdma_queue *queue;
struct nvme_rdma_sgl data_sgl;
+ /* Metadata (T10-PI) support */
+ struct nvme_rdma_sgl *md_sgl;
+ bool use_md;
Do we need a use_md flag vs just using md_sgl as a boolean and/or
using blk_integrity_rq?
md_sgl will be used if we'll get a blk request with blk_integrity (memory
domain).
use_md will be responsible for wire domain.
so instead of this bool we can check in any place (after prev commit
changes):
"
if (queue->pi_support && nvme_ns_has_pi(ns))
req->use_md = c.common.opcode == nvme_cmd_write ||
c.common.opcode == nvme_cmd_read;
"
And this is less readable IMO.
It would obviously have to go into a little helper, but I really hate
adding lots of little fields caching easily derived information. There
are a few exception, for example if we really need to not touch too
many cache lines. Do you have a git tree with your current code? That
might be a little easier to follow than the various patches, maybe
I can think of something better.
+ if (blk_integrity_rq(rq)) {
+ memset(req->md_sgl, 0, sizeof(struct nvme_rdma_sgl));
Why do we need this memset?
just good practice we took from drivers/scsi/scsi_lib.c.
It's not a must and I can remove it if needed and test it.
I think (and please double check) that we initialize all three fields
anyway, so the memset should not be needed.
right:
if (first_chunk && nents_first_chunk) {
if (nents <= nents_first_chunk) {
*table->nents = table->orig_nents* = nents;
sg_init_table(table->sgl, nents);
return 0;
}
}
and also in __sg_alloc_table:
*memset(table, 0, sizeof(*table));*
