diff mbox

mmotm 2018-02-21-14-48 uploaded (mm/page_alloc.c on UML)

Message ID 20180222072037.GC30681@dhcp22.suse.cz (mailing list archive)
State New, archived
Headers show

Commit Message

Michal Hocko Feb. 22, 2018, 7:20 a.m. UTC
On Wed 21-02-18 15:58:41, Randy Dunlap wrote:
> On 02/21/2018 02:48 PM, akpm@linux-foundation.org wrote:
> > The mm-of-the-moment snapshot 2018-02-21-14-48 has been uploaded to
> > 
> >    http://www.ozlabs.org/~akpm/mmotm/
> > 
> > mmotm-readme.txt says
> > 
> > README for mm-of-the-moment:
> > 
> > http://www.ozlabs.org/~akpm/mmotm/
> > 
> > This is a snapshot of my -mm patch queue.  Uploaded at random hopefully
> > more than once a week.
> > 
> > You will need quilt to apply these patches to the latest Linus release (4.x
> > or 4.x-rcY).  The series file is in broken-out.tar.gz and is duplicated in
> > http://ozlabs.org/~akpm/mmotm/series
> > 
> > The file broken-out.tar.gz contains two datestamp files: .DATE and
> > .DATE-yyyy-mm-dd-hh-mm-ss.  Both contain the string yyyy-mm-dd-hh-mm-ss,
> > followed by the base kernel version against which this patch series is to
> > be applied.
> 
> um (or uml) defconfig on i386 and/or x86_64:
> 
> ../mm/page_alloc.c: In function 'memmap_init_zone':
> ../mm/page_alloc.c:5450:5: error: implicit declaration of function 'memblock_next_valid_pfn' [-Werror=implicit-function-declaration]
>      pfn = memblock_next_valid_pfn(pfn, end_pfn) - 1;
>      ^
> 
> 
> probably (?):
> From: Eugeniu Rosca <erosca@de.adit-jv.com>
> Subject: mm: page_alloc: skip over regions of invalid pfns on UMA

Yes. Steven has already reported the same [1]. There are two possible
ways around this. Either provide and empty stub or use ifdef around
memblock_next_valid_pfn. I would use the later because it is less
confusing. We really do not want memblock_next_valid_pfn to be used
outside of memblock aware code.

[1] http://lkml.kernel.org/r/20180222143057.3a1b3746@canb.auug.org.au

Comments

Eugeniu Rosca Feb. 22, 2018, 10:38 a.m. UTC | #1
Hi Michal,

Please, let me know if any action is expected from my end.
Thank you for your support and sorry for the ifdef troubles.

Best regards,
Eugeniu.

On Thu, Feb 22, 2018 at 08:20:37AM +0100, Michal Hocko wrote:
> On Wed 21-02-18 15:58:41, Randy Dunlap wrote:
> > On 02/21/2018 02:48 PM, akpm@linux-foundation.org wrote:
> > > The mm-of-the-moment snapshot 2018-02-21-14-48 has been uploaded to
> > > 
> > >    http://www.ozlabs.org/~akpm/mmotm/
> > > 
> > > mmotm-readme.txt says
> > > 
> > > README for mm-of-the-moment:
> > > 
> > > http://www.ozlabs.org/~akpm/mmotm/
> > > 
> > > This is a snapshot of my -mm patch queue.  Uploaded at random hopefully
> > > more than once a week.
> > > 
> > > You will need quilt to apply these patches to the latest Linus release (4.x
> > > or 4.x-rcY).  The series file is in broken-out.tar.gz and is duplicated in
> > > http://ozlabs.org/~akpm/mmotm/series
> > > 
> > > The file broken-out.tar.gz contains two datestamp files: .DATE and
> > > .DATE-yyyy-mm-dd-hh-mm-ss.  Both contain the string yyyy-mm-dd-hh-mm-ss,
> > > followed by the base kernel version against which this patch series is to
> > > be applied.
> > 
> > um (or uml) defconfig on i386 and/or x86_64:
> > 
> > ../mm/page_alloc.c: In function 'memmap_init_zone':
> > ../mm/page_alloc.c:5450:5: error: implicit declaration of function 'memblock_next_valid_pfn' [-Werror=implicit-function-declaration]
> >      pfn = memblock_next_valid_pfn(pfn, end_pfn) - 1;
> >      ^
> > 
> > 
> > probably (?):
> > From: Eugeniu Rosca <erosca@de.adit-jv.com>
> > Subject: mm: page_alloc: skip over regions of invalid pfns on UMA
> 
> Yes. Steven has already reported the same [1]. There are two possible
> ways around this. Either provide and empty stub or use ifdef around
> memblock_next_valid_pfn. I would use the later because it is less
> confusing. We really do not want memblock_next_valid_pfn to be used
> outside of memblock aware code.
> 
> [1] http://lkml.kernel.org/r/20180222143057.3a1b3746@canb.auug.org.au
> 
> 
> diff --git a/mm/page_alloc.c b/mm/page_alloc.c
> index 4334d3a9c6a2..2836bc9e0999 100644
> --- a/mm/page_alloc.c
> +++ b/mm/page_alloc.c
> @@ -5446,8 +5446,9 @@ void __meminit memmap_init_zone(unsigned long size, int nid, unsigned long zone,
>  			 * end_pfn), such that we hit a valid pfn (or end_pfn)
>  			 * on our next iteration of the loop.
>  			 */
> -			if (IS_ENABLED(CONFIG_HAVE_MEMBLOCK))
> -				pfn = memblock_next_valid_pfn(pfn, end_pfn) - 1;
> +#ifdef CONFIG_HAVE_MEMBLOCK
> +			pfn = memblock_next_valid_pfn(pfn, end_pfn) - 1;
> +#endif
>  			continue;
>  		}
>  		if (!early_pfn_in_nid(pfn, nid))
> -- 
> Michal Hocko
> SUSE Labs
Michal Hocko Feb. 22, 2018, 12:59 p.m. UTC | #2
On Thu 22-02-18 11:38:32, Eugeniu Rosca wrote:
> Hi Michal,
> 
> Please, let me know if any action is expected from my end.

