From 356357a446424c54851608bb7cebce6cead14590 Mon Sep 17 00:00:00 2001 From: Christoph Hellwig Date: Fri, 2 Mar 2012 08:35:08 +0000 Subject: [PATCH] repair: fix incorrect use of thread local data in dir and attr code The attribute and dirv1 code use pthread thread local data incorrectly in a few places, which will make them fail in horrible ways when using the ag_stride options. Replace the use of thread local data with simple local allocations given that there is no needed to micro-optimize these allocations as much as e.g. the extent map. The added benefit is that we have to allocate less memory, and can free it quickly. Reviewed-by: Dave Chinner Reported-by: Tom Crane Tested-by: Tom Crane Signed-off-by: Christoph Hellwig --- repair/attr_repair.c | 12 +++----- repair/dir.c | 67 +++++++++++++++----------------------------- repair/dir.h | 9 ++---- repair/globals.h | 4 --- repair/init.c | 52 ---------------------------------- repair/protos.h | 4 --- 6 files changed, 30 insertions(+), 118 deletions(-) diff --git a/repair/attr_repair.c b/repair/attr_repair.c index a2052ba9a..bab65b120 100644 --- a/repair/attr_repair.c +++ b/repair/attr_repair.c @@ -362,12 +362,6 @@ rmtval_get(xfs_mount_t *mp, xfs_ino_t ino, blkmap_t *blkmap, return (clearit); } -/* - * freespace map for directory and attribute leaf blocks (1 bit per byte) - * 1 == used, 0 == free - */ -size_t ts_attr_freemap_size = sizeof(da_freemap_t) * DA_BMAP_SIZE; - /* The block is read in. The magic number and forward / backward * links are checked by the caller process_leaf_attr. * If any problems occur the routine returns with non-zero. In @@ -502,7 +496,7 @@ process_leaf_attr_block( { xfs_attr_leaf_entry_t *entry; int i, start, stop, clearit, usedbs, firstb, thissize; - da_freemap_t *attr_freemap = ts_attr_freemap(); + da_freemap_t *attr_freemap; clearit = usedbs = 0; *repair = 0; @@ -518,7 +512,7 @@ process_leaf_attr_block( return (1); } - init_da_freemap(attr_freemap); + attr_freemap = alloc_da_freemap(mp); (void) set_da_freemap(mp, attr_freemap, 0, stop); /* go thru each entry checking for problems */ @@ -635,6 +629,8 @@ process_leaf_attr_block( * we can add it then. */ } + + free(attr_freemap); return (clearit); /* and repair */ } diff --git a/repair/dir.c b/repair/dir.c index d575baefd..01c8f10f6 100644 --- a/repair/dir.c +++ b/repair/dir.c @@ -494,23 +494,19 @@ process_shortform_dir( } /* - * freespace map for directory leaf blocks (1 bit per byte) - * 1 == used, 0 == free + * Allocate a freespace map for directory or attr leaf blocks (1 bit per byte) + * 1 == used, 0 == free. */ -size_t ts_dir_freemap_size = sizeof(da_freemap_t) * DA_BMAP_SIZE; - -void -init_da_freemap(da_freemap_t *dir_freemap) +da_freemap_t * +alloc_da_freemap(struct xfs_mount *mp) { - memset(dir_freemap, 0, sizeof(da_freemap_t) * DA_BMAP_SIZE); + return calloc(1, mp->m_sb.sb_blocksize / NBBY); } /* - * sets directory freemap, returns 1 if there is a conflict - * returns 0 if everything's good. the range [start, stop) is set. - * right now, we just use the static array since only one directory - * block will be processed at once even though the interface allows - * you to pass in arbitrary da_freemap_t array's. + * Set the he range [start, stop) in the directory freemap. + * + * Returns 1 if there is a conflict or 0 if everything's good. * * Within a char, the lowest bit of the char represents the byte with * the smallest address @@ -726,28 +722,6 @@ _("- derived hole (base %d, size %d) in block %d, dir inode %" PRIu64 " not foun return(res); } -#if 0 -void -test(xfs_mount_t *mp) -{ - int i = 0; - da_hole_map_t holemap; - - init_da_freemap(dir_freemap); - memset(&holemap, 0, sizeof(da_hole_map_t)); - - set_da_freemap(mp, dir_freemap, 0, 50); - set_da_freemap(mp, dir_freemap, 100, 126); - set_da_freemap(mp, dir_freemap, 126, 129); - set_da_freemap(mp, dir_freemap, 130, 131); - set_da_freemap(mp, dir_freemap, 150, 160); - process_da_freemap(mp, dir_freemap, &holemap); - - return; -} -#endif - - /* * walk tree from root to the left-most leaf block reading in * blocks and setting up cursor. passes back file block number of the @@ -1279,8 +1253,6 @@ verify_da_path(xfs_mount_t *mp, return(0); } -size_t ts_dirbuf_size = 64*1024; - /* * called by both node dir and leaf dir processing routines * validates all contents *but* the sibling pointers (forw/back) @@ -1353,7 +1325,7 @@ process_leaf_dir_block( char fname[MAXNAMELEN + 1]; da_hole_map_t holemap; da_hole_map_t bholemap; - unsigned char *dir_freemap = ts_dir_freemap(); + da_freemap_t *dir_freemap; #ifdef XR_DIR_TRACE fprintf(stderr, "\tprocess_leaf_dir_block - ino %" PRIu64 "\n", ino); @@ -1362,7 +1334,7 @@ process_leaf_dir_block( /* * clear static dir block freespace bitmap */ - init_da_freemap(dir_freemap); + dir_freemap = alloc_da_freemap(mp); *buf_dirty = 0; first_used = mp->m_sb.sb_blocksize; @@ -1374,7 +1346,8 @@ process_leaf_dir_block( do_warn( _("directory block header conflicts with used space in directory inode %" PRIu64 "\n"), ino); - return(1); + res = 1; + goto out; } /* @@ -1690,8 +1663,8 @@ _("entry references free inode %" PRIu64 " in directory %" PRIu64 ", would clear do_warn( _("bad size, entry #%d in dir inode %" PRIu64 ", block %u -- entry overflows block\n"), i, ino, da_bno); - - return(1); + res = 1; + goto out; } start = (__psint_t)&leaf->entries[i] - (__psint_t)leaf;; @@ -1701,7 +1674,8 @@ _("bad size, entry #%d in dir inode %" PRIu64 ", block %u -- entry overflows blo do_warn( _("dir entry slot %d in block %u conflicts with used space in dir inode %" PRIu64 "\n"), i, da_bno, ino); - return(1); + res = 1; + goto out; } /* @@ -2095,7 +2069,7 @@ _("- existing hole info for block %d, dir inode %" PRIu64 " (base, size) - \n"), _("- compacting block %u in dir inode %" PRIu64 "\n"), da_bno, ino); - new_leaf = (xfs_dir_leafblock_t *) ts_dirbuf(); + new_leaf = malloc(mp->m_sb.sb_blocksize); /* * copy leaf block header @@ -2135,6 +2109,7 @@ _("- existing hole info for block %d, dir inode %" PRIu64 " (base, size) - \n"), do_warn( _("not enough space in block %u of dir inode %" PRIu64 " for all entries\n"), da_bno, ino); + free(new_leaf); break; } @@ -2196,6 +2171,7 @@ _("- existing hole info for block %d, dir inode %" PRIu64 " (base, size) - \n"), * final step, copy block back */ memmove(leaf, new_leaf, mp->m_sb.sb_blocksize); + free(new_leaf); *buf_dirty = 1; } else { @@ -2214,10 +2190,13 @@ _("- existing hole info for block %d, dir inode %" PRIu64 " (base, size) - \n"), junk_zerolen_dir_leaf_entries(mp, leaf, ino, buf_dirty); } #endif + +out: + free(dir_freemap); #ifdef XR_DIR_TRACE fprintf(stderr, "process_leaf_dir_block returns %d\n", res); #endif - return((res > 0) ? 1 : 0); + return res > 0 ? 1 : 0; } /* diff --git a/repair/dir.h b/repair/dir.h index a23990d2b..cea31a37b 100644 --- a/repair/dir.h +++ b/repair/dir.h @@ -21,9 +21,6 @@ struct blkmap; -/* 1 bit per byte, max XFS blocksize == 64K bits / NBBY */ -#define DA_BMAP_SIZE 8192 - typedef unsigned char da_freemap_t; /* @@ -75,9 +72,9 @@ err_release_da_cursor( da_bt_cursor_t *cursor, int prev_level); -void -init_da_freemap( - da_freemap_t *dir_freemap); +da_freemap_t * +alloc_da_freemap( + xfs_mount_t *mp); int namecheck( diff --git a/repair/globals.h b/repair/globals.h index 5fb8149b4..e01e4e997 100644 --- a/repair/globals.h +++ b/repair/globals.h @@ -185,10 +185,6 @@ EXTERN xfs_extlen_t sb_inoalignmt; EXTERN __uint32_t sb_unit; EXTERN __uint32_t sb_width; -extern size_t ts_dirbuf_size; -extern size_t ts_dir_freemap_size; -extern size_t ts_attr_freemap_size; - EXTERN pthread_mutex_t *ag_locks; EXTERN int report_interval; diff --git a/repair/init.c b/repair/init.c index d51d6e481..4751a057b 100644 --- a/repair/init.c +++ b/repair/init.c @@ -29,64 +29,13 @@ #include "prefetch.h" #include -/* TODO: dirbuf/freemap key usage is completely b0rked - only used for dirv1 */ -static pthread_key_t dirbuf_key; -static pthread_key_t dir_freemap_key; -static pthread_key_t attr_freemap_key; - -static void -ts_alloc(pthread_key_t key, unsigned n, size_t size) -{ - void *voidp; - voidp = calloc(n, size); - if (voidp == NULL) { - do_error(_("ts_alloc: cannot allocate thread specific storage\n")); - /* NO RETURN */ - return; - } - pthread_setspecific(key, voidp); -} - static void ts_create(void) { - /* create thread specific keys */ - pthread_key_create(&dirbuf_key, NULL); - pthread_key_create(&dir_freemap_key, NULL); - pthread_key_create(&attr_freemap_key, NULL); - pthread_key_create(&dblkmap_key, NULL); pthread_key_create(&ablkmap_key, NULL); } -void -ts_init(void) -{ - - /* allocate thread specific storage */ - ts_alloc(dirbuf_key, 1, ts_dirbuf_size); - ts_alloc(dir_freemap_key, 1, ts_dir_freemap_size); - ts_alloc(attr_freemap_key, 1, ts_attr_freemap_size); -} - -void * -ts_dirbuf(void) -{ - return pthread_getspecific(dirbuf_key); -} - -void * -ts_dir_freemap(void) -{ - return pthread_getspecific(dir_freemap_key); -} - -void * -ts_attr_freemap(void) -{ - return pthread_getspecific(attr_freemap_key); -} - static void increase_rlimit(void) { @@ -153,7 +102,6 @@ xfs_init(libxfs_init_t *args) do_error(_("couldn't initialize XFS library\n")); ts_create(); - ts_init(); increase_rlimit(); pftrace_init(); } diff --git a/repair/protos.h b/repair/protos.h index b66eea137..601f2a966 100644 --- a/repair/protos.h +++ b/repair/protos.h @@ -41,10 +41,6 @@ char *alloc_ag_buf(int size); void print_inode_list(xfs_agnumber_t i); char *err_string(int err_code); -void *ts_attr_freemap(void); -void *ts_dir_freemap(void); -void *ts_dirbuf(void); -void ts_init(void); void thread_init(void); void phase1(struct xfs_mount *); -- 2.47.2