Linux-Media Archive mirror
 help / color / mirror / Atom feed
From: Ricardo Ribalda <ribalda@chromium.org>
To: Andy Walls <awalls@md.metrocast.net>,
	 Mauro Carvalho Chehab <mchehab@kernel.org>,
	Sean Young <sean@mess.org>,  Jarod Wilson <jarod@redhat.com>
Cc: Hans Verkuil <hverkuil-cisco@xs4all.nl>,
	linux-media@vger.kernel.org,  linux-kernel@vger.kernel.org,
	Ricardo Ribalda <ribalda@chromium.org>
Subject: [PATCH 1/5] media: ivtv: Factor out schedule functions
Date: Mon, 06 May 2024 21:10:26 +0000	[thread overview]
Message-ID: <20240506-cocci-locks-v1-1-a67952fe5d19@chromium.org> (raw)
In-Reply-To: <20240506-cocci-locks-v1-0-a67952fe5d19@chromium.org>

Cocci is very confused by unlock-lock a mutex in the middle of a
function.

Factor the schedules out, avoid code duplication and make cocci a bit
happier.

Fix the following cocci warnings:
drivers/media/pci/ivtv/ivtv-fileops.c:223:4-10: preceding lock on line 267
drivers/media/pci/ivtv/ivtv-fileops.c:230:3-9: preceding lock on line 267
drivers/media/pci/ivtv/ivtv-fileops.c:236:4-10: preceding lock on line 267
drivers/media/pci/ivtv/ivtv-fileops.c:245:3-9: preceding lock on line 267
drivers/media/pci/ivtv/ivtv-fileops.c:251:3-9: preceding lock on line 267
drivers/media/pci/ivtv/ivtv-fileops.c:257:3-9: preceding lock on line 267
drivers/media/pci/ivtv/ivtv-fileops.c:272:3-9: preceding lock on line 267
drivers/media/pci/ivtv/ivtv-fileops.c:598:4-10: preceding lock on line 627
drivers/media/pci/ivtv/ivtv-fileops.c:598:4-10: preceding lock on line 689
drivers/media/pci/ivtv/ivtv-fileops.c:606:3-9: preceding lock on line 627
drivers/media/pci/ivtv/ivtv-fileops.c:606:3-9: preceding lock on line 689
drivers/media/pci/ivtv/ivtv-fileops.c:648:3-9: preceding lock on line 627
drivers/media/pci/ivtv/ivtv-fileops.c:648:3-9: preceding lock on line 689
drivers/media/pci/ivtv/ivtv-fileops.c:692:4-10: preceding lock on line 689

Signed-off-by: Ricardo Ribalda <ribalda@chromium.org>
---
 drivers/media/pci/ivtv/ivtv-fileops.c | 66 +++++++++++++++++++++--------------
 1 file changed, 39 insertions(+), 27 deletions(-)

diff --git a/drivers/media/pci/ivtv/ivtv-fileops.c b/drivers/media/pci/ivtv/ivtv-fileops.c
index 4202c3a47d33..cfa28d035586 100644
--- a/drivers/media/pci/ivtv/ivtv-fileops.c
+++ b/drivers/media/pci/ivtv/ivtv-fileops.c
@@ -21,6 +21,7 @@
 #include "ivtv-ioctl.h"
 #include "ivtv-cards.h"
 #include "ivtv-firmware.h"
+#include <linux/lockdep.h>
 #include <media/v4l2-event.h>
 #include <media/i2c/saa7115.h>
 
@@ -190,12 +191,27 @@ static void ivtv_update_pgm_info(struct ivtv *itv)
 	itv->pgm_info_write_idx = (itv->pgm_info_write_idx + i) % itv->pgm_info_num;
 }
 
