Re: NPIV mapping problems (was Re: [PATCH 1/19] add data buffer accessors)

2007-07-09 Thread James Smart

ACK - looks fine..

Thanks

-- james s

FUJITA Tomonori wrote:

I forgot to CC James Smart and send a lpfc patch.

From: FUJITA Tomonori [EMAIL PROTECTED]
Subject: Re: NPIV mapping problems (was Re: [PATCH 1/19] add data buffer 
accessors)
Date: Wed, 04 Jul 2007 17:25:36 +0900


Sorry for the delay...

From: Andrew Vasquez [EMAIL PROTECTED]
Subject: NPIV mapping problems (was Re: [PATCH 1/19] add data buffer accessors)
Date: Fri, 29 Jun 2007 06:23:32 -0700


On Sat, 12 May 2007, James Bottomley wrote:


On Sat, 2007-05-12 at 19:05 +0900, FUJITA Tomonori wrote:

Add a set of accessors for the scsi data buffer. This is in
preparation for chaining sg lists and bidirectional requests (and
possibly, the mid-layer dma mapping).

Signed-off-by: FUJITA Tomonori [EMAIL PROTECTED]
---
 drivers/scsi/scsi_lib.c  |   26 ++
 include/scsi/scsi_cmnd.h |   11 +++
 2 files changed, 37 insertions(+), 0 deletions(-)

diff --git a/drivers/scsi/scsi_lib.c b/drivers/scsi/scsi_lib.c
index 1f5a07b..a2ebe61 100644
--- a/drivers/scsi/scsi_lib.c
+++ b/drivers/scsi/scsi_lib.c
@@ -2290,3 +2290,29 @@ void scsi_kunmap_atomic_sg(void *virt)
kunmap_atomic(virt, KM_BIO_SRC_IRQ);
 }
 EXPORT_SYMBOL(scsi_kunmap_atomic_sg);
+
+int scsi_dma_map(struct device *dev, struct scsi_cmnd *cmd)

Actually, this is redundant.  We make sure the
shost-shost_gendev.parent is the device which should have been passed
in to scsi_add_host().

Well, there's perhaps an unintended side-effect with this assumption,
NPIV-based 'vports' (and their subsequent 'struct device' members) are
not fully initialized objects.

This is what we've run into while working on our NPIV (based) driver
with the 'data buffer' accessors works, while queueing the first
command to an sdev of a freshly created vport:

	[  366.860758] Unable to handle kernel NULL pointer dereference at  RIP: 
	[  366.860762]  [8020fdf6] check_addr+0x10/0x4a
	[  366.860778] PGD 0 
	[  366.860782] Oops:  [1] SMP 
	[  366.860787] CPU 0 
	[  366.860790] Modules linked in: qla2xxx scsi_transport_fc

[  366.860798] Pid: 5757, comm: scsi_wq_2 Not tainted 2.6.22-rc6 #4
[  366.860802] RIP: 0010:[8020fdf6]  [8020fdf6] 
check_addr+0x10/0x4a
[  366.860812] RSP: 0018:810073979960  EFLAGS: 00010082
[  366.860816] RAX:  RBX: 810037cda070 RCX: 
0024
[  366.860821] RDX: 7cd86188 RSI: 81007d800ef8 RDI: 
804c30e1
[  366.860824] RBP:  R08:  R09: 

[  366.860828] R10:  R11: 0006 R12: 
0001
[  366.860834] R13: 81007d800ef8 R14: 810075f9fda0 R15: 
810076d66af8
[  366.860839] FS:  () GS:8057() 
knlGS:
[  366.860844] CS:  0010 DS: 0018 ES: 0018 CR0: 8005003b
[  366.860847] CR2:  CR3: 7ec6b000 CR4: 
06e0
[  366.860853] Process scsi_wq_2 (pid: 5757, threadinfo 
810073978000, task 810037c6ce00)
[  366.860856] Stack:  00011220 8020fea6 
0246 00e1
[  366.860866]  00e1 810076908608 810076d66af8 
803a440c
[  366.860876]  810076908608 8801cdd2 810076908688 
810073979a18
[  366.860885] Call Trace:
[  366.860891]  [8020fea6] nommu_map_sg+0x76/0x8f
[  366.860900]  [803a440c] scsi_dma_map+0x45/0x5c
[  366.860922]  [8801cdd2] 
:qla2xxx:qla24xx_start_scsi+0x118/0x4fb
[  366.860928]  [8039fa74] scsi_done+0x0/0x18
[  366.860942]  [880116f0] 
:qla2xxx:qla24xx_queuecommand+0x148/0x17a
[  366.860948]  [803a01f7] scsi_dispatch_cmd+0x1f6/0x313
[  366.860953]  [803a622b] scsi_request_fn+0x1d7/0x3b8

