Re: [PATCH v6 1/3] genalloc:support memory-allocation with bytes-alignment to genalloc

2015-08-25 Thread Scott Wood
On Tue, 2015-08-25 at 03:09 -0500, Zhao Qiang-B45475 wrote:
> On 08/25/2015 12:01 PM, Laura Abbott wrote:
> 
> > -Original Message-
> > From: Laura Abbott [mailto:labb...@redhat.com]
> > Sent: Tuesday, August 25, 2015 12:01 PM
> > To: Zhao Qiang-B45475; Wood Scott-B07421
> > Cc: linux-kernel@vger.kernel.org; linuxppc-...@lists.ozlabs.org;
> > lau...@codeaurora.org; Xie Xiaobo-R63061; b...@kernel.crashing.org; Li
> > Yang-Leo-R58472; pau...@samba.org
> > Subject: Re: [PATCH v6 1/3] genalloc:support memory-allocation with
> > bytes-alignment to genalloc
> > 
> > On 08/24/2015 07:40 PM, Zhao Qiang wrote:
> > > On 08/25/2015 07:11 AM, Laura Abbott wrote:
> > > > -Original Message-
> > > > From: Laura Abbott [mailto:labb...@redhat.com]
> > > > Sent: Tuesday, August 25, 2015 7:11 AM
> > > > To: Zhao Qiang-B45475; Wood Scott-B07421
> > > > Cc: linux-kernel@vger.kernel.org; linuxppc-...@lists.ozlabs.org;
> > > > lau...@codeaurora.org; Xie Xiaobo-R63061; b...@kernel.crashing.org;
> > > > Li Yang-Leo-R58472; pau...@samba.org
> > > > Subject: Re: [PATCH v6 1/3] genalloc:support memory-allocation with
> > > > bytes-alignment to genalloc
> > > > 
> > > > On 08/24/2015 02:31 AM, Zhao Qiang wrote:
> > > > > Bytes alignment is required to manage some special RAM, so add
> > > > > gen_pool_first_fit_align to genalloc, meanwhile add
> > > > > gen_pool_alloc_data to pass data to gen_pool_first_fit_align(modify
> > > > > gen_pool_alloc as a wrapper)
> > > > > 
> > > > > Signed-off-by: Zhao Qiang 
> > > > > ---
> > > > > Changes for v6:
> > > > >   - patches set v6 include a new patch because of using
> > > > >   - genalloc to manage QE MURAM, patch 0001 is the new
> > > > >   - patch, adding bytes alignment for allocation for use.
> > > > > 
> > > > >include/linux/genalloc.h | 23 +++
> > > > >lib/genalloc.c   | 58
> > > > +++-
> > > > >2 files changed, 72 insertions(+), 9 deletions(-)
> > > > > 
> > > > > diff --git a/include/linux/genalloc.h b/include/linux/genalloc.h
> > > > > index 1ccaab4..55da07e 100644
> > > > > --- a/include/linux/genalloc.h
> > > > > +++ b/include/linux/genalloc.h
> > > > > @@ -34,6 +34,7 @@
> > > > > 
> > > > >struct device;
> > > > >struct device_node;
> > > > > +struct gen_pool;
> > > > > 
> > > > >/**
> > > > > * Allocation callback function type definition @@ -47,7 +48,7 @@
> > > > > typedef unsigned long (*genpool_algo_t)(unsigned long *map,
> > > > >   unsigned long size,
> > > > >   unsigned long start,
> > > > >   unsigned int nr,
> > > > > - void *data);
> > > > > + void *data, struct gen_pool *pool);
> > > > > 
> > > > >/*
> > > > > *  General purpose special memory pool descriptor.
> > > > > @@ -73,6 +74,13 @@ struct gen_pool_chunk {
> > > > >   unsigned long bits[0];  /* bitmap for allocating memory
> > chunk
> > > > */
> > > > >};
> > > > > 
> > > > > +/*
> > > > > + *  gen_pool data descriptor for gen_pool_first_fit_align.
> > > > > + */
> > > > > +struct genpool_data_align {
> > > > > + int align;  /* alignment by bytes for starting 
> > > > > address */
> > > > > +};
> > > > > +
> > > > 
> > > > (sorry for chiming in late, I've been traveling)
> > > > 
> > > > Is there an advantage here to wrapping this in a structure instead of
> > > > just passing a pointer to an align integer?
> > > 
> > > 
> > > Please look at the commit message for
> > > ca279cf1065fb689abea1dc7d8c11787729bb185 which adds "data":
> > > 
> > > "As I can't predict all the possible requirements/needs for all
> > > allocation uses cases, I add a "free" field 'void *data' to pass any
> > > needed information to the allocatio

RE: [PATCH v6 1/3] genalloc:support memory-allocation with bytes-alignment to genalloc

2015-08-25 Thread Zhao Qiang
On 08/25/2015 12:01 PM, Laura Abbott wrote:

> -Original Message-
> From: Laura Abbott [mailto:labb...@redhat.com]
> Sent: Tuesday, August 25, 2015 12:01 PM
> To: Zhao Qiang-B45475; Wood Scott-B07421
> Cc: linux-kernel@vger.kernel.org; linuxppc-...@lists.ozlabs.org;
> lau...@codeaurora.org; Xie Xiaobo-R63061; b...@kernel.crashing.org; Li
> Yang-Leo-R58472; pau...@samba.org
> Subject: Re: [PATCH v6 1/3] genalloc:support memory-allocation with
> bytes-alignment to genalloc
> 
> On 08/24/2015 07:40 PM, Zhao Qiang wrote:
> > On 08/25/2015 07:11 AM, Laura Abbott wrote:
> >> -Original Message-
> >> From: Laura Abbott [mailto:labb...@redhat.com]
> >> Sent: Tuesday, August 25, 2015 7:11 AM
> >> To: Zhao Qiang-B45475; Wood Scott-B07421
> >> Cc: linux-kernel@vger.kernel.org; linuxppc-...@lists.ozlabs.org;
> >> lau...@codeaurora.org; Xie Xiaobo-R63061; b...@kernel.crashing.org;
> >> Li Yang-Leo-R58472; pau...@samba.org
> >> Subject: Re: [PATCH v6 1/3] genalloc:support memory-allocation with
> >> bytes-alignment to genalloc
> >>
> >> On 08/24/2015 02:31 AM, Zhao Qiang wrote:
> >>> Bytes alignment is required to manage some special RAM, so add
> >>> gen_pool_first_fit_align to genalloc, meanwhile add
> >>> gen_pool_alloc_data to pass data to gen_pool_first_fit_align(modify
> >>> gen_pool_alloc as a wrapper)
> >>>
> >>> Signed-off-by: Zhao Qiang 
> >>> ---
> >>> Changes for v6:
> >>>   - patches set v6 include a new patch because of using
> >>>   - genalloc to manage QE MURAM, patch 0001 is the new
> >>>   - patch, adding bytes alignment for allocation for use.
> >>>
> >>>include/linux/genalloc.h | 23 +++
> >>>lib/genalloc.c   | 58
> >> +++-
> >>>2 files changed, 72 insertions(+), 9 deletions(-)
> >>>
> >>> diff --git a/include/linux/genalloc.h b/include/linux/genalloc.h
> >>> index 1ccaab4..55da07e 100644
> >>> --- a/include/linux/genalloc.h
> >>> +++ b/include/linux/genalloc.h
> >>> @@ -34,6 +34,7 @@
> >>>
> >>>struct device;
> >>>struct device_node;
> >>> +struct gen_pool;
> >>>
> >>>/**
> >>> * Allocation callback function type definition @@ -47,7 +48,7 @@
> >>> typedef unsigned long (*genpool_algo_t)(unsigned long *map,
> >>>   unsigned long size,
> >>>   unsigned long start,
> >>>   unsigned int nr,
> >>> - void *data);
> >>> + void *data, struct gen_pool *pool);
> >>>
> >>>/*
> >>> *  General purpose special memory pool descriptor.
> >>> @@ -73,6 +74,13 @@ struct gen_pool_chunk {
> >>>   unsigned long bits[0];  /* bitmap for allocating memory
> chunk
> >> */
> >>>};
> >>>
> >>> +/*
> >>> + *  gen_pool data descriptor for gen_pool_first_fit_align.
> >>> + */
> >>> +struct genpool_data_align {
> >>> + int align;  /* alignment by bytes for starting address */
> >>> +};
> >>> +
> >>
> >> (sorry for chiming in late, I've been traveling)
> >>
> >> Is there an advantage here to wrapping this in a structure instead of
> >> just passing a pointer to an align integer?
> >
> >
> > Please look at the commit message for
> > ca279cf1065fb689abea1dc7d8c11787729bb185 which adds "data":
> >
> > "As I can't predict all the possible requirements/needs for all
> > allocation uses cases, I add a "free" field 'void *data' to pass any
> > needed information to the allocation function.  For example 'data'
> > could be used to handle a structure where you store the alignment, the
> > expected memory bank, the requester device, or any information that
> > could influence the allocation algorithm."
> >
> 
> Right, I understand what the purpose is but I'm not sure what you're
> getting from the structure vs passing a pointer, e.g.
> 
> int align;
> 
> align = 4;
> 
> gen_pool_alloc_data(&pool, size, &align);
> 
> it just seems to obfuscate what's going on by wrapping a single integer
> in a structure that's narrowly defined in a generic function right now. I
> guess it could change later which would necessitate having the structure
> but again it's so generic I'm not sure what else you would pass that
> would be applicable to all clients.

Scott and me have discussed about this issue in my RFC patch.
Please review: http://patchwork.ozlabs.org/patch/493297/

> 
> Thanks,
> Laura
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH v6 1/3] genalloc:support memory-allocation with bytes-alignment to genalloc

