From: Jonathan Nieder <jrnieder@gmail.com>
To: linux-media@vger.kernel.org
Cc: Huber Andreas <hobrom@corax.at>,
Mauro Carvalho Chehab <mchehab@redhat.com>,
Hans Verkuil <hverkuil@xs4all.nl>,
linux-kernel@vger.kernel.org, Ben Hutchings <ben@decadent.org.uk>,
Steven Toth <stoth@kernellabs.com>
Subject: [PATCH 2/7] [media] cx88: fix locking of sub-driver operations
Date: Mon, 4 Apr 2011 22:25:58 -0500 [thread overview]
Message-ID: <20110405032558.GC4498@elie> (raw)
In-Reply-To: <20110405032014.GA4498@elie>
The BKL conversion of this driver seems to have gone wrong.
Loading the cx88-blackbird driver deadlocks.
The cause: mpeg_ops::open in the cx2388x blackbird driver acquires the
device lock and calls the sub-driver's request_acquire, which tries to
acquire the lock again. Fix it by clarifying the semantics of
request_acquire, request_release, advise_acquire, and advise_release:
all will require the caller to hold the device lock.
Based on a patch by Ben Hutchings <ben@decadent.org.uk>.
Reported-by: Andi Huber <hobrom@gmx.at>
Fixes: https://bugzilla.kernel.org/show_bug.cgi?id=31962
Signed-off-by: Jonathan Nieder <jrnieder@gmail.com>
Cc: stable@kernel.org
---
drivers/media/video/cx88/cx88-blackbird.c | 4 ++--
drivers/media/video/cx88/cx88-dvb.c | 3 +--
drivers/media/video/cx88/cx88-mpeg.c | 4 ----
drivers/media/video/cx88/cx88.h | 3 ++-
4 files changed, 5 insertions(+), 9 deletions(-)
diff --git a/drivers/media/video/cx88/cx88-blackbird.c b/drivers/media/video/cx88/cx88-blackbird.c
index b93fbd3..a6f7d53 100644
--- a/drivers/media/video/cx88/cx88-blackbird.c
+++ b/drivers/media/video/cx88/cx88-blackbird.c
@@ -1125,13 +1125,13 @@ static int mpeg_release(struct file *file)
/* Make sure we release the hardware */
drv = cx8802_get_driver(dev, CX88_MPEG_BLACKBIRD);
- mutex_unlock(&dev->core->lock);
-
if (drv)
drv->request_release(drv);
atomic_dec(&dev->core->mpeg_users);
+ mutex_unlock(&dev->core->lock);
+
return 0;
}
diff --git a/drivers/media/video/cx88/cx88-dvb.c b/drivers/media/video/cx88/cx88-dvb.c
index 84002bc..c69df7e 100644
--- a/drivers/media/video/cx88/cx88-dvb.c
+++ b/drivers/media/video/cx88/cx88-dvb.c
@@ -135,8 +135,6 @@ static int cx88_dvb_bus_ctrl(struct dvb_frontend* fe, int acquire)
mutex_lock(&dev->core->lock);
drv = cx8802_get_driver(dev, CX88_MPEG_DVB);
- mutex_unlock(&dev->core->lock);
-
if (drv) {
if (acquire){
dev->frontends.active_fe_id = fe_id;
@@ -146,6 +144,7 @@ static int cx88_dvb_bus_ctrl(struct dvb_frontend* fe, int acquire)
dev->frontends.active_fe_id = 0;
}
}
+ mutex_unlock(&dev->core->lock);
return ret;
}
diff --git a/drivers/media/video/cx88/cx88-mpeg.c b/drivers/media/video/cx88/cx88-mpeg.c
index 918172b..9147c16 100644
--- a/drivers/media/video/cx88/cx88-mpeg.c
+++ b/drivers/media/video/cx88/cx88-mpeg.c
@@ -624,13 +624,11 @@ static int cx8802_request_acquire(struct cx8802_driver *drv)
if (drv->advise_acquire)
{
- mutex_lock(&drv->core->lock);
core->active_ref++;
if (core->active_type_id == CX88_BOARD_NONE) {
core->active_type_id = drv->type_id;
drv->advise_acquire(drv);
}
- mutex_unlock(&drv->core->lock);
mpeg_dbg(1,"%s() Post acquire GPIO=%x\n", __func__, cx_read(MO_GP0_IO));
}
@@ -643,14 +641,12 @@ static int cx8802_request_release(struct cx8802_driver *drv)
{
struct cx88_core *core = drv->core;
- mutex_lock(&drv->core->lock);
if (drv->advise_release && --core->active_ref == 0)
{
drv->advise_release(drv);
core->active_type_id = CX88_BOARD_NONE;
mpeg_dbg(1,"%s() Post release GPIO=%x\n", __func__, cx_read(MO_GP0_IO));
}
- mutex_unlock(&drv->core->lock);
return 0;
}
diff --git a/drivers/media/video/cx88/cx88.h b/drivers/media/video/cx88/cx88.h
index e3d56c2..9731daa 100644
--- a/drivers/media/video/cx88/cx88.h
+++ b/drivers/media/video/cx88/cx88.h
@@ -510,7 +510,8 @@ struct cx8802_driver {
/* Caller must _not_ hold core->lock */
int (*probe)(struct cx8802_driver *drv);
- /* Caller must hold core->lock */
+ /* Callers to the following functions must hold core->lock */
+
int (*remove)(struct cx8802_driver *drv);
/* MPEG 8802 -> mini driver - Access for hardware control */
--
1.7.5.rc0
next prev parent reply other threads:[~2011-04-05 3:26 UTC|newest]
Thread overview: 23+ messages / expand[flat|nested] mbox.gz Atom feed top
[not found] <20110327150610.4029.95961.reportbug@xen.corax.at>
2011-03-27 15:28 ` [linux-dvb] cx88-blackbird broken (since 2.6.37) Jonathan Nieder
2011-04-02 9:38 ` [RFC/PATCH 0/3] locking fixes for cx88 Jonathan Nieder
2011-04-02 9:41 ` [PATCH 1/3] [media] cx88: protect per-device driver list with device lock Jonathan Nieder
2011-04-02 9:41 ` [PATCH 2/3] [media] cx88: fix locking of sub-driver operations Jonathan Nieder
2011-04-02 9:44 ` [PATCH 3/3] [media] cx88: use a mutex to protect cx8802_devlist Jonathan Nieder
[not found] ` <4D971B8D.4040305@corax.at>
2011-04-02 19:29 ` Jonathan Nieder
2011-04-02 20:13 ` Andreas Huber
2011-04-04 2:12 ` Andreas Huber
2011-04-05 1:16 ` Jonathan Nieder
2011-04-02 14:05 ` [RFC/PATCH 0/3] locking fixes for cx88 Andreas Huber
2011-04-02 15:19 ` Ben Hutchings
2011-04-02 18:30 ` Jonathan Nieder
2011-04-05 3:20 ` [RFC/PATCH v2 0/7] " Jonathan Nieder
2011-04-05 3:22 ` [PATCH 1/7] [media] cx88: protect per-device driver list with device lock Jonathan Nieder
2011-04-05 3:25 ` Jonathan Nieder [this message]
2011-04-05 3:27 ` [PATCH 3/7] [media] cx88: hold device lock during sub-driver initialization Jonathan Nieder
2011-04-05 3:27 ` [PATCH 4/7] [media] cx88: use a mutex to protect cx8802_devlist Jonathan Nieder
2011-04-05 3:29 ` [PATCH 5/7] [media] cx88: gracefully reject attempts to use unregistered cx88-blackbird driver Jonathan Nieder
2011-04-05 3:30 ` [PATCH 6/7] [media] cx88: don't use atomic_t for core->mpeg_users Jonathan Nieder
2011-04-05 3:31 ` [PATCH 7/7] [mpeg] cx88: don't use atomic_t for core->users Jonathan Nieder
2011-04-05 3:41 ` Jonathan Nieder
2011-04-05 18:17 ` [RFC/PATCH v2 0/7] locking fixes for cx88 Jonathan Nieder
2011-05-01 9:17 [PATCH v2.6.38 resend 0/7] cx88 deadlock and data races Jonathan Nieder
2011-05-01 9:29 ` [PATCH 2/7] [media] cx88: fix locking of sub-driver operations Jonathan Nieder
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=20110405032558.GC4498@elie \
--to=jrnieder@gmail.com \
--cc=ben@decadent.org.uk \
--cc=hobrom@corax.at \
--cc=hverkuil@xs4all.nl \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-media@vger.kernel.org \
--cc=mchehab@redhat.com \
--cc=stoth@kernellabs.com \
/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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.