Prev: [RFC][PATCH 1/3] Bcache: Version 6
Next: [PATCH] gpiolib: Implement set_debounce for WM831x GPIOs
From: Arnd Bergmann on 7 Jul 2010 10:10
On Wednesday 07 July 2010, Christoph Hellwig wrote:
> I might be missing something, but this doesn't actually push the
> BKL anyway. It adds duplicate BKL locking inside the ->open and
> ->release methods without actually removing it in the callers.
>
> That's a pretty pointless thing to do. If you're not feeling
> confident enough about touching block_dev.c I can look into that
> part of the pushdown, but this patch on it's own is not actually
> useful.

My idea was that the patch in its current form would be less
controversial if it can't break anything, but you're certainly
right that the description doesn't match what the patch does.

I'm now integrating the half of patch 5, as you suggested.
I would much appreciate if you could verify my assumptions
about the blkdev_get/blkdev_put functions in the combined patch.

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: Jens Axboe on 8 Jul 2010 04:50
On 2010-07-07 16:51, Arnd Bergmann wrote:
> The open and release block_device_operations are currently
> called with the BKL held. In order to change that, we must
> first make sure that all drivers that currently rely
> on this have no regressions.
>
> This blindly pushes the BKL into all .open and .release
> operations for all block drivers to prepare for the
> next step. The drivers can subsequently replace the BKL
> with their own locks or remove it completely when it can
> be shown that it is not needed.
>
> The functions blkdev_get and blkdev_put are the only
> remaining users of the big kernel lock in the block
> layer, besides a few uses in the ioctl code, none
> of which need to serialize with blkdev_{get,put}.
>
> Most of these two functions is also under the protection
> of bdev->bd_mutex, including the actual calls to
> ->open and ->release, and the common code does not
> access any global data structures that need the BKL.

This is missing an smp_lock.h include in i2o as well.
You seem to only add these sporadically, I think that
is a bit unsafe since you are relying on unknown
include hierarchies. That tends to break on one arch
or config even if it works in another.

--
Jens Axboe

--
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 8 Jul 2010 08:00
On Thursday 08 July 2010, Jens Axboe wrote:
> This is missing an smp_lock.h include in i2o as well.
> You seem to only add these sporadically, I think that
> is a bit unsafe since you are relying on unknown
> include hierarchies. That tends to break on one arch
> or config even if it works in another.

The i2o driver is the only one missing smp_lock.h, same problem
as the one you mentioned for patch 2/7. In v3 of the series it
was still correct, I broke it when I rushed out v4.

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/
First  |  Prev  | 
Pages: 1 2
Prev: [RFC][PATCH 1/3] Bcache: Version 6
Next: [PATCH] gpiolib: Implement set_debounce for WM831x GPIOs