| Message ID | 20230606073950.225178-2-hch@lst.de (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | [01/31] block: also call ->open for incremental partition opens | expand |
On Tue, Jun 06, 2023 at 09:39:20AM +0200, Christoph Hellwig wrote: > For whole devices ->open is called for each open, but for partitions it > is only called on the first open of a partition. This is problematic > as various block drivers look at open flags and might not do all setup > for ioctl only or NDELAY opens. > > Signed-off-by: Christoph Hellwig <hch@lst.de> > --- This assumes that all drivers deal with additional ->open() calls for each partition correctly which I assumed you checked so, Acked-by: Christian Brauner <brauner@kernel.org>
On Wed, Jun 07, 2023 at 10:14:22AM +0200, Christian Brauner wrote: > This assumes that all drivers deal with additional ->open() calls for > each partition correctly which I assumed you checked so, They have to, because they already get the additional open for extra opens of the whole device. The current behavior is: open("/dev/vdb", ...) ->open called open("/dev/vdb", ...) ->open called ---- open("/dev/vdb", ...) ->open called open("/dev/vdb1", ...) ->open called ---- open("/dev/vdb1", ...) ->open called open("/dev/vdb1", ...) ->open NOT called which is very inconsistent.
On 6/6/23 09:39, Christoph Hellwig wrote: > For whole devices ->open is called for each open, but for partitions it > is only called on the first open of a partition. This is problematic > as various block drivers look at open flags and might not do all setup > for ioctl only or NDELAY opens. > > Signed-off-by: Christoph Hellwig <hch@lst.de> > --- > block/bdev.c | 18 ++++++++---------- > 1 file changed, 8 insertions(+), 10 deletions(-) > Reviewed-by: Hannes Reinecke <hare@suse.de> Cheers, Hannes
diff --git a/block/bdev.c b/block/bdev.c index 5c46ff10770638..981f6135795138 100644 --- a/block/bdev.c +++ b/block/bdev.c @@ -683,9 +683,6 @@ static int blkdev_get_part(struct block_device *part, fmode_t mode) struct gendisk *disk = part->bd_disk; int ret; - if (atomic_read(&part->bd_openers)) - goto done; - ret = blkdev_get_whole(bdev_whole(part), mode); if (ret) return ret; @@ -694,9 +691,10 @@ static int blkdev_get_part(struct block_device *part, fmode_t mode) if (!bdev_nr_sectors(part)) goto out_blkdev_put; - disk->open_partitions++; - set_init_blocksize(part); -done: + if (!atomic_read(&part->bd_openers)) { + disk->open_partitions++; + set_init_blocksize(part); + } atomic_inc(&part->bd_openers); return 0; @@ -709,10 +707,10 @@ static void blkdev_put_part(struct block_device *part, fmode_t mode) { struct block_device *whole = bdev_whole(part); - if (!atomic_dec_and_test(&part->bd_openers)) - return; - blkdev_flush_mapping(part); - whole->bd_disk->open_partitions--; + if (atomic_dec_and_test(&part->bd_openers)) { + blkdev_flush_mapping(part); + whole->bd_disk->open_partitions--; + } blkdev_put_whole(whole, mode); }
For whole devices ->open is called for each open, but for partitions it is only called on the first open of a partition. This is problematic as various block drivers look at open flags and might not do all setup for ioctl only or NDELAY opens. Signed-off-by: Christoph Hellwig <hch@lst.de> --- block/bdev.c | 18 ++++++++---------- 1 file changed, 8 insertions(+), 10 deletions(-)