Re: [PATCH] block: set the bounce_pfn to the actual DMA limit rather than to max memory

Re: [PATCH] block: set the bounce_pfn to the actual DMA limit rather than to max memory

[Luck, Tony]

Starting with the next-20100927 tag of linux-next I saw this error
while booting ia64:

  Unable to handle kernel NULL pointer dereference (address 0000000000000020)
  usb-stor-scan[5915]: Oops 8813272891392 [1]
  Modules linked in: dm_mod usb_storage sg container button usbhid uhci_hcd ehci_hcd usbcore fan processor thermal thermal_sys

  Pid: 5915, CPU 0, comm:        usb-stor-scan
  psr : 00001010085a6010 ifs : 8000000000000894 ip  : [<a00000010012a630>]    Not tainted (2.6.36-rc5-generic-smp-next-20100927)
  ip is at mempool_alloc+0x70/0x200

The problem was that "page_pool" was NULL, but blk_queue_bounce()
had decided to use it!  The code in mm/bounce.c looks quite
fragile here as there are several places where page_pool is used, but
it is only initialized inside #ifdef CONFIG_HIGHMEM (which is not
set on ia64).

Reverting this patch so that the old rules for setting
q->limits.bounce_pfn are used cures the immediate problem.
But I think there must be some deeper issues involved.

I think that reverting means that I take the fast exit
from blk_queue_bounce():

	if (queue_bounce_pfn(q) >= blk_max_pfn)
		return;

which is probably just avoiding the problem, rather than
doing the right thing.

-Tony

Re: [PATCH] block: set the bounce_pfn to the actual DMA limit rather than to max memory

[Malahal Naineni]

Luck, Tony [tony.luck@intel.com] wrote:
> Starting with the next-20100927 tag of linux-next I saw this error
> while booting ia64:
> 
>   Unable to handle kernel NULL pointer dereference (address 0000000000000020)
>   usb-stor-scan[5915]: Oops 8813272891392 [1]
>   Modules linked in: dm_mod usb_storage sg container button usbhid uhci_hcd ehci_hcd usbcore fan processor thermal thermal_sys
> 
>   Pid: 5915, CPU 0, comm:        usb-stor-scan
>   psr : 00001010085a6010 ifs : 8000000000000894 ip  : [<a00000010012a630>]    Not tainted (2.6.36-rc5-generic-smp-next-20100927)
>   ip is at mempool_alloc+0x70/0x200
> 
> The problem was that "page_pool" was NULL, but blk_queue_bounce()
> had decided to use it!  The code in mm/bounce.c looks quite
> fragile here as there are several places where page_pool is used, but
> it is only initialized inside #ifdef CONFIG_HIGHMEM (which is not
> set on ia64).
> 
> Reverting this patch so that the old rules for setting
> q->limits.bounce_pfn are used cures the immediate problem.
> But I think there must be some deeper issues involved.
> 
> I think that reverting means that I take the fast exit
> from blk_queue_bounce():
> 
> 	if (queue_bounce_pfn(q) >= blk_max_pfn)
> 		return;

Let me know if this fixes the problem. Thank you very much.


