regression breaks lowmem reserved RAM

regression breaks lowmem reserved RAM

[Andrea Arcangeli]

This is crashing at boot my lowmem reserved RAM patch. This is causing
GFP_DMA allocations at boot for no good reason. It crashes in my case
because there's no ram below 16M available to linux. Are you sure this
is needed at all, for sure if there's any bug this isn't the right fix.

Please reverse, thanks!

changeset:   87150:1f7afb388483
user:        Yang Shi <yang.shi@windriver.com>
date:        Tue Mar 04 11:20:51 2008 +0100
summary:     Fix DMA access of block device in 64-bit kernel on some non-x86 systems with 4GB or upper 4GB memory

diff --git a/block/blk-settings.c b/block/blk-settings.c
--- a/block/blk-settings.c
+++ b/block/blk-settings.c
@@ -140,7 +140,7 @@ void blk_queue_bounce_limit(struct reque
 	/* Assume anything <= 4GB can be handled by IOMMU.
 	   Actually some IOMMUs can handle everything, but I don't
 	   know of a way to test this here. */
-	if (b_pfn < (min_t(u64, 0xffffffff, BLK_BOUNCE_HIGH) >> PAGE_SHIFT))
+	if (b_pfn <= (min_t(u64, 0xffffffff, BLK_BOUNCE_HIGH) >> PAGE_SHIFT))
 		dma = 1;
 	q->bounce_pfn = max_low_pfn;
 #else

Re: regression breaks lowmem reserved RAM

[Andrea Arcangeli]

Looking a bit closer into this regression the reason this can't be
right is that dma_addr common default is BLK_BOUNCE_HIGH and most
machines have less than 4G. So if you do:

    if (b_pfn <= (min_t(u64, 0xffffffff, BLK_BOUNCE_HIGH) >> PAGE_SHIFT))
	dma = 1

that will translate to:

     if (BLK_BOUNCE_HIGH <= BLK_BOUNCE_HIGH)
     	dma = 1

So for 99% of hardware this will trigger unnecessary GFP_DMA
allocations and isa pooling operations.

Also note how the 32bit code still does b_pfn < blk_max_low_pfn.

I guess this is what you were looking after. I didn't verify but as
far as I can tell, this will stop the regression with isa dma
operations at boot for 99% of blkdev/memory combinations out there and
I guess this fixes the setups with >4G of ram and 32bit pci cards as
well (this also retains symmetry with the 32bit code).

Signed-off-by: Andrea Arcangeli <andrea@qumranet.com>

diff --git a/block/blk-settings.c b/block/blk-settings.c
index 1344a0e..5713f7e 100644
--- a/block/blk-settings.c
+++ b/block/blk-settings.c
@@ -140,7 +140,7 @@ void blk_queue_bounce_limit(struct request_queue *q, u64 dma_addr)
 	/* Assume anything <= 4GB can be handled by IOMMU.
 	   Actually some IOMMUs can handle everything, but I don't
 	   know of a way to test this here. */
-	if (b_pfn <= (min_t(u64, 0xffffffff, BLK_BOUNCE_HIGH) >> PAGE_SHIFT))
+	if (b_pfn < (min_t(u64, 0x100000000UL, BLK_BOUNCE_HIGH) >> PAGE_SHIFT))
 		dma = 1;
 	q->bounce_pfn = max_low_pfn;
 #else

Re: regression breaks lowmem reserved RAM

[yshi]

Hi Andrea,

This patch works well on my machine. Thanks.

Best Regards,
Yang

Andrea Arcangeli ??:
> Looking a bit closer into this regression the reason this can't be
> right is that dma_addr common default is BLK_BOUNCE_HIGH and most
> machines have less than 4G. So if you do:
>
>     if (b_pfn <= (min_t(u64, 0xffffffff, BLK_BOUNCE_HIGH) >> PAGE_SHIFT))
> 	dma = 1
>
> that will translate to:
>
>      if (BLK_BOUNCE_HIGH <= BLK_BOUNCE_HIGH)
>      	dma = 1
>
> So for 99% of hardware this will trigger unnecessary GFP_DMA
> allocations and isa pooling operations.
>
> Also note how the 32bit code still does b_pfn < blk_max_low_pfn.
>
> I guess this is what you were looking after. I didn't verify but as
> far as I can tell, this will stop the regression with isa dma
> operations at boot for 99% of blkdev/memory combinations out there and
> I guess this fixes the setups with >4G of ram and 32bit pci cards as
> well (this also retains symmetry with the 32bit code).
>
> Signed-off-by: Andrea Arcangeli <andrea@qumranet.com>
>
> diff --git a/block/blk-settings.c b/block/blk-settings.c
> index 1344a0e..5713f7e 100644
> --- a/block/blk-settings.c
> +++ b/block/blk-settings.c
> @@ -140,7 +140,7 @@ void blk_queue_bounce_limit(struct request_queue *q, u64 dma_addr)
>  	/* Assume anything <= 4GB can be handled by IOMMU.
>  	   Actually some IOMMUs can handle everything, but I don't
>  	   know of a way to test this here. */
> -	if (b_pfn <= (min_t(u64, 0xffffffff, BLK_BOUNCE_HIGH) >> PAGE_SHIFT))
> +	if (b_pfn < (min_t(u64, 0x100000000UL, BLK_BOUNCE_HIGH) >> PAGE_SHIFT))
>  		dma = 1;
>  	q->bounce_pfn = max_low_pfn;
>  #else
>
>   

Re: regression breaks lowmem reserved RAM

[Jens Axboe]

On Tue, Apr 01 2008, Andrea Arcangeli wrote:
> Looking a bit closer into this regression the reason this can't be
> right is that dma_addr common default is BLK_BOUNCE_HIGH and most
> machines have less than 4G. So if you do:
> 
>     if (b_pfn <= (min_t(u64, 0xffffffff, BLK_BOUNCE_HIGH) >> PAGE_SHIFT))
> 	dma = 1
> 
> that will translate to:
> 
>      if (BLK_BOUNCE_HIGH <= BLK_BOUNCE_HIGH)
>      	dma = 1
> 
> So for 99% of hardware this will trigger unnecessary GFP_DMA
> allocations and isa pooling operations.
> 
> Also note how the 32bit code still does b_pfn < blk_max_low_pfn.
> 
> I guess this is what you were looking after. I didn't verify but as
> far as I can tell, this will stop the regression with isa dma
> operations at boot for 99% of blkdev/memory combinations out there and
> I guess this fixes the setups with >4G of ram and 32bit pci cards as
> well (this also retains symmetry with the 32bit code).
> 
> Signed-off-by: Andrea Arcangeli <andrea@qumranet.com>

Thanks Andrea, this looks much saner!

-- 
Jens Axboe