On Thu, Sep 01, 2016 at 08:32:00AM +0800, Qu Wenruo wrote: > > > At 09/01/2016 07:43 AM, Liu Bo wrote: > > Qgroup function may overwrite the saved error 'err' with 0 > > in case quota is not enabled, and this ends up with a > > endless loop in balance because we keep going back to balance > > the same block group. > > In which case? > > If join_trans() fails, we won't go through qgroup fix. > And before join_trans(), they all go to out_free/out tag. > Nothing to do with qgroup fix.
I don't think so. It doesn't always go to out_free, in the while() loop, it'd break the loop on any error and record it in @err, then go through prepare_to_merge() + merge_reloc_roots() and other stuff. Here's an example for keeping err after the above while loop(), ... err = prepare_to_merge(rc, err); ... Thanks, -liubo > > And if we hit qgroup_fix_relocated_data_extents(), then 'err' is alreayd 0, > nothing we need to save. > > And even for quota disabled case, qgroup_fix_relocated_data_extents() will > just return 0. Nothing wrong at all. > > Or did I miss something? > > Thanks, > Qu > > > > It really should use 'ret' instead. > > > > Signed-off-by: Liu Bo <[email protected]> > > --- > > fs/btrfs/relocation.c | 8 +++++--- > > 1 file changed, 5 insertions(+), 3 deletions(-) > > > > diff --git a/fs/btrfs/relocation.c b/fs/btrfs/relocation.c > > index 8a2c2a0..c0c13dc 100644 > > --- a/fs/btrfs/relocation.c > > +++ b/fs/btrfs/relocation.c > > @@ -4200,9 +4200,11 @@ restart: > > err = PTR_ERR(trans); > > goto out_free; > > } > > - err = qgroup_fix_relocated_data_extents(trans, rc); > > - if (err < 0) { > > - btrfs_abort_transaction(trans, err); > > + ret = qgroup_fix_relocated_data_extents(trans, rc); > > + if (ret < 0) { > > + btrfs_abort_transaction(trans, ret); > > + if (!err) > > + err = ret; > > goto out_free; > > } > > btrfs_commit_transaction(trans, rc->extent_root); > > > > -- To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html
