On 9/1/16 9:27 PM, Dave Chinner wrote:
> From: Dave Chinner <dchinner@xxxxxxxxxx>
>
> To verify that the AGFL contents is sane, we need to have access to
> the indexes that tell us what part of the AGFL is active. We cannot
> access the AGF buffer from the AGFL verifier, so we need to shadow
> these values in the struct xfs_perag so we check them when required.
>
> Signed-off-by: Dave Chinner <dchinner@xxxxxxxxxx>
> ---
> fs/xfs/libxfs/xfs_alloc.c | 4 ++++
> fs/xfs/xfs_mount.h | 2 ++
> 2 files changed, 6 insertions(+)
>
> diff --git a/fs/xfs/libxfs/xfs_alloc.c b/fs/xfs/libxfs/xfs_alloc.c
> index 23559b9..1aef556 100644
> --- a/fs/xfs/libxfs/xfs_alloc.c
> +++ b/fs/xfs/libxfs/xfs_alloc.c
> @@ -2255,6 +2255,7 @@ xfs_alloc_get_freelist(
> be32_add_cpu(&agf->agf_flcount, -1);
> xfs_trans_agflist_delta(tp, -1);
> pag->pagf_flcount--;
> + pag->pagf_flfirst = be32_to_cpu(agf->agf_flfirst);
> xfs_perag_put(pag);
>
> logflags = XFS_AGF_FLFIRST | XFS_AGF_FLCOUNT;
Still reviewing, but this kind of jumped out at me, seems like the get/put
functions are a bit jumbled up:
/*
* Get the block number and update the data structures.
*/
agfl_bno = XFS_BUF_TO_AGFL_BNO(mp, agflbp);
bno = be32_to_cpu(agfl_bno[be32_to_cpu(agf->agf_flfirst)]);
be32_add_cpu(&agf->agf_flfirst, 1);
xfs_trans_brelse(tp, agflbp);
if (be32_to_cpu(agf->agf_flfirst) == XFS_AGFL_SIZE(mp))
agf->agf_flfirst = 0;
pag = xfs_perag_get(mp, be32_to_cpu(agf->agf_seqno));
be32_add_cpu(&agf->agf_flcount, -1);
xfs_trans_agflist_delta(tp, -1);
pag->pagf_flcount--;
xfs_perag_put(pag);
why is there a trans_brelse in between two lines which handle proper setting
of flfirst? I was looking at this thinking about where the pag structures
get updated, then saw the kind of swiss-cheese placement of the first/count
manipulation...
If pagf_flcount/agf_flcount, pagf_flfirst/agf_flfirst etc need to stay in
sync, should there be a wrapper to encapsulate it all?
like:
xfs_agf_{advance/drop/remove}_first(mp, agf, pagf)
{
be32_add_cpu(&agf->agf_flfirst, 1);
if (be32_to_cpu(agf->agf_flfirst) == XFS_AGFL_SIZE(mp))
agf->agf_flfirst = 0;
pag->pagf_flfirst = be32_to_cpu(agf->agf_flfirst);
be32_add_cpu(&agf->agf_flcount, -1);
pag->pagf_flcount--;
}
or something similar?
-Eric
|