Re: [PATCH v2 06/10] CMA: generalize CMA reserved area management functionality
On Sat, Jun 14, 2014 at 03:46:44PM +0530, Aneesh Kumar K.V wrote:
> Joonsoo Kim writes:
>
> > 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.
> >
> > In previous patches, we have prepared some features to generalize
> > CMA reserved area management and now it's time to do it. This patch
> > moves core functions to mm/cma.c and change DMA APIs to use
> > these functions.
> >
> > There is no functional change in DMA APIs.
> >
> > v2: There is no big change from v1 in mm/cma.c. Mostly renaming.
> >
> > Acked-by: Michal Nazarewicz
> > Signed-off-by: Joonsoo Kim
> >
>
> .
>
> > +
> > + mask = cma_bitmap_aligned_mask(cma, align);
> > + bitmap_maxno = cma_bitmap_maxno(cma);
> > + nr_bits = cma_bitmap_pages_to_bits(cma, count);
> > +
> > + for (;;) {
> > + mutex_lock(&cma->lock);
> > + bitmapno = bitmap_find_next_zero_area(cma->bitmap,
> > + bitmap_maxno, start, nr_bits, mask);
> > + if (bitmapno >= bitmap_maxno) {
> > + mutex_unlock(&cma->lock);
> > + break;
> > + }
> > + bitmap_set(cma->bitmap, bitmapno, nr_bits);
> > + /*
> > +* It's safe to drop the lock here. We've marked this region for
> > +* our exclusive use. If the migration fails we will take the
> > +* lock again and unmark it.
> > +*/
> > + mutex_unlock(&cma->lock);
> > +
> > + pfn = cma->base_pfn + (bitmapno << cma->order_per_bit);
> > + 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;
> > + } else if (ret != -EBUSY) {
> > + clear_cma_bitmap(cma, pfn, count);
> > + break;
> > + }
> > +
>
>
> For setting bit map we do
> bitmap_set(cma->bitmap, bitmapno, nr_bits);
> alloc_contig()..
> if (error)
> clear_cma_bitmap(cma, pfn, count);
>
> Why ?
>
> why not bitmap_clear() ?
>
Unlike your psuedo code, for setting bitmap, we do
- grab the mutex
- bitmap_set
- release the mutex
clear_cma_bitmap() handles these things.
Thanks.
--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 06/10] CMA: generalize CMA reserved area management functionality
Joonsoo Kim writes:
> 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.
>
> In previous patches, we have prepared some features to generalize
> CMA reserved area management and now it's time to do it. This patch
> moves core functions to mm/cma.c and change DMA APIs to use
> these functions.
>
> There is no functional change in DMA APIs.
>
> v2: There is no big change from v1 in mm/cma.c. Mostly renaming.
>
> Acked-by: Michal Nazarewicz
> Signed-off-by: Joonsoo Kim
>
.
> +
> + mask = cma_bitmap_aligned_mask(cma, align);
> + bitmap_maxno = cma_bitmap_maxno(cma);
> + nr_bits = cma_bitmap_pages_to_bits(cma, count);
> +
> + for (;;) {
> + mutex_lock(&cma->lock);
> + bitmapno = bitmap_find_next_zero_area(cma->bitmap,
> + bitmap_maxno, start, nr_bits, mask);
> + if (bitmapno >= bitmap_maxno) {
> + mutex_unlock(&cma->lock);
> + break;
> + }
> + bitmap_set(cma->bitmap, bitmapno, nr_bits);
> + /*
> + * It's safe to drop the lock here. We've marked this region for
> + * our exclusive use. If the migration fails we will take the
> + * lock again and unmark it.
> + */
> + mutex_unlock(&cma->lock);
> +
> + pfn = cma->base_pfn + (bitmapno << cma->order_per_bit);
> + 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;
> + } else if (ret != -EBUSY) {
> + clear_cma_bitmap(cma, pfn, count);
> + break;
> + }
> +
For setting bit map we do
bitmap_set(cma->bitmap, bitmapno, nr_bits);
alloc_contig()..
if (error)
clear_cma_bitmap(cma, pfn, count);
Why ?
why not bitmap_clear() ?
-aneesh
--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 06/10] CMA: generalize CMA reserved area management functionality
Joonsoo Kim writes:
> 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.
>
> In previous patches, we have prepared some features to generalize
> CMA reserved area management and now it's time to do it. This patch
> moves core functions to mm/cma.c and change DMA APIs to use
> these functions.
>
> There is no functional change in DMA APIs.
>
> v2: There is no big change from v1 in mm/cma.c. Mostly renaming.
>
> Acked-by: Michal Nazarewicz
> Signed-off-by: Joonsoo Kim
Reviewed-by: Aneesh Kumar K.V
>
> diff --git a/drivers/base/Kconfig b/drivers/base/Kconfig
> index 00e13ce..4eac559 100644
> --- a/drivers/base/Kconfig
> +++ b/drivers/base/Kconfig
> @@ -283,16 +283,6 @@ config CMA_ALIGNMENT
>
> If unsure, leave the default value "8".
>
> -config CMA_AREAS
> - int "Maximum count of the CMA device-private areas"
> - default 7
> - help
> - CMA allows to create CMA areas for particular devices. This parameter
> - sets the maximum number of such device private CMA areas in the
> - system.
> -
> - If unsure, leave the default value "7".
> -
> endif
>
> endmenu
> diff --git a/drivers/base/dma-contiguous.c b/drivers/base/dma-contiguous.c
> index 9bc9340..f177f73 100644
> --- a/drivers/base/dma-contiguous.c
> +++ b/drivers/base/dma-contiguous.c
> @@ -24,25 +24,10 @@
>
> #include
> #include
> -#include
> -#include
> -#include
> #include
> -#include
> -#include
> -#include
> #include
> #include
> -
> -struct cma {
> - unsigned long base_pfn;
> - unsigned long count;
> - unsigned long *bitmap;
> - int order_per_bit; /* Order of pages represented by one bit */
> - struct mutexlock;
> -};
> -
> -struct cma *dma_contiguous_default_area;
> +#include
>
> #ifdef CONFIG_CMA_SIZE_MBYTES
> #define CMA_SIZE_MBYTES CONFIG_CMA_SIZE_MBYTES
> @@ -50,6 +35,8 @@ struct cma *dma_contiguous_default_area;
> #define CMA_SIZE_MBYTES 0
> #endif
>
> +struct cma *dma_contiguous_default_area;
> +
> /*
> * Default global CMA area size can be defined in kernel's .config.
> * This is useful mainly for distro maintainers to create a kernel
> @@ -156,199 +143,13 @@ void __init dma_contiguous_reserve(phys_addr_t limit)
> }
> }
>
> -static DEFINE_MUTEX(cma_mutex);
> -
> -static unsigned long cma_bitmap_aligned_mask(struct cma *cma, int
> align_order)
> -{
> - return (1 << (align_order >> cma->order_per_bit)) - 1;
> -}
> -
> -static unsigned long cma_bitmap_maxno(struct cma *cma)
> -{
> - return cma->count >> cma->order_per_bit;
> -}
> -
> -static unsigned long cma_bitmap_pages_to_bits(struct cma *cma,
> - unsigned long pages)
> -{
> - return ALIGN(pages, 1 << cma->order_per_bit) >> cma->order_per_bit;
> -}
> -
> -static void clear_cma_bitmap(struct cma *cma, unsigned long pfn, int count)
> -{
> - unsigned long bitmapno, nr_bits;
> -
> - bitmapno = (pfn - cma->base_pfn) >> cma->order_per_bit;
> - nr_bits = cma_bitmap_pages_to_bits(cma, count);
> -
> - mutex_lock(&cma->lock);
> - bitmap_clear(cma->bitmap, bitmapno, nr_bits);
> - mutex_unlock(&cma->lock);
> -}
> -
> -static int __init cma_activate_area(struct cma *cma)
> -{
> - int bitmap_maxno = cma_bitmap_maxno(cma);
> - int bitmap_size = BITS_TO_LONGS(bitmap_maxno) * 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__);
> -
> - 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.
> -
Re: [PATCH v2 06/10] CMA: generalize CMA reserved area management functionality
Joonsoo Kim writes:
> 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.
>
> In previous patches, we have prepared some features to generalize
> CMA reserved area management and now it's time to do it. This patch
> moves core functions to mm/cma.c and change DMA APIs to use
> these functions.
>
> There is no functional change in DMA APIs.
>
> v2: There is no big change from v1 in mm/cma.c. Mostly renaming.
>
> Acked-by: Michal Nazarewicz
> Signed-off-by: Joonsoo Kim
Reviewed-by: Aneesh Kumar K.V
>
> diff --git a/drivers/base/Kconfig b/drivers/base/Kconfig
> index 00e13ce..4eac559 100644
> --- a/drivers/base/Kconfig
> +++ b/drivers/base/Kconfig
> @@ -283,16 +283,6 @@ config CMA_ALIGNMENT
>
> If unsure, leave the default value "8".
>
> -config CMA_AREAS
> - int "Maximum count of the CMA device-private areas"
> - default 7
> - help
> - CMA allows to create CMA areas for particular devices. This parameter
> - sets the maximum number of such device private CMA areas in the
> - system.
> -
> - If unsure, leave the default value "7".
> -
> endif
>
> endmenu
> diff --git a/drivers/base/dma-contiguous.c b/drivers/base/dma-contiguous.c
> index 9bc9340..f177f73 100644
> --- a/drivers/base/dma-contiguous.c
> +++ b/drivers/base/dma-contiguous.c
> @@ -24,25 +24,10 @@
>
> #include
> #include
> -#include
> -#include
> -#include
> #include
> -#include
> -#include
> -#include
> #include
> #include
> -
> -struct cma {
> - unsigned long base_pfn;
> - unsigned long count;
> - unsigned long *bitmap;
> - int order_per_bit; /* Order of pages represented by one bit */
> - struct mutexlock;
> -};
> -
> -struct cma *dma_contiguous_default_area;
> +#include
>
> #ifdef CONFIG_CMA_SIZE_MBYTES
> #define CMA_SIZE_MBYTES CONFIG_CMA_SIZE_MBYTES
> @@ -50,6 +35,8 @@ struct cma *dma_contiguous_default_area;
> #define CMA_SIZE_MBYTES 0
> #endif
>
> +struct cma *dma_contiguous_default_area;
> +
> /*
> * Default global CMA area size can be defined in kernel's .config.
> * This is useful mainly for distro maintainers to create a kernel
> @@ -156,199 +143,13 @@ void __init dma_contiguous_reserve(phys_addr_t limit)
> }
> }
>
> -static DEFINE_MUTEX(cma_mutex);
> -
> -static unsigned long cma_bitmap_aligned_mask(struct cma *cma, int
> align_order)
> -{
> - return (1 << (align_order >> cma->order_per_bit)) - 1;
> -}
> -
> -static unsigned long cma_bitmap_maxno(struct cma *cma)
> -{
> - return cma->count >> cma->order_per_bit;
> -}
> -
> -static unsigned long cma_bitmap_pages_to_bits(struct cma *cma,
> - unsigned long pages)
> -{
> - return ALIGN(pages, 1 << cma->order_per_bit) >> cma->order_per_bit;
> -}
> -
> -static void clear_cma_bitmap(struct cma *cma, unsigned long pfn, int count)
> -{
> - unsigned long bitmapno, nr_bits;
> -
> - bitmapno = (pfn - cma->base_pfn) >> cma->order_per_bit;
> - nr_bits = cma_bitmap_pages_to_bits(cma, count);
> -
> - mutex_lock(&cma->lock);
> - bitmap_clear(cma->bitmap, bitmapno, nr_bits);
> - mutex_unlock(&cma->lock);
> -}
> -
> -static int __init cma_activate_area(struct cma *cma)
> -{
> - int bitmap_maxno = cma_bitmap_maxno(cma);
> - int bitmap_size = BITS_TO_LONGS(bitmap_maxno) * 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__);
> -
> - 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.
> -
Re: [PATCH v2 06/10] CMA: generalize CMA reserved area management functionality
On 06/12/2014 11:21 AM, 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.
>
> In previous patches, we have prepared some features to generalize
> CMA reserved area management and now it's time to do it. This patch
> moves core functions to mm/cma.c and change DMA APIs to use
> these functions.
>
> There is no functional change in DMA APIs.
>
> v2: There is no big change from v1 in mm/cma.c. Mostly renaming.
>
> Acked-by: Michal Nazarewicz
> Signed-off-by: Joonsoo Kim
Acked-by: Zhang Yanfei
>
> diff --git a/drivers/base/Kconfig b/drivers/base/Kconfig
> index 00e13ce..4eac559 100644
> --- a/drivers/base/Kconfig
> +++ b/drivers/base/Kconfig
> @@ -283,16 +283,6 @@ config CMA_ALIGNMENT
>
> If unsure, leave the default value "8".
>
> -config CMA_AREAS
> - int "Maximum count of the CMA device-private areas"
> - default 7
> - help
> - CMA allows to create CMA areas for particular devices. This parameter
> - sets the maximum number of such device private CMA areas in the
> - system.
> -
> - If unsure, leave the default value "7".
> -
> endif
>
> endmenu
> diff --git a/drivers/base/dma-contiguous.c b/drivers/base/dma-contiguous.c
> index 9bc9340..f177f73 100644
> --- a/drivers/base/dma-contiguous.c
> +++ b/drivers/base/dma-contiguous.c
> @@ -24,25 +24,10 @@
>
> #include
> #include
> -#include
> -#include
> -#include
> #include
> -#include
> -#include
> -#include
> #include
> #include
> -
> -struct cma {
> - unsigned long base_pfn;
> - unsigned long count;
> - unsigned long *bitmap;
> - int order_per_bit; /* Order of pages represented by one bit */
> - struct mutexlock;
> -};
> -
> -struct cma *dma_contiguous_default_area;
> +#include
>
> #ifdef CONFIG_CMA_SIZE_MBYTES
> #define CMA_SIZE_MBYTES CONFIG_CMA_SIZE_MBYTES
> @@ -50,6 +35,8 @@ struct cma *dma_contiguous_default_area;
> #define CMA_SIZE_MBYTES 0
> #endif
>
> +struct cma *dma_contiguous_default_area;
> +
> /*
> * Default global CMA area size can be defined in kernel's .config.
> * This is useful mainly for distro maintainers to create a kernel
> @@ -156,199 +143,13 @@ void __init dma_contiguous_reserve(phys_addr_t limit)
> }
> }
>
> -static DEFINE_MUTEX(cma_mutex);
> -
> -static unsigned long cma_bitmap_aligned_mask(struct cma *cma, int
> align_order)
> -{
> - return (1 << (align_order >> cma->order_per_bit)) - 1;
> -}
> -
> -static unsigned long cma_bitmap_maxno(struct cma *cma)
> -{
> - return cma->count >> cma->order_per_bit;
> -}
> -
> -static unsigned long cma_bitmap_pages_to_bits(struct cma *cma,
> - unsigned long pages)
> -{
> - return ALIGN(pages, 1 << cma->order_per_bit) >> cma->order_per_bit;
> -}
> -
> -static void clear_cma_bitmap(struct cma *cma, unsigned long pfn, int count)
> -{
> - unsigned long bitmapno, nr_bits;
> -
> - bitmapno = (pfn - cma->base_pfn) >> cma->order_per_bit;
> - nr_bits = cma_bitmap_pages_to_bits(cma, count);
> -
> - mutex_lock(&cma->lock);
> - bitmap_clear(cma->bitmap, bitmapno, nr_bits);
> - mutex_unlock(&cma->lock);
> -}
> -
> -static int __init cma_activate_area(struct cma *cma)
> -{
> - int bitmap_maxno = cma_bitmap_maxno(cma);
> - int bitmap_size = BITS_TO_LONGS(bitmap_maxno) * 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__);
> -
> - 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
> -
Re: [PATCH v2 06/10] CMA: generalize CMA reserved area management functionality
On Thu, Jun 12, 2014 at 04:13:11PM +0900, Minchan Kim wrote: > On Thu, Jun 12, 2014 at 12:21:43PM +0900, 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. > > > > In previous patches, we have prepared some features to generalize > > CMA reserved area management and now it's time to do it. This patch > > moves core functions to mm/cma.c and change DMA APIs to use > > these functions. > > > > There is no functional change in DMA APIs. > > > > v2: There is no big change from v1 in mm/cma.c. Mostly renaming. > > > > Acked-by: Michal Nazarewicz > > Signed-off-by: Joonsoo Kim > > Acutally, I want to remove bool return of cma_release but it's not > a out of scope in this patchset. > > Acked-by: Minchan Kim > > > > > diff --git a/drivers/base/Kconfig b/drivers/base/Kconfig > > index 00e13ce..4eac559 100644 > > --- a/drivers/base/Kconfig > > +++ b/drivers/base/Kconfig > > @@ -283,16 +283,6 @@ config CMA_ALIGNMENT > > > > If unsure, leave the default value "8". > > > > -config CMA_AREAS > > - int "Maximum count of the CMA device-private areas" > > - default 7 > > - help > > - CMA allows to create CMA areas for particular devices. This parameter > > - sets the maximum number of such device private CMA areas in the > > - system. > > - > > - If unsure, leave the default value "7". > > - > > endif > > > > endmenu > > diff --git a/drivers/base/dma-contiguous.c b/drivers/base/dma-contiguous.c > > index 9bc9340..f177f73 100644 > > --- a/drivers/base/dma-contiguous.c > > +++ b/drivers/base/dma-contiguous.c > > @@ -24,25 +24,10 @@ > > > > #include > > #include > > -#include > > -#include > > -#include > > #include > > -#include > > -#include > > -#include > > #include > > #include > > Should we remain log2.h in here? > We should remove it. I will fix it. Thanks. -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 06/10] CMA: generalize CMA reserved area management functionality
On Thu, Jun 12, 2014 at 12:21:43PM +0900, 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.
>
> In previous patches, we have prepared some features to generalize
> CMA reserved area management and now it's time to do it. This patch
> moves core functions to mm/cma.c and change DMA APIs to use
> these functions.
>
> There is no functional change in DMA APIs.
>
> v2: There is no big change from v1 in mm/cma.c. Mostly renaming.
>
> Acked-by: Michal Nazarewicz
> Signed-off-by: Joonsoo Kim
Acutally, I want to remove bool return of cma_release but it's not
a out of scope in this patchset.
Acked-by: Minchan Kim
>
> diff --git a/drivers/base/Kconfig b/drivers/base/Kconfig
> index 00e13ce..4eac559 100644
> --- a/drivers/base/Kconfig
> +++ b/drivers/base/Kconfig
> @@ -283,16 +283,6 @@ config CMA_ALIGNMENT
>
> If unsure, leave the default value "8".
>
> -config CMA_AREAS
> - int "Maximum count of the CMA device-private areas"
> - default 7
> - help
> - CMA allows to create CMA areas for particular devices. This parameter
> - sets the maximum number of such device private CMA areas in the
> - system.
> -
> - If unsure, leave the default value "7".
> -
> endif
>
> endmenu
> diff --git a/drivers/base/dma-contiguous.c b/drivers/base/dma-contiguous.c
> index 9bc9340..f177f73 100644
> --- a/drivers/base/dma-contiguous.c
> +++ b/drivers/base/dma-contiguous.c
> @@ -24,25 +24,10 @@
>
> #include
> #include
> -#include
> -#include
> -#include
> #include
> -#include
> -#include
> -#include
> #include
> #include
Should we remain log2.h in here?
> -
> -struct cma {
> - unsigned long base_pfn;
> - unsigned long count;
> - unsigned long *bitmap;
> - int order_per_bit; /* Order of pages represented by one bit */
> - struct mutexlock;
> -};
> -
> -struct cma *dma_contiguous_default_area;
> +#include
>
> #ifdef CONFIG_CMA_SIZE_MBYTES
> #define CMA_SIZE_MBYTES CONFIG_CMA_SIZE_MBYTES
> @@ -50,6 +35,8 @@ struct cma *dma_contiguous_default_area;
> #define CMA_SIZE_MBYTES 0
> #endif
>
> +struct cma *dma_contiguous_default_area;
> +
> /*
> * Default global CMA area size can be defined in kernel's .config.
> * This is useful mainly for distro maintainers to create a kernel
> @@ -156,199 +143,13 @@ void __init dma_contiguous_reserve(phys_addr_t limit)
> }
> }
>
> -static DEFINE_MUTEX(cma_mutex);
> -
> -static unsigned long cma_bitmap_aligned_mask(struct cma *cma, int
> align_order)
> -{
> - return (1 << (align_order >> cma->order_per_bit)) - 1;
> -}
> -
> -static unsigned long cma_bitmap_maxno(struct cma *cma)
> -{
> - return cma->count >> cma->order_per_bit;
> -}
> -
> -static unsigned long cma_bitmap_pages_to_bits(struct cma *cma,
> - unsigned long pages)
> -{
> - return ALIGN(pages, 1 << cma->order_per_bit) >> cma->order_per_bit;
> -}
> -
> -static void clear_cma_bitmap(struct cma *cma, unsigned long pfn, int count)
> -{
> - unsigned long bitmapno, nr_bits;
> -
> - bitmapno = (pfn - cma->base_pfn) >> cma->order_per_bit;
> - nr_bits = cma_bitmap_pages_to_bits(cma, count);
> -
> - mutex_lock(&cma->lock);
> - bitmap_clear(cma->bitmap, bitmapno, nr_bits);
> - mutex_unlock(&cma->lock);
> -}
> -
> -static int __init cma_activate_area(struct cma *cma)
> -{
> - int bitmap_maxno = cma_bitmap_maxno(cma);
> - int bitmap_size = BITS_TO_LONGS(bitmap_maxno) * 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__);
> -
> - 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
> -
