Re: [PATCH] [scsi] Remove __GFP_DMA

2007-05-27 Thread James Bottomley
On Wed, 2007-05-23 at 20:38 +0100, Alan Cox wrote:
 On Wed, 23 May 2007 15:17:08 -0400
 Salyzyn, Mark [EMAIL PROTECTED] wrote:
 
  The 31 bit limit for some of these cards is a problem, we currently only
  do __GFP_DMA for bounce buffer sg elements allocated for user supplied
  references in ioctls.
  
  I figure we should be using pci_alloc_consistent calls for these
  allocations to more accurately acquire memory within the 31 bit limit if
  necessary, we could switch to these to remove the need for the __GFP_DMA
  flag in the aacraid driver?
 
 That didn't used to work right on the AMD boards when I tried it last as
 we ended up with a buffer that was mapped by the IOMMU for some reason
 and that was not below 2GB.

Is that a bug in the x86_64 subsystem ... or just a bug in the HW?

I'm assuming what happened is that the GART aperture was placed above
the 2GB limit making the GART unable to act as an IOMMU for any devices
with a dma_mask  32bit?

This does show up a problem in our dma_ functions: namely that if the
system says it has an IOMMU then we assume it's fully functional (i.e.
able to reach all addresses).  There was a move afoot a while ago to
replace the single PCI_DMA_BUS_IS_PHYS macro which is used to tell the
system globally if we have an IOMMU with something like
dma_dev_uses_iommu(struct device *) which would do the same thing but on
a per-device basis.  Does this need to be resurrected?

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] [scsi] Remove __GFP_DMA

2007-05-24 Thread Alan Cox
  That didn't used to work right on the AMD boards when I tried it last as
  we ended up with a buffer that was mapped by the IOMMU for some reason
  and that was not below 2GB.
 
 The physical address you mean? If that is still happening then it needs 
 to get fixed. The allocation should not succeed if it can't provide 
 memory that's inside the DMA mask for the device..

But the allocation can succeed - using GFP_DMA at least you can do it as
you get memory below 2^24 you don't need to map via the IOMMU
-
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] [scsi] Remove __GFP_DMA

2007-05-24 Thread Salyzyn, Mark
So, is the sequence:

p = kmalloc(upsg-sg[i].count,GFP_KERNEL);
. . .
addr = pci_map_single(dev-pdev, p, upsg-sg[i].count,
data_dir);

Going to ensure that we have a 31 bit (not 32 bit) physical address?

If not, then I reject this patch. We can not consider replacement with
pci_alloc_consistent until it works on AMD respecting the DMA masks.

