From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-ot0-x242.google.com (mail-ot0-x242.google.com [IPv6:2607:f8b0:4003:c0f::242]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by lists.ozlabs.org (Postfix) with ESMTPS id 3ydQgV5KZrzDrK2 for ; Fri, 17 Nov 2017 15:51:34 +1100 (AEDT) Received: by mail-ot0-x242.google.com with SMTP id g104so1097460otg.7 for ; Thu, 16 Nov 2017 20:51:34 -0800 (PST) MIME-Version: 1.0 In-Reply-To: <77eae60c-a406-dd8c-acb5-9e137153ce7a@linux.vnet.ibm.com> References: <150850568437.9118.13945089249591962212.stgit@ltcalpine2-lp14.aus.stglabs.ibm.com> <77eae60c-a406-dd8c-acb5-9e137153ce7a@linux.vnet.ibm.com> From: Bharata B Rao Date: Fri, 17 Nov 2017 10:21:31 +0530 Message-ID: Subject: Re: [PATCH v2 0/8] powerpc: Support ibm,dynamic-memory-v2 property To: Nathan Fontenot Cc: linuxppc-dev , Aneesh Kumar Content-Type: multipart/alternative; boundary="f4030435ac8c8bc60e055e267d9c" List-Id: Linux on PowerPC Developers Mail List List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , --f4030435ac8c8bc60e055e267d9c Content-Type: text/plain; charset="UTF-8" On Thu, Nov 16, 2017 at 9:31 PM, Nathan Fontenot wrote: > > > On 11/15/2017 11:37 PM, Bharata B Rao wrote: > > On Fri, Oct 20, 2017 at 6:51 PM, Nathan Fontenot < > nfont@linux.vnet.ibm.com > wrote: > > > > This patch set provides a set of updates to de-couple the LMB > information > > provided in the ibm,dynamic-memory device tree property from the > device > > tree property format. A part of this patch series introduces a new > > device tree property format for dynamic memory, ibm-dynamic-meory-v2. > > By separating the device tree format from the information provided by > > the device tree property consumers of this information need not know > > what format is currently being used and provide multiple parsing > routines > > for the information. > > > > The first two patches update the of_get_assoc_arrays() and > > of_get_usable_memory() routines to look up the device node for the > > properties they parse. This is needed because the calling routines > for > > these two functions will not have the device node to pass in in > > subsequent patches. > > > > The third patch adds a new kernel structure, struct drmem_lmb, that > > is used to represent each of the possible LMBs specified in the > > ibm,dynamic-memory* device tree properties. The patch adds code > > to parse the property and build the LMB array data, and updates > prom.c > > to use this new data structure instead of parsing the device tree > directly. > > > > The fourth and fifth patches update the numa and pseries hotplug code > > respectively to use the new LMB array data instead of parsing the > > device tree directly. > > > > The sixth patch moves the of_drconf_cell struct to drmem.h where it > > fits better than prom.h > > > > The seventh patch introduces support for the ibm,dynamic-memory-v2 > > property format by updating the new drmem.c code to be able to parse > > and create this new device tree format. > > > > The last patch in the series updates the architecture vector to > indicate > > support for ibm,dynamic-memory-v2. > > > > > > Here we are consolidating LMBs into LMB sets but still end up working > with individual LMBs during hotplug. Can we instead start working with LMB > sets together during hotplug ? In other words > > In a sense we do do this when handling memory DLPAR indexed-count > requests. This takes a starting > drc index for a LMB and adds/removes the following contiguous > LMBs. This operation is > all-or-nothing, if any LMB fails to add/remove we revert back to the > original state. > I am aware of count-indexed and we do use it for memory hotplug/unplug for KVM on Power. However the RTAS and configure-connector calls there are still per-LMB. > Thi isn't exactly what you're asking for but... > > > > - The RTAS calls involved during DRC acquire stage can be done only once > per LMB set. > > - One configure-connector call for the entire LMB set. > > these two interfaces work on a single drc index, not a set of drc indexes. > Working on a set > of LMBs would require extending the current rtas calls or creating new > ones. > Yes. > > One thing we can look into doing for indexed-count requests is to perform > each of the > steps for all LMBs in the set at once, i.e. make the acquire call for > LMBs, then make the > configure-connector calls for all the LMBs... > That is what I am hinting at to check the feasibility of such a mechanism. Given that all the LMBs of the set are supposed to have similar attributes (like node associativity etc), it makes sense to have a single DRC acquire call and single configure-connector call for the entire set. > The only drawback is this approach would make handling failures and > backing out of the > updates a bit messier, but I've never really thought that optimizing for > the failure > case to be as important. > Yes, error recovery can be messy given that we have multiple calls under DRC acquire call (get-sensor-state and set-indicator). BTW I thought this reorganization involving ibm,drc-info and ibm,dynamic-memory-v2 was for representing and hotplugging huge amounts of memory efficiently and quickly. So you have not yet found the per-LMB calls to be hurting when huge amount of memory is involved ? Regards, Bharata. --f4030435ac8c8bc60e055e267d9c Content-Type: text/html; charset="UTF-8" Content-Transfer-Encoding: quoted-printable

= On Thu, Nov 16, 2017 at 9:31 PM, Nathan Fontenot <nfont@linux.vnet.= ibm.com> wrote:


On 11/15/2017 11:37 PM, Bharata B Rao wrote:
> On Fri, Oct 20, 2017 at 6:51 PM, Nathan Fonteno= t <nfont@linux.vnet.ibm.com<= /a> <mailto:nfont@linux.vnet= .ibm.com>> wrote:
>
>=C2=A0 =C2=A0 =C2=A0This patch set provides a set of updates to de-coup= le the LMB information
>=C2=A0 =C2=A0 =C2=A0provided in the ibm,dynamic-memory device tree prop= erty from the device
>=C2=A0 =C2=A0 =C2=A0tree property format. A part of this patch series i= ntroduces a new
>=C2=A0 =C2=A0 =C2=A0device tree property format for dynamic memory, ibm= -dynamic-meory-v2.
>=C2=A0 =C2=A0 =C2=A0By separating the device tree format from the infor= mation provided by
>=C2=A0 =C2=A0 =C2=A0the device tree property consumers of this informat= ion need not know
>=C2=A0 =C2=A0 =C2=A0what format is currently being used and provide mul= tiple parsing routines
>=C2=A0 =C2=A0 =C2=A0for the information.
>
>=C2=A0 =C2=A0 =C2=A0The first two patches update the of_get_assoc_array= s() and
>=C2=A0 =C2=A0 =C2=A0of_get_usable_memory() routines to look up the devi= ce node for the
>=C2=A0 =C2=A0 =C2=A0properties they parse. This is needed because the c= alling routines for
>=C2=A0 =C2=A0 =C2=A0these two functions will not have the device node t= o pass in in
>=C2=A0 =C2=A0 =C2=A0subsequent patches.
>
>=C2=A0 =C2=A0 =C2=A0The third patch adds a new kernel structure, struct= drmem_lmb, that
>=C2=A0 =C2=A0 =C2=A0is used to represent each of the possible LMBs spec= ified in the
>=C2=A0 =C2=A0 =C2=A0ibm,dynamic-memory* device tree properties. The pat= ch adds code
>=C2=A0 =C2=A0 =C2=A0to parse the property and build the LMB array data,= and updates prom.c
>=C2=A0 =C2=A0 =C2=A0to use this new data structure instead of parsing t= he device tree directly.
>
>=C2=A0 =C2=A0 =C2=A0The fourth and fifth patches update the numa and ps= eries hotplug code
>=C2=A0 =C2=A0 =C2=A0respectively to use the new LMB array data instead = of parsing the
>=C2=A0 =C2=A0 =C2=A0device tree directly.
>
>=C2=A0 =C2=A0 =C2=A0The sixth patch moves the of_drconf_cell struct to = drmem.h where it
>=C2=A0 =C2=A0 =C2=A0fits better than prom.h
>
>=C2=A0 =C2=A0 =C2=A0The seventh patch introduces support for the ibm,dy= namic-memory-v2
>=C2=A0 =C2=A0 =C2=A0property format by updating the new drmem.c code to= be able to parse
>=C2=A0 =C2=A0 =C2=A0and create this new device tree format.
>
>=C2=A0 =C2=A0 =C2=A0The last patch in the series updates the architectu= re vector to indicate
>=C2=A0 =C2=A0 =C2=A0support for ibm,dynamic-memory-v2.
>
>
> Here we are consolidating LMBs into LMB sets but still end up working = with individual LMBs during hotplug. Can we instead start working with LMB = sets together during hotplug ? In other words

In a sense we do do this when handling memory DLPAR indexed-cou= nt requests. This takes a starting
drc index for a LMB and adds/removes the following <count> contiguous= LMBs. This operation is
all-or-nothing, if any LMB fails to add/remove we revert back to the origin= al state.

I am aware of count-indexed a= nd we do use it for memory hotplug/unplug for KVM on Power. However the RTA= S and configure-connector calls there are still per-LMB.


Thi isn't exactly what you're asking for but...
>
> - The RTAS calls involved during DRC acquire stage can be done only on= ce per LMB set.
> - One configure-connector call for the entire LMB set.

these two interfaces work on a single drc index, not a set of drc in= dexes. Working on a set
of LMBs would require extending the current rtas calls or creating new ones= .

Yes.
=C2=A0

One thing we can look into doing for indexed-count requests is to perform e= ach of the
steps for all LMBs in the set at once, i.e. make the acquire call for LMBs,= then make the
configure-connector calls for all the LMBs...

That is what I am hinting at to check the feasibility of such a mech= anism. Given that all the LMBs of the set are supposed to have similar attr= ibutes (like node associativity etc), it makes sense to have a single DRC a= cquire call and single configure-connector call for the entire set.

=

The only drawback is this approach would make handling failures and backing= out of the
updates a bit messier, but I've never really thought that optimizing fo= r the failure
case to be as important.

Yes, error rec= overy can be messy given that we have multiple calls under DRC acquire call= (get-sensor-state and set-indicator).

BTW I thought this= reorganization involving ibm,drc-info and ibm,dynamic-memory-v2 was for re= presenting and hotplugging huge amounts of memory efficiently and quickly. = So you have not yet found the per-LMB calls to be hurting when huge amount = of memory is involved ?

Regards,
Bharata.

--f4030435ac8c8bc60e055e267d9c--