xfs
[Top] [All Lists]

Re: [PATCH 2/2] xfsrestore: change nrh_t to be 64 bits instead of 32

To: wkendall@xxxxxxx
Subject: Re: [PATCH 2/2] xfsrestore: change nrh_t to be 64 bits instead of 32
From: Alex Elder <aelder@xxxxxxx>
Date: Thu, 14 Oct 2010 13:45:12 -0500
Cc: xfs@xxxxxxxxxxx
In-reply-to: <20101012215400.639300973@xxxxxxx>
References: <20101012215322.749700656@xxxxxxx> <20101012215400.639300973@xxxxxxx>
Reply-to: aelder@xxxxxxx
On Tue, 2010-10-12 at 16:53 -0500, wkendall@xxxxxxx wrote:
> plain text document attachment (namreg_64bit)
> An nrh_t refers to a byte offset in a file containing all the pathname
> components from a dump. At an average filename length of 20
> characters, an nrh_t would overflow on a dump containing ~214 million
> directory entries.

This is a good change.  The value of an nrh_t is (opaquely)
assigned as a 64-bit file offset, so it makes sense it should
also be 64 bits.  Much better way of computing HDLMAX also.

Do you know why NODESZ (= 48) is defined as a constant, and
used separate from sizeof (struct node)?  Is the format of
a node structure recorded persistently somewhere?  I don't
know this code well enough to recognize whether the change
to the node_t definition affects only in-memory structures.

Provided there are no ill effects due to changing the
node struct to hold the 64-bit name registry handle:

Reviewed-by: Alex Elder <aelder@xxxxxxx>

> 
> Signed-off-by: Bill Kendall <wkendall@xxxxxxx>
> 
> Index: xfsdump-kernel.org/restore/namreg.h
> ===================================================================
> --- xfsdump-kernel.org.orig/restore/namreg.h
> +++ xfsdump-kernel.org/restore/namreg.h
> @@ -26,8 +26,8 @@
>  
>  /* nrh_t - handle to a registered name
>   */
> -typedef size32_t nrh_t;
> -#define NRH_NULL     SIZE32MAX
> +typedef size64_t nrh_t;
> +#define NRH_NULL     SIZE64MAX
>  
> 
>  /* namreg_init - creates the name registry. resync is TRUE if the
> Index: xfsdump-kernel.org/restore/namreg.c
> ===================================================================
> --- xfsdump-kernel.org.orig/restore/namreg.c
> +++ xfsdump-kernel.org/restore/namreg.c
> @@ -72,26 +72,20 @@ typedef struct namreg_tran namreg_tran_t
>   */
>  #define CHKBITCNT            2
>  #define      CHKBITSHIFT             ( NBBY * sizeof( nrh_t ) - CHKBITCNT )
> -#define      CHKBITLOMASK            ( ( 1 << CHKBITCNT ) - 1 )
> +#define      CHKBITLOMASK            ( ( 1ULL << CHKBITCNT ) - 1 )
>  #define      CHKBITMASK              ( CHKBITLOMASK << CHKBITSHIFT )
>  #define CHKHDLCNT            CHKBITSHIFT
> -#define CHKHDLMASK           ( ( 1 << CHKHDLCNT ) - 1 )
> -#define CHKGETBIT( h )               ( ( h >> CHKBITSHIFT ) & CHKBITLOMASK )
> -#define CHKGETHDL( h )               ( h & CHKHDLMASK )
> -#define CHKMKHDL( c, h )     ( ( ( c << CHKBITSHIFT ) & CHKBITMASK ) \
> +#define CHKHDLMASK           ( ( 1ULL << CHKHDLCNT ) - 1 )
> +#define CHKGETBIT( h )               ( ( (h) >> CHKBITSHIFT ) & CHKBITLOMASK 
> )
> +#define CHKGETHDL( h )               ( (h) & CHKHDLMASK )
> +#define CHKMKHDL( c, h )     ( ( ( (c) << CHKBITSHIFT ) & CHKBITMASK )       
> \
>                                 |                                     \
> -                               ( h & CHKHDLMASK ))
> +                               ( (h) & CHKHDLMASK ))
>  #define HDLMAX                       ( ( off64_t )CHKHDLMASK )
>  
>  #else /* NAMREGCHK */
>  
> -#define HDLMAX                       ( ( ( off64_t )1                        
> \
> -                                 <<                                  \
> -                                 ( ( off64_t )NBBY                   \
> -                                   *                                 \
> -                                   ( off64_t )sizeof( nrh_t )))      \
> -                               -                                     \
> -                               ( off64_t )2 ) /* 2 to avoid NRH_NULL */
> +#define HDLMAX                       ( NRH_NULL - 1 )
>  
>  #endif /* NAMREGCHK */
>  
> Index: xfsdump-kernel.org/restore/tree.c
> ===================================================================
> --- xfsdump-kernel.org.orig/restore/tree.c
> +++ xfsdump-kernel.org/restore/tree.c
> @@ -166,19 +166,18 @@ typedef struct tran tran_t;
>  #define NODESZ       48
>  
>  struct node {
> -     xfs_ino_t n_ino;                /* 8  8 ino */
> -     nrh_t n_nrh;            /* 4 12 handle to name in name registry */
> -     dah_t n_dah;            /* 4 16 handle to directory attributes */
> -     nh_t n_hashh;           /* 4 20 hash array */
> -     nh_t n_parh;            /* 4 24 parent */
> -     nh_t n_sibh;            /* 4 28 sibling list */
> -     nh_t n_sibprevh;        /* 4 32 prev sibling list - dbl link list */
> -     nh_t n_cldh;            /* 4 36 children list */
> -     nh_t n_lnkh;            /* 4 40 hard link list */
> -     gen_t n_gen;            /* 2 42 generation count mod 0x10000 */
> -     u_char_t n_flags;       /* 1 43 action and state flags */
> -     u_char_t n_nodehkbyte;  /* 1 44 given to node abstraction */
> -     int32_t pad;            /* 4 48 padding to 8 byte boundary */
> +     xfs_ino_t n_ino;        /* 8  8 ino */
> +     nrh_t n_nrh;            /* 8 16 handle to name in name registry */
> +     dah_t n_dah;            /* 4 20 handle to directory attributes */
> +     nh_t n_hashh;           /* 4 24 hash array */
> +     nh_t n_parh;            /* 4 28 parent */
> +     nh_t n_sibh;            /* 4 32 sibling list */
> +     nh_t n_sibprevh;        /* 4 36 prev sibling list - dbl link list */
> +     nh_t n_cldh;            /* 4 40 children list */
> +     nh_t n_lnkh;            /* 4 44 hard link list */
> +     gen_t n_gen;            /* 2 46 generation count mod 0x10000 */
> +     u_char_t n_flags;       /* 1 47 action and state flags */
> +     u_char_t n_nodehkbyte;  /* 1 48 given to node abstraction */
>  };
>  
>  typedef struct node node_t;
> @@ -3393,9 +3392,9 @@ Node_free( nh_t *nhp )
>               namreg_del( np->n_nrh );
>               np->n_nrh = NRH_NULL;
>       }
> -     if ( np->n_dah != NRH_NULL ) {
> +     if ( np->n_dah != DAH_NULL ) {
>               dirattr_del( np->n_dah );
> -             np->n_dah = NRH_NULL;
> +             np->n_dah = DAH_NULL;
>       }
>       np->n_flags = 0;
>       np->n_parh = NH_NULL;
> 
> _______________________________________________
> xfs mailing list
> xfs@xxxxxxxxxxx
> http://oss.sgi.com/mailman/listinfo/xfs



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