Sincerely -- Mark Salyzyn

 -Original Message-
 From: [EMAIL PROTECTED] 
 [mailto:[EMAIL PROTECTED] On Behalf Of Aubrey Li
 Sent: Tuesday, May 22, 2007 10:41 PM
 To: Christoph Lameter
 Cc: Bernhard Walle; linux-scsi@vger.kernel.org; Andrew 
 Morton; [EMAIL PROTECTED]; James Bottomley
 Subject: Re: [PATCH] [scsi] Remove __GFP_DMA
 
 
 On 5/23/07, Christoph Lameter [EMAIL PROTECTED] wrote:
  On Mon, 21 May 2007, Bernhard Walle wrote:
 
   [PATCH] [scsi] Remove __GFP_DMA
  
   After 821de3a27bf33f11ec878562577c586cd5f83c64, it's not 
 necessary to alloate a
   DMA buffer any more in sd.c.
  
   Signed-off-by: Bernhard Walle [EMAIL PROTECTED]
 
  Great that avoids a DMA kmalloc slab. Any other GFP_DMAs 
 left in the scsi
  layer?
 
  Acked-by: Christoph Lameter [EMAIL PROTECTED]
 
 Yes, here is another patch
 
 Signed-off-by: Aubrey.Li [EMAIL PROTECTED]
 ---
  drivers/scsi/aacraid/commctrl.c |   12 ++--
  1 files changed, 6 insertions(+), 6 deletions(-)
 
 diff --git a/drivers/scsi/aacraid/commctrl.c 
 b/drivers/scsi/aacraid/commctrl.c
 index 72b0393..405722d 100644
 --- a/drivers/scsi/aacraid/commctrl.c
 +++ b/drivers/scsi/aacraid/commctrl.c
 @@ -580,8 +580,8 @@ static int aac_send_raw_srb(struct aac_dev* dev,
 void __user * arg)
   for (i = 0; i  upsg-count; i++) {
   u64 addr;
   void* p;
 - /* Does this really need to be 
 GFP_DMA? */
 - p = 
 kmalloc(upsg-sg[i].count,GFP_KERNEL|__GFP_DMA);
 +
 + p = 
 kmalloc(upsg-sg[i].count,GFP_KERNEL);
   if(p == 0) {
   
 dprintk((KERN_DEBUGaacraid: Could not allocate SG buffer - size
 = %d buffer number %d of %d\n,
 
 upsg-sg[i].count,i,upsg-count));
 @@ -624,8 +624,8 @@ static int aac_send_raw_srb(struct aac_dev* dev,
 void __user * arg)
   for (i = 0; i  usg-count; i++) {
   u64 addr;
   void* p;
 - /* Does this really need to be 
 GFP_DMA? */
 - p = 
 kmalloc(usg-sg[i].count,GFP_KERNEL|__GFP_DMA);
 +
 + p = 
 kmalloc(usg-sg[i].count,GFP_KERNEL);
   if(p == 0) {
   kfree (usg);
   
 dprintk((KERN_DEBUGaacraid: Could not allocate SG buffer - size
 = %d buffer number %d of %d\n,
 @@ -666,8 +666,8 @@ static int aac_send_raw_srb(struct aac_dev* dev,
 void __user * arg)
   for (i = 0; i  upsg-count; i++) {
   u64 addr;
   void* p;
 - /* Does this really need to be 
 GFP_DMA? */
 - p = 
 kmalloc(usg-sg[i].count,GFP_KERNEL|__GFP_DMA);
 +
 + p = 
 kmalloc(usg-sg[i].count,GFP_KERNEL);
   if(p == 0) {
   
 dprintk((KERN_DEBUGaacraid: Could not allocate SG buffer - size
 = %d buffer number %d of %d\n,
 
 usg-sg[i].count,i,usg-count));
 -- 
 1.5.1.1
 -
 To unsubscribe from this list: send the line unsubscribe 
 linux-kernel in
 the body of a message to [EMAIL PROTECTED]
 More majordomo info at  http://vger.kernel.org/majordomo-info.html
 Please read the FAQ at  http://www.tux.org/lkml/
 
-
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] [scsi] Remove __GFP_DMA

2007-05-24 Thread James Bottomley
On Thu, 2007-05-24 at 09:24 -0400, Salyzyn, Mark wrote:
 So, is the sequence:
 
   p = kmalloc(upsg-sg[i].count,GFP_KERNEL);
   . . .
   addr = pci_map_single(dev-pdev, p, upsg-sg[i].count,
 data_dir);
 
 Going to ensure that we have a 31 bit (not 32 bit) physical address?

No, unfortunately.  Implementing kmalloc_mask() and kmalloc_dev() was
something I said I'd do ... about two years ago.

 If not, then I reject this patch. We can not consider replacement with
 pci_alloc_consistent until it works on AMD respecting the DMA masks.

It should, I believe ... x86_64 has a complex allocation scheme where
for masks  32 bit it first tries in GFP_DMA32 and sees if it gets lucky
before falling back to GFP_DMA. i386 just goes straight to GFP_DMA.

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] [scsi] Remove __GFP_DMA

2007-05-24 Thread Christoph Lameter
On Thu, 24 May 2007, Salyzyn, Mark wrote:

 So, is the sequence:
 
   p = kmalloc(upsg-sg[i].count,GFP_KERNEL);
   . . .
   addr = pci_map_single(dev-pdev, p, upsg-sg[i].count,
 data_dir);
 
 Going to ensure that we have a 31 bit (not 32 bit) physical address?

Only if you have less than 2G of memory. So no.

-
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] [scsi] Remove __GFP_DMA

2007-05-24 Thread Christoph Lameter
On Thu, 24 May 2007, James Bottomley wrote:

  Going to ensure that we have a 31 bit (not 32 bit) physical address?
 
 No, unfortunately.  Implementing kmalloc_mask() and kmalloc_dev() was
 something I said I'd do ... about two years ago.

Tell me more about these ideas. 
-
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] [scsi] Remove __GFP_DMA

2007-05-24 Thread James Bottomley
On Thu, 2007-05-24 at 10:00 -0700, Christoph Lameter wrote:
 On Thu, 24 May 2007, James Bottomley wrote:
 
   Going to ensure that we have a 31 bit (not 32 bit) physical address?
  
  No, unfortunately.  Implementing kmalloc_mask() and kmalloc_dev() was
  something I said I'd do ... about two years ago.
 
 Tell me more about these ideas. 

Oh, it was this Kernel Summit presentation which discussed it

http://licensing.steeleye.com/support/papers/kernel_summit_iommu.pdf

The writeup of the session is here:

http://lwn.net/Articles/144100/

The idea was basically to match an allocation to a device mask.  I was
going to do a generic implementation (which would probably kmalloc,
check the physaddr and fall back to GFP_DMA if we were unlucky) but
allow the architectures to override.

However, the majority of need (except for the aacraid like devices) was
solved by GFP_DMA32

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] [scsi] Remove __GFP_DMA

2007-05-24 Thread Christoph Lameter
On Thu, 24 May 2007, James Bottomley wrote:

 The idea was basically to match an allocation to a device mask.  I was
 going to do a generic implementation (which would probably kmalloc,
 check the physaddr and fall back to GFP_DMA if we were unlucky) but
 allow the architectures to override.

H... We could actually implement something like it in the slab 
allocators. The mask parameter would lead the allocator to check if the 
objects are in a satisfactory range. If not it could examine its partial 
lists for slabs that satisfy the range. If that does not work then it 
would eventually go to the page allocator to ask for a page in a fitting 
range.

That wont be fast though. How performance sensitive are the allocations?

-
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] [scsi] Remove __GFP_DMA

2007-05-24 Thread James Bottomley
On Thu, 2007-05-24 at 10:22 -0700, Christoph Lameter wrote:
 On Thu, 24 May 2007, James Bottomley wrote:
 
  The idea was basically to match an allocation to a device mask.  I was
  going to do a generic implementation (which would probably kmalloc,
  check the physaddr and fall back to GFP_DMA if we were unlucky) but
  allow the architectures to override.
 
 H... We could actually implement something like it in the slab 
 allocators. The mask parameter would lead the allocator to check if the 
 objects are in a satisfactory range. If not it could examine its partial 
 lists for slabs that satisfy the range. If that does not work then it 
 would eventually go to the page allocator to ask for a page in a fitting 
 range.
 
 That wont be fast though. How performance sensitive are the allocations?

Most of them aren't very.  They're usually things that are set at driver
intialisation time (shared mailbox memory etc).

If the allocation is performance sensitive and has to be done at command
or ioctl submission time, we tend to feed the consistent or mapped
memory into a dma pool which pre allocates and solves the performance
issue.

In the aacraid case, the mbox is done at ioctl time, but isn't
incredibly performance sensitive.

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] [scsi] Remove __GFP_DMA

2007-05-23 Thread James Bottomley
On Wed, 2007-05-23 at 10:41 +0800, Aubrey Li wrote:
 On 5/23/07, Christoph Lameter [EMAIL PROTECTED] wrote:
  On Mon, 21 May 2007, Bernhard Walle wrote:
 
   [PATCH] [scsi] Remove __GFP_DMA
  
   After 821de3a27bf33f11ec878562577c586cd5f83c64, it's not necessary to 
   alloate a
   DMA buffer any more in sd.c.
  
   Signed-off-by: Bernhard Walle [EMAIL PROTECTED]
 
  Great that avoids a DMA kmalloc slab. Any other GFP_DMAs left in the scsi
  layer?
 
  Acked-by: Christoph Lameter [EMAIL PROTECTED]
 
 Yes, here is another patch

I'll defer to Mark on this one.  However, please remember that you can't
just blindly remove GFP_DMA ... there are some cards which require it.

Aacraid is one example ... it has a set of cards that can only DMA to 31
bits.  For them, the GFP_DMA is necessary:  The allocation in question
is a scatterlist, which must be within the device DMA mask.

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] [scsi] Remove __GFP_DMA

2007-05-23 Thread James Bottomley
On Wed, 2007-05-23 at 11:55 -0400, Robert P. J. Day wrote:
 On Wed, 23 May 2007, James Bottomley wrote:
 
  I'll defer to Mark on this one.  However, please remember that you
  can't just blindly remove GFP_DMA ... there are some cards which
  require it.
 
  Aacraid is one example ... it has a set of cards that can only DMA
  to 31 bits.  For them, the GFP_DMA is necessary:  The allocation in
  question is a scatterlist, which must be within the device DMA mask.
 
 a question i asked a while back, and still haven't seen an answer for
 -- given this in include/linux/gfp.h:
 
   #define GFP_DMA __GFP_DMA
 
 is there a qualitative difference between these two macros?  is there
 *supposed* to be?  if there isn't, one would think that just one
 variation would be sufficient.

__GFP_ are the raw GFP flags ... the GFP_ are combinations of the flags
with predefined meanings.  There's no convention that you have to use
one form or the other when making combinations of the allocation flags.
Historically that's lead to things like

GFP_ATOMIC | __GFP_DMA (indicating additional DMA zone to the usual
atomic flags)

and

GFP_ATOMIC | GFP_DMA (indicating same thing, but with the defined flags)

You can argue that GFP_DMA has a pretty bogus GFP meaning and should
never appear on its own, which, to my mind makes the former usage
preferable.  However, GFP_DMA has been in linux since pretty much the
dawn of ISA drivers, so I think it's conventionally well understood.

James


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] [scsi] Remove __GFP_DMA

