General DMA zone rework

Background:

The 16MB Linux ZONE_DMA has some long standing problems on x86. Traditionally it was designed only for ISA dma which is limited to 24bit (16MB). This means it has a fixed 16MB size.

On 32bit i386 with its limited virtual memory space the next zone is lowmem with ~900MB (on default split) which works for a lot of devices, but not all. But on x86-64 the next zone is only 4GB (DMA32) which is too big for quite a lot more devices (typically 30,31 or 28bit limitations).

While the DMA zone is in a true VM zone and could be in theory used for any user allocations in practice the VM has a concept called lower zone protection to avoid low memory deadlocks that keeps ZONE_DMA nearly always free unless the caller specifies GFP_DMA directly. This means in practice it does not participate in the rest of the automatic VM balancing and its memory is essentially reserved.

Then there is another pool used on x86-64: the swiotlb pool. It is 64MB by default and used to bounce buffer in the pci DMA API in the low level drivers for any devices that have 32bit limitations (very common)

Swiotlb and the DMA zone already interact. For consistent mappings swiotlb will already allocate from both the DMA zone and from the swiotlb pool as needed. On the other hand swiotlb is a truly separate pool not directly visible to the normal zone balancing (although it happens to be in the DMA32 zone). In practice ZONE_DMA behaves very similar in that respect.

Driver interfaces:

When drivers need DMA able memory they typically use the pci_*/dma_*
interfaces which allow specifying device masks. There are two interfaces
here:

dma_alloc_coherent/pci_alloc_consistent to get a block of coherent memory honouring a device DMA mask and mapped into an IOMMU as needed. And pci_map_*/dma_map_* to remap an arbitary block to the DMA mask of the device and into the IOMMU.

Both ways have their own disadvantages: coherent mappings can have some bad performance penalties and high setup costs on some platforms which are not full IO coherent, so they are not encouraged for high volume driver data. And pci/dma_map_* will always bounce buffer on the common x86 swiotlb case so it might be quite expensive. Also on a lot of IOMMU implementations (in particularly x86 swiotlb/pci-gart) pci/dma_map_* does not support remapping to any DMA masks smaller than 32bit so it cannot actually be used for ISA or any other device with <32bit DMA mask.

Then there is the old style way of directly telling the allocators what memory you need by using GFP_DMA or GFP_DMA32 and then later using bounce less pci/dma_map_*. That also has its own set of problems: first GFP_DMA varies between architectures. On x86 it is always 16MB, on IA64 4GB, on some other architectures it doesn't exist at all or has other sizes. GFP_DMA32 often doesn't exist at all (although it can be often replaced with GFP_KERNEL on 32bit platforms). This means any caller needs to have knowledge about its platform which is often non portable.

Then the other problem is that it these are only single bits into small fixed zones. So for example if a user has a 30bit DMA zone limit on 64bit they have no other choice than to use GFP_DMA and when they need more than 16MB of memory they lose. On the other hand on a lot of other boxes which don't have any devices with <4GB dma masks ZONE_DMA is just wasted memory.

Then GFP_DMA is also not a very clean interface. It is usually not documented what device mask is really needed. Also some driver authors misunderstand it as meaning "required for any DMA" which is not correct. And often it actually requires dma masks larger than 24bit (16MB) so the fixed 24bit on x86 is limiting.

The pci_alloc_consistent implementation on x86 also has more problems:
usually it cannot use an IOMMU to remap the dma memory so they actually have to allocate memory with physical addresses according to the dma mask of the passed device. All they can do for this is to map it to GFP_DMA (16MB small), GFP_DMA32 (big, but sometimes too big) or by getting it from the swiotlb pool. It also attempts to get fitting memory from the main allocator. That works mostly, but has bad corner cases and is quite inelegant.

In practice this leads to various unfortunate situations: either the 64bit system has upto 100MB of reserved memory wasted (ZONE_DMA + swiotlb), but does not have any devices that require bouncing to 16MB or 4GB.

Or the system has devices that need bouncing to <4GB, but the pools in their default size are too small and can overflow. There are various hac^wworkarounds in drivers for this problem, but it still causes problems for users. The ZONE_DMA can also not be enlarged because a lot of drivers "know" that it is only 16MB and expect 16MB memory from it.

On 32bit x86 the problem is a little less severe because of the 900MB ZONE_NORMAL which fits most devices, but there are still some problems with more obscure devices with sufficiently small DMA masks. And ISA devices still fit in badly.

Requirements for a solution:

There is clearly a need for a new better low memory bounce pool on x86. It must be larger than 16MB and actually be variable sized. Also the driver interfaces are inadequate. All DMA memory allocation should specify what mask they actually need. That allows to extend the pool and use a single pool for multiple masks.

The new pool must be isolated from the rest of the VM. Otherwise it cannot be safely used in any device driver paths who cannot necessarily safely allocate memory (e.g. the block write out path is not allowed to do this to avoid deadlocks while swapping) The current effective pools (ZONE_DMA, swiotlb) are already isolated in practice so this won't make much difference.

Proposed solution:

I chose to implement a new "maskable memory" allocator to solve these problems. The existing page buddy allocator is not really suited for this because the data structures don't allow cheap allocation by physical address boundary.

The allocator has a separate pool of memory that it grabs at boot using the bootmem allocator. The memory is part of the lowest zone, but practically invisible to the normal page allocator or VM.

The allocator is very simple: it works with pages and uses a bitmap to find memory. It also uses a simple rotating cursor through the bitmap. It is very similar to the allocators used by the various IOMMU implementations. While this is not a true O(1) allocator, in practice it tends to find free pages very quickly and it is quite flexible. Also it makes it very simple to allocate below arbitary address boundaries. It has one advantage over buddy in that it doesn't require all blocks to be size of power of two. It only rounds to pages. So especially larger blocks tend to have less overhead.

The allocator knows how to fall back to the other zones if the mask is sufficiently big enough, so it can be used for arbitrary masks.

I chose to only implement a page mask allocator only, not "kmalloc_mask", because the various drivers I looked at actually tended to allocate quite large objects towards a page. Also if a sub page allocator is really needed there are several existing ones that could be relatively easily adopted (mm/dmapool.c or the lib/bitmap.c allocator) on top of an page allocator.

The maskable allocator's pool is variable sized and the user can set it to any size needed (upto 2GB currently). The default sizing heuristics are for now the same as in the old code: by default all free memory below 16MB is put into the pool (in practice that is only ~8MB or so usable because the kernel is loaded there too) and swiotlb is needed another 64MB of low memory are reserved too. The user can override this using the command line.
Any other subsystems can also increase the memory reservation (but this currently has to happen early while bootmem is still active)

In the future I hope to make this more flexible. In particular the low memory could not be fully reserved, but only put into the "moveable" zone and then later as devices are discovered and e.g. block devices are set up this pre reservation could be actually reserved. This would then actually allow to use a lot of the ~100MB that currently go to waste on x86-64. But so far that is not implemented yet.

swiotlb doesn't maintain an own pool anymore, but just allocates using the mask allocator. THis is safe because the maskable pool is isolated from the rest of the VM and not prone to OOM deadlocks. This is admittedly more a heuristic, than a strict 100% guarantee, but it is not worse than the old swiotlb. Also all users of the maskable allocators currently are benign and won't overflow it in dynamic situations I believe.

The internal implementations

ZONE_DMA is disabled for x86 with the maskable allocator enabled.

The maskable allocator requires some simple modifications in the architecture start up code. These are currently only done for x86. All other architectures keep the same GFP_DMA semantics as they had before. Adapting other architectures wouldn't be very difficult.

The longer term goal is to convert all GFP_DMA allocations to always specify the correct mask and then eventually remove GFP_DMA.

Especially I hope kmalloc/kmem_cache_alloc GFP_DMA can be removed soon. I have some patches to eliminate those users. Then slab wouldn't need to maintain DMA caches anymore.

This patch kit only contains the core changes for the actual allocator, swiotlb conversion and pci_alloc_consistent/dma_alloc_coherent() Between swiotlb and the later changes this already means that
a lot of drivers use it.

The existing GFP_DMA users transparently fall back to a maskable allocation with 16MB.

I have various other driver conversions in the pipeline, but I will post these sepately to not distract too much from the review of the main code.

-Andi

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in the body to [email protected]. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"[email protected]"> [email protected] </a>

This is needed for some followup patches. Some old drivers (like xd.c) pass 0 to get_order and the compat wrapper for the mask allocator doesn't like the resulting underflow.

---
 include/asm-generic/page.h |    3 +++
 1 file changed, 3 insertions(+)

