On Thu, Jan 04, 2018 at 04:38:03AM -0800, Christoph Hellwig wrote: > On Thu, Jan 04, 2018 at 06:39:24PM +0800, Ming Lei wrote: > > When detaching loop disk, neither we remove loop partitions, nor > > invalidate them. This way may cause data loss, and often confuse > > people. > > > > This patch fixes the above issue by removing & invalidating loop > > partitions in loop_clr_fd(), > > Please add a comment explaining the call (e.g. the weird loop > case of keeping unbound devices around). OK. > > Also dasd seems to implement this functionality by iterating over > all partitions and then calling ioctl_by_bdev(bdev, BLKPG, ..) on > it. Please switch it to you new helper assuming there is a good > reason to even do this to start with. CCing the maintainers to > confirm that. This patch doesn't consider to hold bdev lock, and loop's case can be a bit complicated since loop_clr_fd() can be called in release path. I will investigate a bit if it is possible to figure out a general helper to cover both. > > And while you'e at it split the block and loop bits into separate > patches. OK. > > > +void del_gendisk_partitions(struct gendisk *disk) > > Add a comment explanining the function, please. OK. > > > struct disk_part_iter piter; > > struct hd_struct *part; > > > > - blk_integrity_del(disk); > > - disk_del_events(disk); > > - > > /* invalidate stuff */ > > disk_part_iter_init(&piter, disk, > > DISK_PITER_INCL_EMPTY | DISK_PITER_REVERSE); > > @@ -714,7 +711,17 @@ void del_gendisk(struct gendisk *disk) > > disk_part_iter_exit(&piter); > > > > invalidate_partition(disk, 0); > > +} > > +EXPORT_SYMBOL(del_gendisk_partitions); > > EXPORT_SYMBOL_GPL, please. OK. -- Ming
