RE: Make __memblock_free_early a wrapper of memblock_free rather dup it

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



I believe I explained why we choose it to be a wrapper,
" I noticed that __memblock_free_early and memblock_free has the same code. At first I think we can delete __memblock_free_early till __memblock_free_late remind me __memblock_free_early is meaningful. It’s a note to call this before struct page was initialized."

Andrew may choose plan as he see fit.

Regards,
Matt

-----Original Message-----
From: Mike Rapoport [mailto:rppt@xxxxxxxxxxxxx] 
Sent: 2018年11月25日 18:30
To: Andrew Morton
Cc: Wang, Matt; linux-mm@xxxxxxxxx
Subject: Re: Make __memblock_free_early a wrapper of memblock_free rather dup it


[EXTERNAL EMAIL] 

On Wed, Nov 21, 2018 at 09:27:40PM -0800, Andrew Morton wrote:
> On Thu, 22 Nov 2018 04:01:53 +0000 "Wang, Matt" <Matt.Wang@xxxxxxxx> wrote:
> 
> > Subject: [PATCH] Make __memblock_free_early a wrapper of 
> > memblock_free rather  than dup it
> > 
> > Signed-off-by: Wentao Wang <witallwang@xxxxxxxxx>
> > ---
> >  mm/memblock.c | 7 +------
> >  1 file changed, 1 insertion(+), 6 deletions(-)
> > 
> > diff --git a/mm/memblock.c b/mm/memblock.c index 9a2d5ae..08bf136 
> > 100644
> > --- a/mm/memblock.c
> > +++ b/mm/memblock.c
> > @@ -1546,12 +1546,7 @@ void * __init memblock_alloc_try_nid(
> >   */
> >  void __init __memblock_free_early(phys_addr_t base, phys_addr_t 
> > size)  {
> > -	phys_addr_t end = base + size - 1;
> > -
> > -	memblock_dbg("%s: [%pa-%pa] %pF\n",
> > -		     __func__, &base, &end, (void *)_RET_IP_);
> > -	kmemleak_free_part_phys(base, size);
> > -	memblock_remove_range(&memblock.reserved, base, size);
> > +	memblock_free(base, size);
> >  }
> 
> hm, I suppose so.  The debug messaging becomes less informative but 
> the duplication is indeed irritating and if we really want to show the 
> different caller info in the messages, we could do it in a smarter 
> fashion.

Sorry for jumping late, but I believe the better way would be simply replace the only two calls to __memblock_free_early() with calls to memblock_free().

The patch below is based on the current mmots.

>From 4de5a2aabb0b898c6b4add6bf91175fc55725362 Mon Sep 17 00:00:00 2001
From: Mike Rapoport <rppt@xxxxxxxxxxxxx>
Date: Sun, 25 Nov 2018 12:20:46 +0200
Subject: [PATCH] memblock: replace usage of __memblock_free_early() with
 memblock_free()

The __memblock_free_early() function is only used by the convinince wrappers, so essentially we wrap a call to memblock_free() twice.
Replace calls of __memblock_free_early() with calls to memblock_free() and drop the former.

Signed-off-by: Mike Rapoport <rppt@xxxxxxxxxxxxx>
---
 include/linux/memblock.h |  5 ++---
 mm/memblock.c            | 22 ++++++++--------------
 2 files changed, 10 insertions(+), 17 deletions(-)

diff --git a/include/linux/memblock.h b/include/linux/memblock.h index 5ba52a7..e9e4017 100644
--- a/include/linux/memblock.h
+++ b/include/linux/memblock.h
@@ -154,7 +154,6 @@ void __next_mem_range_rev(u64 *idx, int nid, enum memblock_flags flags,  void __next_reserved_mem_region(u64 *idx, phys_addr_t *out_start,
 				phys_addr_t *out_end);
 
-void __memblock_free_early(phys_addr_t base, phys_addr_t size);  void __memblock_free_late(phys_addr_t base, phys_addr_t size);
 
 /**
@@ -452,13 +451,13 @@ static inline void * __init memblock_alloc_node_nopanic(phys_addr_t size,  static inline void __init memblock_free_early(phys_addr_t base,
 					      phys_addr_t size)
 {
-	__memblock_free_early(base, size);
+	memblock_free(base, size);
 }
 
 static inline void __init memblock_free_early_nid(phys_addr_t base,
 						  phys_addr_t size, int nid)
 {
-	__memblock_free_early(base, size);
+	memblock_free(base, size);
 }
 
 static inline void __init memblock_free_late(phys_addr_t base, phys_addr_t size) diff --git a/mm/memblock.c b/mm/memblock.c index 0559979..b842ce1 100644
--- a/mm/memblock.c
+++ b/mm/memblock.c
@@ -800,7 +800,14 @@ int __init_memblock memblock_remove(phys_addr_t base, phys_addr_t size)
 	return memblock_remove_range(&memblock.memory, base, size);  }
 
-
+/**
+ * memblock_free - free boot memory block
+ * @base: phys starting address of the  boot memory block
+ * @size: size of the boot memory block in bytes
+ *
+ * Free boot memory block previously allocated by memblock_alloc_xx() API.
+ * The freeing memory will not be released to the buddy allocator.
+ */
 int __init_memblock memblock_free(phys_addr_t base, phys_addr_t size)  {
 	phys_addr_t end = base + size - 1;
@@ -1600,19 +1607,6 @@ void * __init memblock_alloc_try_nid(  }
 
 /**
- * __memblock_free_early - free boot memory block
- * @base: phys starting address of the  boot memory block
- * @size: size of the boot memory block in bytes
- *
- * Free boot memory block previously allocated by memblock_alloc_xx() API.
- * The freeing memory will not be released to the buddy allocator.
- */
-void __init __memblock_free_early(phys_addr_t base, phys_addr_t size) -{
-	memblock_free(base, size);
-}
-
-/**
  * __memblock_free_late - free bootmem block pages directly to buddy allocator
  * @base: phys starting address of the  boot memory block
  * @size: size of the boot memory block in bytes
--
2.7.4


-- 
Sincerely yours,
Mike.





[Index of Archives]     [Linux ARM Kernel]     [Linux ARM]     [Linux Omap]     [Fedora ARM]     [IETF Annouce]     [Bugtraq]     [Linux OMAP]     [Linux MIPS]     [eCos]     [Asterisk Internet PBX]     [Linux API]

  Powered by Linux