Patchwork lib/genalloc.c: rename addr_in_gen_pool to gen_pool_has_addr

login
register
mail settings
Submitter Benjamin Serebrin via iommu
Date Dec. 28, 2018, 8:39 a.m.
Message ID <20181228083950.20398-1-sjhuang@iluvatar.ai>
Download mbox | patch
Permalink /patch/690571/
State New
Headers show

Comments

Benjamin Serebrin via iommu - Dec. 28, 2018, 8:39 a.m.
Follow the kernel conventions, rename addr_in_gen_pool to
gen_pool_has_addr.

Signed-off-by: Huang Shijie <sjhuang@iluvatar.ai>
---
 arch/arm/mm/dma-mapping.c | 2 +-
 drivers/misc/sram-exec.c  | 2 +-
 include/linux/genalloc.h  | 2 +-
 kernel/dma/remap.c        | 2 +-
 lib/genalloc.c            | 6 +++---
 5 files changed, 7 insertions(+), 7 deletions(-)
Benjamin Serebrin via iommu - Dec. 28, 2018, 9:17 a.m.
On Fri, Dec 28, 2018 at 09:48:34AM +0100, Christoph Hellwig wrote:
> On Fri, Dec 28, 2018 at 04:39:50PM +0800, Huang Shijie wrote:
> > Follow the kernel conventions, rename addr_in_gen_pool to
> > gen_pool_has_addr.
> 
> Which convention?  The old name certainly looks more sensible to me.
I submitted a patch to export the symbol, addr_in_gen_pool.
But most the exported symbols are named like gen_pool_*.

What about I add a macro in the header genalloc.h, such as:

#define addr_in_gen_pool 	gen_pool_has_addr

> 
> If we really want to change anything about this function I'd suggest
> to drop the size argument, as the address itself should describe the
> region good enough.

Maybe others need the @size argument. I am not sure if it is right to remove
the size argument...

Thanks
Huang Shijie
Andrew Morton - Dec. 28, 2018, 11:39 p.m.
On Fri, 28 Dec 2018 09:48:34 +0100 Christoph Hellwig <hch@lst.de> wrote:

> On Fri, Dec 28, 2018 at 04:39:50PM +0800, Huang Shijie wrote:
> > Follow the kernel conventions, rename addr_in_gen_pool to
> > gen_pool_has_addr.
> 
> Which convention?

That symbols from subsystem foo should be called foo_*.  Not uniformly
observed by any means, but it is a thing.

> The old name certainly looks more sensible to me.

It does read better, but it's rather a standout in this case:

q:/usr/src/25> grep EXPORT_SYMBOL lib/genalloc.c
EXPORT_SYMBOL(gen_pool_create);
EXPORT_SYMBOL(gen_pool_add_virt);
EXPORT_SYMBOL(gen_pool_virt_to_phys);
EXPORT_SYMBOL(gen_pool_destroy);
EXPORT_SYMBOL(gen_pool_alloc);
EXPORT_SYMBOL(gen_pool_alloc_algo);
EXPORT_SYMBOL(gen_pool_dma_alloc);
EXPORT_SYMBOL(gen_pool_free);
EXPORT_SYMBOL(gen_pool_for_each_chunk);
EXPORT_SYMBOL(addr_in_gen_pool);
EXPORT_SYMBOL_GPL(gen_pool_avail);
EXPORT_SYMBOL_GPL(gen_pool_size);
EXPORT_SYMBOL(gen_pool_set_algo);
EXPORT_SYMBOL(gen_pool_first_fit);
EXPORT_SYMBOL(gen_pool_first_fit_align);
EXPORT_SYMBOL(gen_pool_fixed_alloc);
EXPORT_SYMBOL(gen_pool_first_fit_order_align);
EXPORT_SYMBOL(gen_pool_best_fit);
EXPORT_SYMBOL_GPL(gen_pool_get);
EXPORT_SYMBOL(devm_gen_pool_create);
EXPORT_SYMBOL_GPL(of_gen_pool_get);

Patch

