Patchwork 2.6.34-rc3 : Badness at lib/dma-debug.c:820 during ibmvscsi init

login
register
mail settings
Submitter FUJITA Tomonori
Date April 2, 2010, 6:50 a.m.
Message ID <20100402154959K.fujita.tomonori@lab.ntt.co.jp>
Download mbox | patch
Permalink /patch/49279/
State Not Applicable
Headers show

Comments

FUJITA Tomonori - April 2, 2010, 6:50 a.m.
On Fri, 02 Apr 2010 12:07:58 +0530
Sachin Sant <sachinp@in.ibm.com> wrote:

> FUJITA Tomonori wrote:
> > On Fri, 02 Apr 2010 10:48:55 +0530
> > Sachin Sant <sachinp@in.ibm.com> wrote:
> >
> >   
> >> 2.6.34-rc3 boot on a power5 box produces this badness message
> >> during ibmvscsi initialization.
> >>
> >> ibmvscsi 30000003: Client reserve enabled
> >> ibmvscsi 30000003: sent SRP login
> >> ibmvscsi 30000003: SRP_LOGIN succeeded
> >> ibmvscsi 30000003: DMA-API: device driver frees DMA memory with wrong function [device address=0x0000000000011520] [size=36 bytes] [mapped as scather-gather] [unmapped as single]
> >>     
> >
> > ibmvscsi has been incompatible with the dma debug facility, I
> > guess. The driver uses dma_unmap_single for buffers mapped via
> > dma_map_sg. It works but it's the API violation.
> >
> > Does this patch work?
> >   
> Yes, this patch fixed the issue for me. Thanks

Thanks, here's the patch in the proper format.

=
From: FUJITA Tomonori <fujita.tomonori@lab.ntt.co.jp>
Subject: [PATCH] ibmvscsi: fix DMA API misuse

ibmvscsi uses dma_unmap_single() for buffers mapped via
dma_map_sg(). It works however it's the API violation. The DMA debug
facility complains about it:

http://marc.info/?l=linux-scsi&m=127018555013151&w=2

Reported-by: Sachin Sant <sachinp@in.ibm.com>
Tested-by: Sachin Sant <sachinp@in.ibm.com>
Signed-off-by: FUJITA Tomonori <fujita.tomonori@lab.ntt.co.jp>
---
 drivers/scsi/ibmvscsi/ibmvscsi.c |   29 ++---------------------------
 1 files changed, 2 insertions(+), 27 deletions(-)
Benjamin Herrenschmidt - April 27, 2010, 7:05 a.m.
On Fri, 2010-04-02 at 15:50 +0900, FUJITA Tomonori wrote:

> Thanks, here's the patch in the proper format.
> 
> =
> From: FUJITA Tomonori <fujita.tomonori@lab.ntt.co.jp>
> Subject: [PATCH] ibmvscsi: fix DMA API misuse
> 
> ibmvscsi uses dma_unmap_single() for buffers mapped via
> dma_map_sg(). It works however it's the API violation. The DMA debug
> facility complains about it:

Hi folks !

Is this going to be merged via the scsi tree ?

Thanks !

Cheers,
Ben.

