* [PATCH V2] mm/memory_hotplug: Drop 'reason' argument from check_pfn_span()
@ 2022-05-26 2:12 Anshuman Khandual
2022-05-30 5:03 ` Oscar Salvador
2022-05-30 6:54 ` David Hildenbrand
0 siblings, 2 replies; 4+ messages in thread
From: Anshuman Khandual @ 2022-05-26 2:12 UTC (permalink / raw)
To: linux-mm
Cc: Anshuman Khandual, Oscar Salvador, David Hildenbrand,
Andrew Morton, linux-kernel
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@suse.de>
Cc: David Hildenbrand <david@redhat.com>
Cc: Andrew Morton <akpm@linux-foundation.org>
Cc: linux-mm@kvack.org
Cc: linux-kernel@vger.kernel.org
Signed-off-by: Anshuman Khandual <anshuman.khandual@arm.com>
---
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@arm.com/
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))
return -EINVAL;
- }
return 0;
}
@@ -320,9 +315,10 @@ int __ref __add_pages(int nid, unsigned long pfn, unsigned long nr_pages,
altmap->alloc = 0;
}
- err = check_pfn_span(pfn, nr_pages, "add");
- if (err)
- return err;
+ if (check_pfn_span(pfn, nr_pages)) {
+ WARN(1, "Misaligned %s start: %#lx end: #%lx\n", __func__, pfn, pfn + nr_pages - 1);
+ return -EINVAL;
+ }
for (; pfn < end_pfn; pfn += cur_nr_pages) {
/* Select all remaining pages up to the next section boundary */
@@ -518,8 +514,10 @@ void __remove_pages(unsigned long pfn, unsigned long nr_pages,
map_offset = vmem_altmap_offset(altmap);
- if (check_pfn_span(pfn, nr_pages, "remove"))
+ if (check_pfn_span(pfn, nr_pages)) {
+ WARN(1, "Misaligned %s start: %#lx end: #%lx\n", __func__, pfn, pfn + nr_pages - 1);
return;
+ }
for (; pfn < end_pfn; pfn += cur_nr_pages) {
cond_resched();
--
2.20.1
^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [PATCH V2] mm/memory_hotplug: Drop 'reason' argument from check_pfn_span()
2022-05-26 2:12 [PATCH V2] mm/memory_hotplug: Drop 'reason' argument from check_pfn_span() Anshuman Khandual
@ 2022-05-30 5:03 ` Oscar Salvador
2022-05-30 6:54 ` David Hildenbrand
1 sibling, 0 replies; 4+ messages in thread
From: Oscar Salvador @ 2022-05-30 5:03 UTC (permalink / raw)
To: Anshuman Khandual
Cc: linux-mm, David Hildenbrand, Andrew Morton, linux-kernel
On Thu, May 26, 2022 at 07:42:58AM +0530, 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@suse.de>
> Cc: David Hildenbrand <david@redhat.com>
> Cc: Andrew Morton <akpm@linux-foundation.org>
> Cc: linux-mm@kvack.org
> Cc: linux-kernel@vger.kernel.org
> Signed-off-by: Anshuman Khandual <anshuman.khandual@arm.com>
Acked-by: Oscar Salvador <osalvador@suse.de>
> ---
> 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@arm.com/
>
> 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))
> return -EINVAL;
> - }
> return 0;
> }
>
> @@ -320,9 +315,10 @@ int __ref __add_pages(int nid, unsigned long pfn, unsigned long nr_pages,
> altmap->alloc = 0;
> }
>
> - err = check_pfn_span(pfn, nr_pages, "add");
> - if (err)
> - return err;
> + if (check_pfn_span(pfn, nr_pages)) {
> + WARN(1, "Misaligned %s start: %#lx end: #%lx\n", __func__, pfn, pfn + nr_pages - 1);
> + return -EINVAL;
> + }
>
> for (; pfn < end_pfn; pfn += cur_nr_pages) {
> /* Select all remaining pages up to the next section boundary */
> @@ -518,8 +514,10 @@ void __remove_pages(unsigned long pfn, unsigned long nr_pages,
>
> map_offset = vmem_altmap_offset(altmap);
>
> - if (check_pfn_span(pfn, nr_pages, "remove"))
> + if (check_pfn_span(pfn, nr_pages)) {
> + WARN(1, "Misaligned %s start: %#lx end: #%lx\n", __func__, pfn, pfn + nr_pages - 1);
> return;
> + }
>
> for (; pfn < end_pfn; pfn += cur_nr_pages) {
> cond_resched();
> --
> 2.20.1
>
--
Oscar Salvador
SUSE Labs
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH V2] mm/memory_hotplug: Drop 'reason' argument from check_pfn_span()
2022-05-26 2:12 [PATCH V2] mm/memory_hotplug: Drop 'reason' argument from check_pfn_span() Anshuman Khandual
2022-05-30 5:03 ` Oscar Salvador
@ 2022-05-30 6:54 ` David Hildenbrand
2022-05-30 9:03 ` Anshuman Khandual
1 sibling, 1 reply; 4+ messages in thread
From: David Hildenbrand @ 2022-05-30 6:54 UTC (permalink / raw)
To: Anshuman Khandual, linux-mm; +Cc: Oscar Salvador, Andrew Morton, linux-kernel
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@suse.de>
> Cc: David Hildenbrand <david@redhat.com>
> Cc: Andrew Morton <akpm@linux-foundation.org>
> Cc: linux-mm@kvack.org
> Cc: linux-kernel@vger.kernel.org
> Signed-off-by: Anshuman Khandual <anshuman.khandual@arm.com>
> ---
> 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@arm.com/
>
> 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))
Reviewed-by: David Hildenbrand <david@redhat.com>
--
Thanks,
David / dhildenb
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH V2] mm/memory_hotplug: Drop 'reason' argument from check_pfn_span()
2022-05-30 6:54 ` David Hildenbrand
@ 2022-05-30 9:03 ` Anshuman Khandual
0 siblings, 0 replies; 4+ messages in thread
From: Anshuman Khandual @ 2022-05-30 9:03 UTC (permalink / raw)
To: David Hildenbrand, linux-mm; +Cc: Oscar Salvador, Andrew Morton, linux-kernel
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@suse.de>
>> Cc: David Hildenbrand <david@redhat.com>
>> Cc: Andrew Morton <akpm@linux-foundation.org>
>> Cc: linux-mm@kvack.org
>> Cc: linux-kernel@vger.kernel.org
>> Signed-off-by: Anshuman Khandual <anshuman.khandual@arm.com>
>> ---
>> 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@arm.com/
>>
>> 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.
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2022-05-30 9:03 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-05-26 2:12 [PATCH V2] mm/memory_hotplug: Drop 'reason' argument from check_pfn_span() Anshuman Khandual
2022-05-30 5:03 ` Oscar Salvador
2022-05-30 6:54 ` David Hildenbrand
2022-05-30 9:03 ` Anshuman Khandual
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.