+static void ivtv_schedule(struct ivtv_stream *s)
+{
+	struct ivtv *itv = s->itv;
+	DEFINE_WAIT(wait);
+
+	lockdep_assert_held(&itv->serialize_lock);
+
+	mutex_unlock(&itv->serialize_lock);
+	prepare_to_wait(&s->waitq, &wait, TASK_INTERRUPTIBLE);
+	/* New buffers might have become free before we were added to the waitqueue */
+	if (!s->q_free.buffers)
+		schedule();
+	finish_wait(&s->waitq, &wait);
+	mutex_lock(&itv->serialize_lock);
+}
+
 static struct ivtv_buffer *ivtv_get_buffer(struct ivtv_stream *s, int non_block, int *err)
 {
 	struct ivtv *itv = s->itv;
 	struct ivtv_stream *s_vbi = &itv->streams[IVTV_ENC_STREAM_TYPE_VBI];
 	struct ivtv_buffer *buf;
-	DEFINE_WAIT(wait);
 
 	*err = 0;
 	while (1) {
@@ -258,13 +274,7 @@ static struct ivtv_buffer *ivtv_get_buffer(struct ivtv_stream *s, int non_block,
 		}
 
 		/* wait for more data to arrive */
-		mutex_unlock(&itv->serialize_lock);
-		prepare_to_wait(&s->waitq, &wait, TASK_INTERRUPTIBLE);
-		/* New buffers might have become available before we were added to the waitqueue */
-		if (!s->q_full.buffers)
-			schedule();
-		finish_wait(&s->waitq, &wait);
-		mutex_lock(&itv->serialize_lock);
+		ivtv_schedule(s);
 		if (signal_pending(current)) {
 			/* return if a signal was received */
 			IVTV_DEBUG_INFO("User stopped %s\n", s->name);
@@ -533,6 +543,25 @@ int ivtv_start_decoding(struct ivtv_open_id *id, int speed)
 	return 0;
 }
 
+static int ivtv_schedule_dma(struct ivtv_stream *s)
+{
+	struct ivtv *itv = s->itv;
+	int got_sig;
+	DEFINE_WAIT(wait);
+
+	lockdep_assert_held(&itv->serialize_lock);
+
+	mutex_unlock(&itv->serialize_lock);
+	prepare_to_wait(&itv->dma_waitq, &wait, TASK_INTERRUPTIBLE);
+	while (!(got_sig = signal_pending(current)) &&
+	       test_bit(IVTV_F_S_DMA_PENDING, &s->s_flags))
+		schedule();
+	finish_wait(&itv->dma_waitq, &wait);
+	mutex_lock(&itv->serialize_lock);
+
+	return got_sig;
+}
+
 static ssize_t ivtv_write(struct file *filp, const char __user *user_buf, size_t count, loff_t *pos)
 {
 	struct ivtv_open_id *id = fh2id(filp->private_data);
@@ -544,7 +573,6 @@ static ssize_t ivtv_write(struct file *filp, const char __user *user_buf, size_t
 	int bytes_written = 0;
 	int mode;
 	int rc;
-	DEFINE_WAIT(wait);
 
 	IVTV_DEBUG_HI_FILE("write %zd bytes to %s\n", count, s->name);
 
@@ -618,13 +646,7 @@ static ssize_t ivtv_write(struct file *filp, const char __user *user_buf, size_t
 			break;
 		if (filp->f_flags & O_NONBLOCK)
 			return -EAGAIN;
-		mutex_unlock(&itv->serialize_lock);
-		prepare_to_wait(&s->waitq, &wait, TASK_INTERRUPTIBLE);
-		/* New buffers might have become free before we were added to the waitqueue */
-		if (!s->q_free.buffers)
-			schedule();
-		finish_wait(&s->waitq, &wait);
-		mutex_lock(&itv->serialize_lock);
+		ivtv_schedule(s);
 		if (signal_pending(current)) {
 			IVTV_DEBUG_INFO("User stopped %s\n", s->name);
 			return -EINTR;
@@ -674,20 +696,10 @@ static ssize_t ivtv_write(struct file *filp, const char __user *user_buf, size_t
 
 	if (test_bit(IVTV_F_S_NEEDS_DATA, &s->s_flags)) {
 		if (s->q_full.length >= itv->dma_data_req_size) {
-			int got_sig;
-
 			if (mode == OUT_YUV)
 				ivtv_yuv_setup_stream_frame(itv);
 
-			mutex_unlock(&itv->serialize_lock);
-			prepare_to_wait(&itv->dma_waitq, &wait, TASK_INTERRUPTIBLE);
-			while (!(got_sig = signal_pending(current)) &&
-					test_bit(IVTV_F_S_DMA_PENDING, &s->s_flags)) {
-				schedule();
-			}
-			finish_wait(&itv->dma_waitq, &wait);
-			mutex_lock(&itv->serialize_lock);
-			if (got_sig) {
+			if (ivtv_schedule_dma(s)) {
 				IVTV_DEBUG_INFO("User interrupted %s\n", s->name);
 				return -EINTR;
 			}

-- 
2.45.0.rc1.225.g2a3ae87e7f-goog


  reply	other threads:[~2024-05-06 21:10 UTC|newest]

Thread overview: 6+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2024-05-06 21:10 [PATCH 0/5] media: Fix some cocci locks warnings Ricardo Ribalda
2024-05-06 21:10 ` Ricardo Ribalda [this message]
2024-05-06 21:10 ` [PATCH 2/5] media: imon: Fix race getting ictx->lock Ricardo Ribalda
2024-05-06 21:10 ` [PATCH 3/5] media: dvb-frontends/stv090x: Refactor tuner_i2c_lock Ricardo Ribalda
2024-05-06 21:10 ` [PATCH 4/5] media: go7007: Refactor Adlink PCI-MPG24 i2c mutex Ricardo Ribalda
2024-05-06 21:10 ` [PATCH 5/5] media: drivers/media/dvb-core: Refactor dvb_frontend_open Ricardo Ribalda

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20240506-cocci-locks-v1-1-a67952fe5d19@chromium.org \
    --to=ribalda@chromium.org \
    --cc=awalls@md.metrocast.net \
    --cc=hverkuil-cisco@xs4all.nl \
    --cc=jarod@redhat.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-media@vger.kernel.org \
    --cc=mchehab@kernel.org \
    --cc=sean@mess.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).