netdev.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 0/8] Fix minor address type errors
@ 2013-10-27 21:47 Ben Hutchings
  2013-10-27 21:51 ` [PATCH 2/8] farsync: Fix confusion about DMA address and buffer offset types Ben Hutchings
  2013-10-27 21:54 ` [PATCH 7/8] rds: Pass pointers to virt_to_page(), not integers Ben Hutchings
  0 siblings, 2 replies; 8+ messages in thread
From: Ben Hutchings @ 2013-10-27 21:47 UTC (permalink / raw)
  To: LKML
  Cc: linux-scsi, dri-devel-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW,
	linux-rdma-u79uwXL29TY76Z2rM5mHXA, netdev-u79uwXL29TY76Z2rM5mHXA,
	rds-devel-N0ozoZBvEnrZJqsBc5GL+g

[-- Attachment #1: Type: text/plain, Size: 1275 bytes --]

Various bits of code are mixing making assumptions about the size of
dma_addr_t or resource_size_t, or mixing up pointer and integer types.

All these fixes are based on compiler warnings and so far as I can see
the bugs are practically harmless.

Ben.

Ben Hutchings (8):
  IB/cxgb4: Fix formatting of physical address
  farsync: Fix confusion about DMA address and buffer offset types
  drm: Do not include page offset in argument to virt_to_page()
  drm: Pass pointers to virt_to_page()
  [SCSI] tgt: Pass pointers to virt_to_page(), not integers
  uio: Pass pointers to virt_to_page(), not integers
  rds: Pass pointers to virt_to_page(), not integers
  [SCSI] pmcraid: Pass pointers to access_ok(), not integers

 drivers/gpu/drm/drm_pci.c            |  4 ++--
 drivers/gpu/drm/drm_vm.c             |  2 +-
 drivers/infiniband/hw/cxgb4/device.c |  4 ++--
 drivers/net/wan/farsync.c            | 19 +++++++------------
 drivers/scsi/pmcraid.c               |  3 ++-
 drivers/scsi/scsi_tgt_if.c           |  2 +-
 drivers/uio/uio.c                    |  6 ++++--
 net/rds/message.c                    |  2 +-
 8 files changed, 20 insertions(+), 22 deletions(-)


-- 
Ben Hutchings
If at first you don't succeed, you're doing about average.

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 828 bytes --]

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

* [PATCH 2/8] farsync: Fix confusion about DMA address and buffer offset types
  2013-10-27 21:47 [PATCH 0/8] Fix minor address type errors Ben Hutchings
