From mboxrd@z Thu Jan 1 00:00:00 1970 From: majianpeng Subject: Re: Re: [PATCH V2 0/2] Auto stop async-write on block device when device removed. Date: Sun, 29 Sep 2013 16:46:07 +0800 Message-ID: <201309291646039330492@gmail.com> References: <201309171121559232246@gmail.com> <201309241107330800706@gmail.com>, Reply-To: majianpeng Mime-Version: 1.0 Content-Type: text/plain; charset="gb2312" Content-Transfer-Encoding: base64 Cc: axboe , viro , LKML , linux-fsdevel To: "Jeff Moyer" Return-path: Received: from mail-pa0-f53.google.com ([209.85.220.53]:42388 "EHLO mail-pa0-f53.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1753483Ab3I2IqL (ORCPT ); Sun, 29 Sep 2013 04:46:11 -0400 Sender: linux-fsdevel-owner@vger.kernel.org List-ID: Pm1hamlhbnBlbmcgPG1hamlhbnBlbmdAZ21haWwuY29tPiB3cml0ZXM6DQo+DQo+Pj5tYWppYW5w ZW5nIDxtYWppYW5wZW5nQGdtYWlsLmNvbT4gd3JpdGVzOg0KPj4+DQo+Pj4+IEZvciBhc3luYy13 cml0ZSBvbiBibG9jayBkZXZpY2UsaWYgZGV2aWNlIHJlbW92ZWQsYnV0IHRoZSB2ZnMgZG9uJ3Qg a25vdyBpdC4NCj4+Pj4gSXQgd2lsbCBjb250aW51ZSB0byBkby4NCj4+Pj4gUGF0Y2gxIHNldCBz aXplIG9mIGlub2RlIG9mIGJsb2NrIGRldmljZSB0byB6ZXJvIHdoZW4gcmVtb3ZlZCBkaXNrLkJ5 IHRoaXMsdmZzIGtub3cgDQo+Pj4+IGRpc2sgY2hhbmdlZC4NCj4+Pj4gUGF0aDIgYWRkIHNpemUt Y2hlY2sgb24gYmxrX2Fpb193cml0ZS5JZiBwb3Mgb2Ygd3JpdGUgbGFyZ2VyIHRoYW4gc2l6ZSBv ZiBpbm9kZSxpdCB3aWxsDQo+Pj4+IHJldHVybiB6ZXJvLlNvIHRoZSB1c2VyIGNhbiBjaGVjayBk aXNrIHN0YXRlLg0KPj4+DQo+Pj5PSywgc28gdGhlIGJhc2ljIHByb2JsZW0gaXMgdGhhdCBfX2dl bmVyaWNfZmlsZV9haW9fd3JpdGUgd2lsbCBhbHdheXMNCj4+PnJldHVybiAwIGFmdGVyIGRldmlj ZSByZW1vdmFsLCB5ZXM/ICBJJ20gbm90IHN1cmUgd2h5IHRoYXQncyBhIHJlYWwNCj4+Pmlzc3Vl LCBjYW4geW91IGV4cGxhaW4gZXhhY3RseSB3aHkgeW91J3JlIHRyeWluZyB0byBjaGFuZ2UgdGhp cz8NCj4+Pg0KPj4gQXQgcHJlbnNldCwgdGhlIF9fZ2VuZXJpY19maWxlX2Fpb193cml0ZSBkb24n dCByZXR1cm4gemVybyByYXRoZXIgdGhhdCB0aGUgd2FudGVkIHNpemUuDQo+PiBTbyB0aGUgdXNl ciBjYW4ndCBrbm93IHRoZSBkaXNrIHJlbW92ZWQuIA0KPj4gRm9yIGV4YW1wbGU6DQo+PiBkZCBp Zj0vZGV2L3plcm8gb2Y9dXNiLWRpc2sgYnM9NjRrDQo+PiBXaGVuIHJlbW92ZWQgdXNiLWRpc2ss IGRkIHN0b3BlZCB1bnRpbCByZWFjaGVkIHRoZSBlbmRvZiB1c2ItZGlzay4NCj4NCj5BaCwgcmln aHQsIGl0J3MganVzdCB3cml0aW5nIHRvIHRoZSBwYWdlIGNhY2hlLiAgSSB0aGluayB0aGUgb25s eSByZWFzb24NCj55b3UgZ2V0IG1vcmUgdGltZWx5IGVycm9ycyB3aGVuIGRvaW5nIHRoZSBzYW1l IHRoaW5nIHRvIGEgZmlsZSBvbiBhIGZpbGUNCj5zeXN0ZW0gaXMgdGhhdCB0aGVyZSBpcyBzb21l IHN5bmNocm9ub3VzIG1ldGFkYXRhIG9yIGpvdXJuYWwgSS9PIHRoYXQNCj53aWxsIGdldCBFSU8g YW5kIHJlc3VsdCBpbiB0aGUgZmlsZSBzeXN0ZW0gYmVpbmcgc2V0IHJlYWQtb25seS4NCj4NCj5U aGUgYmlnZ2VyIHF1ZXN0aW9uIGlzIHdoZXRoZXIgd2Ugd2FudCB0byBjaGFuZ2UgdGhpcyBsb25n LXN0YW5kaW5nDQo+YmVoYXZpb3VyIG9mIGhvdyBvdXIgd3JpdGUtYmFjayBjYWNoZSB3b3Jrcy4g IEkgZG9uJ3Qga25vdyB0aGF0IGl0J3MNCj5yZWFsbHkgd29ydGggaXQsIGhvbmVzdGx5LiAgSWYg eW91IHdhbnQgdG8gZW5zdXJlIGRhdGEgaXMgb24gZGlzaywgeW91DQo+b3BlbiB0aGUgZmlsZSBP X1NZTkMgb3IgeW91IGlzc3VlIGFuIGZzeW5jLCBhbmQgdGhvc2UgY2FsbHMgd2lsbCByZXR1cm4N Cj5hbiBlcnJvciBmb3IgYSByZW1vdmVkIGJsb2NrIGRldmljZS4gIFNvLCBJIGd1ZXNzIEknbGwg YXNrIHRoZSBzYW1lDQo+cXVlc3Rpb24gYWdhaW46IHdoeSBhcmUgeW91IGxvb2tpbmcgYXQgdGhp cz8gIElzIHRoZXJlIHNvbWUgYXBwbGljYXRpb24NCj55b3UgY2FyZSBhYm91dCB0aGF0IGRvZXMg YnVmZmVyZWQgSS9PIHRvIHRoZSBibG9jayBkZXZpY2UgYW5kIG5ldmVyIGRvZXMNCj5hbiBmc3lu Yz8NCj4NCj4+IFVzaW5nIHRoaXMgcGF0Y2gsIGFmdGVyIHJlbW92ZWQgZGlzaywgdGhlIGFpby13 cml0ZSB3aWxsIHJldHVybiB6ZXJvLkkNCj4+IHRoaW5rIHRoZSB1cHBlciB1c2VyIHdpbGwgY2hl Y2suICAob3IgaWYgdGhlIHNpemUgb2YgYmxvY2sgaXMgemVybywgd2UNCj4+IHJldHVybiAtRU5P U1BDKS4NCj4+DQo+Pj5BcyBmb3IgeW91ciBwYXRjaGVzLCBJIGRvbid0IHRoaW5rIHRoYXQgcHV0 dGluZyB0aGUgaV9zaXplX3dyaXRlIGludG8NCj4+PmludmFsaWRhdGVfcGFydGl0aW9ucyBpcyBh IGdvb2QgaWRlYS4gIENvbnNpZGVyIHRoZSBjYXNlIG9mIHJlc2Nhbm5pbmcNCj4+PnBhcnRpdGlv bnM6IHlvdSB3aWxsIGFsd2F5cyBkZXRlY3QgYSBzaXplIGNoYW5nZSBub3csIHdoaWNoIGlzIG5v dCBnb29kLg0KPj4+DQo+PiBZZXMuQnV0IGluIGZ1bmMgcmVzY2FuX3BhcnRpdGlvbnMsIGFmdGVy IGludmFsaWRhdGVfcGFydGl0aW9ucyBpdCB3aWxsDQo+PiBjYWxsIGNoZWNrX2Rpc2tfc2l6ZV9j aGFuZ2UgdG8gc2V0IHNpemUgb2YgYmxvY2tfZGV2aWNlLg0KPg0KPlRoZSBwcm9ibGVtIHdpdGgg ZG9pbmcgYW4gaV9zaXplX3dyaXRlIG9mIDAgaW5zaWRlIG9mDQo+aW52YWxpZGF0ZV9wYXJ0aXRp b25zIGlzIHRoYXQgaXQgaXNuJ3QganVzdCBjYWxsZWQgZm9yIHRoZSBjYXNlIHdoZXJlIGENCj5k ZXZpY2UgaXMgcmVtb3ZlZC4gIEEgdXNlciBjYW4gaW5pdGlhdGUgYSByZXNjYW4gb2YgcGFydGl0 aW9ucy4gIEluIHN1Y2gNCj5hIGNhc2UsIHdlIGRvbid0IHdhbnQgdG8gZXZpY3QgYWxsIG9mIHRo ZSBjYWNoZWQgZGF0YSBmb3IgdW5jaGFuZ2VkDQo+cGFydGl0aW9ucy4NCj4NCj5UaGUgY2FsbCBj aGFpbiBpcyBsaWtlIHRoaXM6DQo+DQo+YmxrZGV2X2lvY3RsDQo+YmxrZGV2X3JlcmVhZF9wYXJ0 DQo+cmVzY2FuX3BhcnRpdGlvbnMNCj5jaGVja19kaXNrX3NpemVfY2hhbmdlDQo+DQo+Tm93IGxv b2sgYW5kIHNlZSB3aGF0IGNoZWNrX2Rpc2tfc2l6ZV9jaGFuZ2Ugd2lsbCBkbyB3aGVuIGl0IGZp bmRzIG91dA0KPnRoYXQgdGhlIHNpemUgaGFzIGNoYW5nZWQ6DQo+DQo+dm9pZCBjaGVja19kaXNr X3NpemVfY2hhbmdlKHN0cnVjdCBnZW5kaXNrICpkaXNrLCBzdHJ1Y3QgYmxvY2tfZGV2aWNlDQo+ KmJkZXYpDQo+ew0KPiAgICAgICAgbG9mZl90IGRpc2tfc2l6ZSwgYmRldl9zaXplOw0KPg0KPiAg ICAgICAgZGlza19zaXplID0gKGxvZmZfdClnZXRfY2FwYWNpdHkoZGlzaykgPDwgOTsNCj4gICAg ICAgIGJkZXZfc2l6ZSA9IGlfc2l6ZV9yZWFkKGJkZXYtPmJkX2lub2RlKTsNCj4gICAgICAgIGlm IChkaXNrX3NpemUgIT0gYmRldl9zaXplKSB7DQo+ICAgICAgICAgICAgICAgIGNoYXIgbmFtZVtC REVWTkFNRV9TSVpFXTsNCj4NCj4gICAgICAgICAgICAgICAgZGlza19uYW1lKGRpc2ssIDAsIG5h bWUpOw0KPiAgICAgICAgICAgICAgICBwcmludGsoS0VSTl9JTkZPDQo+ICAgICAgICAgICAgICAg ICAgICAgICAiJXM6IGRldGVjdGVkIGNhcGFjaXR5IGNoYW5nZSBmcm9tICVsbGQgdG8NCj4gICAg ICAgICAgICAgICAgJWxsZFxuIiwNCj4gICAgICAgICAgICAgICAgICAgICAgIG5hbWUsIGJkZXZf c2l6ZSwgZGlza19zaXplKTsNCj4gICAgICAgICAgICAgICAgaV9zaXplX3dyaXRlKGJkZXYtPmJk X2lub2RlLCBkaXNrX3NpemUpOw0KPiAgICAgICAgICAgICAgICBmbHVzaF9kaXNrKGJkZXYsIGZh bHNlKTsgIDw9PT09PT09PT09PT09DQo+ICAgICAgICB9DQo+fQ0KPg0KPlRoYXQgd2lsbCBpbnZh bGlkYXRlIGFsbCBvZiB0aGUgbWV0YWRhdGEgZm9yIGFueSBtb3VudGVkIGZpbGUgc3lzdGVtcyBv bg0KPnRoZSBkZXZpY2UuICBBbHNvLCB5b3UnbGwgZ2V0IGEgYmlnIG5hc3R5IHdhcm5pbmcgaWYg YW55IGZpbGVzIGFyZSBkaXJ0eToNCj4NCj4gICAgICAgICAgICAgICAgcHJpbnRrKEtFUk5fV0FS TklORyAiVkZTOiBidXN5IGlub2RlcyBvbiBjaGFuZ2VkIG1lZGlhIG9yICINCj4gICAgICAgICAg ICAgICAgICAgICAgICJyZXNpemVkIGRpc2sgJXNcbiIsIG5hbWUpOw0KPg0KPkFuZCB0aGUgcmVh bGl0eSBpcyB0aGF0IHdlIGhhdmVuJ3QgY2hhbmdlZCBhbnl0aGluZywgc28gdGhlcmUncyBubyBu ZWVkDQo+Zm9yIHRoaXMuDQpZZXMuIEhvdyBhYm91dCB0aG9zZSBjb2RlOg0KDQpkaWZmIC0tZ2l0 IGEvYmxvY2svZ2VuaGQuYyBiL2Jsb2NrL2dlbmhkLmMNCmluZGV4IDc5MWY0MTkuLmMyNzliMzQg MTAwNjQ0DQotLS0gYS9ibG9jay9nZW5oZC5jDQorKysgYi9ibG9jay9nZW5oZC5jDQpAQCAtNjM0 LDYgKzYzNCw3IEBAIHZvaWQgZGVsX2dlbmRpc2soc3RydWN0IGdlbmRpc2sgKmRpc2spDQogew0K ICAgICAgICBzdHJ1Y3QgZGlza19wYXJ0X2l0ZXIgcGl0ZXI7DQogICAgICAgIHN0cnVjdCBoZF9z dHJ1Y3QgKnBhcnQ7DQorICAgICAgIHN0cnVjdCBibG9ja19kZXZpY2UgKmJkZXY7DQogDQogICAg ICAgIGRpc2tfZGVsX2V2ZW50cyhkaXNrKTsNCiANCkBAIC02NDIsMTIgKzY0MywyNSBAQCB2b2lk IGRlbF9nZW5kaXNrKHN0cnVjdCBnZW5kaXNrICpkaXNrKQ0KICAgICAgICAgICAgICAgICAgICAg ICAgICAgICBESVNLX1BJVEVSX0lOQ0xfRU1QVFkgfCBESVNLX1BJVEVSX1JFVkVSU0UpOw0KICAg ICAgICB3aGlsZSAoKHBhcnQgPSBkaXNrX3BhcnRfaXRlcl9uZXh0KCZwaXRlcikpKSB7DQogICAg ICAgICAgICAgICAgaW52YWxpZGF0ZV9wYXJ0aXRpb24oZGlzaywgcGFydC0+cGFydG5vKTsNCisg ICAgICAgICAgICAgICBiZGV2ID0gYmRnZXRfZGlzayhkaXNrLCBwYXJ0LT5wYXJ0bm8pOw0KKyAg ICAgICAgICAgICAgIGlmIChiZGV2KSB7DQorICAgICAgICAgICAgICAgICAgICAgICBpX3NpemVf d3JpdGUoYmRldi0+YmRfaW5vZGUsIDApOw0KKyAgICAgICAgICAgICAgICAgICAgICAgYmRwdXQo YmRldik7DQorICAgICAgICAgICAgICAgfQ0KKw0KICAgICAgICAgICAgICAgIGRlbGV0ZV9wYXJ0 aXRpb24oZGlzaywgcGFydC0+cGFydG5vKTsNCiAgICAgICAgfQ0KICAgICAgICBkaXNrX3BhcnRf aXRlcl9leGl0KCZwaXRlcik7DQogDQogICAgICAgIGludmFsaWRhdGVfcGFydGl0aW9uKGRpc2ss IDApOw0KICAgICAgICBzZXRfY2FwYWNpdHkoZGlzaywgMCk7DQorDQorICAgICAgIGJkZXYgPSBi ZGdldF9kaXNrKGRpc2ssIDApOw0KKyAgICAgICBpZiAoYmRldikgew0KKyAgICAgICAgICAgICAg IGlfc2l6ZV93cml0ZShiZGV2LT5iZF9pbm9kZSwgMCk7DQorICAgICAgICAgICAgICAgYmRwdXQo YmRldik7DQorICAgICAgIH0NCisNCiAgICAgICAgZGlzay0+ZmxhZ3MgJj0gfkdFTkhEX0ZMX1VQ Ow0KIA0KICAgICAgICBzeXNmc19yZW1vdmVfbGluaygmZGlza190b19kZXYoZGlzayktPmtvYmos ICJiZGkiKTsNCg0KV2Ugb25seSBzZXQgaW5vZGUgdG8gemVybyBpbiBkZWxfZ2VuZGlzay4NCj4N Cj5BZnRlciBsb29raW5nIGF0IHRoZSBjb2RlIGZ1cnRoZXIsIHdoeSBkbyB5b3UgZXZlbiBuZWVk IHRvIGFkZCB0aGUNCj5zZWNvbmQgcGF0Y2g/ICBnZW5lcmljX3dyaXRlX2NoZWNrcyB3aWxsIGNo ZWNrIGZvciBhIHdyaXRlIHBhc3QgdGhlIGVuZA0KPm9mIHRoZSBibG9jayBkZXZpY2UuDQo+DQpZ ZXMsIGluIGdlbmVyaWNfd3JpdGVfY2hlY2tzIGl0IHdpbGwgY2hlY2sgc2l6ZSBzbyBwYXRjaDIg ZG9uJ3QgbmVlZC4NCg0KVGhhbmtzIQ0KSmlhbnBlbmcgTWENCg== From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1754129Ab3I2IqP (ORCPT ); Sun, 29 Sep 2013 04:46:15 -0400 Received: from mail-pa0-f53.google.com ([209.85.220.53]:42388 "EHLO mail-pa0-f53.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1753483Ab3I2IqL (ORCPT ); Sun, 29 Sep 2013 04:46:11 -0400 Date: Sun, 29 Sep 2013 16:46:07 +0800 From: majianpeng To: "Jeff Moyer" Cc: axboe , viro , LKML , linux-fsdevel Reply-To: majianpeng Subject: Re: Re: [PATCH V2 0/2] Auto stop async-write on block device when device removed. References: <201309171121559232246@gmail.com> <201309241107330800706@gmail.com>, X-Priority: 3 X-GUID: FFE4548D-12B6-46EB-B0AA-F6AC7C029A2E X-Has-Attach: no X-Mailer: Foxmail 7.0.1.93[cn] Mime-Version: 1.0 Message-ID: <201309291646039330492@gmail.com> Content-Type: text/plain; charset="gb2312" Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Transfer-Encoding: 8bit X-MIME-Autoconverted: from base64 to 8bit by mail.home.local id r8T8kaEc001435 >majianpeng writes: > >>>majianpeng 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+{G{ayʇڙ,jfhz_(階ݢj"mG?&~iOzv^m ?I