>majianpeng <majianpeng@xxxxxxxxx> writes:
>
>>>majianpeng <majianpeng@xxxxxxxxx> writes:
>>>
>>>> For async-write on block device,if device removed,but the vfs don't know it.
>>>> It will continue to do.
>>>> Patch1 set size of inode of block device to zero when removed disk.By this,vfs know
>>>> disk changed.
>>>> Path2 add size-check on blk_aio_write.If pos of write larger than size of inode,it will
>>>> return zero.So the user can check disk state.
>>>
>>>OK, so the basic problem is that __generic_file_aio_write will always
>>>return 0 after device removal, yes? I'm not sure why that's a real
>>>issue, can you explain exactly why you're trying to change this?
>>>
>> At prenset, the __generic_file_aio_write don't return zero rather that the wanted size.
>> So the user can't know the disk removed.
>> For example:
>> dd if=/dev/zero of=usb-disk bs=64k
>> When removed usb-disk, dd stoped until reached the endof usb-disk.
>
>Ah, right, it's just writing to the page cache. I think the only reason
>you get more timely errors when doing the same thing to a file on a file
>system is that there is some synchronous metadata or journal I/O that
>will get EIO and result in the file system being set read-only.
>
>The bigger question is whether we want to change this long-standing
>behaviour of how our write-back cache works. I don't know that it's
>really worth it, honestly. If you want to ensure data is on disk, you
>open the file O_SYNC or you issue an fsync, and those calls will return
>an error for a removed block device. So, I guess I'll ask the same
>question again: why are you looking at this? Is there some application
>you care about that does buffered I/O to the block device and never does
>an fsync?
>
>> Using this patch, after removed disk, the aio-write will return zero.I
>> think the upper user will check. (or if the size of block is zero, we
>> return -ENOSPC).
>>
>>>As for your patches, I don't think that putting the i_size_write into
>>>invalidate_partitions is a good idea. Consider the case of rescanning
>>>partitions: you will always detect a size change now, which is not good.
>>>
>> Yes.But in func rescan_partitions, after invalidate_partitions it will
>> call check_disk_size_change to set size of block_device.
>
>The problem with doing an i_size_write of 0 inside of
>invalidate_partitions is that it isn't just called for the case where a
>device is removed. A user can initiate a rescan of partitions. In such
>a case, we don't want to evict all of the cached data for unchanged
>partitions.
>
>The call chain is like this:
>
>blkdev_ioctl
>blkdev_reread_part
>rescan_partitions
>check_disk_size_change
>
>Now look and see what check_disk_size_change will do when it finds out
>that the size has changed:
>
>void check_disk_size_change(struct gendisk *disk, struct block_device
>*bdev)
>{
> loff_t disk_size, bdev_size;
>
> disk_size = (loff_t)get_capacity(disk) << 9;
> bdev_size = i_size_read(bdev->bd_inode);
> if (disk_size != bdev_size) {
> char name[BDEVNAME_SIZE];
>
> disk_name(disk, 0, name);
> printk(KERN_INFO
> "%s: detected capacity change from %lld to
> %lld\n",
> name, bdev_size, disk_size);
> i_size_write(bdev->bd_inode, disk_size);
> flush_disk(bdev, false); <=============
> }
>}
>
>That will invalidate all of the metadata for any mounted file systems on
>the device. Also, you'll get a big nasty warning if any files are dirty:
>
> printk(KERN_WARNING "VFS: busy inodes on changed media or "
> "resized disk %s\n", name);
>
>And the reality is that we haven't changed anything, so there's no need
>for this.
Yes. How about those code:
diff --git a/block/genhd.c b/block/genhd.c
index 791f419..c279b34 100644
--- a/block/genhd.c
+++ b/block/genhd.c
@@ -634,6 +634,7 @@ void del_gendisk(struct gendisk *disk)
{
struct disk_part_iter piter;
struct hd_struct *part;
+ struct block_device *bdev;
disk_del_events(disk);
@@ -642,12 +643,25 @@ void del_gendisk(struct gendisk *disk)
DISK_PITER_INCL_EMPTY | DISK_PITER_REVERSE);
while ((part = disk_part_iter_next(&piter))) {
invalidate_partition(disk, part->partno);
+ bdev = bdget_disk(disk, part->partno);
+ if (bdev) {
+ i_size_write(bdev->bd_inode, 0);
+ bdput(bdev);
+ }
+
delete_partition(disk, part->partno);
}
disk_part_iter_exit(&piter);
invalidate_partition(disk, 0);
set_capacity(disk, 0);
+
+ bdev = bdget_disk(disk, 0);
+ if (bdev) {
+ i_size_write(bdev->bd_inode, 0);
+ bdput(bdev);
+ }
+
disk->flags &= ~GENHD_FL_UP;
sysfs_remove_link(&disk_to_dev(disk)->kobj, "bdi");
We only set inode to zero in del_gendisk.
>
>After looking at the code further, why do you even need to add the
>second patch? generic_write_checks will check for a write past the end
>of the block device.
>
Yes, in generic_write_checks it will check size so patch2 don't need.
Thanks!
Jianpeng Ma
?韬{.n?????%??檩??w?{.n???{饼?z鳐??骅w* jg????????G??⒏ ⒎?:+v???????? ????＂??????
