* [PATCH] fs: don't flush pagecache when expanding block device
@ 2018-03-16 6:55 shunki-fujita
2018-03-16 22:22 ` Andrew Morton
` (2 more replies)
0 siblings, 3 replies; 4+ messages in thread
From: shunki-fujita @ 2018-03-16 6:55 UTC (permalink / raw)
To: linux-fsdevel, linux-kernel; +Cc: viro, akpm, shunki-fujita
When changing the size of a block device, its all caches are freed.
It's necessary on shrinking to prevent spurious I/Os to the disappeared region.
However, on expanding, such kind of I/Os doesn't happen.
Similar things can be considered for btrfs filesystem resize and resize2fs,
but they are designed not to cache drops when expanding.
Therefore this patch removes unnecessary cache drop.
Signed-off-by: Shunki Fujita <shunki-fujita@cybozu.co.jp>
---
fs/block_dev.c | 9 ++++++++-
1 file changed, 8 insertions(+), 1 deletion(-)
diff --git a/fs/block_dev.c b/fs/block_dev.c
index fe09ef9..243f4002 100644
--- a/fs/block_dev.c
+++ b/fs/block_dev.c
@@ -1337,7 +1337,14 @@ void check_disk_size_change(struct gendisk *disk, struct block_device *bdev)
"%s: detected capacity change from %lld to %lld\n",
disk->disk_name, bdev_size, disk_size);
i_size_write(bdev->bd_inode, disk_size);
- flush_disk(bdev, false);
+ if (bdev_size > disk_size) {
+ flush_disk(bdev, false);
+ } else {
+ if (!bdev->bd_disk)
+ return;
+ if (disk_part_scan_enabled(bdev->bd_disk))
+ bdev->bd_invalidated = 1;
+ }
}
}
EXPORT_SYMBOL(check_disk_size_change);
--
2.7.4
^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [PATCH] fs: don't flush pagecache when expanding block device
2018-03-16 6:55 [PATCH] fs: don't flush pagecache when expanding block device shunki-fujita
@ 2018-03-16 22:22 ` Andrew Morton
2018-03-19 10:52 ` Shunki Fujita
2018-03-19 11:00 ` [PATCH v2] " shunki-fujita
2 siblings, 0 replies; 4+ messages in thread
From: Andrew Morton @ 2018-03-16 22:22 UTC (permalink / raw)
To: shunki-fujita; +Cc: linux-fsdevel, linux-kernel, viro, Jens Axboe
On Fri, 16 Mar 2018 15:55:53 +0900 shunki-fujita <shunki-fujita@cybozu.co.jp> wrote:
> When changing the size of a block device, its all caches are freed.
> It's necessary on shrinking to prevent spurious I/Os to the disappeared region.
> However, on expanding, such kind of I/Os doesn't happen.
>
> Similar things can be considered for btrfs filesystem resize and resize2fs,
> but they are designed not to cache drops when expanding.
> Therefore this patch removes unnecessary cache drop.
Yes, the patch removes the flush_disk() call when shrinking. But it
adds additional code which is unchangelogged and uncommented. What's
happening here?
> --- a/fs/block_dev.c
> +++ b/fs/block_dev.c
> @@ -1337,7 +1337,14 @@ void check_disk_size_change(struct gendisk *disk, struct block_device *bdev)
> "%s: detected capacity change from %lld to %lld\n",
> disk->disk_name, bdev_size, disk_size);
> i_size_write(bdev->bd_inode, disk_size);
> - flush_disk(bdev, false);
> + if (bdev_size > disk_size) {
> + flush_disk(bdev, false);
> + } else {
> + if (!bdev->bd_disk)
> + return;
> + if (disk_part_scan_enabled(bdev->bd_disk))
> + bdev->bd_invalidated = 1;
> + }
> }
> }
Oh, I see. It's a copy-n-paste of some mystery uncommented,
lost-in-the-mists-of-time code from flush_disk(). Argh.
The world would be a better place if we could move that bit of code
into a separate function, figure out what it does, document it and call
it from both sites.
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] fs: don't flush pagecache when expanding block device
2018-03-16 6:55 [PATCH] fs: don't flush pagecache when expanding block device shunki-fujita
2018-03-16 22:22 ` Andrew Morton
@ 2018-03-19 10:52 ` Shunki Fujita
2018-03-19 11:00 ` [PATCH v2] " shunki-fujita
2 siblings, 0 replies; 4+ messages in thread
From: Shunki Fujita @ 2018-03-19 10:52 UTC (permalink / raw)
To: Andrew Morton; +Cc: linux-fsdevel, linux-kernel, viro, Jens Axboe
Hi Andrew,
> On Fri, 16 Mar 2018 15:55:53 +0900 shunki-fujita <shunki-fujita@cybozu.co.jp> wrote:
>
> > When changing the size of a block device, its all caches are freed.
> > It's necessary on shrinking to prevent spurious I/Os to the disappeared region.
> > However, on expanding, such kind of I/Os doesn't happen.
> >
> > Similar things can be considered for btrfs filesystem resize and resize2fs,
> > but they are designed not to cache drops when expanding.
> > Therefore this patch removes unnecessary cache drop.
>
> Yes, the patch removes the flush_disk() call when shrinking. But it
> adds additional code which is unchangelogged and uncommented. What's
> happening here?
>
> > --- a/fs/block_dev.c
> > +++ b/fs/block_dev.c
> > @@ -1337,7 +1337,14 @@ void check_disk_size_change(struct gendisk *disk, struct block_device *bdev)
> > "%s: detected capacity change from %lld to %lld\n",
> > disk->disk_name, bdev_size, disk_size);
> > i_size_write(bdev->bd_inode, disk_size);
> > - flush_disk(bdev, false);
> > + if (bdev_size > disk_size) {
> > + flush_disk(bdev, false);
> > + } else {
> > + if (!bdev->bd_disk)
> > + return;
> > + if (disk_part_scan_enabled(bdev->bd_disk))
> > + bdev->bd_invalidated = 1;
> > + }
> > }
> > }
>
> Oh, I see. It's a copy-n-paste of some mystery uncommented,
> lost-in-the-mists-of-time code from flush_disk(). Argh.
>
> The world would be a better place if we could move that bit of code
> into a separate function, figure out what it does, document it and call
> it from both sites.
>
This was an unnecessary additional code.
Since bdev-> bd_invalidated always 0 after check_disk_size_change (), no additional code was required.
https://github.com/torvalds/linux/blob/master/fs/block_dev.c#L1365-L1366
I will repost the fix patch.
Thanks,
Shunki
^ permalink raw reply [flat|nested] 4+ messages in thread
* [PATCH v2] fs: don't flush pagecache when expanding block device
2018-03-16 6:55 [PATCH] fs: don't flush pagecache when expanding block device shunki-fujita
2018-03-16 22:22 ` Andrew Morton
2018-03-19 10:52 ` Shunki Fujita
@ 2018-03-19 11:00 ` shunki-fujita
2 siblings, 0 replies; 4+ messages in thread
From: shunki-fujita @ 2018-03-19 11:00 UTC (permalink / raw)
To: akpm; +Cc: linux-fsdevel, linux-kernel, viro, axboe, shunki-fujita
When changing the size of a block device, its all caches are freed.
It's necessary on shrinking to prevent spurious I/Os to the disappeared region.
However, on expanding, such kind of I/Os doesn't happen.
Similar things can be considered for btrfs filesystem resize and resize2fs,
but they are designed not to cache drops when expanding.
Therefore this patch removes unnecessary cache drop.
Signed-off-by: Shunki Fujita <shunki-fujita@cybozu.co.jp>
---
Changes in v2:
- Remove unnecessary changes (Since bdev-> bd_invalidated always 0 after check_disk_size_change ()[1])
- Add document of check_disk_size_change ()
[1] https://github.com/torvalds/linux/blob/master/fs/block_dev.c#L1365-L1366
fs/block_dev.c | 6 ++++--
1 file changed, 4 insertions(+), 2 deletions(-)
diff --git a/fs/block_dev.c b/fs/block_dev.c
index fe09ef9..63f1269 100644
--- a/fs/block_dev.c
+++ b/fs/block_dev.c
@@ -1324,7 +1324,8 @@ static void flush_disk(struct block_device *bdev, bool kill_dirty)
* @bdev: struct bdev to adjust.
*
* This routine checks to see if the bdev size does not match the disk size
- * and adjusts it if it differs.
+ * and adjusts it if it differs. When shrinking the bdev size, its all caches
+ * are freed.
*/
void check_disk_size_change(struct gendisk *disk, struct block_device *bdev)
{
@@ -1337,7 +1338,8 @@ void check_disk_size_change(struct gendisk *disk, struct block_device *bdev)
"%s: detected capacity change from %lld to %lld\n",
disk->disk_name, bdev_size, disk_size);
i_size_write(bdev->bd_inode, disk_size);
- flush_disk(bdev, false);
+ if (bdev_size > disk_size)
+ flush_disk(bdev, false);
}
}
EXPORT_SYMBOL(check_disk_size_change);
--
2.7.4
^ permalink raw reply related [flat|nested] 4+ messages in thread
end of thread, other threads:[~2018-03-19 11:01 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-03-16 6:55 [PATCH] fs: don't flush pagecache when expanding block device shunki-fujita
2018-03-16 22:22 ` Andrew Morton
2018-03-19 10:52 ` Shunki Fujita
2018-03-19 11:00 ` [PATCH v2] " shunki-fujita
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.