2007-05-23 Thread Salyzyn, Mark
The 31 bit limit for some of these cards is a problem, we currently only
do __GFP_DMA for bounce buffer sg elements allocated for user supplied
references in ioctls.

I figure we should be using pci_alloc_consistent calls for these
allocations to more accurately acquire memory within the 31 bit limit if
necessary, we could switch to these to remove the need for the __GFP_DMA
flag in the aacraid driver?

-- Mark

-Original Message-
From: [EMAIL PROTECTED]
[mailto:[EMAIL PROTECTED] On Behalf Of James Bottomley
Sent: Wednesday, May 23, 2007 11:18 AM
To: Aubrey Li
Cc: Christoph Lameter; Bernhard Walle; linux-scsi@vger.kernel.org;
Andrew Morton; [EMAIL PROTECTED]
Subject: Re: [PATCH] [scsi] Remove __GFP_DMA

On Wed, 2007-05-23 at 10:41 +0800, Aubrey Li wrote:
 On 5/23/07, Christoph Lameter [EMAIL PROTECTED] wrote:
  On Mon, 21 May 2007, Bernhard Walle wrote:
 
   [PATCH] [scsi] Remove __GFP_DMA
  
   After 821de3a27bf33f11ec878562577c586cd5f83c64, it's not necessary
to alloate a
   DMA buffer any more in sd.c.
  
   Signed-off-by: Bernhard Walle [EMAIL PROTECTED]
 
  Great that avoids a DMA kmalloc slab. Any other GFP_DMAs left in the
scsi
  layer?
 
  Acked-by: Christoph Lameter [EMAIL PROTECTED]
 
 Yes, here is another patch

I'll defer to Mark on this one.  However, please remember that you can't
just blindly remove GFP_DMA ... there are some cards which require it.

Aacraid is one example ... it has a set of cards that can only DMA to 31
bits.  For them, the GFP_DMA is necessary:  The allocation in question
is a scatterlist, which must be within the device DMA mask.

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
-
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] [scsi] Remove __GFP_DMA

