mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
From: Stefan Richter <stefanr@s5r6.in-berlin.de>
To: linux1394-devel@lists.sourceforge.net
Cc: linux-kernel@vger.kernel.org, Arnd Bergmann <arnd@arndb.de>,
	Dan Dennedy <dan@dennedy.org>
Subject: [PATCH] ieee1394: char device files are not seekable (BKL removal)
Date: Sat, 27 Mar 2010 10:20:18 +0100 (CET)	[thread overview]
Message-ID: <tkrat.3d0d526ea7cecc39@s5r6.in-berlin.de> (raw)
In-Reply-To: <4BAD4795.2040700@s5r6.in-berlin.de>

The raw1394 character device file ABI is based on
  - write() or ioctl() to initiate actions,
    with write being used exactly like _IOW ioctls,
  - read() to consume events,
  - mmap() for isochronous I/O DMA buffers.

The video1394 character device file ABI is based on
  - ioctl() to initiate actions,
  - mmap() for isochronous I/O DMA buffers.

The dv1394 character device file ABI is based on
  - ioctl() to initiate actions,
  - read() and write() for simple serial reception and transmission of
    DV streams with a copy_to/from_user,
  - mmap() for isochronous I/O DMA buffers for I/O without user copy.

lseek(), pread(), pwrite() on the other hand are not applicable to
/dev/raw1394, /dev/video1394/*, and /dev/dv1394/* device files.

Alas, file_operations.llseek == NULL causes lseek() to call
fs/read_write.c::default_llseek per default.  This looks like not doing
any harm in either of the three drivers, but it grabs the Big Kernel
Lock.  We don't want that, and we should return an error on lseek() and
friends.  This is provided by fs/read_write.c::no_llseek which we get if
we clear the FMODE_LSEEK (and FMODE_PREAD, FMODE_PWRITE) flag by means
of nonseekable_open().

Side note:  Apart from this oversight regarding default_llseek, the
raw1394, video1394, and dv1394 interfaces have been converted away from
BKL usage some time ago.  Although all this is legacy code which should
be left in peace until it is eventually removed (as it is superseded by
firewire-core's <linux/firewire-cdev.h> ABI), this change seems still
worth doing to further minimize the presence of BKL usage in the kernel.

Signed-off-by: Stefan Richter <stefanr@s5r6.in-berlin.de>
---

Somebody correct me if I misunderstood anything of these ABIs.

This patch is motivated by Arnd's
"bkl removal: make unlocked_ioctl mandatory"
http://git.kernel.org/?p=linux/kernel/git/arnd/playground.git;a=blobdiff;f=drivers/ieee1394/raw1394.c;h=7c312331fc14facd8d3e2f8cf05daf090fd88e71;hp=8aa56ac07e2920d371a5af41eca7f09c8b752380;hb=05e7753338045e9ee3950b2da032c5e5774efa90;hpb=03165e1d096afb4b1d9cfccdad66eed038121cec
etc., and
"BKL removal: mark remaining users as 'depends on BKL'"
http://git.kernel.org/?p=linux/kernel/git/arnd/playground.git;a=blobdiff;f=drivers/ieee1394/Kconfig;h=0d4954c2615233ccdd8ed28ba2e7b36ed1c7941d;hp=e02096cf7d95bfa383bddb703bbfbff6b782e688;hb=abb83d8fe5f8dcc8fca09bd9117429f73e1417e0;hpb=33c014b118f45516113d4b6823e40ea6f834dc6a


 drivers/ieee1394/dv1394.c    |    2 +-
 drivers/ieee1394/raw1394.c   |    2 +-
 drivers/ieee1394/video1394.c |    2 +-
 3 files changed, 3 insertions(+), 3 deletions(-)

Index: b/drivers/ieee1394/dv1394.c
===================================================================
--- a/drivers/ieee1394/dv1394.c
+++ b/drivers/ieee1394/dv1394.c
@@ -1824,7 +1824,7 @@ static int dv1394_open(struct inode *ino
 	       "and will not be available in the new firewire driver stack. "
 	       "Try libraw1394 based programs instead.\n", current->comm);
 
-	return 0;
+	return nonseekable_open(inode, file);
 }
 
 
Index: b/drivers/ieee1394/raw1394.c
===================================================================
--- a/drivers/ieee1394/raw1394.c
+++ b/drivers/ieee1394/raw1394.c
@@ -2834,7 +2834,7 @@ static int raw1394_open(struct inode *in
 
 	file->private_data = fi;
 
-	return 0;
+	return nonseekable_open(inode, file);
 }
 
 static int raw1394_release(struct inode *inode, struct file *file)
Index: b/drivers/ieee1394/video1394.c
===================================================================
--- a/drivers/ieee1394/video1394.c
+++ b/drivers/ieee1394/video1394.c
@@ -1239,7 +1239,7 @@ static int video1394_open(struct inode *
 	ctx->current_ctx = NULL;
 	file->private_data = ctx;
 
-	return 0;
+	return nonseekable_open(inode, file);
 }
 
 static int video1394_release(struct inode *inode, struct file *file)

-- 
Stefan Richter
-=====-==-=- --== ==-==
http://arcgraph.de/sr/


  parent reply	other threads:[~2010-03-27  9:20 UTC|newest]

Thread overview: 49+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2010-03-24 21:40 [GIT, RFC] Killing the Big Kernel Lock Arnd Bergmann
2010-03-24 21:07 ` Andrew Morton
2010-03-25 10:26   ` Arnd Bergmann
2010-03-28 20:33     ` Frederic Weisbecker
2010-03-24 21:53 ` Roland Dreier
2010-03-24 21:59   ` Arnd Bergmann
2010-03-31  5:22     ` Roland Dreier
2010-03-24 22:10 ` Alan Cox
2010-03-24 22:25   ` Arnd Bergmann
2010-03-24 22:23 ` Ingo Molnar
2010-03-25 12:55 ` Jiri Kosina
2010-03-25 13:06   ` Arnd Bergmann
2010-03-25 13:38     ` Arnd Bergmann
2010-03-26 23:47       ` Stefan Richter
2010-03-27  9:16         ` [PATCH] firewire: char device files are not seekable (BKL removal) Stefan Richter
2010-03-27  9:20         ` Stefan Richter [this message]
2010-03-27 10:40         ` [PATCH RFC] DVB: add dvb_generic_nonseekable_open, dvb_generic_unlocked_ioctl, use in firedtv Stefan Richter
2010-03-28 14:47           ` [PATCH RFC v2] " Stefan Richter
2010-03-27 14:37         ` [GIT, RFC] Killing the Big Kernel Lock Arnd Bergmann
2010-03-28 12:27           ` Stefan Richter
2010-03-28 20:05             ` Arnd Bergmann
2010-03-28 20:15               ` Frederic Weisbecker
2010-03-28 21:34                 ` Arnd Bergmann
2010-03-28 23:24                   ` Frederic Weisbecker
2010-04-08 20:45               ` Jan Blunck
2010-04-08 21:27                 ` Arnd Bergmann
2010-04-08 21:30                   ` Frederic Weisbecker
2010-04-09 11:02                   ` Jan Blunck
2010-04-10 15:13           ` Stefan Richter
2010-03-28 21:58   ` Andi Kleen
2010-03-29  1:07     ` [GIT, RFC] Killing the Big Kernel Lock II Andi Kleen
2010-03-29 11:48       ` Arnd Bergmann
2010-03-29 12:30         ` Andi Kleen
2010-03-29 14:43           ` Arnd Bergmann
2010-03-29 20:11             ` Andi Kleen
2010-03-31 15:30               ` Arnd Bergmann
2010-03-25 13:40 ` [GIT, RFC] Killing the Big Kernel Lock Dan Carpenter
2010-03-25 14:14   ` Arnd Bergmann
2010-03-28 20:04 ` Frederic Weisbecker
2010-03-28 20:11 ` Frederic Weisbecker
2010-03-28 23:18 ` Frederic Weisbecker
2010-03-28 23:38   ` Frederic Weisbecker
2010-03-29 11:04     ` Arnd Bergmann
2010-03-29 17:59       ` Frederic Weisbecker
2010-03-29 21:18         ` Arnd Bergmann
2010-03-29 12:45 ` John Kacur
2010-03-31 22:11 ` Roland Dreier
2010-03-31 22:20   ` Frederic Weisbecker
2010-04-01  8:50   ` Arnd Bergmann

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=tkrat.3d0d526ea7cecc39@s5r6.in-berlin.de \
    --to=stefanr@s5r6.in-berlin.de \
    --cc=arnd@arndb.de \
    --cc=dan@dennedy.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux1394-devel@lists.sourceforge.net \
    /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

Powered by JetHome