On Thu, Jun 05, 2014 at 11:09:05PM +0530, Aneesh Kumar K.V wrote:
> Joonsoo Kim <iamjoonsoo.kim@xxxxxxx> 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.
> >
> > Signed-off-by: Joonsoo Kim <iamjoonsoo.kim@xxxxxxx>
>
>
> 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.
> >
> > diff --git a/drivers/base/Kconfig b/drivers/base/Kconfig
> > index 00e13ce..b3fe1cc 100644
> > --- a/drivers/base/Kconfig
> > +++ b/drivers/base/Kconfig
> > @@ -283,7 +283,7 @@ config CMA_ALIGNMENT
> >
> > If unsure, leave the default value "8".
> >
> > -config CMA_AREAS
> > +config DMA_CMA_AREAS
> > int "Maximum count of the CMA device-private areas"
> > default 7
> > help
>
> for ex: The above can be a seperate patch along with further DMA related
> cleanups .
Okay.
>
> > diff --git a/drivers/base/dma-contiguous.c b/drivers/base/dma-contiguous.c
> > index 83969f8..48cdac8 100644
> > --- a/drivers/base/dma-contiguous.c
> > +++ b/drivers/base/dma-contiguous.c
> > @@ -186,7 +186,7 @@ static int __init cma_activate_area(struct cma *cma)
> > return 0;
> > }
> >
> > -static struct cma cma_areas[MAX_CMA_AREAS];
> > +static struct cma cma_areas[MAX_DMA_CMA_AREAS];
> > static unsigned cma_area_count;
> >
> > static int __init cma_init_reserved_areas(void)
> > diff --git a/include/linux/cma.h b/include/linux/cma.h
> > new file mode 100644
> > index 0000000..60ba06f
> > --- /dev/null
> > +++ b/include/linux/cma.h
> > @@ -0,0 +1,28 @@
> > +/*
> > + * Contiguous Memory Allocator
> > + *
> > + * Copyright LG Electronics Inc., 2014
> > + * Written by:
> > + * Joonsoo Kim <iamjoonsoo.kim@xxxxxxx>
> > + *
> > + * 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 __CMA_H__
> > +#define __CMA_H__
> > +
> > +struct cma;
> > +
> > +extern struct page *cma_alloc(struct cma *cma, unsigned long count,
> > + unsigned long align);
> > +extern bool cma_release(struct cma *cma, struct page *pages,
> > + unsigned long count);
> > +extern int __init cma_declare_contiguous(phys_addr_t size, phys_addr_t base,
> > + phys_addr_t limit, phys_addr_t alignment,
> > + unsigned long bitmap_shift, bool fixed,
> > + struct cma **res_cma);
> > +#endif
> > diff --git a/include/linux/dma-contiguous.h b/include/linux/dma-contiguous.h
> > index 772eab5..dfb1dc9 100644
> > --- a/include/linux/dma-contiguous.h
> > +++ b/include/linux/dma-contiguous.h
> > @@ -63,7 +63,7 @@ struct device;
> > * There is always at least global CMA area and a few optional device
> > * private areas configured in kernel .config.
> > */
> > -#define MAX_CMA_AREAS (1 + CONFIG_CMA_AREAS)
> > +#define MAX_DMA_CMA_AREAS (1 + CONFIG_DMA_CMA_AREAS)
> >
> > extern struct cma *dma_contiguous_default_area;
> >
> > @@ -123,7 +123,7 @@ bool dma_release_from_contiguous(struct device *dev, struct page *pages,
> >
> > #else
> >
> > -#define MAX_CMA_AREAS (0)
> > +#define MAX_DMA_CMA_AREAS (0)
> >
> > static inline struct cma *dev_get_cma_area(struct device *dev)
> > {
> > diff --git a/mm/Kconfig b/mm/Kconfig
> > index 7511b4a..0877ddc 100644
> > --- a/mm/Kconfig
> > +++ b/mm/Kconfig
> > @@ -515,6 +515,17 @@ config CMA_DEBUG
> > processing calls such as dma_alloc_from_contiguous().
> > This option does not affect warning and error messages.
> >
> > +config CMA_AREAS
> > + int "Maximum count of the CMA areas"
> > + depends on CMA
> > + default 7
> > + help
> > + CMA allows to create CMA areas for particular purpose, mainly,
> > + used as device private area. This parameter sets the maximum
> > + number of CMA area in the system.
> > +
> > + If unsure, leave the default value "7".
> > +
>
> If we have 7 declare continugous request, a kvm cma allocation request will fail ?
Yes. If you need more, you can increase it. :)
> > config ZBUD
> > tristate
> > default n
> > diff --git a/mm/Makefile b/mm/Makefile
> > index 1eaa70b..bc0422b 100644
> > --- a/mm/Makefile
> > +++ b/mm/Makefile
> > @@ -62,3 +62,4 @@ obj-$(CONFIG_MEMORY_ISOLATION) += page_isolation.o
> > obj-$(CONFIG_ZBUD) += zbud.o
> > obj-$(CONFIG_ZSMALLOC) += zsmalloc.o
> > obj-$(CONFIG_GENERIC_EARLY_IOREMAP) += early_ioremap.o
> > +obj-$(CONFIG_CMA) += cma.o
> > diff --git a/mm/cma.c b/mm/cma.c
> > new file mode 100644
> > index 0000000..0dae88d
> > --- /dev/null
> > +++ b/mm/cma.c
> > @@ -0,0 +1,329 @@
> > +/*
> > + * Contiguous Memory Allocator
> > + *
> > + * Copyright (c) 2010-2011 by Samsung Electronics.
> > + * Copyright IBM Corporation, 2013
> > + * Copyright LG Electronics Inc., 2014
> > + * Written by:
> > + * Marek Szyprowski <m.szyprowski@xxxxxxxxxxx>
> > + * Michal Nazarewicz <mina86@xxxxxxxxxx>
> > + * Aneesh Kumar K.V <aneesh.kumar@xxxxxxxxxxxxxxxxxx>
> > + * Joonsoo Kim <iamjoonsoo.kim@xxxxxxx>
> > + *
> > + * 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.
> > + */
> > +
> > +#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>
> > +
> > +struct cma {
> > + unsigned long base_pfn;
> > + unsigned long count;
> > + unsigned long *bitmap;
> > + unsigned long bitmap_shift;
>
> 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.
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@xxxxxxxxx. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@xxxxxxxxx";> email@xxxxxxxxx </a>