Hi Darrick, On Tue, Jun 02, 2020 at 05:22:22PM -0700, Darrick J. Wong wrote: > On Tue, Jun 02, 2020 at 10:52:38PM +0800, Gao Xiang wrote: > > Sometimes no need to play with perag_tree since for many > > cases perag can also be accessed by agbp reliably. > > > > Signed-off-by: Gao Xiang <hsiangkao@xxxxxxxxxx> > > --- > > Not sure addressing all the cases, but seems mostly. > > Kindly correct me if something wrong somewhere... > > > > fs/xfs/libxfs/xfs_ag.c | 4 ++-- > > fs/xfs/libxfs/xfs_alloc.c | 22 ++++++----------- > > fs/xfs/libxfs/xfs_alloc_btree.c | 10 ++++---- > > fs/xfs/libxfs/xfs_ialloc.c | 28 ++++++---------------- > > fs/xfs/libxfs/xfs_refcount_btree.c | 5 ++-- > > fs/xfs/libxfs/xfs_rmap_btree.c | 5 ++-- > > fs/xfs/xfs_inode.c | 38 +++++++++--------------------- > > 7 files changed, 35 insertions(+), 77 deletions(-) > > > > diff --git a/fs/xfs/libxfs/xfs_ag.c b/fs/xfs/libxfs/xfs_ag.c > > index 9d84007a5c65..8cf73fe4338e 100644 > > --- a/fs/xfs/libxfs/xfs_ag.c > > +++ b/fs/xfs/libxfs/xfs_ag.c > > @@ -563,7 +563,8 @@ xfs_ag_get_geometry( > > error = xfs_alloc_read_agf(mp, NULL, agno, 0, &agf_bp); > > if (error) > > goto out_agi; > > - pag = xfs_perag_get(mp, agno); > > + > > + pag = agi_bp->b_pag; > > > > /* Fill out form. */ > > memset(ageo, 0, sizeof(*ageo)); > > @@ -583,7 +584,6 @@ xfs_ag_get_geometry( > > xfs_ag_geom_health(pag, ageo); > > > > /* Release resources. */ > > - xfs_perag_put(pag); > > Looks like a reaosnable pattern throughout the codebase. Did fstests > cough up any new errors? Actually I add more extra ASSERTs (to check pag_agno vs agno) around all the cases (many of them aren't shown in this patch), and have been running fstests and fsstress with ASSERT version for a while. It seems no visible crash. But I'm not sure how many exist failures are (at least no panic yells out)... > > > xfs_buf_relse(agf_bp); ... > > - xfs_perag_put(pag); > > - return error; > > + /* Point the head of the list to the next unlinked inode. */ > > + return xfs_iunlink_update_bucket(tp, agno, agibp, bucket_index, > > + next_agino); > > Why not cut out the agno argument here too? Surely you could obtain it > from agibp->b_pag->pag_agno. Ditto for xfs_iunlink_map_prev. Okay, thanks for pointing out... I think there are many indirect potential cleanups indeed.... Will fix the above cases in the next version... Thanks, Gao Xiang > > --D > > > } > > > > /* > > -- > > 2.18.1 > > >
