xfs_repair: multithread phase 2
PATCH 1/1 xfs_repair: multithread phase 2
From: Dave Chinner <[email protected]>
Running some recent repair tests on broken filesystem meant running phase 1 and 2 repeatedly to reproduce an issue at the start of phase
3. Phase 2 was taking approximately 10 minutes to run as it processes each AG serially.
Phase 2 can be trivially parallelised - it is simply scanning the per AG trees to calculate free block counts and free and used inodes counts. This can be done safely in parallel by giving each AG it's own structure to aggregate counts into, then once the AG scan is complete adding them all together.
This patch uses 32-way threading which results in no noticable slowdown on single SATA drives with NCQ, but results in ~10x reduction in runtime on a 12 disk RAID-0 array.
--- repair/phase2.c | 20 +--- repair/scan.c | 294 +++++++++++++++++++++++++++++---------------------- repair/scan.h | 39 +------ repair/xfs_repair.c | 10 ++- 4 files changed, 182 insertions(+), 181 deletions(-)
repair/phase2.c +5 -15
@@ -24,10 +24,9 @@#include "err_protos.h" #include "incore.h" #include "progress.h" +#include "scan.h" void set_mp (xfs_mount_t *mpp); -void scan_ag (xfs_agnumber_t agno); -void validate_sb (struct xfs_sb *sb);/* workaround craziness in the xlog routines */ int xlog_recover_do_trans (xlog_t *log,xlog_recover_t *t,int p) {return 0 ; }
@@ -107,9 +106,10 @@ zero_log(xfs_mount_t *mp)*/ void -phase2 (xfs_mount_t *mp) +phase2 ( +struct xfs_mount *mp, +int scan_threads) { -xfs_agnumber_t i;int j;ino_tree_node_t *ino_rec;
@@ -138,17 +138,7 @@ phase2(xfs_mount_t *mp)set_progress_msg (PROG_FMT_SCAN_AG , (__uint64_t ) glob_agcount); -for (i =0 ; i < mp->m_sb.sb_agcount; i++) { -scan_ag (i); -#ifdef XR_INODE_TRACE -print_inode_list (i); -#endif - } - -/* -* Validate that our manual counts match the superblock. -*/ -validate_sb (&mp->m_sb); +scan_ags (mp, scan_threads);print_final_rpt ();
repair/scan.c +166 -128
469 lines not on this page · raw message
repair/scan.h +3 -36
@@ -20,19 +20,6 @@struct blkmap ; -void scan_sbtree ( -xfs_agblock_t root, -int nlevels, -xfs_agnumber_t agno, -int suspect, -void (*func)(struct xfs_btree_block *block, -int level, -xfs_agblock_t bno, -xfs_agnumber_t agno, -int suspect, -int isroot), -int isroot); - int scan_lbtree( xfs_dfsbno_t root, int nlevels,
@@ -74,29 +61,9 @@ int scanfunc_bmap(int check_dups, int *dirty); -void scanfunc_bno ( -struct xfs_btree_block *block, -int level, -xfs_agblock_t bno, -xfs_agnumber_t agno, -int suspect, -int isroot); - -void scanfunc_cnt ( -struct xfs_btree_block *block, -int level, -xfs_agblock_t bno, -xfs_agnumber_t agno, -int suspect, -int isroot); -void -scanfunc_ino ( -struct xfs_btree_block *block, -int level, -xfs_agblock_t bno, -xfs_agnumber_t agno, -int suspect, -int isroot); +scan_ags ( +struct xfs_mount *mp, +int scan_threads);#endif /* _XR_SCAN_H */
repair/xfs_repair.c +8 -2
@@ -33,7 +33,7 @@#define rounddown (x, y) (((x)/(y))*(y))extern void phase1 (xfs_mount_t *); -extern void phase2 (xfs_mount_t *); +extern void phase2 (xfs_mount_t *,int );extern void phase3 (xfs_mount_t *);extern void phase4 (xfs_mount_t *);extern void phase5 (xfs_mount_t *);
@@ -63,6 +63,8 @@ char *o_opts[] = { "ag_stride",#define FORCE_GEO 5 "force_geometry", +#define PHASE2_THREADS 6 +"phase2_threads" , NULL };
@@ -80,6 +82,7 @@ char *c_opts[] = {static int ihash_option_used;static int bhash_option_used;static long max_mem_specified;/* in megabytes */ +static int phase2_threads =32 ;static void usage (void )
@@ -266,6 +269,9 @@ process_args(int argc, char **argv)respec ('o' , o_opts,FORCE_GEO ); force_geo =1 ;break ; +case PHASE2_THREADS : + phase2_threads = (int )strtol (val,NULL ,0 ); +break ;default :unknown ('o' , val);break ;
@@ -709,7 +715,7 @@ main(int argc, char **argv) }/* make sure the per-ag freespace maps are ok so we can mount the fs */ -phase2 (mp); +phase2 (mp, phase2_threads);timestamp (PHASE_END ,2 ,NULL );if (do_prefetch)
Signature · 6 lines
1.7.2.3 _______________________________________________ xfs mailing list [email protected] http://oss.sgi.com/mailman/listinfo/xfs
Quoted 3 lines · On Montag, 10. Januar 2011 Dave Chinner wrote:
This patch uses 32-way threading which results in no noticable slowdown on single SATA drives with NCQ, but results in ~10x reduction in runtime on a 12 disk RAID-0 array.
Is the fixed 32-way number reasonable, or shouldn't that be "number of available cpu cores"-way? Why threading when you have a single core cpu?
Signature · 11 lines
mit freundlichen Grüssen, Michael Monnerie, Ing. BSc it-management Internet Services: Protéger http://proteger.at [gesprochen: Prot-e-schee] Tel: +43 660 / 415 6531 // ****** Radiointerview zum Thema Spam ****** // http://www.it-podcast.at/archiv.html#podcast-100716 // // Haus zu verkaufen: http://zmi.at/langegg/
_______________________________________________
xfs mailing list
[email protected]
http://oss.sgi.com/mailman/listinfo/xfs
Quoted 7 lines · Michael Monnerie
Quoted 3 lines · On Montag, 10. Januar 2011 Dave Chinner wrote:
This patch uses 32-way threading which results in no noticable slowdown on single SATA drives with NCQ, but results in ~10x reduction in runtime on a 12 disk RAID-0 array.
Is the fixed 32-way number reasonable, or shouldn't that be "number of available cpu cores"-way? Why threading when you have a single core cpu?
Sure, 32-way is reasonable on a single disk and CPU. Pretty much every sata disk supports NCQ these days, and default to a depth of 32, which means we can have 32 concurrent reads in progress at once. Phase 2 is all synchronous IO, so the only way to hide the IO latency is to queue work to multiple threads and switch between the threadsto work on another queue when the current one blocks waiting for IO.
Basically the threading being used to drive IO level concurrency, not to drive CPU level concurrency - the total CPU usage of phase 2 is less than what even a slow CPU can provide, so to keep it busy we need lots of concurrent IO streams in progress at once...
And if you want to change it, there's a command line option for it.
Cheers,
Dave.
Signature · 7 lines
Dave Chinner [email protected] _______________________________________________ xfs mailing list [email protected] http://oss.sgi.com/mailman/listinfo/xfs
Quoted 7 lines · On Montag, 10. Januar 2011 Dave Chinner wrote:
Pretty much
every sata disk supports NCQ these days, and default to a depth of 32, which means we can have 32 concurrent reads in progress at once. Phase 2 is all synchronous IO, so the only way to hide the IO latency is to queue work to multiple threads and switch between the threadsto work on another queue when the current one blocks waiting for IO.
This is interesting. Did you measure this with a rotating single disk? Is the idle time between two synchronous reads bigger than the time needed to move the disk head to another cylinder and read a sector? That takes ~15ms on a normal disk, incredibly long compared to cpu speed. Even with NCQ, the disk would have to swing the head a lot, and just from thinking about it I wouldn't believe that it's faster like this.
But I'm sure you tested it so I take it as given that it's like that. Cool improvement, btw :-)
Signature · 11 lines
mit freundlichen Grüssen, Michael Monnerie, Ing. BSc it-management Internet Services: Protéger http://proteger.at [gesprochen: Prot-e-schee] Tel: +43 660 / 415 6531 // ****** Radiointerview zum Thema Spam ****** // http://www.it-podcast.at/archiv.html#podcast-100716 // // Haus zu verkaufen: http://zmi.at/langegg/
_______________________________________________
xfs mailing list
[email protected]
http://oss.sgi.com/mailman/listinfo/xfs
Quoted 10 lines · Michael Monnerie
Quoted 7 lines · On Montag, 10. Januar 2011 Dave Chinner wrote:
Pretty much
every sata disk supports NCQ these days, and default to a depth of 32, which means we can have 32 concurrent reads in progress at once. Phase 2 is all synchronous IO, so the only way to hide the IO latency is to queue work to multiple threads and switch between the threadsto work on another queue when the current one blocks waiting for IO.This is interesting. Did you measure this with a rotating single disk?
Take a look at the patch description.
_______________________________________________
xfs mailing list
[email protected]
http://oss.sgi.com/mailman/listinfo/xfs
Quoted 9 lines · Dave Chinner
Quoted 7 lines · Michael Monnerie
Quoted 3 lines · On Montag, 10. Januar 2011 Dave Chinner wrote:
This patch uses 32-way threading which results in no noticable slowdown on single SATA drives with NCQ, but results in ~10x reduction in runtime on a 12 disk RAID-0 array.
Is the fixed 32-way number reasonable, or shouldn't that be "number of available cpu cores"-way? Why threading when you have a single core cpu?
Quoted 7 lines
Sure, 32-way is reasonable on a single disk and CPU. Pretty much every sata disk supports NCQ these days, and default to a depth of 32, which means we can have 32 concurrent reads in progress at once. Phase 2 is all synchronous IO, so the only way to hide the IO latency is to queue work to multiple threads and switch between the threadsto work on another queue when the current one blocks waiting for IO.
The default queue depth for ATA NCQ actually is 31, not 32 for some odd reason.
_______________________________________________
xfs mailing list
[email protected]
http://oss.sgi.com/mailman/listinfo/xfs
Quoted 18 lines · Christoph Hellwig
Quoted 9 lines · Dave Chinner
Quoted 7 lines · Michael Monnerie
Quoted 3 lines · On Montag, 10. Januar 2011 Dave Chinner wrote:
This patch uses 32-way threading which results in no noticable slowdown on single SATA drives with NCQ, but results in ~10x reduction in runtime on a 12 disk RAID-0 array.
Is the fixed 32-way number reasonable, or shouldn't that be "number of available cpu cores"-way? Why threading when you have a single core cpu?
Quoted 7 lines
Sure, 32-way is reasonable on a single disk and CPU. Pretty much every sata disk supports NCQ these days, and default to a depth of 32, which means we can have 32 concurrent reads in progress at once. Phase 2 is all synchronous IO, so the only way to hide the IO latency is to queue work to multiple threads and switch between the threadsto work on another queue when the current one blocks waiting for IO.
Quoted 2 lines
The default queue depth for ATA NCQ actually is 31, not 32 for some odd reason.
AFAIR the original discussion it's because Depth 0 and 32 use the same value. And the knowledge that (HDD-)firmware tends to be buggy, so it was decided to stay on the safe side and not use "0/32".
Bis denn
Signature · 9 lines
Real Programmers consider "what you see is what you get" to be just as bad a concept in Text Editors as it is in women. No, the Real Programmer wants a "you asked for it, you got it" text editor -- complicated, cryptic, powerful, unforgiving, dangerous. _______________________________________________ xfs mailing list [email protected] http://oss.sgi.com/mailman/listinfo/xfs
Looks good except for some trivial nitpicks below,
Btw, your previous patch used just
"repair:" as the Subject prefix, while this one uses xfs_repair. I don't really care about, but we should standardize on one. The more recent usage seems to include the xfs_ prefix.
repair/scan.c _("%s freespace btree block claimed (state %d), agno %d, bno %d, suspect %d\n"), · line 711
Quoted 2 lines
repair/scan.c +2
-0+ scanfunc_bno : scanfunc_cnt,0 , + (void *)agcnts);
no need for the void cast.
Quoted 1 line
+#define
SCAN_THREADS32
this is unused now.
repair/scan.c scan_ag( · line 1338
Quoted 6 lines
repair/scan.c +6
-0+ agcnts =malloc (mp->m_sb.sb_agcount *sizeof (*agcnts)); +if (!agcnts) { +do_abort (_ ("no memory for ag header counts\n" )); +return ; + } +memset (agcnts,0 , mp->m_sb.sb_agcount *sizeof (*agcnts));
this could use a calloc.
repair/xfs_repair.c process_args(int argc, char **argv) · line 274
Quoted 4 lines
repair/xfs_repair.c +3
-0break ; +case PHASE2_THREADS : + phase2_threads = (int )strtol (val,NULL ,0 ); +break ;
This option also needs to be documented in the man page. Also shouldn't we try to handle errors from strtol? Also maybe strtoul would be a better choice as we certainly don't want a negative number of threads.
_______________________________________________
xfs mailing list
[email protected]
http://oss.sgi.com/mailman/listinfo/xfs
Quoted 13 lines · Dave Chinner
From: Dave Chinner <[email protected]>
Running some recent repair tests on broken filesystem meant running phase 1 and 2 repeatedly to reproduce an issue at the start of phase
3. Phase 2 was taking approximately 10 minutes to run as it processes each AG serially.Phase 2 can be trivially parallelised - it is simply scanning the per AG trees to calculate free block counts and free and used inodes counts. This can be done safely in parallel by giving each AG it's own structure to aggregate counts into, then once the AG scan is complete adding them all together.
Quoted 5 lines
This patch uses 32-way threading which results in no noticable slowdown on single SATA drives with NCQ, but results in ~10x reduction in runtime on a 12 disk RAID-0 array.
Signed-off-by Dave Chinner <[email protected]>
This looks good. Sorry I didn't say so earlier (I signed off on your first one).
_______________________________________________
xfs mailing list
[email protected]
http://oss.sgi.com/mailman/listinfo/xfs