Index: linux/include/asm-generic/page.h
===================================================================
@@ -11,6 +11,9 @@ static __inline__ __attribute_const__ in
 {
 	int order;
 
+	if (size == 0)
+		return 0;
+
 	size = (size - 1) >> (PAGE_SHIFT - 1);
 	order = -1;
 	do {

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in the body to [email protected]. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"[email protected]"> [email protected] </a>

Avoids global namespace clash with later patch that exports page_alloc's bad_page. bad_page is not used outside kvm_main.c, so making it static is fine.

---
 include/linux/kvm_host.h |    2 --
 virt/kvm/kvm_main.c      |    4 ++--
 2 files changed, 2 insertions(+), 4 deletions(-)

Index: linux/include/linux/kvm_host.h
===================================================================
@@ -150,8 +150,6 @@ void kvm_exit(void);
 static inline int is_error_hpa(hpa_t hpa) { return hpa >> HPA_MSB; }
 struct page *gva_to_page(struct kvm_vcpu *vcpu, gva_t gva);
 
-extern struct page *bad_page;
-
 int is_error_page(struct page *page);
 int kvm_is_error_hva(unsigned long addr);
 int kvm_set_memory_region(struct kvm *kvm,

Index: linux/virt/kvm/kvm_main.c
===================================================================
@@ -52,6 +52,8 @@ MODULE_LICENSE("GPL");
 DEFINE_SPINLOCK(kvm_lock);
 LIST_HEAD(vm_list);
 
+static struct page *bad_page;
+
 static cpumask_t cpus_hardware_enabled;
 
 struct kmem_cache *kvm_vcpu_cache;

@@ -1271,8 +1273,6 @@ static struct sys_device kvm_sysdev = {
 	.cls = &kvm_sysdev_class,
 };
 
-struct page *bad_page;
-
 static inline
 struct kvm_vcpu *preempt_notifier_to_vcpu(struct preempt_notifier *pn)
 {

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in the body to [email protected]. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"[email protected]"> [email protected] </a>

The maskable allocator needs some functions from page_alloc.c exported; in particular free_pages_check and prep_new_page. Do that using mm/internal.h

Also extend free_pages_check to support custom flags and allow prep_new_page to ignore the Reserved bit.

No behaviour change itself; just some code movement.

---
 mm/internal.h   |   67 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++
 mm/page_alloc.c |   63 +++-------------------------------------------------
 2 files changed, 71 insertions(+), 59 deletions(-)

Index: linux/mm/page_alloc.c
===================================================================
@@ -44,7 +44,6 @@
 #include <linux/backing-dev.h>
 #include <linux/fault-inject.h>
 #include <linux/page-isolation.h>
-#include <linux/memcontrol.h>
 
 #include <asm/tlbflush.h>
 #include <asm/div64.h>

@@ -220,7 +219,7 @@ static inline int bad_range(struct zone 
 }
 #endif
 
-static void bad_page(struct page *page)
+void bad_page(struct page *page)
 {
 	void *pc = page_get_page_cgroup(page);
 

@@ -456,33 +455,6 @@ static inline void __free_one_page(struc
 	zone->free_area[order].nr_free++;
 }
 
-static inline int free_pages_check(struct page *page)
-{
-	if (unlikely(page_mapcount(page) |
-		(page->mapping != NULL)  |
-		(page_get_page_cgroup(page) != NULL) |
-		(page_count(page) != 0)  |
-		(page->flags & (
-			1 << PG_lru	|
-			1 << PG_private |
-			1 << PG_locked	|
-			1 << PG_active	|
-			1 << PG_slab	|
-			1 << PG_swapcache |
-			1 << PG_writeback |
-			1 << PG_reserved |
-			1 << PG_buddy ))))
-		bad_page(page);
-	if (PageDirty(page))
-		__ClearPageDirty(page);
-	/*
-	 * For now, we report if PG_reserved was found set, but do not
-	 * clear it, and do not free the page.  But we shall soon need
-	 * to do more, for when the ZERO_PAGE count wraps negative.
-	 */
-	return PageReserved(page);
-}
-
 /*
  * Frees a list of pages. 
  * Assumes all pages on list are in same zone, and of same order.

@@ -528,7 +500,7 @@ static void __free_pages_ok(struct page 
 	int reserved = 0;
 
 	for (i = 0 ; i < (1 << order) ; ++i)
-		reserved += free_pages_check(page + i);
+		reserved += free_pages_check(page + i, 0);
 	if (reserved)
 		return;
 

@@ -608,36 +580,9 @@ static inline void expand(struct zone *z
  */
 static int prep_new_page(struct page *page, int order, gfp_t gfp_flags)
 {
-	if (unlikely(page_mapcount(page) |
-		(page->mapping != NULL)  |
-		(page_get_page_cgroup(page) != NULL) |
-		(page_count(page) != 0)  |
-		(page->flags & (
-			1 << PG_lru	|
-			1 << PG_private	|
-			1 << PG_locked	|
-			1 << PG_active	|
-			1 << PG_dirty	|
-			1 << PG_slab    |
-			1 << PG_swapcache |
-			1 << PG_writeback |
-			1 << PG_reserved |
-			1 << PG_buddy ))))
-		bad_page(page);
-
-	/*
-	 * For now, we report if PG_reserved was found set, but do not
-	 * clear it, and do not allocate the page: as a safety net.
-	 */
-	if (PageReserved(page))
+	if (page_prep_struct(page))
 		return 1;
 
-	page->flags &= ~(1 << PG_uptodate | 1 << PG_error | 1 << PG_readahead |
-			1 << PG_referenced | 1 << PG_arch_1 |
-			1 << PG_owner_priv_1 | 1 << PG_mappedtodisk);
-	set_page_private(page, 0);
-	set_page_refcounted(page);
-
 	arch_alloc_page(page, order);
 	kernel_map_pages(page, 1 << order, 1);
 

@@ -992,7 +937,7 @@ static void free_hot_cold_page(struct pa
 
 	if (PageAnon(page))
 		page->mapping = NULL;
-	if (free_pages_check(page))
+	if (free_pages_check(page, 0))
 		return;
 
 	if (!PageHighMem(page))

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in the body to [email protected]. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"[email protected]"> [email protected] </a>

Hi Andi.

[Sam Ravnborg - Fri, Mar 07, 2008 at 07:19:43PM +0100]

| Hi Andi.
| 
| > Index: linux/mm/internal.h
| > ===================================================================
| > --- linux.orig/mm/internal.h
| > +++ linux/mm/internal.h
| > @@ -12,6 +12,7 @@
| >  #define __MM_INTERNAL_H
| >  
| >  #include <linux/mm.h>
| > +#include <linux/memcontrol.h>
| >  
| >  static inline void set_page_count(struct page *page, int v)
| >  {
| > @@ -48,6 +49,72 @@ static inline unsigned long page_order(s
| >  	return page_private(page);
| >  }
| >  
| > +extern void bad_page(struct page *page);
| > +
| > +static inline int free_pages_check(struct page *page, unsigned long addflags)
| > +{
| > +	if (unlikely(page_mapcount(page) |
| > +		(page->mapping != NULL)  |
| > +		(page_get_page_cgroup(page) != NULL) |
| > +		(page_count(page) != 0)  |
| > +		(page->flags & (
| > +			addflags |
| > +			1 << PG_lru	|
| > +			1 << PG_private |
| > +			1 << PG_locked	|
| > +			1 << PG_active	|
| > +			1 << PG_slab	|
| > +			1 << PG_swapcache |
| > +			1 << PG_writeback |
| > +			1 << PG_reserved |
| > +			1 << PG_buddy))))
| > +		bad_page(page);
| > +	if (PageDirty(page))
| > +		__ClearPageDirty(page);
| > +	/*
| > +	 * For now, we report if PG_reserved was found set, but do not
| > +	 * clear it, and do not free the page.  But we shall soon need
| > +	 * to do more, for when the ZERO_PAGE count wraps negative.
| > +	 */
| > +	return PageReserved(page);
| > +}
| Looks a bit too big for an inline in a header (~9 lines of code)?
| 

well, it will not be that big in compiled form 'cause in real it is

	page->flags & (addflags | (precompiled constant))

| > +
| > +/* Set up a struc page for business during allocation */
| > +static inline int page_prep_struct(struct page *page)
| > +{
| > +	if (unlikely(page_mapcount(page) |
| > +		(page->mapping != NULL)  |
| > +		(page_get_page_cgroup(page) != NULL) |
| > +		(page_count(page) != 0)  |
| > +		(page->flags & (
| > +			1 << PG_lru	|
| > +			1 << PG_private	|
| > +			1 << PG_locked	|
| > +			1 << PG_active	|
| > +			1 << PG_dirty	|
| > +			1 << PG_slab    |
| > +			1 << PG_swapcache |
| > +			1 << PG_writeback |
| > +			1 << PG_reserved |
| > +			1 << PG_buddy))))
| > +		bad_page(page);
| > +
| > +	/*
| > +	 * For now, we report if PG_reserved was found set, but do not
| > +	 * clear it, and do not allocate the page: as a safety net.
| > +	 */
| > +	if (PageReserved(page))
| > +		return 1;
| > +
| > +	page->flags &= ~(1 << PG_uptodate | 1 << PG_error | 1 << PG_readahead |
| > +			1 << PG_referenced | 1 << PG_arch_1 |
| > +			1 << PG_owner_priv_1 | 1 << PG_mappedtodisk);
| > +	set_page_private(page, 0);
| > +	set_page_refcounted(page);
| > +
| > +	return 0;
| > +}
| Again - looks too big to inline..
| 
| But for both I do not know where they are used and how often.
| 
| 	Sam
| --
| 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/
| 

- Cyrill -

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in the body to [email protected]. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"[email protected]"> [email protected] </a>

For the mask allocator it is not that critical, but for the main allocator it is on the very very hot page allocation/free path. That is why I didn't want to out-of-line it.

Note I didn't change it, just moved it.

-Andi

They are a bit on the extensive side now, but I figured out more data is better for now.

The actual counts are added in the new files.

---
 include/linux/vmstat.h |    4 ++++
 mm/vmstat.c            |   10 ++++++++++
 2 files changed, 14 insertions(+)

Index: linux/include/linux/vmstat.h
===================================================================
@@ -37,6 +37,10 @@ enum vm_event_item { PGPGIN, PGPGOUT, PS
 		FOR_ALL_ZONES(PGSCAN_DIRECT),
 		PGINODESTEAL, SLABS_SCANNED, KSWAPD_STEAL, KSWAPD_INODESTEAL,
 		PAGEOUTRUN, ALLOCSTALL, PGROTATED,
+#ifdef CONFIG_MASK_ALLOC
+		MASK_ALLOC, MASK_FREE, MASK_BITMAP_SKIP, MASK_WAIT,
+		MASK_HIGHER, MASK_LOW_WASTE, MASK_HIGH_WASTE,
+#endif
 		NR_VM_EVENT_ITEMS
 };
 

Index: linux/mm/vmstat.c
===================================================================
@@ -644,6 +644,16 @@ static const char * const vmstat_text[] 
 	"allocstall",
 
 	"pgrotated",
+
+#ifdef CONFIG_MASK_ALLOC
+	"mask_alloc",
+	"mask_free",
+	"mask_bitmap_skip",
+	"mask_wait",
+	"mask_higher",
+	"mask_low_waste",
+	"mask_high_waste",
+#endif
 #endif
 };
 

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in the body to [email protected]. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"[email protected]"> [email protected] </a>

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in the body to [email protected]. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"[email protected]"> [email protected] </a>

This is the core code of the maskable allocator. Introduction appended.

Background:

The 16MB Linux ZONE_DMA has some long standing problems on x86. Traditionally it was designed only for ISA dma which is limited to 24bit (16MB). This means it has a fixed 16MB size.

On 32bit i386 with its limited virtual memory space the next zone is lowmem with ~900MB (on default split) which works for a lot of devices, but not all. But on x86-64 the next zone is only 4GB (DMA32) which is too big for quite a lot more devices (typically 30,31 or 28bit limitations).

While the DMA zone is in a true VM zone and could be in theory used for any user allocations in practice the VM has a concept called lower zone protection to avoid low memory deadlocks that keeps ZONE_DMA nearly always free unless the caller specifies GFP_DMA directly. This means in practice it does not participate in the rest of the automatic VM balancing and its memory is essentially reserved.

Then there is another pool used on x86-64: the swiotlb pool. It is 64MB by default and used to bounce buffer in the pci DMA API in the low level drivers for any devices that have 32bit limitations (very common)

Swiotlb and the DMA zone already interact. For consistent mappings swiotlb will already allocate from both the DMA zone and from the swiotlb pool as needed. On the other hand swiotlb is a truly separate pool not directly visible to the normal zone balancing (although it happens to be in the DMA32 zone). In practice ZONE_DMA behaves very similar in that respect.

Driver interfaces:

When drivers need DMA able memory they typically use the pci_*/dma_*
interfaces which allow specifying device masks. There are two interfaces
here:

dma_alloc_coherent/pci_alloc_consistent to get a block of coherent memory honouring a device DMA mask and mapped into an IOMMU as needed. And pci_map_*/dma_map_* to remap an arbitary block to the DMA mask of the device and into the IOMMU.

Both ways have their own disadvantages: coherent mappings can have some bad performance penalties and high setup costs on some platforms which are not full IO coherent, so they are not encouraged for high volume driver data. And pci/dma_map_* will always bounce buffer on the common x86 swiotlb case so it might be quite expensive. Also on a lot of IOMMU implementations (in particularly x86 swiotlb/pci-gart) pci/dma_map_* does not support remapping to any DMA masks smaller than 32bit so it cannot actually be used for ISA or any other device with <32bit DMA mask.

Then there is the old style way of directly telling the allocators what memory you need by using GFP_DMA or GFP_DMA32 and then later using bounce less pci/dma_map_*. That also has its own set of problems: first GFP_DMA varies between architectures. On x86 it is always 16MB, on IA64 4GB, on some other architectures it doesn't exist at all or has other sizes. GFP_DMA32 often doesn't exist at all (although it can be often replaced with GFP_KERNEL on 32bit platforms). This means any caller needs to have knowledge about its platform which is often non portable.

Then the other problem is that it these are only single bits into small fixed zones. So for example if a user has a 30bit DMA zone limit on 64bit they have no other choice than to use GFP_DMA and when they need more than 16MB of memory they lose. On the other hand on a lot of other boxes which don't have any devices with <4GB dma masks ZONE_DMA is just wasted memory.

Then GFP_DMA is also not a very clean interface. It is usually not documented what device mask is really needed. Also some driver authors misunderstand it as meaning "required for any DMA" which is not correct. And often it actually requires dma masks larger than 24bit (16MB) so the fixed 24bit on x86 is limiting.

The pci_alloc_consistent implementation on x86 also has more problems:
usually it cannot use an IOMMU to remap the dma memory so they actually have to allocate memory with physical addresses according to the dma mask of the passed device. All they can do for this is to map it to GFP_DMA (16MB small), GFP_DMA32 (big, but sometimes too big) or by getting it from the swiotlb pool. It also attempts to get fitting memory from the main allocator. That works mostly, but has bad corner cases and is quite inelegant.

In practice this leads to various unfortunate situations: either the 64bit system has upto 100MB of reserved memory wasted (ZONE_DMA + swiotlb), but does not have any devices that require bouncing to 16MB or 4GB.

Or the system has devices that need bouncing to <4GB, but the pools in their default size are too small and can overflow. There are various hac^wworkarounds in drivers for this problem, but it still causes problems for users. The ZONE_DMA can also not be enlarged because a lot of drivers "know" that it is only 16MB and expect 16MB memory from it.

On 32bit x86 the problem is a little less severe because of the 900MB ZONE_NORMAL which fits most devices, but there are still some problems with more obscure devices with sufficiently small DMA masks. And ISA devices still fit in badly.

Requirements for a solution:

There is clearly a need for a new better low memory bounce pool on x86. It must be larger than 16MB and actually be variable sized. Also the driver interfaces are inadequate. All DMA memory allocation should specify what mask they actually need. That allows to extend the pool and use a single pool for multiple masks.

The new pool must be isolated from the rest of the VM. Otherwise it cannot be safely used in any device driver paths who cannot necessarily safely allocate memory (e.g. the block write out path is not allowed to do this to avoid deadlocks while swapping) The current effective pools (ZONE_DMA, swiotlb) are already isolated in practice so this won't make much difference.

Proposed solution:

I chose to implement a new "maskable memory" allocator to solve these problems. The existing page buddy allocator is not really suited for this because the data structures don't allow cheap allocation by physical address boundary.

The allocator has a separate pool of memory that it grabs at boot using the bootmem allocator. The memory is part of the lowest zone, but practically invisible to the normal page allocator or VM.

The allocator is very simple: it works with pages and uses a bitmap to find memory. It also uses a simple rotating cursor through the bitmap. It is very similar to the allocators used by the various IOMMU implementations. While this is not a true O(1) allocator, in practice it tends to find free pages very quickly and it is quite flexible. Also it makes it very simple to allocate below arbitary address boundaries. It has one advantage over buddy in that it doesn't require all blocks to be size of power of two. It only rounds to pages. So especially larger blocks tend to have less overhead.

The allocator knows how to fall back to the other zones if the mask is sufficiently big enough, so it can be used for arbitrary masks.

I chose to only implement a page mask allocator only, not "kmalloc_mask", because the various drivers I looked at actually tended to allocate quite large objects towards a page. Also if a sub page allocator is really needed there are several existing ones that could be relatively easily adopted (mm/dmapool.c or the lib/bitmap.c allocator) on top of an page allocator.

The maskable allocator's pool is variable sized and the user can set it to any size needed (upto 2GB currently). The default sizing heuristics are for now the same as in the old code: by default all free memory below 16MB is put into the pool (in practice that is only ~8MB or so usable because the kernel is loaded there too) and swiotlb is needed another 64MB of low memory are reserved too. The user can override this using the command line.
Any other subsystems can also increase the memory reservation (but this currently has to happen early while bootmem is still active)

In the future I hope to make this more flexible. In particular the low memory could not be fully reserved, but only put into the "moveable" zone and then later as devices are discovered and e.g. block devices are set up this pre reservation could be actually reserved. This would then actually allow to use a lot of the ~100MB that currently go to waste on x86-64. But so far that is not implemented yet.

Quirks:

The maskable allocator is able to sleep (for freeing
of other maskable allocations). It currently does this
using a simple timeout before it fails. This needs more evaluation on how well it really works under low memory conditions.

I added quite a lot of statistics counters for now to better evaluate it. Some of them might be later removed.

There is currently no higher priority pool for GFP_ATOMIC. In general memory pressure on the mask allocator should be less because normal user space doesn't directly allocate from it. That is why I didn't bother implementing it right now. If its absence is a problem it could be added later.

---
 Documentation/DocBook/kernel-api.tmpl |    1 
 Documentation/kernel-parameters.txt   |    3 
 include/linux/gfp.h                   |   52 +++
 include/linux/page-flags.h            |    5 
 mm/Makefile                           |    1 
 mm/mask-alloc.c                       |  504 ++++++++++++++++++++++++++++++++++
 mm/page_alloc.c                       |    4 
 7 files changed, 565 insertions(+), 5 deletions(-)

Index: linux/mm/Makefile
===================================================================
@@ -33,4 +33,5 @@ obj-$(CONFIG_MIGRATION) += migrate.o
 obj-$(CONFIG_SMP) += allocpercpu.o
 obj-$(CONFIG_QUICKLIST) += quicklist.o
 obj-$(CONFIG_CGROUP_MEM_RES_CTLR) += memcontrol.o
+obj-$(CONFIG_MASK_ALLOC) += mask-alloc.o
 

Index: linux/include/linux/page-flags.h
===================================================================
@@ -89,6 +89,7 @@
 #define PG_mappedtodisk		16	/* Has blocks allocated on-disk */
 #define PG_reclaim		17	/* To be reclaimed asap */
 #define PG_buddy		19	/* Page is free, on buddy lists */
+#define PG_mask_alloc		20	/* Page managed by the Mask allocator */
 
 /* PG_readahead is only used for file reads; PG_reclaim is only for writes */
 #define PG_readahead		PG_reclaim /* Reminder to do async read-ahead */

@@ -256,6 +257,10 @@ static inline void SetPageUptodate(struc
 #define __SetPageCompound(page)	__set_bit(PG_compound, &(page)->flags)
 #define __ClearPageCompound(page) __clear_bit(PG_compound, &(page)->flags)
 
+#define PageMaskAlloc(page)	test_bit(PG_mask_alloc, &(page)->flags)
+#define __SetPageMaskAlloc(page)	__set_bit(PG_mask_alloc, &(page)->flags)
+#define __ClearPageMaskAlloc(page) __clear_bit(PG_mask_alloc, &(page)->flags)
+
 /*
  * PG_reclaim is used in combination with PG_compound to mark the
  * head and tail of a compound page

Index: linux/Documentation/kernel-parameters.txt
===================================================================
@@ -2116,6 +2116,9 @@ and is between 256 and 4096 characters. 
 	norandmaps	Don't use address space randomization
 			Equivalent to echo 0 > /proc/sys/kernel/randomize_va_space
 
+	maskzone=size[MG] Set size of maskable DMA zone to size.
+		 force	Always allocate from the mask zone (for testing)
+
 ______________________________________________________________________
 
 TODO:

Index: linux/mm/page_alloc.c
===================================================================
@@ -500,7 +500,7 @@ static void __free_pages_ok(struct page 
 	int reserved = 0;
 
 	for (i = 0 ; i < (1 << order) ; ++i)
-		reserved += free_pages_check(page + i, 0);
+		reserved += free_pages_check(page + i, 1 << PG_mask_alloc);
 	if (reserved)
 		return;
 

@@ -937,7 +937,7 @@ static void free_hot_cold_page(struct pa
 
 	if (PageAnon(page))
 		page->mapping = NULL;
-	if (free_pages_check(page, 0))
+	if (free_pages_check(page, 1 << PG_mask_alloc))
 		return;
 
 	if (!PageHighMem(page))

Index: linux/Documentation/DocBook/kernel-api.tmpl
===================================================================
@@ -164,6 +164,7 @@ X!Ilib/string.c
 !Emm/memory.c
 !Emm/vmalloc.c
 !Imm/page_alloc.c
+!Emm/mask-alloc.c
 !Emm/mempool.c
 !Emm/dmapool.c
 !Emm/page-writeback.c

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in the body to [email protected]. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"[email protected]"> [email protected] </a>

Hi Andi,

You're right -- this needs a check. Weird wonder why the tests didn't catch that.

Thanks,
-Andi

What is your question?

-Andi

That line seems odd. Is it correct?
Why 2 spaces between force and Always? Why is Always capitalized? Could one of those words be dropped? They seem a bit redundant.

~Randy

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to [email protected].  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"[email protected]"> [email protected] </a>

The option is either maskzone=size[MG] or maskzone=force Each followed with a sentence describing them.

I tried to make this clear by lining them up, but apparently failed. You have a preferred way to formatting such multiple choice options?

-Andi

Oh. I see.

I don't know that we have a certain/fixed way for that.

How about:

maskzone=
Format: {size[MG] | force}
size -- Set size of maskable DMA zone to size force -- Always allocate from the mask zone (for testing)

---
~Randy

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in the body to [email protected]. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"[email protected]"> [email protected] </a>

[Andi Kleen - Fri, Mar 07, 2008 at 10:07:16AM +0100] [...]

| +
| +/**
| + * alloc_pages_mask - Alloc page(s) in a specific address range.
| + * @gfp:      Standard GFP mask. See get_free_pages for a list valid flags.
| + * @size:     Allocate size worth of pages. Rounded up to PAGE_SIZE.
| + * @mask:     Memory must fit into mask physical address.
| + *
| + * Returns a struct page *
| + *
| + * Manage dedicated maskable low memory zone. This zone are isolated
| + * from the normal zones. This is only a single continuous zone.
| + * The main difference to the standard allocator is that it tries
| + * to allocate memory with an physical address fitting in the passed mask.
| + *
| + * Warning: the size is in bytes, not in order like get_free_pages.
| + */
| +struct page *
| +alloc_pages_mask(gfp_t gfp, unsigned size, u64 mask)
| +{
| +	unsigned long max_pfn = mask >> PAGE_SHIFT;
| +	unsigned pages = (size + PAGE_SIZE - 1) >> PAGE_SHIFT;
| +	struct page *p;
| +	unsigned left = (gfp & __GFP_REPEAT) ? ~0 : mask_timeout, oleft;
| +	unsigned order = get_order(size);
| +
| +	BUG_ON(size < MASK_MIN_SIZE);	/* You likely passed order by mistake */
| +	BUG_ON(gfp & (__GFP_DMA|__GFP_DMA32|__GFP_COMP));
| +
| +	/* Add fault injection here */
| +
| +again:
| +	count_vm_event(MASK_ALLOC);
| +	if (!force_mask) {
| +		/* First try normal allocation in suitable zones
| +		 * RED-PEN if size fits very badly in PS<<order don't do this?
| +		 */
| +		p = alloc_higher_pages(gfp, order, max_pfn);
| +
| +		/*
| +		 * If the mask covers everything don't bother with the low zone
| +		 * This way we avoid running out of low memory on a higher zones
| +		 * OOM too.
| +		 */
| +		if (p != NULL || max_pfn >= max_low_pfn) {

Andi, I'm a little confused by _this_ statistics. We could get p = NULL there and change MASK_HIGH_WASTE even have mask not allocated. Am I wrong or miss something? Or maybe there should be '&&' instead of '||'?

| +			count_vm_event(MASK_HIGHER);
| +			count_vm_events(MASK_HIGH_WASTE,
| +					(PAGE_SIZE << order) - size);
| +			return p;
| +		}
| +	}
| +
| +	might_sleep_if(gfp & __GFP_WAIT);
| +	do {
| +		int i;
| +		long pfn;
| +
| +		/* Implement waiter fairness queueing here? */
| +
| +		pfn = alloc_mask(pages, max_pfn);
| +		if (pfn != -1L) {
| +			p = pfn_to_page(pfn);
| +
| +			Mprintk("mask page %lx size %d mask %Lx\n",
| +			       po, size, mask);
| +
| +			BUG_ON(pfn + pages > mask_max_pfn);
| +
| +			if (page_prep_struct(p))
| +				goto again;
| +
| +			kernel_map_pages(p, pages, 1);
| +
| +			for (i = 0; i < pages; i++) {
| +				struct page *n = p + i;
| +				BUG_ON(!test_bit(pfn_to_maskbm_index(pfn+i),
| +						mask_bitmap));
| +				BUG_ON(!PageMaskAlloc(n));
| +				arch_alloc_page(n, 0);
| +				if (gfp & __GFP_ZERO)
| +					clear_page(page_address(n));
| +			}
| +
| +			count_vm_events(MASK_LOW_WASTE, pages*PAGE_SIZE-size);
| +			return p;
| +		}
| +
| +		if (!(gfp & __GFP_WAIT))
| +			break;
| +
| +		oleft = left;
| +		left = wait_for_mask_free(left);
| +		count_vm_events(MASK_WAIT, left - oleft);
| +	} while (left > 0);
| +
| +	if (!(gfp & __GFP_NOWARN)) {
| +		printk(KERN_ERR
| +		"%s: Cannot allocate maskable memory size %u gfp %x mask %Lx\n",
| +				current->comm, size, gfp, mask);
| +		dump_stack();
| +	}
| +	return NULL;
| +}
| +EXPORT_SYMBOL(alloc_pages_mask);
| +

[...]
- Cyrill -

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in the body to [email protected]. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"[email protected]"> [email protected] </a>

You're right the statistics counter is increased incorrectly for the p == NULL case. I'll fix that thanks. || is correct, see the comment above.

-Andi

Sorry, please ignore above comment :(

Thanks,
-Kame

It's not. Since the buddy allocator is not used MAX_ORDER doesn't really exist for the mask allocator. It can handle arbitary size allocations.

Also strictly seen the maskzone is part of the lowmem zone.

-Andi

Hi, Andi,

As I dig through this patch, I find it mostly makes sense; seems like it could be a good idea. I did have one little API question...

Thanks.

Actually it's both.

mask is the standard term used by the PCI-DMA API for the same thing and since one of the main purposes of the mask allocator is to implement underlying support for that interface it seemed fitting to use the same convention.

-Andi

Add code to the normal allocation path to call the mask allocator transparently for GFP_DMA allocations.

This is only a temporary measure and will go away as soon as all the drivers are converted to use the mask allocator or one of its callers (like PCI DMA API) directly.

But currently it is the only sane way to do a bisectable step by step conversion of the GFP_DMA users in tree.

Right now the special pagevec free function won't handle mask allocated pages.

---
 include/linux/page-flags.h |    4 ++++
 mm/page_alloc.c            |   18 ++++++++++++++++++
 2 files changed, 22 insertions(+)

Index: linux/mm/page_alloc.c
===================================================================
@@ -499,6 +499,12 @@ static void __free_pages_ok(struct page 
 	int i;
 	int reserved = 0;
 
+	/* Compat code for now. Will go away */
+	if (unlikely(PageMaskAlloc(page))) {
+		__free_pages_mask(page, PAGE_SIZE << order);
+		return;
+	}
+
 	for (i = 0 ; i < (1 << order) ; ++i)
 		reserved += free_pages_check(page + i, 1 << PG_mask_alloc);
 	if (reserved)

@@ -1422,6 +1428,13 @@ __alloc_pages(gfp_t gfp_mask, unsigned i
 	if (should_fail_alloc_page(gfp_mask, order))
 		return NULL;
 
+#ifdef CONFIG_MASK_ALLOC
+	/* Compat code for now. Will go away. */
+	if (unlikely(gfp_mask & GFP_DMA))
+		return alloc_pages_mask(gfp_mask & ~__GFP_DMA,
+					PAGE_SIZE << order, TRAD_DMA_MASK);
+#endif
+
 restart:
 	z = zonelist->zones;  /* the list of zones suitable for gfp_mask */
 

@@ -1635,6 +1648,11 @@ void __pagevec_free(struct pagevec *pvec
 
 void __free_pages(struct page *page, unsigned int order)
 {
+	/* Compat code for now. Will go away */
+	if (unlikely(PageMaskAlloc(page))) {
+		__free_pages_mask(page, PAGE_SIZE << order);
+		return;
+	}
 	if (put_page_testzero(page)) {
 		if (order == 0)
 			free_hot_page(page);

Index: linux/include/linux/page-flags.h
===================================================================
@@ -257,9 +257,13 @@ static inline void SetPageUptodate(struc
 #define __SetPageCompound(page)	__set_bit(PG_compound, &(page)->flags)
 #define __ClearPageCompound(page) __clear_bit(PG_compound, &(page)->flags)
 
+#ifdef CONFIG_MASK_ALLOC
 #define PageMaskAlloc(page)	test_bit(PG_mask_alloc, &(page)->flags)
 #define __SetPageMaskAlloc(page)	__set_bit(PG_mask_alloc, &(page)->flags)
 #define __ClearPageMaskAlloc(page) __clear_bit(PG_mask_alloc, &(page)->flags)
+#else
+#define PageMaskAlloc(page)	0
+#endif
 
 /*
  * PG_reclaim is used in combination with PG_compound to mark the

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in the body to [email protected]. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"[email protected]"> [email protected] </a>

- Disable old ZONE_DMA
No fixed size ZONE_DMA now anymore. All existing users of __GFP_DMA rely on the compat call to the maskable allocator in alloc/free_pages
- Call maskable allocator initialization functions at boot
- Add TRAD_DMA_MASK for the compat functions
- Remove dma_reserve call

---
 arch/x86/Kconfig           |    4 ++--
 arch/x86/kernel/setup_32.c |    3 +--
 arch/x86/kernel/setup_64.c |    1 +
 arch/x86/mm/discontig_32.c |    2 --
 arch/x86/mm/init_32.c      |    2 ++
 arch/x86/mm/init_64.c      |    9 ++-------
 arch/x86/mm/numa_64.c      |    1 -
 include/asm-x86/dma.h      |    2 ++
 8 files changed, 10 insertions(+), 14 deletions(-)

Index: linux/arch/x86/kernel/setup_64.c
===================================================================
@@ -425,6 +425,7 @@ void __init setup_arch(char **cmdline_p)
 	}
 #endif
 	reserve_crashkernel();
+	init_mask_zone(__pa(MAX_DMA_ADDRESS));
 	paging_init();
 	map_vsyscall();
 

Index: linux/arch/x86/kernel/setup_32.c
===================================================================
@@ -438,8 +438,6 @@ void __init zone_sizes_init(void)
 {
 	unsigned long max_zone_pfns[MAX_NR_ZONES];
 	memset(max_zone_pfns, 0, sizeof(max_zone_pfns));
-	max_zone_pfns[ZONE_DMA] =
-		virt_to_phys((char *)MAX_DMA_ADDRESS) >> PAGE_SHIFT;
 	max_zone_pfns[ZONE_NORMAL] = max_low_pfn;
 #ifdef CONFIG_HIGHMEM
 	max_zone_pfns[ZONE_HIGHMEM] = highend_pfn;

@@ -811,6 +809,7 @@ void __init setup_arch(char **cmdline_p)
 	 * NOTE: at this point the bootmem allocator is fully available.
 	 */
 
+	init_mask_zone(__pa(MAX_DMA_ADDRESS));
 #ifdef CONFIG_BLK_DEV_INITRD
 	relocate_initrd();
 #endif

Index: linux/arch/x86/mm/init_64.c
===================================================================
@@ -50,8 +50,6 @@
 const struct dma_mapping_ops *dma_ops;
 EXPORT_SYMBOL(dma_ops);
 
-static unsigned long dma_reserve __initdata;
-
 DEFINE_PER_CPU(struct mmu_gather, mmu_gathers);
 
 /*

@@ -451,7 +449,6 @@ void __init paging_init(void)
 	unsigned long max_zone_pfns[MAX_NR_ZONES];
 
 	memset(max_zone_pfns, 0, sizeof(max_zone_pfns));
-	max_zone_pfns[ZONE_DMA] = MAX_DMA_PFN;
 	max_zone_pfns[ZONE_DMA32] = MAX_DMA32_PFN;
 	max_zone_pfns[ZONE_NORMAL] = end_pfn;
 

@@ -514,6 +511,8 @@ void __init mem_init(void)
 
 	pci_iommu_alloc();
 
+	prepare_mask_zone();
+
 	/* clear_bss() already clear the empty_zero_page */
 
 	reservedpages = 0;

@@ -671,10 +670,6 @@ void __init reserve_bootmem_generic(unsi
 #else
 	reserve_bootmem(phys, len, BOOTMEM_DEFAULT);
 #endif
-	if (phys+len <= MAX_DMA_PFN*PAGE_SIZE) {
-		dma_reserve += len / PAGE_SIZE;
-		set_dma_reserve(dma_reserve);
-	}
 }
 
 int kern_addr_valid(unsigned long addr)

Index: linux/arch/x86/mm/numa_64.c
===================================================================
@@ -575,7 +575,6 @@ void __init paging_init(void)
 	unsigned long max_zone_pfns[MAX_NR_ZONES];
 
 	memset(max_zone_pfns, 0, sizeof(max_zone_pfns));
-	max_zone_pfns[ZONE_DMA] = MAX_DMA_PFN;
 	max_zone_pfns[ZONE_DMA32] = MAX_DMA32_PFN;
 	max_zone_pfns[ZONE_NORMAL] = end_pfn;
 

Index: linux/arch/x86/Kconfig
===================================================================
@@ -63,8 +63,8 @@ config FAST_CMPXCHG_LOCAL
 config MMU
 	def_bool y
 
-config ZONE_DMA
-	def_bool y
+config MASK_ALLOC
+       def_bool y
 
 config QUICKLIST
 	def_bool X86_32

Index: linux/arch/x86/mm/discontig_32.c
===================================================================
@@ -400,8 +400,6 @@ void __init zone_sizes_init(void)
 	int nid;
 	unsigned long max_zone_pfns[MAX_NR_ZONES];
 	memset(max_zone_pfns, 0, sizeof(max_zone_pfns));
-	max_zone_pfns[ZONE_DMA] =
-		virt_to_phys((char *)MAX_DMA_ADDRESS) >> PAGE_SHIFT;
 	max_zone_pfns[ZONE_NORMAL] = max_low_pfn;
 #ifdef CONFIG_HIGHMEM
 	max_zone_pfns[ZONE_HIGHMEM] = highend_pfn;

Index: linux/include/asm-x86/dma.h
===================================================================
@@ -316,4 +316,6 @@ extern int isa_dma_bridge_buggy;
 #define isa_dma_bridge_buggy	(0)
 #endif
 
+#define TRAD_DMA_MASK 0xffffff /* 16MB, 24bit */
+
 #endif /* _ASM_X86_DMA_H */

Index: linux/arch/x86/mm/init_32.c
===================================================================
@@ -571,6 +571,8 @@ void __init mem_init(void)
 	int codesize, reservedpages, datasize, initsize;
 	int tmp, bad_ppro;
 
+	prepare_mask_zone();
+
 #ifdef CONFIG_FLATMEM
 	BUG_ON(!mem_map);
 #endif

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in the body to [email protected]. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"[email protected]"> [email protected] </a>

I see TRAD_DMA_MASK used by patch 6 and patch 7, but only here in this later patch it is defined.
Looks like we have build breakage - but I have not checked it.

Sam

It is only used when the architecture defines CONFIG_MASK_ALLOC and that is not done by any until the later patches.

-Andi

Understood - I just took a quick look and wanted to let you know.

Sam

This looks okay for the disabling part. But note that there are various uses of MAX_DMA_ADDRESS (sparsemem, bootmem allocator) that are currently suboptimal because they set a boundary at 16MB for allocation of potentially large operating system structures. That boundary continues to exist despite the removal of ZONE_DMA?

It would be better to remove ZONE_DMA32 instead and enlarge ZONE_DMA so that it can take over the role of ZONE_DMA. Set the boundary for MAX_DMA_ADDRESS to the boundary for ZONE_DMA32. Then the allocations for sparse and bootmem will be allocated above 4GB which leaves lots of the lower space available for 32 bit DMA capable devices.

Removal of ZONE_DMA32 would allow us to remove support for that zone from the kernel in general since x86_64 is the only user of that zone.

Trouble is likely that there are existing users that have the expectation of a 4GB boundary on ZONE_DMA32 and 16MB on ZONE_DMA (particularly old drivers). Could we not clean that up?

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in the body to [email protected]. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"[email protected]"> [email protected] </a>

good. i like the idea...

How about system with only 4G or less?

YH

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in the body to [email protected]. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"[email protected]"> [email protected] </a>

Then it does not matter unless the devices can only access 1G or 2G.

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in the body to [email protected]. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"[email protected]"> [email protected] </a>

I didn't bother change bootmem because the mask allocation runs sufficiently early that it just allocates the memory it needs and bootmem will skip that then because it sees it as used. Moving it up would just save a few cycles in skipping bits in the bitmap. Ok it wouldn't hurt I guess, but also not really help.

What sparsemem reference do you mean?

It still exists, but is variable now.

I didn't want to do this because the DMA zone is ill defined (see my long intro in 0/0), so really nobody should be using it directly.

But the ZONE_DMA32 actually makes sense, but changing the semantics under such a large driver base isn't a good idea.

My goal is still to eliminate all the GFP_DMAs for x86 (it is not that much work left ...) and then just remove the compat hooks. The few GFP_DMA32 users can stay though.

It depends on the requirements of the bootmem user. Some do need memory <4GB, some do not. The mask allocator itself is a client in fact and it requires low memory of course.

-Andi

Its just the use of MAX_DMA_ADDRESS in mm/sparse-vmemmap.c

The driver base can only be the x86_64 only device drivers since the zone is not used by any other architectures. That is fairly small AFAICT.

The point is that it would be good to relocate as much memory allocated at boot as possible beyond 4GB.

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in the body to [email protected]. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"[email protected]"> [email protected] </a>

x86 was the only user and that one doesn't use it anymore since it was converted to mask alloc. So remove it.

---
 include/linux/mm.h |    1 -
 mm/page_alloc.c    |   24 ------------------------
 2 files changed, 25 deletions(-)

Index: linux/mm/page_alloc.c
===================================================================
@@ -120,7 +120,6 @@ int min_free_kbytes = 1024;
 
 unsigned long __meminitdata nr_kernel_pages;
 unsigned long __meminitdata nr_all_pages;
-static unsigned long __meminitdata dma_reserve;
 
 #ifdef CONFIG_ARCH_POPULATES_NODE_MAP
   /*

@@ -3321,13 +3320,6 @@ static void __paginginit free_area_init_
 				"  %s zone: %lu pages exceeds realsize %lu\n",
 				zone_names[j], memmap_pages, realsize);
 
-		/* Account for reserved pages */
-		if (j == 0 && realsize > dma_reserve) {
-			realsize -= dma_reserve;
-			printk(KERN_DEBUG "  %s zone: %lu pages reserved\n",
-					zone_names[0], dma_reserve);
-		}
-
 		if (!is_highmem_idx(j))
 			nr_kernel_pages += realsize;
 		nr_all_pages += realsize;

@@ -3906,22 +3898,6 @@ early_param("movablecore", cmdline_parse
 
 #endif /* CONFIG_ARCH_POPULATES_NODE_MAP */
 
-/**
- * set_dma_reserve - set the specified number of pages reserved in the first zone
- * @new_dma_reserve: The number of pages to mark reserved
- *
- * The per-cpu batchsize and zone watermarks are determined by present_pages.
- * In the DMA zone, a significant percentage may be consumed by kernel image
- * and other unfreeable allocations which can skew the watermarks badly. This
- * function may optionally be used to account for unfreeable pages in the
- * first zone (e.g., ZONE_DMA). The effect will be lower watermarks and
- * smaller per-cpu batchsize.
- */
-void __init set_dma_reserve(unsigned long new_dma_reserve)
-{
-	dma_reserve = new_dma_reserve;
-}
-
 #ifndef CONFIG_NEED_MULTIPLE_NODES
 static bootmem_data_t contig_bootmem_data;
 struct pglist_data contig_page_data = { .bdata = &contig_bootmem_data };

Index: linux/include/linux/mm.h
===================================================================
@@ -987,7 +987,6 @@ extern void sparse_memory_present_with_a
 extern int early_pfn_to_nid(unsigned long pfn);
 #endif /* CONFIG_HAVE_ARCH_EARLY_PFN_TO_NID */
 #endif /* CONFIG_ARCH_POPULATES_NODE_MAP */
-extern void set_dma_reserve(unsigned long new_dma_reserve);
 extern void memmap_init_zone(unsigned long, int, unsigned long,
 				unsigned long, enum memmap_context);
 extern void setup_per_zone_pages_min(void);

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in the body to [email protected]. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"[email protected]"> [email protected] </a>

---
 arch/x86/kernel/pci-dma_32.c |   17 ++++++++++-------
 1 file changed, 10 insertions(+), 7 deletions(-)

Index: linux/arch/x86/kernel/pci-dma_32.c
===================================================================
@@ -27,11 +27,12 @@ void *dma_alloc_coherent(struct device *
 {
 	void *ret;
 	struct dma_coherent_mem *mem = dev ? dev->dma_mem : NULL;
-	int order = get_order(size);
+	u64 mask;
 	/* ignore region specifiers */
-	gfp &= ~(__GFP_DMA | __GFP_HIGHMEM);
+	gfp &= ~__GFP_HIGHMEM;
 
 	if (mem) {
+		int order = get_order(size);
 		int page = bitmap_find_free_region(mem->bitmap, mem->size,
 						     order);
 		if (page >= 0) {

@@ -44,10 +45,12 @@ void *dma_alloc_coherent(struct device *
 			return NULL;
 	}
 
-	if (dev == NULL || (dev->coherent_dma_mask < 0xffffffff))
-		gfp |= GFP_DMA;
+	if (dev == NULL)
+		mask = TRAD_DMA_MASK;
+	else
+		mask = dev->coherent_dma_mask;
 
-	ret = (void *)__get_free_pages(gfp, order);
+	ret = get_pages_mask(gfp, size, mask);
 
 	if (ret != NULL) {
 		memset(ret, 0, size);

@@ -61,15 +64,15 @@ void dma_free_coherent(struct device *de
 			 void *vaddr, dma_addr_t dma_handle)
 {
 	struct dma_coherent_mem *mem = dev ? dev->dma_mem : NULL;
-	int order = get_order(size);
 
 	WARN_ON(irqs_disabled());	/* for portability */
 	if (mem && vaddr >= mem->virt_base && vaddr < (mem->virt_base + (mem->size << PAGE_SHIFT))) {
+		int order = get_order(size);
 		int page = (vaddr - mem->virt_base) >> PAGE_SHIFT;
 
 		bitmap_release_region(mem->bitmap, page, order);
 	} else
-		free_pages((unsigned long)vaddr, order);
+		free_pages_mask(vaddr, size);
 }
 EXPORT_SYMBOL(dma_free_coherent);
 

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in the body to [email protected]. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"[email protected]"> [email protected] </a>

---
 arch/x86/kernel/pci-dma_64.c |   49 +++++++++++++------------------------------
 1 file changed, 15 insertions(+), 34 deletions(-)

Index: linux/arch/x86/kernel/pci-dma_64.c
===================================================================
@@ -47,11 +47,16 @@ struct device fallback_dev = {
 
 /* Allocate DMA memory on node near device */
 noinline static void *
-dma_alloc_pages(struct device *dev, gfp_t gfp, unsigned order)
+dma_alloc_pages(struct device *dev, gfp_t gfp, unsigned size,
+		unsigned long dma_mask)
 {
 	struct page *page;
 	int node;
 
+	/* For small masks use DMA allocator without node affinity */
+	if (dma_mask < DMA_32BIT_MASK)
+		return get_pages_mask(gfp, size, dma_mask);
+
 	node = dev_to_node(dev);
 	if (node == -1)
 		node = numa_node_id();

@@ -59,7 +64,8 @@ dma_alloc_pages(struct device *dev, gfp_
 	if (node < first_node(node_online_map))
 		node = first_node(node_online_map);
 
-	page = alloc_pages_node(node, gfp, order);
+	page = alloc_pages_node(node, gfp, get_order(size));
+
 	return page ? page_address(page) : NULL;
 }
 

@@ -91,15 +97,10 @@ dma_alloc_coherent(struct device *dev, s
 	   uses the normal dma_mask for alloc_coherent. */
 	dma_mask &= *dev->dma_mask;
 
-	/* Why <=? Even when the mask is smaller than 4GB it is often
-	   larger than 16MB and in this case we have a chance of
-	   finding fitting memory in the next higher zone first. If
-	   not retry with true GFP_DMA. -AK */
 	if (dma_mask <= DMA_32BIT_MASK)
 		gfp |= GFP_DMA32;
 
- again:
-	memory = dma_alloc_pages(dev, gfp, get_order(size));
+	memory = dma_alloc_pages(dev, gfp, size, dma_mask);
 	if (memory == NULL)
 		return NULL;
 

@@ -108,25 +109,10 @@ dma_alloc_coherent(struct device *dev, s
 		bus = virt_to_bus(memory);
 	        high = (bus + size) >= dma_mask;
 		mmu = high;
-		if (force_iommu && !(gfp & GFP_DMA))
+		if (force_iommu)
 			mmu = 1;
 		else if (high) {
-			free_pages((unsigned long)memory,
-				   get_order(size));
-
-			/* Don't use the 16MB ZONE_DMA unless absolutely
-			   needed. It's better to use remapping first. */
-			if (dma_mask < DMA_32BIT_MASK && !(gfp & GFP_DMA)) {
-				gfp = (gfp & ~GFP_DMA32) | GFP_DMA;
-				goto again;
-			}
-
-			/* Let low level make its own zone decisions */
-			gfp &= ~(GFP_DMA32|GFP_DMA);
-
-			if (dma_ops->alloc_coherent)
-				return dma_ops->alloc_coherent(dev, size,
-							   dma_handle, gfp);
+			free_pages_mask(memory, size);
 			return NULL;
 		}
 

@@ -137,12 +123,6 @@ dma_alloc_coherent(struct device *dev, s
 		}
 	}
 
-	if (dma_ops->alloc_coherent) {
-		free_pages((unsigned long)memory, get_order(size));
-		gfp &= ~(GFP_DMA|GFP_DMA32);
-		return dma_ops->alloc_coherent(dev, size, dma_handle, gfp);
-	}
-
 	if (dma_ops->map_simple) {
 		*dma_handle = dma_ops->map_simple(dev, memory,
 					      size,

@@ -153,7 +133,7 @@ dma_alloc_coherent(struct device *dev, s
 
 	if (panic_on_overflow)
 		panic("dma_alloc_coherent: IOMMU overflow by %lu bytes\n",size);
-	free_pages((unsigned long)memory, get_order(size));
+	free_pages_mask(memory, size);
 	return NULL;
 }
 EXPORT_SYMBOL(dma_alloc_coherent);

@@ -166,9 +146,10 @@ void dma_free_coherent(struct device *de
 			 void *vaddr, dma_addr_t bus)
 {
 	WARN_ON(irqs_disabled());	/* for portability */
+	/* RED-PEN swiotlb does unnecessary copy here */
 	if (dma_ops->unmap_single)
 		dma_ops->unmap_single(dev, bus, size, 0);
-	free_pages((unsigned long)vaddr, get_order(size));
+	free_pages_mask(vaddr, size);
 }
 EXPORT_SYMBOL(dma_free_coherent);
 

@@ -191,7 +172,7 @@ int dma_supported(struct device *dev, u6
 
 	/* Copied from i386. Doesn't make much sense, because it will
 	   only work for pci_alloc_coherent.
-	   The caller just has to use GFP_DMA in this case. */
+	   The caller just has to use *_mask allocations in this case. */
         if (mask < DMA_24BIT_MASK)
                 return 0;
 

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in the body to [email protected]. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"[email protected]"> [email protected] </a>

---
 include/linux/vmstat.h |    4 ++++
 mm/vmstat.c            |    6 ++++++
 2 files changed, 10 insertions(+)

Index: linux/include/linux/vmstat.h
===================================================================
@@ -41,6 +41,10 @@ enum vm_event_item { PGPGIN, PGPGOUT, PS
 		MASK_ALLOC, MASK_FREE, MASK_BITMAP_SKIP, MASK_WAIT,
 		MASK_HIGHER, MASK_LOW_WASTE, MASK_HIGH_WASTE,
 #endif
+#ifdef CONFIG_SWIOTLB_MASK_ALLOC
+		SWIOTLB_USED_PAGES, SWIOTLB_BYTES_WASTED, SWIOTLB_NUM_ALLOCS,
+		SWIOTLB_NUM_FREES,
+#endif
 		NR_VM_EVENT_ITEMS
 };
 

Index: linux/mm/vmstat.c
===================================================================
@@ -654,6 +654,12 @@ static const char * const vmstat_text[] 
 	"mask_low_waste",
 	"mask_high_waste",
 #endif
+#ifdef CONFIG_SWIOTLB_MASK_ALLOC
+	"swiotlb_used_pages",
+	"swiotlb_bytes_wasted",
+	"swiotlb_num_allocs",
+	"swiotlb_num_frees",
+#endif
 #endif
 };
 

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in the body to [email protected]. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"[email protected]"> [email protected] </a>

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in the body to [email protected]. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"[email protected]"> [email protected] </a>

The swiotlb and DMA pools already interact. Instead of using two separate pools just always use the mask allocator pool. When swiotlb is active the mask allocator pool is extended by the respective swiotlb size (64MB by default as with the old code). Then all swiotlb bouncing happens directly through the maskable zone.

This is ok because there are no "normal" memory consumers in the maskable zone and (nearly) all users who allocate from it would already allocate from swiotlb before.

The code is based on the original swiotlb code, but heavily modified.

One difference now is that the swiotlb is only managed in page size chunks now. The previous implementation used 2K "slabs". I'm still running statistics to see if it's worth doing something about this.

lib/swiotlb.c is now not used anymore on x86.

---
 arch/x86/Kconfig                 |    6 
 arch/x86/kernel/Makefile         |    2 
 arch/x86/kernel/pci-dma_64.c     |    4 
 arch/x86/kernel/pci-swiotlb_64.c |  431 +++++++++++++++++++++++++++++++++++++--
 include/asm-x86/swiotlb.h        |    2 
 include/linux/page-flags.h       |    1 
 6 files changed, 420 insertions(+), 26 deletions(-)

Index: linux/arch/x86/kernel/pci-dma_64.c
===================================================================
@@ -250,7 +250,7 @@ static __init int iommu_setup(char *p)
 		if (!strncmp(p, "nodac", 5))
 			forbid_dac = -1;
 
-#ifdef CONFIG_SWIOTLB
+#ifdef CONFIG_SWIOTLB_MASK_ALLOC
 		if (!strncmp(p, "soft",4))
 			swiotlb = 1;
 #endif

@@ -288,7 +288,7 @@ void __init pci_iommu_alloc(void)
 
 	detect_intel_iommu();
 
-#ifdef CONFIG_SWIOTLB
+#ifdef CONFIG_SWIOTLB_MASK_ALLOC
 	pci_swiotlb_init();
 #endif
 }

Index: linux/arch/x86/kernel/Makefile
===================================================================
@@ -95,5 +95,5 @@ ifeq ($(CONFIG_X86_64),y)
 
         obj-$(CONFIG_GART_IOMMU)	+= pci-gart_64.o aperture_64.o
         obj-$(CONFIG_CALGARY_IOMMU)	+= pci-calgary_64.o tce_64.o
-        obj-$(CONFIG_SWIOTLB)		+= pci-swiotlb_64.o
+	obj-$(CONFIG_SWIOTLB_MASK_ALLOC) += pci-swiotlb_64.o
 endif

Index: linux/arch/x86/Kconfig
===================================================================
@@ -437,7 +437,7 @@ config HPET_EMULATE_RTC
 config GART_IOMMU
 	bool "GART IOMMU support" if EMBEDDED
 	default y
-	select SWIOTLB
+	select SWIOTLB_MASK_ALLOC
 	select AGP
 	depends on X86_64 && PCI
 	help

@@ -453,7 +453,7 @@ config GART_IOMMU
 
 config CALGARY_IOMMU
 	bool "IBM Calgary IOMMU support"
-	select SWIOTLB
+	select SWIOTLB_MASK_ALLOC
 	depends on X86_64 && PCI && EXPERIMENTAL
 	help
 	  Support for hardware IOMMUs in IBM's xSeries x366 and x460

@@ -484,7 +484,7 @@ config IOMMU_HELPER
 	def_bool (CALGARY_IOMMU || GART_IOMMU)
 
 # need this always selected by IOMMU for the VIA workaround
-config SWIOTLB
+config SWIOTLB_MASK_ALLOC
 	bool
 	help
 	  Support for software bounce buffers used on x86-64 systems

Index: linux/include/asm-x86/swiotlb.h
===================================================================
@@ -43,7 +43,7 @@ extern void swiotlb_init(void);
 
 extern int swiotlb_force;
 
-#ifdef CONFIG_SWIOTLB
+#ifdef CONFIG_SWIOTLB_MASK_ALLOC
 extern int swiotlb;
 #else
 #define swiotlb 0

Index: linux/include/linux/page-flags.h
===================================================================
@@ -107,6 +107,7 @@
  *         63                            32                              0
  */
 #define PG_uncached		31	/* Page has been mapped as uncached */
+#define PG_swiotlb		30	/* Page in swiotlb */
 #endif
 
 /*

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in the body to [email protected]. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"[email protected]"> [email protected] </a>

Just a side-comment -- not necessarily, given CONFIG_PHYSICAL_START.

Rene.

By the way, 1/13 didn't make it to LKML.

Rene.

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in the body to [email protected]. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"[email protected]"> [email protected] </a>

Yes, i was told. Not sure what happened. I'm pretty sure my script has posted it. If anybody wants it it's in ftp://firstfloor.org/pub/ak/mask/patches/bitops

-Andi

"In practice". People usually do not change that. Anyways the dma zone will be fully compatible. If you have moved your kernel outside the 16MB area and there is nothing else there (normally there is some other stuff there too) you'll get the full 16MB for the default compat pool.

-Andi

Em Fri, 7 Mar 2008 10:07:10 +0100 (CET)
Andi Kleen <[email protected]> escreveu:

| I chose to implement a new "maskable memory" allocator to solve these
| problems. The existing page buddy allocator is not really suited for
| this because the data structures don't allow cheap allocation by physical 
| address boundary. 

These patches are supposed to work, I think?

I've tried to give them a try but got some problems. First, the
simple test case seems to fail miserably:

"""
testing mask alloc upto 24 bits
gpm1 3 mask 3fffff size 20440 total 62KB failed
gpm1 4 mask 3fffff size 24369 total 62KB failed
gpm1 6 mask 3fffff size 15255 total 64KB failed
gpm1 7 mask 3fffff size 12676 total 64KB failed
gpm1 8 mask 3fffff size 23917 total 64KB failed
gpm1 9 mask 3fffff size 11682 total 64KB failed
gpm1 10 mask 3fffff size 23091 total 64KB failed
gpm1 11 mask 3fffff size 16880 total 64KB failed
gpm1 12 mask 3fffff size 17257 total 64KB failed
gpm1 13 mask 3fffff size 8686 total 64KB failed
gpm1 14 mask 3fffff size 9871 total 64KB failed
gpm1 15 mask 3fffff size 19740 total 64KB failed
gpm1 16 mask 3fffff size 11557 total 64KB failed
gpm1 18 mask 3fffff size 23723 total 67KB failed
gpm1 19 mask 3fffff size 16136 total 67KB failed
gpm2 6 mask 3fffff size 4471 failed
gpm2 7 mask 3fffff size 16868 failed
gpm2 8 mask 3fffff size 22093 failed
gpm2 9 mask 3fffff size 17666 failed
gpm2 11 mask 3fffff size 14416 failed
gpm2 12 mask 3fffff size 10825 failed
gpm2 13 mask 3fffff size 3918 failed
gpm2 14 mask 3fffff size 6255 failed
gpm2 15 mask 3fffff size 2428 failed
gpm2 16 mask 3fffff size 517 failed
gpm2 18 mask 3fffff size 12890 failed
gpm2 19 mask 3fffff size 3211 failed
verify & free
mask fffff
mask 1fffff
mask 3fffff
mask 7fffff
mask ffffff
done
"""

Then boot up goes on and while init is running I get this:

"""
Starting udev: ------------[ cut here ]------------ kernel BUG at mm/mask-alloc.c:178!
invalid opcode: 0000 [#1]

Modules linked in: parport_pc(+) parport snd_via82xx(+) gameport snd_ac97_codec rtc_cmos ac97_bus rtc_core rtc_lib snd_pcm snd_timer snd_page_alloc snd_mpu401_uart snd_rawmidi snd_seq_device pcspkr snd soundcore evdev i2c_viapro i2c_core thermal button processor ohci1394 ide_cd_mod ieee1394 cdrom firewire_ohci shpchp via_agp firewire_core crc_itu_t agpgart pci_hotplug 8139too mii via82cxxx ide_disk ide_core ext3 jbd uhci_hcd ohci_hcd ehci_hcd usbcore

Pid: 1088, comm: modprobe Not tainted (2.6.25-0.rc4.1mdv #1)
EIP: 0060:[<c016d493>] EFLAGS: 00010206 CPU: 0
EIP is at alloc_pages_mask+0x74/0x3de
EAX: 00000000 EBX: 00001000 ECX: df8fb854 EDX: 000004e2
ESI: 00000000 EDI: 00010000 EBP: dfbf9c44 ESP: dfbf9bec
DS: 007b ES: 007b FS: 0000 GS: 0033 SS: 0068
Process modprobe (pid: 1088, ti=dfbf8000 task=dfb9c1a0 task.ti=dfbf8000)

Stack: c0486858 004f1000 004f1000 dfbf9c04 00001000 000052d0 dfbf9c28 000fffff 
       00000001 000004e2 dfbf8000 00000000 c0496ea0 dfb9c1a0 00000000 dfbf9c40 
       c010ab33 00000000 c02c9250 00001000 00000000 00010000 dfbf9c54 c016d80b 
Call Trace:
 [<c010ab33>] ? save_stack_trace+0x1c/0x3a
 [<c016d80b>] ? get_pages_mask+0xe/0x23
 [<c0107b83>] ? dma_alloc_coherent+0xab/0xe5
 [<c010aab6>] ? save_stack_address+0x0/0x2c
 [<e095634f>] ? snd_dma_alloc_pages+0xef/0x1ef [snd_page_alloc]
 [<e0956a17>] ? snd_malloc_sgbuf_pages+0xdd/0x170 [snd_page_alloc]
 [<c0137e1f>] ? trace_hardirqs_on+0xe6/0x11b
 [<c02bf3ff>] ? __mutex_unlock_slowpath+0xf2/0x104
 [<e0956409>] ? snd_dma_alloc_pages+0x1a9/0x1ef [snd_page_alloc]
 [<c02bf419>] ? mutex_unlock+0x8/0xa
 [<e095689f>] ? snd_dma_get_reserved_buf+0xb4/0xbd [snd_page_alloc]
 [<e09a1d77>] ? snd_pcm_lib_preallocate_pages+0x5b/0x115 [snd_pcm]
 [<e09a1e62>] ? snd_pcm_lib_preallocate_pages_for_all+0x31/0x56 [snd_pcm]
 [<e09ae2ab>] ? snd_via82xx_probe+0xb22/0xe6d [snd_via82xx]
 [<c02bf043>] ? mutex_trylock+0xf3/0x10f
 [<e09ac083>] ? snd_via82xx_mixer_free_ac97+0x0/0x12 [snd_via82xx]
 [<c02c0785>] ? _spin_unlock+0x1d/0x20
 [<c01e02d4>] ? pci_match_device+0x9a/0xa5
 [<c01e0393>] ? pci_device_probe+0x39/0x59
 [<c0238115>] ? driver_probe_device+0x9f/0x119
 [<c0238274>] ? __driver_attach+0x4a/0x7f
 [<c02376ed>] ? bus_for_each_dev+0x39/0x5b
 [<c0237fad>] ? driver_attach+0x14/0x16
 [<c023822a>] ? __driver_attach+0x0/0x7f
 [<c0237dc7>] ? bus_add_driver+0x9d/0x1ae
 [<c0238433>] ? driver_register+0x47/0xa3
 [<c01e055f>] ? __pci_register_driver+0x40/0x6d
 [<e093f017>] ? alsa_card_via82xx_init+0x17/0x19 [snd_via82xx]
 [<c013ed7a>] ? sys_init_module+0x13b0/0x14a8
 [<c0122ae5>] ? __request_region+0x0/0x90
 [<c010496d>] ? sysenter_past_esp+0xb6/0xc9
 [<c0137e1f>] ? trace_hardirqs_on+0xe6/0x11b
 [<c0104924>] ? sysenter_past_esp+0x6d/0xc9
 =======================
Code: 00 74 1b 8b 45 b8 c7 45 d4 ff ff ff ff 48 c1 e8 0b ff 45 d4 d1 e8 75 f9 83 7d b8 0f 77 04 0f 0b eb fe f7 45 bc 05 40 00 00 74 04 <0f> 0b eb fe 8b 4d bc 80 cd 02 89 4d b0 d1 e9 89 4d ac ff 05 84 
EIP: [<c016d493>] alloc_pages_mask+0x74/0x3de SS:ESP 0068:dfbf9bec
---[ end trace c02107712b611dcb ]---

""""

How can I help to debug this?

Btw, I've created a package to test this for my convenience
(that's why you see 'mdv' in the kernel name), but it's a vanilla kernel with your patches on top.

Luiz Fernando N. Capitulino

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to [email protected].  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"[email protected]"> [email protected] </a>

Yes they work fine here and survived quite some stress testing. But of course I only have limited configurations.

Hmm I guess you got a pretty filled up 16MB area already. Do you have a full log? It just couldn't allocate some memory for the 24bit mask, but that is likely because it just ran out of memory.

I suppose it will work if you cut down the allocations in the test case a bit, e.g. decrease NUMALLOC to 10 and perhaps MAX_LEN to 5*PAGE_SIZE. Did that in my copy.

Ah, I see the problem. Your sound driver allocates a dma area < 16 bytes. I had added a BUG_ON for that to catch some mistakes (of passing order instead of size), but it triggers here incorrectly.

Didn't see that in my testing.

I put up an updated patchkit on ftp://firstfloor.org/pub/ak/mask/patches/ It also has some other fixes.

Can you retest with that please?

Thanks for testing.

-Andi

Em Sat, 8 Mar 2008 01:46:54 +0100
Andi Kleen <[email protected]> escreveu:

| >  I've tried to give them a try but got some problems. First, the
| > simple test case seems to fail miserably:
| 
| Hmm I guess you got a pretty filled up 16MB area already.
| Do you have a full log? It just couldn't allocate some memory
| for the 24bit mask, but that is likely because it just ran out of 
| memory.

You'll find the full log here:

http://users.mandriva.com.br/~lcapitulino/tmp/minicom.cap

| I suppose it will work if you cut down the allocations in the 
| test case a bit, e.g. decrease NUMALLOC to 10 and perhaps MAX_LEN to
| 5*PAGE_SIZE. Did that in my copy.

[...]

| I put up an updated patchkit on ftp://firstfloor.org/pub/ak/mask/patches/
| It also has some other fixes.
| 
| Can you retest with that please?

Yes, will do. But I have something to suggest.

I saw that the patches you've posted here are only a subset of
the patches you have in your FTP. Should I test all the patches you have or only the subset you posted?

If you want to get only the subset tested, would be nice to
create a new directory in the FTP for them, that would make my life easier a bit.

Luiz Fernando N. Capitulino

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to [email protected].  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"[email protected]"> [email protected] </a>

The ftp directory contains the SCSI patchkit (which I have been posting to linux-scsi for quite some time, but it didn't get applied yet) and the block patchkit (which hit linux-kernel earlier, but was unfortunately not graced by a reply by the block maintainer so far)

Best you test all together. The subsets are independent (as in kernel should work with any subset of them), but they all clean up DMA memory related issues.

-Andi

Em Mon, 10 Mar 2008 19:08:43 +0100
Andi Kleen <[email protected]> escreveu:

| > you have or only the subset you posted?
| 
| Best you test all together. The subsets are independent
| (as in kernel should work with any subset of them), but 
| they all clean up DMA memory related issues.

Ok, now it works with the test case fixed:

"""
testing mask alloc upto 24 bits
verify & free
mask fffff
mask 1fffff
mask 3fffff
mask 7fffff
mask ffffff
done
"""

Do you remember the BUG_ON() I was getting because of the
sound card driver [1] ?

It seems to me that the problem is that it's setting
__GFP_COMP. The following patch fixes it for me:

"""

Index: linux-2.6.24/sound/core/memalloc.c
===================================================================
@@ -218,7 +218,6 @@ static void *snd_malloc_dev_pages(struct
 	snd_assert(dma != NULL, return NULL);
 	pg = get_order(size);
 	gfp_flags = GFP_KERNEL
-		| __GFP_COMP	/* compound page lets parts be mapped */
 		| __GFP_NORETRY /* don't trigger OOM-killer */
 		| __GFP_NOWARN; /* no stack trace print - this call is non-critical */
 	res = dma_alloc_coherent(dev, PAGE_SIZE << pg, dma, gfp_flags);

"""

Also, I think you forgot to protect the __free_pages_mask()
call with "#ifdef CONFIG_MASK_ALLOC" in patch mask-compat.

Question: let's say that the devices I have consumes only
5MB of the whole reserved pool (16MB), of course that it's ok to reduce the pool to 5MB, right?

I have more machines to test this stuff...

[1] http://users.mandriva.com.br/~lcapitulino/tmp/minicom.cap

Luiz Fernando N. Capitulino

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to [email protected].  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"[email protected]"> [email protected] </a>

PageMaskAlloc() returns constant 0 in this case so the compiler should e liminate it.

It's likely less than 16MB because there are already some users in there like the kernel text. The default sizing without swiotlb just grabs anything left over up to the 16Mb boundary.

Yes, but not leaving any free over is a little risky, someone might need more later. But e.g. going down to 8-9MB would be likely possible.

The long term plan (but that is some time off and a little vague still) would be to let the various subsystem size the mask zone dynamically for their need.

Then especially on 64bit swiotlb machines who allocate 64MB by default for swiotlb that would save significant memory.

Thanks. Keep me posted.

-Andi

Em Tue, 11 Mar 2008 18:35:40 +0100
Andi Kleen <[email protected]> escreveu:

| On Tue, Mar 11, 2008 at 02:26:24PM -0300, Luiz Fernando N. Capitulino wrote:
| > ===================================================================
| > --- linux-2.6.24.orig/sound/core/memalloc.c
| > +++ linux-2.6.24/sound/core/memalloc.c
| > @@ -218,7 +218,6 @@ static void *snd_malloc_dev_pages(struct
| >  	snd_assert(dma != NULL, return NULL);
| >  	pg = get_order(size);
| >  	gfp_flags = GFP_KERNEL
| > -		| __GFP_COMP	/* compound page lets parts be mapped */
| 
| Oops. Thanks. I'll double check that. mask allocator indeed doesn't
| handle __GFP_COMP and nobody should be passing that into dma_alloc_coherent
| anyways. But the bug you got was for the small size wasn't it?

No, it triggers the BUG_ON() which checks the gfp, not the one
which checks MASK_MIN_SIZE.

On the other hand I'm not sure whether it does the right thing
(ie, pass size in bytes instead of order) it does:

"""
pg = get_order(size);
[...]
res = dma_alloc_coherent(dev, PAGE_SIZE << pg, dma, gfp_flags); if (res != NULL)

	inc_snd_pages(pg);

"""

Maybe it could be changed to:

"""
res = dma_alloc_coherent(dev, size, dma, gfp_flags); if (res != NULL)

	inc_snd_pages(get_order(size));

"""

But sound works (simple tests).

| > ok to reduce the pool to 5MB, right?
| 
| Yes, but not leaving any free over is a little risky, someone might need
| more later. But e.g. going down to 8-9MB would be likely possible.

Ok.

| The long term plan (but that is some time off and a little vague still) 
| would be to let the various subsystem size the mask zone dynamically for 
| their need.

That sounds cool.

Luiz Fernando N. Capitulino

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to [email protected].  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"[email protected]"> [email protected] </a>

I see. I misdiagnosed your original problem then. But fixing the size < 16 bytes case was a good idea anyways, someone else would have triggered that.

With the mask allocator it can be changed to pass size directly and save some memory. Before that it didn't make any difference.

Can you perhaps send me a complete patch fixing that for sound and the __GFP_COMP with description and Signed-off-by etc.? I can add it to my patchkit then and you would be correctly attributed. Otherwise I can do it myself too if you prefer. I'll also do a grep over the tree for other such bogus __GFP_COMP users. That was an issue I hadn't considered before.

Agreed.

-Andi

Em Tue, 11 Mar 2008 19:49:26 +0100
Andi Kleen <[email protected]> escreveu:

| > | Oops. Thanks. I'll double check that. mask allocator indeed doesn't
| > | handle __GFP_COMP and nobody should be passing that into dma_alloc_coherent
| > | anyways. But the bug you got was for the small size wasn't it?
| > 
| >  No, it triggers the BUG_ON() which checks the gfp, not the one
| > which checks MASK_MIN_SIZE.
| 
| I see. I misdiagnosed your original problem then. But fixing the 
| size < 16 bytes case was a good idea anyways, someone else would
| have triggered that.

I see.

| Can you perhaps send me a complete patch fixing that for sound and the 
| __GFP_COMP with description and Signed-off-by etc.? I can add it to my 
| patchkit then and you would be correctly attributed. Otherwise I can do it 
| myself too if you prefer. I'll also do a grep over the tree for other
| such bogus __GFP_COMP users. That was an issue I hadn't considered before.

Here are you (passed minimal tests).

------
ALSA: Convert snd_malloc_dev_pages() to the mask allocator

The mask allocator do not handle the __GFP_COMP flag and will BUG_ON() if that flag is passed to it.

Also, we should pass the allocation size in bytes to dma_alloc_coherent().

Index: linux-2.6.24/sound/core/memalloc.c
===================================================================
@@ -210,20 +210,17 @@ void snd_free_pages(void *ptr, size_t si
 /* allocate the coherent DMA pages */
 static void *snd_malloc_dev_pages(struct device *dev, size_t size, dma_addr_t *dma)
 {
-	int pg;
 	void *res;
 	gfp_t gfp_flags;
 
 	snd_assert(size > 0, return NULL);
 	snd_assert(dma != NULL, return NULL);
-	pg = get_order(size);
 	gfp_flags = GFP_KERNEL
-		| __GFP_COMP	/* compound page lets parts be mapped */
 		| __GFP_NORETRY /* don't trigger OOM-killer */
 		| __GFP_NOWARN; /* no stack trace print - this call is non-critical */
-	res = dma_alloc_coherent(dev, PAGE_SIZE << pg, dma, gfp_flags);
+	res = dma_alloc_coherent(dev, size, dma, gfp_flags);
 	if (res != NULL)
-		inc_snd_pages(pg);
+		inc_snd_pages(get_order(size));
 
 	return res;
 }

Luiz Fernando N. Capitulino

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to [email protected].  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"[email protected]"> [email protected] </a>

That would be greatly appreciated. Thanks for doing this.

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in the body to [email protected]. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"[email protected]"> [email protected] </a>

I'm afraid it would not help you directly because you would still need to maintain that code for s390 (seems to be a heavy GFP_DMA user) and probably some other architectures (unless you can get these maintainers to get rid of GFP_DMA too) With my plan it can be just ifdefed and the ifdef not enabled on x86.

-Andi

Undefining ZONE_DMA will remove support for GFP_DMA from the slab allocators. Your patch is already doing that.

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in the body to [email protected]. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"[email protected]"> [email protected] </a>