Re: [PATCH v2 17/20] mm: free_area_init: allow defining max_zone_pfn in descending order

Re: [PATCH v2 17/20] mm: free_area_init: allow defining max_zone_pfn in descending order

[Vineet Gupta]

SGkgTWlrZSwNCg0KT24gNS80LzIwIDg6MzkgQU0sIE1pa2UgUmFwb3BvcnQgd3JvdGU6DQo+IE9u
IFN1biwgTWF5IDAzLCAyMDIwIGF0IDExOjQzOjAwQU0gLTA3MDAsIEd1ZW50ZXIgUm9lY2sgd3Jv
dGU6DQo+PiBPbiBTdW4sIE1heSAwMywgMjAyMCBhdCAxMDo0MTozOEFNIC0wNzAwLCBHdWVudGVy
IFJvZWNrIHdyb3RlOg0KPj4+IEhpLA0KPj4+DQo+Pj4gT24gV2VkLCBBcHIgMjksIDIwMjAgYXQg
MDM6MTE6MjNQTSArMDMwMCwgTWlrZSBSYXBvcG9ydCB3cm90ZToNCj4+Pj4gRnJvbTogTWlrZSBS
YXBvcG9ydCA8cnBwdEBsaW51eC5pYm0uY29tPg0KPj4+Pg0KPj4+PiBTb21lIGFyY2hpdGVjdHVy
ZXMgKGUuZy4gQVJDKSBoYXZlIHRoZSBaT05FX0hJR0hNRU0gem9uZSBiZWxvdyB0aGUNCj4+Pj4g
Wk9ORV9OT1JNQUwuIEFsbG93aW5nIGZyZWVfYXJlYV9pbml0KCkgcGFyc2UgbWF4X3pvbmVfcGZu
IGFycmF5IGV2ZW4gaXQgaXMNCj4+Pj4gc29ydGVkIGluIGRlc2NlbmRpbmcgb3JkZXIgYWxsb3dz
IHVzaW5nIGZyZWVfYXJlYV9pbml0KCkgb24gc3VjaA0KPj4+PiBhcmNoaXRlY3R1cmVzLg0KPj4+
Pg0KPj4+PiBBZGQgdG9wIC0+IGRvd24gdHJhdmVyc2FsIG9mIG1heF96b25lX3BmbiBhcnJheSBp
biBmcmVlX2FyZWFfaW5pdCgpIGFuZCB1c2UNCj4+Pj4gdGhlIGxhdHRlciBpbiBBUkMgbm9kZS96
b25lIGluaXRpYWxpemF0aW9uLg0KPj4+Pg0KPj4+PiBTaWduZWQtb2ZmLWJ5OiBNaWtlIFJhcG9w
b3J0IDxycHB0QGxpbnV4LmlibS5jb20+DQo+Pj4NCj4+PiBUaGlzIHBhdGNoIGNhdXNlcyBteSBt
aWNyb2JsYXplZWwgcWVtdSBib290IHRlc3QgaW4gbGludXgtbmV4dCB0byBmYWlsLg0KPj4+IFJl
dmVydGluZyBpdCBmaXhlcyB0aGUgcHJvYmxlbS4NCj4+Pg0KPj4gVGhlIHNhbWUgcHJvYmxlbSBp
cyBzZWVuIHdpdGggczM5MCBlbXVsYXRpb25zLg0KPiANCj4gWWVhaCwgdGhpcyBwYXRjaCBicmVh
a3Mgc29tZSBvdGhlcnMgYXMgd2VsbCA6KA0KPiANCj4gTXkgYXNzdW1wdGlvbiB0aGF0IG1heF96
b25lX3BmbiBkZWZpbmVzIGFyY2hpdGVjdHVyYWwgbGltaXQgZm9yIG1heGltYWwNCj4gUEZOIHRo
YXQgY2FuIGJlbG9uZyB0byBhIHpvbmUgd2FzIG92ZXItb3B0aW1pc3RpYy4gU2V2ZXJhbCBhcmNo
ZXMNCj4gYWN0dWFsbHkgZG8gdGhhdCwgYnV0IG90aGVycyBkbw0KPiANCj4gCW1heF96b25lX3Bm
bltaT05FX0RNQV0gPSBNQVhfRE1BX1BGTjsNCj4gCW1heF96b25lX3BmbltaT05FX05PUk1BTF0g
PSBtYXhfcGZuOw0KPiANCj4gd2hlcmUgTUFYX0RNQV9QRk4gaXMgYnVpbGQtdGltZSBjb25zdHJh
aW4gYW5kIG1heF9wZm4gaXMgcnVuIHRpbWUgbGltaXQNCj4gZm9yIHRoZSBjdXJyZW50IHN5c3Rl
bS4NCj4gDQo+IFNvLCB3aGVuIG1heF9wZm4gaXMgbG93ZXIgdGhhbiBNQVhfRE1BX1BGTiwgdGhl
IGZyZWVfaW5pdF9hcmVhKCkgd2lsbA0KPiBjb25zaWRlciBtYXhfem9uZV9wZm4gYXMgZGVzY2Vu
ZGluZyBhbmQgd2lsbCB3cm9uZ2x5IGNhbGN1bGF0ZSB6b25lDQo+IGV4dGVudHMuDQo+IA0KPiBU
aGF0IHNhaWQsIGluc3RlYWQgb2YgdHJ5aW5nIHRvIGNyZWF0ZSBhIGdlbmVyaWMgd2F5IHRvIHNw
ZWNpYWwgY2FzZQ0KPiBBUkMsIEkgc3VnZ2VzdCB0byBzaW1wbHkgdXNlIHRoZSBiZWxvdyBwYXRj
aCBpbnN0ZWFkLg0KDQpFdmVuIGZvciBBUkMgaXQgd2lsbCBiZSBhIGJpdCBtb3JlIGNvbXBsaWNh
dGVkLiBIaWdobWVtIG9uIEFSQyBjYW4gYmUgc2V0dXAgaW4gMg0Kd2F5cyBzdWNoIHRoYXQgaXQg
aXMgZGVzY2VuZGluZyBpbiBvbmUgY2FzZSwgYW5kIGFzY2VuZGluZyBpbiBvdGhlciAody5yLnQN
CiJub3JtYWwiIG1lbSkgOi0oDQoNCkZpcnN0IHNvbWUgYmFzaWMgaW5mbyBhYm91dCBhbiBBUkMg
TU1VIGJhc2VkIHN5c3RlbQ0KDQpBUkMgbG9naWNhbCBhZGRyZXNzIHNwYWNlICh2YXJpb3VzIGFk
ZHJlc3NlcyBlbWJlZGRlZCBpbiBiaW5hcmllcykNCiAtIHRyYW5zbGF0ZWQgKDAgdG8gMHg2RkZG
X0ZGRkYpICAtIGZvciB1c2Vyc3BhY2UNCiAtIHVudHJhbnNsYXRlZCAoMHg4MDAwXzAwMDAgdG8g
MHhGRkZGX0ZGRkYpIC0ga2VybmVsDQoNCkFSQyBQaHlzaWNhbCBhZGRyZXNzIHNwYWNlIGlzIHR5
cGljYWxseSBmcm9tIDB4ODAwMF8wMDAwIHRvIDB4RjAwMF8wMDAwLg0KQWJvdmUgdHJhbnNsYXRl
ZCBzcGFjZSBtYXBzIGhlcmUgdmlhIE1NVS4gVW50cmFuc2xhdGVkIGlzIGltcGxpY2l0bHkgbWFw
cGVkIChubw0KTU1VIGludm9sdmVkKS4NCg0KVGhlIHBoeXNpY2FsIGFkZHJlc3MgaW4gdHVybiBt
YXBzIHRvIGEgQnVzIGFkZHJlc3MgLyBtZW1vcnkgKGRvbmUgYXQgdGhlDQppbnRlci1jb25uZWN0
L05vQykuIFR5cGljYWxseSBQaHlzaWNhbCAweDgwMDBfMDAwMCBtYXAgdG8gRERSIDANCg0KTm93
LA0KLSBISUdITUVNIHcvbyBQQUU0MCBhZGRzIFBoeXNpY2FsIGFkZHJlc3Mgc3BhY2UgMCB0byAw
eDdGRkZfRkZGRi4NCi0gSElHSE1FTSB3aXRoIFBBRTQwIHVzZXMgcGh5c2ljYWwgYWRkcmVzcyBz
cGFjZSBmcm9tIDB4MV8wMDAwXzAwMDAgdXB3YXJkcy4NCg0KQnV0IHRoZW4geW91IGNvdWxkIGFs
c28gaGF2ZSBhIHN5c3RlbSB3aGljaCBoYXMgYm90aCBvZiBhYm92ZSBzbyB0aGUgYmltb2RhbCB1
cC9kbg0Kd29uJ3Qgd29yay4NCg0KV2hpbGUgSSBhcHByZWNpYXRlIHRoZSBlZmZvcnQgdG8gcmVk
dWNlIGNvbXBsZXhpdHksIGl0IHNlZW1zIHRoZSBjdXJyZW50IHdheSBvZg0Kc2V0dGluZyB0aGlu
Z3MgdXAgYWxsb3dzIGZvciBtb3JlIGZsZXhpYmlsaXR5IGluIHNwZWNpZnlpbmcgdGhlIHN5c3Rl
bSBtZW1vcnkgbWFwLg0KDQpQUzogSSBoYXZlbid0IGxvb2tlZCBhdCB5b3VyIHNlcmllcyB0b28g
Y2FyZWZ1bGx5LCB0aGUgbWVudGlvbiBvZiBBUkMgY2F1Z2h0IG15DQphdHRlbnRpb24gOi0pIEkg
Z3Vlc3MgSSBuZWVkIHRvIHJlYWQgaXQgbW9yZSBjYXJlZnVsbHkgdG8gdW5kZXJzdGFuZC4NCg0K
DQo+IA0KPiBkaWZmIC0tZ2l0IGEvYXJjaC9hcmMvbW0vaW5pdC5jIGIvYXJjaC9hcmMvbW0vaW5p
dC5jDQo+IGluZGV4IDQxZWI5YmUxNjUzYy4uMzg2OTU5YmFjM2QyIDEwMDY0NA0KPiAtLS0gYS9h
cmNoL2FyYy9tbS9pbml0LmMNCj4gKysrIGIvYXJjaC9hcmMvbW0vaW5pdC5jDQo+IEBAIC03Nyw2
ICs3NywxMSBAQCB2b2lkIF9faW5pdCBlYXJseV9pbml0X2R0X2FkZF9tZW1vcnlfYXJjaCh1NjQg
YmFzZSwgdTY0IHNpemUpDQo+ICAJCWJhc2UsIFRPX01CKHNpemUpLCAhaW5fdXNlID8gIk5vdCB1
c2VkIjoiIik7DQo+ICB9DQo+ICANCj4gK2Jvb2wgYXJjaF9oYXNfZGVzY2VuZGluZ19tYXhfem9u
ZV9wZm5zKHZvaWQpDQo+ICt7DQo+ICsJcmV0dXJuIHRydWU7DQo+ICt9DQo+ICsNCj4gIC8qDQo+
ICAgKiBGaXJzdCBtZW1vcnkgc2V0dXAgcm91dGluZSBjYWxsZWQgZnJvbSBzZXR1cF9hcmNoKCkN
Cj4gICAqIDEuIHNldHVwIHN3YXBwZXIncyBtbSBAaW5pdF9tbQ0KPiBkaWZmIC0tZ2l0IGEvbW0v
cGFnZV9hbGxvYy5jIGIvbW0vcGFnZV9hbGxvYy5jDQo+IGluZGV4IGI5OTBlOTczNDQ3NC4uMTE0
ZjBlMDI3MTQ0IDEwMDY0NA0KPiAtLS0gYS9tbS9wYWdlX2FsbG9jLmMNCj4gKysrIGIvbW0vcGFn
ZV9hbGxvYy5jDQo+IEBAIC03MzA3LDYgKzczMDcsMTUgQEAgc3RhdGljIHZvaWQgY2hlY2tfZm9y
X21lbW9yeShwZ19kYXRhX3QgKnBnZGF0LCBpbnQgbmlkKQ0KPiAgCX0NCj4gIH0NCj4gIA0KPiAr
LyoNCj4gKyAqIFNvbWUgYXJjaGl0ZWN0dXJzLCBlLmcuIEFSQyBtYXkgaGF2ZSBaT05FX0hJR0hN
RU0gYmVsb3cgWk9ORV9OT1JNQUwuIEZvcg0KPiArICogc3VjaCBjYXNlcyB3ZSBhbGxvdyBtYXhf
em9uZV9wZm4gc29ydGVkIGluIHRoZSBkZXNjZW5kaW5nIG9yZGVyDQo+ICsgKi8NCj4gK2Jvb2wg
X193ZWFrIGFyY2hfaGFzX2Rlc2NlbmRpbmdfbWF4X3pvbmVfcGZucyh2b2lkKQ0KPiArew0KPiAr
CXJldHVybiBmYWxzZTsNCj4gK30NCj4gKw0KPiAgLyoqDQo+ICAgKiBmcmVlX2FyZWFfaW5pdCAt
IEluaXRpYWxpc2UgYWxsIHBnX2RhdGFfdCBhbmQgem9uZSBkYXRhDQo+ICAgKiBAbWF4X3pvbmVf
cGZuOiBhbiBhcnJheSBvZiBtYXggUEZOcyBmb3IgZWFjaCB6b25lDQo+IEBAIC03MzI0LDcgKzcz
MzMsNyBAQCB2b2lkIF9faW5pdCBmcmVlX2FyZWFfaW5pdCh1bnNpZ25lZCBsb25nICptYXhfem9u
ZV9wZm4pDQo+ICB7DQo+ICAJdW5zaWduZWQgbG9uZyBzdGFydF9wZm4sIGVuZF9wZm47DQo+ICAJ
aW50IGksIG5pZCwgem9uZTsNCj4gLQlib29sIGRlc2NlbmRpbmcgPSBmYWxzZTsNCj4gKwlib29s
IGRlc2NlbmRpbmc7DQo+ICANCj4gIAkvKiBSZWNvcmQgd2hlcmUgdGhlIHpvbmUgYm91bmRhcmll
cyBhcmUgKi8NCj4gIAltZW1zZXQoYXJjaF96b25lX2xvd2VzdF9wb3NzaWJsZV9wZm4sIDAsDQo+
IEBAIC03MzMzLDE0ICs3MzQyLDcgQEAgdm9pZCBfX2luaXQgZnJlZV9hcmVhX2luaXQodW5zaWdu
ZWQgbG9uZyAqbWF4X3pvbmVfcGZuKQ0KPiAgCQkJCXNpemVvZihhcmNoX3pvbmVfaGlnaGVzdF9w
b3NzaWJsZV9wZm4pKTsNCj4gIA0KPiAgCXN0YXJ0X3BmbiA9IGZpbmRfbWluX3Bmbl93aXRoX2Fj
dGl2ZV9yZWdpb25zKCk7DQo+IC0NCj4gLQkvKg0KPiAtCSAqIFNvbWUgYXJjaGl0ZWN0dXJzLCBl
LmcuIEFSQyBtYXkgaGF2ZSBaT05FX0hJR0hNRU0gYmVsb3cNCj4gLQkgKiBaT05FX05PUk1BTC4g
Rm9yIHN1Y2ggY2FzZXMgd2UgYWxsb3cgbWF4X3pvbmVfcGZuIHNvcnRlZCBpbiB0aGUNCj4gLQkg
KiBkZXNjZW5kaW5nIG9yZGVyDQo+IC0JICovDQo+IC0JaWYgKE1BWF9OUl9aT05FUyA+IDEgJiYg
bWF4X3pvbmVfcGZuWzBdID4gbWF4X3pvbmVfcGZuWzFdKQ0KPiAtCQlkZXNjZW5kaW5nID0gdHJ1
ZTsNCj4gKwlkZXNjZW5kaW5nID0gYXJjaF9oYXNfZGVzY2VuZGluZ19tYXhfem9uZV9wZm5zKCk7
DQo+ICANCj4gIAlmb3IgKGkgPSAwOyBpIDwgTUFYX05SX1pPTkVTOyBpKyspIHsNCj4gIAkJaWYg
KGRlc2NlbmRpbmcpDQo+IA0K

