On Mon, Jul 23, 2018 at 11:41:24PM +1000, Michael Ellerman wrote:
John Allen <jal...@linux.ibm.com> writes:
While handling PRRN events, the time to handle the actual hotplug events
dwarfs the time it takes to perform the device tree updates and queue the
hotplug events. In the case that PRRN events are being queued continuously,
hotplug events have been observed to be queued faster than the kernel can
actually handle them. This patch avoids the problem by waiting for a
hotplug request to complete before queueing more hotplug events.
So do we need the hotplug work queue at all? Can we just call
handle_dlpar_errorlog() directly?
Or are we using the work queue to serialise things? And if so would a
mutex be better?
Right, the workqueue is meant to serialize all hotplug events and it
gets used for more than just PRRN events. I believe the motivation for
using the workqueue over a mutex is that KVM guests initiate hotplug
events through the hotplug interrupt and can queue fairly large requests
meaning that in this scenario, waiting for a lock would block interrupts
for a while. Using the workqueue allows us to serialize hotplug events
from different sources in the same way without worrying about the
context in which the event is generated.
It looks like prrn_update_node() is called via at least, prrn_work_fn()
and post_mobility_fixup().
The latter is called from migration_store(), which seems like it would
be harmless. But also from pseries_suspend_enable_irqs() which I'm less
clear on.
Yeah, that doesn't seem to make sense based on the function name. Odd
that prrn_update_node is being called from anywhere outside of handling
PRRN events. Perhaps if other code paths are using the function, it
needs a more generic name.
-John
cheers
diff --git a/arch/powerpc/platforms/pseries/mobility.c
b/arch/powerpc/platforms/pseries/mobility.c
index 8a8033a249c7..49930848fa78 100644
--- a/arch/powerpc/platforms/pseries/mobility.c
+++ b/arch/powerpc/platforms/pseries/mobility.c
@@ -242,6 +242,7 @@ static int add_dt_node(__be32 parent_phandle, __be32
drc_index)
static void prrn_update_node(__be32 phandle)
{
struct pseries_hp_errorlog *hp_elog;
+ struct completion hotplug_done;
struct device_node *dn;
/*
@@ -263,7 +264,9 @@ static void prrn_update_node(__be32 phandle)
hp_elog->id_type = PSERIES_HP_ELOG_ID_DRC_INDEX;
hp_elog->_drc_u.drc_index = phandle;
- queue_hotplug_event(hp_elog, NULL, NULL);
+ init_completion(&hotplug_done);
+ queue_hotplug_event(hp_elog, &hotplug_done, NULL);
+ wait_for_completion(&hotplug_done);
kfree(hp_elog);
}
--
2.17.1
