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 <dchinner@redhat.com>
Reported-by: Tom Crane <T.Crane@rhul.ac.uk>
Tested-by: Tom Crane <T.Crane@rhul.ac.uk>
Signed-off-by: Christoph Hellwig <hch@lst.de>

diff --git a/repair/attr_repair.c b/repair/attr_repair.c
index a2052ba9a2c0ea438c7bbf96f4436ada021a49f0..bab65b120d678ebba9c895bf54139b024eeab776 100644 (file)
--- 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 d575baefdd684d9d6ded60cecc854e5220c539fa..01c8f10f6fa52f02d8b2d4063ffc25d1b98ec366 100644 (file)
--- 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 a23990d2b16a4334e263cc136a3a7d5deacfda4b..cea31a37b4502e923ebcd4da1a8bd80aec716fa5 100644 (file)
--- 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 5fb8149b4422af26d88a85402181f59b7271d670..e01e4e9973f876a2b5b76af81da2ff859b696c77 100644 (file)
--- 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 d51d6e481fdcf7bfaceac8aa85e069ad4940d359..4751a057b0cbfda3dae5dccf72a7dd8d0af72bf3 100644 (file)
--- a/repair/init.c
+++ b/repair/init.c
@@ -29,64 +29,13 @@
 #include "prefetch.h"
 #include <sys/resource.h>
 
-/* 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 b66eea1375d6e048e4edc633e1cddcdfd0c2d86a..601f2a9663fbbffc4af86dd0fc34cc10c65e05d0 100644 (file)
--- 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 *);