the failure point is this check (line 16) in check_addr():

(gdb) l* check_addr+0x10
0x8020fdf6 is in check_addr (arch/x86_64/kernel/pci-nommu.c:16).
11  #include asm/dma.h
12
13  static int
14  check_addr(char *name, struct device *hwdev, dma_addr_t bus, 
size_t size)
15  {
16  if (hwdev  bus + size  *hwdev-dma_mask) {
17  if (*hwdev-dma_mask = DMA_32BIT_MASK)
18  printk(KERN_ERR
19  nommu_%s: overflow %Lx+%zu of device 
mask %Lx\n,
20  name, (long long)bus, size,

hwdev-dma_mask (an 'u64 *') is NULL, as a corresponding 'set'
function is never getting called via pci_set_dma_mask() and
pci_set_consistent_dma_mask() on a vport.

Yeah, I use lpfc's NPIV but haven't hit this because I use swiotlb...



I don't believe we'd want to:

1) have each LLD cache

RE: NPIV mapping problems (was Re: [PATCH 1/19] add data buffer accessors)

2007-07-05 Thread Seokmann Ju
On Wed, 2007-07-04 at 1:26 +0900, FUJITA Tomonori wrote:
 On Fri, 29 Jun 2007 06:23:32 -0700 Andrew Vasquez wrote:
  One possiblity (the least intrusive) would be to add a
  scsi_dma_map_with_device() function which takes the proper (LLD
  defined) 'struct device' as a parameter, as was originally 
 proposed, 
  and have NPIV-aware drivers call that function during the 
 mappings of 
  physical *and* virtual dma-mappings.
 
 It's ok. But if only NPIV-aware drivers need to handle this, 
 would it be better to just use dma_map_sg instead of scsi_dma_map?
I agree with you on the suggestion.
A patch for NPIV support on qla2xxx module that includes this change
will be followed.

Seokmann
-
To unsubscribe from this list: send the line unsubscribe linux-scsi in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: NPIV mapping problems (was Re: [PATCH 1/19] add data buffer accessors)

2007-07-04 Thread FUJITA Tomonori
Sorry for the delay...

From: Andrew Vasquez [EMAIL PROTECTED]
Subject: NPIV mapping problems (was Re: [PATCH 1/19] add data buffer accessors)
Date: Fri, 29 Jun 2007 06:23:32 -0700

 On Sat, 12 May 2007, James Bottomley wrote:
 
  On Sat, 2007-05-12 at 19:05 +0900, FUJITA Tomonori wrote:
   Add a set of accessors for the scsi data buffer. This is in
   preparation for chaining sg lists and bidirectional requests (and
   possibly, the mid-layer dma mapping).
   
   Signed-off-by: FUJITA Tomonori [EMAIL PROTECTED]
   ---
drivers/scsi/scsi_lib.c  |   26 ++
include/scsi/scsi_cmnd.h |   11 +++
2 files changed, 37 insertions(+), 0 deletions(-)
   
   diff --git a/drivers/scsi/scsi_lib.c b/drivers/scsi/scsi_lib.c
   index 1f5a07b..a2ebe61 100644
   --- a/drivers/scsi/scsi_lib.c
   +++ b/drivers/scsi/scsi_lib.c
   @@ -2290,3 +2290,29 @@ void scsi_kunmap_atomic_sg(void *virt)
 kunmap_atomic(virt, KM_BIO_SRC_IRQ);
}
EXPORT_SYMBOL(scsi_kunmap_atomic_sg);
   +
   +int scsi_dma_map(struct device *dev, struct scsi_cmnd *cmd)
  
  Actually, this is redundant.  We make sure the
  shost-shost_gendev.parent is the device which should have been passed
  in to scsi_add_host().
 
 Well, there's perhaps an unintended side-effect with this assumption,
 NPIV-based 'vports' (and their subsequent 'struct device' members) are
 not fully initialized objects.
 
 This is what we've run into while working on our NPIV (based) driver
 with the 'data buffer' accessors works, while queueing the first
 command to an sdev of a freshly created vport:
 
   [  366.860758] Unable to handle kernel NULL pointer dereference at 
  RIP: 
   [  366.860762]  [8020fdf6] check_addr+0x10/0x4a
   [  366.860778] PGD 0 
   [  366.860782] Oops:  [1] SMP 
   [  366.860787] CPU 0 
   [  366.860790] Modules linked in: qla2xxx scsi_transport_fc
   [  366.860798] Pid: 5757, comm: scsi_wq_2 Not tainted 2.6.22-rc6 #4
   [  366.860802] RIP: 0010:[8020fdf6]  [8020fdf6] 
 check_addr+0x10/0x4a
   [  366.860812] RSP: 0018:810073979960  EFLAGS: 00010082
   [  366.860816] RAX:  RBX: 810037cda070 RCX: 
 0024
   [  366.860821] RDX: 7cd86188 RSI: 81007d800ef8 RDI: 
 804c30e1
   [  366.860824] RBP:  R08:  R09: 
 
   [  366.860828] R10:  R11: 0006 R12: 
 0001
   [  366.860834] R13: 81007d800ef8 R14: 810075f9fda0 R15: 
 810076d66af8
   [  366.860839] FS:  () GS:8057() 
 knlGS:
   [  366.860844] CS:  0010 DS: 0018 ES: 0018 CR0: 8005003b
   [  366.860847] CR2:  CR3: 7ec6b000 CR4: 
 06e0
   [  366.860853] Process scsi_wq_2 (pid: 5757, threadinfo 
 810073978000, task 810037c6ce00)
   [  366.860856] Stack:  00011220 8020fea6 
 0246 00e1
   [  366.860866]  00e1 810076908608 810076d66af8 
 803a440c
   [  366.860876]  810076908608 8801cdd2 810076908688 
 810073979a18
   [  366.860885] Call Trace:
   [  366.860891]  [8020fea6] nommu_map_sg+0x76/0x8f
   [  366.860900]  [803a440c] scsi_dma_map+0x45/0x5c
   [  366.860922]  [8801cdd2] 
 :qla2xxx:qla24xx_start_scsi+0x118/0x4fb
   [  366.860928]  [8039fa74] scsi_done+0x0/0x18
   [  366.860942]  [880116f0] 
 :qla2xxx:qla24xx_queuecommand+0x148/0x17a
   [  366.860948]  [803a01f7] scsi_dispatch_cmd+0x1f6/0x313
   [  366.860953]  [803a622b] scsi_request_fn+0x1d7/0x3b8
 
 the failure point is this check (line 16) in check_addr():
 
   (gdb) l* check_addr+0x10
   0x8020fdf6 is in check_addr (arch/x86_64/kernel/pci-nommu.c:16).
   11  #include asm/dma.h
   12
   13  static int
   14  check_addr(char *name, struct device *hwdev, dma_addr_t bus, 
 size_t size)
   15  {
   16  if (hwdev  bus + size  *hwdev-dma_mask) {
   17  if (*hwdev-dma_mask = DMA_32BIT_MASK)
   18  printk(KERN_ERR
   19  nommu_%s: overflow %Lx+%zu of 
 device mask %Lx\n,
   20  name, (long long)bus, size,
 
 hwdev-dma_mask (an 'u64 *') is NULL, as a corresponding 'set'
 function is never getting called via pci_set_dma_mask() and
 pci_set_consistent_dma_mask() on a vport.

Yeah, I use lpfc's NPIV but haven't hit this because I use swiotlb...


 I don't believe we'd want to:
 
 1) have each LLD cache the physical-port's previously determined
dma-mask, and call pci_set_[consistent_]dma_mask() for each
instantiated vport.
 
the problem with this approach is 'knowing' how

Re: NPIV mapping problems (was Re: [PATCH 1/19] add data buffer accessors)

2007-07-04 Thread FUJITA Tomonori
I forgot to CC James Smart and send a lpfc patch.

From: FUJITA Tomonori [EMAIL PROTECTED]
Subject: Re: NPIV mapping problems (was Re: [PATCH 1/19] add data buffer 
accessors)
Date: Wed, 04 Jul 2007 17:25:36 +0900

 Sorry for the delay...
 
 From: Andrew Vasquez [EMAIL PROTECTED]
 Subject: NPIV mapping problems (was Re: [PATCH 1/19] add data buffer 
 accessors)
 Date: Fri, 29 Jun 2007 06:23:32 -0700
 
  On Sat, 12 May 2007, James Bottomley wrote:
  
   On Sat, 2007-05-12 at 19:05 +0900, FUJITA Tomonori wrote:
Add a set of accessors for the scsi data buffer. This is in
preparation for chaining sg lists and bidirectional requests (and
possibly, the mid-layer dma mapping).

Signed-off-by: FUJITA Tomonori [EMAIL PROTECTED]
---
 drivers/scsi/scsi_lib.c  |   26 ++
 include/scsi/scsi_cmnd.h |   11 +++
 2 files changed, 37 insertions(+), 0 deletions(-)

diff --git a/drivers/scsi/scsi_lib.c b/drivers/scsi/scsi_lib.c
index 1f5a07b..a2ebe61 100644
--- a/drivers/scsi/scsi_lib.c
+++ b/drivers/scsi/scsi_lib.c
@@ -2290,3 +2290,29 @@ void scsi_kunmap_atomic_sg(void *virt)
kunmap_atomic(virt, KM_BIO_SRC_IRQ);
 }
 EXPORT_SYMBOL(scsi_kunmap_atomic_sg);
+
+int scsi_dma_map(struct device *dev, struct scsi_cmnd *cmd)
   
   Actually, this is redundant.  We make sure the
   shost-shost_gendev.parent is the device which should have been passed
   in to scsi_add_host().
  
  Well, there's perhaps an unintended side-effect with this assumption,
  NPIV-based 'vports' (and their subsequent 'struct device' members) are
  not fully initialized objects.
  
  This is what we've run into while working on our NPIV (based) driver
  with the 'data buffer' accessors works, while queueing the first
  command to an sdev of a freshly created vport:
  
  [  366.860758] Unable to handle kernel NULL pointer dereference at 
   RIP: 
  [  366.860762]  [8020fdf6] check_addr+0x10/0x4a
  [  366.860778] PGD 0 
  [  366.860782] Oops:  [1] SMP 
  [  366.860787] CPU 0 
  [  366.860790] Modules linked in: qla2xxx scsi_transport_fc
  [  366.860798] Pid: 5757, comm: scsi_wq_2 Not tainted 2.6.22-rc6 #4
  [  366.860802] RIP: 0010:[8020fdf6]  [8020fdf6] 
  check_addr+0x10/0x4a
  [  366.860812] RSP: 0018:810073979960  EFLAGS: 00010082
  [  366.860816] RAX:  RBX: 810037cda070 RCX: 
  0024
  [  366.860821] RDX: 7cd86188 RSI: 81007d800ef8 RDI: 
  804c30e1
  [  366.860824] RBP:  R08:  R09: 
  
  [  366.860828] R10:  R11: 0006 R12: 
  0001
  [  366.860834] R13: 81007d800ef8 R14: 810075f9fda0 R15: 
  810076d66af8
  [  366.860839] FS:  () GS:8057() 
  knlGS:
  [  366.860844] CS:  0010 DS: 0018 ES: 0018 CR0: 8005003b
  [  366.860847] CR2:  CR3: 7ec6b000 CR4: 
  06e0
  [  366.860853] Process scsi_wq_2 (pid: 5757, threadinfo 
  810073978000, task 810037c6ce00)
  [  366.860856] Stack:  00011220 8020fea6 
  0246 00e1
  [  366.860866]  00e1 810076908608 810076d66af8 
  803a440c
  [  366.860876]  810076908608 8801cdd2 810076908688 
  810073979a18
  [  366.860885] Call Trace:
  [  366.860891]  [8020fea6] nommu_map_sg+0x76/0x8f
  [  366.860900]  [803a440c] scsi_dma_map+0x45/0x5c
  [  366.860922]  [8801cdd2] 
  :qla2xxx:qla24xx_start_scsi+0x118/0x4fb
  [  366.860928]  [8039fa74] scsi_done+0x0/0x18
  [  366.860942]  [880116f0] 
  :qla2xxx:qla24xx_queuecommand+0x148/0x17a
  [  366.860948]  [803a01f7] scsi_dispatch_cmd+0x1f6/0x313
  [  366.860953]  [803a622b] scsi_request_fn+0x1d7/0x3b8
  
  the failure point is this check (line 16) in check_addr():
  
  (gdb) l* check_addr+0x10
  0x8020fdf6 is in check_addr (arch/x86_64/kernel/pci-nommu.c:16).
  11  #include asm/dma.h
  12
  13  static int
  14  check_addr(char *name, struct device *hwdev, dma_addr_t bus, 
  size_t size)
  15  {
  16  if (hwdev  bus + size  *hwdev-dma_mask) {
  17  if (*hwdev-dma_mask = DMA_32BIT_MASK)
  18  printk(KERN_ERR
  19  nommu_%s: overflow %Lx+%zu of 
  device mask %Lx\n,
  20  name, (long long)bus, size,
  
  hwdev-dma_mask (an 'u64 *') is NULL, as a corresponding 'set'
  function is never getting called via pci_set_dma_mask() and
  pci_set_consistent_dma_mask() on a vport.
 
 Yeah, I use lpfc's NPIV but haven't hit this because I use

Re: [PATCH 1/19] add data buffer accessors

2007-05-14 Thread Benny Halevy
FUJITA Tomonori wrote:
 +#define scsi_for_each_sg(cmd, sg, nseg, __i) \
 + for (__i = 0, sg = scsi_sglist(cmd); __i  (nseg); __i++, (sg)++)
 +

This feels like a layering violation, why not use for_each_sg()?

+#define scsi_for_each_sg(cmd, sg, nseg, __i)   \
for_each_sg(scsi_sglist(cmd), (sg), (nseg), (__i))  \

That said, I'm not sure that scsi_for_each_sg() is worth abstracting
since the caller can just as well do for_each_sg() directly
as sketched above...

Benny
-
To unsubscribe from this list: send the line unsubscribe linux-scsi in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH 1/19] add data buffer accessors

2007-05-14 Thread Benny Halevy
FUJITA Tomonori wrote:
 From: Benny Halevy [EMAIL PROTECTED]
 Subject: Re: [PATCH 1/19] add data buffer accessors
 Date: Mon, 14 May 2007 10:57:08 +0300
 
 FUJITA Tomonori wrote:
 +#define scsi_for_each_sg(cmd, sg, nseg, __i)   \
 +   for (__i = 0, sg = scsi_sglist(cmd); __i  (nseg); __i++, (sg)++)
 +
 This feels like a layering violation, why not use for_each_sg()?

 +#define scsi_for_each_sg(cmd, sg, nseg, __i)\
  for_each_sg(scsi_sglist(cmd), (sg), (nseg), (__i))  \
 
 As I said before, when for_each_sg is ready, we'll convert
 scsi_for_each_sg to use for_each_sg.

thanks. works for me.

 
 
 That said, I'm not sure that scsi_for_each_sg() is worth abstracting
 since the caller can just as well do for_each_sg() directly
 as sketched above...
 
 I'm not sure why you think it's a layering violation.

I'd like to think of struct scatterlist as an abstract data type
with its own traversal method that hides its internals.
Not a layer per-se but more of an abstraction...

 
 With scsi_for_each_sg(), many drivers don't need scsi_sglist().

Sure, just my two cents...

-
To unsubscribe from this list: send the line unsubscribe linux-scsi in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH 1/19] add data buffer accessors

2007-05-13 Thread Stefan Richter
FUJITA Tomonori wrote:
 From: Christoph Hellwig [EMAIL PROTECTED]
...
 As James already said the device argument should not be needed at all.
...
 +int scsi_dma_map(struct scsi_cmnd *cmd)
 +{
 + int nseg = 0;
 +
 + if (scsi_sg_count(cmd)) {
 + struct device *dev = cmd-device-host-shost_gendev.parent;
...

As a side note:

What is currently not supported by SCSI mid layer is that an sdev could
be moved from one initiator port to another.  If that happens, transport
layer drivers have to destroy the sdev and create a new one.  Or they
could represent /all/ initiator ports by a single shost, but then they
can't use the two new scsi_dma_ wrappers.  (Assuming there aren't any
other issues which would prevent the single-shost approach.)
-- 
Stefan Richter
-=-=-=== -=-= -==-=
http://arcgraph.de/sr/
-
To unsubscribe from this list: send the line unsubscribe linux-scsi in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH 1/19] add data buffer accessors

2007-05-12 Thread James Bottomley
On Sat, 2007-05-12 at 19:05 +0900, FUJITA Tomonori wrote:
 Add a set of accessors for the scsi data buffer. This is in
 preparation for chaining sg lists and bidirectional requests (and
 possibly, the mid-layer dma mapping).
 
 Signed-off-by: FUJITA Tomonori [EMAIL PROTECTED]
 ---
  drivers/scsi/scsi_lib.c  |   26 ++
  include/scsi/scsi_cmnd.h |   11 +++
  2 files changed, 37 insertions(+), 0 deletions(-)
 
 diff --git a/drivers/scsi/scsi_lib.c b/drivers/scsi/scsi_lib.c
 index 1f5a07b..a2ebe61 100644
 --- a/drivers/scsi/scsi_lib.c
 +++ b/drivers/scsi/scsi_lib.c
 @@ -2290,3 +2290,29 @@ void scsi_kunmap_atomic_sg(void *virt)
   kunmap_atomic(virt, KM_BIO_SRC_IRQ);
  }
  EXPORT_SYMBOL(scsi_kunmap_atomic_sg);
 +
 +int scsi_dma_map(struct device *dev, struct scsi_cmnd *cmd)

Actually, this is redundant.  We make sure the
shost-shost_gendev.parent is the device which should have been passed
in to scsi_add_host().

 +{
 + int nseg = 0;
 +
 + if (cmd-use_sg) {
 + struct scatterlist *sg =
 + (struct scatterlist *) cmd-request_buffer;
 +
 + nseg = dma_map_sg(dev, sg, cmd-use_sg, cmd-sc_data_direction);
 + if (unlikely(!nseg))
 + return -ENOMEM;
 + }
 + return nseg;
 +}
 +EXPORT_SYMBOL(scsi_dma_map);
 +
 +void scsi_dma_unmap(struct device *dev, struct scsi_cmnd *cmd)

Same here.

James


-
To unsubscribe from this list: send the line unsubscribe linux-scsi in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH 1/19] add data buffer accessors

2007-05-12 Thread Christoph Hellwig
 +int scsi_dma_map(struct device *dev, struct scsi_cmnd *cmd)
 +{
 + int nseg = 0;
 +
 + if (cmd-use_sg) {
 + struct scatterlist *sg =
 + (struct scatterlist *) cmd-request_buffer;
 +
 + nseg = dma_map_sg(dev, sg, cmd-use_sg, cmd-sc_data_direction);
 + if (unlikely(!nseg))
 + return -ENOMEM;
 + }
 + return nseg;
 +}
 +EXPORT_SYMBOL(scsi_dma_map);

As James already said the device argument should not be needed at all.
Also please add kerneldoc comment describing exported functions.

-
To unsubscribe from this list: send the line unsubscribe linux-scsi in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH 1/19] add data buffer accessors

