Messages in this thread | | | Date | Wed, 8 Apr 2015 17:03:25 +0200 | From | Peter Zijlstra <> | Subject | Re: [PATCH 1/7] block: export blkdev_reread_part() and __blkdev_reread_part() |
| |
On Wed, Apr 08, 2015 at 10:50:56PM +0800, Ming Lei wrote: > > +/* > > + * This is an exported API for the block driver, and will not > > + * acquire bd_mutex, leaving it up to the caller to handle > > + * any necessary locking. > > Actually, the function is introduced and should be used in case > that bd_mutex has been held already, such as clearing fd in > loop release(). > > > + */ > > +int __blkdev_reread_part(struct block_device *bdev) > > { > > struct gendisk *disk = bdev->bd_disk; > >
lockdep_assert_held(&bdev->bd_mutex);
is an excellent means of avoiding that comment and verifying its actually true :-)
> > if (!disk_part_scan_enabled(disk) || bdev != bdev->bd_contains) > > return -EINVAL; > > if (!capable(CAP_SYS_ADMIN)) > > return -EACCES; > > + > > + return rescan_partitions(disk, bdev); > > +} > > +EXPORT_SYMBOL(__blkdev_reread_part); > > + > > +/* > > + * This is an exported API for the block driver, and will > > + * acquire bd_mutex. Make sure you aren't calling it with > > + * bd_mutex already held, or we'll return -EBUSY. > > Strictly speaking, it should be "Make sure you aren't calling it > with bd_mutex already held in current context". > > > + */ > > +int blkdev_reread_part(struct block_device *bdev) > > +{ > > + int res; > > + > > if (!mutex_trylock(&bdev->bd_mutex)) > > return -EBUSY;
Is that really needed? It seems rather poor form.
> > - res = rescan_partitions(disk, bdev); > > + res = __blkdev_reread_part(bdev); > > mutex_unlock(&bdev->bd_mutex); > > + > > return res; > > } > > +EXPORT_SYMBOL(blkdev_reread_part);
| |