2007-05-23 Thread Alan Cox
On Wed, 23 May 2007 15:17:08 -0400
Salyzyn, Mark [EMAIL PROTECTED] wrote:

 The 31 bit limit for some of these cards is a problem, we currently only
 do __GFP_DMA for bounce buffer sg elements allocated for user supplied
 references in ioctls.
 
 I figure we should be using pci_alloc_consistent calls for these
 allocations to more accurately acquire memory within the 31 bit limit if
 necessary, we could switch to these to remove the need for the __GFP_DMA
 flag in the aacraid driver?

That didn't used to work right on the AMD boards when I tried it last as
we ended up with a buffer that was mapped by the IOMMU for some reason
and that was not below 2GB.

-
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] [scsi] Remove __GFP_DMA

2007-05-23 Thread Robert Hancock

Alan Cox wrote:

On Wed, 23 May 2007 15:17:08 -0400
Salyzyn, Mark [EMAIL PROTECTED] wrote:


The 31 bit limit for some of these cards is a problem, we currently only
do __GFP_DMA for bounce buffer sg elements allocated for user supplied
references in ioctls.

I figure we should be using pci_alloc_consistent calls for these
allocations to more accurately acquire memory within the 31 bit limit if
necessary, we could switch to these to remove the need for the __GFP_DMA
flag in the aacraid driver?


