| Message ID | 20180808152926.28842-2-jallen@linux.ibm.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | powerpc/pseries: Improve serialization of PRRN events | expand |
| Context | Check | Description |
|---|---|---|
| snowpatch_ozlabs/apply_patch | success | next/apply_patch Successfully applied |
| snowpatch_ozlabs/checkpatch | success | Test checkpatch on branch next |
John Allen <jallen@linux.ibm.com> writes: > When a PRRN event is being handled and another PRRN event comes in, the > second event will block rtas polling waiting on the first to complete, > preventing any further rtas events from being handled. This can be > especially problematic in case that PRRN events are continuously being > queued in which case rtas polling gets indefinitely blocked completely. > > This patch removes the blocking call to flush_work and allows the > default workqueue behavior to handle duplicate events. > > Signed-off-by: John Allen <jallen@linux.ibm.com> > --- > v3: > -Scrap the mutex as it only replicates existing workqueue behavior. > v2: > -Unlock prrn_lock when PRRN operations are complete, not after handler is > scheduled. > -Remove call to flush_work, the previous broken method of serializing > PRRN events. > --- > arch/powerpc/kernel/rtasd.c | 1 - > 1 file changed, 1 deletion(-) > > diff --git a/arch/powerpc/kernel/rtasd.c b/arch/powerpc/kernel/rtasd.c > index 44d66c33d59d..2017934e5985 100644 > --- a/arch/powerpc/kernel/rtasd.c > +++ b/arch/powerpc/kernel/rtasd.c > @@ -290,7 +290,6 @@ static DECLARE_WORK(prrn_work, prrn_work_fn); > > static void prrn_schedule_update(u32 scope) > { > - flush_work(&prrn_work); > prrn_update_scope = scope; > schedule_work(&prrn_work); > } I think we can still lose a "scope" value here. ie. prrn_schedule_update(1) prrn_update_scope = 1; schedule_work(&prrn_work); // scheduled but doesn't run yet ... prrn_schedule_update(2) prrn_update_scope = 2; schedule_work(&prrn_work); // it's still scheduled // finally prrn_work_fn(..) pseries_devicetree_update(-prrn_update_scope); // aka 2 So we lost the "1" value. We could fix that by allocating a work struct that holds the scope value. But I'd really like to try just calling the prrn_work_fn() directly, we're already running as a scheduled work function, I don't see any benefit in scheduling more work. cheers
diff --git a/arch/powerpc/kernel/rtasd.c b/arch/powerpc/kernel/rtasd.c index 44d66c33d59d..2017934e5985 100644 --- a/arch/powerpc/kernel/rtasd.c +++ b/arch/powerpc/kernel/rtasd.c @@ -290,7 +290,6 @@ static DECLARE_WORK(prrn_work, prrn_work_fn); static void prrn_schedule_update(u32 scope) { - flush_work(&prrn_work); prrn_update_scope = scope; schedule_work(&prrn_work); }
When a PRRN event is being handled and another PRRN event comes in, the second event will block rtas polling waiting on the first to complete, preventing any further rtas events from being handled. This can be especially problematic in case that PRRN events are continuously being queued in which case rtas polling gets indefinitely blocked completely. This patch removes the blocking call to flush_work and allows the default workqueue behavior to handle duplicate events. Signed-off-by: John Allen <jallen@linux.ibm.com> --- v3: -Scrap the mutex as it only replicates existing workqueue behavior. v2: -Unlock prrn_lock when PRRN operations are complete, not after handler is scheduled. -Remove call to flush_work, the previous broken method of serializing PRRN events. --- arch/powerpc/kernel/rtasd.c | 1 - 1 file changed, 1 deletion(-)