Hi, On 2020/1/6 18:05, Ming Lei wrote: > On Mon, Jan 06, 2020 at 05:41:45PM +0800, Hou Tao wrote: >> Hi, >> [snipped] >> Yes. The solution you proposed also adds an invocation of percpu_ref_tryget_live() >> in the fast path. Not sure which one will have a better performance. However the >> reason we prefer the index caching is the simplicity instead of performance. > > No, hd_struct_try_get() and hd_struct_get() is always called once for one IO, the > patch I proposed changes nothing about this usage. > > Please take a close look at the patch: > > https://lore.kernel.org/linux-block/5cc465cc-d68c-088e-0729-2695279c7853@xxxxxxxxxx/T/#m8f3e6b4e77eadf006ce142a84c966f50f3a9ae26 > > which just moves hd_struct_try_get() from blk_account_io_start() into > disk_map_sector_rcu(), doesn't it? > Yes, you are right. And a little suggestion for your patch: @@ -283,8 +289,9 @@ void delete_partition(struct gendisk *disk, int partno) if (!part) return; + get_device(disk_to_dev(disk)); rcu_assign_pointer(ptbl->part[partno], NULL); - rcu_assign_pointer(ptbl->last_lookup, NULL); + kobject_put(part->holder_dir); device_del(part_to_dev(part)); Could we move the call of get_device() into add_partition, and that will make the assignment of disk to p->disk in add_partition() be natural ? Maybe there is no need to add a new disk field in hd_struct, because the kobject of gendisk is already the parent of hd_struct. But make use of part->__dev.parent after the calling of device_del() is a bad_idea. Regards, Tao > > Thanks, > Ming > > > . >