That didn't used to work right on the AMD boards when I tried it last as
we ended up with a buffer that was mapped by the IOMMU for some reason
and that was not below 2GB.


The physical address you mean? If that is still happening then it needs 
to get fixed. The allocation should not succeed if it can't provide 
memory that's inside the DMA mask for the device..


--
Robert Hancock  Saskatoon, SK, Canada
To email, remove nospam from [EMAIL PROTECTED]
Home Page: http://www.roberthancock.com/

-
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] [scsi] Remove __GFP_DMA

2007-05-22 Thread Christoph Lameter
On Mon, 21 May 2007, Bernhard Walle wrote:

 [PATCH] [scsi] Remove __GFP_DMA
 
 After 821de3a27bf33f11ec878562577c586cd5f83c64, it's not necessary to alloate 
 a
 DMA buffer any more in sd.c.
 
 Signed-off-by: Bernhard Walle [EMAIL PROTECTED]

Great that avoids a DMA kmalloc slab. Any other GFP_DMAs left in the scsi 
layer?

Acked-by: Christoph Lameter [EMAIL PROTECTED]
-
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] [scsi] Remove __GFP_DMA

2007-05-22 Thread Aubrey Li

On 5/23/07, Christoph Lameter [EMAIL PROTECTED] wrote:

On Mon, 21 May 2007, Bernhard Walle wrote:

 [PATCH] [scsi] Remove __GFP_DMA

 After 821de3a27bf33f11ec878562577c586cd5f83c64, it's not necessary to alloate 
a
 DMA buffer any more in sd.c.

 Signed-off-by: Bernhard Walle [EMAIL PROTECTED]

Great that avoids a DMA kmalloc slab. Any other GFP_DMAs left in the scsi
layer?

Acked-by: Christoph Lameter [EMAIL PROTECTED]


Yes, here is another patch

Signed-off-by: Aubrey.Li [EMAIL PROTECTED]
---
drivers/scsi/aacraid/commctrl.c |   12 ++--
1 files changed, 6 insertions(+), 6 deletions(-)

diff --git a/drivers/scsi/aacraid/commctrl.c b/drivers/scsi/aacraid/commctrl.c
index 72b0393..405722d 100644
--- a/drivers/scsi/aacraid/commctrl.c
+++ b/drivers/scsi/aacraid/commctrl.c
@@ -580,8 +580,8 @@ static int aac_send_raw_srb(struct aac_dev* dev,
void __user * arg)
for (i = 0; i  upsg-count; i++) {
u64 addr;
void* p;
-   /* Does this really need to be GFP_DMA? */
-   p = 
kmalloc(upsg-sg[i].count,GFP_KERNEL|__GFP_DMA);
+
+   p = kmalloc(upsg-sg[i].count,GFP_KERNEL);
if(p == 0) {
dprintk((KERN_DEBUGaacraid: Could not 
allocate SG buffer - size
= %d buffer number %d of %d\n,
  upsg-sg[i].count,i,upsg-count));
@@ -624,8 +624,8 @@ static int aac_send_raw_srb(struct aac_dev* dev,
void __user * arg)
for (i = 0; i  usg-count; i++) {
u64 addr;
void* p;
-   /* Does this really need to be GFP_DMA? */
-   p = 
kmalloc(usg-sg[i].count,GFP_KERNEL|__GFP_DMA);
+
+   p = kmalloc(usg-sg[i].count,GFP_KERNEL);
if(p == 0) {
kfree (usg);
dprintk((KERN_DEBUGaacraid: Could not 
allocate SG buffer - size
= %d buffer number %d of %d\n,
@@ -666,8 +666,8 @@ static int aac_send_raw_srb(struct aac_dev* dev,
void __user * arg)
for (i = 0; i  upsg-count; i++) {
u64 addr;
void* p;
-   /* Does this really need to be GFP_DMA? */
-   p = 
kmalloc(usg-sg[i].count,GFP_KERNEL|__GFP_DMA);
+
+   p = kmalloc(usg-sg[i].count,GFP_KERNEL);
if(p == 0) {
dprintk((KERN_DEBUGaacraid: Could not 
allocate SG buffer - size
= %d buffer number %d of %d\n,
  usg-sg[i].count,i,usg-count));
--
1.5.1.1
-
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