| Message ID | 20190613083819.6998-1-michal.kalderon@marvell.com |
|---|---|
| Headers | show |
| Series | RDMA/qedr: Use the doorbell overflow recovery mechanism for RDMA | expand |
On Thu, 2019-06-13 at 11:38 +0300, Michal Kalderon wrote: > This patch series used the doorbell overflow recovery mechanism > introduced in > commit 36907cd5cd72 ("qed: Add doorbell overflow recovery mechanism") > for rdma ( RoCE and iWARP ) > > rdma-core pull request #493 > > Changes from V2: > - Don't use long-lived kmap. Instead use user-trigger mmap for the > doorbell recovery entries. > - Modify dpi_addr to be denoted with __iomem and avoid redundant > casts > > Changes from V1: > - call kmap to map virtual address into kernel space > - modify db_rec_delete to be void > - remove some cpu_to_le16 that were added to previous patch which are > correct but not related to the overflow recovery mechanism. Will be > submitted as part of a different patch > > > Michal Kalderon (3): > qed*: Change dpi_addr to be denoted with __iomem > RDMA/qedr: Add doorbell overflow recovery support > RDMA/qedr: Add iWARP doorbell recovery support > > drivers/infiniband/hw/qedr/main.c | 2 +- > drivers/infiniband/hw/qedr/qedr.h | 27 +- > drivers/infiniband/hw/qedr/verbs.c | 387 > ++++++++++++++++++++++++----- > drivers/net/ethernet/qlogic/qed/qed_rdma.c | 6 +- > include/linux/qed/qed_rdma_if.h | 2 +- > include/uapi/rdma/qedr-abi.h | 25 ++ > 6 files changed, 378 insertions(+), 71 deletions(-) > Hi Michal, In patch 2 and 3 both, you still have quite a few casts to (u8 __iomem *). Why not just define the struct elements as u8 __iomem * instead of void __iomem * and avoid all the casts?
> From: linux-rdma-owner@vger.kernel.org <linux-rdma- > owner@vger.kernel.org> On Behalf Of Doug Ledford > > On Thu, 2019-06-13 at 11:38 +0300, Michal Kalderon wrote: > > This patch series used the doorbell overflow recovery mechanism > > introduced in commit 36907cd5cd72 ("qed: Add doorbell overflow > > recovery mechanism") for rdma ( RoCE and iWARP ) > > > > rdma-core pull request #493 > > > > Changes from V2: > > - Don't use long-lived kmap. Instead use user-trigger mmap for the > > doorbell recovery entries. > > - Modify dpi_addr to be denoted with __iomem and avoid redundant > > casts > > > > Changes from V1: > > - call kmap to map virtual address into kernel space > > - modify db_rec_delete to be void > > - remove some cpu_to_le16 that were added to previous patch which are > > correct but not related to the overflow recovery mechanism. Will be > > submitted as part of a different patch > > > > > > Michal Kalderon (3): > > qed*: Change dpi_addr to be denoted with __iomem > > RDMA/qedr: Add doorbell overflow recovery support > > RDMA/qedr: Add iWARP doorbell recovery support > > > > drivers/infiniband/hw/qedr/main.c | 2 +- > > drivers/infiniband/hw/qedr/qedr.h | 27 +- > > drivers/infiniband/hw/qedr/verbs.c | 387 > > ++++++++++++++++++++++++----- > > drivers/net/ethernet/qlogic/qed/qed_rdma.c | 6 +- > > include/linux/qed/qed_rdma_if.h | 2 +- > > include/uapi/rdma/qedr-abi.h | 25 ++ > > 6 files changed, 378 insertions(+), 71 deletions(-) > > > > Hi Michal, > > In patch 2 and 3 both, you still have quite a few casts to (u8 __iomem *). > Why not just define the struct elements as u8 __iomem * instead of void > __iomem * and avoid all the casts? > Hi Doug, Thanks for the review. The remaining casts are due to pointer arithmetic and not variable assignments as before. Removing the cast entirely will require quite a lot of changes in qed and in rdma-core which I would be happy to avoid at this time. Please reconsider, Thanks again Michal > -- > Doug Ledford <dledford@redhat.com> > GPG KeyID: B826A3330E572FDD > Fingerprint = AE6B 1BDA 122B 23B4 265B 1274 B826 A333 0E57 2FDD
On Fri, Jun 21, 2019 at 07:49:39PM +0000, Michal Kalderon wrote: > > From: linux-rdma-owner@vger.kernel.org <linux-rdma- > > owner@vger.kernel.org> On Behalf Of Doug Ledford > > > > On Thu, 2019-06-13 at 11:38 +0300, Michal Kalderon wrote: > > > This patch series used the doorbell overflow recovery mechanism > > > introduced in commit 36907cd5cd72 ("qed: Add doorbell overflow > > > recovery mechanism") for rdma ( RoCE and iWARP ) > > > > > > rdma-core pull request #493 > > > > > > Changes from V2: > > > - Don't use long-lived kmap. Instead use user-trigger mmap for the > > > doorbell recovery entries. > > > - Modify dpi_addr to be denoted with __iomem and avoid redundant > > > casts > > > > > > Changes from V1: > > > - call kmap to map virtual address into kernel space > > > - modify db_rec_delete to be void > > > - remove some cpu_to_le16 that were added to previous patch which are > > > correct but not related to the overflow recovery mechanism. Will be > > > submitted as part of a different patch > > > > > > > > > Michal Kalderon (3): > > > qed*: Change dpi_addr to be denoted with __iomem > > > RDMA/qedr: Add doorbell overflow recovery support > > > RDMA/qedr: Add iWARP doorbell recovery support > > > > > > drivers/infiniband/hw/qedr/main.c | 2 +- > > > drivers/infiniband/hw/qedr/qedr.h | 27 +- > > > drivers/infiniband/hw/qedr/verbs.c | 387 > > > ++++++++++++++++++++++++----- > > > drivers/net/ethernet/qlogic/qed/qed_rdma.c | 6 +- > > > include/linux/qed/qed_rdma_if.h | 2 +- > > > include/uapi/rdma/qedr-abi.h | 25 ++ > > > 6 files changed, 378 insertions(+), 71 deletions(-) > > > > > > > Hi Michal, > > > > In patch 2 and 3 both, you still have quite a few casts to (u8 __iomem *). > > Why not just define the struct elements as u8 __iomem * instead of void > > __iomem * and avoid all the casts? > > > Hi Doug, > > Thanks for the review. The remaining casts are due to pointer arithmetic and not variable assignments > as before. Removing the cast entirely will require quite a lot of changes in qed and in rdma-core > which I would be happy to avoid at this time. In linux pointer math on a void * acts the same as a u8 so you should never need to cast a void * to a u8 just to do math? Jason
> From: Jason Gunthorpe <jgg@ziepe.ca> > Sent: Friday, June 21, 2019 10:58 PM > > External Email > > ---------------------------------------------------------------------- > On Fri, Jun 21, 2019 at 07:49:39PM +0000, Michal Kalderon wrote: > > > From: linux-rdma-owner@vger.kernel.org <linux-rdma- > > > owner@vger.kernel.org> On Behalf Of Doug Ledford > > > > > > On Thu, 2019-06-13 at 11:38 +0300, Michal Kalderon wrote: > > > > This patch series used the doorbell overflow recovery mechanism > > > > introduced in commit 36907cd5cd72 ("qed: Add doorbell overflow > > > > recovery mechanism") for rdma ( RoCE and iWARP ) > > > > > > > > rdma-core pull request #493 > > > > > > > > Changes from V2: > > > > - Don't use long-lived kmap. Instead use user-trigger mmap for the > > > > doorbell recovery entries. > > > > - Modify dpi_addr to be denoted with __iomem and avoid redundant > > > > casts > > > > > > > > Changes from V1: > > > > - call kmap to map virtual address into kernel space > > > > - modify db_rec_delete to be void > > > > - remove some cpu_to_le16 that were added to previous patch which > are > > > > correct but not related to the overflow recovery mechanism. Will be > > > > submitted as part of a different patch > > > > > > > > > > > > Michal Kalderon (3): > > > > qed*: Change dpi_addr to be denoted with __iomem > > > > RDMA/qedr: Add doorbell overflow recovery support > > > > RDMA/qedr: Add iWARP doorbell recovery support > > > > > > > > drivers/infiniband/hw/qedr/main.c | 2 +- > > > > drivers/infiniband/hw/qedr/qedr.h | 27 +- > > > > drivers/infiniband/hw/qedr/verbs.c | 387 > > > > ++++++++++++++++++++++++----- > > > > drivers/net/ethernet/qlogic/qed/qed_rdma.c | 6 +- > > > > include/linux/qed/qed_rdma_if.h | 2 +- > > > > include/uapi/rdma/qedr-abi.h | 25 ++ > > > > 6 files changed, 378 insertions(+), 71 deletions(-) > > > > > > > > > > Hi Michal, > > > > > > In patch 2 and 3 both, you still have quite a few casts to (u8 __iomem *). > > > Why not just define the struct elements as u8 __iomem * instead of > > > void __iomem * and avoid all the casts? > > > > > Hi Doug, > > > > Thanks for the review. The remaining casts are due to pointer > > arithmetic and not variable assignments as before. Removing the cast > > entirely will require quite a lot of changes in qed and in rdma-core which I > would be happy to avoid at this time. > > In linux pointer math on a void * acts the same as a u8 so you should never > need to cast a void * to a u8 just to do math? > Ok, thanks. Sent v4. Michal > Jason
This patch series used the doorbell overflow recovery mechanism introduced in commit 36907cd5cd72 ("qed: Add doorbell overflow recovery mechanism") for rdma ( RoCE and iWARP ) rdma-core pull request #493 Changes from V2: - Don't use long-lived kmap. Instead use user-trigger mmap for the doorbell recovery entries. - Modify dpi_addr to be denoted with __iomem and avoid redundant casts Changes from V1: - call kmap to map virtual address into kernel space - modify db_rec_delete to be void - remove some cpu_to_le16 that were added to previous patch which are correct but not related to the overflow recovery mechanism. Will be submitted as part of a different patch Michal Kalderon (3): qed*: Change dpi_addr to be denoted with __iomem RDMA/qedr: Add doorbell overflow recovery support RDMA/qedr: Add iWARP doorbell recovery support drivers/infiniband/hw/qedr/main.c | 2 +- drivers/infiniband/hw/qedr/qedr.h | 27 +- drivers/infiniband/hw/qedr/verbs.c | 387 ++++++++++++++++++++++++----- drivers/net/ethernet/qlogic/qed/qed_rdma.c | 6 +- include/linux/qed/qed_rdma_if.h | 2 +- include/uapi/rdma/qedr-abi.h | 25 ++ 6 files changed, 378 insertions(+), 71 deletions(-)