All of lore.kernel.org
 help / color / mirror / Atom feed
From: Alexey Klimov <klimov.linux@gmail.com>
To: linux-media@vger.kernel.org
Cc: Alessio Igor Bogani <abogani@texware.it>,
	Douglas Schilling Landgraf <dougsland@gmail.com>,
	Mauro Carvalho Chehab <mchehab@infradead.org>
Subject: [RFC] BKL in open functions in drivers
Date: Thu, 02 Apr 2009 01:00:56 +0400	[thread overview]
Message-ID: <1238619656.3986.88.camel@tux.localhost> (raw)

Hello,

Few days ago Alessio Igor Bogani<abogani@texware.it> sent me patch
that removes BKLs like lock/unlock_kernel() in open call and place mutex
there in media/radio/radio-mr800.c.
This patch broke the driver, so we figured out new approah. We added one
more mutex lock that was used in open call. The patch is below: 

diff -r ffa5df73ebeb linux/drivers/media/radio/radio-mr800.c
--- a/linux/drivers/media/radio/radio-mr800.c Fri Mar 13 00:43:34 2009
+0000
+++ b/linux/drivers/media/radio/radio-mr800.c	Thu Apr 02 00:40:56 2009
+0400
@@ -163,6 +163,7 @@
 
 	unsigned char *buffer;
 	struct mutex lock;	/* buffer locking */
+	struct mutex open;
 	int curfreq;
 	int stereo;
 	int users;
@@ -570,7 +571,7 @@
 	struct amradio_device *radio = video_get_drvdata(video_devdata(file));
 	int retval;
 
-	lock_kernel();
+	mutex_lock(&radio->open);
 
 	radio->users = 1;
 	radio->muted = 1;
@@ -580,7 +581,7 @@
 		amradio_dev_warn(&radio->videodev->dev,
 			"radio did not start up properly\n");
 		radio->users = 0;
-		unlock_kernel();
+		mutex_unlock(&radio->open);
 		return -EIO;
 	}
 
@@ -594,7 +595,7 @@
 		amradio_dev_warn(&radio->videodev->dev,
 			"set frequency failed\n");
 
-	unlock_kernel();
+	mutex_unlock(&radio->open);
 	return 0;
 }
 
@@ -735,6 +736,7 @@
 	radio->stereo = -1;
 
 	mutex_init(&radio->lock);
+	mutex_init(&radio->open);
 
 	video_set_drvdata(radio->videodev, radio);
 	retval = video_register_device(radio->videodev, VFL_TYPE_RADIO,
radio_nr);

I tested such approach using stress tool that tries to open /dev/radio0
few hundred times. Looks fine. 

So, questions are:

1) What for is lock/unlock_kernel() used in open?
2) Can it be replaced by mutex, for example?

Please, comments, exaplanations are more than welcome.

Thanks,
-- 
best regards, Klimov Alexey


             reply	other threads:[~2009-04-01 21:01 UTC|newest]

Thread overview: 4+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2009-04-01 21:00 Alexey Klimov [this message]
2009-04-02  7:29 ` [RFC] BKL in open functions in drivers Hans Verkuil
2009-04-02 18:25   ` Alexey Klimov
2009-04-02 18:39     ` Hans Verkuil

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=1238619656.3986.88.camel@tux.localhost \
    --to=klimov.linux@gmail.com \
    --cc=abogani@texware.it \
    --cc=dougsland@gmail.com \
    --cc=linux-media@vger.kernel.org \
    --cc=mchehab@infradead.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 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.