xfs
[Top] [All Lists]

Re: [PATCH 3/9] xfs: add CRC checks to the AGF

To: Dave Chinner <david@xxxxxxxxxxxxx>
Subject: Re: [PATCH 3/9] xfs: add CRC checks to the AGF
From: Ben Myers <bpm@xxxxxxx>
Date: Thu, 21 Feb 2013 16:53:05 -0600
Cc: xfs@xxxxxxxxxxx
Delivered-to: xfs@xxxxxxxxxxx
In-reply-to: <1358774760-21841-4-git-send-email-david@xxxxxxxxxxxxx>
References: <1358774760-21841-1-git-send-email-david@xxxxxxxxxxxxx> <1358774760-21841-4-git-send-email-david@xxxxxxxxxxxxx>
User-agent: Mutt/1.5.20 (2009-06-14)
Dave,

Just some minor nits.

On Tue, Jan 22, 2013 at 12:25:54AM +1100, Dave Chinner wrote:
> From: Dave Chinner <dchinner@xxxxxxxxxx>
> 
> THe AGF already has some self identifying fields (e.g. the sequence
  The

> number) so we only need to add the uuid to it to identify the
> filesystem it belongs to. The location is fixed based on the
> sequence number, so there's no need to add a block number, either.
> 
> Hence the only additional fields are the CRC and LSN fields. These
> are unlogged, so place some space between the end of the logged
> fields and them so that future expansion of the AGF for legged
                                                          logged

> fields can be placed adjacent to the existing logged fields and
> hence not complicate the field-derived range based logging we
> currently have.
> 
> Based originally on a patch from myself, modified further by
> Christoph Hellwig and then modified again to fit into the
> verifier structure with additional fields by myself. The multiple
> signed-off-by tags indicate the age and history of this patch.
> 
> Signed-off-by: Dave Chinner <dgc@xxxxxxx>
> Signed-off-by: Christoph Hellwig <hch@xxxxxx>
> Signed-off-by: Dave Chinner <dchinner@xxxxxxxxxx>
> ---
>  fs/xfs/xfs_ag.h          |   21 +++++++++++-
>  fs/xfs/xfs_alloc.c       |   80 
> ++++++++++++++++++++++++++++++++--------------
>  fs/xfs/xfs_buf_item.h    |    4 ++-
>  fs/xfs/xfs_fsops.c       |    3 ++
>  fs/xfs/xfs_log_recover.c |    7 ++++
>  5 files changed, 89 insertions(+), 26 deletions(-)
> 
> diff --git a/fs/xfs/xfs_ag.h b/fs/xfs/xfs_ag.h
> index f2aeedb..b382703 100644
> --- a/fs/xfs/xfs_ag.h
> +++ b/fs/xfs/xfs_ag.h
> @@ -63,12 +63,29 @@ typedef struct xfs_agf {
>       __be32          agf_spare0;     /* spare field */
>       __be32          agf_levels[XFS_BTNUM_AGF];      /* btree levels */
>       __be32          agf_spare1;     /* spare field */
> +
>       __be32          agf_flfirst;    /* first freelist block's index */
>       __be32          agf_fllast;     /* last freelist block's index */
>       __be32          agf_flcount;    /* count of blocks in freelist */
>       __be32          agf_freeblks;   /* total free blocks */
> +
>       __be32          agf_longest;    /* longest free space */
>       __be32          agf_btreeblks;  /* # of blocks held in AGF btrees */
> +     uuid_t          agf_uuid;       /* uuid of filesystem */
> +
> +     /*
> +      * reserve some contiguous space for future logged fields before we add
> +      * the unlogged fields. This makes the range logging via flags and
> +      * structure offsets much simpler.
> +      */
> +     __be64          agf_spare64[16];
> +
> +     /* unlogged fields, written during buffer writeback. */
> +     __be64          agf_lsn;        /* last write sequence */
> +     __be32          agf_crc;        /* crc of agf sector */
> +     __be32          agf_spare2;
> +
> +     /* structure must be padded to 64 bit alignment */
>  } xfs_agf_t;
>  
>  #define      XFS_AGF_MAGICNUM        0x00000001
> @@ -83,6 +100,7 @@ typedef struct xfs_agf {
>  #define      XFS_AGF_FREEBLKS        0x00000200
>  #define      XFS_AGF_LONGEST         0x00000400
>  #define      XFS_AGF_BTREEBLKS       0x00000800
> +#define      XFS_AGF_UUID            0x00001000
>  #define      XFS_AGF_NUM_BITS        12
                                        13 ?
You added a 13th bit.

How do you envision having the uuid in these structures will be helpful?

Regards,
        Ben

<Prev in Thread] Current Thread [Next in Thread>