* [PATCH v2 0/2] mm/memory_hotplug: __add_pages() and __remove_pages() cleanups @ 2020-02-28 9:58 David Hildenbrand 2020-02-28 9:58 ` [PATCH v2 1/2] mm/memory_hotplug: simplify calculation of number of pages in __remove_pages() David Hildenbrand 2020-02-28 9:58 ` [PATCH v2 2/2] mm/memory_hotplug: cleanup __add_pages() David Hildenbrand 0 siblings, 2 replies; 17+ messages in thread From: David Hildenbrand @ 2020-02-28 9:58 UTC (permalink / raw) To: linux-kernel; +Cc: linux-mm, David Hildenbrand 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/20200205135251.37488-1-david@redhat.com 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(-) -- 2.24.1 ^ permalink raw reply [flat|nested] 17+ messages in thread
* [PATCH v2 1/2] mm/memory_hotplug: simplify calculation of number of pages in __remove_pages() 2020-02-28 9:58 [PATCH v2 0/2] mm/memory_hotplug: __add_pages() and __remove_pages() cleanups David Hildenbrand @ 2020-02-28 9:58 ` David Hildenbrand 2020-02-28 10:22 ` Baoquan He ` (2 more replies) 2020-02-28 9:58 ` [PATCH v2 2/2] mm/memory_hotplug: cleanup __add_pages() David Hildenbrand 1 sibling, 3 replies; 17+ messages in thread From: David Hildenbrand @ 2020-02-28 9:58 UTC (permalink / raw) To: linux-kernel Cc: linux-mm, David Hildenbrand, Segher Boessenkool, Andrew Morton, Oscar Salvador, Michal Hocko, Baoquan He, Dan Williams, Wei Yang 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 <segher@kernel.crashing.org> Cc: Andrew Morton <akpm@linux-foundation.org> Cc: Oscar Salvador <osalvador@suse.de> Cc: Michal Hocko <mhocko@kernel.org> Cc: Baoquan He <bhe@redhat.com> Cc: Dan Williams <dan.j.williams@intel.com> Cc: Wei Yang <richardw.yang@linux.intel.com> Signed-off-by: David Hildenbrand <david@redhat.com> --- mm/memory_hotplug.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/mm/memory_hotplug.c b/mm/memory_hotplug.c index 4a9b3f6c6b37..8fe7e32dad48 100644 --- a/mm/memory_hotplug.c +++ b/mm/memory_hotplug.c @@ -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; } -- 2.24.1 ^ permalink raw reply related [flat|nested] 17+ messages in thread
* Re: [PATCH v2 1/2] mm/memory_hotplug: simplify calculation of number of pages in __remove_pages() 2020-02-28 9:58 ` [PATCH v2 1/2] mm/memory_hotplug: simplify calculation of number of pages in __remove_pages() David Hildenbrand @ 2020-02-28 10:22 ` Baoquan He 2020-02-28 13:25 ` Wei Yang 2020-03-29 19:19 ` David Hildenbrand 2 siblings, 0 replies; 17+ messages in thread From: Baoquan He @ 2020-02-28 10:22 UTC (permalink / raw) To: David Hildenbrand Cc: linux-kernel, linux-mm, Segher Boessenkool, Andrew Morton, Oscar Salvador, Michal Hocko, Dan Williams, Wei Yang On 02/28/20 at 10:58am, David Hildenbrand wrote: > 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 <segher@kernel.crashing.org> > Cc: Andrew Morton <akpm@linux-foundation.org> > Cc: Oscar Salvador <osalvador@suse.de> > Cc: Michal Hocko <mhocko@kernel.org> > Cc: Baoquan He <bhe@redhat.com> > Cc: Dan Williams <dan.j.williams@intel.com> > Cc: Wei Yang <richardw.yang@linux.intel.com> > Signed-off-by: David Hildenbrand <david@redhat.com> > --- > mm/memory_hotplug.c | 3 ++- > 1 file changed, 2 insertions(+), 1 deletion(-) > > diff --git a/mm/memory_hotplug.c b/mm/memory_hotplug.c > index 4a9b3f6c6b37..8fe7e32dad48 100644 > --- a/mm/memory_hotplug.c > +++ b/mm/memory_hotplug.c > @@ -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); Reviewed-by: Baoquan He <bhe@redhat.com> ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH v2 1/2] mm/memory_hotplug: simplify calculation of number of pages in __remove_pages() 2020-02-28 9:58 ` [PATCH v2 1/2] mm/memory_hotplug: simplify calculation of number of pages in __remove_pages() David Hildenbrand 2020-02-28 10:22 ` Baoquan He @ 2020-02-28 13:25 ` Wei Yang 2020-03-29 19:19 ` David Hildenbrand 2 siblings, 0 replies; 17+ messages in thread From: Wei Yang @ 2020-02-28 13:25 UTC (permalink / raw) To: David Hildenbrand Cc: linux-kernel, linux-mm, Segher Boessenkool, Andrew Morton, Oscar Salvador, Michal Hocko, Baoquan He, Dan Williams On Fri, Feb 28, 2020 at 10:58:18AM +0100, David Hildenbrand wrote: >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 <segher@kernel.crashing.org> >Cc: Andrew Morton <akpm@linux-foundation.org> >Cc: Oscar Salvador <osalvador@suse.de> >Cc: Michal Hocko <mhocko@kernel.org> >Cc: Baoquan He <bhe@redhat.com> >Cc: Dan Williams <dan.j.williams@intel.com> >Cc: Wei Yang <richardw.yang@linux.intel.com> >Signed-off-by: David Hildenbrand <david@redhat.com> Reviewed-by: Wei Yang <richard.weiyang@gmail.com> >--- > mm/memory_hotplug.c | 3 ++- > 1 file changed, 2 insertions(+), 1 deletion(-) > >diff --git a/mm/memory_hotplug.c b/mm/memory_hotplug.c >index 4a9b3f6c6b37..8fe7e32dad48 100644 >--- a/mm/memory_hotplug.c >+++ b/mm/memory_hotplug.c >@@ -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; > } >-- >2.24.1 -- Wei Yang Help you, Help me ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH v2 1/2] mm/memory_hotplug: simplify calculation of number of pages in __remove_pages() 2020-02-28 9:58 ` [PATCH v2 1/2] mm/memory_hotplug: simplify calculation of number of pages in __remove_pages() David Hildenbrand 2020-02-28 10:22 ` Baoquan He 2020-02-28 13:25 ` Wei Yang @ 2020-03-29 19:19 ` David Hildenbrand 2020-03-29 20:09 ` Linus Torvalds 2 siblings, 1 reply; 17+ messages in thread From: David Hildenbrand @ 2020-03-29 19:19 UTC (permalink / raw) To: linux-kernel Cc: linux-mm, Segher Boessenkool, Andrew Morton, Oscar Salvador, Michal Hocko, Baoquan He, Dan Williams, Wei Yang, Linus Torvalds On 28.02.20 10:58, David Hildenbrand wrote: > 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/20200228095819.10750-2-david@redhat.com/raw " ... fro= m" should simply be "... from" -- Thanks, David / dhildenb ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH v2 1/2] mm/memory_hotplug: simplify calculation of number of pages in __remove_pages() 2020-03-29 19:19 ` David Hildenbrand @ 2020-03-29 20:09 ` Linus Torvalds 2020-03-29 20:17 ` David Hildenbrand 2020-03-29 20:18 ` Linus Torvalds 0 siblings, 2 replies; 17+ messages in thread From: Linus Torvalds @ 2020-03-29 20:09 UTC (permalink / raw) To: David Hildenbrand Cc: Linux Kernel Mailing List, Linux-MM, Segher Boessenkool, Andrew Morton, Oscar Salvador, Michal Hocko, Baoquan He, Dan Williams, Wei Yang On Sun, Mar 29, 2020 at 12:19 PM David Hildenbrand <david@redhat.com> wrote: > > 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/20200228095819.10750-2-david@redhat.com/raw 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/20200228095819.10750-2-david@redhat.com/ 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%25akpm@linux-foundation.org/ 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%25akpm@linux-foundation.org/raw it has no content transfer encoding line in the headers. Linus ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH v2 1/2] mm/memory_hotplug: simplify calculation of number of pages in __remove_pages() 2020-03-29 20:09 ` Linus Torvalds @ 2020-03-29 20:17 ` David Hildenbrand 2020-03-29 20:26 ` Linus Torvalds 2020-03-29 20:18 ` Linus Torvalds 1 sibling, 1 reply; 17+ messages in thread From: David Hildenbrand @ 2020-03-29 20:17 UTC (permalink / raw) To: Linus Torvalds Cc: David Hildenbrand, Linux Kernel Mailing List, Linux-MM, Segher Boessenkool, Andrew Morton, Oscar Salvador, Michal Hocko, Baoquan He, Dan Williams, Wei Yang > Am 29.03.2020 um 22:10 schrieb Linus Torvalds <torvalds@linux-foundation.org>: > > On Sun, Mar 29, 2020 at 12:19 PM David Hildenbrand <david@redhat.com> wrote: >> >> 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/20200228095819.10750-2-david@redhat.com/raw > > 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/20200228095819.10750-2-david@redhat.com/ > > 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%25akpm@linux-foundation.org/ > > 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%25akpm@linux-foundation.org/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! > > Linus > ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH v2 1/2] mm/memory_hotplug: simplify calculation of number of pages in __remove_pages() 2020-03-29 20:17 ` David Hildenbrand @ 2020-03-29 20:26 ` Linus Torvalds 0 siblings, 0 replies; 17+ messages in thread From: Linus Torvalds @ 2020-03-29 20:26 UTC (permalink / raw) To: David Hildenbrand Cc: Linux Kernel Mailing List, Linux-MM, Segher Boessenkool, Andrew Morton, Oscar Salvador, Michal Hocko, Baoquan He, Dan Williams, Wei Yang On Sun, Mar 29, 2020 at 1:17 PM David Hildenbrand <david@redhat.com> wrote: > > 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 ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH v2 1/2] mm/memory_hotplug: simplify calculation of number of pages in __remove_pages() 2020-03-29 20:09 ` Linus Torvalds 2020-03-29 20:17 ` David Hildenbrand @ 2020-03-29 20:18 ` Linus Torvalds 2020-03-29 20:41 ` David Hildenbrand 1 sibling, 1 reply; 17+ messages in thread From: Linus Torvalds @ 2020-03-29 20:18 UTC (permalink / raw) To: David Hildenbrand Cc: Linux Kernel Mailing List, Linux-MM, Segher Boessenkool, Andrew Morton, Oscar Salvador, Michal Hocko, Baoquan He, Dan Williams, Wei Yang On Sun, Mar 29, 2020 at 1:09 PM Linus Torvalds <torvalds@linux-foundation.org> wrote: > > In contrast, look at the email that Andrew sent me and that I complained about: > > https://lore.kernel.org/linux-mm/20200329021719.MBKzW0xSl%25akpm@linux-foundation.org/ Hmm. I'm trying to figure out how and where Andrew got the original from you. There's https://lore.kernel.org/linux-mm/20200124155336.17126-1-david@redhat.com/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/20200128093542.6908-1-david@redhat.com/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 ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH v2 1/2] mm/memory_hotplug: simplify calculation of number of pages in __remove_pages() 2020-03-29 20:18 ` Linus Torvalds @ 2020-03-29 20:41 ` David Hildenbrand 2020-03-30 22:14 ` Andrew Morton 0 siblings, 1 reply; 17+ messages in thread From: David Hildenbrand @ 2020-03-29 20:41 UTC (permalink / raw) To: Linus Torvalds Cc: David Hildenbrand, Linux Kernel Mailing List, Linux-MM, Segher Boessenkool, Andrew Morton, Oscar Salvador, Michal Hocko, Baoquan He, Dan Williams, Wei Yang > Am 29.03.2020 um 22:19 schrieb Linus Torvalds <torvalds@linux-foundation.org>: > > On Sun, Mar 29, 2020 at 1:09 PM Linus Torvalds > <torvalds@linux-foundation.org> wrote: >> >> In contrast, look at the email that Andrew sent me and that I complained about: >> >> https://lore.kernel.org/linux-mm/20200329021719.MBKzW0xSl%25akpm@linux-foundation.org/ > > Hmm. I'm trying to figure out how and where Andrew got the original from you. > > There's > > https://lore.kernel.org/linux-mm/20200124155336.17126-1-david@redhat.com/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/20200128093542.6908-1-david@redhat.com/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. *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 ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH v2 1/2] mm/memory_hotplug: simplify calculation of number of pages in __remove_pages() 2020-03-29 20:41 ` David Hildenbrand @ 2020-03-30 22:14 ` Andrew Morton 2020-03-30 22:27 ` Linus Torvalds 0 siblings, 1 reply; 17+ messages in thread From: Andrew Morton @ 2020-03-30 22:14 UTC (permalink / raw) To: David Hildenbrand Cc: Linus Torvalds, Linux Kernel Mailing List, Linux-MM, Segher Boessenkool, Oscar Salvador, Michal Hocko, Baoquan He, Dan Williams, Wei Yang On Sun, 29 Mar 2020 22:41:18 +0200 David Hildenbrand <david@redhat.com> wrote: > > https://lore.kernel.org/linux-mm/20200128093542.6908-1-david@redhat.com/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. > > *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. ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH v2 1/2] mm/memory_hotplug: simplify calculation of number of pages in __remove_pages() 2020-03-30 22:14 ` Andrew Morton @ 2020-03-30 22:27 ` Linus Torvalds 0 siblings, 0 replies; 17+ messages in thread From: Linus Torvalds @ 2020-03-30 22:27 UTC (permalink / raw) To: Andrew Morton Cc: David Hildenbrand, Linux Kernel Mailing List, Linux-MM, Segher Boessenkool, Oscar Salvador, Michal Hocko, Baoquan He, Dan Williams, Wei Yang On Mon, Mar 30, 2020 at 3:14 PM Andrew Morton <akpm@linux-foundation.org> wrote: > > 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 ^ permalink raw reply [flat|nested] 17+ messages in thread
* [PATCH v2 2/2] mm/memory_hotplug: cleanup __add_pages() 2020-02-28 9:58 [PATCH v2 0/2] mm/memory_hotplug: __add_pages() and __remove_pages() cleanups David Hildenbrand 2020-02-28 9:58 ` [PATCH v2 1/2] mm/memory_hotplug: simplify calculation of number of pages in __remove_pages() David Hildenbrand @ 2020-02-28 9:58 ` David Hildenbrand 2020-02-28 10:34 ` Baoquan He 2020-02-28 13:26 ` Wei Yang 1 sibling, 2 replies; 17+ messages in thread From: David Hildenbrand @ 2020-02-28 9:58 UTC (permalink / raw) To: linux-kernel Cc: linux-mm, David Hildenbrand, Segher Boessenkool, Andrew Morton, Oscar Salvador, Michal Hocko, Baoquan He, Dan Williams, Wei Yang Let's drop the basically unused section stuff and simplify. The logic now matches the logic in __remove_pages(). Cc: Segher Boessenkool <segher@kernel.crashing.org> Cc: Andrew Morton <akpm@linux-foundation.org> Cc: Oscar Salvador <osalvador@suse.de> Cc: Michal Hocko <mhocko@kernel.org> Cc: Baoquan He <bhe@redhat.com> Cc: Dan Williams <dan.j.williams@intel.com> Cc: Wei Yang <richardw.yang@linux.intel.com> Signed-off-by: David Hildenbrand <david@redhat.com> --- mm/memory_hotplug.c | 18 +++++++----------- 1 file changed, 7 insertions(+), 11 deletions(-) diff --git a/mm/memory_hotplug.c b/mm/memory_hotplug.c index 8fe7e32dad48..1a00b5a37ef6 100644 --- a/mm/memory_hotplug.c +++ b/mm/memory_hotplug.c @@ -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(); -- 2.24.1 ^ permalink raw reply related [flat|nested] 17+ messages in thread
* Re: [PATCH v2 2/2] mm/memory_hotplug: cleanup __add_pages() 2020-02-28 9:58 ` [PATCH v2 2/2] mm/memory_hotplug: cleanup __add_pages() David Hildenbrand @ 2020-02-28 10:34 ` Baoquan He 2020-02-28 11:14 ` David Hildenbrand 2020-02-28 13:26 ` Wei Yang 1 sibling, 1 reply; 17+ messages in thread From: Baoquan He @ 2020-02-28 10:34 UTC (permalink / raw) To: David Hildenbrand Cc: linux-kernel, linux-mm, Segher Boessenkool, Andrew Morton, Oscar Salvador, Michal Hocko, Dan Williams, Wei Yang On 02/28/20 at 10:58am, David Hildenbrand wrote: > Let's drop the basically unused section stuff and simplify. The logic > now matches the logic in __remove_pages(). > > Cc: Segher Boessenkool <segher@kernel.crashing.org> > Cc: Andrew Morton <akpm@linux-foundation.org> > Cc: Oscar Salvador <osalvador@suse.de> > Cc: Michal Hocko <mhocko@kernel.org> > Cc: Baoquan He <bhe@redhat.com> > Cc: Dan Williams <dan.j.williams@intel.com> > Cc: Wei Yang <richardw.yang@linux.intel.com> > Signed-off-by: David Hildenbrand <david@redhat.com> > --- > mm/memory_hotplug.c | 18 +++++++----------- > 1 file changed, 7 insertions(+), 11 deletions(-) > > diff --git a/mm/memory_hotplug.c b/mm/memory_hotplug.c > index 8fe7e32dad48..1a00b5a37ef6 100644 > --- a/mm/memory_hotplug.c > +++ b/mm/memory_hotplug.c > @@ -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); 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. Reviewed-by: Baoquan He <bhe@redhat.com> > if (err) > break; > - pfn += pfns; > - nr_pages -= pfns; > cond_resched(); > } > vmemmap_populate_print_last(); > -- > 2.24.1 > ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH v2 2/2] mm/memory_hotplug: cleanup __add_pages() 2020-02-28 10:34 ` Baoquan He @ 2020-02-28 11:14 ` David Hildenbrand 2020-03-01 5:39 ` Baoquan He 0 siblings, 1 reply; 17+ messages in thread From: David Hildenbrand @ 2020-02-28 11:14 UTC (permalink / raw) To: Baoquan He Cc: linux-kernel, linux-mm, Segher Boessenkool, Andrew Morton, Oscar Salvador, Michal Hocko, Dan Williams, Wei Yang On 28.02.20 11:34, Baoquan He wrote: > On 02/28/20 at 10:58am, David Hildenbrand wrote: >> Let's drop the basically unused section stuff and simplify. The logic >> now matches the logic in __remove_pages(). >> >> Cc: Segher Boessenkool <segher@kernel.crashing.org> >> Cc: Andrew Morton <akpm@linux-foundation.org> >> Cc: Oscar Salvador <osalvador@suse.de> >> Cc: Michal Hocko <mhocko@kernel.org> >> Cc: Baoquan He <bhe@redhat.com> >> Cc: Dan Williams <dan.j.williams@intel.com> >> Cc: Wei Yang <richardw.yang@linux.intel.com> >> Signed-off-by: David Hildenbrand <david@redhat.com> >> --- >> mm/memory_hotplug.c | 18 +++++++----------- >> 1 file changed, 7 insertions(+), 11 deletions(-) >> >> diff --git a/mm/memory_hotplug.c b/mm/memory_hotplug.c >> index 8fe7e32dad48..1a00b5a37ef6 100644 >> --- a/mm/memory_hotplug.c >> +++ b/mm/memory_hotplug.c >> @@ -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); > > 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. > 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! -- Thanks, David / dhildenb ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH v2 2/2] mm/memory_hotplug: cleanup __add_pages() 2020-02-28 11:14 ` David Hildenbrand @ 2020-03-01 5:39 ` Baoquan He 0 siblings, 0 replies; 17+ messages in thread From: Baoquan He @ 2020-03-01 5:39 UTC (permalink / raw) To: David Hildenbrand Cc: linux-kernel, linux-mm, Segher Boessenkool, Andrew Morton, Oscar Salvador, Michal Hocko, Dan Williams, Wei Yang On 02/28/20 at 12:14pm, David Hildenbrand wrote: > On 28.02.20 11:34, Baoquan He wrote: > > On 02/28/20 at 10:58am, David Hildenbrand wrote: > >> Let's drop the basically unused section stuff and simplify. The logic > >> now matches the logic in __remove_pages(). > >> > >> Cc: Segher Boessenkool <segher@kernel.crashing.org> > >> Cc: Andrew Morton <akpm@linux-foundation.org> > >> Cc: Oscar Salvador <osalvador@suse.de> > >> Cc: Michal Hocko <mhocko@kernel.org> > >> Cc: Baoquan He <bhe@redhat.com> > >> Cc: Dan Williams <dan.j.williams@intel.com> > >> Cc: Wei Yang <richardw.yang@linux.intel.com> > >> Signed-off-by: David Hildenbrand <david@redhat.com> > >> --- > >> mm/memory_hotplug.c | 18 +++++++----------- > >> 1 file changed, 7 insertions(+), 11 deletions(-) > >> > >> diff --git a/mm/memory_hotplug.c b/mm/memory_hotplug.c > >> index 8fe7e32dad48..1a00b5a37ef6 100644 > >> --- a/mm/memory_hotplug.c > >> +++ b/mm/memory_hotplug.c > >> @@ -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); > > > > 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 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. > sub-section hotadd support. > > > 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! > > > -- > Thanks, > > David / dhildenb ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH v2 2/2] mm/memory_hotplug: cleanup __add_pages() 2020-02-28 9:58 ` [PATCH v2 2/2] mm/memory_hotplug: cleanup __add_pages() David Hildenbrand 2020-02-28 10:34 ` Baoquan He @ 2020-02-28 13:26 ` Wei Yang 1 sibling, 0 replies; 17+ messages in thread From: Wei Yang @ 2020-02-28 13:26 UTC (permalink / raw) To: David Hildenbrand Cc: linux-kernel, linux-mm, Segher Boessenkool, Andrew Morton, Oscar Salvador, Michal Hocko, Baoquan He, Dan Williams, Wei Yang On Fri, Feb 28, 2020 at 10:58:19AM +0100, David Hildenbrand wrote: >Let's drop the basically unused section stuff and simplify. The logic >now matches the logic in __remove_pages(). > >Cc: Segher Boessenkool <segher@kernel.crashing.org> >Cc: Andrew Morton <akpm@linux-foundation.org> >Cc: Oscar Salvador <osalvador@suse.de> >Cc: Michal Hocko <mhocko@kernel.org> >Cc: Baoquan He <bhe@redhat.com> >Cc: Dan Williams <dan.j.williams@intel.com> >Cc: Wei Yang <richardw.yang@linux.intel.com> >Signed-off-by: David Hildenbrand <david@redhat.com> Reviewed-by: Wei Yang <richard.weiyang@gmail.com> >--- > mm/memory_hotplug.c | 18 +++++++----------- > 1 file changed, 7 insertions(+), 11 deletions(-) > >diff --git a/mm/memory_hotplug.c b/mm/memory_hotplug.c >index 8fe7e32dad48..1a00b5a37ef6 100644 >--- a/mm/memory_hotplug.c >+++ b/mm/memory_hotplug.c >@@ -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(); >-- >2.24.1 -- Wei Yang Help you, Help me ^ permalink raw reply [flat|nested] 17+ messages in thread
end of thread, other threads:[~2020-03-30 22:28 UTC | newest] Thread overview: 17+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2020-02-28 9:58 [PATCH v2 0/2] mm/memory_hotplug: __add_pages() and __remove_pages() cleanups David Hildenbrand 2020-02-28 9:58 ` [PATCH v2 1/2] mm/memory_hotplug: simplify calculation of number of pages in __remove_pages() David Hildenbrand 2020-02-28 10:22 ` Baoquan He 2020-02-28 13:25 ` Wei Yang 2020-03-29 19:19 ` David Hildenbrand 2020-03-29 20:09 ` Linus Torvalds 2020-03-29 20:17 ` David Hildenbrand 2020-03-29 20:26 ` Linus Torvalds 2020-03-29 20:18 ` Linus Torvalds 2020-03-29 20:41 ` David Hildenbrand 2020-03-30 22:14 ` Andrew Morton 2020-03-30 22:27 ` Linus Torvalds 2020-02-28 9:58 ` [PATCH v2 2/2] mm/memory_hotplug: cleanup __add_pages() David Hildenbrand 2020-02-28 10:34 ` Baoquan He 2020-02-28 11:14 ` David Hildenbrand 2020-03-01 5:39 ` Baoquan He 2020-02-28 13:26 ` Wei Yang
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).