Hi Thomas, Thanks for you comments On 6/25/19 3:45 PM, Thomas Gleixner wrote: > Hoan, > > On Tue, 25 Jun 2019, Hoan Tran OS wrote: > > Please use 'x86/Kconfig: ' as prefix. > >> This patch removes CONFIG_NODES_SPAN_OTHER_NODES as it's >> enabled by default with NUMA. > > Please do not use 'This patch' in changelogs. It's pointless because we > already know that this is a patch. > > See also Documentation/process/submitting-patches.rst and search for 'This > patch' > > Simply say: > > Remove CONFIG_NODES_SPAN_OTHER_NODES as it's enabled by default with > NUMA. > Got it, will fix > But ..... > >> @@ -1567,15 +1567,6 @@ config X86_64_ACPI_NUMA >> ---help--- >> Enable ACPI SRAT based node topology detection. >> >> -# Some NUMA nodes have memory ranges that span >> -# other nodes. Even though a pfn is valid and >> -# between a node's start and end pfns, it may not >> -# reside on that node. See memmap_init_zone() >> -# for details. >> -config NODES_SPAN_OTHER_NODES >> - def_bool y >> - depends on X86_64_ACPI_NUMA > > the changelog does not mention that this lifts the dependency on > X86_64_ACPI_NUMA and therefore enables that functionality for anything > which has NUMA enabled including 32bit. > I think this config is used for a NUMA layout which NUMA nodes addresses are spanned to other nodes. I think 32bit NUMA also have the same issue with that layout. Please correct me if I'm wrong. > The core mm change gives no helpful information either. You just copied the > above comment text from some random Kconfig. Yes, as it's a correct comment and is used at multiple places. Thanks Hoan > > This needs a bit more data in the changelogs and the cover letter: > > - Why is it useful to enable it unconditionally > > - Why is it safe to do so, even if the architecture had constraints on > it > > - What's the potential impact > > Thanks, > > tglx >