I do not thing anything is really needed right now. If you have a strong
opinion about the solution (ifdef vs. noop stub) then speak up.

> Thank you for your support and sorry for the ifdef troubles.

No troubles at all. It was me who pushed you this direction...
Eugeniu Rosca Feb. 22, 2018, 1:08 p.m. UTC | #3
On Thu, Feb 22, 2018 at 01:59:55PM +0100, Michal Hocko wrote:
> On Thu 22-02-18 11:38:32, Eugeniu Rosca wrote:
> > Hi Michal,
> > 
> > Please, let me know if any action is expected from my end.
> 
> I do not thing anything is really needed right now. If you have a strong
> opinion about the solution (ifdef vs. noop stub) then speak up.

No different preference on my side. I was more thinking if you are going
to amend the patch or create a fix on top of it. Since it didn't reach
mainline, it makes sense to amend it. If you can do it without the
intervention of the author, that's also fine for me.

> > Thank you for your support and sorry for the ifdef troubles.
> 
> No troubles at all. It was me who pushed you this direction...
> -- 
> Michal Hocko
> SUSE Labs

Thanks,
Eugeniu.
Michal Hocko Feb. 22, 2018, 1:26 p.m. UTC | #4
On Thu 22-02-18 14:08:14, Eugeniu Rosca wrote:
> On Thu, Feb 22, 2018 at 01:59:55PM +0100, Michal Hocko wrote:
> > On Thu 22-02-18 11:38:32, Eugeniu Rosca wrote:
> > > Hi Michal,
> > > 
> > > Please, let me know if any action is expected from my end.
> > 
> > I do not thing anything is really needed right now. If you have a strong
> > opinion about the solution (ifdef vs. noop stub) then speak up.
> 
> No different preference on my side. I was more thinking if you are going
> to amend the patch or create a fix on top of it. Since it didn't reach
> mainline, it makes sense to amend it. If you can do it without the
> intervention of the author, that's also fine for me.

Andrew usually takes the incremental fix and then squash them when
sending to Linus
diff mbox

Patch

diff --git a/mm/page_alloc.c b/mm/page_alloc.c
index 4334d3a9c6a2..2836bc9e0999 100644
--- a/mm/page_alloc.c
+++ b/mm/page_alloc.c
@@ -5446,8 +5446,9 @@  void __meminit memmap_init_zone(unsigned long size, int nid, unsigned long zone,
 			 * end_pfn), such that we hit a valid pfn (or end_pfn)
 			 * on our next iteration of the loop.
 			 */
-			if (IS_ENABLED(CONFIG_HAVE_MEMBLOCK))
-				pfn = memblock_next_valid_pfn(pfn, end_pfn) - 1;
+#ifdef CONFIG_HAVE_MEMBLOCK
+			pfn = memblock_next_valid_pfn(pfn, end_pfn) - 1;
+#endif
 			continue;
 		}
 		if (!early_pfn_in_nid(pfn, nid))