On 10/10/2018 11:59 AM, Nathan Fontenot wrote:
> On 10/09/2018 03:36 PM, Michael Bringmann wrote:
>> migration/memory: This patch adds a new pseries hotplug action
>> for CPU and memory operations, PSERIES_HP_ELOG_ACTION_READD_MULTIPLE.
>> This is a variant of the READD operation which performs the action
>> upon multiple instances of the resource at one time. The operation
>> is to be triggered by device-tree analysis of updates by RTAS events
>> analyzed by 'migation_store' during post-migration processing. It
>> will be used for memory updates, initially.
>>
>> Signed-off-by: Michael Bringmann <m...@linux.vnet.ibm.com>
>> ---
>> Changes in v04:
>> -- Move init of 'lmb->internal_flags' in init_drmem_v2_lmbs to
>> previous patch.
>> -- Pull in implementation of dlpar_memory_readd_multiple() to go
>> with operation flag.
>> ---
>> arch/powerpc/include/asm/rtas.h | 1 +
>> arch/powerpc/platforms/pseries/hotplug-memory.c | 31
>> +++++++++++++++++++++++
>> 2 files changed, 32 insertions(+)
>>
>> diff --git a/arch/powerpc/include/asm/rtas.h
>> b/arch/powerpc/include/asm/rtas.h
>> index 0183e95..cc00451 100644
>> --- a/arch/powerpc/include/asm/rtas.h
>> +++ b/arch/powerpc/include/asm/rtas.h
>> @@ -333,6 +333,7 @@ struct pseries_hp_errorlog {
>> #define PSERIES_HP_ELOG_ACTION_ADD 1
>> #define PSERIES_HP_ELOG_ACTION_REMOVE 2
>> #define PSERIES_HP_ELOG_ACTION_READD 3
>> +#define PSERIES_HP_ELOG_ACTION_READD_MULTIPLE 4
>>
>> #define PSERIES_HP_ELOG_ID_DRC_NAME 1
>> #define PSERIES_HP_ELOG_ID_DRC_INDEX 2
>> diff --git a/arch/powerpc/platforms/pseries/hotplug-memory.c
>> b/arch/powerpc/platforms/pseries/hotplug-memory.c
>> index 9a15d39..bf2420a 100644
>> --- a/arch/powerpc/platforms/pseries/hotplug-memory.c
>> +++ b/arch/powerpc/platforms/pseries/hotplug-memory.c
>> @@ -546,6 +546,30 @@ static int dlpar_memory_readd_by_index(u32 drc_index)
>> return rc;
>> }
>>
>> +static int dlpar_memory_readd_multiple(void)
>> +{
>> + struct drmem_lmb *lmb;
>> + int rc;
>> +
>> + pr_info("Attempting to update multiple LMBs\n");
>> +
>> + for_each_drmem_lmb(lmb) {
>> + if (drmem_lmb_update(lmb)) {
>> + rc = dlpar_remove_lmb(lmb);
>> +
>> + if (!rc) {
>> + rc = dlpar_add_lmb(lmb);
>> + if (rc)
>> + dlpar_release_drc(lmb->drc_index);
>> + }
>
> The work you're doing here is essentially the same that is done in
> dlpar_memory_readd_by_index(). Perhaps pulling the commin bits of both
> routines into a helper routine. This could include the success/failure
> messages in dlpar_memory_readd_by_index()
Really, only the interior of the loop is common to the two functions.
Creating a helper that incorporated the loop would mean either several
helper functions customized to each path (and a lot more code).
Or a common helper function that does everything for both paths, and
would be harder to understand/maintain.
It would be a lot cleaner to put the common loop interior into a helper
function,
and retain the other two functions with their unique loop + test + extra
operations. I will update with this method.
>
> -Nathan
Michael
>
>> +
>> + drmem_remove_lmb_update(lmb);
>> + }
>> + }
>> +
>> + return rc;
>> +}
>> +
>> static int dlpar_memory_remove_by_ic(u32 lmbs_to_remove, u32 drc_index)
>> {
>> struct drmem_lmb *lmb, *start_lmb, *end_lmb;
>> @@ -646,6 +670,10 @@ static int dlpar_memory_readd_by_index(u32 drc_index)
>> {
>> return -EOPNOTSUPP;
>> }
>> +static int dlpar_memory_readd_multiple(void)
>> +{
>> + return -EOPNOTSUPP;
>> +}
>>
>> static int dlpar_memory_remove_by_ic(u32 lmbs_to_remove, u32 drc_index)
>> {
>> @@ -923,6 +951,9 @@ int dlpar_memory(struct pseries_hp_errorlog *hp_elog)
>> drc_index = hp_elog->_drc_u.drc_index;
>> rc = dlpar_memory_readd_by_index(drc_index);
>> break;
>> + case PSERIES_HP_ELOG_ACTION_READD_MULTIPLE:
>> + rc = dlpar_memory_readd_multiple();
>> + break;
>> default:
>> pr_err("Invalid action (%d) specified\n", hp_elog->action);
>> rc = -EINVAL;
>>
>
>
--
Michael W. Bringmann
Linux Technology Center
IBM Corporation
Tie-Line 363-5196
External: (512) 286-5196
Cell: (512) 466-0650
m...@linux.vnet.ibm.com
