regression breaks lowmem reserved RAM

PATCH Andrea Arcangeli 3 participants 4 messages Mar 28, 2008 – Apr 1, 2008

Subsystems (2)

KVM Block layer

lore

Share & tools

lore

Andrea Arcangeli

author

2008-03-28 19:49 UTC #0 lore raw

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 <[email protected]>
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

block/blk-settings.c +1 -1

@@ -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

-------------------------------------------------------------------------
Check out the new SourceForge.net Marketplace.
It's the best place to buy or sell services for
just about anything Open Source.
http://ad.doubleclick.net/clk;164216239;13503038;w?http://sf.net/marketplace

Andrea Arcangeli

author

2008-04-01 10:53 UTC #1 lore raw

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 <[email protected]>

block/blk-settings.c +1 -1

index 1344a0e..5713f7e 100644
@@ -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

yshi

2008-04-01 11:27 UTC #2 lore raw

Hi Andrea,

This patch works well on my machine. Thanks.

Best Regards,
Yang

Andrea Arcangeli ??:

Quoted 32 lines

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 <[email protected]>
block/blk-settings.c +0 -0
index 1344a0e..5713f7e 100644

block/blk-settings.c void blk_queue_bounce_limit(struct request_queue *q, u64 dma_addr) · line 146

Quoted 8 lines

block/blk-settings.c +1 -1

@@ -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

Jens Axboe

2008-04-01 12:45 UTC #3 lore raw

In reply to Andrea Arcangeli

Quoted 17 lines · 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.

Quoted 7 lines

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 <[email protected]>

Thanks Andrea, this looks much saner!

Signature · 1 line

Jens Axboe