Re: [PATCHv16 28/34] v4l2-mem2mem: Simplify exiting the function in v4l2_m2m_try_schedule

[Ezequiel Garcia]

Hi Hans, Sakari:

On Thu, 2018-07-05 at 18:03 +0200, Hans Verkuil wrote:
> From: Sakari Ailus 
> 
> The v4l2_m2m_try_schedule function acquires and releases multiple
> spinlocks; simplify unlocking the job lock by adding a label to unlock the
> job lock and exit the function.
> 
> 

Why not just going the whole way and do the same for the other spinlocks?

Something along these lines:

diff --git a/drivers/media/v4l2-core/v4l2-mem2mem.c 
b/drivers/media/v4l2-core/v4l2-mem2mem.c
index 114d50cf22c5..565b5946d907 100644
--- a/drivers/media/v4l2-core/v4l2-mem2mem.c
+++ b/drivers/media/v4l2-core/v4l2-mem2mem.c
@@ -253,31 +253,25 @@ void v4l2_m2m_try_schedule(struct v4l2_m2m_ctx *m2m_ctx)
/* If the context is aborted then don't schedule it */
if (m2m_ctx->job_flags & TRANS_ABORT) {
dprintk("Aborted context\n");
-   goto out_unlock;
+   goto job_unlock;
}
 
if (m2m_ctx->job_flags & TRANS_QUEUED) {
dprintk("On job queue already\n");
-   goto out_unlock;
+   goto job_unlock;
}
 
spin_lock_irqsave(&m2m_ctx->out_q_ctx.rdy_spinlock, flags_out);
if (list_empty(&m2m_ctx->out_q_ctx.rdy_queue)
&& !m2m_ctx->out_q_ctx.buffered) {
-   spin_unlock_irqrestore(&m2m_ctx->out_q_ctx.rdy_spinlock,
-   flags_out);
dprintk("No input buffers available\n");
goto out_unlock;
}
spin_lock_irqsave(&m2m_ctx->cap_q_ctx.rdy_spinlock, flags_cap);
if (list_empty(&m2m_ctx->cap_q_ctx.rdy_queue)
&& !m2m_ctx->cap_q_ctx.buffered) {
-   spin_unlock_irqrestore(&m2m_ctx->cap_q_ctx.rdy_spinlock,
-   flags_cap);
-   spin_unlock_irqrestore(&m2m_ctx->out_q_ctx.rdy_spinlock,
-   flags_out);
dprintk("No output buffers available\n");
-   goto out_unlock;
+   goto cap_unlock;
}
spin_unlock_irqrestore(&m2m_ctx->cap_q_ctx.rdy_spinlock, flags_cap);
spin_unlock_irqrestore(&m2m_ctx->out_q_ctx.rdy_spinlock, flags_out);
@@ -297,7 +291,11 @@ void v4l2_m2m_try_schedule(struct v4l2_m2m_ctx *m2m_ctx)
 
return;
 
+cap_unlock:
+   spin_unlock_irqrestore(&m2m_ctx->cap_q_ctx.rdy_spinlock, flags_cap);
 out_unlock:
+   spin_unlock_irqrestore(&m2m_ctx->out_q_ctx.rdy_spinlock, flags_out);
+job_unlock:
spin_unlock_irqrestore(&m2m_dev->job_spinlock, flags_job);
 
return;
-- 
2.18.0.rc2

[PATCHv16 28/34] v4l2-mem2mem: Simplify exiting the function in v4l2_m2m_try_schedule

[Hans Verkuil]

From: Sakari Ailus 

The v4l2_m2m_try_schedule function acquires and releases multiple
spinlocks; simplify unlocking the job lock by adding a label to unlock the
job lock and exit the function.

Signed-off-by: Sakari Ailus 
---
 drivers/media/v4l2-core/v4l2-mem2mem.c | 22 --
 1 file changed, 12 insertions(+), 10 deletions(-)

diff --git a/drivers/media/v4l2-core/v4l2-mem2mem.c 
b/drivers/media/v4l2-core/v4l2-mem2mem.c
index 9bfe1c456046..114d50cf22c5 100644
--- a/drivers/media/v4l2-core/v4l2-mem2mem.c
+++ b/drivers/media/v4l2-core/v4l2-mem2mem.c
@@ -252,15 +252,13 @@ void v4l2_m2m_try_schedule(struct v4l2_m2m_ctx *m2m_ctx)
 
/* If the context is aborted then don't schedule it */
if (m2m_ctx->job_flags & TRANS_ABORT) {
-   spin_unlock_irqrestore(&m2m_dev->job_spinlock, flags_job);
dprintk("Aborted context\n");
-   return;
+   goto out_unlock;
}
 
if (m2m_ctx->job_flags & TRANS_QUEUED) {
-   spin_unlock_irqrestore(&m2m_dev->job_spinlock, flags_job);
dprintk("On job queue already\n");
-   return;
+   goto out_unlock;
}
 
spin_lock_irqsave(&m2m_ctx->out_q_ctx.rdy_spinlock, flags_out);
@@ -268,9 +266,8 @@ void v4l2_m2m_try_schedule(struct v4l2_m2m_ctx *m2m_ctx)
&& !m2m_ctx->out_q_ctx.buffered) {
spin_unlock_irqrestore(&m2m_ctx->out_q_ctx.rdy_spinlock,
flags_out);
-   spin_unlock_irqrestore(&m2m_dev->job_spinlock, flags_job);
dprintk("No input buffers available\n");
-   return;
+   goto out_unlock;
}
spin_lock_irqsave(&m2m_ctx->cap_q_ctx.rdy_spinlock, flags_cap);
if (list_empty(&m2m_ctx->cap_q_ctx.rdy_queue)
@@ -279,18 +276,16 @@ void v4l2_m2m_try_schedule(struct v4l2_m2m_ctx *m2m_ctx)
flags_cap);
spin_unlock_irqrestore(&m2m_ctx->out_q_ctx.rdy_spinlock,
flags_out);
-   spin_unlock_irqrestore(&m2m_dev->job_spinlock, flags_job);
dprintk("No output buffers available\n");
-   return;
+   goto out_unlock;
}
spin_unlock_irqrestore(&m2m_ctx->cap_q_ctx.rdy_spinlock, flags_cap);
spin_unlock_irqrestore(&m2m_ctx->out_q_ctx.rdy_spinlock, flags_out);
 
if (m2m_dev->m2m_ops->job_ready
&& (!m2m_dev->m2m_ops->job_ready(m2m_ctx->priv))) {
-   spin_unlock_irqrestore(&m2m_dev->job_spinlock, flags_job);
dprintk("Driver not ready\n");
-   return;
+   goto out_unlock;
}
 
list_add_tail(&m2m_ctx->queue, &m2m_dev->job_queue);
@@ -299,6 +294,13 @@ void v4l2_m2m_try_schedule(struct v4l2_m2m_ctx *m2m_ctx)
spin_unlock_irqrestore(&m2m_dev->job_spinlock, flags_job);
 
v4l2_m2m_try_run(m2m_dev);
+
+   return;
+
+out_unlock:
+   spin_unlock_irqrestore(&m2m_dev->job_spinlock, flags_job);
+
+   return;
 }
 EXPORT_SYMBOL_GPL(v4l2_m2m_try_schedule);
 
-- 
2.18.0