| Message ID | 20191021083808.19335-2-jack@suse.cz (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | bdev: Refresh bdev size for disks without partitioning | expand |
On 10/21/19 10:38 AM, Jan Kara wrote: > Currently, block device size in not updated on second and further open > for block devices where partition scan is disabled. This is particularly > annoying for example for DVD drives as that means block device size does > not get updated once the media is inserted into a drive if the device is > already open when inserting the media. This is actually always the case > for example when pktcdvd is in use. > > Fix the problem by revalidating block device size on every open even for > devices with partition scan disabled. > > Signed-off-by: Jan Kara <jack@suse.cz> > --- > fs/block_dev.c | 19 ++++++++++--------- > 1 file changed, 10 insertions(+), 9 deletions(-) > > diff --git a/fs/block_dev.c b/fs/block_dev.c > index 88c6d35ec71d..d612468ee66b 100644 > --- a/fs/block_dev.c > +++ b/fs/block_dev.c > @@ -1403,11 +1403,7 @@ static void flush_disk(struct block_device *bdev, bool kill_dirty) > "resized disk %s\n", > bdev->bd_disk ? bdev->bd_disk->disk_name : ""); > } > - > - if (!bdev->bd_disk) > - return; > - if (disk_part_scan_enabled(bdev->bd_disk)) > - bdev->bd_invalidated = 1; > + bdev->bd_invalidated = 1; > } > > /** > @@ -1514,10 +1510,15 @@ static void __blkdev_put(struct block_device *bdev, fmode_t mode, int for_part); > > static void bdev_disk_changed(struct block_device *bdev, bool invalidate) > { > - if (invalidate) > - invalidate_partitions(bdev->bd_disk, bdev); > - else > - rescan_partitions(bdev->bd_disk, bdev); > + if (disk_part_scan_enabled(bdev->bd_disk)) { > + if (invalidate) > + invalidate_partitions(bdev->bd_disk, bdev); > + else > + rescan_partitions(bdev->bd_disk, bdev); Maybe use the new common helper to replace above. Thanks, Guoqing > + } else { > + check_disk_size_change(bdev->bd_disk, bdev, !invalidate); > + bdev->bd_invalidated = 0; > + } > } > > /*
On Mon 21-10-19 10:49:54, Guoqing Jiang wrote: > > > On 10/21/19 10:38 AM, Jan Kara wrote: > > Currently, block device size in not updated on second and further open > > for block devices where partition scan is disabled. This is particularly > > annoying for example for DVD drives as that means block device size does > > not get updated once the media is inserted into a drive if the device is > > already open when inserting the media. This is actually always the case > > for example when pktcdvd is in use. > > > > Fix the problem by revalidating block device size on every open even for > > devices with partition scan disabled. > > > > Signed-off-by: Jan Kara <jack@suse.cz> > > --- > > fs/block_dev.c | 19 ++++++++++--------- > > 1 file changed, 10 insertions(+), 9 deletions(-) > > > > diff --git a/fs/block_dev.c b/fs/block_dev.c > > index 88c6d35ec71d..d612468ee66b 100644 > > --- a/fs/block_dev.c > > +++ b/fs/block_dev.c > > @@ -1403,11 +1403,7 @@ static void flush_disk(struct block_device *bdev, bool kill_dirty) > > "resized disk %s\n", > > bdev->bd_disk ? bdev->bd_disk->disk_name : ""); > > } > > - > > - if (!bdev->bd_disk) > > - return; > > - if (disk_part_scan_enabled(bdev->bd_disk)) > > - bdev->bd_invalidated = 1; > > + bdev->bd_invalidated = 1; > > } > > /** > > @@ -1514,10 +1510,15 @@ static void __blkdev_put(struct block_device *bdev, fmode_t mode, int for_part); > > static void bdev_disk_changed(struct block_device *bdev, bool invalidate) > > { > > - if (invalidate) > > - invalidate_partitions(bdev->bd_disk, bdev); > > - else > > - rescan_partitions(bdev->bd_disk, bdev); > > + if (disk_part_scan_enabled(bdev->bd_disk)) { > > + if (invalidate) > > + invalidate_partitions(bdev->bd_disk, bdev); > > + else > > + rescan_partitions(bdev->bd_disk, bdev); > > Maybe use the new common helper to replace above. What do you mean exactly? Because there's only this place that has the code pattern here... Honza > > + } else { > > + check_disk_size_change(bdev->bd_disk, bdev, !invalidate); > > + bdev->bd_invalidated = 0; > > + } > > } > > /* >
On 10/21/19 11:12 AM, Jan Kara wrote: > On Mon 21-10-19 10:49:54, Guoqing Jiang wrote: >> >> On 10/21/19 10:38 AM, Jan Kara wrote: >>> Currently, block device size in not updated on second and further open >>> for block devices where partition scan is disabled. This is particularly >>> annoying for example for DVD drives as that means block device size does >>> not get updated once the media is inserted into a drive if the device is >>> already open when inserting the media. This is actually always the case >>> for example when pktcdvd is in use. >>> >>> Fix the problem by revalidating block device size on every open even for >>> devices with partition scan disabled. >>> >>> Signed-off-by: Jan Kara <jack@suse.cz> >>> --- >>> fs/block_dev.c | 19 ++++++++++--------- >>> 1 file changed, 10 insertions(+), 9 deletions(-) >>> >>> diff --git a/fs/block_dev.c b/fs/block_dev.c >>> index 88c6d35ec71d..d612468ee66b 100644 >>> --- a/fs/block_dev.c >>> +++ b/fs/block_dev.c >>> @@ -1403,11 +1403,7 @@ static void flush_disk(struct block_device *bdev, bool kill_dirty) >>> "resized disk %s\n", >>> bdev->bd_disk ? bdev->bd_disk->disk_name : ""); >>> } >>> - >>> - if (!bdev->bd_disk) >>> - return; >>> - if (disk_part_scan_enabled(bdev->bd_disk)) >>> - bdev->bd_invalidated = 1; >>> + bdev->bd_invalidated = 1; >>> } >>> /** >>> @@ -1514,10 +1510,15 @@ static void __blkdev_put(struct block_device *bdev, fmode_t mode, int for_part); >>> static void bdev_disk_changed(struct block_device *bdev, bool invalidate) >>> { >>> - if (invalidate) >>> - invalidate_partitions(bdev->bd_disk, bdev); >>> - else >>> - rescan_partitions(bdev->bd_disk, bdev); >>> + if (disk_part_scan_enabled(bdev->bd_disk)) { >>> + if (invalidate) >>> + invalidate_partitions(bdev->bd_disk, bdev); >>> + else >>> + rescan_partitions(bdev->bd_disk, bdev); >> Maybe use the new common helper to replace above. > What do you mean exactly? Because there's only this place that has the code > pattern here... The above looks same as the new function. +static void bdev_disk_changed(struct block_device *bdev, bool invalidate) +{ + if (invalidate) + invalidate_partitions(bdev->bd_disk, bdev); + else + rescan_partitions(bdev->bd_disk, bdev); +} So maybe just call it in the 'if' branch, am I misunderstand something? static void bdev_disk_changed(struct block_device *bdev, bool invalidate) { if (disk_part_scan_enabled(bdev->bd_disk)) bdev_disk_changed(bdev, invalidate) else { check_disk_size_change(bdev->bd_disk, bdev, !invalidate); bdev->bd_invalidated = 0; } } Thanks, Guoqing
On 21/10/2019 11:27, Guoqing Jiang wrote: >>>> static void bdev_disk_changed(struct block_device *bdev, bool >>>> invalidate) >>>> { >>>> - if (invalidate) >>>> - invalidate_partitions(bdev->bd_disk, bdev); >>>> - else >>>> - rescan_partitions(bdev->bd_disk, bdev); >>>> + if (disk_part_scan_enabled(bdev->bd_disk)) { >>>> + if (invalidate) >>>> + invalidate_partitions(bdev->bd_disk, bdev); >>>> + else >>>> + rescan_partitions(bdev->bd_disk, bdev); >>> Maybe use the new common helper to replace above. >> What do you mean exactly? Because there's only this place that has the >> code >> pattern here... > > The above looks same as the new function. The above is changed in the new bdev_disk_changed() function. Patch 1/2 factors out the pattern and this patch changes the behaviour
On 10/21/19 11:36 AM, Johannes Thumshirn wrote: > On 21/10/2019 11:27, Guoqing Jiang wrote: >>>>> static void bdev_disk_changed(struct block_device *bdev, bool >>>>> invalidate) >>>>> { >>>>> - if (invalidate) >>>>> - invalidate_partitions(bdev->bd_disk, bdev); >>>>> - else >>>>> - rescan_partitions(bdev->bd_disk, bdev); >>>>> + if (disk_part_scan_enabled(bdev->bd_disk)) { >>>>> + if (invalidate) >>>>> + invalidate_partitions(bdev->bd_disk, bdev); >>>>> + else >>>>> + rescan_partitions(bdev->bd_disk, bdev); >>>> Maybe use the new common helper to replace above. >>> What do you mean exactly? Because there's only this place that has the >>> code >>> pattern here... >> The above looks same as the new function. > The above is changed in the new bdev_disk_changed() function. Patch 1/2 > factors out the pattern and this patch changes the behaviour > Oops, yes, sorry for the noise. Thanks, Guoqing
On Mon 21-10-19 11:27:36, Guoqing Jiang wrote: > > > On 10/21/19 11:12 AM, Jan Kara wrote: > > On Mon 21-10-19 10:49:54, Guoqing Jiang wrote: > > > > > > On 10/21/19 10:38 AM, Jan Kara wrote: > > > > Currently, block device size in not updated on second and further open > > > > for block devices where partition scan is disabled. This is particularly > > > > annoying for example for DVD drives as that means block device size does > > > > not get updated once the media is inserted into a drive if the device is > > > > already open when inserting the media. This is actually always the case > > > > for example when pktcdvd is in use. > > > > > > > > Fix the problem by revalidating block device size on every open even for > > > > devices with partition scan disabled. > > > > > > > > Signed-off-by: Jan Kara <jack@suse.cz> > > > > --- > > > > fs/block_dev.c | 19 ++++++++++--------- > > > > 1 file changed, 10 insertions(+), 9 deletions(-) > > > > > > > > diff --git a/fs/block_dev.c b/fs/block_dev.c > > > > index 88c6d35ec71d..d612468ee66b 100644 > > > > --- a/fs/block_dev.c > > > > +++ b/fs/block_dev.c > > > > @@ -1403,11 +1403,7 @@ static void flush_disk(struct block_device *bdev, bool kill_dirty) > > > > "resized disk %s\n", > > > > bdev->bd_disk ? bdev->bd_disk->disk_name : ""); > > > > } > > > > - > > > > - if (!bdev->bd_disk) > > > > - return; > > > > - if (disk_part_scan_enabled(bdev->bd_disk)) > > > > - bdev->bd_invalidated = 1; > > > > + bdev->bd_invalidated = 1; > > > > } > > > > /** > > > > @@ -1514,10 +1510,15 @@ static void __blkdev_put(struct block_device *bdev, fmode_t mode, int for_part); > > > > static void bdev_disk_changed(struct block_device *bdev, bool invalidate) > > > > { > > > > - if (invalidate) > > > > - invalidate_partitions(bdev->bd_disk, bdev); > > > > - else > > > > - rescan_partitions(bdev->bd_disk, bdev); > > > > + if (disk_part_scan_enabled(bdev->bd_disk)) { > > > > + if (invalidate) > > > > + invalidate_partitions(bdev->bd_disk, bdev); > > > > + else > > > > + rescan_partitions(bdev->bd_disk, bdev); > > > Maybe use the new common helper to replace above. > > What do you mean exactly? Because there's only this place that has the code > > pattern here... > > The above looks same as the new function. > > +static void bdev_disk_changed(struct block_device *bdev, bool invalidate) > +{ > + if (invalidate) > + invalidate_partitions(bdev->bd_disk, bdev); > + else > + rescan_partitions(bdev->bd_disk, bdev); > +} > > So maybe just call it in the 'if' branch, am I misunderstand something? I think you misparsed the diff ;) The code that you mention as duplicated just gets reindented by the patch. > static void bdev_disk_changed(struct block_device *bdev, bool invalidate) > { > if (disk_part_scan_enabled(bdev->bd_disk)) > bdev_disk_changed(bdev, invalidate) > else { > check_disk_size_change(bdev->bd_disk, bdev, !invalidate); > bdev->bd_invalidated = 0; > } > } This has infinite recursion in it - you call bdev_disk_changed() from bdev_disk_changed(). Honza
On 10/21/19 11:40 AM, Jan Kara wrote: > On Mon 21-10-19 11:27:36, Guoqing Jiang wrote: >> >> On 10/21/19 11:12 AM, Jan Kara wrote: >>> On Mon 21-10-19 10:49:54, Guoqing Jiang wrote: >>>> On 10/21/19 10:38 AM, Jan Kara wrote: >>>>> Currently, block device size in not updated on second and further open >>>>> for block devices where partition scan is disabled. This is particularly >>>>> annoying for example for DVD drives as that means block device size does >>>>> not get updated once the media is inserted into a drive if the device is >>>>> already open when inserting the media. This is actually always the case >>>>> for example when pktcdvd is in use. >>>>> >>>>> Fix the problem by revalidating block device size on every open even for >>>>> devices with partition scan disabled. >>>>> >>>>> Signed-off-by: Jan Kara <jack@suse.cz> >>>>> --- >>>>> fs/block_dev.c | 19 ++++++++++--------- >>>>> 1 file changed, 10 insertions(+), 9 deletions(-) >>>>> >>>>> diff --git a/fs/block_dev.c b/fs/block_dev.c >>>>> index 88c6d35ec71d..d612468ee66b 100644 >>>>> --- a/fs/block_dev.c >>>>> +++ b/fs/block_dev.c >>>>> @@ -1403,11 +1403,7 @@ static void flush_disk(struct block_device *bdev, bool kill_dirty) >>>>> "resized disk %s\n", >>>>> bdev->bd_disk ? bdev->bd_disk->disk_name : ""); >>>>> } >>>>> - >>>>> - if (!bdev->bd_disk) >>>>> - return; >>>>> - if (disk_part_scan_enabled(bdev->bd_disk)) >>>>> - bdev->bd_invalidated = 1; >>>>> + bdev->bd_invalidated = 1; >>>>> } >>>>> /** >>>>> @@ -1514,10 +1510,15 @@ static void __blkdev_put(struct block_device *bdev, fmode_t mode, int for_part); >>>>> static void bdev_disk_changed(struct block_device *bdev, bool invalidate) >>>>> { >>>>> - if (invalidate) >>>>> - invalidate_partitions(bdev->bd_disk, bdev); >>>>> - else >>>>> - rescan_partitions(bdev->bd_disk, bdev); >>>>> + if (disk_part_scan_enabled(bdev->bd_disk)) { >>>>> + if (invalidate) >>>>> + invalidate_partitions(bdev->bd_disk, bdev); >>>>> + else >>>>> + rescan_partitions(bdev->bd_disk, bdev); >>>> Maybe use the new common helper to replace above. >>> What do you mean exactly? Because there's only this place that has the code >>> pattern here... >> The above looks same as the new function. >> >> +static void bdev_disk_changed(struct block_device *bdev, bool invalidate) >> +{ >> + if (invalidate) >> + invalidate_partitions(bdev->bd_disk, bdev); >> + else >> + rescan_partitions(bdev->bd_disk, bdev); >> +} >> >> So maybe just call it in the 'if' branch, am I misunderstand something? > I think you misparsed the diff ;) The code that you mention as duplicated > just gets reindented by the patch. > >> static void bdev_disk_changed(struct block_device *bdev, bool invalidate) >> { >> if (disk_part_scan_enabled(bdev->bd_disk)) >> bdev_disk_changed(bdev, invalidate) >> else { >> check_disk_size_change(bdev->bd_disk, bdev, !invalidate); >> bdev->bd_invalidated = 0; >> } >> } > This has infinite recursion in it - you call bdev_disk_changed() from > bdev_disk_changed(). Right :-[, I need some coffee to refresh, sorry for the noise. BRs, Guoqing
diff --git a/fs/block_dev.c b/fs/block_dev.c index 88c6d35ec71d..d612468ee66b 100644 --- a/fs/block_dev.c +++ b/fs/block_dev.c @@ -1403,11 +1403,7 @@ static void flush_disk(struct block_device *bdev, bool kill_dirty) "resized disk %s\n", bdev->bd_disk ? bdev->bd_disk->disk_name : ""); } - - if (!bdev->bd_disk) - return; - if (disk_part_scan_enabled(bdev->bd_disk)) - bdev->bd_invalidated = 1; + bdev->bd_invalidated = 1; } /** @@ -1514,10 +1510,15 @@ static void __blkdev_put(struct block_device *bdev, fmode_t mode, int for_part); static void bdev_disk_changed(struct block_device *bdev, bool invalidate) { - if (invalidate) - invalidate_partitions(bdev->bd_disk, bdev); - else - rescan_partitions(bdev->bd_disk, bdev); + if (disk_part_scan_enabled(bdev->bd_disk)) { + if (invalidate) + invalidate_partitions(bdev->bd_disk, bdev); + else + rescan_partitions(bdev->bd_disk, bdev); + } else { + check_disk_size_change(bdev->bd_disk, bdev, !invalidate); + bdev->bd_invalidated = 0; + } } /*
Currently, block device size in not updated on second and further open for block devices where partition scan is disabled. This is particularly annoying for example for DVD drives as that means block device size does not get updated once the media is inserted into a drive if the device is already open when inserting the media. This is actually always the case for example when pktcdvd is in use. Fix the problem by revalidating block device size on every open even for devices with partition scan disabled. Signed-off-by: Jan Kara <jack@suse.cz> --- fs/block_dev.c | 19 ++++++++++--------- 1 file changed, 10 insertions(+), 9 deletions(-)