From: Dan Carpenter on
There is a typo in this one:

commit 12c8fcce56c0de4fdcacf048fe723c8778af940d
Author: Arnd Bergmann <arnd(a)relay.de.ibm.com>
Date: Wed Mar 24 20:08:55 2010 +0100

block: replace BKL with global mutex

It doesn't seem to interact with much else, so give
this a try.

Signed-off-by: Arnd Bergmann <arnd(a)arndb.de>

diff --git a/kernel/trace/blktrace.c b/kernel/trace/blktrace.c
index d9d6206..9c1277a 100644
--- a/kernel/trace/blktrace.c
+++ b/kernel/trace/blktrace.c

[ snip ]

@@ -1641,7 +1643,7 @@ static ssize_t sysfs_blk_trace_attr_store(struct device *dev,

ret = -ENXIO;

- lock_kernel();
+ mutex_unlock(&blkdev_mutex);
p = dev_to_part(dev);
bdev = bdget(part_devt(p));
if (bdev == NULL)


--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo(a)vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
From: Arnd Bergmann on
On Thursday 25 March 2010, Dan Carpenter wrote:
> There is a typo in this one:

> @@ -1641,7 +1643,7 @@ static ssize_t sysfs_blk_trace_attr_store(struct device *dev,
>
> ret = -ENXIO;
>
> - lock_kernel();
> + mutex_unlock(&blkdev_mutex);
> p = dev_to_part(dev);
> bdev = bdget(part_devt(p));
> if (bdev == NULL)
>

Fixed now, thanks for reporting!

Arnd
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo(a)vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
From: Stefan Richter on
Arnd Bergmann wrote:
> On Thursday 25 March 2010, Arnd Bergmann wrote:
>> On Thursday 25 March 2010, Jiri Kosina wrote:
>>> config USB
>>> tristate "Support for Host-side USB"
>>> depends on USB_ARCH_HAS_HCD && BKL
>>>
>>> Well, that's very interesting definition of "obscure" :)
>>>
>> That's why I said /mostly/ obscure modules. There are soundcore, usb-core, drm,
>> vfat and a few other very common ones, along with many obscure ones.
>
> FWIW, this is the full list of 148 modules that require the BKL in an x86 allmodconfig,
> which is probably the configuration with the largest code coverage.
....
> drivers/media/dvb/firewire/firedtv.ko
....
> drivers/ieee1394/video1394.ko
> drivers/ieee1394/raw1394.ko
> drivers/ieee1394/dv1394.ko
....
> drivers/firewire/firewire-core.ko

firewire-core and raw1394 do not actually require the BKL, they only
miss to declare their files as not seekable. I will post patches which
change these accordingly.

My guess is that there is nothing to seek in dv1394, video1394, and
firedtv either.

Needless to say, there may be other character device file interfaces
which cannot be seeked but don't admit it yet.
--
Stefan Richter
-=====-==-=- --== ==-==
http://arcgraph.de/sr/
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo(a)vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
From: Arnd Bergmann on
On Saturday 27 March 2010 00:47:33 Stefan Richter wrote:

> firewire-core and raw1394 do not actually require the BKL, they only
> miss to declare their files as not seekable. I will post patches which
> change these accordingly.
>
> My guess is that there is nothing to seek in dv1394, video1394, and
> firedtv either.
>
> Needless to say, there may be other character device file interfaces
> which cannot be seeked but don't admit it yet.
>

Your patches look good, but it would be helpful to also set .llseek = no_llseek
in the file operations, because that is much easier to grep for than
only the nonseekable_open. While it's technically a NOP on the presence of
nonseekable_open, it will help that I don't accidentally apply my patch on
top of yours.

Arnd
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo(a)vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
From: Stefan Richter on
Arnd Bergmann wrote:
> Your patches look good, but it would be helpful to also set .llseek = no_llseek
> in the file operations, because that is much easier to grep for than
> only the nonseekable_open. While it's technically a NOP on the presence of
> nonseekable_open, it will help that I don't accidentally apply my patch on
> top of yours.

Sounds like a plan, but (a) if my .llseek = no_llseek and your .llseek =
default_llseek are not within diff context range, you (or whoever else
merges mine and yours) only get a compiler warning (Initializer entry
defined twice) rather than a merge conflict which couldn't be missed,
(b) there won't be a merge conflict in "BKL removal: mark remaining
users as 'depends on BKL'". (c) While I don't mind adding more visual
clutter to ieee1394/*, I prefer terse coding in firewire/*.

How about I put my nonseekable_open additions into a release branch and
send you a pull request after a few days exposure in linux-next? If you
do not plan to respin your patch queue soon or at all, I could even let
you pull a for-arnd branch with a semantically correct merge of yours
and mine.

General thoughts:

".llseek = NULL," so far meant "do the Right Thing on lseek() and
friends, as far as the fs core can tell". Shouldn't we keep it that
way? It's as close to other ".method = NULL," as it can get, which
either mean "silently skip this method if it doesn't matter" (e.g.
..flush) or "fail attempts to use this method with a fitting errno" (e.g.
..write).

Of course, as we have already seen with infiniband, firewire, ieee1394,
..llseek = NULL is ambiguous in practice. Does the driver really want to
use default_llseek, or should it rather use no_llseek and/or
nonseekable_open, or should it even implement a dummy_llseek() { return
0; } which avoids the BKL but preserves ABI behaviour? This needs to be
resolved for each and every case eventually, regardless of whether or
when your addition of .llseek = default_llseek enters mainline.
--
Stefan Richter
-=====-==-=- --== ===--
http://arcgraph.de/sr/
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo(a)vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/