From: Joonsoo Kim <hidden> Date: 2014-06-03 01:08:45
Currently, there are two users on CMA functionality, one is the DMA
subsystem and the other is the kvm on powerpc. They have their own code
to manage CMA reserved area even if they looks really similar.
From my guess, it is caused by some needs on bitmap management. Kvm side
wants to maintain bitmap not for 1 page, but for more size. Eventually it
use bitmap where one bit represents 64 pages.
When I implement CMA related patches, I should change those two places
to apply my change and it seem to be painful to me. I want to change
this situation and reduce future code management overhead through
this patch.
This change could also help developer who want to use CMA in their
new feature development, since they can use CMA easily without
copying & pasting this reserved area management code.
Now, we are in merge window, so this is not for merging. I'd like to
listen opinion from people who related to this stuff before actually
trying to merge this patchset. If all agree with this change, I will
resend it after rc1.
Thanks.
Joonsoo Kim (3):
CMA: generalize CMA reserved area management functionality
DMA, CMA: use general CMA reserved area management framework
PPC, KVM, CMA: use general CMA reserved area management framework
arch/powerpc/kvm/book3s_hv_builtin.c | 17 +-
arch/powerpc/kvm/book3s_hv_cma.c | 240 -------------------------
arch/powerpc/kvm/book3s_hv_cma.h | 27 ---
drivers/base/Kconfig | 10 --
drivers/base/dma-contiguous.c | 230 ++----------------------
include/linux/cma.h | 28 +++
include/linux/dma-contiguous.h | 7 +-
mm/Kconfig | 11 ++
mm/Makefile | 1 +
mm/cma.c | 329 ++++++++++++++++++++++++++++++++++
10 files changed, 396 insertions(+), 504 deletions(-)
delete mode 100644 arch/powerpc/kvm/book3s_hv_cma.c
delete mode 100644 arch/powerpc/kvm/book3s_hv_cma.h
create mode 100644 include/linux/cma.h
create mode 100644 mm/cma.c
--
1.7.9.5
From: Joonsoo Kim <hidden> Date: 2014-06-03 01:08:50
Now, we have general CMA reserved area management framework,
so use it for future maintainabilty. There is no functional change.
Signed-off-by: Joonsoo Kim <redacted>
@@ -224,176 +163,31 @@ int __init dma_contiguous_reserve_area(phys_addr_t size, phys_addr_t base,phys_addr_tlimit,structcma**res_cma,boolfixed){-structcma*cma=&cma_areas[cma_area_count];-phys_addr_talignment;-intret=0;--pr_debug("%s(size %lx, base %08lx, limit %08lx)\n",__func__,-(unsignedlong)size,(unsignedlong)base,-(unsignedlong)limit);--/* Sanity checks */-if(cma_area_count==ARRAY_SIZE(cma_areas)){-pr_err("Not enough slots for CMA reserved regions!\n");-return-ENOSPC;-}--if(!size)-return-EINVAL;--/* Sanitise input arguments */-alignment=PAGE_SIZE<<max(MAX_ORDER-1,pageblock_order);-base=ALIGN(base,alignment);-size=ALIGN(size,alignment);-limit&=~(alignment-1);--/* Reserve memory */-if(base&&fixed){-if(memblock_is_region_reserved(base,size)||-memblock_reserve(base,size)<0){-ret=-EBUSY;-gotoerr;-}-}else{-phys_addr_taddr=memblock_alloc_range(size,alignment,base,-limit);-if(!addr){-ret=-ENOMEM;-gotoerr;-}else{-base=addr;-}-}--/*-*Eachreservedareamustbeinitialisedlater,whenmorekernel-*subsystems(likeslaballocator)areavailable.-*/-cma->base_pfn=PFN_DOWN(base);-cma->count=size>>PAGE_SHIFT;-*res_cma=cma;-cma_area_count++;+intret;+structcma*cma;-pr_info("CMA: reserved %ld MiB at %08lx\n",(unsignedlong)size/SZ_1M,-(unsignedlong)base);+ret=cma_declare_contiguous(size,base,limit,0,0,fixed,&cma);+if(ret)+returnret;/* Architecture specific contiguous memory fixup. */dma_contiguous_early_fixup(base,size);-return0;-err:-pr_err("CMA: failed to reserve %ld MiB\n",(unsignedlong)size/SZ_1M);-returnret;-}+*res_cma=cma;-staticvoidclear_cma_bitmap(structcma*cma,unsignedlongpfn,intcount)-{-mutex_lock(&cma->lock);-bitmap_clear(cma->bitmap,pfn-cma->base_pfn,count);-mutex_unlock(&cma->lock);+return0;}-/**-*dma_alloc_from_contiguous()-allocatepagesfromcontiguousarea-*@dev:Pointertodeviceforwhichtheallocationisperformed.-*@count:Requestednumberofpages.-*@align:Requestedalignmentofpages(inPAGE_SIZEorder).-*-*Thisfunctionallocatesmemorybufferforspecifieddevice.Ituses-*devicespecificcontiguousmemoryareaifavailableorthedefault-*globalone.Requiresarchitecturespecificdev_get_cma_area()helper-*function.-*/structpage*dma_alloc_from_contiguous(structdevice*dev,intcount,unsignedintalign){-unsignedlongmask,pfn,pageno,start=0;-structcma*cma=dev_get_cma_area(dev);-structpage*page=NULL;-intret;--if(!cma||!cma->count)-returnNULL;-if(align>CONFIG_CMA_ALIGNMENT)align=CONFIG_CMA_ALIGNMENT;-pr_debug("%s(cma %p, count %d, align %d)\n",__func__,(void*)cma,-count,align);--if(!count)-returnNULL;--mask=(1<<align)-1;---for(;;){-mutex_lock(&cma->lock);-pageno=bitmap_find_next_zero_area(cma->bitmap,cma->count,-start,count,mask);-if(pageno>=cma->count){-mutex_unlock(&cma->lock);-break;-}-bitmap_set(cma->bitmap,pageno,count);-/*-*It'ssafetodropthelockhere.We'vemarkedthisregionfor-*ourexclusiveuse.Ifthemigrationfailswewilltakethe-*lockagainandunmarkit.-*/-mutex_unlock(&cma->lock);--pfn=cma->base_pfn+pageno;-mutex_lock(&cma_mutex);-ret=alloc_contig_range(pfn,pfn+count,MIGRATE_CMA);-mutex_unlock(&cma_mutex);-if(ret==0){-page=pfn_to_page(pfn);-break;-}elseif(ret!=-EBUSY){-clear_cma_bitmap(cma,pfn,count);-break;-}-clear_cma_bitmap(cma,pfn,count);-pr_debug("%s(): memory range at %p is busy, retrying\n",-__func__,pfn_to_page(pfn));-/* try again with a bit different memory target */-start=pageno+mask+1;-}--pr_debug("%s(): returned %p\n",__func__,page);-returnpage;+returncma_alloc(dev_get_cma_area(dev),count,align);}-/**-*dma_release_from_contiguous()-releaseallocatedpages-*@dev:Pointertodeviceforwhichthepageswereallocated.-*@pages:Allocatedpages.-*@count:Numberofallocatedpages.-*-*Thisfunctionreleasesmemoryallocatedbydma_alloc_from_contiguous().-*Itreturnsfalsewhenprovidedpagesdonotbelongtocontiguousareaand-*trueotherwise.-*/booldma_release_from_contiguous(structdevice*dev,structpage*pages,intcount){-structcma*cma=dev_get_cma_area(dev);-unsignedlongpfn;--if(!cma||!pages)-returnfalse;--pr_debug("%s(page %p)\n",__func__,(void*)pages);--pfn=page_to_pfn(pages);--if(pfn<cma->base_pfn||pfn>=cma->base_pfn+cma->count)-returnfalse;--VM_BUG_ON(pfn+count>cma->base_pfn+cma->count);--free_contig_range(pfn,count);-clear_cma_bitmap(cma,pfn,count);--returntrue;+returncma_release(dev_get_cma_area(dev),pages,count);}
From: Joonsoo Kim <hidden> Date: 2014-06-03 01:08:52
Now, we have general CMA reserved area management framework,
so use it for future maintainabilty. There is no functional change.
Signed-off-by: Joonsoo Kim <redacted>
@@ -42,6 +44,8 @@ static unsigned long kvm_cma_resv_ratio = 5;unsignedlongkvm_rma_pages=(1<<27)>>PAGE_SHIFT;/* 128MB */EXPORT_SYMBOL_GPL(kvm_rma_pages);+staticstructcma*kvm_cma;+/* Work out RMLS (real mode limit selector) field value for a given RMA size.AssumesPOWER7orPPC970.*/staticinlineintlpcr_rmls(unsignedlongrma_size)
@@ -133,13 +137,13 @@ struct page *kvm_alloc_hpt(unsigned long nr_pages)/* Old CPUs require HPT aligned on a multiple of its size */if(!cpu_has_feature(CPU_FTR_ARCH_206))align_pages=nr_pages;-returnkvm_alloc_cma(nr_pages,align_pages);+returncma_alloc(kvm_cma,nr_pages,get_order(align_pages));}EXPORT_SYMBOL_GPL(kvm_alloc_hpt);voidkvm_release_hpt(structpage*page,unsignedlongnr_pages){-kvm_release_cma(page,nr_pages);+cma_release(kvm_cma,page,nr_pages);}EXPORT_SYMBOL_GPL(kvm_release_hpt);
@@ -1,27 +0,0 @@-/*- * Contiguous Memory Allocator for ppc KVM hash pagetable based on CMA- * for DMA mapping framework- *- * Copyright IBM Corporation, 2013- * Author Aneesh Kumar K.V <aneesh.kumar@linux.vnet.ibm.com>- *- * This program is free software; you can redistribute it and/or- * modify it under the terms of the GNU General Public License as- * published by the Free Software Foundation; either version 2 of the- * License or (at your optional) any later version of the license.- *- */--#ifndef __POWERPC_KVM_CMA_ALLOC_H__-#define __POWERPC_KVM_CMA_ALLOC_H__-/*- * Both RMA and Hash page allocation will be multiple of 256K.- */-#define KVM_CMA_CHUNK_ORDER 18--extern struct page *kvm_alloc_cma(unsigned long nr_pages,- unsigned long align_pages);-extern bool kvm_release_cma(struct page *pages, unsigned long nr_pages);-extern long kvm_cma_declare_contiguous(phys_addr_t size,- phys_addr_t alignment) __init;-#endif
From: Joonsoo Kim <hidden> Date: 2014-06-03 01:09:53
Currently, there are two users on CMA functionality, one is the DMA
subsystem and the other is the kvm on powerpc. They have their own code
to manage CMA reserved area even if they looks really similar.
From my guess, it is caused by some needs on bitmap management. Kvm side
wants to maintain bitmap not for 1 page, but for more size. Eventually it
use bitmap where one bit represents 64 pages.
When I implement CMA related patches, I should change those two places
to apply my change and it seem to be painful to me. I want to change
this situation and reduce future code management overhead through
this patch.
This change could also help developer who want to use CMA in their
new feature development, since they can use CMA easily without
copying & pasting this reserved area management code.
Signed-off-by: Joonsoo Kim <redacted>
@@ -0,0 +1,329 @@+/*+*ContiguousMemoryAllocator+*+*Copyright(c)2010-2011bySamsungElectronics.+*CopyrightIBMCorporation,2013+*CopyrightLGElectronicsInc.,2014+*Writtenby:+*MarekSzyprowski<m.szyprowski@samsung.com>+*MichalNazarewicz<mina86@mina86.com>+*AneeshKumarK.V<aneesh.kumar@linux.vnet.ibm.com>+*JoonsooKim<iamjoonsoo.kim@lge.com>+*+*Thisprogramisfreesoftware;youcanredistributeitand/or+*modifyitunderthetermsoftheGNUGeneralPublicLicenseas+*publishedbytheFreeSoftwareFoundation;eitherversion2ofthe+*Licenseor(atyouroptional)anylaterversionofthelicense.+*/++#define pr_fmt(fmt) "cma: " fmt++#ifdef CONFIG_CMA_DEBUG+#ifndef DEBUG+# define DEBUG+#endif+#endif++#include<linux/memblock.h>+#include<linux/err.h>+#include<linux/mm.h>+#include<linux/mutex.h>+#include<linux/sizes.h>+#include<linux/slab.h>++structcma{+unsignedlongbase_pfn;+unsignedlongcount;+unsignedlong*bitmap;+unsignedlongbitmap_shift;+structmutexlock;+};++/*+*ThereisalwaysatleastglobalCMAareaandafewoptional+*areasconfiguredinkernel.config.+*/+#define MAX_CMA_AREAS (1 + CONFIG_CMA_AREAS)++staticstructcmacma_areas[MAX_CMA_AREAS];+staticunsignedcma_area_count;+staticDEFINE_MUTEX(cma_mutex);++staticunsignedlongcma_bitmap_mask(structcma*cma,+unsignedlongalign_order)+{+return(1<<(align_order>>cma->bitmap_shift))-1;+}++staticunsignedlongcma_bitmap_max_no(structcma*cma)+{+returncma->count>>cma->bitmap_shift;+}++staticunsignedlongcma_bitmap_pages_to_bits(structcma*cma,+unsignedlongpages)+{+returnALIGN(pages,1<<cma->bitmap_shift)>>cma->bitmap_shift;+}++staticvoidclear_cma_bitmap(structcma*cma,unsignedlongpfn,intcount)+{+unsignedlongbitmapno,nr_bits;++bitmapno=(pfn-cma->base_pfn)>>cma->bitmap_shift;+nr_bits=cma_bitmap_pages_to_bits(cma,count);++mutex_lock(&cma->lock);+bitmap_clear(cma->bitmap,bitmapno,nr_bits);+mutex_unlock(&cma->lock);+}++staticint__initcma_activate_area(structcma*cma)+{+intmax_bitmapno=cma_bitmap_max_no(cma);+intbitmap_size=BITS_TO_LONGS(max_bitmapno)*sizeof(long);+unsignedlongbase_pfn=cma->base_pfn,pfn=base_pfn;+unsignedi=cma->count>>pageblock_order;+structzone*zone;++pr_debug("%s()\n",__func__);+if(!cma->count)+return0;++cma->bitmap=kzalloc(bitmap_size,GFP_KERNEL);+if(!cma->bitmap)+return-ENOMEM;++WARN_ON_ONCE(!pfn_valid(pfn));+zone=page_zone(pfn_to_page(pfn));++do{+unsignedj;++base_pfn=pfn;+for(j=pageblock_nr_pages;j;--j,pfn++){+WARN_ON_ONCE(!pfn_valid(pfn));+/*+*alloc_contig_rangerequiresthepfnrange+*specifiedtobeinthesamezone.Makethis+*simplebyforcingtheentireCMAresvrange+*tobeinthesamezone.+*/+if(page_zone(pfn_to_page(pfn))!=zone)+gotoerr;+}+init_cma_reserved_pageblock(pfn_to_page(base_pfn));+}while(--i);++mutex_init(&cma->lock);+return0;++err:+kfree(cma->bitmap);+return-EINVAL;+}++staticint__initcma_init_reserved_areas(void)+{+inti;++for(i=0;i<cma_area_count;i++){+intret=cma_activate_area(&cma_areas[i]);++if(ret)+returnret;+}++return0;+}+core_initcall(cma_init_reserved_areas);++/**+*cma_declare_contiguous()-reservecustomcontiguousarea+*@size:Sizeofthereservedarea(inbytes),+*@base:Baseaddressofthereservedareaoptional,use0forany+*@limit:Endaddressofthereservedmemory(optional,0forany).+*@bitmap_shift:Orderofpagesrepresentedbyonebitonbitmap.+*@fixed:hintaboutwheretoplacethereservedarea+*@res_cma:Pointertostorethecreatedcmaregion.+*+*Thisfunctionreservesmemoryfromearlyallocator.Itshouldbe+*calledbyarchspecificcodeoncetheearlyallocator(memblockorbootmem)+*hasbeenactivatedandallothersubsystemshavealreadyallocated/reserved+*memory.Thisfunctionallowstocreatecustomreservedareas.+*+*If@fixedistrue,reservecontiguousareaatexactly@base.Iffalse,+*reserveinrangefrom@baseto@limit.+*/+int__initcma_declare_contiguous(phys_addr_tsize,phys_addr_tbase,+phys_addr_tlimit,phys_addr_talignment,+unsignedlongbitmap_shift,boolfixed,+structcma**res_cma)+{+structcma*cma=&cma_areas[cma_area_count];+intret=0;++pr_debug("%s(size %lx, base %08lx, limit %08lx, alignment %08lx)\n",+__func__,(unsignedlong)size,(unsignedlong)base,+(unsignedlong)limit,(unsignedlong)alignment);++/* Sanity checks */+if(cma_area_count==ARRAY_SIZE(cma_areas)){+pr_err("Not enough slots for CMA reserved regions!\n");+return-ENOSPC;+}++if(!size)+return-EINVAL;++/*+*Sanitiseinputarguments.+*CMAareashouldbeatleastMAX_ORDER-1aligned.Otherwise,+*CMAareacouldbemergedintootherMIGRATE_TYPEbybuddymechanism+*andCMApropertywillbebroken.+*/+alignment>>=PAGE_SHIFT;+alignment=PAGE_SIZE<<max3(MAX_ORDER-1,pageblock_order,+(int)alignment);+base=ALIGN(base,alignment);+size=ALIGN(size,alignment);+limit&=~(alignment-1);+/* size should be aligned with bitmap_shift */+BUG_ON(!IS_ALIGNED(size>>PAGE_SHIFT,1<<cma->bitmap_shift));++/* Reserve memory */+if(base&&fixed){+if(memblock_is_region_reserved(base,size)||+memblock_reserve(base,size)<0){+ret=-EBUSY;+gotoerr;+}+}else{+phys_addr_taddr=memblock_alloc_range(size,alignment,base,+limit);+if(!addr){+ret=-ENOMEM;+gotoerr;+}else{+base=addr;+}+}++/*+*Eachreservedareamustbeinitialisedlater,whenmorekernel+*subsystems(likeslaballocator)areavailable.+*/+cma->base_pfn=PFN_DOWN(base);+cma->count=size>>PAGE_SHIFT;+cma->bitmap_shift=bitmap_shift;+*res_cma=cma;+cma_area_count++;++pr_info("CMA: reserved %ld MiB at %08lx\n",(unsignedlong)size/SZ_1M,+(unsignedlong)base);++return0;++err:+pr_err("CMA: failed to reserve %ld MiB\n",(unsignedlong)size/SZ_1M);+returnret;+}++/**+*cma_alloc()-allocatepagesfromcontiguousarea+*@cma:Contiguousmemoryregionforwhichtheallocationisperformed.+*@count:Requestednumberofpages.+*@align:Requestedalignmentofpages(inPAGE_SIZEorder).+*+*Thisfunctionallocatespartofcontiguousmemoryonspecific+*contiguousmemoryarea.+*/+structpage*cma_alloc(structcma*cma,unsignedlongcount,+unsignedlongalign)+{+unsignedlongmask,pfn,start=0;+unsignedlongmax_bitmapno,bitmapno,nr_bits;+structpage*page=NULL;+intret;++if(!cma||!cma->count)+returnNULL;++pr_debug("%s(cma %p, count %ld, align %ld)\n",__func__,(void*)cma,+count,align);++if(!count)+returnNULL;++mask=cma_bitmap_mask(cma,align);+max_bitmapno=cma_bitmap_max_no(cma);+nr_bits=cma_bitmap_pages_to_bits(cma,count);++for(;;){+mutex_lock(&cma->lock);+bitmapno=bitmap_find_next_zero_area(cma->bitmap,+max_bitmapno,start,nr_bits,mask);+if(bitmapno>=max_bitmapno){+mutex_unlock(&cma->lock);+break;+}+bitmap_set(cma->bitmap,bitmapno,nr_bits);+/*+*It'ssafetodropthelockhere.We'vemarkedthisregionfor+*ourexclusiveuse.Ifthemigrationfailswewilltakethe+*lockagainandunmarkit.+*/+mutex_unlock(&cma->lock);++pfn=cma->base_pfn+(bitmapno<<cma->bitmap_shift);+mutex_lock(&cma_mutex);+ret=alloc_contig_range(pfn,pfn+count,MIGRATE_CMA);+mutex_unlock(&cma_mutex);+if(ret==0){+page=pfn_to_page(pfn);+break;+}+clear_cma_bitmap(cma,pfn,count);+if(ret!=-EBUSY)+break;++pr_debug("%s(): memory range at %p is busy, retrying\n",+__func__,pfn_to_page(pfn));+/* try again with a bit different memory target */+start=bitmapno+mask+1;+}++pr_debug("%s(): returned %p\n",__func__,page);+returnpage;+}++/**+*cma_release()-releaseallocatedpages+*@cma:Contiguousmemoryregionforwhichtheallocationisperformed.+*@pages:Allocatedpages.+*@count:Numberofallocatedpages.+*+*Thisfunctionreleasesmemoryallocatedbyalloc_cma().+*Itreturnsfalsewhenprovidedpagesdonotbelongtocontiguousareaand+*trueotherwise.+*/+boolcma_release(structcma*cma,structpage*pages,unsignedlongcount)+{+unsignedlongpfn;++if(!cma||!pages)+returnfalse;++pr_debug("%s(page %p)\n",__func__,(void*)pages);++pfn=page_to_pfn(pages);+if(pfn<cma->base_pfn||pfn>=cma->base_pfn+cma->count)+returnfalse;++VM_BUG_ON(pfn+count>cma->base_pfn+cma->count);++free_contig_range(pfn,count);+clear_cma_bitmap(cma,pfn,count);++returntrue;+}
From: Michal Nazarewicz <hidden> Date: 2014-06-03 06:56:12
On Tue, Jun 03 2014, Joonsoo Kim wrote:
Currently, there are two users on CMA functionality, one is the DMA
subsystem and the other is the kvm on powerpc. They have their own code
to manage CMA reserved area even if they looks really similar.
From my guess, it is caused by some needs on bitmap management. Kvm side
wants to maintain bitmap not for 1 page, but for more size. Eventually it
use bitmap where one bit represents 64 pages.
When I implement CMA related patches, I should change those two places
to apply my change and it seem to be painful to me. I want to change
this situation and reduce future code management overhead through
this patch.
This change could also help developer who want to use CMA in their
new feature development, since they can use CMA easily without
copying & pasting this reserved area management code.
Signed-off-by: Joonsoo Kim <redacted>
Some small comments below, but in general
Acked-by: Michal Nazarewicz <redacted>
+static int __init cma_activate_area(struct cma *cma)
+{
+ int max_bitmapno =3D cma_bitmap_max_no(cma);
+ int bitmap_size =3D BITS_TO_LONGS(max_bitmapno) * sizeof(long);
+ unsigned long base_pfn =3D cma->base_pfn, pfn =3D base_pfn;
+ unsigned i =3D cma->count >> pageblock_order;
+ struct zone *zone;
+
+ pr_debug("%s()\n", __func__);
+ if (!cma->count)
+ return 0;
Alternatively:
+ if (!i)
+ return 0;
+
+ cma->bitmap =3D kzalloc(bitmap_size, GFP_KERNEL);
+ if (!cma->bitmap)
+ return -ENOMEM;
+
+ WARN_ON_ONCE(!pfn_valid(pfn));
+ zone =3D page_zone(pfn_to_page(pfn));
+
+ do {
+ unsigned j;
+
+ base_pfn =3D pfn;
+ for (j =3D pageblock_nr_pages; j; --j, pfn++) {
+ WARN_ON_ONCE(!pfn_valid(pfn));
+ /*
+ * alloc_contig_range requires the pfn range
+ * specified to be in the same zone. Make this
+ * simple by forcing the entire CMA resv range
+ * to be in the same zone.
+ */
+ if (page_zone(pfn_to_page(pfn)) !=3D zone)
+ goto err;
+ }
+ init_cma_reserved_pageblock(pfn_to_page(base_pfn));
+ } while (--i);
+
+ mutex_init(&cma->lock);
+ return 0;
+
+err:
+ kfree(cma->bitmap);
+ return -EINVAL;
+}
+static int __init cma_init_reserved_areas(void)
+{
+ int i;
+
+ for (i =3D 0; i < cma_area_count; i++) {
+ int ret =3D cma_activate_area(&cma_areas[i]);
+
+ if (ret)
+ return ret;
+ }
+
+ return 0;
+}
Or even:
static int __init cma_init_reserved_areas(void)
{
int i, ret =3D 0;
for (i =3D 0; !ret && i < cma_area_count; ++i)
ret =3D cma_activate_area(&cma_areas[i]);
return ret;
}
Perhaps it would make sense to move this initialisation to the far end
of this function?
+ int ret =3D 0;
+
+ pr_debug("%s(size %lx, base %08lx, limit %08lx, alignment %08lx)\n",
+ __func__, (unsigned long)size, (unsigned long)base,
+ (unsigned long)limit, (unsigned long)alignment);
+
+ /* Sanity checks */
+ if (cma_area_count =3D=3D ARRAY_SIZE(cma_areas)) {
+ pr_err("Not enough slots for CMA reserved regions!\n");
+ return -ENOSPC;
+ }
+
+ if (!size)
+ return -EINVAL;
+
+ /*
+ * Sanitise input arguments.
+ * CMA area should be at least MAX_ORDER - 1 aligned. Otherwise,
+ * CMA area could be merged into other MIGRATE_TYPE by buddy mechanism
+ * and CMA property will be broken.
+ */
+ alignment >>=3D PAGE_SHIFT;
+ alignment =3D PAGE_SIZE << max3(MAX_ORDER - 1, pageblock_order,
+ (int)alignment);
+ base =3D ALIGN(base, alignment);
+ size =3D ALIGN(size, alignment);
+ limit &=3D ~(alignment - 1);
+ /* size should be aligned with bitmap_shift */
+ BUG_ON(!IS_ALIGNED(size >> PAGE_SHIFT, 1 << cma->bitmap_shift));
cma->bitmap_shift is not yet initialised thus the above line should be:
BUG_ON(!IS_ALIGNED(size >> PAGE_SHIFT, 1 << bitmap_shift));
+
+ /* Reserve memory */
+ if (base && fixed) {
+ if (memblock_is_region_reserved(base, size) ||
+ memblock_reserve(base, size) < 0) {
+ ret =3D -EBUSY;
+ goto err;
+ }
+ } else {
+ phys_addr_t addr =3D memblock_alloc_range(size, alignment, base,
+ limit);
+ if (!addr) {
+ ret =3D -ENOMEM;
+ goto err;
+ } else {
+ base =3D addr;
+ }
+ }
+
+ /*
+ * Each reserved area must be initialised later, when more kernel
+ * subsystems (like slab allocator) are available.
+ */
+ cma->base_pfn =3D PFN_DOWN(base);
+ cma->count =3D size >> PAGE_SHIFT;
+ cma->bitmap_shift =3D bitmap_shift;
+ *res_cma =3D cma;
+ cma_area_count++;
+
+ pr_info("CMA: reserved %ld MiB at %08lx\n", (unsigned long)size / SZ_1M,
+ (unsigned long)base);
Doesn't this message end up being: =E2=80=9Ccma: CMA: reserved =E2=80=A6=E2=
=80=9D? pr_fmt adds
=E2=80=9Ccma:=E2=80=9D at the beginning, doesn't it? So we should probably=
drop =E2=80=9CCMA:=E2=80=9D
here.
From: Michal Nazarewicz <hidden> Date: 2014-06-03 07:01:00
On Tue, Jun 03 2014, Joonsoo Kim wrote:
Now, we have general CMA reserved area management framework,
so use it for future maintainabilty. There is no functional change.
Signed-off-by: Joonsoo Kim <redacted>
From: Michal Nazarewicz <hidden> Date: 2014-06-03 07:02:27
On Tue, Jun 03 2014, Joonsoo Kim wrote:
Now, we have general CMA reserved area management framework,
so use it for future maintainabilty. There is no functional change.
Signed-off-by: Joonsoo Kim <redacted>
Acked-by: Michal Nazarewicz <redacted>
--=20
Best regards, _ _
.o. | Liege of Serenely Enlightened Majesty of o' \,=3D./ `o
..o | Computer Science, Micha=C5=82 =E2=80=9Cmina86=E2=80=9D Nazarewicz =
(o o)
ooo +--[off-list ref]--<xmpp:mina86@jabber.org>--ooO--(_)--Ooo--
From: Paolo Bonzini <pbonzini@redhat.com> Date: 2014-06-03 09:21:40
Il 03/06/2014 09:02, Michal Nazarewicz ha scritto:
On Tue, Jun 03 2014, Joonsoo Kim wrote:
quoted
Now, we have general CMA reserved area management framework,
so use it for future maintainabilty. There is no functional change.
Signed-off-by: Joonsoo Kim <redacted>
Acked-by: Michal Nazarewicz <redacted>
Acked-by: Paolo Bonzini <pbonzini@redhat.com>
Aneesh, can you test this series?
Paolo
Il 03/06/2014 09:02, Michal Nazarewicz ha scritto:
quoted
On Tue, Jun 03 2014, Joonsoo Kim wrote:
quoted
Now, we have general CMA reserved area management framework,
so use it for future maintainabilty. There is no functional change.
Signed-off-by: Joonsoo Kim <redacted>
Acked-by: Michal Nazarewicz <redacted>
Acked-by: Paolo Bonzini <pbonzini@redhat.com>
Aneesh, can you test this series?
Sorry for the late reply. I will test this and update here.
-aneesh
Currently, there are two users on CMA functionality, one is the DMA
subsystem and the other is the kvm on powerpc. They have their own code
to manage CMA reserved area even if they looks really similar.
From my guess, it is caused by some needs on bitmap management. Kvm side
wants to maintain bitmap not for 1 page, but for more size. Eventually it
use bitmap where one bit represents 64 pages.
When I implement CMA related patches, I should change those two places
to apply my change and it seem to be painful to me. I want to change
this situation and reduce future code management overhead through
this patch.
This change could also help developer who want to use CMA in their
new feature development, since they can use CMA easily without
copying & pasting this reserved area management code.
Signed-off-by: Joonsoo Kim <redacted>
The way patches are split makes it difficult to review. Will it be
possible to make changes against one implementation and them move that
to generic code. That helps in finding out what exactly changed.
I guess this is added to accommodate the kvm specific alloc chunks. May
be you should do this as a patch against kvm implementation and then
move the code to generic ?
From: Joonsoo Kim <hidden> Date: 2014-06-10 02:38:13
On Tue, Jun 03, 2014 at 08:56:00AM +0200, Michal Nazarewicz wrote:
On Tue, Jun 03 2014, Joonsoo Kim wrote:
quoted
Currently, there are two users on CMA functionality, one is the DMA
subsystem and the other is the kvm on powerpc. They have their own code
to manage CMA reserved area even if they looks really similar.
From my guess, it is caused by some needs on bitmap management. Kvm side
wants to maintain bitmap not for 1 page, but for more size. Eventually it
use bitmap where one bit represents 64 pages.
When I implement CMA related patches, I should change those two places
to apply my change and it seem to be painful to me. I want to change
this situation and reduce future code management overhead through
this patch.
This change could also help developer who want to use CMA in their
new feature development, since they can use CMA easily without
copying & pasting this reserved area management code.
Signed-off-by: Joonsoo Kim <redacted>
Some small comments below, but in general
Acked-by: Michal Nazarewicz <redacted>
+static int __init cma_activate_area(struct cma *cma)
+{
+ int max_bitmapno = cma_bitmap_max_no(cma);
+ int bitmap_size = BITS_TO_LONGS(max_bitmapno) * sizeof(long);
+ unsigned long base_pfn = cma->base_pfn, pfn = base_pfn;
+ unsigned i = cma->count >> pageblock_order;
+ struct zone *zone;
+
+ pr_debug("%s()\n", __func__);
+ if (!cma->count)
+ return 0;
Alternatively:
+ if (!i)
+ return 0;
I prefer cma->count than i, since it represents what it does itself.
quoted
+
+ cma->bitmap = kzalloc(bitmap_size, GFP_KERNEL);
+ if (!cma->bitmap)
+ return -ENOMEM;
+
+ WARN_ON_ONCE(!pfn_valid(pfn));
+ zone = page_zone(pfn_to_page(pfn));
+
+ do {
+ unsigned j;
+
+ base_pfn = pfn;
+ for (j = pageblock_nr_pages; j; --j, pfn++) {
+ WARN_ON_ONCE(!pfn_valid(pfn));
+ /*
+ * alloc_contig_range requires the pfn range
+ * specified to be in the same zone. Make this
+ * simple by forcing the entire CMA resv range
+ * to be in the same zone.
+ */
+ if (page_zone(pfn_to_page(pfn)) != zone)
+ goto err;
+ }
+ init_cma_reserved_pageblock(pfn_to_page(base_pfn));
+ } while (--i);
+
+ mutex_init(&cma->lock);
+ return 0;
+
+err:
+ kfree(cma->bitmap);
+ return -EINVAL;
+}
quoted
+static int __init cma_init_reserved_areas(void)
+{
+ int i;
+
+ for (i = 0; i < cma_area_count; i++) {
+ int ret = cma_activate_area(&cma_areas[i]);
+
+ if (ret)
+ return ret;
+ }
+
+ return 0;
+}
Or even:
static int __init cma_init_reserved_areas(void)
{
int i, ret = 0;
for (i = 0; !ret && i < cma_area_count; ++i)
ret = cma_activate_area(&cma_areas[i]);
return ret;
}
I think that originial implementation is better, since it seems
more readable to me.
From: Joonsoo Kim <hidden> Date: 2014-06-10 02:45:23
On Tue, Jun 03, 2014 at 09:00:48AM +0200, Michal Nazarewicz wrote:
On Tue, Jun 03 2014, Joonsoo Kim wrote:
quoted
Now, we have general CMA reserved area management framework,
so use it for future maintainabilty. There is no functional change.
Signed-off-by: Joonsoo Kim <redacted>
Without including device.h, build failure occurs.
In dma-contiguous.h, we try to access to dev->cma_area, so we need
device.h. In the past, we included it luckily by swap.h in
drivers/base/dma-contiguous.c. Swap.h includes node.h and then node.h
includes device.h, so we were happy. But, in this patch, I remove
'include <linux/swap.h>' so we need to include device.h explicitly.
Thanks.
From: Joonsoo Kim <hidden> Date: 2014-06-10 02:57:59
On Thu, Jun 05, 2014 at 11:09:05PM +0530, Aneesh Kumar K.V wrote:
Joonsoo Kim [off-list ref] writes:
quoted
Currently, there are two users on CMA functionality, one is the DMA
subsystem and the other is the kvm on powerpc. They have their own code
to manage CMA reserved area even if they looks really similar.
From my guess, it is caused by some needs on bitmap management. Kvm side
wants to maintain bitmap not for 1 page, but for more size. Eventually it
use bitmap where one bit represents 64 pages.
When I implement CMA related patches, I should change those two places
to apply my change and it seem to be painful to me. I want to change
this situation and reduce future code management overhead through
this patch.
This change could also help developer who want to use CMA in their
new feature development, since they can use CMA easily without
copying & pasting this reserved area management code.
Signed-off-by: Joonsoo Kim <redacted>
The way patches are split makes it difficult to review. Will it be
possible to make changes against one implementation and them move that
to generic code. That helps in finding out what exactly changed.
Hello,
You are right! I will respin this patchset as the form you
recommended.
I guess this is added to accommodate the kvm specific alloc chunks. May
be you should do this as a patch against kvm implementation and then
move the code to generic ?
Yes, this is for kvm specific alloc chunks. I will consider which one
is better for the base implementation and makes patches against it.
Thanks.
From: Michal Nazarewicz <hidden> Date: 2014-06-11 08:25:08
On Tue, Jun 10 2014, Joonsoo Kim [off-list ref] wrote:
Without including device.h, build failure occurs.
In dma-contiguous.h, we try to access to dev->cma_area, so we need
device.h. In the past, we included it luckily by swap.h in
drivers/base/dma-contiguous.c. Swap.h includes node.h and then node.h
includes device.h, so we were happy. But, in this patch, I remove
'include <linux/swap.h>' so we need to include device.h explicitly.
Ack.
--=20
Best regards, _ _
.o. | Liege of Serenely Enlightened Majesty of o' \,=3D./ `o
..o | Computer Science, Micha=C5=82 =E2=80=9Cmina86=E2=80=9D Nazarewicz =
(o o)
ooo +--[off-list ref]--<xmpp:mina86@jabber.org>--ooO--(_)--Ooo--