2007-05-12 Thread FUJITA Tomonori
From: Christoph Hellwig [EMAIL PROTECTED]
Subject: Re: [PATCH 1/19] add data buffer accessors
Date: Sat, 12 May 2007 16:31:51 +0100

  +int scsi_dma_map(struct device *dev, struct scsi_cmnd *cmd)
  +{
  +   int nseg = 0;
  +
  +   if (cmd-use_sg) {
  +   struct scatterlist *sg =
  +   (struct scatterlist *) cmd-request_buffer;
  +
  +   nseg = dma_map_sg(dev, sg, cmd-use_sg, cmd-sc_data_direction);
  +   if (unlikely(!nseg))
  +   return -ENOMEM;
  +   }
  +   return nseg;
  +}
  +EXPORT_SYMBOL(scsi_dma_map);
 
 As James already said the device argument should not be needed at all.
 Also please add kerneldoc comment describing exported functions.

Thanks. How about the this one?

I've also updated the drivers though I don't send them since the
changes are just about scsi_dma_map/unmap():

git://git.kernel.org/pub/scm/linux/kernel/git/tomo/linux-2.6-bidi.git cleanups


James, do you want me to send them to the mailing list, send them
after unifying two patches for each driver, or wait for other guys'
comments for for some time?


From 8fc2ab69fc110117b8a6a0af8fb6ba2a784b918e Mon Sep 17 00:00:00 2001
From: FUJITA Tomonori [EMAIL PROTECTED]
Date: Sun, 13 May 2007 03:34:57 +0900
Subject: [PATCH] add data buffer accessors

