| Message ID | 20170209124433.2626-2-jack@suse.cz (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Thu, Feb 09, 2017 at 01:44:24PM +0100, Jan Kara wrote: > Move bdev_unhash_inode() after invalidate_partition() as > invalidate_partition() looks up bdev and will unnecessarily recreate it > if bdev_unhash_inode() destroyed it. Also use part_devt() when calling > bdev_unhash_inode() instead of manually creating the device number. > > Signed-off-by: Jan Kara <jack@suse.cz> Acked-by: Tejun Heo <tj@kernel.org> > @@ -648,9 +648,8 @@ void del_gendisk(struct gendisk *disk) > disk_part_iter_init(&piter, disk, > DISK_PITER_INCL_EMPTY | DISK_PITER_REVERSE); > while ((part = disk_part_iter_next(&piter))) { > - bdev_unhash_inode(MKDEV(disk->major, > - disk->first_minor + part->partno)); > invalidate_partition(disk, part->partno); > + bdev_unhash_inode(part_devt(part)); > delete_partition(disk, part->partno); So, before this patch, invalidate_partition() would have operated on a newly created inode and thus wouldn't have actually invalidated the existing bdev / mapping, right? Thanks.
On Sun 12-02-17 12:58:33, Tejun Heo wrote: > On Thu, Feb 09, 2017 at 01:44:24PM +0100, Jan Kara wrote: > > Move bdev_unhash_inode() after invalidate_partition() as > > invalidate_partition() looks up bdev and will unnecessarily recreate it > > if bdev_unhash_inode() destroyed it. Also use part_devt() when calling > > bdev_unhash_inode() instead of manually creating the device number. > > > > Signed-off-by: Jan Kara <jack@suse.cz> > > Acked-by: Tejun Heo <tj@kernel.org> > > > @@ -648,9 +648,8 @@ void del_gendisk(struct gendisk *disk) > > disk_part_iter_init(&piter, disk, > > DISK_PITER_INCL_EMPTY | DISK_PITER_REVERSE); > > while ((part = disk_part_iter_next(&piter))) { > > - bdev_unhash_inode(MKDEV(disk->major, > > - disk->first_minor + part->partno)); > > invalidate_partition(disk, part->partno); > > + bdev_unhash_inode(part_devt(part)); > > delete_partition(disk, part->partno); > > So, before this patch, invalidate_partition() would have operated on a > newly created inode and thus wouldn't have actually invalidated the > existing bdev / mapping, right? Yes, that is another effect. I'll add a note about this to the changelog. Thanks for noting this. Honza
diff --git a/block/genhd.c b/block/genhd.c index d9ccd42f3675..6cb9f3a34a92 100644 --- a/block/genhd.c +++ b/block/genhd.c @@ -648,9 +648,8 @@ void del_gendisk(struct gendisk *disk) disk_part_iter_init(&piter, disk, DISK_PITER_INCL_EMPTY | DISK_PITER_REVERSE); while ((part = disk_part_iter_next(&piter))) { - bdev_unhash_inode(MKDEV(disk->major, - disk->first_minor + part->partno)); invalidate_partition(disk, part->partno); + bdev_unhash_inode(part_devt(part)); delete_partition(disk, part->partno); } disk_part_iter_exit(&piter);
Move bdev_unhash_inode() after invalidate_partition() as invalidate_partition() looks up bdev and will unnecessarily recreate it if bdev_unhash_inode() destroyed it. Also use part_devt() when calling bdev_unhash_inode() instead of manually creating the device number. Signed-off-by: Jan Kara <jack@suse.cz> --- block/genhd.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-)