| Message ID | 1468257695-22445-1-git-send-email-bo.li.liu@oracle.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
On 07/11/2016 01:21 PM, Liu Bo wrote: > Mounting a btrfs can resume previous balance operations asynchronously. > An user got a crash when one drive has some corrupt sectors. > > Since balance can cancel itself in case of any error, we can gracefully > return errors to upper layers and let balance do the cancel job. > > Reported-by: sash <master.b.at.raven@chefmail.de> > Signed-off-by: Liu Bo <bo.li.liu@oracle.com> v2 Looks good, thanks Liu. Signed-off-by: Chris Mason <clm@fb.com> -- To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Mon, Jul 11, 2016 at 6:21 PM, Liu Bo <bo.li.liu@oracle.com> wrote: > Mounting a btrfs can resume previous balance operations asynchronously. > An user got a crash when one drive has some corrupt sectors. > > Since balance can cancel itself in case of any error, we can gracefully > return errors to upper layers and let balance do the cancel job. > > Reported-by: sash <master.b.at.raven@chefmail.de> > Signed-off-by: Liu Bo <bo.li.liu@oracle.com> > --- > v2: - Initialize path with NULL. > - Show more information when we bail out. > > fs/btrfs/volumes.c | 30 ++++++++++++++++++++++++++---- > 1 file changed, 26 insertions(+), 4 deletions(-) > > diff --git a/fs/btrfs/volumes.c b/fs/btrfs/volumes.c > index 589f128..348a183 100644 > --- a/fs/btrfs/volumes.c > +++ b/fs/btrfs/volumes.c > @@ -3421,7 +3421,7 @@ static int __btrfs_balance(struct btrfs_fs_info *fs_info) > u64 size_to_free; > u64 chunk_type; > struct btrfs_chunk *chunk; > - struct btrfs_path *path; > + struct btrfs_path *path = NULL; > struct btrfs_key key; > struct btrfs_key found_key; > struct btrfs_trans_handle *trans; > @@ -3455,13 +3455,35 @@ static int __btrfs_balance(struct btrfs_fs_info *fs_info) > ret = btrfs_shrink_device(device, old_size - size_to_free); > if (ret == -ENOSPC) > break; > - BUG_ON(ret); > + if (ret) { > + /* btrfs_shrink_device never returns ret > 0 */ > + WARN_ON(ret > 0); > + goto error; > + } > > trans = btrfs_start_transaction(dev_root, 0); > - BUG_ON(IS_ERR(trans)); > + if (IS_ERR(trans)) { > + ret = PTR_ERR(trans); > + btrfs_info(fs_info, > + "%s:%d fails on btrfs_start_transaction() right after shrinking devivce %s (original size is %llu new size is %llu", devivce -> device > + __func__, __LINE__, > + rcu_str_deref(device->name), old_size, > + old_size - size_to_free); > + goto error; > + } > > ret = btrfs_grow_device(trans, device, old_size); > - BUG_ON(ret); > + if (ret) { > + btrfs_end_transaction(trans, dev_root); > + /* btrfs_grow_device never returns ret > 0 */ > + WARN_ON(ret > 0); > + btrfs_info(fs_info, > + "%s:%d fails on btrfs_grow_device() right after shrinking devivce %s (original size is %llu new size is %llu", devivce -> device > + __func__, __LINE__, > + rcu_str_deref(device->name), old_size, > + old_size - size_to_free); > + goto error; > + } > > btrfs_end_transaction(trans, dev_root); > } > -- > 2.5.5 > > -- > To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in > the body of a message to majordomo@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html
On Mon, Jul 11, 2016 at 10:21:35AM -0700, Liu Bo wrote: > + } > > trans = btrfs_start_transaction(dev_root, 0); > - BUG_ON(IS_ERR(trans)); > + if (IS_ERR(trans)) { > + ret = PTR_ERR(trans); > + btrfs_info(fs_info, This could be btrfs_info_in_rcu for clarity (using the rcu_string) > + "%s:%d fails on btrfs_start_transaction() right after shrinking devivce %s (original size is %llu new size is %llu", > + __func__, __LINE__, I'm not sure the function and line is necessary, we don't use it anywhere else. I'd suggest a slight modification: "resize: unable to start transaction after shrinking device %s (error %d), old size %llu, new size %llu" > + rcu_str_deref(device->name), old_size, > + old_size - size_to_free); > + goto error; > + } > > ret = btrfs_grow_device(trans, device, old_size); > - BUG_ON(ret); > + if (ret) { > + btrfs_end_transaction(trans, dev_root); > + /* btrfs_grow_device never returns ret > 0 */ > + WARN_ON(ret > 0); > + btrfs_info(fs_info, > + "%s:%d fails on btrfs_grow_device() right after shrinking devivce %s (original size is %llu new size is %llu", "resize: unable to grow device after shrinking device %s (error %d), old size %llu, new size %llu" -- To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/fs/btrfs/volumes.c b/fs/btrfs/volumes.c index 589f128..348a183 100644 --- a/fs/btrfs/volumes.c +++ b/fs/btrfs/volumes.c @@ -3421,7 +3421,7 @@ static int __btrfs_balance(struct btrfs_fs_info *fs_info) u64 size_to_free; u64 chunk_type; struct btrfs_chunk *chunk; - struct btrfs_path *path; + struct btrfs_path *path = NULL; struct btrfs_key key; struct btrfs_key found_key; struct btrfs_trans_handle *trans; @@ -3455,13 +3455,35 @@ static int __btrfs_balance(struct btrfs_fs_info *fs_info) ret = btrfs_shrink_device(device, old_size - size_to_free); if (ret == -ENOSPC) break; - BUG_ON(ret); + if (ret) { + /* btrfs_shrink_device never returns ret > 0 */ + WARN_ON(ret > 0); + goto error; + } trans = btrfs_start_transaction(dev_root, 0); - BUG_ON(IS_ERR(trans)); + if (IS_ERR(trans)) { + ret = PTR_ERR(trans); + btrfs_info(fs_info, + "%s:%d fails on btrfs_start_transaction() right after shrinking devivce %s (original size is %llu new size is %llu", + __func__, __LINE__, + rcu_str_deref(device->name), old_size, + old_size - size_to_free); + goto error; + } ret = btrfs_grow_device(trans, device, old_size); - BUG_ON(ret); + if (ret) { + btrfs_end_transaction(trans, dev_root); + /* btrfs_grow_device never returns ret > 0 */ + WARN_ON(ret > 0); + btrfs_info(fs_info, + "%s:%d fails on btrfs_grow_device() right after shrinking devivce %s (original size is %llu new size is %llu", + __func__, __LINE__, + rcu_str_deref(device->name), old_size, + old_size - size_to_free); + goto error; + } btrfs_end_transaction(trans, dev_root); }
Mounting a btrfs can resume previous balance operations asynchronously. An user got a crash when one drive has some corrupt sectors. Since balance can cancel itself in case of any error, we can gracefully return errors to upper layers and let balance do the cancel job. Reported-by: sash <master.b.at.raven@chefmail.de> Signed-off-by: Liu Bo <bo.li.liu@oracle.com> --- v2: - Initialize path with NULL. - Show more information when we bail out. fs/btrfs/volumes.c | 30 ++++++++++++++++++++++++++---- 1 file changed, 26 insertions(+), 4 deletions(-)