This adds a set of accessors for the scsi data buffer. This is in
preparation for chaining sg lists and bidirectional requests (and
possibly, the mid-layer dma mapping).

Signed-off-by: FUJITA Tomonori [EMAIL PROTECTED]
---
 drivers/scsi/scsi_lib.c  |   38 ++
 include/scsi/scsi_cmnd.h |   11 +++
 2 files changed, 49 insertions(+), 0 deletions(-)

diff --git a/drivers/scsi/scsi_lib.c b/drivers/scsi/scsi_lib.c
index 1f5a07b..70454b4 100644
--- a/drivers/scsi/scsi_lib.c
+++ b/drivers/scsi/scsi_lib.c
@@ -2290,3 +2290,41 @@ void scsi_kunmap_atomic_sg(void *virt)
kunmap_atomic(virt, KM_BIO_SRC_IRQ);
 }
 EXPORT_SYMBOL(scsi_kunmap_atomic_sg);
+
+/**
+ * scsi_dma_map - perform DMA mapping against command's sg lists
+ * @cmd:   scsi command
+ *
+ * Returns the number of sg lists actually used, zero if the sg lists
+ * is NULL, or -ENOMEM if the mapping failed.
+ */
+int scsi_dma_map(struct scsi_cmnd *cmd)
+{
+   int nseg = 0;
+
+   if (scsi_sg_count(cmd)) {
+   struct device *dev = cmd-device-host-shost_gendev.parent;
+
+   nseg = dma_map_sg(dev, scsi_sglist(cmd), scsi_sg_count(cmd),
+ cmd-sc_data_direction);
+   if (unlikely(!nseg))
+   return -ENOMEM;
+   }
+   return nseg;
+}
+EXPORT_SYMBOL(scsi_dma_map);
+
+/**
+ * scsi_dma_unmap - unmap command's sg lists mapped by scsi_dma_map
+ * @cmd:   scsi command
+ */
+void scsi_dma_unmap(struct scsi_cmnd *cmd)
+{
+   if (scsi_sg_count(cmd)) {
+   struct device *dev = cmd-device-host-shost_gendev.parent;
+
+   dma_unmap_sg(dev, scsi_sglist(cmd), scsi_sg_count(cmd),
+cmd-sc_data_direction);
+   }
+}
+EXPORT_SYMBOL(scsi_dma_unmap);
diff --git a/include/scsi/scsi_cmnd.h b/include/scsi/scsi_cmnd.h
index a2e0c10..996ff36 100644
--- a/include/scsi/scsi_cmnd.h
+++ b/include/scsi/scsi_cmnd.h
@@ -135,4 +135,15 @@ extern void scsi_kunmap_atomic_sg(void *
 extern struct scatterlist *scsi_alloc_sgtable(struct scsi_cmnd *, gfp_t);
 extern void scsi_free_sgtable(struct scatterlist *, int);
 
+extern int scsi_dma_map(struct scsi_cmnd *cmd);
+extern void scsi_dma_unmap(struct scsi_cmnd *cmd);
+
+#define scsi_sg_count(cmd) ((cmd)-use_sg)
+#define scsi_sglist(cmd) ((struct scatterlist *)(cmd)-request_buffer)
+#define scsi_bufflen(cmd) ((cmd)-request_bufflen)
+#define scsi_resid(cmd) ((cmd)-resid)
+
+#define scsi_for_each_sg(cmd, sg, nseg, __i)   \
+   for (__i = 0, sg = scsi_sglist(cmd); __i  (nseg); __i++, (sg)++)
+
 #endif /* _SCSI_SCSI_CMND_H */
-- 
1.4.3.2


-
To unsubscribe from this list: send the line unsubscribe linux-scsi in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html