diff -r 1a48e21f1e50 drivers/scsi/scsi_lib.c
--- a/drivers/scsi/scsi_lib.c	Fri Sep 24 09:44:52 2010 -0700
+++ b/drivers/scsi/scsi_lib.c	Tue Sep 28 15:55:10 2010 -0700
@@ -1590,7 +1590,7 @@ static void scsi_request_fn(struct reque
 u64 scsi_calculate_bounce_limit(struct Scsi_Host *shost)
 {
 	struct device *host_dev;
-	u64 bounce_limit = 0xffffffff;
+	u64 bounce_limit = BLK_BOUNCE_HIGH;
 
 	if (shost->unchecked_isa_dma)
 		return BLK_BOUNCE_ISA;

RE: [PATCH] block: set the bounce_pfn to the actual DMA limit rather than to max memory

[Luck, Tony]

> Let me know if this fixes the problem. Thank you very much.

No it doesn't :-(

I still end up with 0xffffffff from the *host_dev->dma_mask
later in that function.

Just for two devices though ... stack traces lead back to
usb_stor_scan_thread() for these two.

-Tony

Re: [PATCH] block: set the bounce_pfn to the actual DMA limit rather than to max memory

[Malahal Naineni]

Luck, Tony [tony.luck@intel.com] wrote:
> > Let me know if this fixes the problem. Thank you very much.
> 
> No it doesn't :-(
> 
> I still end up with 0xffffffff from the *host_dev->dma_mask
> later in that function.
> 
> Just for two devices though ... stack traces lead back to
> usb_stor_scan_thread() for these two.

What is the USB host controller's PCI device and vendor IDs? I see
ehci-ps3.c  and ohci-ps3.c setting it to DMA_BIT_MASK(32). That may
present another bug???

I don't know USB much, but someone seems to be lying that they can't do DMA
beyond 4GB!

How much memory you have on the system? This probably doesn't matter but
good to know.

Thanks, Malahal.

RE: [PATCH] block: set the bounce_pfn to the actual DMA limit rather than to max memory

[Luck, Tony]

>What is the USB host controller's PCI device and vendor IDs? I see
>ehci-ps3.c  and ohci-ps3.c setting it to DMA_BIT_MASK(32). That may
>present another bug???

I have a pair of ehci devices:

00:1a.7 USB Controller: Intel Corporation 82801JI (ICH10 Family) USB2 EHCI Controller #2 (prog-if 20 [EHCI])
        Subsystem: Platform Technologies, Inc. Device 6249

00:1d.7 USB Controller: Intel Corporation 82801JI (ICH10 Family) USB2 EHCI Controller #1 (prog-if 20 [EHCI])
        Subsystem: Device 2920:0034

>I don't know USB much, but someone seems to be lying that they can't do
>DMA beyond 4GB!

The drivers may be confused about this.  ia64 systems support DMA to
all addresses - either because they have an IOMMU that makes DMA
possible to any address, or because they use swiotlb to pretend
that they can.

You are probably correct that this is the real source of the bug.
Your patch just stopped the block layer from hiding the problem.

>How much memory you have on the system? This probably doesn't matter but
>good to know.

This is a little machine - just 16GB.

-Tony

Re: [PATCH] block: set the bounce_pfn to the actual DMA limit rather than to max memory

[Malahal Naineni]

Luck, Tony [tony.luck@intel.com] wrote:
> >What is the USB host controller's PCI device and vendor IDs? I see
> >ehci-ps3.c  and ohci-ps3.c setting it to DMA_BIT_rMASK(32). That may
> >present another bug???
> e
> I have a pair of ehci devices:
> 
> 00:1a.7 USB Controller: Intel Corporation 82801JI (ICH10 Family) USB2 EHCI Controller #2 (prog-if 20 [EHCI])
>         Subsystem: Platform Technologies, Inc. Device 6249
> 
> 00:1d.7 USB Controller: Intel Corporation 82801JI (ICH10 Family) USB2 EHCI Controller #1 (prog-if 20 [EHCI])
>         Subsystem: Device 2920:0034
> 
> >I don't know USB much, but someone seems to be lying that they can't do
> >DMA beyond 4GB!
> 
> The drivers may be confused about this.  ia64 systems support DMA to
> all addresses - either because they have an IOMMU that makes DMA
> possible to any address, or because they use swiotlb to pretend
> that they can.

That makes sense! This patch should work and considers the above
behaviour just as the old code. Please revert the commit and apply this
patch instead. Please let me know if this works.


block: set the bounce_pfn to the actual DMA limit rather than to max memory

The bounce_pfn of the request queue in 64 bit systems is set to the
current max_low_pfn. Adding more memory later makes this incorrect.
Memory allocated beyond this boot time max_low_pfn appear to require
bounce buffers (bounce buffers are actually not allocated but used in
calculating segments that may result in "over max segments limit"
errors).

Signed-off-by: Malahal Naineni (malahal@us.ibm.com)

diff -r 09daf852c1c5 block/blk-settings.c
--- a/block/blk-settings.c	Thu Sep 09 12:10:43 2010 -0700
+++ b/block/blk-settings.c	Tue Sep 28 22:43:40 2010 -0700
@@ -214,7 +214,7 @@ void blk_queue_bounce_limit(struct reque
 	 */
 	if (b_pfn < (min_t(u64, 0xffffffffUL, BLK_BOUNCE_HIGH) >> PAGE_SHIFT))
 		dma = 1;
-	q->limits.bounce_pfn = max_low_pfn;
+	q->limits.bounce_pfn = max(max_low_pfn, b_pfn);
 #else
 	if (b_pfn < blk_max_low_pfn)
 		dma = 1;

RE: [PATCH] block: set the bounce_pfn to the actual DMA limit rather than to max memory

[Luck, Tony]

> That makes sense! This patch should work and considers the above
> behaviour just as the old code. Please revert the commit and apply this
> patch instead. Please let me know if this works.

Yes, this new version works.

Tested-by: Tony Luck <tony.luck@intel.com>

Thanks

-Tony