@ 2013-10-27 21:51 ` Ben Hutchings
  2013-10-28  4:26   ` David Miller
  2013-10-27 21:54 ` [PATCH 7/8] rds: Pass pointers to virt_to_page(), not integers Ben Hutchings
  1 sibling, 1 reply; 8+ messages in thread
From: Ben Hutchings @ 2013-10-27 21:51 UTC (permalink / raw)
  To: Kevin Curtis; +Cc: LKML, netdev

[-- Attachment #1: Type: text/plain, Size: 2809 bytes --]

Use dma_addr_t for DMA address parameters and u32 for shared memory
offset parameters.

Do not assume that dma_addr_t is the same as unsigned long; it will
not be in PAE configurations.  Truncate DMA addresses to 32 bits when
printing them.  This is OK because the DMA mask for this device is
32-bit (per default).

Compile-tested only.

Signed-off-by: Ben Hutchings <ben@decadent.org.uk>
---
 drivers/net/wan/farsync.c | 19 +++++++------------
 1 file changed, 7 insertions(+), 12 deletions(-)

diff --git a/drivers/net/wan/farsync.c b/drivers/net/wan/farsync.c
index 3f0c4f2..4b36676 100644
--- a/drivers/net/wan/farsync.c
+++ b/drivers/net/wan/farsync.c
@@ -886,15 +886,13 @@ fst_rx_dma_complete(struct fst_card_info *card, struct fst_port_info *port,
  *      Receive a frame through the DMA
  */
 static inline void
-fst_rx_dma(struct fst_card_info *card, dma_addr_t skb,
-	   dma_addr_t mem, int len)
+fst_rx_dma(struct fst_card_info *card, dma_addr_t skb, u32 mem, int len)
 {
 	/*
 	 * This routine will setup the DMA and start it
 	 */
 
-	dbg(DBG_RX, "In fst_rx_dma %lx %lx %d\n",
-	    (unsigned long) skb, (unsigned long) mem, len);
+	dbg(DBG_RX, "In fst_rx_dma %x %x %d\n", (u32)skb, mem, len);
 	if (card->dmarx_in_progress) {
 		dbg(DBG_ASS, "In fst_rx_dma while dma in progress\n");
 	}
@@ -915,20 +913,19 @@ fst_rx_dma(struct fst_card_info *card, dma_addr_t skb,
  *      Send a frame through the DMA
  */
 static inline void
-fst_tx_dma(struct fst_card_info *card, unsigned char *skb,
-	   unsigned char *mem, int len)
+fst_tx_dma(struct fst_card_info *card, dma_addr_t skb, u32 mem, int len)
 {
 	/*
 	 * This routine will setup the DMA and start it.
 	 */
 
-	dbg(DBG_TX, "In fst_tx_dma %p %p %d\n", skb, mem, len);
+	dbg(DBG_TX, "In fst_tx_dma %x %x %d\n", (u32)skb, mem, len);
 	if (card->dmatx_in_progress) {
 		dbg(DBG_ASS, "In fst_tx_dma while dma in progress\n");
 	}
 
-	outl((unsigned long) skb, card->pci_conf + DMAPADR1);	/* Copy from here */
-	outl((unsigned long) mem, card->pci_conf + DMALADR1);	/* to here */
+	outl(skb, card->pci_conf + DMAPADR1);	/* Copy from here */
+	outl(mem, card->pci_conf + DMALADR1);	/* to here */
 	outl(len, card->pci_conf + DMASIZ1);	/* for this length */
 	outl(0x000000004, card->pci_conf + DMADPR1);	/* In this direction */
 
@@ -1405,9 +1402,7 @@ do_bottom_half_tx(struct fst_card_info *card)
 					card->dma_len_tx = skb->len;
 					card->dma_txpos = port->txpos;
 					fst_tx_dma(card,
-						   (char *) card->
-						   tx_dma_handle_card,
-						   (char *)
+						   card->tx_dma_handle_card,
 						   BUF_OFFSET(txBuffer[pi]
 							      [port->txpos][0]),
 						   skb->len);

-- 
Ben Hutchings
If at first you don't succeed, you're doing about average.

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 828 bytes --]

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

* [PATCH 7/8] rds: Pass pointers to virt_to_page(), not integers
  2013-10-27 21:47 [PATCH 0/8] Fix minor address type errors Ben Hutchings
  2013-10-27 21:51 ` [PATCH 2/8] farsync: Fix confusion about DMA address and buffer offset types Ben Hutchings
@ 2013-10-27 21:54 ` Ben Hutchings
  2013-10-28  4:26   ` David Miller
  1 sibling, 1 reply; 8+ messages in thread
From: Ben Hutchings @ 2013-10-27 21:54 UTC (permalink / raw)
  To: Venkat Venkatsubra; +Cc: LKML, rds-devel, netdev