Re: [PATCH v2 17/20] mm: free_area_init: allow defining max_zone_pfn in descending order

[Mike Rapoport]

Hi Vineet,

On Tue, May 05, 2020 at 06:23:37AM +0000, Vineet Gupta wrote:
> Hi Mike,
> 
> On 5/4/20 8:39 AM, Mike Rapoport wrote:
> > On Sun, May 03, 2020 at 11:43:00AM -0700, Guenter Roeck wrote:
> >> On Sun, May 03, 2020 at 10:41:38AM -0700, Guenter Roeck wrote:
> >>> Hi,
> >>>
> >>> On Wed, Apr 29, 2020 at 03:11:23PM +0300, Mike Rapoport wrote:
> >>>> From: Mike Rapoport <rppt@linux.ibm.com>
> >>>>
> >>>> Some architectures (e.g. ARC) have the ZONE_HIGHMEM zone below the
> >>>> ZONE_NORMAL. Allowing free_area_init() parse max_zone_pfn array even it is
> >>>> sorted in descending order allows using free_area_init() on such
> >>>> architectures.
> >>>>
> >>>> Add top -> down traversal of max_zone_pfn array in free_area_init() and use
> >>>> the latter in ARC node/zone initialization.
> >>>>
> >>>> Signed-off-by: Mike Rapoport <rppt@linux.ibm.com>
> >>>
> >>> This patch causes my microblazeel qemu boot test in linux-next to fail.
> >>> Reverting it fixes the problem.
> >>>
> >> The same problem is seen with s390 emulations.
> > 
> > Yeah, this patch breaks some others as well :(
> > 
> > My assumption that max_zone_pfn defines architectural limit for maximal
> > PFN that can belong to a zone was over-optimistic. Several arches
> > actually do that, but others do
> > 
> > 	max_zone_pfn[ZONE_DMA] = MAX_DMA_PFN;
> > 	max_zone_pfn[ZONE_NORMAL] = max_pfn;
> > 
> > where MAX_DMA_PFN is build-time constrain and max_pfn is run time limit
> > for the current system.
> > 
> > So, when max_pfn is lower than MAX_DMA_PFN, the free_init_area() will
> > consider max_zone_pfn as descending and will wrongly calculate zone
> > extents.
> > 
> > That said, instead of trying to create a generic way to special case
> > ARC, I suggest to simply use the below patch instead.
> 
> Even for ARC it will be a bit more complicated. Highmem on ARC can be setup in 2
> ways such that it is descending in one case, and ascending in other (w.r.t
> "normal" mem) :-(

Yeah, and this makes ARC really special :)

> First some basic info about an ARC MMU based system
> 
> ARC logical address space (various addresses embedded in binaries)
>  - translated (0 to 0x6FFF_FFFF)  - for userspace
>  - untranslated (0x8000_0000 to 0xFFFF_FFFF) - kernel
> 
> ARC Physical address space is typically from 0x8000_0000 to 0xF000_0000.
> Above translated space maps here via MMU. Untranslated is implicitly mapped (no
> MMU involved).
> 
> The physical address in turn maps to a Bus address / memory (done at the
> inter-connect/NoC). Typically Physical 0x8000_0000 map to DDR 0
> 
> Now,
> - HIGHMEM w/o PAE40 adds Physical address space 0 to 0x7FFF_FFFF.
> - HIGHMEM with PAE40 uses physical address space from 0x1_0000_0000 upwards.
> 
> But then you could also have a system which has both of above so the bimodal up/dn
> won't work.

From the code I've got the impression that it is either one of them. I.e
the physical memory is either at

0x8000_0000 - <end of DDR 0 bank>
0x0000_0000 - <end of DDR 1 bank>

or

0x0_8000_0000 - <end of DDR 0 bank>
0x1_0000_0000 - <end of DDR 1 bank>

Is this possible to have a system with three live ranges? Like

0x0_0000_0000 - <end of DDR 1 bank>
0x0_8000_0000 - <end of DDR 0 bank>
0x1_0000_0000 - <end of DDR 2 bank>

> While I appreciate the effort to reduce complexity, it seems the
> current way of
> setting things up allows for more flexibility in specifying the system memory map.
>
> PS: I haven't looked at your series too carefully, the mention of ARC caught my
> attention :-) I guess I need to read it more carefully to understand.
 
