2009/2/26 Josef Bacik <jo...@redhat.com>: > On Wed, Feb 25, 2009 at 02:13:10PM -0500, Chris Ball wrote: >> Hi, >> >> This patch (against experimental HEAD) attempts to make shrinking more >> robust, by only updating device size if we've succeeded in creating >> enough free space without any failures in btrfs_relocate_chunk(). >> >> Here's a log with my patch applied. The two things to note are that a >> near-limit shrink now works, and that a failed shrink (in this case, >> trying to shrink to less than the used space) no longer updates the >> device size erroneously: >> >> http://dev.laptop.org/~cjb/btrfs/shrink-log >> >> Please review carefully -- I'm still new to btrfs. The short version of >> the patch is: >> >> * create a success path, as a break out of the while(1) relocating >> (rather than going to the "done" label). >> * move the device size updating code into that path >> * leave "path->reada = 2;" behind in the entry path, since path is >> used by the searching operation rather than the later resize. >> >> Thanks! >> >> Signed-off-by: Chris Ball <c...@laptop.org> >> >> diff --git a/fs/btrfs/volumes.c b/fs/btrfs/volumes.c >> index 1316139..e2fa072 100644 >> --- a/fs/btrfs/volumes.c >> +++ b/fs/btrfs/volumes.c >> @@ -1815,30 +1815,8 @@ int btrfs_shrink_device(struct btrfs_device *device, >> u64 new_size) >> if (!path) >> return -ENOMEM; >> >> - trans = btrfs_start_transaction(root, 1); >> - if (!trans) { >> - ret = -ENOMEM; >> - goto done; >> - } >> - >> path->reada = 2; >> >> - lock_chunks(root); >> - >> - device->total_bytes = new_size; >> - if (device->writeable) >> - device->fs_devices->total_rw_bytes -= diff; > > So I think you still want to do this part, to keep the allocator from actually > allocating new space in the area we are trying to cull with the shrink, we > just > don't want to update the ondisk stuff just yet, so everything else can be > moved > to below the loop. > > So this >> - ret = btrfs_update_device(trans, device); >> - if (ret) { >> - unlock_chunks(root); >> - btrfs_end_transaction(trans, root); >> - goto done; >> - } >> - WARN_ON(diff > old_total); >> - btrfs_set_super_total_bytes(super_copy, old_total - diff); > > to here should all be moved below like you have it. Other than that it looks > good. Thanks, >
This isn't working. we don't call btrfs_update_device here, but it can be called in other places. I think we should add a new field in btrfs_device to reflect the on disk device size, and update it when shrinking succeeds. Regards Yan Zheng -- To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
