RE: Re: [PATCH v3] RDMA/siw: Pass a pointer to virt_to_page()

* RE: Re: [PATCH v3] RDMA/siw: Pass a pointer to virt_to_page()
@ 2022-09-05 12:02 Bernard Metzler
  2022-09-05 12:07 ` Linus Walleij
  0 siblings, 1 reply; 3+ messages in thread
From: Bernard Metzler @ 2022-09-05 12:02 UTC (permalink / raw)
  To: Leon Romanovsky, Linus Walleij; +Cc: Jason Gunthorpe, linux-rdma



> -----Original Message-----
> From: Leon Romanovsky <leonro@nvidia.com>
> Sent: Sunday, 4 September 2022 09:23
> To: Linus Walleij <linus.walleij@linaro.org>
> Cc: Jason Gunthorpe <jgg@nvidia.com>; Bernard Metzler
> <BMT@zurich.ibm.com>; linux-rdma@vger.kernel.org
> Subject: [EXTERNAL] Re: [PATCH v3] RDMA/siw: Pass a pointer to
> virt_to_page()
> 
> On Fri, Sep 02, 2022 at 11:59:18PM +0200, Linus Walleij wrote:
> > Functions that work on a pointer to virtual memory such as
> > virt_to_pfn() and users of that function such as
> > virt_to_page() are supposed to pass a pointer to virtual
> > memory, ideally a (void *) or other pointer. However since
> > many architectures implement virt_to_pfn() as a macro,
> > this function becomes polymorphic and accepts both a
> > (unsigned long) and a (void *).
> >
> > If we instead implement a proper virt_to_pfn(void *addr)
> > function the following happens (occurred on arch/arm):
> >
> > drivers/infiniband/sw/siw/siw_qp_tx.c:32:23: warning: incompatible
> >   integer to pointer conversion passing 'dma_addr_t' (aka 'unsigned
> int')
> >   to parameter of type 'const void *' [-Wint-conversion]
> > drivers/infiniband/sw/siw/siw_qp_tx.c:32:37: warning: passing argument
> >   1 of 'virt_to_pfn' makes pointer from integer without a cast
> >   [-Wint-conversion]
> > drivers/infiniband/sw/siw/siw_qp_tx.c:538:36: warning: incompatible
> >   integer to pointer conversion passing 'unsigned long long'
> >   to parameter of type 'const void *' [-Wint-conversion]
> >
> > Fix this with an explicit cast. In one case where the SIW
> > SGE uses an unaligned u64 we need a double cast modifying the
> > virtual address (va) to a platform-specific uintptr_t before
> > casting to a (void *).
> >
> > Fixes: b9be6f18cf9e ("rdma/siw: transmit path")
> > Cc: linux-rdma@vger.kernel.org
> > Signed-off-by: Linus Walleij <linus.walleij@linaro.org>
> > ---
> > ChangeLog v2->v3:
> > - Add Fixes: tag.
> > ChangeLog v1->v2:
> > - Change the local va variable to be uintptr_t, avoiding
> >   double casts in two spots.
> > ---
> >  drivers/infiniband/sw/siw/siw_qp_tx.c | 18 ++++++++++++++----
> >  1 file changed, 14 insertions(+), 4 deletions(-)
> >
> 
> Thanks, applied.

Thanks Linus, thanks Leon. I looked at this only today.

Can we easily fix the two line wraps introduced by this
patch? Without sending an explicit patch on top -- I'd
suggest adding just two line breaks to it. I'd be happy
to see siw code continues to adhere to the 80 char's
per line style.

Thanks very much!
Bernard.

diff --git a/drivers/infiniband/sw/siw/siw_qp_tx.c b/drivers/infiniband/sw/siw/siw_qp_tx.c
index 7d47b521070b..00137dd0223c 100644
--- a/drivers/infiniband/sw/siw/siw_qp_tx.c
+++ b/drivers/infiniband/sw/siw/siw_qp_tx.c
@@ -537,7 +537,8 @@ static int siw_tx_hdt(struct siw_iwarp_tx *c_tx, struct socket *s)
					 * Cast to an uintptr_t to preserve all 64 bits
					 * in sge->laddr.
					 */
-					uintptr_t va = (uintptr_t)(sge->laddr + sge_off);
+					uintptr_t va =
+						(uintptr_t)(sge->laddr + sge_off);
 
					/*
					 * virt_to_page() takes a (void *) pointer
@@ -545,7 +546,8 @@ static int siw_tx_hdt(struct siw_iwarp_tx *c_tx, struct socket *s)
					 * bits on a 64 bit platform and 32 bits on a
					 * 32 bit platform.
					 */
-					page_array[seg] = virt_to_page((void *)(va & PAGE_MASK));
+					page_array[seg] =
+						virt_to_page((void *)(va & PAGE_MASK));
					if (do_crc)
						crypto_shash_update(
							c_tx->mpa_crc_hd,


^ permalink raw reply related	[flat|nested] 3+ messages in thread