2015-08-24 Thread Laura Abbott

On 08/24/2015 07:40 PM, Zhao Qiang wrote:

On 08/25/2015 07:11 AM, Laura Abbott wrote:

-Original Message-
From: Laura Abbott [mailto:labb...@redhat.com]
Sent: Tuesday, August 25, 2015 7:11 AM
To: Zhao Qiang-B45475; Wood Scott-B07421
Cc: linux-kernel@vger.kernel.org; linuxppc-...@lists.ozlabs.org;
lau...@codeaurora.org; Xie Xiaobo-R63061; b...@kernel.crashing.org; Li
Yang-Leo-R58472; pau...@samba.org
Subject: Re: [PATCH v6 1/3] genalloc:support memory-allocation with
bytes-alignment to genalloc

On 08/24/2015 02:31 AM, Zhao Qiang wrote:

Bytes alignment is required to manage some special RAM, so add
gen_pool_first_fit_align to genalloc, meanwhile add
gen_pool_alloc_data to pass data to gen_pool_first_fit_align(modify
gen_pool_alloc as a wrapper)

Signed-off-by: Zhao Qiang 
---
Changes for v6:
- patches set v6 include a new patch because of using
- genalloc to manage QE MURAM, patch 0001 is the new
- patch, adding bytes alignment for allocation for use.

   include/linux/genalloc.h | 23 +++
   lib/genalloc.c   | 58