That would be cool :)

> > 
> > diff --git a/arch/arc/mm/init.c b/arch/arc/mm/init.c
> > index 41eb9be1653c..386959bac3d2 100644
> > --- a/arch/arc/mm/init.c
> > +++ b/arch/arc/mm/init.c
> > @@ -77,6 +77,11 @@ void __init early_init_dt_add_memory_arch(u64 base, u64 size)
> >  		base, TO_MB(size), !in_use ? "Not used":"");
> >  }
> >  
> > +bool arch_has_descending_max_zone_pfns(void)
> > +{
> > +	return true;
> > +}
> > +
> >  /*
> >   * First memory setup routine called from setup_arch()
> >   * 1. setup swapper's mm @init_mm
> > diff --git a/mm/page_alloc.c b/mm/page_alloc.c
> > index b990e9734474..114f0e027144 100644
> > --- a/mm/page_alloc.c
> > +++ b/mm/page_alloc.c
> > @@ -7307,6 +7307,15 @@ static void check_for_memory(pg_data_t *pgdat, int nid)
> >  	}
> >  }
> >  
> > +/*
> > + * Some architecturs, e.g. ARC may have ZONE_HIGHMEM below ZONE_NORMAL. For
> > + * such cases we allow max_zone_pfn sorted in the descending order
> > + */
> > +bool __weak arch_has_descending_max_zone_pfns(void)
> > +{
> > +	return false;
> > +}
> > +
> >  /**
> >   * free_area_init - Initialise all pg_data_t and zone data
> >   * @max_zone_pfn: an array of max PFNs for each zone
> > @@ -7324,7 +7333,7 @@ void __init free_area_init(unsigned long *max_zone_pfn)
> >  {
> >  	unsigned long start_pfn, end_pfn;
> >  	int i, nid, zone;
> > -	bool descending = false;
> > +	bool descending;
> >  
> >  	/* Record where the zone boundaries are */
> >  	memset(arch_zone_lowest_possible_pfn, 0,
> > @@ -7333,14 +7342,7 @@ void __init free_area_init(unsigned long *max_zone_pfn)
> >  				sizeof(arch_zone_highest_possible_pfn));
> >  
> >  	start_pfn = find_min_pfn_with_active_regions();
> > -
> > -	/*
> > -	 * Some architecturs, e.g. ARC may have ZONE_HIGHMEM below
> > -	 * ZONE_NORMAL. For such cases we allow max_zone_pfn sorted in the
> > -	 * descending order
> > -	 */
> > -	if (MAX_NR_ZONES > 1 && max_zone_pfn[0] > max_zone_pfn[1])
> > -		descending = true;
> > +	descending = arch_has_descending_max_zone_pfns();
> >  
> >  	for (i = 0; i < MAX_NR_ZONES; i++) {
> >  		if (descending)
> > 

-- 
Sincerely yours,
Mike.

Re: [PATCH v2 17/20] mm: free_area_init: allow defining max_zone_pfn in descending order

[Vineet Gupta]

T24gNS81LzIwIDI6MTkgQU0sIE1pa2UgUmFwb3BvcnQgd3JvdGU6DQo+IEZyb20gdGhlIGNvZGUg
SSd2ZSBnb3QgdGhlIGltcHJlc3Npb24gdGhhdCBpdCBpcyBlaXRoZXIgb25lIG9mIHRoZW0uIEku
ZQ0KPiB0aGUgcGh5c2ljYWwgbWVtb3J5IGlzIGVpdGhlciBhdA0KPg0KPiAweDgwMDBfMDAwMCAt
IDxlbmQgb2YgRERSIDAgYmFuaz4NCj4gMHgwMDAwXzAwMDAgLSA8ZW5kIG9mIEREUiAxIGJhbms+
DQo+DQo+IG9yDQo+DQo+IDB4MF84MDAwXzAwMDAgLSA8ZW5kIG9mIEREUiAwIGJhbms+DQo+IDB4
MV8wMDAwXzAwMDAgLSA8ZW5kIG9mIEREUiAxIGJhbms+DQo+DQo+IElzIHRoaXMgcG9zc2libGUg
dG8gaGF2ZSBhIHN5c3RlbSB3aXRoIHRocmVlIGxpdmUgcmFuZ2VzPyBMaWtlDQo+DQo+IDB4MF8w
MDAwXzAwMDAgLSA8ZW5kIG9mIEREUiAxIGJhbms+DQo+IDB4MF84MDAwXzAwMDAgLSA8ZW5kIG9m
IEREUiAwIGJhbms+DQo+IDB4MV8wMDAwXzAwMDAgLSA8ZW5kIG9mIEREUiAyIGJhbms+DQoNCldl
IGRvbid0IGhhdmUgc3VjaCBhIHN5c3RlbSwgYnV0IGl0IGlzIGluZGVlZCBwb3NzaWJsZSBpbiB0
aGVvcnkuIFRoZSBxdWVzdGlvbiBpcw0KwqAtIENhbiBvdGhlciBhcmNoZXMgaGF2ZSBzdWNoIGEg
c2V0dXAgdG9vDQrCoC0gSXMgaXQgbm90IGJldHRlciB0byBoYXZlIHRoZSBjb3JlIHJldGFpbiB0
aGUgZmxleGliaWxpdHkganVzdCBpbiBjYXNlDQoNClRoeCwNCi1WaW5lZXQNCg=

Re: [PATCH v2 17/20] mm: free_area_init: allow defining max_zone_pfn in descending order

[Mike Rapoport]

On Tue, May 05, 2020 at 06:07:46PM +0000, Vineet Gupta wrote:
> On 5/5/20 2:19 AM, Mike Rapoport wrote:
> > From the code I've got the impression that it is either one of them. I.e
> > the physical memory is either at
> >
> > 0x8000_0000 - <end of DDR 0 bank>
> > 0x0000_0000 - <end of DDR 1 bank>
> >
> > or
> >
> > 0x0_8000_0000 - <end of DDR 0 bank>
> > 0x1_0000_0000 - <end of DDR 1 bank>
> >
> > Is this possible to have a system with three live ranges? Like
> >
> > 0x0_0000_0000 - <end of DDR 1 bank>
> > 0x0_8000_0000 - <end of DDR 0 bank>
> > 0x1_0000_0000 - <end of DDR 2 bank>
> 
> We don't have such a system, but it is indeed possible in theory. The question is
>  - Can other arches have such a setup too

At the moment all architectures that support HIGHMEM have it above
DMA/NORMAL. I'm not sure if such a setup is theoretically possible for
other architectures, but as of now none of them support it in Linux.

The general case is somewhat like

	max_dma_pfn <= max_normal_pfn < max_high_pfn

And of course, either max_dma_pfn or max_high_pfn or both may be not
needed for an architecture.

>  - Is it not better to have the core retain the flexibility just in case

Hmm, there is indeed flexibility in the nodes and zones initialization,
but if you'd look more closely to free_area_init*() and friends, there
is a lot of cruft and retrofitting ;-)

