On 5/30/22 12:24, David Hildenbrand wrote: > On 26.05.22 04:12, Anshuman Khandual wrote: >> In check_pfn_span(), a 'reason' string is being used to recreate the caller >> function name, while printing the warning message. It is really unnecessary >> as the warning message could just be printed inside the caller depending on >> the return code. Currently there are just two callers for check_pfn_span() >> i.e __add_pages() and __remove_pages(). Let's clean this up. >> >> Cc: Oscar Salvador <osalvador@xxxxxxx> >> Cc: David Hildenbrand <david@xxxxxxxxxx> >> Cc: Andrew Morton <akpm@xxxxxxxxxxxxxxxxxxxx> >> Cc: linux-mm@xxxxxxxxx >> Cc: linux-kernel@xxxxxxxxxxxxxxx >> Signed-off-by: Anshuman Khandual <anshuman.khandual@xxxxxxx> >> --- >> Changes in V2: >> >> - Fixed typo in commit message >> - Dropped using 'ret' to capture check_pfn_span() return value in __add_pages() >> >> Changes in V1: >> >> https://lore.kernel.org/all/20220525033910.3781764-1-anshuman.khandual@xxxxxxx/ >> >> mm/memory_hotplug.c | 20 +++++++++----------- >> 1 file changed, 9 insertions(+), 11 deletions(-) >> >> diff --git a/mm/memory_hotplug.c b/mm/memory_hotplug.c >> index 416b38ca8def..3b24386e9276 100644 >> --- a/mm/memory_hotplug.c >> +++ b/mm/memory_hotplug.c >> @@ -220,8 +220,7 @@ static void release_memory_resource(struct resource *res) >> kfree(res); >> } >> >> -static int check_pfn_span(unsigned long pfn, unsigned long nr_pages, >> - const char *reason) >> +static int check_pfn_span(unsigned long pfn, unsigned long nr_pages) >> { >> /* >> * Disallow all operations smaller than a sub-section and only >> @@ -238,12 +237,8 @@ static int check_pfn_span(unsigned long pfn, unsigned long nr_pages, >> min_align = PAGES_PER_SUBSECTION; >> else >> min_align = PAGES_PER_SECTION; >> - if (!IS_ALIGNED(pfn, min_align) >> - || !IS_ALIGNED(nr_pages, min_align)) { >> - WARN(1, "Misaligned __%s_pages start: %#lx end: #%lx\n", >> - reason, pfn, pfn + nr_pages - 1); >> + if (!IS_ALIGNED(pfn, min_align) || !IS_ALIGNED(nr_pages, min_align)) > We could do > > if (!IS_ALIGNED(pfn | nr_pages, min_align)) Sure, will change it.