+++-

   2 files changed, 72 insertions(+), 9 deletions(-)

diff --git a/include/linux/genalloc.h b/include/linux/genalloc.h index
1ccaab4..55da07e 100644
--- a/include/linux/genalloc.h
+++ b/include/linux/genalloc.h
@@ -34,6 +34,7 @@

   struct device;
   struct device_node;
+struct gen_pool;

   /**
* Allocation callback function type definition @@ -47,7 +48,7 @@
typedef unsigned long (*genpool_algo_t)(unsigned long *map,
unsigned long size,
unsigned long start,
unsigned int nr,
-   void *data);
+   void *data, struct gen_pool *pool);

   /*
*  General purpose special memory pool descriptor.
@@ -73,6 +74,13 @@ struct gen_pool_chunk {
unsigned long bits[0];  /* bitmap for allocating memory chunk

*/

   };

+/*
+ *  gen_pool data descriptor for gen_pool_first_fit_align.
+ */
+struct genpool_data_align {
+   int align;  /* alignment by bytes for starting address */
+};
+


(sorry for chiming in late, I've been traveling)

Is there an advantage here to wrapping this in a structure instead of
just passing a pointer to an align integer?



Please look at the commit message for
ca279cf1065fb689abea1dc7d8c11787729bb185 which adds "data":

"As I can't predict all the possible requirements/needs for all allocation
uses cases, I add a "free" field 'void *data' to pass any needed
information to the allocation function.  For example 'data' could be used
to handle a structure where you store the alignment, the expected memory
bank, the requester device, or any information that could influence the
allocation algorithm."



Right, I understand what the purpose is but I'm not sure what you're getting
from the structure vs passing a pointer, e.g.

int align;

align = 4;

gen_pool_alloc_data(&pool, size, &align);

it just seems to obfuscate what's going on by wrapping a single integer in
a structure that's narrowly defined in a generic function right now. I guess
it could change later which would necessitate having the structure but again
it's so generic I'm not sure what else you would pass that would be applicable
to all clients.

Thanks,
Laura
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


RE: [PATCH v6 1/3] genalloc:support memory-allocation with bytes-alignment to genalloc

2015-08-24 Thread Zhao Qiang
On 08/25/2015 07:11 AM, Laura Abbott wrote:
> -Original Message-
> From: Laura Abbott [mailto:labb...@redhat.com]
> Sent: Tuesday, August 25, 2015 7:11 AM
> To: Zhao Qiang-B45475; Wood Scott-B07421
> Cc: linux-kernel@vger.kernel.org; linuxppc-...@lists.ozlabs.org;
> lau...@codeaurora.org; Xie Xiaobo-R63061; b...@kernel.crashing.org; Li
> Yang-Leo-R58472; pau...@samba.org
> Subject: Re: [PATCH v6 1/3] genalloc:support memory-allocation with
> bytes-alignment to genalloc
> 
> On 08/24/2015 02:31 AM, Zhao Qiang wrote:
> > Bytes alignment is required to manage some special RAM, so add
> > gen_pool_first_fit_align to genalloc, meanwhile add
> > gen_pool_alloc_data to pass data to gen_pool_first_fit_align(modify
> > gen_pool_alloc as a wrapper)
> >
> > Signed-off-by: Zhao Qiang 
> > ---
> > Changes for v6:
> > - patches set v6 include a new patch because of using
> > - genalloc to manage QE MURAM, patch 0001 is the new
> > - patch, adding bytes alignment for allocation for use.
> >
> >   include/linux/genalloc.h | 23 +++
> >   lib/genalloc.c   | 58
> +++-
> >   2 files changed, 72 insertions(+), 9 deletions(-)
> >
> > diff --git a/include/linux/genalloc.h b/include/linux/genalloc.h index
> > 1ccaab4..55da07e 100644
> > --- a/include/linux/genalloc.h
> > +++ b/include/linux/genalloc.h
> > @@ -34,6 +34,7 @@
> >
> >   struct device;
> >   struct device_node;
> > +struct gen_pool;
> >
> >   /**
> >* Allocation callback function type definition @@ -47,7 +48,7 @@
> > typedef unsigned long (*genpool_algo_t)(unsigned long *map,
> > unsigned long size,
> > unsigned long start,
> > unsigned int nr,
> > -   void *data);
> > +   void *data, struct gen_pool *pool);
> >
> >   /*
> >*  General purpose special memory pool descriptor.
> > @@ -73,6 +74,13 @@ struct gen_pool_chunk {
> > unsigned long bits[0];  /* bitmap for allocating memory chunk
> */
> >   };
> >
> > +/*
> > + *  gen_pool data descriptor for gen_pool_first_fit_align.
> > + */
> > +struct genpool_data_align {
> > +   int align;  /* alignment by bytes for starting address */
> > +};
> > +
> 
> (sorry for chiming in late, I've been traveling)
> 
> Is there an advantage here to wrapping this in a structure instead of
> just passing a pointer to an align integer?


Please look at the commit message for
ca279cf1065fb689abea1dc7d8c11787729bb185 which adds "data":

"As I can't predict all the possible requirements/needs for all allocation
uses cases, I add a "free" field 'void *data' to pass any needed 
information to the allocation function.  For example 'data' could be used 
to handle a structure where you store the alignment, the expected memory 
bank, the requester device, or any information that could influence the 
allocation algorithm."

> 
> Thanks,
> Laura
-Zhao 
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH v6 1/3] genalloc:support memory-allocation with bytes-alignment to genalloc