What we have is two mutually exclusive paths, one that relies on the
architecture to calculate zone sizes and find the holes between the
zones (!CONFIG_HAVE_MEMBLOCK_NODE_MAP) and the other one that only
requires the architectures to pass possible limit for each zone and
detects the actual zone spans based on the knowlegde about the actual
physical memory layout that comes from memblock.

These patches attempt to drop the older method and switch all the
architectures to use newer and simpler one.

If the requirement to have support for 3-banks is a theoretical
possibility, I would prefer to adjust ARC's version of
arch_has_descending_max_zone_pfns() to cope with either of 2-banks
configuration (PAE40 and non-PAE40) and deal with the third bank when/if
it actually materializes.

> Thx,
> -Vineet

-- 
Sincerely yours,
Mike.

Re: [PATCH v2 17/20] mm: free_area_init: allow defining max_zone_pfn in descending order

[Mike Rapoport]

On Tue, May 05, 2020 at 11:15:22PM +0300, Mike Rapoport wrote:
> On Tue, May 05, 2020 at 06:07:46PM +0000, Vineet Gupta wrote:
> > On 5/5/20 2:19 AM, Mike Rapoport wrote:
> 
> >  - Is it not better to have the core retain the flexibility just in case
> 
> If the requirement to have support for 3-banks is a theoretical
> possibility, I would prefer to adjust ARC's version of
> arch_has_descending_max_zone_pfns() to cope with either of 2-banks
> configuration (PAE40 and non-PAE40) and deal with the third bank when/if
> it actually materializes.
> 
> > Thx,
> > -Vineet
> 