[-- Attachment #1: Type: text/plain, Size: 923 bytes --]

Most architectures define virt_to_page() as a macro that casts its
argument such that an argument of type unsigned long will be accepted
without complaint.  However, the proper type is void *, and passing
unsigned long results in a warning on MIPS.

Compile-tested only.

Signed-off-by: Ben Hutchings <ben@decadent.org.uk>
---
 net/rds/message.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/net/rds/message.c b/net/rds/message.c
index aba232f..1913fc9 100644
--- a/net/rds/message.c
+++ b/net/rds/message.c
@@ -257,7 +257,7 @@ struct rds_message *rds_message_map_pages(unsigned long *page_addrs, unsigned in
 
 	for (i = 0; i < rm->data.op_nents; ++i) {
 		sg_set_page(&rm->data.op_sg[i],
-				virt_to_page(page_addrs[i]),
+				virt_to_page((void *)page_addrs[i]),
 				PAGE_SIZE, 0);
 	}
 


-- 
Ben Hutchings
If at first you don't succeed, you're doing about average.

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 828 bytes --]

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

* Re: [PATCH 2/8] farsync: Fix confusion about DMA address and buffer offset types
  2013-10-27 21:51 ` [PATCH 2/8] farsync: Fix confusion about DMA address and buffer offset types Ben Hutchings
@ 2013-10-28  4:26   ` David Miller
  2013-10-28  4:51     ` Ben Hutchings
  0 siblings, 1 reply; 8+ messages in thread
From: David Miller @ 2013-10-28  4:26 UTC (permalink / raw)
  To: ben; +Cc: kevin.curtis, linux-kernel, netdev

From: Ben Hutchings <ben@decadent.org.uk>
Date: Sun, 27 Oct 2013 21:51:44 +0000

> -	dbg(DBG_TX, "In fst_tx_dma %p %p %d\n", skb, mem, len);
> +	dbg(DBG_TX, "In fst_tx_dma %x %x %d\n", (u32)skb, mem, len);

Please use %p for the skb pointer instead of casting it (which btw
will introduce a warning on 64-bit).

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

* Re: [PATCH 7/8] rds: Pass pointers to virt_to_page(), not integers
  2013-10-27 21:54 ` [PATCH 7/8] rds: Pass pointers to virt_to_page(), not integers Ben Hutchings
@ 2013-10-28  4:26   ` David Miller
  2013-10-28 14:22     ` Venkat Venkatsubra
  0 siblings, 1 reply; 8+ messages in thread
From: David Miller @ 2013-10-28  4:26 UTC (permalink / raw)
  To: ben; +Cc: venkat.x.venkatsubra, linux-kernel, rds-devel, netdev

From: Ben Hutchings <ben@decadent.org.uk>
Date: Sun, 27 Oct 2013 21:54:16 +0000

> Most architectures define virt_to_page() as a macro that casts its
> argument such that an argument of type unsigned long will be accepted
> without complaint.  However, the proper type is void *, and passing
> unsigned long results in a warning on MIPS.
> 
> Compile-tested only.
> 
> Signed-off-by: Ben Hutchings <ben@decadent.org.uk>

This looks fine:

Acked-by: David S. Miller <davem@davemloft.net>

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

* Re: [PATCH 2/8] farsync: Fix confusion about DMA address and buffer offset types
  2013-10-28  4:26   ` David Miller
@ 2013-10-28  4:51     ` Ben Hutchings
  2013-10-28  4:59       ` David Miller
  0 siblings, 1 reply; 8+ messages in thread
From: Ben Hutchings @ 2013-10-28  4:51 UTC (permalink / raw)
  To: David Miller; +Cc: kevin.curtis, linux-kernel, netdev

[-- Attachment #1: Type: text/plain, Size: 743 bytes --]

On Mon, 2013-10-28 at 00:26 -0400, David Miller wrote:
> From: Ben Hutchings <ben@decadent.org.uk>
> Date: Sun, 27 Oct 2013 21:51:44 +0000
> 
> > -     dbg(DBG_TX, "In fst_tx_dma %p %p %d\n", skb, mem, len);
> > +     dbg(DBG_TX, "In fst_tx_dma %x %x %d\n", (u32)skb, mem, len);
> 
> Please use %p for the skb pointer instead of casting it (which btw
> will introduce a warning on 64-bit).

skb is the DMA address of the data in the sk_buff.  Yes, this is really
unusual naming.

Ben.

-- 
Ben Hutchings
[W]e found...that it wasn't as easy to get programs right as we had thought.
... I realized that a large part of my life from then on was going to be spent
in finding mistakes in my own programs. - Maurice Wilkes, 1949

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 828 bytes --]

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

* Re: [PATCH 2/8] farsync: Fix confusion about DMA address and buffer offset types
  2013-10-28  4:51     ` Ben Hutchings
@ 2013-10-28  4:59       ` David Miller
  0 siblings, 0 replies; 8+ messages in thread
From: David Miller @ 2013-10-28  4:59 UTC (permalink / raw)
  To: ben; +Cc: kevin.curtis, linux-kernel, netdev

From: Ben Hutchings <ben@decadent.org.uk>
Date: Mon, 28 Oct 2013 04:51:25 +0000

> On Mon, 2013-10-28 at 00:26 -0400, David Miller wrote:
>> From: Ben Hutchings <ben@decadent.org.uk>
>> Date: Sun, 27 Oct 2013 21:51:44 +0000
>> 
>> > -     dbg(DBG_TX, "In fst_tx_dma %p %p %d\n", skb, mem, len);
>> > +     dbg(DBG_TX, "In fst_tx_dma %x %x %d\n", (u32)skb, mem, len);
>> 
>> Please use %p for the skb pointer instead of casting it (which btw
>> will introduce a warning on 64-bit).
> 
> skb is the DMA address of the data in the sk_buff.  Yes, this is really
> unusual naming.

Hmmm, Ok then I guess. :-/

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

* RE: [PATCH 7/8] rds: Pass pointers to virt_to_page(), not integers
  2013-10-28  4:26   ` David Miller
@ 2013-10-28 14:22     ` Venkat Venkatsubra
  0 siblings, 0 replies; 8+ messages in thread
From: Venkat Venkatsubra @ 2013-10-28 14:22 UTC (permalink / raw)
  To: David Miller, ben; +Cc: linux-kernel, rds-devel, netdev

-----Original Message-----
From: David Miller [mailto:davem@davemloft.net] 
Sent: Sunday, October 27, 2013 11:27 PM
To: ben@decadent.org.uk
Cc: Venkat Venkatsubra; linux-kernel@vger.kernel.org; rds-devel@oss.oracle.com; netdev@vger.kernel.org
Subject: Re: [PATCH 7/8] rds: Pass pointers to virt_to_page(), not integers

From: Ben Hutchings <ben@decadent.org.uk>
Date: Sun, 27 Oct 2013 21:54:16 +0000

> Most architectures define virt_to_page() as a macro that casts its 
> argument such that an argument of type unsigned long will be accepted 
> without complaint.  However, the proper type is void *, and passing 
> unsigned long results in a warning on MIPS.
> 
> Compile-tested only.
> 
> Signed-off-by: Ben Hutchings <ben@decadent.org.uk>

This looks fine:

Acked-by: David S. Miller <davem@davemloft.net>
Acked-by: Venkat Venkatsubra <venkat.x.venkatsubra@oracle.com>

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

end of thread, other threads:[~2013-10-28 14:22 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2013-10-27 21:47 [PATCH 0/8] Fix minor address type errors Ben Hutchings
2013-10-27 21:51 ` [PATCH 2/8] farsync: Fix confusion about DMA address and buffer offset types Ben Hutchings
2013-10-28  4:26   ` David Miller
2013-10-28  4:51     ` Ben Hutchings
2013-10-28  4:59       ` David Miller
2013-10-27 21:54 ` [PATCH 7/8] rds: Pass pointers to virt_to_page(), not integers Ben Hutchings
2013-10-28  4:26   ` David Miller
2013-10-28 14:22     ` Venkat Venkatsubra

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).