block: factor out bd_may_claim()
PATCH 1/2 block: factor out bd_may_claim()
Factor out bd_may_claim() from bd_claim(), add comments and apply a couple of cosmetic edits. This is to prepare for further updates to claim path.
---
Jens, these two patches fix a subtle problem in block device open path where O_EXCL open attempts can interfere with an already exclusively open device.
These definitely are not for this merge window and need to get good amount testing in linux-next.
Thanks.
fs/block_dev.c | 65 +++++++++++++++++++++++++++++++++++++++++---------------- 1 file changed, 47 insertions(+), 18 deletions(-)
fs/block_dev.c +47 -18
@@ -660,41 +660,70 @@ void bd_forget(struct inode *inode)iput (bdev->bd_inode); } -int bd_claim (struct block_device *bdev,void *holder) +/** +* bd_may_claim - test whether a block device can be claimed +* @bdev: block device of interest +* @whole: whole block device containing @bdev, may equal @bdev +* @holder: holder trying to claim @bdev +* +* Test whther @bdev can be claimed by @holder. +* +* CONTEXT: +* spin_lock(&bdev_lock). +* +* RETURNS: +* %true if @bdev can be claimed, %false otherwise. +*/ +static bool bd_may_claim (struct block_device *bdev,struct block_device *whole, +void *holder) { -int res; -spin_lock (&bdev_lock); - -/* first decide result */ if (bdev->bd_holder == holder) - res =0 ;/* already a holder */ +return true;/* already a holder */ else if (bdev->bd_holder !=NULL ) - res = -EBUSY ;/* held by someone else */ +return false;/* held by someone else */ else if (bdev->bd_contains == bdev) - res =0 ;/* is a whole device which isn't held */ +return true;/* is a whole device which isn't held */ -else if (bdev->bd_contains->bd_holder == bd_claim) - res =0 ;/* is a partition of a device that is being partitioned */ -else if (bdev->bd_contains->bd_holder !=NULL ) - res = -EBUSY ;/* is a partition of a held device */ +else if (whole->bd_holder == bd_claim) +return true;/* is a partition of a device that is being partitioned */ +else if (whole->bd_holder !=NULL ) +return false;/* is a partition of a held device */ else - res =0 ;/* is a partition of an un-held device */ +return true;/* is a partition of an un-held device */ +} -/* now impose change */ -if (res==0 ) { +/** +* bd_claim - claim a block device +* @bdev: block device to claim +* @holder: holder trying to claim @bdev +* +* Try to claim @bdev. +* +* RETURNS: +* 0 if successful, -EBUSY if @bdev is already claimed. +*/ +int bd_claim (struct block_device *bdev,void *holder) +{ +struct block_device *whole = bdev->bd_contains; +int res = -EBUSY ; + +spin_lock (&bdev_lock); + +if (bd_may_claim (bdev, whole, holder)) {/* note that for a whole device bd_holders * will be incremented twice, and bd_holder will * be set to bd_claim before being set to holder */ - bdev->bd_contains->bd_holders ++; - bdev->bd_contains->bd_holder = bd_claim; + whole->bd_holders++; + whole->bd_holder = bd_claim; bdev->bd_holders++; bdev->bd_holder = holder; + res =0 ; } +spin_unlock (&bdev_lock);return res; } -EXPORT_SYMBOL (bd_claim);void bd_release (struct block_device *bdev)
PATCH 2/2 block: implement bd_claiming and claiming block
Currently, device claiming for exclusive open is done after low level open - disk->fops->open() - has completed successfully. This means that exclusive open attempts while a device is already exclusively open will fail only after disk->fops->open() is called.
cdrom driver issues commands during open() which means that O_EXCL open attempt can unintentionally inject commands to in-progress command stream for burning thus disturbing burning process. In most cases, this doesn't cause problems because the first command to be issued is TUR which most devices can process in the middle of burning. However, depending on how a device replies to TUR during burning, cdrom driver may end up issuing further commands.
This can't be resolved trivially by moving bd_claim() before doing actual open() because that means an open attempt which will end up failing could interfere other legit O_EXCL open attempts. ie. unconfirmed open attempts can fail others.
This patch resolves the problem by introducing claiming block which is started by bd_start_claiming() and terminated either by bd_claim() or bd_abort_claiming(). bd_claim() from inside a claiming block is guaranteed to succeed and once a claiming block is started, other bd_start_claiming() or bd_claim() attempts block till the current claiming block is terminated.
bd_claim() can still be used standalone although now it always synchronizes against claiming blocks, so the existing users will keep working without any change.
blkdev_open() and open_bdev_exclusive() are converted to use claiming blocks so that exclusive open attempts from these functions don't interfere with the existing exclusive open.
This problem was discovered while investigating bko#15403.
https://bugzilla.kernel.org/show_bug.cgi?id=15403
The burning problem itself can be resolved by updating userspace probing tools to always open w/ O_EXCL.
--- fs/block_dev.c | 198 ++++++++++++++++++++++++++++++++++++++++++++++------- include/linux/fs.h | 1 2 files changed, 175 insertions(+), 24 deletions(-)
include/linux/fs.h +1 -0
@@ -651,6 +651,7 @@ struct block_device {int bd_openers;struct mutex bd_mutex;/* open/close mutex */ struct list_head bd_inodes; +void * bd_claiming;void * bd_holder;int bd_holders;#ifdef CONFIG_SYSFS
fs/block_dev.c +174 -24
261 lines not on this page · raw message