> http://marc.info/?l=linux-scsi&m=127018555013151&w=2
> 
> Reported-by: Sachin Sant <sachinp@in.ibm.com>
> Tested-by: Sachin Sant <sachinp@in.ibm.com>
> Signed-off-by: FUJITA Tomonori <fujita.tomonori@lab.ntt.co.jp>
> ---
>  drivers/scsi/ibmvscsi/ibmvscsi.c |   29 ++---------------------------
>  1 files changed, 2 insertions(+), 27 deletions(-)
> 
> diff --git a/drivers/scsi/ibmvscsi/ibmvscsi.c b/drivers/scsi/ibmvscsi/ibmvscsi.c
> index dc1bcbe..0856436 100644
> --- a/drivers/scsi/ibmvscsi/ibmvscsi.c
> +++ b/drivers/scsi/ibmvscsi/ibmvscsi.c
> @@ -322,16 +322,6 @@ static void set_srp_direction(struct scsi_cmnd *cmd,
>  		srp_cmd->buf_fmt = fmt;
>  }
>  
> -static void unmap_sg_list(int num_entries,
> -		struct device *dev,
> -		struct srp_direct_buf *md)
> -{
> -	int i;
> -
> -	for (i = 0; i < num_entries; ++i)
> -		dma_unmap_single(dev, md[i].va, md[i].len, DMA_BIDIRECTIONAL);
> -}
> -
>  /**
>   * unmap_cmd_data: - Unmap data pointed in srp_cmd based on the format
>   * @cmd:	srp_cmd whose additional_data member will be unmapped
> @@ -349,24 +339,9 @@ static void unmap_cmd_data(struct srp_cmd *cmd,
>  
>  	if (out_fmt == SRP_NO_DATA_DESC && in_fmt == SRP_NO_DATA_DESC)
>  		return;
> -	else if (out_fmt == SRP_DATA_DESC_DIRECT ||
> -		 in_fmt == SRP_DATA_DESC_DIRECT) {
> -		struct srp_direct_buf *data =
> -			(struct srp_direct_buf *) cmd->add_data;
> -		dma_unmap_single(dev, data->va, data->len, DMA_BIDIRECTIONAL);
> -	} else {
> -		struct srp_indirect_buf *indirect =
> -			(struct srp_indirect_buf *) cmd->add_data;
> -		int num_mapped = indirect->table_desc.len /
> -			sizeof(struct srp_direct_buf);
>  
> -		if (num_mapped <= MAX_INDIRECT_BUFS) {
> -			unmap_sg_list(num_mapped, dev, &indirect->desc_list[0]);
> -			return;
> -		}
> -
> -		unmap_sg_list(num_mapped, dev, evt_struct->ext_list);
> -	}
> +	if (evt_struct->cmnd)
> +		scsi_dma_unmap(evt_struct->cmnd);
>  }
>  
>  static int map_sg_list(struct scsi_cmnd *cmd, int nseg,
FUJITA Tomonori - April 27, 2010, 9:34 a.m.
On Tue, 27 Apr 2010 17:05:21 +1000
Benjamin Herrenschmidt <benh@kernel.crashing.org> wrote:

> On Fri, 2010-04-02 at 15:50 +0900, FUJITA Tomonori wrote:
> 
> > Thanks, here's the patch in the proper format.
> > 
> > =
> > From: FUJITA Tomonori <fujita.tomonori@lab.ntt.co.jp>
> > Subject: [PATCH] ibmvscsi: fix DMA API misuse
> > 
> > ibmvscsi uses dma_unmap_single() for buffers mapped via
> > dma_map_sg(). It works however it's the API violation. The DMA debug
> > facility complains about it:
> 
> Hi folks !
> 
> Is this going to be merged via the scsi tree ?

Already merged (a71fa1fc43a29133f13ae6ada1a389ca298c0934).

Patch

diff --git a/drivers/scsi/ibmvscsi/ibmvscsi.c b/drivers/scsi/ibmvscsi/ibmvscsi.c
index dc1bcbe..0856436 100644
--- a/drivers/scsi/ibmvscsi/ibmvscsi.c
+++ b/drivers/scsi/ibmvscsi/ibmvscsi.c
@@ -322,16 +322,6 @@  static void set_srp_direction(struct scsi_cmnd *cmd,
 		srp_cmd->buf_fmt = fmt;
 }
 
-static void unmap_sg_list(int num_entries,
-		struct device *dev,
-		struct srp_direct_buf *md)
-{
-	int i;
-
-	for (i = 0; i < num_entries; ++i)
-		dma_unmap_single(dev, md[i].va, md[i].len, DMA_BIDIRECTIONAL);
-}
-
 /**
  * unmap_cmd_data: - Unmap data pointed in srp_cmd based on the format
  * @cmd:	srp_cmd whose additional_data member will be unmapped
@@ -349,24 +339,9 @@  static void unmap_cmd_data(struct srp_cmd *cmd,
 
 	if (out_fmt == SRP_NO_DATA_DESC && in_fmt == SRP_NO_DATA_DESC)
 		return;
-	else if (out_fmt == SRP_DATA_DESC_DIRECT ||
-		 in_fmt == SRP_DATA_DESC_DIRECT) {
-		struct srp_direct_buf *data =
-			(struct srp_direct_buf *) cmd->add_data;
-		dma_unmap_single(dev, data->va, data->len, DMA_BIDIRECTIONAL);
-	} else {
-		struct srp_indirect_buf *indirect =
-			(struct srp_indirect_buf *) cmd->add_data;
-		int num_mapped = indirect->table_desc.len /
-			sizeof(struct srp_direct_buf);
 
-		if (num_mapped <= MAX_INDIRECT_BUFS) {
-			unmap_sg_list(num_mapped, dev, &indirect->desc_list[0]);
-			return;
-		}
-
-		unmap_sg_list(num_mapped, dev, evt_struct->ext_list);
-	}
+	if (evt_struct->cmnd)
+		scsi_dma_unmap(evt_struct->cmnd);
 }
 
 static int map_sg_list(struct scsi_cmnd *cmd, int nseg,