| Message ID | a2c023e9-6997-d1a3-0110-fa8c788e64ee@linux.vnet.ibm.com (mailing list archive) |
|---|---|
| State | Not Applicable |
| Headers | show |
| Series | powerpc/hotplug: Update affinity for migrated CPUs | expand |
On Fri, Jul 13, 2018 at 03:18:01PM -0500, Michael Bringmann wrote: >migration/dlpar: This patch adds function dlpar_queue_action() >which will queued up information about a CPU/Memory 'readd' >operation according to resource type, action code, and DRC index. >At a subsequent point, the list of operations can be run/played >in series. Examples of such oprations include 'readd' of CPU >and Memory blocks identified as having changed their associativity >during an LPAR migration event. > >Signed-off-by: Michael Bringmann <mwb@linux.vnet.ibm.com> >--- >Changes in patch: > -- Correct drc_index before adding to pseries_hp_errorlog struct > -- Correct text of notice > -- Revise queuing model to save up all of the DLPAR actions for > later execution. > -- Restore list init statement missing from patch > -- Move call to apply queued operations into 'mobility.c' > -- Compress some code > -- Rename some of queueing function APIs > -- Revise implementation to push execution of queued operations > to a workqueue task. > -- Cleanup reference to outdated queuing operation. >--- > arch/powerpc/include/asm/rtas.h | 2 + > arch/powerpc/platforms/pseries/dlpar.c | 61 +++++++++++++++++++++++++++++ > arch/powerpc/platforms/pseries/mobility.c | 4 ++ > arch/powerpc/platforms/pseries/pseries.h | 2 + > 4 files changed, 69 insertions(+) > >diff --git a/arch/powerpc/include/asm/rtas.h b/arch/powerpc/include/asm/rtas.h >index 71e393c..4f601c7 100644 >--- a/arch/powerpc/include/asm/rtas.h >+++ b/arch/powerpc/include/asm/rtas.h >@@ -310,12 +310,14 @@ struct pseries_hp_errorlog { > struct { __be32 count, index; } ic; > char drc_name[1]; > } _drc_u; >+ struct list_head list; > }; > > #define PSERIES_HP_ELOG_RESOURCE_CPU 1 > #define PSERIES_HP_ELOG_RESOURCE_MEM 2 > #define PSERIES_HP_ELOG_RESOURCE_SLOT 3 > #define PSERIES_HP_ELOG_RESOURCE_PHB 4 >+#define PSERIES_HP_ELOG_RESOURCE_PMT 5 > > #define PSERIES_HP_ELOG_ACTION_ADD 1 > #define PSERIES_HP_ELOG_ACTION_REMOVE 2 >diff --git a/arch/powerpc/platforms/pseries/dlpar.c b/arch/powerpc/platforms/pseries/dlpar.c >index a0b20c0..7264b8e 100644 >--- a/arch/powerpc/platforms/pseries/dlpar.c >+++ b/arch/powerpc/platforms/pseries/dlpar.c >@@ -25,6 +25,7 @@ > #include <asm/prom.h> > #include <asm/machdep.h> > #include <linux/uaccess.h> >+#include <linux/delay.h> > #include <asm/rtas.h> > > static struct workqueue_struct *pseries_hp_wq; >@@ -329,6 +330,8 @@ int dlpar_release_drc(u32 drc_index) > return 0; > } > >+static int dlpar_pmt(struct pseries_hp_errorlog *work); >+ > static int handle_dlpar_errorlog(struct pseries_hp_errorlog *hp_elog) > { > int rc; >@@ -357,6 +360,9 @@ static int handle_dlpar_errorlog(struct pseries_hp_errorlog *hp_elog) > case PSERIES_HP_ELOG_RESOURCE_CPU: > rc = dlpar_cpu(hp_elog); > break; >+ case PSERIES_HP_ELOG_RESOURCE_PMT: >+ rc = dlpar_pmt(hp_elog); >+ break; > default: > pr_warn_ratelimited("Invalid resource (%d) specified\n", > hp_elog->resource); >@@ -407,6 +413,61 @@ void queue_hotplug_event(struct pseries_hp_errorlog *hp_errlog, > } > } > >+LIST_HEAD(dlpar_delayed_list); >+ >+int dlpar_queue_action(int resource, int action, u32 drc_index) >+{ >+ struct pseries_hp_errorlog *hp_errlog; >+ >+ hp_errlog = kmalloc(sizeof(struct pseries_hp_errorlog), GFP_KERNEL); >+ if (!hp_errlog) >+ return -ENOMEM; >+ >+ hp_errlog->resource = resource; >+ hp_errlog->action = action; >+ hp_errlog->id_type = PSERIES_HP_ELOG_ID_DRC_INDEX; >+ hp_errlog->_drc_u.drc_index = cpu_to_be32(drc_index); >+ >+ list_add_tail(&hp_errlog->list, &dlpar_delayed_list); >+ >+ return 0; >+} >+ >+static int dlpar_pmt(struct pseries_hp_errorlog *work) >+{ >+ struct list_head *pos, *q; >+ >+ ssleep(15); Why do we need to sleep for so long here? -John >+ >+ list_for_each_safe(pos, q, &dlpar_delayed_list) { >+ struct pseries_hp_errorlog *tmp; >+ >+ tmp = list_entry(pos, struct pseries_hp_errorlog, list); >+ handle_dlpar_errorlog(tmp); >+ >+ list_del(pos); >+ kfree(tmp); >+ >+ ssleep(10); >+ } >+ >+ return 0; >+} >+ >+int dlpar_queued_actions_run(void) >+{ >+ if (!list_empty(&dlpar_delayed_list)) { >+ struct pseries_hp_errorlog hp_errlog; >+ >+ hp_errlog.resource = PSERIES_HP_ELOG_RESOURCE_PMT; >+ hp_errlog.action = 0; >+ hp_errlog.id_type = 0; >+ >+ queue_hotplug_event(&hp_errlog, 0, 0); >+ } >+ return 0; >+} >+ > static int dlpar_parse_resource(char **cmd, struct pseries_hp_errorlog *hp_elog) > { > char *arg; >diff --git a/arch/powerpc/platforms/pseries/mobility.c b/arch/powerpc/platforms/pseries/mobility.c >index f6364d9..d0d1cae 100644 >--- a/arch/powerpc/platforms/pseries/mobility.c >+++ b/arch/powerpc/platforms/pseries/mobility.c >@@ -378,6 +378,10 @@ static ssize_t migration_store(struct class *class, > return rc; > > post_mobility_fixup(); >+ >+ /* Apply any necessary changes identified during fixup */ >+ dlpar_queued_actions_run(); >+ > return count; > } > >diff --git a/arch/powerpc/platforms/pseries/pseries.h b/arch/powerpc/platforms/pseries/pseries.h >index 60db2ee..72ca996 100644 >--- a/arch/powerpc/platforms/pseries/pseries.h >+++ b/arch/powerpc/platforms/pseries/pseries.h >@@ -61,6 +61,8 @@ extern struct device_node *dlpar_configure_connector(__be32, > > void queue_hotplug_event(struct pseries_hp_errorlog *hp_errlog, > struct completion *hotplug_done, int *rc); >+int dlpar_queue_action(int resource, int action, u32 drc_index); >+int dlpar_queued_actions_run(void); > #ifdef CONFIG_MEMORY_HOTPLUG > int dlpar_memory(struct pseries_hp_errorlog *hp_elog); > #else >
On 07/13/2018 03:18 PM, Michael Bringmann wrote: > migration/dlpar: This patch adds function dlpar_queue_action() > which will queued up information about a CPU/Memory 'readd' > operation according to resource type, action code, and DRC index. > At a subsequent point, the list of operations can be run/played > in series. Examples of such oprations include 'readd' of CPU > and Memory blocks identified as having changed their associativity > during an LPAR migration event. > > Signed-off-by: Michael Bringmann <mwb@linux.vnet.ibm.com> > --- > Changes in patch: > -- Correct drc_index before adding to pseries_hp_errorlog struct > -- Correct text of notice > -- Revise queuing model to save up all of the DLPAR actions for > later execution. > -- Restore list init statement missing from patch > -- Move call to apply queued operations into 'mobility.c' > -- Compress some code > -- Rename some of queueing function APIs > -- Revise implementation to push execution of queued operations > to a workqueue task. > -- Cleanup reference to outdated queuing operation. > --- > arch/powerpc/include/asm/rtas.h | 2 + > arch/powerpc/platforms/pseries/dlpar.c | 61 +++++++++++++++++++++++++++++ > arch/powerpc/platforms/pseries/mobility.c | 4 ++ > arch/powerpc/platforms/pseries/pseries.h | 2 + > 4 files changed, 69 insertions(+) > > diff --git a/arch/powerpc/include/asm/rtas.h b/arch/powerpc/include/asm/rtas.h > index 71e393c..4f601c7 100644 > --- a/arch/powerpc/include/asm/rtas.h > +++ b/arch/powerpc/include/asm/rtas.h > @@ -310,12 +310,14 @@ struct pseries_hp_errorlog { > struct { __be32 count, index; } ic; > char drc_name[1]; > } _drc_u; > + struct list_head list; > }; > > #define PSERIES_HP_ELOG_RESOURCE_CPU 1 > #define PSERIES_HP_ELOG_RESOURCE_MEM 2 > #define PSERIES_HP_ELOG_RESOURCE_SLOT 3 > #define PSERIES_HP_ELOG_RESOURCE_PHB 4 > +#define PSERIES_HP_ELOG_RESOURCE_PMT 5 > > #define PSERIES_HP_ELOG_ACTION_ADD 1 > #define PSERIES_HP_ELOG_ACTION_REMOVE 2 > diff --git a/arch/powerpc/platforms/pseries/dlpar.c b/arch/powerpc/platforms/pseries/dlpar.c > index a0b20c0..7264b8e 100644 > --- a/arch/powerpc/platforms/pseries/dlpar.c > +++ b/arch/powerpc/platforms/pseries/dlpar.c > @@ -25,6 +25,7 @@ > #include <asm/prom.h> > #include <asm/machdep.h> > #include <linux/uaccess.h> > +#include <linux/delay.h> > #include <asm/rtas.h> > > static struct workqueue_struct *pseries_hp_wq; > @@ -329,6 +330,8 @@ int dlpar_release_drc(u32 drc_index) > return 0; > } > > +static int dlpar_pmt(struct pseries_hp_errorlog *work); > + > static int handle_dlpar_errorlog(struct pseries_hp_errorlog *hp_elog) > { > int rc; > @@ -357,6 +360,9 @@ static int handle_dlpar_errorlog(struct pseries_hp_errorlog *hp_elog) > case PSERIES_HP_ELOG_RESOURCE_CPU: > rc = dlpar_cpu(hp_elog); > break; > + case PSERIES_HP_ELOG_RESOURCE_PMT: > + rc = dlpar_pmt(hp_elog); > + break; > default: > pr_warn_ratelimited("Invalid resource (%d) specified\n", > hp_elog->resource); > @@ -407,6 +413,61 @@ void queue_hotplug_event(struct pseries_hp_errorlog *hp_errlog, > } > } > > +LIST_HEAD(dlpar_delayed_list); > + > +int dlpar_queue_action(int resource, int action, u32 drc_index) > +{ > + struct pseries_hp_errorlog *hp_errlog; > + > + hp_errlog = kmalloc(sizeof(struct pseries_hp_errorlog), GFP_KERNEL); > + if (!hp_errlog) > + return -ENOMEM; > + > + hp_errlog->resource = resource; > + hp_errlog->action = action; > + hp_errlog->id_type = PSERIES_HP_ELOG_ID_DRC_INDEX; > + hp_errlog->_drc_u.drc_index = cpu_to_be32(drc_index); > + > + list_add_tail(&hp_errlog->list, &dlpar_delayed_list); > + > + return 0; > +} > + > +static int dlpar_pmt(struct pseries_hp_errorlog *work) > +{ > + struct list_head *pos, *q; > + > + ssleep(15); > + > + list_for_each_safe(pos, q, &dlpar_delayed_list) { > + struct pseries_hp_errorlog *tmp; > + > + tmp = list_entry(pos, struct pseries_hp_errorlog, list); > + handle_dlpar_errorlog(tmp); > + > + list_del(pos); > + kfree(tmp); > + > + ssleep(10); > + } > + > + return 0; > +} > + > +int dlpar_queued_actions_run(void) > +{ > + if (!list_empty(&dlpar_delayed_list)) { > + struct pseries_hp_errorlog hp_errlog; > + > + hp_errlog.resource = PSERIES_HP_ELOG_RESOURCE_PMT; > + hp_errlog.action = 0; > + hp_errlog.id_type = 0; > + > + queue_hotplug_event(&hp_errlog, 0, 0); > + } > + return 0; > +} I'm a bit confused by this. Is there a reason this needs to queue a hotplug event instead of just walking the list as is done in dlpar_pmt? -Nathan > + > static int dlpar_parse_resource(char **cmd, struct pseries_hp_errorlog *hp_elog) > { > char *arg; > diff --git a/arch/powerpc/platforms/pseries/mobility.c b/arch/powerpc/platforms/pseries/mobility.c > index f6364d9..d0d1cae 100644 > --- a/arch/powerpc/platforms/pseries/mobility.c > +++ b/arch/powerpc/platforms/pseries/mobility.c > @@ -378,6 +378,10 @@ static ssize_t migration_store(struct class *class, > return rc; > > post_mobility_fixup(); > + > + /* Apply any necessary changes identified during fixup */ > + dlpar_queued_actions_run(); > + > return count; > } > > diff --git a/arch/powerpc/platforms/pseries/pseries.h b/arch/powerpc/platforms/pseries/pseries.h > index 60db2ee..72ca996 100644 > --- a/arch/powerpc/platforms/pseries/pseries.h > +++ b/arch/powerpc/platforms/pseries/pseries.h > @@ -61,6 +61,8 @@ extern struct device_node *dlpar_configure_connector(__be32, > > void queue_hotplug_event(struct pseries_hp_errorlog *hp_errlog, > struct completion *hotplug_done, int *rc); > +int dlpar_queue_action(int resource, int action, u32 drc_index); > +int dlpar_queued_actions_run(void); > #ifdef CONFIG_MEMORY_HOTPLUG > int dlpar_memory(struct pseries_hp_errorlog *hp_elog); > #else >
See below. On 07/23/2018 10:54 AM, John Allen wrote: > On Fri, Jul 13, 2018 at 03:18:01PM -0500, Michael Bringmann wrote: >> migration/dlpar: This patch adds function dlpar_queue_action() >> which will queued up information about a CPU/Memory 'readd' >> operation according to resource type, action code, and DRC index. >> At a subsequent point, the list of operations can be run/played >> in series. Examples of such oprations include 'readd' of CPU >> and Memory blocks identified as having changed their associativity >> during an LPAR migration event. >> >> Signed-off-by: Michael Bringmann <mwb@linux.vnet.ibm.com> >> --- >> Changes in patch: >> -- Correct drc_index before adding to pseries_hp_errorlog struct >> -- Correct text of notice >> -- Revise queuing model to save up all of the DLPAR actions for >> later execution. >> -- Restore list init statement missing from patch >> -- Move call to apply queued operations into 'mobility.c' >> -- Compress some code >> -- Rename some of queueing function APIs >> -- Revise implementation to push execution of queued operations >> to a workqueue task. >> -- Cleanup reference to outdated queuing operation. >> --- >> arch/powerpc/include/asm/rtas.h | 2 + >> arch/powerpc/platforms/pseries/dlpar.c | 61 +++++++++++++++++++++++++++++ >> arch/powerpc/platforms/pseries/mobility.c | 4 ++ >> arch/powerpc/platforms/pseries/pseries.h | 2 + >> 4 files changed, 69 insertions(+) >> >> diff --git a/arch/powerpc/include/asm/rtas.h b/arch/powerpc/include/asm/rtas.h >> index 71e393c..4f601c7 100644 >> --- a/arch/powerpc/include/asm/rtas.h >> +++ b/arch/powerpc/include/asm/rtas.h >> @@ -310,12 +310,14 @@ struct pseries_hp_errorlog { >> struct { __be32 count, index; } ic; >> char drc_name[1]; >> } _drc_u; >> + struct list_head list; >> }; >> >> #define PSERIES_HP_ELOG_RESOURCE_CPU 1 >> #define PSERIES_HP_ELOG_RESOURCE_MEM 2 >> #define PSERIES_HP_ELOG_RESOURCE_SLOT 3 >> #define PSERIES_HP_ELOG_RESOURCE_PHB 4 >> +#define PSERIES_HP_ELOG_RESOURCE_PMT 5 >> >> #define PSERIES_HP_ELOG_ACTION_ADD 1 >> #define PSERIES_HP_ELOG_ACTION_REMOVE 2 >> diff --git a/arch/powerpc/platforms/pseries/dlpar.c b/arch/powerpc/platforms/pseries/dlpar.c >> index a0b20c0..7264b8e 100644 >> --- a/arch/powerpc/platforms/pseries/dlpar.c >> +++ b/arch/powerpc/platforms/pseries/dlpar.c >> @@ -25,6 +25,7 @@ >> #include <asm/prom.h> >> #include <asm/machdep.h> >> #include <linux/uaccess.h> >> +#include <linux/delay.h> >> #include <asm/rtas.h> >> >> static struct workqueue_struct *pseries_hp_wq; >> @@ -329,6 +330,8 @@ int dlpar_release_drc(u32 drc_index) >> return 0; >> } >> >> +static int dlpar_pmt(struct pseries_hp_errorlog *work); >> + >> static int handle_dlpar_errorlog(struct pseries_hp_errorlog *hp_elog) >> { >> int rc; >> @@ -357,6 +360,9 @@ static int handle_dlpar_errorlog(struct pseries_hp_errorlog *hp_elog) >> case PSERIES_HP_ELOG_RESOURCE_CPU: >> rc = dlpar_cpu(hp_elog); >> break; >> + case PSERIES_HP_ELOG_RESOURCE_PMT: >> + rc = dlpar_pmt(hp_elog); >> + break; >> default: >> pr_warn_ratelimited("Invalid resource (%d) specified\n", >> hp_elog->resource); >> @@ -407,6 +413,61 @@ void queue_hotplug_event(struct pseries_hp_errorlog *hp_errlog, >> } >> } >> >> +LIST_HEAD(dlpar_delayed_list); >> + >> +int dlpar_queue_action(int resource, int action, u32 drc_index) >> +{ >> + struct pseries_hp_errorlog *hp_errlog; >> + >> + hp_errlog = kmalloc(sizeof(struct pseries_hp_errorlog), GFP_KERNEL); >> + if (!hp_errlog) >> + return -ENOMEM; >> + >> + hp_errlog->resource = resource; >> + hp_errlog->action = action; >> + hp_errlog->id_type = PSERIES_HP_ELOG_ID_DRC_INDEX; >> + hp_errlog->_drc_u.drc_index = cpu_to_be32(drc_index); >> + >> + list_add_tail(&hp_errlog->list, &dlpar_delayed_list); >> + >> + return 0; >> +} >> + >> +static int dlpar_pmt(struct pseries_hp_errorlog *work) >> +{ >> + struct list_head *pos, *q; >> + >> + ssleep(15); > > Why do we need to sleep for so long here? One or more drivers were finishing their initialization about the same time as the 'migration_store' operation was completing. E.g. 'ibmvscsi' When we did 'dlpar cpu readd' operations at the same time early on in testing, the errors / warnings were inconsistent. I put in a delay to get beyond the driver re-init after migration. 15 seconds was an estimate. As it occurs after the completion of the 'migration store' operation, it does not delay any responses returned to the HMC. We can rerun tests with reduced / removed delays to see if they are still necessary given some other corrections in the mix. > > -John Michael >> + >> + list_for_each_safe(pos, q, &dlpar_delayed_list) { >> + struct pseries_hp_errorlog *tmp; >> + >> + tmp = list_entry(pos, struct pseries_hp_errorlog, list); >> + handle_dlpar_errorlog(tmp); >> + >> + list_del(pos); >> + kfree(tmp); >> + >> + ssleep(10); >> + } >> + >> + return 0; >> +} >> + >> +int dlpar_queued_actions_run(void) >> +{ >> + if (!list_empty(&dlpar_delayed_list)) { >> + struct pseries_hp_errorlog hp_errlog; >> + >> + hp_errlog.resource = PSERIES_HP_ELOG_RESOURCE_PMT; >> + hp_errlog.action = 0; >> + hp_errlog.id_type = 0; >> + >> + queue_hotplug_event(&hp_errlog, 0, 0); >> + } >> + return 0; >> +} >> + >> static int dlpar_parse_resource(char **cmd, struct pseries_hp_errorlog *hp_elog) >> { >> char *arg; >> diff --git a/arch/powerpc/platforms/pseries/mobility.c b/arch/powerpc/platforms/pseries/mobility.c >> index f6364d9..d0d1cae 100644 >> --- a/arch/powerpc/platforms/pseries/mobility.c >> +++ b/arch/powerpc/platforms/pseries/mobility.c >> @@ -378,6 +378,10 @@ static ssize_t migration_store(struct class *class, >> return rc; >> >> post_mobility_fixup(); >> + >> + /* Apply any necessary changes identified during fixup */ >> + dlpar_queued_actions_run(); >> + >> return count; >> } >> >> diff --git a/arch/powerpc/platforms/pseries/pseries.h b/arch/powerpc/platforms/pseries/pseries.h >> index 60db2ee..72ca996 100644 >> --- a/arch/powerpc/platforms/pseries/pseries.h >> +++ b/arch/powerpc/platforms/pseries/pseries.h >> @@ -61,6 +61,8 @@ extern struct device_node *dlpar_configure_connector(__be32, >> >> void queue_hotplug_event(struct pseries_hp_errorlog *hp_errlog, >> struct completion *hotplug_done, int *rc); >> +int dlpar_queue_action(int resource, int action, u32 drc_index); >> +int dlpar_queued_actions_run(void); >> #ifdef CONFIG_MEMORY_HOTPLUG >> int dlpar_memory(struct pseries_hp_errorlog *hp_elog); >> #else >> > >
See below. On 07/23/2018 12:51 PM, Nathan Fontenot wrote: > On 07/13/2018 03:18 PM, Michael Bringmann wrote: >> migration/dlpar: This patch adds function dlpar_queue_action() >> which will queued up information about a CPU/Memory 'readd' >> operation according to resource type, action code, and DRC index. >> At a subsequent point, the list of operations can be run/played >> in series. Examples of such oprations include 'readd' of CPU >> and Memory blocks identified as having changed their associativity >> during an LPAR migration event. > >> Signed-off-by: Michael Bringmann <mwb@linux.vnet.ibm.com> >> --- >> Changes in patch: >> -- Correct drc_index before adding to pseries_hp_errorlog struct >> -- Correct text of notice >> -- Revise queuing model to save up all of the DLPAR actions for >> later execution. >> -- Restore list init statement missing from patch >> -- Move call to apply queued operations into 'mobility.c' >> -- Compress some code >> -- Rename some of queueing function APIs >> -- Revise implementation to push execution of queued operations >> to a workqueue task. >> -- Cleanup reference to outdated queuing operation. >> --- >> arch/powerpc/include/asm/rtas.h | 2 + >> arch/powerpc/platforms/pseries/dlpar.c | 61 +++++++++++++++++++++++++++++ >> arch/powerpc/platforms/pseries/mobility.c | 4 ++ >> arch/powerpc/platforms/pseries/pseries.h | 2 + >> 4 files changed, 69 insertions(+) >> >> diff --git a/arch/powerpc/include/asm/rtas.h b/arch/powerpc/include/asm/rtas.h >> index 71e393c..4f601c7 100644 >> --- a/arch/powerpc/include/asm/rtas.h >> +++ b/arch/powerpc/include/asm/rtas.h >> @@ -310,12 +310,14 @@ struct pseries_hp_errorlog { >> struct { __be32 count, index; } ic; >> char drc_name[1]; >> } _drc_u; >> + struct list_head list; >> }; >> >> #define PSERIES_HP_ELOG_RESOURCE_CPU 1 >> #define PSERIES_HP_ELOG_RESOURCE_MEM 2 >> #define PSERIES_HP_ELOG_RESOURCE_SLOT 3 >> #define PSERIES_HP_ELOG_RESOURCE_PHB 4 >> +#define PSERIES_HP_ELOG_RESOURCE_PMT 5 >> >> #define PSERIES_HP_ELOG_ACTION_ADD 1 >> #define PSERIES_HP_ELOG_ACTION_REMOVE 2 >> diff --git a/arch/powerpc/platforms/pseries/dlpar.c b/arch/powerpc/platforms/pseries/dlpar.c >> index a0b20c0..7264b8e 100644 >> --- a/arch/powerpc/platforms/pseries/dlpar.c >> +++ b/arch/powerpc/platforms/pseries/dlpar.c >> @@ -25,6 +25,7 @@ >> #include <asm/prom.h> >> #include <asm/machdep.h> >> #include <linux/uaccess.h> >> +#include <linux/delay.h> >> #include <asm/rtas.h> >> >> static struct workqueue_struct *pseries_hp_wq; >> @@ -329,6 +330,8 @@ int dlpar_release_drc(u32 drc_index) >> return 0; >> } >> >> +static int dlpar_pmt(struct pseries_hp_errorlog *work); >> + >> static int handle_dlpar_errorlog(struct pseries_hp_errorlog *hp_elog) >> { >> int rc; >> @@ -357,6 +360,9 @@ static int handle_dlpar_errorlog(struct pseries_hp_errorlog *hp_elog) >> case PSERIES_HP_ELOG_RESOURCE_CPU: >> rc = dlpar_cpu(hp_elog); >> break; >> + case PSERIES_HP_ELOG_RESOURCE_PMT: >> + rc = dlpar_pmt(hp_elog); >> + break; >> default: >> pr_warn_ratelimited("Invalid resource (%d) specified\n", >> hp_elog->resource); >> @@ -407,6 +413,61 @@ void queue_hotplug_event(struct pseries_hp_errorlog *hp_errlog, >> } >> } >> >> +LIST_HEAD(dlpar_delayed_list); >> + >> +int dlpar_queue_action(int resource, int action, u32 drc_index) >> +{ >> + struct pseries_hp_errorlog *hp_errlog; >> + >> + hp_errlog = kmalloc(sizeof(struct pseries_hp_errorlog), GFP_KERNEL); >> + if (!hp_errlog) >> + return -ENOMEM; >> + >> + hp_errlog->resource = resource; >> + hp_errlog->action = action; >> + hp_errlog->id_type = PSERIES_HP_ELOG_ID_DRC_INDEX; >> + hp_errlog->_drc_u.drc_index = cpu_to_be32(drc_index); >> + >> + list_add_tail(&hp_errlog->list, &dlpar_delayed_list); >> + >> + return 0; >> +} >> + >> +static int dlpar_pmt(struct pseries_hp_errorlog *work) >> +{ >> + struct list_head *pos, *q; >> + >> + ssleep(15); >> + >> + list_for_each_safe(pos, q, &dlpar_delayed_list) { >> + struct pseries_hp_errorlog *tmp; >> + >> + tmp = list_entry(pos, struct pseries_hp_errorlog, list); >> + handle_dlpar_errorlog(tmp); >> + >> + list_del(pos); >> + kfree(tmp); >> + >> + ssleep(10); >> + } >> + >> + return 0; >> +} >> + >> +int dlpar_queued_actions_run(void) >> +{ >> + if (!list_empty(&dlpar_delayed_list)) { >> + struct pseries_hp_errorlog hp_errlog; >> + >> + hp_errlog.resource = PSERIES_HP_ELOG_RESOURCE_PMT; >> + hp_errlog.action = 0; >> + hp_errlog.id_type = 0; >> + >> + queue_hotplug_event(&hp_errlog, 0, 0); > + } >> + return 0; >> +} > > I'm a bit confused by this. Is there a reason this needs to queue a > hotplug event instead of just walking the list as is done in dlpar_pmt? Up to this point, the operations have only been added to 'dlpar_delayed_list'. This function separates the execution of the CPU readd and Memory readd operations from the execution of 'migration store'. If we walk the list here, then we add the execution time of all of the readd operations to the time of 'migration store'. This is not a large problem in small systems like we have in the Kernel Team. This may be a major issue though, for production SAP HANA systems where we may be readding thousands of pages of memory. By pushing the execution of the CPU readd and Memory readd operations after, and separate, from the execution of 'migration store', we do not delay the end of the operation or the return of the completion status to an associated HMC. > > -Nathan Michael > >> + >> static int dlpar_parse_resource(char **cmd, struct pseries_hp_errorlog *hp_elog) >> { >> char *arg; >> diff --git a/arch/powerpc/platforms/pseries/mobility.c b/arch/powerpc/platforms/pseries/mobility.c >> index f6364d9..d0d1cae 100644 >> --- a/arch/powerpc/platforms/pseries/mobility.c >> +++ b/arch/powerpc/platforms/pseries/mobility.c >> @@ -378,6 +378,10 @@ static ssize_t migration_store(struct class *class, >> return rc; >> >> post_mobility_fixup(); >> + >> + /* Apply any necessary changes identified during fixup */ >> + dlpar_queued_actions_run(); >> + >> return count; >> } >> >> diff --git a/arch/powerpc/platforms/pseries/pseries.h b/arch/powerpc/platforms/pseries/pseries.h >> index 60db2ee..72ca996 100644 >> --- a/arch/powerpc/platforms/pseries/pseries.h >> +++ b/arch/powerpc/platforms/pseries/pseries.h >> @@ -61,6 +61,8 @@ extern struct device_node *dlpar_configure_connector(__be32, >> >> void queue_hotplug_event(struct pseries_hp_errorlog *hp_errlog, >> struct completion *hotplug_done, int *rc); >> +int dlpar_queue_action(int resource, int action, u32 drc_index); >> +int dlpar_queued_actions_run(void); >> #ifdef CONFIG_MEMORY_HOTPLUG >> int dlpar_memory(struct pseries_hp_errorlog *hp_elog); >> #else >> > >
Michael Bringmann <mwb@linux.vnet.ibm.com> writes: > On 07/23/2018 10:54 AM, John Allen wrote: >> On Fri, Jul 13, 2018 at 03:18:01PM -0500, Michael Bringmann wrote: >>> @@ -407,6 +413,61 @@ void queue_hotplug_event(struct pseries_hp_errorlog *hp_errlog, ... >>> + >>> +static int dlpar_pmt(struct pseries_hp_errorlog *work) >>> +{ >>> + struct list_head *pos, *q; >>> + >>> + ssleep(15); >> >> Why do we need to sleep for so long here? > > One or more drivers were finishing their initialization about the same > time as the 'migration_store' operation was completing. E.g. 'ibmvscsi' > When we did 'dlpar cpu readd' operations at the same time early on in > testing, the errors / warnings were inconsistent. I put in a delay to > get beyond the driver re-init after migration. 15 seconds was an estimate. > As it occurs after the completion of the 'migration store' operation, > it does not delay any responses returned to the HMC. > > We can rerun tests with reduced / removed delays to see if they are still > necessary given some other corrections in the mix. Please do. I'm not inclined to merge code with "sleep 15" in it, unless there's some incredibly good reason for it. cheers
Michael Bringmann <mwb@linux.vnet.ibm.com> writes: > On 07/23/2018 12:51 PM, Nathan Fontenot wrote: >> On 07/13/2018 03:18 PM, Michael Bringmann wrote: ... >>> + >>> +int dlpar_queued_actions_run(void) >>> +{ >>> + if (!list_empty(&dlpar_delayed_list)) { >>> + struct pseries_hp_errorlog hp_errlog; >>> + >>> + hp_errlog.resource = PSERIES_HP_ELOG_RESOURCE_PMT; >>> + hp_errlog.action = 0; >>> + hp_errlog.id_type = 0; >>> + >>> + queue_hotplug_event(&hp_errlog, 0, 0); > + } >>> + return 0; >>> +} >> >> I'm a bit confused by this. Is there a reason this needs to queue a >> hotplug event instead of just walking the list as is done in dlpar_pmt? > > Up to this point, the operations have only been added to 'dlpar_delayed_list'. > This function separates the execution of the CPU readd and Memory readd > operations from the execution of 'migration store'. If we walk the list > here, then we add the execution time of all of the readd operations to > the time of 'migration store'. This is not a large problem in small > systems like we have in the Kernel Team. This may be a major issue though, > for production SAP HANA systems where we may be readding thousands of pages > of memory. By pushing the execution of the CPU readd and Memory readd > operations after, and separate, from the execution of 'migration store', > we do not delay the end of the operation or the return of the completion > status to an associated HMC. But you can't return completion status, because the operation hasn't completed. The migration isn't finished until we've updated the topology for the new system. If you decouple things like this you now have no way of reporting progress or an error to the caller. So I'm unconvinced this is the right solution. cheers
diff --git a/arch/powerpc/include/asm/rtas.h b/arch/powerpc/include/asm/rtas.h index 71e393c..4f601c7 100644 --- a/arch/powerpc/include/asm/rtas.h +++ b/arch/powerpc/include/asm/rtas.h @@ -310,12 +310,14 @@ struct pseries_hp_errorlog { struct { __be32 count, index; } ic; char drc_name[1]; } _drc_u; + struct list_head list; }; #define PSERIES_HP_ELOG_RESOURCE_CPU 1 #define PSERIES_HP_ELOG_RESOURCE_MEM 2 #define PSERIES_HP_ELOG_RESOURCE_SLOT 3 #define PSERIES_HP_ELOG_RESOURCE_PHB 4 +#define PSERIES_HP_ELOG_RESOURCE_PMT 5 #define PSERIES_HP_ELOG_ACTION_ADD 1 #define PSERIES_HP_ELOG_ACTION_REMOVE 2 diff --git a/arch/powerpc/platforms/pseries/dlpar.c b/arch/powerpc/platforms/pseries/dlpar.c index a0b20c0..7264b8e 100644 --- a/arch/powerpc/platforms/pseries/dlpar.c +++ b/arch/powerpc/platforms/pseries/dlpar.c @@ -25,6 +25,7 @@ #include <asm/prom.h> #include <asm/machdep.h> #include <linux/uaccess.h> +#include <linux/delay.h> #include <asm/rtas.h> static struct workqueue_struct *pseries_hp_wq; @@ -329,6 +330,8 @@ int dlpar_release_drc(u32 drc_index) return 0; } +static int dlpar_pmt(struct pseries_hp_errorlog *work); + static int handle_dlpar_errorlog(struct pseries_hp_errorlog *hp_elog) { int rc; @@ -357,6 +360,9 @@ static int handle_dlpar_errorlog(struct pseries_hp_errorlog *hp_elog) case PSERIES_HP_ELOG_RESOURCE_CPU: rc = dlpar_cpu(hp_elog); break; + case PSERIES_HP_ELOG_RESOURCE_PMT: + rc = dlpar_pmt(hp_elog); + break; default: pr_warn_ratelimited("Invalid resource (%d) specified\n", hp_elog->resource); @@ -407,6 +413,61 @@ void queue_hotplug_event(struct pseries_hp_errorlog *hp_errlog, } } +LIST_HEAD(dlpar_delayed_list); + +int dlpar_queue_action(int resource, int action, u32 drc_index) +{ + struct pseries_hp_errorlog *hp_errlog; + + hp_errlog = kmalloc(sizeof(struct pseries_hp_errorlog), GFP_KERNEL); + if (!hp_errlog) + return -ENOMEM; + + hp_errlog->resource = resource; + hp_errlog->action = action; + hp_errlog->id_type = PSERIES_HP_ELOG_ID_DRC_INDEX; + hp_errlog->_drc_u.drc_index = cpu_to_be32(drc_index); + + list_add_tail(&hp_errlog->list, &dlpar_delayed_list); + + return 0; +} + +static int dlpar_pmt(struct pseries_hp_errorlog *work) +{ + struct list_head *pos, *q; + + ssleep(15); + + list_for_each_safe(pos, q, &dlpar_delayed_list) { + struct pseries_hp_errorlog *tmp; + + tmp = list_entry(pos, struct pseries_hp_errorlog, list); + handle_dlpar_errorlog(tmp); + + list_del(pos); + kfree(tmp); + + ssleep(10); + } + + return 0; +} + +int dlpar_queued_actions_run(void) +{ + if (!list_empty(&dlpar_delayed_list)) { + struct pseries_hp_errorlog hp_errlog; + + hp_errlog.resource = PSERIES_HP_ELOG_RESOURCE_PMT; + hp_errlog.action = 0; + hp_errlog.id_type = 0; + + queue_hotplug_event(&hp_errlog, 0, 0); + } + return 0; +} + static int dlpar_parse_resource(char **cmd, struct pseries_hp_errorlog *hp_elog) { char *arg; diff --git a/arch/powerpc/platforms/pseries/mobility.c b/arch/powerpc/platforms/pseries/mobility.c index f6364d9..d0d1cae 100644 --- a/arch/powerpc/platforms/pseries/mobility.c +++ b/arch/powerpc/platforms/pseries/mobility.c @@ -378,6 +378,10 @@ static ssize_t migration_store(struct class *class, return rc; post_mobility_fixup(); + + /* Apply any necessary changes identified during fixup */ + dlpar_queued_actions_run(); + return count; } diff --git a/arch/powerpc/platforms/pseries/pseries.h b/arch/powerpc/platforms/pseries/pseries.h index 60db2ee..72ca996 100644 --- a/arch/powerpc/platforms/pseries/pseries.h +++ b/arch/powerpc/platforms/pseries/pseries.h @@ -61,6 +61,8 @@ extern struct device_node *dlpar_configure_connector(__be32, void queue_hotplug_event(struct pseries_hp_errorlog *hp_errlog, struct completion *hotplug_done, int *rc); +int dlpar_queue_action(int resource, int action, u32 drc_index); +int dlpar_queued_actions_run(void); #ifdef CONFIG_MEMORY_HOTPLUG int dlpar_memory(struct pseries_hp_errorlog *hp_elog); #else
migration/dlpar: This patch adds function dlpar_queue_action() which will queued up information about a CPU/Memory 'readd' operation according to resource type, action code, and DRC index. At a subsequent point, the list of operations can be run/played in series. Examples of such oprations include 'readd' of CPU and Memory blocks identified as having changed their associativity during an LPAR migration event. Signed-off-by: Michael Bringmann <mwb@linux.vnet.ibm.com> --- Changes in patch: -- Correct drc_index before adding to pseries_hp_errorlog struct -- Correct text of notice -- Revise queuing model to save up all of the DLPAR actions for later execution. -- Restore list init statement missing from patch -- Move call to apply queued operations into 'mobility.c' -- Compress some code -- Rename some of queueing function APIs -- Revise implementation to push execution of queued operations to a workqueue task. -- Cleanup reference to outdated queuing operation. --- arch/powerpc/include/asm/rtas.h | 2 + arch/powerpc/platforms/pseries/dlpar.c | 61 +++++++++++++++++++++++++++++ arch/powerpc/platforms/pseries/mobility.c | 4 ++ arch/powerpc/platforms/pseries/pseries.h | 2 + 4 files changed, 69 insertions(+)