Skip to contentSkip to conversation

xfsrestore: lift directory entry count limitations

PATCH Superseded [email protected] 2 participants 7 messages Oct 12, 2010 – Oct 15, 2010

v1 of 3 Superseded Latest is v3 →

lore
Share & tools
lore
#1 lore raw

PATCH 1/2 xfsrestore: turn off NODECHK

The NODECHK macro should only be enabled as needed for development/debugging. Having it on limits xfsrestore to 268 million directory entries instead of 4 billion.

restore/Makefile +1 -1
Index: xfsdump-kernel.org/restore/Makefile
===================================================================
@@ -103,7 +103,7 @@ LLDLIBS = $(LIBUUID) $(LIBHANDLE) $(LIBA
 LTDEPENDENCIES = $(LIBRMT)
 
 LCFLAGS = -DRESTORE -DRMT -DBASED -DDOSOCKS -DINVCONVFIX -DPIPEINVFIX \
-	-DEOMFIX -DSESSCPLT -DWHITEPARSE -DNODECHK -DDIRENTHDR_CHECKSUM \
+	-DEOMFIX -DSESSCPLT -DWHITEPARSE -DDIRENTHDR_CHECKSUM \
 	-DF_FSSETDM
 
 default: depend $(LTCOMMAND)

_______________________________________________
xfs mailing list
[email protected]
http://oss.sgi.com/mailman/listinfo/xfs

Reviewed-by on v1 1/2
#2 lore raw

Re: [PATCH 1/2] xfsrestore: turn off NODECHK

Quoted 4 lines · On Tue, 2010-10-12 at 16:53 -0500, [email protected] wrote:

plain text document attachment (no_node_check) The NODECHK macro should only be enabled as needed for development/debugging. Having it on limits xfsrestore to 268 million directory entries instead of 4 billion.

I was going to suggest you leave it in the Makefile in commented-out form, perhaps with an explanation that it's available for development. But since NODECHK is only ever referenced inside "restore/node.c", maybe such a thing could be done inside that file instead.

I.e., something like:

/*
* NODECHK can be enabled for development and debugging
* in order to enforce additional checks on handles
* when nodes are mapped, unmapped, and freed. It should
* not be used for production code, it cuts into the number
* of directory entries xfsrestore can handle.
*/
/* #define NODECHK */

I'll leave it up to you whether you think that's worth doing (and if so, let me know). Either way:

Quoted 16 lines

Index: xfsdump-kernel.org/restore/Makefile
===================================================================

restore/Makefile +1 -1
@@ -103,7 +103,7 @@ LLDLIBS = $(LIBUUID) $(LIBHANDLE) $(LIBA
 LTDEPENDENCIES = $(LIBRMT)
 
 LCFLAGS = -DRESTORE -DRMT -DBASED -DDOSOCKS -DINVCONVFIX -DPIPEINVFIX \
-	-DEOMFIX -DSESSCPLT -DWHITEPARSE -DNODECHK -DDIRENTHDR_CHECKSUM \
+	-DEOMFIX -DSESSCPLT -DWHITEPARSE -DDIRENTHDR_CHECKSUM \
 	-DF_FSSETDM
 
 default: depend $(LTCOMMAND)
Quoted 4 lines

_______________________________________________
xfs mailing list
[email protected]
http://oss.sgi.com/mailman/listinfo/xfs

_______________________________________________
xfs mailing list
[email protected]
http://oss.sgi.com/mailman/listinfo/xfs

#3 lore raw

Re: [PATCH 1/2] xfsrestore: turn off NODECHK

Quoted 23 lines · Alex Elder
Quoted 4 lines · On Tue, 2010-10-12 at 16:53 -0500, [email protected] wrote:

plain text document attachment (no_node_check) The NODECHK macro should only be enabled as needed for development/debugging. Having it on limits xfsrestore to 268 million directory entries instead of 4 billion.

I was going to suggest you leave it in the Makefile in commented-out form, perhaps with an explanation that it's available for development. But since NODECHK is only ever referenced inside "restore/node.c", maybe such a thing could be done inside that file instead.

I.e., something like:

/*
* NODECHK can be enabled for development and debugging
* in order to enforce additional checks on handles
* when nodes are mapped, unmapped, and freed. It should
* not be used for production code, it cuts into the number
* of directory entries xfsrestore can handle.
*/
/* #define NODECHK */

Quoted 4 lines

I'll leave it up to you whether you think that's worth doing (and if so, let me know). Either way:

Sounds like a good idea. I'll add something like this and repost.

Bill

Quoted 15 lines
Quoted 15 lines

Index: xfsdump-kernel.org/restore/Makefile
===================================================================

restore/Makefile +1 -1
@@ -103,7 +103,7 @@ LLDLIBS = $(LIBUUID) $(LIBHANDLE) $(LIBA
  LTDEPENDENCIES = $(LIBRMT)
 
  LCFLAGS = -DRESTORE -DRMT -DBASED -DDOSOCKS -DINVCONVFIX -DPIPEINVFIX \
-	-DEOMFIX -DSESSCPLT -DWHITEPARSE -DNODECHK -DDIRENTHDR_CHECKSUM \
+	-DEOMFIX -DSESSCPLT -DWHITEPARSE -DDIRENTHDR_CHECKSUM \
  	-DF_FSSETDM
 
Quoted 8 lines
Quoted 6 lines
restore/Makefile +0 -0
  default: depend $(LTCOMMAND)

_______________________________________________
xfs mailing list
[email protected]
http://oss.sgi.com/mailman/listinfo/xfs

_______________________________________________
xfs mailing list
[email protected]
http://oss.sgi.com/mailman/listinfo/xfs

#4 lore raw

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

In reply to [email protected]

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.

restore/namreg.h +2 -2
Index: 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
restore/namreg.c +7 -13
Index: 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 */
 
restore/tree.c +14 -15
Index: 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
[email protected]
http://oss.sgi.com/mailman/listinfo/xfs

Reviewed-by on v1 2/2
#5 lore raw

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

Quoted 5 lines · On Tue, 2010-10-12 at 16:53 -0500, [email protected] 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:

Quoted 105 lines

Index: xfsdump-kernel.org/restore/namreg.h
===================================================================

restore/namreg.h +2 -2
@@ -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
===================================================================

restore/namreg.c +7 -13
@@ -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
===================================================================

restore/tree.c +14 -15
@@ -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;
Quoted 4 lines

_______________________________________________
xfs mailing list
[email protected]
http://oss.sgi.com/mailman/listinfo/xfs

_______________________________________________
xfs mailing list
[email protected]
http://oss.sgi.com/mailman/listinfo/xfs

#6 lore raw

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

Quoted 11 lines · Alex Elder
Quoted 5 lines · On Tue, 2010-10-12 at 16:53 -0500, [email protected] 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.

Quoted 5 lines

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.

The nodes are stored on disk in the event of a cumulative restore (level 0 + level 1 + ...) or when resuming a previously interrupted xfsrestore session. So persistent, but temporary.

There is no versioning of any of the data structures that xfsrestore stores on disk. It's assumed that you're running on a system with the same endianness, same page size and same xfsrestore build.

At a minimum this should be documented in the man page. I'll also look at what can be done about detecting this at runtime.

Bill

Quoted 111 lines

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

Quoted 105 lines

Index: xfsdump-kernel.org/restore/namreg.h
===================================================================

restore/namreg.h +2 -2
@@ -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
===================================================================

restore/namreg.c +7 -13
@@ -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
===================================================================

restore/tree.c +14 -15
@@ -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;
Quoted 6 lines
Quoted 4 lines

_______________________________________________
xfs mailing list
[email protected]
http://oss.sgi.com/mailman/listinfo/xfs

_______________________________________________
xfs mailing list
[email protected]
http://oss.sgi.com/mailman/listinfo/xfs