On Tue, Nov 03, 2020 at 12:03:26PM +0100, Boris Brezillon wrote:
On Tue, 3 Nov 2020 11:25:40 +0100 Daniel Vetter daniel@ffwll.ch wrote:
On Tue, Nov 03, 2020 at 09:13:47AM +0100, Boris Brezillon wrote:
We've fixed many races in panfrost_job_timedout() but some remain. Instead of trying to fix it again, let's simplify the logic and move the reset bits to a separate work scheduled when one of the queue reports a timeout.
v3:
- Replace the atomic_cmpxchg() by an atomic_xchg() (Robin Murphy)
- Add Steven's R-b
v2:
- Use atomic_cmpxchg() to conditionally schedule the reset work (Steven Price)
Fixes: 1a11a88cfd9a ("drm/panfrost: Fix job timeout handling") Cc: stable@vger.kernel.org Signed-off-by: Boris Brezillon boris.brezillon@collabora.com Reviewed-by: Steven Price steven.price@arm.com
Sprinkling the dma_fence annotations over this would be really nice ...
You mean something like that?
That's just the irq annotations, i.e. the one that's already guaranteed by the irq vs. locks checks. So this does nothing.
What I mean is annotating your new reset work (it's part of the critical path to complete batches, since it's holding up other batches that are stuck in the scheduler still), and the drm/scheduler annotations I've floated a while ago. The drm/scheduler annotations are stuck somewhat for lack of feedback from any of the driver teams using it though :-/
The thing is pulling something out into a worker of it's own generally doesn't fix any deadlocks, it just hides them from lockdep. So it would be good to make sure lockdep can see through your maze again. -Daniel
--->8--- From 4f90ee2972eaec0332833ff6f9ea078acbfa899a Mon Sep 17 00:00:00 2001 From: Boris Brezillon boris.brezillon@collabora.com Date: Tue, 3 Nov 2020 12:01:09 +0100 Subject: [PATCH] drm/panfrost: Annotate dma_fence signalling
Annotate dma_fence signalling to help lockdep catch deadlocks.
Signed-off-by: Boris Brezillon boris.brezillon@collabora.com
drivers/gpu/drm/panfrost/panfrost_job.c | 3 +++ 1 file changed, 3 insertions(+)
diff --git a/drivers/gpu/drm/panfrost/panfrost_job.c b/drivers/gpu/drm/panfrost/panfrost_job.c index 569a099dc10e..046cb3677332 100644 --- a/drivers/gpu/drm/panfrost/panfrost_job.c +++ b/drivers/gpu/drm/panfrost/panfrost_job.c @@ -482,7 +482,9 @@ static irqreturn_t panfrost_job_irq_handler(int irq, void *data)
if (status & JOB_INT_MASK_DONE(j)) { struct panfrost_job *job;
bool cookie;cookie = dma_fence_begin_signalling(); spin_lock(&pfdev->js->job_lock); job = pfdev->jobs[j]; /* Only NULL if job timeout occurred */@@ -496,6 +498,7 @@ static irqreturn_t panfrost_job_irq_handler(int irq, void *data) pm_runtime_put_autosuspend(pfdev->dev); } spin_unlock(&pfdev->js->job_lock);
dma_fence_end_signalling(cookie);}
status &= ~mask;