2015-08-24 Thread Laura Abbott

On 08/24/2015 02:31 AM, Zhao Qiang wrote:

Bytes alignment is required to manage some special RAM,
so add gen_pool_first_fit_align to genalloc,
meanwhile add gen_pool_alloc_data to pass data to
gen_pool_first_fit_align(modify gen_pool_alloc as a wrapper)

Signed-off-by: Zhao Qiang 
---
Changes for v6:
- patches set v6 include a new patch because of using
- genalloc to manage QE MURAM, patch 0001 is the new
- patch, adding bytes alignment for allocation for use.

  include/linux/genalloc.h | 23 +++
  lib/genalloc.c   | 58 +++-
  2 files changed, 72 insertions(+), 9 deletions(-)

diff --git a/include/linux/genalloc.h b/include/linux/genalloc.h
index 1ccaab4..55da07e 100644
--- a/include/linux/genalloc.h
+++ b/include/linux/genalloc.h
@@ -34,6 +34,7 @@

  struct device;
  struct device_node;
+struct gen_pool;

  /**
   * Allocation callback function type definition
@@ -47,7 +48,7 @@ typedef unsigned long (*genpool_algo_t)(unsigned long *map,
unsigned long size,
unsigned long start,
unsigned int nr,
-   void *data);
+   void *data, struct gen_pool *pool);

  /*
   *  General purpose special memory pool descriptor.
@@ -73,6 +74,13 @@ struct gen_pool_chunk {
unsigned long bits[0];  /* bitmap for allocating memory chunk */
  };