The fix below should take care of any 2-bank configurations. 
This is vs. current mmotm.

From eb8124fb3584607d1036b7ae00c8092ae43e480d Mon Sep 17 00:00:00 2001
From: Mike Rapoport <rppt@linux.ibm.com>
Date: Thu, 7 May 2020 23:44:15 +0300
Subject: [PATCH] arc: free_area_init(): take into account PAE40 mode

The arch_has_descending_max_zone_pfns() does not take into account physical
memory layout for PAE40 configuration.
With PAE40 enabled, the HIGHMEM is actually higher than NORMAL and
arch_has_descending_max_zone_pfns() should return false in this case.

Signed-off-by: Mike Rapoport <rppt@linux.ibm.com>
---
 arch/arc/mm/init.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/arc/mm/init.c b/arch/arc/mm/init.c
index 386959bac3d2..e7bdc2ac1c87 100644
--- a/arch/arc/mm/init.c
+++ b/arch/arc/mm/init.c
@@ -79,7 +79,7 @@ void __init early_init_dt_add_memory_arch(u64 base, u64 size)
 
 bool arch_has_descending_max_zone_pfns(void)
 {
-	return true;
+	return !IS_ENABLED(CONFIG_ARC_HAS_PAE40);
 }
 
 /*
-- 
2.26.1


-- 
Sincerely yours,
Mike.

Re: [PATCH v2 17/20] mm: free_area_init: allow defining max_zone_pfn in descending order

[Vineet Gupta]

T24gNS83LzIwIDE6NTkgUE0sIE1pa2UgUmFwb3BvcnQgd3JvdGU6DQo+IE9uIFR1ZSwgTWF5IDA1
LCAyMDIwIGF0IDExOjE1OjIyUE0gKzAzMDAsIE1pa2UgUmFwb3BvcnQgd3JvdGU6DQo+PiBPbiBU
dWUsIE1heSAwNSwgMjAyMCBhdCAwNjowNzo0NlBNICswMDAwLCBWaW5lZXQgR3VwdGEgd3JvdGU6
DQo+Pj4gT24gNS81LzIwIDI6MTkgQU0sIE1pa2UgUmFwb3BvcnQgd3JvdGU6DQo+Pj4gwqAtIElz
IGl0IG5vdCBiZXR0ZXIgdG8gaGF2ZSB0aGUgY29yZSByZXRhaW4gdGhlIGZsZXhpYmlsaXR5IGp1
c3QgaW4gY2FzZQ0KPj4gSWYgdGhlIHJlcXVpcmVtZW50IHRvIGhhdmUgc3VwcG9ydCBmb3IgMy1i
YW5rcyBpcyBhIHRoZW9yZXRpY2FsDQo+PiBwb3NzaWJpbGl0eSwgSSB3b3VsZCBwcmVmZXIgdG8g
YWRqdXN0IEFSQydzIHZlcnNpb24gb2YNCj4+IGFyY2hfaGFzX2Rlc2NlbmRpbmdfbWF4X3pvbmVf
cGZucygpIHRvIGNvcGUgd2l0aCBlaXRoZXIgb2YgMi1iYW5rcw0KPj4gY29uZmlndXJhdGlvbiAo
UEFFNDAgYW5kIG5vbi1QQUU0MCkgYW5kIGRlYWwgd2l0aCB0aGUgdGhpcmQgYmFuayB3aGVuL2lm
DQo+PiBpdCBhY3R1YWxseSBtYXRlcmlhbGl6ZXMuDQoNCkZhaXIgZW5vdWdoLg0KDQo+IFRoZSBm
aXggYmVsb3cgc2hvdWxkIHRha2UgY2FyZSBvZiBhbnkgMi1iYW5rIGNvbmZpZ3VyYXRpb25zLiAN
Cj4gVGhpcyBpcyB2cy4gY3VycmVudCBtbW90bS4NCj4NCj4gRnJvbSBlYjgxMjRmYjM1ODQ2MDdk
MTAzNmI3YWUwMGM4MDkyYWU0M2U0ODBkIE1vbiBTZXAgMTcgMDA6MDA6MDAgMjAwMQ0KPiBGcm9t
OiBNaWtlIFJhcG9wb3J0IDxycHB0QGxpbnV4LmlibS5jb20+DQo+IERhdGU6IFRodSwgNyBNYXkg
MjAyMCAyMzo0NDoxNSArMDMwMA0KPiBTdWJqZWN0OiBbUEFUQ0hdIGFyYzogZnJlZV9hcmVhX2lu
aXQoKTogdGFrZSBpbnRvIGFjY291bnQgUEFFNDAgbW9kZQ0KPg0KPiBUaGUgYXJjaF9oYXNfZGVz
Y2VuZGluZ19tYXhfem9uZV9wZm5zKCkgZG9lcyBub3QgdGFrZSBpbnRvIGFjY291bnQgcGh5c2lj
YWwNCj4gbWVtb3J5IGxheW91dCBmb3IgUEFFNDAgY29uZmlndXJhdGlvbi4NCj4gV2l0aCBQQUU0
MCBlbmFibGVkLCB0aGUgSElHSE1FTSBpcyBhY3R1YWxseSBoaWdoZXIgdGhhbiBOT1JNQUwgYW5k
DQo+IGFyY2hfaGFzX2Rlc2NlbmRpbmdfbWF4X3pvbmVfcGZucygpIHNob3VsZCByZXR1cm4gZmFs
c2UgaW4gdGhpcyBjYXNlLg0KPg0KPiBTaWduZWQtb2ZmLWJ5OiBNaWtlIFJhcG9wb3J0IDxycHB0
QGxpbnV4LmlibS5jb20+DQoNCkxHVE0uDQoNCkFja2VkLWJ5OiBWaW5lZXQgR3VwdGEgPHZndXB0
YUBzeW5vcHN5cy5jb20+DQoNClRoeCwNCg0KPiAtLS0NCj4gIGFyY2gvYXJjL21tL2luaXQuYyB8
IDIgKy0NCj4gIDEgZmlsZSBjaGFuZ2VkLCAxIGluc2VydGlvbigrKSwgMSBkZWxldGlvbigtKQ0K
Pg0KPiBkaWZmIC0tZ2l0IGEvYXJjaC9hcmMvbW0vaW5pdC5jIGIvYXJjaC9hcmMvbW0vaW5pdC5j
DQo+IGluZGV4IDM4Njk1OWJhYzNkMi4uZTdiZGMyYWMxYzg3IDEwMDY0NA0KPiAtLS0gYS9hcmNo
L2FyYy9tbS9pbml0LmMNCj4gKysrIGIvYXJjaC9hcmMvbW0vaW5pdC5jDQo+IEBAIC03OSw3ICs3
OSw3IEBAIHZvaWQgX19pbml0IGVhcmx5X2luaXRfZHRfYWRkX21lbW9yeV9hcmNoKHU2NCBiYXNl
LCB1NjQgc2l6ZSkNCj4gIA0KPiAgYm9vbCBhcmNoX2hhc19kZXNjZW5kaW5nX21heF96b25lX3Bm
bnModm9pZCkNCj4gIHsNCj4gLQlyZXR1cm4gdHJ1ZTsNCj4gKwlyZXR1cm4gIUlTX0VOQUJMRUQo
Q09ORklHX0FSQ19IQVNfUEFFNDApOw0KPiAgfQ0KPiAgDQo+ICAvKg0KDQo