From 8ac75b96be71e20ec1785ca18170890c4dfffe87 Mon Sep 17 00:00:00 2001 From: Ian Munsie Date: Fri, 8 May 2015 22:55:18 +1000 Subject: cxl: Use call_rcu to reduce latency when releasing the afu fd The afu fd release path was identified as a significant bottleneck in the overall performance of cxl. While an optimal AFU design would minimise the need to close & reopen the AFU fd, it is not always practical to avoid. The bottleneck seems to be down to the call to synchronize_rcu(), which will block until every other thread is guaranteed to be out of an RCU critical section. Replace it with call_rcu() to free the context structures later so we can return to the application sooner. This reduces the time spent in the fd release path from 13356 usec to 13.3 usec - about a 100x speed up. Reported-by: Fei K Chen Signed-off-by: Ian Munsie Signed-off-by: Michael Ellerman --- drivers/misc/cxl/context.c | 15 ++++++++++----- 1 file changed, 10 insertions(+), 5 deletions(-) (limited to 'drivers/misc/cxl/context.c') diff --git a/drivers/misc/cxl/context.c b/drivers/misc/cxl/context.c index d1b55fe62817..78ce990d77ca 100644 --- a/drivers/misc/cxl/context.c +++ b/drivers/misc/cxl/context.c @@ -232,12 +232,9 @@ void cxl_context_detach_all(struct cxl_afu *afu) mutex_unlock(&afu->contexts_lock); } -void cxl_context_free(struct cxl_context *ctx) +static void reclaim_ctx(struct rcu_head *rcu) { - mutex_lock(&ctx->afu->contexts_lock); - idr_remove(&ctx->afu->contexts_idr, ctx->pe); - mutex_unlock(&ctx->afu->contexts_lock); - synchronize_rcu(); + struct cxl_context *ctx = container_of(rcu, struct cxl_context, rcu); free_page((u64)ctx->sstp); ctx->sstp = NULL; @@ -245,3 +242,11 @@ void cxl_context_free(struct cxl_context *ctx) put_pid(ctx->pid); kfree(ctx); } + +void cxl_context_free(struct cxl_context *ctx) +{ + mutex_lock(&ctx->afu->contexts_lock); + idr_remove(&ctx->afu->contexts_idr, ctx->pe); + mutex_unlock(&ctx->afu->contexts_lock); + call_rcu(&ctx->rcu, reclaim_ctx); +} -- cgit v1.2.2 From 6428832a7bfae73345706d63a228a6ce60af0081 Mon Sep 17 00:00:00 2001 From: Michael Neuling Date: Wed, 27 May 2015 16:07:07 +1000 Subject: cxl: Add cookie parameter to afu_release_irqs() Add cookie parameter to afu_release_irqs() so that we can pass in a different cookie than the context structure. This will be useful for other kernel drivers that want to call this but get their own cookie back in the interrupt handler. Update all existing call sites. Signed-off-by: Michael Neuling Acked-by: Ian Munsie Signed-off-by: Michael Ellerman --- drivers/misc/cxl/context.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'drivers/misc/cxl/context.c') diff --git a/drivers/misc/cxl/context.c b/drivers/misc/cxl/context.c index 78ce990d77ca..36bb8e4195d5 100644 --- a/drivers/misc/cxl/context.c +++ b/drivers/misc/cxl/context.c @@ -186,7 +186,7 @@ static void __detach_context(struct cxl_context *ctx) return; WARN_ON(cxl_detach_process(ctx)); - afu_release_irqs(ctx); + afu_release_irqs(ctx, ctx); flush_work(&ctx->fault_work); /* Only needed for dedicated process */ wake_up_all(&ctx->wq); } -- cgit v1.2.2 From eda3693c842ed169af66af943554c648633769d0 Mon Sep 17 00:00:00 2001 From: Michael Neuling Date: Wed, 27 May 2015 16:07:08 +1000 Subject: cxl: Rework detach context functions Rework __detach_context() and cxl_context_detach() so we can reuse them in the kernel API. Signed-off-by: Michael Neuling Acked-by: Ian Munsie Signed-off-by: Michael Ellerman --- drivers/misc/cxl/context.c | 20 +++++++++++++------- 1 file changed, 13 insertions(+), 7 deletions(-) (limited to 'drivers/misc/cxl/context.c') diff --git a/drivers/misc/cxl/context.c b/drivers/misc/cxl/context.c index 36bb8e4195d5..7d857b7d686d 100644 --- a/drivers/misc/cxl/context.c +++ b/drivers/misc/cxl/context.c @@ -174,7 +174,7 @@ int cxl_context_iomap(struct cxl_context *ctx, struct vm_area_struct *vma) * return until all outstanding interrupts for this context have completed. The * hardware should no longer access *ctx after this has returned. */ -static void __detach_context(struct cxl_context *ctx) +int __detach_context(struct cxl_context *ctx) { enum cxl_context_status status; @@ -183,12 +183,10 @@ static void __detach_context(struct cxl_context *ctx) ctx->status = CLOSED; mutex_unlock(&ctx->status_mutex); if (status != STARTED) - return; + return -EBUSY; WARN_ON(cxl_detach_process(ctx)); - afu_release_irqs(ctx, ctx); - flush_work(&ctx->fault_work); /* Only needed for dedicated process */ - wake_up_all(&ctx->wq); + return 0; } /* @@ -199,7 +197,15 @@ static void __detach_context(struct cxl_context *ctx) */ void cxl_context_detach(struct cxl_context *ctx) { - __detach_context(ctx); + int rc; + + rc = __detach_context(ctx); + if (rc) + return; + + afu_release_irqs(ctx, ctx); + flush_work(&ctx->fault_work); /* Only needed for dedicated process */ + wake_up_all(&ctx->wq); } /* @@ -216,7 +222,7 @@ void cxl_context_detach_all(struct cxl_afu *afu) * Anything done in here needs to be setup before the IDR is * created and torn down after the IDR removed */ - __detach_context(ctx); + cxl_context_detach(ctx); /* * We are force detaching - remove any active PSA mappings so -- cgit v1.2.2 From 7bb5d91a4dda92e28b2704a3e7ebe94260ccd6f1 Mon Sep 17 00:00:00 2001 From: Michael Neuling Date: Wed, 27 May 2015 16:07:14 +1000 Subject: cxl: Rework context lifetimes This reworks contexts lifetimes a bit to enable the kernel API where we may want to reuse contexts. Here we will want to start and stop contexts without freeing them. Start context does the get pid & ctx so stop context will need to do the puts. Here we move put pid & ctx to the detach context path which will become part of the stop context path. Signed-off-by: Michael Neuling Acked-by: Ian Munsie Signed-off-by: Michael Ellerman --- drivers/misc/cxl/context.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) (limited to 'drivers/misc/cxl/context.c') diff --git a/drivers/misc/cxl/context.c b/drivers/misc/cxl/context.c index 7d857b7d686d..2a4c80ac322a 100644 --- a/drivers/misc/cxl/context.c +++ b/drivers/misc/cxl/context.c @@ -186,6 +186,9 @@ int __detach_context(struct cxl_context *ctx) return -EBUSY; WARN_ON(cxl_detach_process(ctx)); + flush_work(&ctx->fault_work); /* Only needed for dedicated process */ + put_pid(ctx->pid); + cxl_ctx_put(); return 0; } @@ -204,7 +207,6 @@ void cxl_context_detach(struct cxl_context *ctx) return; afu_release_irqs(ctx, ctx); - flush_work(&ctx->fault_work); /* Only needed for dedicated process */ wake_up_all(&ctx->wq); } @@ -245,7 +247,6 @@ static void reclaim_ctx(struct rcu_head *rcu) free_page((u64)ctx->sstp); ctx->sstp = NULL; - put_pid(ctx->pid); kfree(ctx); } -- cgit v1.2.2 From 5caaf5346892d1e7f0b8b7223062644f8538483f Mon Sep 17 00:00:00 2001 From: Ian Munsie Date: Tue, 7 Jul 2015 15:45:46 +1000 Subject: cxl: Fail mmap if requested mapping is larger than assigned problem state area This patch makes the mmap call fail outright if the requested region is larger than the problem state area assigned to the context so the error is reported immediately rather than waiting for an attempt to access an address out of bounds. Although we never expect users to map more than the assigned problem state area and are not aware of anyone doing this (other than for testing), this does have the potential to break users if someone has used a larger range regardless. I'm submitting it for consideration, but if this change is not considered acceptable the previous patch is sufficient to prevent access out of bounds without breaking anyone. Signed-off-by: Ian Munsie Signed-off-by: Michael Ellerman --- drivers/misc/cxl/context.c | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) (limited to 'drivers/misc/cxl/context.c') diff --git a/drivers/misc/cxl/context.c b/drivers/misc/cxl/context.c index 2a4c80ac322a..6236d4a1f7ad 100644 --- a/drivers/misc/cxl/context.c +++ b/drivers/misc/cxl/context.c @@ -145,8 +145,16 @@ static const struct vm_operations_struct cxl_mmap_vmops = { */ int cxl_context_iomap(struct cxl_context *ctx, struct vm_area_struct *vma) { + u64 start = vma->vm_pgoff << PAGE_SHIFT; u64 len = vma->vm_end - vma->vm_start; - len = min(len, ctx->psn_size); + + if (ctx->afu->current_mode == CXL_MODE_DEDICATED) { + if (start + len > ctx->afu->adapter->ps_size) + return -EINVAL; + } else { + if (start + len > ctx->psn_size) + return -EINVAL; + } if (ctx->afu->current_mode != CXL_MODE_DEDICATED) { /* make sure there is a valid per process space for this AFU */ -- cgit v1.2.2 From 10a5894f2dedd8a26b3132497445b314c0d952c4 Mon Sep 17 00:00:00 2001 From: Ian Munsie Date: Tue, 7 Jul 2015 15:45:45 +1000 Subject: cxl: Fix off by one error allowing subsequent mmap page to be accessed It was discovered that if a process mmaped their problem state area they were able to access one page more than expected, potentially allowing them to access the problem state area of an unrelated process. This was due to a simple off by one error in the mmap fault handler introduced in 0712dc7e73e59d79bcead5d5520acf4e9e917e87 ("cxl: Fix issues when unmapping contexts"), which is fixed in this patch. Cc: stable@vger.kernel.org Fixes: 0712dc7e73e5 ("cxl: Fix issues when unmapping contexts") Signed-off-by: Ian Munsie Signed-off-by: Michael Ellerman --- drivers/misc/cxl/context.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) (limited to 'drivers/misc/cxl/context.c') diff --git a/drivers/misc/cxl/context.c b/drivers/misc/cxl/context.c index 6236d4a1f7ad..1287148629c0 100644 --- a/drivers/misc/cxl/context.c +++ b/drivers/misc/cxl/context.c @@ -113,11 +113,11 @@ static int cxl_mmap_fault(struct vm_area_struct *vma, struct vm_fault *vmf) if (ctx->afu->current_mode == CXL_MODE_DEDICATED) { area = ctx->afu->psn_phys; - if (offset > ctx->afu->adapter->ps_size) + if (offset >= ctx->afu->adapter->ps_size) return VM_FAULT_SIGBUS; } else { area = ctx->psn_phys; - if (offset > ctx->psn_size) + if (offset >= ctx->psn_size) return VM_FAULT_SIGBUS; } -- cgit v1.2.2