+/*
+ *  gen_pool data descriptor for gen_pool_first_fit_align.
+ */
+struct genpool_data_align {
+   int align;  /* alignment by bytes for starting address */
+};
+


(sorry for chiming in late, I've been traveling)

Is there an advantage here to wrapping this in a structure instead of just
passing a pointer to an align integer?

Thanks,
Laura

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


[PATCH v6 1/3] genalloc:support memory-allocation with bytes-alignment to genalloc

2015-08-24 Thread Zhao Qiang
Bytes alignment is required to manage some special RAM,
so add gen_pool_first_fit_align to genalloc,
meanwhile add gen_pool_alloc_data to pass data to
gen_pool_first_fit_align(modify gen_pool_alloc as a wrapper)

Signed-off-by: Zhao Qiang 
---
Changes for v6:
- patches set v6 include a new patch because of using 
- genalloc to manage QE MURAM, patch 0001 is the new 
- patch, adding bytes alignment for allocation for use.

 include/linux/genalloc.h | 23 +++
 lib/genalloc.c   | 58 +++-
 2 files changed, 72 insertions(+), 9 deletions(-)

diff --git a/include/linux/genalloc.h b/include/linux/genalloc.h
index 1ccaab4..55da07e 100644
--- a/include/linux/genalloc.h
+++ b/include/linux/genalloc.h
@@ -34,6 +34,7 @@
 
 struct device;
 struct device_node;
+struct gen_pool;
 
 /**
  * Allocation callback function type definition
@@ -47,7 +48,7 @@ typedef unsigned long (*genpool_algo_t)(unsigned long *map,
unsigned long size,
unsigned long start,
unsigned int nr,
-   void *data);
+   void *data, struct gen_pool *pool);
 
 /*
  *  General purpose special memory pool descriptor.
@@ -73,6 +74,13 @@ struct gen_pool_chunk {
unsigned long bits[0];  /* bitmap for allocating memory chunk */
 };
 
