Re: [PATCH 4/6] IB/srp: Fix indirect data buffer rkey endianness

[Sagi Grimberg]

diff --git a/drivers/infiniband/ulp/srp/ib_srp.c
b/drivers/infiniband/ulp/srp/ib_srp.c
index 72fac20..fac1423 100644
--- a/drivers/infiniband/ulp/srp/ib_srp.c
+++ b/drivers/infiniband/ulp/srp/ib_srp.c
@@ -1662,7 +1662,7 @@ static int srp_map_data(struct scsi_cmnd *scmnd,
struct srp_rdma_ch *ch,
  return ret;
  req->nmdesc++;
  } else {
-idb_rkey = target->global_mr->rkey;
+idb_rkey = cpu_to_be32(target->global_mr->rkey);
  }


Wouldn't it make more sense to define idb_rkey to be u32 and change
endianness when assigning it to the indirect desc?


Hello Sagi,

That's possible, but that would cause the endianness of the indirect
data buffer rkey to be changed three times - a first time in
srp_map_desc(), a second time in srp_map_idb() and a third time in
srp_map_data(). Hence the choice to fix the IDB rkey via the above patch.


Fine with me.
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 4/6] IB/srp: Fix indirect data buffer rkey endianness

[Christoph Hellwig]

Looks good,

Reviewed-by: Christoph Hellwig 
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 4/6] IB/srp: Fix indirect data buffer rkey endianness

[Bart Van Assche]

Detected by sparse.

Fixes: commit 330179f2fa93 ("IB/srp: Register the indirect data buffer 
descriptor")
Signed-off-by: Bart Van Assche 
Cc: stable  # v4.3+
Cc: Sagi Grimberg 
Cc: Christoph Hellwig 
Cc: Sebastian Parschauer 
---
 drivers/infiniband/ulp/srp/ib_srp.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/infiniband/ulp/srp/ib_srp.c 
b/drivers/infiniband/ulp/srp/ib_srp.c
index 72fac20..fac1423 100644
--- a/drivers/infiniband/ulp/srp/ib_srp.c
+++ b/drivers/infiniband/ulp/srp/ib_srp.c
@@ -1662,7 +1662,7 @@ static int srp_map_data(struct scsi_cmnd *scmnd, struct 
srp_rdma_ch *ch,
return ret;
req->nmdesc++;
} else {
-   idb_rkey = target->global_mr->rkey;
+   idb_rkey = cpu_to_be32(target->global_mr->rkey);
}
 
indirect_hdr->table_desc.va = cpu_to_be64(req->indirect_dma_addr);
-- 
2.1.4

--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 4/6] IB/srp: Fix indirect data buffer rkey endianness

[Sagi Grimberg]

On 01/12/2015 20:18, Bart Van Assche wrote:

Detected by sparse.

Fixes: commit 330179f2fa93 ("IB/srp: Register the indirect data buffer 
descriptor")
Signed-off-by: Bart Van Assche 
Cc: stable  # v4.3+
Cc: Sagi Grimberg 
Cc: Christoph Hellwig 
Cc: Sebastian Parschauer 
---
  drivers/infiniband/ulp/srp/ib_srp.c | 2 +-
  1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/infiniband/ulp/srp/ib_srp.c 
b/drivers/infiniband/ulp/srp/ib_srp.c
index 72fac20..fac1423 100644
--- a/drivers/infiniband/ulp/srp/ib_srp.c
+++ b/drivers/infiniband/ulp/srp/ib_srp.c
@@ -1662,7 +1662,7 @@ static int srp_map_data(struct scsi_cmnd *scmnd, struct 
srp_rdma_ch *ch,
return ret;
req->nmdesc++;
} else {
-   idb_rkey = target->global_mr->rkey;
+   idb_rkey = cpu_to_be32(target->global_mr->rkey);
}


Wouldn't it make more sense to define idb_rkey to be u32 and change
endianness when assigning it to the indirect desc?
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 4/6] IB/srp: Fix indirect data buffer rkey endianness

[Bart Van Assche]

On 12/01/2015 10:37 AM, Sagi Grimberg wrote:

On 01/12/2015 20:18, Bart Van Assche wrote:

Detected by sparse.

Fixes: commit 330179f2fa93 ("IB/srp: Register the indirect data buffer
descriptor")
Signed-off-by: Bart Van Assche 
Cc: stable  # v4.3+
Cc: Sagi Grimberg 
Cc: Christoph Hellwig 
Cc: Sebastian Parschauer 
---
  drivers/infiniband/ulp/srp/ib_srp.c | 2 +-
  1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/infiniband/ulp/srp/ib_srp.c
b/drivers/infiniband/ulp/srp/ib_srp.c
index 72fac20..fac1423 100644
--- a/drivers/infiniband/ulp/srp/ib_srp.c
+++ b/drivers/infiniband/ulp/srp/ib_srp.c
@@ -1662,7 +1662,7 @@ static int srp_map_data(struct scsi_cmnd *scmnd,
struct srp_rdma_ch *ch,
  return ret;
  req->nmdesc++;
  } else {
-idb_rkey = target->global_mr->rkey;
+idb_rkey = cpu_to_be32(target->global_mr->rkey);
  }


Wouldn't it make more sense to define idb_rkey to be u32 and change
endianness when assigning it to the indirect desc?


Hello Sagi,

That's possible, but that would cause the endianness of the indirect 
data buffer rkey to be changed three times - a first time in 
srp_map_desc(), a second time in srp_map_idb() and a third time in 
srp_map_data(). Hence the choice to fix the IDB rkey via the above patch.


Bart.
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html