Hi,

在 2023/02/09 17:57, Jan Kara 写道:

On Thu 09-02-23 17:32:45, Yu Kuai wrote:

quoted

在 2023/02/09 17:04, Jan Kara 写道:

quoted

On Wed 08-02-23 22:13:02, Yu Kuai wrote:

quoted

Hi,

在 2023/02/08 20:02, Jan Kara 写道:

quoted

After some thought I don't like opencoding blkdev_get_by_dev() in disk_scan
partitions. But I agree Christoph's approach with blkdev_get_whole() does
not quite work either. We could propagate holder/owner into
blkdev_get_whole() to fix Christoph's check but still we are left with a
question what to do with GD_NEED_PART_SCAN set bit when we get into
blkdev_get_whole() and find out we are not elligible to rescan partitions.
Because then some exclusive opener later might be caught by surprise when
the partition rescan happens due to this bit being set from the past failed
attempt to rescan.

So what we could do is play a similar trick as we do in the loop device and
do in disk_scan_partitions():

	/*
	 * If we don't hold exclusive handle for the device, upgrade to it
	 * here to avoid changing partitions under exclusive owner.
	 */
	if (!(mode & FMODE_EXCL)) {

This is not necessary, all the caller make sure FMODE_EXCL is not set.

Yes, but we need to propagate it correctly from blkdev_common_ioctl() now,
exactly so that ioctl does not fail if you exclusively opened the device as
you realized below :)

Ok, I get it that you want to pass in FMODE_EXCL from ioctl(). But I'm
not sure let others fail to open device extl is a good idea.

Other exclusive openers will not fail. They will block until we call
bd_abort_claiming() after the partitions have been reread.

Yes, you're right, rescan and other exclusively openers will be
synchronized by bd_prepare_to_claim().

quoted

I still prefer to open code blkdev_get_by_dev(), because many operations
is not necessary here. And this way, we can clear GD_NEED_PART_SCAN
inside open_mutex if rescan failed.

I understand many operations are not needed there but this is not a hot
path and leaking of bdev internal details into genhd.c is not a good
practice either (e.g. you'd have to make blkdev_get_whole() extern).

I was thinking that disk_scan_partitions() can be moved to bdev.c to
avoid that...

We could create a special helper for partition rescan in block/bdev.c to
hide the details but think that bd_start_claiming() - bd_abort_claiming()
trick is the simplest solution to temporarily grab exclusive ownership if
we don't have it.

Now I'm good with this solution. By the way, do you think we must make
sure the first partition scan will be proceed?

Thanks,
Kuai

								Honza