+/*
+ *  gen_pool data descriptor for gen_pool_first_fit_align.
+ */
+struct genpool_data_align {
+   int align;  /* alignment by bytes for starting address */
+};
+
 extern struct gen_pool *gen_pool_create(int, int);
 extern phys_addr_t gen_pool_virt_to_phys(struct gen_pool *pool, unsigned long);
 extern int gen_pool_add_virt(struct gen_pool *, unsigned long, phys_addr_t,
@@ -96,6 +104,7 @@ static inline int gen_pool_add(struct gen_pool *pool, 
unsigned long addr,
 }
 extern void gen_pool_destroy(struct gen_pool *);
 extern unsigned long gen_pool_alloc(struct gen_pool *, size_t);
+extern unsigned long gen_pool_alloc_data(struct gen_pool *, size_t, void 
*data);
 extern void *gen_pool_dma_alloc(struct gen_pool *pool, size_t size,
dma_addr_t *dma);
 extern void gen_pool_free(struct gen_pool *, unsigned long, size_t);
@@ -108,14 +117,20 @@ extern void gen_pool_set_algo(struct gen_pool *pool, 
genpool_algo_t algo,
void *data);
 
 extern unsigned long gen_pool_first_fit(unsigned long *map, unsigned long size,
-   unsigned long start, unsigned int nr, void *data);
+   unsigned long start, unsigned int nr, void *data,
+   struct gen_pool *pool);
+
+extern unsigned long gen_pool_first_fit_align(unsigned long *map,
+   unsigned long size, unsigned long start, unsigned int nr,
+   void *data, struct gen_pool *pool);
 
 extern unsigned long gen_pool_first_fit_order_align(unsigned long *map,
unsigned long size, unsigned long start, unsigned int nr,
-   void *data);
+   void *data, struct gen_pool *pool);
 
 extern unsigned long gen_pool_best_fit(unsigned long *map, unsigned long size,
-   unsigned long start, unsigned int nr, void *data);
+   unsigned long start, unsigned int nr, void *data,
+   struct gen_pool *pool);
 
 extern struct gen_pool *devm_gen_pool_create(struct device *dev,
int min_alloc_order, int nid);
diff --git a/lib/genalloc.c b/lib/genalloc.c
index d214866..b8762b1 100644
--- a/lib/genalloc.c
+++ b/lib/genalloc.c
@@ -269,6 +269,24 @@ EXPORT_SYMBOL(gen_pool_destroy);
  */
 unsigned long gen_pool_alloc(struct gen_pool *pool, size_t size)
 {
+   return gen_pool_alloc_data(pool, size, pool->data);
+}
+EXPORT_SYMBOL(gen_pool_alloc);
+
+/**
+ * gen_pool_alloc_data - allocate special memory from the pool
+ * @pool: pool to allocate from
+ * @size: number of bytes to allocate from the pool
+ * @data: data passed to algorithm
+ *
+ * Allocate the requested number of bytes from the specified pool.
+ * Uses the pool allocation function (with first-fit algorithm by default).
+ * Can not be used in NMI handler on architectures without
+ * NMI-safe cmpxchg implementation.
+ */
+unsigned long gen_pool_alloc_data(struct gen_pool *pool, size_t size,
+   void *data)
+{
struct gen_pool_chunk *chunk;
unsigned long addr = 0;
int order = pool->min_alloc_order;
@@ -290,7 +308,7 @@ unsigned long gen_pool_alloc(struct gen_pool *pool, size_t 
size)
end_bit = chunk_size(chunk) >> order;
 retry:
start_bit = pool->algo(chunk->bits, end_bit, start_bit, nbits,
-   pool->data);
+   data, pool);
if (start_bit >= end_bit)
continue;
remain = bitmap_set_ll(chunk->bits, start_bit, nbits);
@@ -309,7 +327,7 @@ retry:
rcu_read_unlock();
return addr;
 }
-EXPOR