[06/67] mm: factor out next_present_section_nr()

Commit Message

Andrew Morton Feb. 4, 2020, 1:34 a.m. UTC

From: David Hildenbrand <[e-mail]>
Subject: mm: factor out next_present_section_nr()

Let's move it to the header and use the shorter variant from
mm/page_alloc.c (the original one will also check
"__highest_present_section_nr + 1", which is not necessary).  While at it,
make the section_nr in next_pfn() const.

In next_pfn(), we now return section_nr_to_pfn(-1) instead of -1 once we
exceed __highest_present_section_nr, which doesn't make a difference i= n
the caller as it is big enough (>=3D all sane end_pfn).

Link: http://lkml.kernel.org/r/[e-mail]
Signed-off-by: David Hildenbrand <[e-mail]>
Acked-by: Kirill A. Shutemov <[e-mail]>
Cc: Baoquan He <[e-mail]>
Cc: Dan Williams <[e-mail]>
Cc: "Jin, Zhi" <[e-mail]>
Cc: "Kirill A. Shutemov" <[e-mail]>
Cc: Mel Gorman <[e-mail]>
Cc: Michal Hocko <[e-mail]>
Cc: Michal Hocko <[e-mail]>
Cc: Oscar Salvador <[e-mail]>
Cc: Pavel Tatashin <[e-mail]>
Cc: Vlastimil Babka <[e-mail]>
Signed-off-by: Andrew Morton <[e-mail]>
---

 include/linux/mmzone.h |   10 ++++++++++
 mm/page_alloc.c        |   11 ++---------
 mm/sparse.c            |   10 ----------
 3 files changed, 12 insertions(+), 19 deletions(-)

Comments

Linus Torvalds Feb. 4, 2020, 3:04 a.m. UTC | #1

More signs of your - or somebody elses - email scripts being broken:

On Tue, Feb 4, 2020 at 1:34 AM Andrew Morton <[e-mail]> wrote:
>
> From: David Hildenbrand <[e-mail]>
> Subject: mm: factor out next_present_section_nr()
>
> Let's move it to the header and use the shorter variant from
> mm/page_alloc.c (the original one will also check
> "__highest_present_section_nr + 1", which is not necessary).  While at it,
> make the section_nr in next_pfn() const.
>
> In next_pfn(), we now return section_nr_to_pfn(-1) instead of -1 once we
> exceed __highest_present_section_nr, which doesn't make a difference i= n
> the caller as it is big enough (>=3D all sane end_pfn).

Here, look at that "i= n". It looks like it was a MIME line-break (so
"in" was MIME-encoded and turned into "i=\nn") followed by you or
David re-flowing the text without MIME-decoding it.

And note the ">=3D" thing. It should be just ">=" but again there is
left-over crud from using MIME-encoded data without decoding it.

I am noticing that this has apparently happened before too. And maybe
it's not you. Maybe it's David Hildenbrand that has sent you already
corrupted data.

Doing a

    git log --grep="=3D"

shows that this mistake has been done by others before. But it's not
_that_ common. I find 25 occurrences of that "=3D" thing in the logs
over the whole history of the kernel.

The "=\n" thing is much harder to grep for quite that trivially, or
the other "random utf8 encoded as MIME and never decoded properly"
stuff.

But the fact that I found at least _two_ of these cases in just this
series, and it had that broken coverletter too, makes me go "Hmm".

I tried to look for other cases, but those two emails (both from David
Hildenbrand, soo...) were the only ones I found in this series of 67.

I can fix them up, of course, but I really hate how somebody has some
workflow that generates this corruption.

MIME corruption in the patches themselves tends to be much more
obvious ("it doesn't apply" or "it no longer builds"). But in the
commit message it's not always clear. So please be *careful*.

I'm jetlagged in Cambridge UK and was going to apply this series since
I was awake anyway, but now I'm not sure I should do that.

At a minimum I want to know what the base commit was supposed to be..

             Linus

Andrew Morton Feb. 4, 2020, 4:29 a.m. UTC | #2

On Tue, 4 Feb 2020 03:04:40 +0000 Linus Torvalds <[e-mail]> wrote:

> On Tue, Feb 4, 2020 at 1:34 AM Andrew Morton <[e-mail]> wrote:
> >
> > From: David Hildenbrand <[e-mail]>
> > Subject: mm: factor out next_present_section_nr()
> >
> > Let's move it to the header and use the shorter variant from
> > mm/page_alloc.c (the original one will also check
> > "__highest_present_section_nr + 1", which is not necessary).  While at it,
> > make the section_nr in next_pfn() const.
> >
> > In next_pfn(), we now return section_nr_to_pfn(-1) instead of -1 once we
> > exceed __highest_present_section_nr, which doesn't make a difference i= n
> > the caller as it is big enough (>=3D all sane end_pfn).
> 
> Here, look at that "i= n". It looks like it was a MIME line-break (so
> "in" was MIME-encoded and turned into "i=\nn") followed by you or
> David re-flowing the text without MIME-decoding it.

Yes, my MUA is not good about converting all input.  I have tools to fix
up the patches post-facto but changelogs have been a challenge.

