| Message ID | 1533269016-16238-1-git-send-email-arbab@linux.ibm.com (mailing list archive) |
|---|---|
| State | Accepted |
| Commit | 9eab9901b015f489199105c470de1ffc337cfabb |
| Headers | show |
| Series | powerpc/powernv: Fix concurrency issue with npu->mmio_atsd_usage | expand |
| Context | Check | Description |
|---|---|---|
| snowpatch_ozlabs/apply_patch | success | next/apply_patch Successfully applied |
| snowpatch_ozlabs/checkpatch | success | Test checkpatch on branch next |
| snowpatch_ozlabs/build-ppc64le | success | Test build-ppc64le on branch next |
| snowpatch_ozlabs/build-ppc64be | success | Test build-ppc64be on branch next |
| snowpatch_ozlabs/build-ppc64e | success | Test build-ppc64e on branch next |
| snowpatch_ozlabs/build-ppc32 | success | Test build-ppc32 on branch next |
> There may be a long-term way to fix this at a larger scale, but for now > resolve the immediate problem by gating our call to > test_and_set_bit_lock() with one to test_bit(), which is obviously > implemented without using a store. I am less sure of this now but am continuing to investigate. However this patch looks good. Acked-by: Alistair Popple <alistair@popple.id.au> > Signed-off-by: Reza Arbab <arbab@linux.ibm.com> > --- > arch/powerpc/platforms/powernv/npu-dma.c | 5 +++-- > 1 file changed, 3 insertions(+), 2 deletions(-) > > diff --git a/arch/powerpc/platforms/powernv/npu-dma.c b/arch/powerpc/platforms/powernv/npu-dma.c > index 8cdf91f..c773465 100644 > --- a/arch/powerpc/platforms/powernv/npu-dma.c > +++ b/arch/powerpc/platforms/powernv/npu-dma.c > @@ -437,8 +437,9 @@ static int get_mmio_atsd_reg(struct npu *npu) > int i; > > for (i = 0; i < npu->mmio_atsd_count; i++) { > - if (!test_and_set_bit_lock(i, &npu->mmio_atsd_usage)) > - return i; > + if (!test_bit(i, &npu->mmio_atsd_usage)) > + if (!test_and_set_bit_lock(i, &npu->mmio_atsd_usage)) > + return i; > } > > return -ENOSPC; >
On Fri, 2018-08-03 at 04:03:36 UTC, Reza Arbab wrote: > We've encountered a performance issue when multiple processors stress > {get,put}_mmio_atsd_reg(). These functions contend for mmio_atsd_usage, > an unsigned long used as a bitmask. > > The accesses to mmio_atsd_usage are done using test_and_set_bit_lock() > and clear_bit_unlock(). As implemented, both of these will require a > (successful) stwcx to that same cache line. > > What we end up with is thread A, attempting to unlock, being slowed by > other threads repeatedly attempting to lock. A's stwcx instructions fail > and retry because the memory reservation is lost every time a different > thread beats it to the punch. > > There may be a long-term way to fix this at a larger scale, but for now > resolve the immediate problem by gating our call to > test_and_set_bit_lock() with one to test_bit(), which is obviously > implemented without using a store. > > Signed-off-by: Reza Arbab <arbab@linux.ibm.com> > Acked-by: Alistair Popple <alistair@popple.id.au> Applied to powerpc next, thanks. https://git.kernel.org/powerpc/c/9eab9901b015f489199105c470de1f cheers
diff --git a/arch/powerpc/platforms/powernv/npu-dma.c b/arch/powerpc/platforms/powernv/npu-dma.c index 8cdf91f..c773465 100644 --- a/arch/powerpc/platforms/powernv/npu-dma.c +++ b/arch/powerpc/platforms/powernv/npu-dma.c @@ -437,8 +437,9 @@ static int get_mmio_atsd_reg(struct npu *npu) int i; for (i = 0; i < npu->mmio_atsd_count; i++) { - if (!test_and_set_bit_lock(i, &npu->mmio_atsd_usage)) - return i; + if (!test_bit(i, &npu->mmio_atsd_usage)) + if (!test_and_set_bit_lock(i, &npu->mmio_atsd_usage)) + return i; } return -ENOSPC;
We've encountered a performance issue when multiple processors stress {get,put}_mmio_atsd_reg(). These functions contend for mmio_atsd_usage, an unsigned long used as a bitmask. The accesses to mmio_atsd_usage are done using test_and_set_bit_lock() and clear_bit_unlock(). As implemented, both of these will require a (successful) stwcx to that same cache line. What we end up with is thread A, attempting to unlock, being slowed by other threads repeatedly attempting to lock. A's stwcx instructions fail and retry because the memory reservation is lost every time a different thread beats it to the punch. There may be a long-term way to fix this at a larger scale, but for now resolve the immediate problem by gating our call to test_and_set_bit_lock() with one to test_bit(), which is obviously implemented without using a store. Signed-off-by: Reza Arbab <arbab@linux.ibm.com> --- arch/powerpc/platforms/powernv/npu-dma.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-)