[PATCH 1/2] benet: use the dma state API instead of the pci equivalents

[PATCH 1/2] benet: use the dma state API instead of the pci equivalents

[FUJITA Tomonori]

The DMA API is preferred.

Signed-off-by: FUJITA Tomonori <fujita.tomonori@lab.ntt.co.jp>
---
 drivers/net/benet/be.h      |    2 +-
 drivers/net/benet/be_main.c |    4 ++--
 2 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/net/benet/be.h b/drivers/net/benet/be.h
index 8f07525..bddae08 100644
--- a/drivers/net/benet/be.h
+++ b/drivers/net/benet/be.h
@@ -206,7 +206,7 @@ struct be_tx_obj {
 /* Struct to remember the pages posted for rx frags */
 struct be_rx_page_info {
 	struct page *page;
-	dma_addr_t bus;
+	DEFINE_DMA_UNMAP_ADDR(bus);
 	u16 page_offset;
 	bool last_page_user;
 };
diff --git a/drivers/net/benet/be_main.c b/drivers/net/benet/be_main.c
index 17282df..8d5e27b 100644
--- a/drivers/net/benet/be_main.c
+++ b/drivers/net/benet/be_main.c
@@ -682,7 +682,7 @@ get_rx_page_info(struct be_adapter *adapter, u16 frag_idx)
 	BUG_ON(!rx_page_info->page);
 
 	if (rx_page_info->last_page_user) {
-		pci_unmap_page(adapter->pdev, pci_unmap_addr(rx_page_info, bus),
+		pci_unmap_page(adapter->pdev, dma_unmap_addr(rx_page_info, bus),
 			adapter->big_page_size, PCI_DMA_FROMDEVICE);
 		rx_page_info->last_page_user = false;
 	}
@@ -993,7 +993,7 @@ static void be_post_rx_frags(struct be_adapter *adapter)
 		}
 		page_offset = page_info->page_offset;
 		page_info->page = pagep;
-		pci_unmap_addr_set(page_info, bus, page_dmaaddr);
+		dma_unmap_addr_set(page_info, bus, page_dmaaddr);
 		frag_dmaaddr = page_dmaaddr + page_info->page_offset;
 
 		rxd = queue_head_node(rxq);
-- 
1.7.0

[PATCH 2/2] benet: fix the misusage of zero dma address

[FUJITA Tomonori]

benet driver wrongly assumes that zero is an invalid dma address
(calls dma_unmap_page for only non zero dma addresses). Zero is a
valid dma address on some architectures. The dma length can be used
here.

Signed-off-by: FUJITA Tomonori <fujita.tomonori@lab.ntt.co.jp>
---
 drivers/net/benet/be_main.c |    6 ++++--
 1 files changed, 4 insertions(+), 2 deletions(-)

diff --git a/drivers/net/benet/be_main.c b/drivers/net/benet/be_main.c
index 8d5e27b..8828c7d 100644
--- a/drivers/net/benet/be_main.c
+++ b/drivers/net/benet/be_main.c
@@ -394,13 +394,15 @@ static void unmap_tx_frag(struct pci_dev *pdev, struct be_eth_wrb *wrb,
 	be_dws_le_to_cpu(wrb, sizeof(*wrb));
 
 	dma = (u64)wrb->frag_pa_hi << 32 | (u64)wrb->frag_pa_lo;
-	if (dma != 0) {
+	if (wrb->frag_len) {
 		if (unmap_single)
 			pci_unmap_single(pdev, dma, wrb->frag_len,
 				PCI_DMA_TODEVICE);
 		else
 			pci_unmap_page(pdev, dma, wrb->frag_len,
 				PCI_DMA_TODEVICE);
+
+		wrb->frag_len = 0;
 	}
 }
 
@@ -466,9 +468,9 @@ dma_err:
 	txq->head = map_head;
 	while (copied) {
 		wrb = queue_head_node(txq);
+		copied -= wrb->frag_len;
 		unmap_tx_frag(pdev, wrb, map_single);
 		map_single = false;
-		copied -= wrb->frag_len;
 		queue_head_inc(txq);
 	}
 	return 0;
-- 
1.7.0

Re: [PATCH 2/2] benet: fix the misusage of zero dma address

[Sathya Perla]

Hi Fujita, thanks for the patch; pls see below:

On 02/04/10 11:53 +0900, FUJITA Tomonori wrote:
> benet driver wrongly assumes that zero is an invalid dma address
> (calls dma_unmap_page for only non zero dma addresses). Zero is a
> valid dma address on some architectures. The dma length can be used
> here.
> 
> Signed-off-by: FUJITA Tomonori <fujita.tomonori@lab.ntt.co.jp>
> ---
>  drivers/net/benet/be_main.c |    6 ++++--
>  1 files changed, 4 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/net/benet/be_main.c b/drivers/net/benet/be_main.c
> index 8d5e27b..8828c7d 100644
> --- a/drivers/net/benet/be_main.c
> +++ b/drivers/net/benet/be_main.c
> @@ -394,13 +394,15 @@ static void unmap_tx_frag(struct pci_dev *pdev, struct be_eth_wrb *wrb,
>  	be_dws_le_to_cpu(wrb, sizeof(*wrb));
>  
>  	dma = (u64)wrb->frag_pa_hi << 32 | (u64)wrb->frag_pa_lo;
> -	if (dma != 0) {
> +	if (wrb->frag_len) {
>  		if (unmap_single)
>  			pci_unmap_single(pdev, dma, wrb->frag_len,
>  				PCI_DMA_TODEVICE);
>  		else
>  			pci_unmap_page(pdev, dma, wrb->frag_len,
>  				PCI_DMA_TODEVICE);
> +
> +		wrb->frag_len = 0;
Why does wrb->frag_len need to be reset here?
In the TX path, it is set to the proper value for data wrbs and zero
for dummy and hdr wrbs.
>  	}
>  }
>  
> @@ -466,9 +468,9 @@ dma_err:
>  	txq->head = map_head;
>  	while (copied) {
>  		wrb = queue_head_node(txq);
> +		copied -= wrb->frag_len;
>  		unmap_tx_frag(pdev, wrb, map_single);
>  		map_single = false;
> -		copied -= wrb->frag_len;
>  		queue_head_inc(txq);
>  	}
>  	return 0;
> -- 
> 1.7.0
> 

Re: [PATCH 2/2] benet: fix the misusage of zero dma address

[FUJITA Tomonori]

On Mon, 5 Apr 2010 12:40:59 +0530
Sathya Perla <sathyap@serverengines.com> wrote:

> Hi Fujita, thanks for the patch; pls see below:
> 
> On 02/04/10 11:53 +0900, FUJITA Tomonori wrote:
> > benet driver wrongly assumes that zero is an invalid dma address
> > (calls dma_unmap_page for only non zero dma addresses). Zero is a
> > valid dma address on some architectures. The dma length can be used
> > here.
> > 
> > Signed-off-by: FUJITA Tomonori <fujita.tomonori@lab.ntt.co.jp>
> > ---
> >  drivers/net/benet/be_main.c |    6 ++++--
> >  1 files changed, 4 insertions(+), 2 deletions(-)
> > 
> > diff --git a/drivers/net/benet/be_main.c b/drivers/net/benet/be_main.c
> > index 8d5e27b..8828c7d 100644
> > --- a/drivers/net/benet/be_main.c
> > +++ b/drivers/net/benet/be_main.c
> > @@ -394,13 +394,15 @@ static void unmap_tx_frag(struct pci_dev *pdev, struct be_eth_wrb *wrb,
> >  	be_dws_le_to_cpu(wrb, sizeof(*wrb));
> >  
> >  	dma = (u64)wrb->frag_pa_hi << 32 | (u64)wrb->frag_pa_lo;
> > -	if (dma != 0) {
> > +	if (wrb->frag_len) {
> >  		if (unmap_single)
> >  			pci_unmap_single(pdev, dma, wrb->frag_len,
> >  				PCI_DMA_TODEVICE);
> >  		else
> >  			pci_unmap_page(pdev, dma, wrb->frag_len,
> >  				PCI_DMA_TODEVICE);
> > +
> > +		wrb->frag_len = 0;
> Why does wrb->frag_len need to be reset here?
> In the TX path, it is set to the proper value for data wrbs and zero
> for dummy and hdr wrbs.

I guess that I misunderstood why unmap_tx_frag() checks a dma address.
The checking is necessary to avoid calling pci_unamp_* API for dummy
hdr wrbs?

Anyway, if wrb->frag_len doesn't need to be reset here, the following
patch is ok?

=
From: FUJITA Tomonori <fujita.tomonori@lab.ntt.co.jp>
Subject: [PATCH v2 2/2] benet: fix the misusage of zero dma address

benet driver wrongly assumes that zero is an invalid dma address
(calls dma_unmap_page for only non zero dma addresses). Zero is a
valid dma address on some architectures. The dma length can be used
here.

Signed-off-by: FUJITA Tomonori <fujita.tomonori@lab.ntt.co.jp>
---
 drivers/net/benet/be_main.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/drivers/net/benet/be_main.c b/drivers/net/benet/be_main.c
index a27a0a1..c95e1a2 100644
--- a/drivers/net/benet/be_main.c
+++ b/drivers/net/benet/be_main.c
@@ -404,7 +404,7 @@ static void unmap_tx_frag(struct pci_dev *pdev, struct be_eth_wrb *wrb,
 	be_dws_le_to_cpu(wrb, sizeof(*wrb));
 
 	dma = (u64)wrb->frag_pa_hi << 32 | (u64)wrb->frag_pa_lo;
-	if (dma != 0) {
+	if (wrb->frag_len) {
 		if (unmap_single)
 			pci_unmap_single(pdev, dma, wrb->frag_len,
 				PCI_DMA_TODEVICE);
-- 
1.7.0

Re: [PATCH 2/2] benet: fix the misusage of zero dma address

[Sathya Perla]

On 05/04/10 16:40 +0900, FUJITA Tomonori wrote:
> > > +		wrb->frag_len = 0;
> > Why does wrb->frag_len need to be reset here?
> > In the TX path, it is set to the proper value for data wrbs and zero
> > for dummy and hdr wrbs.
> 
> I guess that I misunderstood why unmap_tx_frag() checks a dma address.
> The checking is necessary to avoid calling pci_unamp_* API for dummy
> hdr wrbs?
Yes.
> 
> Anyway, if wrb->frag_len doesn't need to be reset here, the following
> patch is ok?
Yes. Thanks.

Acked-by: Sathya Perla <sathyap@serverengines.com>
> 
> =
> From: FUJITA Tomonori <fujita.tomonori@lab.ntt.co.jp>
> Subject: [PATCH v2 2/2] benet: fix the misusage of zero dma address
> 
> benet driver wrongly assumes that zero is an invalid dma address
> (calls dma_unmap_page for only non zero dma addresses). Zero is a
> valid dma address on some architectures. The dma length can be used
> here.
> 
> Signed-off-by: FUJITA Tomonori <fujita.tomonori@lab.ntt.co.jp>
> ---
>  drivers/net/benet/be_main.c |    2 +-
>  1 files changed, 1 insertions(+), 1 deletions(-)
> 
> diff --git a/drivers/net/benet/be_main.c b/drivers/net/benet/be_main.c
> index a27a0a1..c95e1a2 100644
> --- a/drivers/net/benet/be_main.c
> +++ b/drivers/net/benet/be_main.c
> @@ -404,7 +404,7 @@ static void unmap_tx_frag(struct pci_dev *pdev, struct be_eth_wrb *wrb,
>  	be_dws_le_to_cpu(wrb, sizeof(*wrb));
>  
>  	dma = (u64)wrb->frag_pa_hi << 32 | (u64)wrb->frag_pa_lo;
> -	if (dma != 0) {
> +	if (wrb->frag_len) {
>  		if (unmap_single)
>  			pci_unmap_single(pdev, dma, wrb->frag_len,
>  				PCI_DMA_TODEVICE);
> -- 
> 1.7.0
> 
> 

Re: [PATCH 2/2] benet: fix the misusage of zero dma address

[FUJITA Tomonori]

On Mon, 5 Apr 2010 13:52:02 +0530
Sathya Perla <sathyap@serverengines.com> wrote:

> On 05/04/10 16:40 +0900, FUJITA Tomonori wrote:
> > > > +		wrb->frag_len = 0;
> > > Why does wrb->frag_len need to be reset here?
> > > In the TX path, it is set to the proper value for data wrbs and zero
> > > for dummy and hdr wrbs.
> > 
> > I guess that I misunderstood why unmap_tx_frag() checks a dma address.
> > The checking is necessary to avoid calling pci_unamp_* API for dummy
> > hdr wrbs?
> Yes.
> > 
> > Anyway, if wrb->frag_len doesn't need to be reset here, the following
> > patch is ok?
> Yes. Thanks.
> 
> Acked-by: Sathya Perla <sathyap@serverengines.com>

Thanks!

Can I also get your ack on "[PATCH 1/2] benet: use the dma state API
instead of the pci equivalents"?

http://patchwork.ozlabs.org/patch/49265/


The reason why I want to replace the PCI DMA state API is:

http://marc.info/?l=linux-netdev&m=127037540020276&w=2

Note that I use DEFINE_DMA_UNMAP_ADDR for bus in struct
be_rx_page_info because dma_unmap_addr and DEFINE_DMA_UNMAP_ADDR are
supposed to be used together.

Re: [PATCH 1/2] benet: use the dma state API instead of the pci equivalents

[Sathya Perla]

Thanks.

Acked-by: Sathya Perla <sathyap@serverengines.com>

On 02/04/10 11:53 +0900, FUJITA Tomonori wrote:
> The DMA API is preferred.
> 
> Signed-off-by: FUJITA Tomonori <fujita.tomonori@lab.ntt.co.jp>
> ---
>  drivers/net/benet/be.h      |    2 +-
>  drivers/net/benet/be_main.c |    4 ++--
>  2 files changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/net/benet/be.h b/drivers/net/benet/be.h
> index 8f07525..bddae08 100644
> --- a/drivers/net/benet/be.h
> +++ b/drivers/net/benet/be.h
> @@ -206,7 +206,7 @@ struct be_tx_obj {
>  /* Struct to remember the pages posted for rx frags */
>  struct be_rx_page_info {
>  	struct page *page;
> -	dma_addr_t bus;
> +	DEFINE_DMA_UNMAP_ADDR(bus);
>  	u16 page_offset;
>  	bool last_page_user;
>  };
> diff --git a/drivers/net/benet/be_main.c b/drivers/net/benet/be_main.c
> index 17282df..8d5e27b 100644
> --- a/drivers/net/benet/be_main.c
> +++ b/drivers/net/benet/be_main.c
> @@ -682,7 +682,7 @@ get_rx_page_info(struct be_adapter *adapter, u16 frag_idx)
>  	BUG_ON(!rx_page_info->page);
>  
>  	if (rx_page_info->last_page_user) {
> -		pci_unmap_page(adapter->pdev, pci_unmap_addr(rx_page_info, bus),
> +		pci_unmap_page(adapter->pdev, dma_unmap_addr(rx_page_info, bus),
>  			adapter->big_page_size, PCI_DMA_FROMDEVICE);
>  		rx_page_info->last_page_user = false;
>  	}
> @@ -993,7 +993,7 @@ static void be_post_rx_frags(struct be_adapter *adapter)
>  		}
>  		page_offset = page_info->page_offset;
>  		page_info->page = pagep;
> -		pci_unmap_addr_set(page_info, bus, page_dmaaddr);
> +		dma_unmap_addr_set(page_info, bus, page_dmaaddr);
>  		frag_dmaaddr = page_dmaaddr + page_info->page_offset;
>  
>  		rxd = queue_head_node(rxq);
> -- 
> 1.7.0
> 
> --
> To unsubscribe from this list: send the line "unsubscribe netdev" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 1/2] benet: use the dma state API instead of the pci equivalents

[David Miller]

From: Sathya Perla <sathyap@serverengines.com>
Date: Mon, 5 Apr 2010 14:34:57 +0530

> Thanks.
> 
> Acked-by: Sathya Perla <sathyap@serverengines.com>

Applied to net-next-2.6

Re: [PATCH 2/2] benet: fix the misusage of zero dma address

[David Miller]

From: Sathya Perla <sathyap@serverengines.com>
Date: Mon, 5 Apr 2010 13:52:02 +0530

> On 05/04/10 16:40 +0900, FUJITA Tomonori wrote:
>> > > +		wrb->frag_len = 0;
>> > Why does wrb->frag_len need to be reset here?
>> > In the TX path, it is set to the proper value for data wrbs and zero
>> > for dummy and hdr wrbs.
>> 
>> I guess that I misunderstood why unmap_tx_frag() checks a dma address.
>> The checking is necessary to avoid calling pci_unamp_* API for dummy
>> hdr wrbs?
> Yes.
>> 
>> Anyway, if wrb->frag_len doesn't need to be reset here, the following
>> patch is ok?
> Yes. Thanks.
> 
> Acked-by: Sathya Perla <sathyap@serverengines.com>

Applied to net-next-2.6