| Message ID | 565DE487.2010803@sandisk.com (mailing list archive) |
|---|---|
| State | Accepted |
| Headers | show |
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 <bart.vanassche@sandisk.com> > Cc: stable <stable@vger.kernel.org> # v4.3+ > Cc: Sagi Grimberg <sagig@mellanox.com> > Cc: Christoph Hellwig <hch@lst.de> > Cc: Sebastian Parschauer <sebastian.riemer@profitbricks.com> > --- > 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 majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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 <bart.vanassche@sandisk.com> >> Cc: stable <stable@vger.kernel.org> # v4.3+ >> Cc: Sagi Grimberg <sagig@mellanox.com> >> Cc: Christoph Hellwig <hch@lst.de> >> Cc: Sebastian Parschauer <sebastian.riemer@profitbricks.com> >> --- >> 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 majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
>>> 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 majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Looks good,
Reviewed-by: Christoph Hellwig <hch@lst.de>
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
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);
Detected by sparse. Fixes: commit 330179f2fa93 ("IB/srp: Register the indirect data buffer descriptor") Signed-off-by: Bart Van Assche <bart.vanassche@sandisk.com> Cc: stable <stable@vger.kernel.org> # v4.3+ Cc: Sagi Grimberg <sagig@mellanox.com> Cc: Christoph Hellwig <hch@lst.de> Cc: Sebastian Parschauer <sebastian.riemer@profitbricks.com> --- drivers/infiniband/ulp/srp/ib_srp.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)