diff --git a/arch/arm/mm/dma-mapping.c b/arch/arm/mm/dma-mapping.c
index f1e2922e447c..a876101a0f82 100644
--- a/arch/arm/mm/dma-mapping.c
+++ b/arch/arm/mm/dma-mapping.c
@@ -563,7 +563,7 @@  static void *__alloc_from_pool(size_t size, struct page **ret_page)
 
 static bool __in_atomic_pool(void *start, size_t size)
 {
-	return addr_in_gen_pool(atomic_pool, (unsigned long)start, size);
+	return gen_pool_has_addr(atomic_pool, (unsigned long)start, size);
 }
 
 static int __free_from_pool(void *start, size_t size)
diff --git a/drivers/misc/sram-exec.c b/drivers/misc/sram-exec.c
index 426ad912b441..d054e2842a5f 100644
--- a/drivers/misc/sram-exec.c
+++ b/drivers/misc/sram-exec.c
@@ -96,7 +96,7 @@  void *sram_exec_copy(struct gen_pool *pool, void *dst, void *src,
 	if (!part)
 		return NULL;
 
-	if (!addr_in_gen_pool(pool, (unsigned long)dst, size))
+	if (!gen_pool_has_addr(pool, (unsigned long)dst, size))
 		return NULL;
 
 	base = (unsigned long)part->base;
diff --git a/include/linux/genalloc.h b/include/linux/genalloc.h
index dd0a452373e7..c5e5bed82fbc 100644
--- a/include/linux/genalloc.h
+++ b/include/linux/genalloc.h
@@ -156,7 +156,7 @@  extern struct gen_pool *devm_gen_pool_create(struct device *dev,
 		int min_alloc_order, int nid, const char *name);
 extern struct gen_pool *gen_pool_get(struct device *dev, const char *name);
 
-bool addr_in_gen_pool(struct gen_pool *pool, unsigned long start,
+extern bool gen_pool_has_addr(struct gen_pool *pool, unsigned long start,
 			size_t size);
 
 #ifdef CONFIG_OF
diff --git a/kernel/dma/remap.c b/kernel/dma/remap.c
index 18cc09fc27b9..5022b904d7bc 100644
--- a/kernel/dma/remap.c
+++ b/kernel/dma/remap.c
@@ -158,7 +158,7 @@  int __init dma_atomic_pool_init(gfp_t gfp, pgprot_t prot)
 
 bool dma_in_atomic_pool(void *start, size_t size)
 {
-	return addr_in_gen_pool(atomic_pool, (unsigned long)start, size);
+	return gen_pool_has_addr(atomic_pool, (unsigned long)start, size);
 }
 
 void *dma_alloc_from_pool(size_t size, struct page **ret_page, gfp_t flags)
diff --git a/lib/genalloc.c b/lib/genalloc.c
index 96b6f478d275..946bbeefec81 100644
--- a/lib/genalloc.c
+++ b/lib/genalloc.c
@@ -423,7 +423,7 @@  void gen_pool_for_each_chunk(struct gen_pool *pool,
 EXPORT_SYMBOL(gen_pool_for_each_chunk);
 
 /**
- * addr_in_gen_pool - checks if an address falls within the range of a pool
+ * gen_pool_has_addr - checks if an address falls within the range of a pool
  * @pool:	the generic memory pool
  * @start:	start address
  * @size:	size of the region
@@ -431,7 +431,7 @@  EXPORT_SYMBOL(gen_pool_for_each_chunk);
  * Check if the range of addresses falls within the specified pool. Returns
  * true if the entire range is contained in the pool and false otherwise.
  */
-bool addr_in_gen_pool(struct gen_pool *pool, unsigned long start,
+bool gen_pool_has_addr(struct gen_pool *pool, unsigned long start,
 			size_t size)
 {
 	bool found = false;
@@ -450,7 +450,7 @@  bool addr_in_gen_pool(struct gen_pool *pool, unsigned long start,
 	rcu_read_unlock();
 	return found;
 }
-EXPORT_SYMBOL(addr_in_gen_pool);
+EXPORT_SYMBOL(gen_pool_has_addr);
 
 /**
  * gen_pool_avail - get available free space of the pool