I have updated my nightly check-patches-for-crap-and-email-it-to-andrew
script to check for this.

David Hildenbrand Feb. 4, 2020, 8:22 a.m. UTC | #3

On 04.02.20 04:04, Linus Torvalds wrote:
> More signs of your - or somebody elses - email scripts being broken:
> 
> On Tue, Feb 4, 2020 at 1:34 AM Andrew Morton <[e-mail]> wrote:
>>
>> From: David Hildenbrand <[e-mail]>
>> Subject: mm: factor out next_present_section_nr()
>>
>> Let's move it to the header and use the shorter variant from
>> mm/page_alloc.c (the original one will also check
>> "__highest_present_section_nr + 1", which is not necessary).  While at it,
>> make the section_nr in next_pfn() const.
>>
>> In next_pfn(), we now return section_nr_to_pfn(-1) instead of -1 once we
>> exceed __highest_present_section_nr, which doesn't make a difference i= n
>> the caller as it is big enough (>=3D all sane end_pfn).
> 
> Here, look at that "i= n". It looks like it was a MIME line-break (so
> "in" was MIME-encoded and turned into "i=\nn") followed by you or
> David re-flowing the text without MIME-decoding it.
> 
> And note the ">=3D" thing. It should be just ">=" but again there is
> left-over crud from using MIME-encoded data without decoding it.
> 
> I am noticing that this has apparently happened before too. And maybe
> it's not you. Maybe it's David Hildenbrand that has sent you already
> corrupted data.
> 
> Doing a
> 
>     git log --grep="=3D"
> 
> shows that this mistake has been done by others before. But it's not
> _that_ common. I find 25 occurrences of that "=3D" thing in the logs
> over the whole history of the kernel.
> 

Easter eggs :)

> The "=\n" thing is much harder to grep for quite that trivially, or
> the other "random utf8 encoded as MIME and never decoded properly"
> stuff.
> 
> But the fact that I found at least _two_ of these cases in just this
> series, and it had that broken coverletter too, makes me go "Hmm".
> 
> I tried to look for other cases, but those two emails (both from David
> Hildenbrand, soo...) were the only ones I found in this series of 67.
> 
> I can fix them up, of course, but I really hate how somebody has some
> workflow that generates this corruption.

I'm a simple man - I use vim+git with barely any custom scripts (well,
only a cc-cmd script shared my Michal once). I've been sending patches
from the same machine for years without any such glitches.

The only thing I do manually is copying the cover letter from
Thunderbird + adapting it when resending a version. But as the patch
descriptions themselves are messed up, that doesn't explain the story.

I *suspect* this is the result of (one of many) temporary mail server
issues we had at Red Hat when switching providers (although a weird one
- but I remember there were weird ones).

Patch

--- a/include/linux/mmzone.h~mm-factor-out-next_present_section_nr
+++ a/include/linux/mmzone.h
@@ -1379,6 +1379,16 @@  static inline int pfn_present(unsigned l
 	return present_section(__nr_to_section(pfn_to_section_nr(pfn)));
 }
 
+static inline unsigned long next_present_section_nr(unsigned long section_nr)
+{
+	while (++section_nr <= __highest_present_section_nr) {
+		if (present_section_nr(section_nr))
+			return section_nr;
+	}
+
+	return -1;
+}
+
 /*
  * These are _only_ used during initialisation, therefore they
  * can use __initdata ...  They could have names to indicate
--- a/mm/page_alloc.c~mm-factor-out-next_present_section_nr
+++ a/mm/page_alloc.c
@@ -5852,18 +5852,11 @@  overlap_memmap_init(unsigned long zone,
 /* Skip PFNs that belong to non-present sections */
 static inline __meminit unsigned long next_pfn(unsigned long pfn)
 {
-	unsigned long section_nr;
+	const unsigned long section_nr = pfn_to_section_nr(++pfn);
 
-	section_nr = pfn_to_section_nr(++pfn);
 	if (present_section_nr(section_nr))
 		return pfn;
-
-	while (++section_nr <= __highest_present_section_nr) {
-		if (present_section_nr(section_nr))
-			return section_nr_to_pfn(section_nr);
-	}
-
-	return -1;
+	return section_nr_to_pfn(next_present_section_nr(section_nr));
 }
 #else
 static inline __meminit unsigned long next_pfn(unsigned long pfn)
--- a/mm/sparse.c~mm-factor-out-next_present_section_nr
+++ a/mm/sparse.c
@@ -198,16 +198,6 @@  static void section_mark_present(struct
 	ms->section_mem_map |= SECTION_MARKED_PRESENT;
 }
 
-static inline unsigned long next_present_section_nr(unsigned long section_nr)
-{
-	do {
-		section_nr++;
-		if (present_section_nr(section_nr))
-			return section_nr;
-	} while ((section_nr <= __highest_present_section_nr));
-
-	return -1;
-}
 #define for_each_present_section_nr(start, section_nr)		\
 	for (section_nr = next_present_section_nr(start-1);	\
 	     ((section_nr != -1) &&				\