Message ID | 20191006155955.31445-4-leon@kernel.org |
---|---|
State | Not Applicable |
Delegated to: | David Miller |
Headers | show |
Series | Optimize SGL registration | expand |
> /* > - * Check if the device might use memory registration. This is currently only > - * true for iWarp devices. In the future we can hopefully fine tune this based > - * on HCA driver input. > + * Check if the device might use memory registration. > */ Please keep the important bits of this comments instead of just removing them. > { > @@ -30,6 +28,8 @@ static inline bool rdma_rw_can_use_mr(struct ib_device *dev, u8 port_num) > return true; > if (unlikely(rdma_rw_force_mr)) > return true; > + if (dev->attrs.max_sgl_rd) > + return true; Logically this should go before the rdma_rw_force_mr check. > if (unlikely(rdma_rw_force_mr)) > return true; > + if (dev->attrs.max_sgl_rd && dir == DMA_FROM_DEVICE > + && dma_nents > dev->attrs.max_sgl_rd) Wrong indendation. The && belongs on the first line. And again, this logically belongs before the rdma_rw_force_mr check.
On Sun, Oct 06, 2019 at 11:58:25PM -0700, Christoph Hellwig wrote: > > /* > > - * Check if the device might use memory registration. This is currently only > > - * true for iWarp devices. In the future we can hopefully fine tune this based > > - * on HCA driver input. > > + * Check if the device might use memory registration. > > */ > > Please keep the important bits of this comments instead of just > removing them. > > > { > > @@ -30,6 +28,8 @@ static inline bool rdma_rw_can_use_mr(struct ib_device *dev, u8 port_num) > > return true; > > if (unlikely(rdma_rw_force_mr)) > > return true; > > + if (dev->attrs.max_sgl_rd) > > + return true; > > Logically this should go before the rdma_rw_force_mr check. > > > if (unlikely(rdma_rw_force_mr)) > > return true; > > + if (dev->attrs.max_sgl_rd && dir == DMA_FROM_DEVICE > > + && dma_nents > dev->attrs.max_sgl_rd) > > Wrong indendation. The && belongs on the first line. And again, this > logically belongs before the rdma_rw_force_mr check. I'll fix. Thanks
On 10/7/2019 10:54 AM, Leon Romanovsky wrote: > On Sun, Oct 06, 2019 at 11:58:25PM -0700, Christoph Hellwig wrote: >>> /* >>> - * Check if the device might use memory registration. This is currently only >>> - * true for iWarp devices. In the future we can hopefully fine tune this based >>> - * on HCA driver input. >>> + * Check if the device might use memory registration. >>> */ >> Please keep the important bits of this comments instead of just >> removing them. >> >>> { >>> @@ -30,6 +28,8 @@ static inline bool rdma_rw_can_use_mr(struct ib_device *dev, u8 port_num) >>> return true; >>> if (unlikely(rdma_rw_force_mr)) >>> return true; >>> + if (dev->attrs.max_sgl_rd) >>> + return true; >> Logically this should go before the rdma_rw_force_mr check. >> >>> if (unlikely(rdma_rw_force_mr)) >>> return true; >>> + if (dev->attrs.max_sgl_rd && dir == DMA_FROM_DEVICE >>> + && dma_nents > dev->attrs.max_sgl_rd) >> Wrong indendation. The && belongs on the first line. And again, this >> logically belongs before the rdma_rw_force_mr check. > I'll fix. > > Thanks The above comments looks reasonable. Nice optimization Yamin.
diff --git a/drivers/infiniband/core/rw.c b/drivers/infiniband/core/rw.c index 5337393d4dfe..ecff40efcb88 100644 --- a/drivers/infiniband/core/rw.c +++ b/drivers/infiniband/core/rw.c @@ -20,9 +20,7 @@ module_param_named(force_mr, rdma_rw_force_mr, bool, 0); MODULE_PARM_DESC(force_mr, "Force usage of MRs for RDMA READ/WRITE operations"); /* - * Check if the device might use memory registration. This is currently only - * true for iWarp devices. In the future we can hopefully fine tune this based - * on HCA driver input. + * Check if the device might use memory registration. */ static inline bool rdma_rw_can_use_mr(struct ib_device *dev, u8 port_num) { @@ -30,6 +28,8 @@ static inline bool rdma_rw_can_use_mr(struct ib_device *dev, u8 port_num) return true; if (unlikely(rdma_rw_force_mr)) return true; + if (dev->attrs.max_sgl_rd) + return true; return false; } @@ -37,9 +37,6 @@ static inline bool rdma_rw_can_use_mr(struct ib_device *dev, u8 port_num) * Check if the device will use memory registration for this RW operation. * We currently always use memory registrations for iWarp RDMA READs, and * have a debug option to force usage of MRs. - * - * XXX: In the future we can hopefully fine tune this based on HCA driver - * input. */ static inline bool rdma_rw_io_needs_mr(struct ib_device *dev, u8 port_num, enum dma_data_direction dir, int dma_nents) @@ -48,6 +45,9 @@ static inline bool rdma_rw_io_needs_mr(struct ib_device *dev, u8 port_num, return true; if (unlikely(rdma_rw_force_mr)) return true; + if (dev->attrs.max_sgl_rd && dir == DMA_FROM_DEVICE + && dma_nents > dev->attrs.max_sgl_rd) + return true; return false; }