mm/memory_hotplug: __add_pages() and __remove_pages() cleanups
Subsystems (2)
v2 cover mm/memory_hotplug: __add_pages() and __remove_pages() cleanups
This is the follow up of:
"[PATCH v1] mm/memory_hotplug: Easier calculation to get pages to
next section boundary" [1]
Cleanups as a follow up on commit 52fb87c81f11 ("mm/memory_hotplug:
cleanup __remove_pages()").
v1 -> v2:
- "mm/memory_hotplug: simplify calculation of number of pages in __remove_pages()"
-- Rephrased subject/description, but left patch as is
- "mm/memory_hotplug: cleanup __add_pages()"
-- Make the logic match the logic in __remove_pages()
[1] https://lkml.kernel.org/r/[email protected]
David Hildenbrand (2):
mm/memory_hotplug: simplify calculation of number of pages in
__remove_pages()
mm/memory_hotplug: cleanup __add_pages()
mm/memory_hotplug.c | 21 +++++++++------------ 1 file changed, 9 insertions(+), 12 deletions(-)
Signature · 1 line
2.24.1
PATCH v2 1/2 mm/memory_hotplug: simplify calculation of number of pages in __remove_pages()
In commit 52fb87c81f11 ("mm/memory_hotplug: cleanup __remove_pages()"), we cleaned up __remove_pages(), and introduced a shorter variant to calculate the number of pages to the next section boundary.
Turns out we can make this calculation easier to read. We always want to have the number of pages (> 0) to the next section boundary, starting from the current pfn.
We'll clean up __remove_pages() in a follow-up patch and directly make use of this computation.
--- mm/memory_hotplug.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-)
mm/memory_hotplug.c +2 -1
@@ -534,7 +534,8 @@ void __remove_pages(unsigned long pfn, unsigned long nr_pages,for (; pfn < end_pfn; pfn += cur_nr_pages) {cond_resched ();/* Select all remaining pages up to the next section boundary */ - cur_nr_pages =min (end_pfn - pfn, -(pfn |PAGE_SECTION_MASK )); + cur_nr_pages =min (end_pfn - pfn, +SECTION_ALIGN_UP (pfn +1 ) - pfn);__remove_section (pfn, cur_nr_pages, map_offset, altmap); map_offset =0 ; }
Signature · 1 line
2.24.1
Re: [PATCH v2 1/2] mm/memory_hotplug: simplify calculation of number of pages in __remove_pages()
Quoted 28 lines · David Hildenbrand
In commit
52fb87c81f11("mm/memory_hotplug: cleanup__remove_pages()"), we cleaned up__remove_pages(), and introduced a shorter variant to calculate the number of pages to the next section boundary.Turns out we can make this calculation easier to read. We always want to have the number of pages (> 0) to the next section boundary, starting from the current pfn.
We'll clean up
__remove_pages()in a follow-up patch and directly make use of this computation.Suggested-by Segher Boessenkool <[email protected]>Cc Andrew Morton <[email protected]>Cc Oscar Salvador <[email protected]>Cc Michal Hocko <[email protected]>Cc Baoquan He <[email protected]>Cc Dan Williams <[email protected]>Cc Wei Yang <[email protected]>Signed-off-by David Hildenbrand <[email protected]>--- mm/memory_hotplug.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-)mm/memory_hotplug.c +0
-0
mm/memory_hotplug.c void __remove_pages(unsigned long pfn, unsigned long nr_pages, · line 538
Quoted 6 lines
mm/memory_hotplug.c +2
-1@@ -534,7 +534,8 @@ void __remove_pages(unsigned long pfn, unsigned long nr_pages,for (; pfn < end_pfn; pfn += cur_nr_pages) {cond_resched ();/* Select all remaining pages up to the next section boundary */ - cur_nr_pages =min (end_pfn - pfn, -(pfn |PAGE_SECTION_MASK )); + cur_nr_pages =min (end_pfn - pfn, +SECTION_ALIGN_UP (pfn +1 ) - pfn);
Re: [PATCH v2 1/2] mm/memory_hotplug: simplify calculation of number of pages in __remove_pages()
In reply to David HildenbrandQuoted 11 lines · David Hildenbrand
In commit
52fb87c81f11("mm/memory_hotplug: cleanup__remove_pages()"), we cleaned up__remove_pages(), and introduced a shorter variant to calculate the number of pages to the next section boundary.Turns out we can make this calculation easier to read. We always want to have the number of pages (> 0) to the next section boundary, starting from the current pfn.
We'll clean up
__remove_pages()in a follow-up patch and directly make use of this computation.
Quoted 8 lines
Suggested-by Segher Boessenkool <[email protected]>Cc Andrew Morton <[email protected]>Cc Oscar Salvador <[email protected]>Cc Michal Hocko <[email protected]>Cc Baoquan He <[email protected]>Cc Dan Williams <[email protected]>Cc Wei Yang <[email protected]>Signed-off-by David Hildenbrand <[email protected]>
Quoted 20 lines
---
mm/memory_hotplug.c| 3 ++-1 file changed, 2 insertions(+), 1 deletion(-)mm/memory_hotplug.c +2
-1@@ -534,7 +534,8 @@ void __remove_pages(unsigned long pfn, unsigned long nr_pages,for (; pfn < end_pfn; pfn += cur_nr_pages) {cond_resched ();/* Select all remaining pages up to the next section boundary */ - cur_nr_pages =min (end_pfn - pfn, -(pfn |PAGE_SECTION_MASK )); + cur_nr_pages =min (end_pfn - pfn, +SECTION_ALIGN_UP (pfn +1 ) - pfn);__remove_section (pfn, cur_nr_pages, map_offset, altmap); map_offset =0 ; }Signature · 1 line
2.24.1
Signature · 2 lines
Wei Yang Help you, Help me
Re: [PATCH v2 1/2] mm/memory_hotplug: simplify calculation of number of pages in __remove_pages()
In reply to David HildenbrandQuoted 7 lines · David Hildenbrand
In commit
52fb87c81f11("mm/memory_hotplug: cleanup__remove_pages()"), we cleaned up__remove_pages(), and introduced a shorter variant to calculate the number of pages to the next section boundary.Turns out we can make this calculation easier to read. We always want to have the number of pages (> 0) to the next section boundary, starting from the current pfn.
@Andrew
This patch seems to have another of these weird MIME crap in it. (my other patches in -next seem to be fine)
See
https://lore.kernel.org/linux-mm/[email protected]/raw
"
... fro=
m"
should simply be "... from"
Signature · 3 lines
Thanks, David / dhildenb
Re: [PATCH v2 1/2] mm/memory_hotplug: simplify calculation of number of pages in __remove_pages()
Quoted 7 lines · David Hildenbrand
This patch seems to have another of these weird MIME crap in it. (my other patches in -next seem to be fine)
See
That email actually looks fine.
Yes, it has that
fro=
m
pattern, but it also has
Content-Transfer-Encoding: quoted-printable
so the recipient should be doing the right thing with that pattern.
The patch itself also has MIME encoding in it:
- cur_nr_pages =3D min(end_pfn - pfn, -(pfn | PAGE_SECTION_MASK));
+ cur_nr_pages =3D min(end_pfn - pfn,
so the patch wouldn't even apply unless the recipient did the proper MIME decode of the message.
That's also why the non-raw message looks fine:
https://lore.kernel.org/linux-mm/[email protected]/
because the raw message data has the proper encoding information.
In contrast, look at the email that Andrew sent me and that I complained about:
https://lore.kernel.org/linux-mm/20200329021719.MBKzW0xSl%[email protected]/
and notice how that *non-raw* email has that
Withou=
t
pattern in it. And when you look at the raw one:
https://lore.kernel.org/linux-mm/20200329021719.MBKzW0xSl%[email protected]/raw
it has no content transfer encoding line in the headers.
Linus
Re: [PATCH v2 1/2] mm/memory_hotplug: simplify calculation of number of pages in __remove_pages()
Quoted 46 lines
Am 29.03.2020 um 22:10 schrieb Linus Torvalds <[email protected]>:
Quoted 7 lines · David Hildenbrand
This patch seems to have another of these weird MIME crap in it. (my other patches in -next seem to be fine)
See
That email actually looks fine.
Yes, it has that
fro=
mpattern, but it also has
Content-Transfer-Encoding: quoted-printable
so the recipient should be doing the right thing with that pattern.
The patch itself also has MIME encoding in it:
-
cur_nr_pages=3Dmin(end_pfn - pfn, -(pfn | PAGE_SECTION_MASK));
+cur_nr_pages=3D min(end_pfn - pfn,so the patch wouldn't even apply unless the recipient did the proper MIME decode of the message.
That's also why the non-raw message looks fine:
https://lore.kernel.org/linux-mm/[email protected]/
because the raw message data has the proper encoding information.
In contrast, look at the email that Andrew sent me and that I complained about:
https://lore.kernel.org/linux-mm/20200329021719.MBKzW0xSl%[email protected]/
and notice how that *non-raw* email has that
Quoted 8 lines
Withou=
tpattern in it. And when you look at the raw one:
https://lore.kernel.org/linux-mm/20200329021719.MBKzW0xSl%[email protected]/raw
it has no content transfer encoding line in the headers.
Interesting, at least the patch in -next is messed up. I remember Andrew adapted some scripts, maybe this is a leftover.
Cheers!
Quoted 3 lines
Linus
Re: [PATCH v2 1/2] mm/memory_hotplug: simplify calculation of number of pages in __remove_pages()
Quoted 2 lines · David Hildenbrand
Interesting, at least the patch in -next is messed up. I remember Andrew adapted some scripts, maybe this is a leftover.
It does look like s broken MIME decoding script somewhere.
But it's odd, because as mentioned, Andrew definitely handles MIME in other places correctly - including your messages when it comes to the patch data itself. It's only the message above the patch that hasn't been properly decoded.
Curious.
Linus
Re: [PATCH v2 1/2] mm/memory_hotplug: simplify calculation of number of pages in __remove_pages()
In reply to Linus TorvaldsQuoted 4 lines · Linus Torvalds
In contrast, look at the email that Andrew sent me and that I complained about:
https://lore.kernel.org/linux-mm/20200329021719.MBKzW0xSl%[email protected]/
Hmm. I'm trying to figure out how and where Andrew got the original from you.
There's
https://lore.kernel.org/linux-mm/[email protected]/raw
but again, that one actually looks fine. It has that
Content-Transfer-Encoding: quoted-printable
header line, but it doesn't even have the "=\n" pattern in the text at all. It does have MIME encoding in the patch, but that's all fine.
Then there's a new version:
https://lore.kernel.org/linux-mm/[email protected]/raw
and that one *does* have the "Withou=\nt" pattern in it. But it still has the proper
Content-Transfer-Encoding: quoted-printable
in it, so the recipient should decode it just fine (and again, you can see that in the non-raw email - it looks just fine).
So your emails on lore look fine. I'm not seeing how that got corrupted.
Linus
Re: [PATCH v2 1/2] mm/memory_hotplug: simplify calculation of number of pages in __remove_pages()
Quoted 31 lines
Am 29.03.2020 um 22:19 schrieb Linus Torvalds <[email protected]>:
On Sun, Mar 29, 2020 at 1:09 PM Linus Torvalds
Quoted 4 lines · <[email protected]> wrote:
In contrast, look at the email that Andrew sent me and that I complained about:
https://lore.kernel.org/linux-mm/20200329021719.MBKzW0xSl%[email protected]/
Hmm. I'm trying to figure out how and where Andrew got the original from you.
There's
https://lore.kernel.org/linux-mm/[email protected]/raw
but again, that one actually looks fine. It has that
Content-Transfer-Encoding: quoted-printable
header line, but it doesn't even have the "=\n" pattern in the text at all. It does have MIME encoding in the patch, but that's all fine.
Then there's a new version:
https://lore.kernel.org/linux-mm/[email protected]/raw
and that one *does* have the "Withou=\nt" pattern in it. But it still has the proper
Quoted 6 lines
Content-Transfer-Encoding: quoted-printable
in it, so the recipient should decode it just fine (and again, you can see that in the non-raw email - it looks just fine).
So your emails on lore look fine. I'm not seeing how that got corrupted.
*maybe* Andrew updated only the patch description, copying the raw content. Eventually he converts MIME only when importing, so the description got corrupted.
... or the mail he received via cc got messed up by my mailing infrastructure.
Cheers
Andrew Morton
maintainerRe: [PATCH v2 1/2] mm/memory_hotplug: simplify calculation of number of pages in __remove_pages()
Quoted 7 lines · On Sun, 29 Mar 2020 22:41:18 +0200 David Hildenbrand <[email protected]> wrote:
Quoted 7 lines
https://lore.kernel.org/linux-mm/[email protected]/raw
and that one *does* have the "Withou=\nt" pattern in it. But it still has the proper
Content-Transfer-Encoding: quoted-printable
Quoted 8 lines
Quoted 4 lines
in it, so the recipient should decode it just fine (and again, you can see that in the non-raw email - it looks just fine).
So your emails on lore look fine. I'm not seeing how that got corrupted.
*maybe* Andrew updated only the patch description, copying the raw content. Eventually he converts MIME only when importing, so the description got corrupted.
... or the mail he received via cc got messed up by my mailing infrastructure.
Something like that. It's sylpheed being weird. In some situations (save as...) it will perform the mime conversion but in others (%f in an action menu) it does not. So for David's patches I do save-as for the patch and manually edit the mime out of changelog. It's basically "only David" so I haven't got around to doing anything smarter.
My nightly check-all-the-patches-for-various-cruft script emails me about =3D but I didn't' have a test for "o=$m". I just added one.
Re: [PATCH v2 1/2] mm/memory_hotplug: simplify calculation of number of pages in __remove_pages()
Quoted 3 lines · Andrew Morton
My nightly check-all-the-patches-for-various-cruft script emails me about =3D but I didn't' have a test for "o=$m". I just added one.
=3D may be the common one, but =20 and =09 are others that end up showing up when whitespace gets quoted for various reasons.
Another one is =46 for 'F'. Why? Because some mailers think that "From" at the beginning of a line is the mbox beginning marker, and they'll escape any line that begins with "From" to use "=46rom" instead.
Those mailers are wrong (at a _minimum_ it's "From " with a space, and you generally should be even stricter than that), but it happens.
And obviously, if there is real 8-bit stuff, you'll get all the real odd hex noise.
Linus
PATCH v2 2/2 mm/memory_hotplug: cleanup __add_pages()
In reply to David HildenbrandLet's drop the basically unused section stuff and simplify. The logic now matches the logic in __remove_pages().
--- mm/memory_hotplug.c | 18 +++++++----------- 1 file changed, 7 insertions(+), 11 deletions(-)
mm/memory_hotplug.c +7 -11
@@ -307,8 +307,9 @@ static int check_hotplug_memory_addressable(unsigned long pfn,int __ref __add_pages (int nid,unsigned long pfn,unsigned long nr_pages,struct mhp_restrictions *restrictions) { +const unsigned long end_pfn = pfn + nr_pages; +unsigned long cur_nr_pages;int err; -unsigned long nr, start_sec, end_sec;struct vmem_altmap *altmap = restrictions->altmap; err =check_hotplug_memory_addressable (pfn, nr_pages);
@@ -331,18 +332,13 @@ int __ref __add_pages(int nid, unsigned long pfn, unsigned long nr_pages,if (err)return err; - start_sec =pfn_to_section_nr (pfn); - end_sec =pfn_to_section_nr (pfn + nr_pages -1 ); -for (nr = start_sec; nr <= end_sec; nr++) { -unsigned long pfns; - - pfns =min (nr_pages,PAGES_PER_SECTION - - (pfn & ~PAGE_SECTION_MASK )); - err =sparse_add_section (nid, pfn, pfns, altmap); +for (; pfn < end_pfn; pfn += cur_nr_pages) { +/* Select all remaining pages up to the next section boundary */ + cur_nr_pages =min (end_pfn - pfn, +SECTION_ALIGN_UP (pfn +1 ) - pfn); + err =sparse_add_section (nid, pfn, cur_nr_pages, altmap);if (err)break ; - pfn += pfns; - nr_pages -= pfns;cond_resched (); }vmemmap_populate_print_last ();
Signature · 1 line
2.24.1
Re: [PATCH v2 2/2] mm/memory_hotplug: cleanup __add_pages()
Quoted 37 lines · David Hildenbrand
Let's drop the basically unused section stuff and simplify. The logic now matches the logic in
__remove_pages().Cc Segher Boessenkool <[email protected]>Cc Andrew Morton <[email protected]>Cc Oscar Salvador <[email protected]>Cc Michal Hocko <[email protected]>Cc Baoquan He <[email protected]>Cc Dan Williams <[email protected]>Cc Wei Yang <[email protected]>Signed-off-by David Hildenbrand <[email protected]>--- mm/memory_hotplug.c | 18 +++++++----------- 1 file changed, 7 insertions(+), 11 deletions(-)mm/memory_hotplug.c +2
-4@@ -307,8 +307,9 @@ static int check_hotplug_memory_addressable(unsigned long pfn,int __ref __add_pages (int nid,unsigned long pfn,unsigned long nr_pages,struct mhp_restrictions *restrictions) { +const unsigned long end_pfn = pfn + nr_pages; +unsigned long cur_nr_pages;int err; -unsigned long nr, start_sec, end_sec;struct vmem_altmap *altmap = restrictions->altmap; err =check_hotplug_memory_addressable (pfn, nr_pages);@@ -331,18 +332,13 @@ int __ref __add_pages(int nid, unsigned long pfn, unsigned long nr_pages,if (err)return err; - start_sec =pfn_to_section_nr (pfn); - end_sec =pfn_to_section_nr (pfn + nr_pages -1 ); -for (nr = start_sec; nr <= end_sec; nr++) {
mm/memory_hotplug.c int __ref __add_pages(int nid, unsigned long pfn, unsigned long nr_pages, · line 339
Quoted 10 lines
mm/memory_hotplug.c +5
-5-unsigned long pfns; - - pfns =min (nr_pages,PAGES_PER_SECTION - - (pfn & ~PAGE_SECTION_MASK )); - err =sparse_add_section (nid, pfn, pfns, altmap); +for (; pfn < end_pfn; pfn += cur_nr_pages) { +/* Select all remaining pages up to the next section boundary */ + cur_nr_pages =min (end_pfn - pfn, +SECTION_ALIGN_UP (pfn +1 ) - pfn); + err =sparse_add_section (nid, pfn, cur_nr_pages, altmap);
Honestly, I am not a big fan of this kind of code refactoring. The old code may span seveal more lines or define several several more veriables, but logic is clear, and no visible defect. It's hard to say how much we can benefit from this kind of code simplifying, and reviewing it will take people more time. While for the code style consistency with __remove_page(), I would like to see it's merged. My personal opinion.
Quoted 10 lines
mm/memory_hotplug.c +0
-2if (err)break ; - pfn += pfns; - nr_pages -= pfns;cond_resched (); }vmemmap_populate_print_last ();Signature · 1 line
2.24.1
Re: [PATCH v2 2/2] mm/memory_hotplug: cleanup __add_pages()
Quoted 44 lines · Baoquan He
Quoted 43 lines · David Hildenbrand
Let's drop the basically unused section stuff and simplify. The logic now matches the logic in
__remove_pages().Cc Segher Boessenkool <[email protected]>Cc Andrew Morton <[email protected]>Cc Oscar Salvador <[email protected]>Cc Michal Hocko <[email protected]>Cc Baoquan He <[email protected]>Cc Dan Williams <[email protected]>Cc Wei Yang <[email protected]>Signed-off-by David Hildenbrand <[email protected]>--- mm/memory_hotplug.c | 18 +++++++----------- 1 file changed, 7 insertions(+), 11 deletions(-)mm/memory_hotplug.c +3
-9@@ -307,8 +307,9 @@ static int check_hotplug_memory_addressable(unsigned long pfn,int __ref __add_pages (int nid,unsigned long pfn,unsigned long nr_pages,struct mhp_restrictions *restrictions) { +const unsigned long end_pfn = pfn + nr_pages; +unsigned long cur_nr_pages;int err; -unsigned long nr, start_sec, end_sec;struct vmem_altmap *altmap = restrictions->altmap; err =check_hotplug_memory_addressable (pfn, nr_pages);@@ -331,18 +332,13 @@ int __ref __add_pages(int nid, unsigned long pfn, unsigned long nr_pages,if (err)return err; - start_sec =pfn_to_section_nr (pfn); - end_sec =pfn_to_section_nr (pfn + nr_pages -1 ); -for (nr = start_sec; nr <= end_sec; nr++) { -unsigned long pfns; - - pfns =min (nr_pages,PAGES_PER_SECTION - - (pfn & ~PAGE_SECTION_MASK )); - err =sparse_add_section (nid, pfn, pfns, altmap); +for (; pfn < end_pfn; pfn += cur_nr_pages) {
Quoted 8 lines
mm/memory_hotplug.c int __ref __add_pages(int nid, unsigned long pfn, unsigned long nr_pages, · line 339
Quoted 4 lines
mm/memory_hotplug.c +4
-0+/* Select all remaining pages up to the next section boundary */ + cur_nr_pages =min (end_pfn - pfn, +SECTION_ALIGN_UP (pfn +1 ) - pfn); + err =sparse_add_section (nid, pfn, cur_nr_pages, altmap);Honestly, I am not a big fan of this kind of code refactoring. The old code may span seveal more lines or define several several more veriables, but logic is clear, and no visible defect. It's hard to say how much we
I'm sorry, but iterating over variables and not using a single one in the body is definitely not clean, at least IMHO. Leftover from sub-section hotadd support.
Quoted 3 lines
can benefit from this kind of code simplifying, and reviewing it will take people more time. While for the code style consistency with
__remove_page(), I would like to see it's merged. My personal opinion.
Thanks!
Signature · 3 lines
Thanks, David / dhildenb
Re: [PATCH v2 2/2] mm/memory_hotplug: cleanup __add_pages()
Quoted 54 lines · David Hildenbrand
Quoted 44 lines · Baoquan He
Quoted 43 lines · David Hildenbrand
Let's drop the basically unused section stuff and simplify. The logic now matches the logic in
__remove_pages().Cc Segher Boessenkool <[email protected]>Cc Andrew Morton <[email protected]>Cc Oscar Salvador <[email protected]>Cc Michal Hocko <[email protected]>Cc Baoquan He <[email protected]>Cc Dan Williams <[email protected]>Cc Wei Yang <[email protected]>Signed-off-by David Hildenbrand <[email protected]>--- mm/memory_hotplug.c | 18 +++++++----------- 1 file changed, 7 insertions(+), 11 deletions(-)mm/memory_hotplug.c +3
-9@@ -307,8 +307,9 @@ static int check_hotplug_memory_addressable(unsigned long pfn,int __ref __add_pages (int nid,unsigned long pfn,unsigned long nr_pages,struct mhp_restrictions *restrictions) { +const unsigned long end_pfn = pfn + nr_pages; +unsigned long cur_nr_pages;int err; -unsigned long nr, start_sec, end_sec;struct vmem_altmap *altmap = restrictions->altmap; err =check_hotplug_memory_addressable (pfn, nr_pages);@@ -331,18 +332,13 @@ int __ref __add_pages(int nid, unsigned long pfn, unsigned long nr_pages,if (err)return err; - start_sec =pfn_to_section_nr (pfn); - end_sec =pfn_to_section_nr (pfn + nr_pages -1 ); -for (nr = start_sec; nr <= end_sec; nr++) { -unsigned long pfns; - - pfns =min (nr_pages,PAGES_PER_SECTION - - (pfn & ~PAGE_SECTION_MASK )); - err =sparse_add_section (nid, pfn, pfns, altmap); +for (; pfn < end_pfn; pfn += cur_nr_pages) {Quoted 8 lines
mm/memory_hotplug.c int __ref __add_pages(int nid, unsigned long pfn, unsigned long nr_pages, · line 339
Quoted 4 lines
mm/memory_hotplug.c +4
-0+/* Select all remaining pages up to the next section boundary */ + cur_nr_pages =min (end_pfn - pfn, +SECTION_ALIGN_UP (pfn +1 ) - pfn); + err =sparse_add_section (nid, pfn, cur_nr_pages, altmap);Honestly, I am not a big fan of this kind of code refactoring. The old code may span seveal more lines or define several several more veriables, but logic is clear, and no visible defect. It's hard to say how much we
Quoted 2 lines
I'm sorry, but iterating over variables and not using a single one in the body is definitely not clean, at least IMHO. Leftover from
Hmm, sometime we do use iterator to loop over only, it's just not so good. People usually have their own preferred coding style, and try to optimize to remove the itch in heart, totally understand. I have no strong objection to this, as long as it gets support from reviewers, it's not a bad thing.
Quoted 13 lines
sub-section hotadd support.
Quoted 3 lines
can benefit from this kind of code simplifying, and reviewing it will take people more time. While for the code style consistency with
__remove_page(), I would like to see it's merged. My personal opinion.Thanks!
Signature · 3 lines
Thanks, David / dhildenb
Re: [PATCH v2 2/2] mm/memory_hotplug: cleanup __add_pages()
In reply to David HildenbrandQuoted 11 lines · David Hildenbrand
Let's drop the basically unused section stuff and simplify. The logic now matches the logic in
__remove_pages().Cc Segher Boessenkool <[email protected]>Cc Andrew Morton <[email protected]>Cc Oscar Salvador <[email protected]>Cc Michal Hocko <[email protected]>Cc Baoquan He <[email protected]>Cc Dan Williams <[email protected]>Cc Wei Yang <[email protected]>Signed-off-by David Hildenbrand <[email protected]>
Quoted 45 lines
---
mm/memory_hotplug.c| 18 +++++++-----------1 file changed, 7 insertions(+), 11 deletions(-)mm/memory_hotplug.c +7
-11@@ -307,8 +307,9 @@ static int check_hotplug_memory_addressable(unsigned long pfn,int __ref __add_pages (int nid,unsigned long pfn,unsigned long nr_pages,struct mhp_restrictions *restrictions) { +const unsigned long end_pfn = pfn + nr_pages; +unsigned long cur_nr_pages;int err; -unsigned long nr, start_sec, end_sec;struct vmem_altmap *altmap = restrictions->altmap; err =check_hotplug_memory_addressable (pfn, nr_pages);@@ -331,18 +332,13 @@ int __ref __add_pages(int nid, unsigned long pfn, unsigned long nr_pages,if (err)return err; - start_sec =pfn_to_section_nr (pfn); - end_sec =pfn_to_section_nr (pfn + nr_pages -1 ); -for (nr = start_sec; nr <= end_sec; nr++) { -unsigned long pfns; - - pfns =min (nr_pages,PAGES_PER_SECTION - - (pfn & ~PAGE_SECTION_MASK )); - err =sparse_add_section (nid, pfn, pfns, altmap); +for (; pfn < end_pfn; pfn += cur_nr_pages) { +/* Select all remaining pages up to the next section boundary */ + cur_nr_pages =min (end_pfn - pfn, +SECTION_ALIGN_UP (pfn +1 ) - pfn); + err =sparse_add_section (nid, pfn, cur_nr_pages, altmap);if (err)break ; - pfn += pfns; - nr_pages -= pfns;cond_resched (); }vmemmap_populate_print_last ();Signature · 1 line
2.24.1
Signature · 2 lines
Wei Yang Help you, Help me