| Message ID | 20181107173859.24096-3-logang@deltatee.com (mailing list archive) |
|---|---|
| State | Superseded, archived |
| Headers | show |
| Series | Introduce common code for risc-v sparsemem support | expand |
On Wed, 7 Nov 2018 10:38:59 -0700 Logan Gunthorpe <logang@deltatee.com> wrote: > Presently the arches arm64, arm and sh have a function which loops through > each memblock and calls memory present. riscv will require a similar > function. > > Introduce a common memblocks_present() function that can be used by > all the arches. Subsequent patches will cleanup the arches that > make use of this. > > ... > > --- a/mm/sparse.c > +++ b/mm/sparse.c > @@ -239,6 +239,17 @@ void __init memory_present(int nid, unsigned long start, unsigned long end) > } > } > > +void __init memblocks_present(void) > +{ > + struct memblock_region *reg; > + > + for_each_memblock(memory, reg) { > + memory_present(memblock_get_region_node(reg), > + memblock_region_memory_base_pfn(reg), > + memblock_region_memory_end_pfn(reg)); > + } > +} > + I don't like the name much. To me, memblocks_present means "are memblocks present" whereas this actually means "memblocks are present". But whatever. A little covering comment which describes what this does and why it does it would be nice. Acked-by: Andrew Morton <akpm@linux-foundation.org> I can grab both patches and shall sneak them into 4.20-rcX, but feel free to merge them into some git tree if you'd prefer. If I see them turn up in linux-next I shall drop my copy.
On 2018-11-07 1:12 p.m., Andrew Morton wrote: >> +void __init memblocks_present(void) >> +{ >> + struct memblock_region *reg; >> + >> + for_each_memblock(memory, reg) { >> + memory_present(memblock_get_region_node(reg), >> + memblock_region_memory_base_pfn(reg), >> + memblock_region_memory_end_pfn(reg)); >> + } >> +} >> + > > I don't like the name much. To me, memblocks_present means "are > memblocks present" whereas this actually means "memblocks are present". > But whatever. A little covering comment which describes what this > does and why it does it would be nice. The same argument can be made about the existing memory_present() function and I think it's worth keeping the naming consistent. I'll add a comment and resend shortly. > Acked-by: Andrew Morton <akpm@linux-foundation.org> > > I can grab both patches and shall sneak them into 4.20-rcX, but feel > free to merge them into some git tree if you'd prefer. If I see them > turn up in linux-next I shall drop my copy. Sounds good, thanks. Logan
Logan, On Wed, 7 Nov 2018, Logan Gunthorpe wrote: > On 2018-11-07 1:12 p.m., Andrew Morton wrote: > >> +void __init memblocks_present(void) > >> +{ > >> + struct memblock_region *reg; > >> + > >> + for_each_memblock(memory, reg) { > >> + memory_present(memblock_get_region_node(reg), > >> + memblock_region_memory_base_pfn(reg), > >> + memblock_region_memory_end_pfn(reg)); > >> + } > >> +} > >> + > > > > I don't like the name much. To me, memblocks_present means "are > > memblocks present" whereas this actually means "memblocks are present". > > But whatever. A little covering comment which describes what this > > does and why it does it would be nice. > > The same argument can be made about the existing memory_present() > function and I think it's worth keeping the naming consistent. I'll add > a comment and resend shortly. Actually if both names suck, then there also is the option to rename both instead of adding a comment to explain the suckage. Thanks, tglx
On 2018-11-07 1:26 p.m., Thomas Gleixner wrote: > Logan, > > On Wed, 7 Nov 2018, Logan Gunthorpe wrote: >> On 2018-11-07 1:12 p.m., Andrew Morton wrote: >>>> +void __init memblocks_present(void) >>>> +{ >>>> + struct memblock_region *reg; >>>> + >>>> + for_each_memblock(memory, reg) { >>>> + memory_present(memblock_get_region_node(reg), >>>> + memblock_region_memory_base_pfn(reg), >>>> + memblock_region_memory_end_pfn(reg)); >>>> + } >>>> +} >>>> + >>> >>> I don't like the name much. To me, memblocks_present means "are >>> memblocks present" whereas this actually means "memblocks are present". >>> But whatever. A little covering comment which describes what this >>> does and why it does it would be nice. >> >> The same argument can be made about the existing memory_present() >> function and I think it's worth keeping the naming consistent. I'll add >> a comment and resend shortly. > > Actually if both names suck, then there also is the option to rename both > instead of adding a comment to explain the suckage. Ok, well, I wasn't expecting to take on a big rename like that as it would create a patch touching a bunch of arches and mm files... But if we can come to some agreement on a better name and someone is willing to take that patch without significant delay then I'd be happy to create the patch and add it to the start of my series. Some ideas for new names: mark_memory_present() / mark_memblocks_present() set_memory_present() / set_memblocks_present() memory_register() / memblocks_register() register_memory() / register_memblocks() Logan
On Wed, 7 Nov 2018 13:36:34 -0700 Logan Gunthorpe <logang@deltatee.com> wrote: > > Actually if both names suck, then there also is the option to rename both > > instead of adding a comment to explain the suckage. > > Ok, well, I wasn't expecting to take on a big rename like that as it > would create a patch touching a bunch of arches and mm files... But if > we can come to some agreement on a better name and someone is willing to > take that patch without significant delay then I'd be happy to create > the patch and add it to the start of my series. Some other time ;) Let's just get the missing documentation added for now?
On Wed, 7 Nov 2018, Andrew Morton wrote: > On Wed, 7 Nov 2018 13:36:34 -0700 Logan Gunthorpe <logang@deltatee.com> wrote: > > > > Actually if both names suck, then there also is the option to rename both > > > instead of adding a comment to explain the suckage. > > > > Ok, well, I wasn't expecting to take on a big rename like that as it > > would create a patch touching a bunch of arches and mm files... But if > > we can come to some agreement on a better name and someone is willing to > > take that patch without significant delay then I'd be happy to create > > the patch and add it to the start of my series. > > Some other time ;) More precise: Manjana. You live way too close to Mexico :)
Hey Andrew, On 2018-11-07 1:12 p.m., Andrew Morton wrote: > Acked-by: Andrew Morton <akpm@linux-foundation.org> > > I can grab both patches and shall sneak them into 4.20-rcX, but feel > free to merge them into some git tree if you'd prefer. If I see them > turn up in linux-next I shall drop my copy. Just wanted to check if you are still planning to get these patches into 4.20-rcX. It would really help us if you can do this seeing we then won't have to delay a cycle and can target the riscv sparsemem code for 4.21. Thanks, Logan
On Thu, 6 Dec 2018 10:40:31 -0700 Logan Gunthorpe <logang@deltatee.com> wrote: > Hey Andrew, > > On 2018-11-07 1:12 p.m., Andrew Morton wrote: > > Acked-by: Andrew Morton <akpm@linux-foundation.org> > > > > I can grab both patches and shall sneak them into 4.20-rcX, but feel > > free to merge them into some git tree if you'd prefer. If I see them > > turn up in linux-next I shall drop my copy. > > Just wanted to check if you are still planning to get these patches into > 4.20-rcX. It would really help us if you can do this seeing we then > won't have to delay a cycle and can target the riscv sparsemem code for > 4.21. > Ah, OK, I assumed that it would be merged via an arm tree. I moved mm-introduce-common-struct_page_max_shift-define.patch mm-sparse-add-common-helper-to-mark-all-memblocks-present.patch to head-of-queue. Shall send to Linus next week.
diff --git a/include/linux/mmzone.h b/include/linux/mmzone.h index 847705a6d0ec..db023a92f3a4 100644 --- a/include/linux/mmzone.h +++ b/include/linux/mmzone.h @@ -783,6 +783,12 @@ void memory_present(int nid, unsigned long start, unsigned long end); static inline void memory_present(int nid, unsigned long start, unsigned long end) {} #endif +#if defined(CONFIG_SPARSEMEM) +void memblocks_present(void); +#else +static inline void memblocks_present(void) {} +#endif + #ifdef CONFIG_HAVE_MEMORYLESS_NODES int local_memory_node(int node_id); #else diff --git a/mm/sparse.c b/mm/sparse.c index 33307fc05c4d..5841e4ad6d93 100644 --- a/mm/sparse.c +++ b/mm/sparse.c @@ -239,6 +239,17 @@ void __init memory_present(int nid, unsigned long start, unsigned long end) } } +void __init memblocks_present(void) +{ + struct memblock_region *reg; + + for_each_memblock(memory, reg) { + memory_present(memblock_get_region_node(reg), + memblock_region_memory_base_pfn(reg), + memblock_region_memory_end_pfn(reg)); + } +} + /* * Subtle, we encode the real pfn into the mem_map such that * the identity pfn - section_mem_map will return the actual
Presently the arches arm64, arm and sh have a function which loops through each memblock and calls memory present. riscv will require a similar function. Introduce a common memblocks_present() function that can be used by all the arches. Subsequent patches will cleanup the arches that make use of this. Signed-off-by: Logan Gunthorpe <logang@deltatee.com> Cc: Andrew Morton <akpm@linux-foundation.org> Cc: Michal Hocko <mhocko@suse.com> Cc: Vlastimil Babka <vbabka@suse.cz> Cc: Oscar Salvador <osalvador@suse.de> --- include/linux/mmzone.h | 6 ++++++ mm/sparse.c | 11 +++++++++++ 2 files changed, 17 insertions(+)