From 9b8ddf36a1bfc6b74b75a3b5d0d42bb1a07003fc Mon Sep 17 00:00:00 2001 From: Andrew Price Date: Jan 13 2023 17:54:58 +0000 Subject: [PATCH 1/3] libgfs2: Add member name prefixes to struct _lgfs2_rgrps Make these names more greppable. Signed-off-by: Andrew Price --- diff --git a/gfs2/libgfs2/check_rgrp.c b/gfs2/libgfs2/check_rgrp.c index 23f267a..2671004 100644 --- a/gfs2/libgfs2/check_rgrp.c +++ b/gfs2/libgfs2/check_rgrp.c @@ -57,8 +57,8 @@ static void mockup_rgrps(void) static void teardown_rgrps(void) { - close(tc_rgrps->sdp->device_fd); - free(tc_rgrps->sdp); + close(tc_rgrps->rgs_sdp->device_fd); + free(tc_rgrps->rgs_sdp); lgfs2_rgrp_bitbuf_free(lgfs2_rgrp_first(tc_rgrps)); lgfs2_rgrps_free(&tc_rgrps); } @@ -115,7 +115,7 @@ START_TEST(test_rbm_find_lastblock) /* Flag all blocks as allocated... */ for (i = 0; i < rg->rt_length; i++) - memset(rg->bits[i].bi_data, 0xff, rgs->sdp->sd_bsize); + memset(rg->bits[i].bi_data, 0xff, rgs->rgs_sdp->sd_bsize); /* ...except the final one */ err = lgfs2_set_bitmap(rg, rg->rt_data0 + rg->rt_data - 1, GFS2_BLKST_FREE); @@ -132,7 +132,7 @@ END_TEST START_TEST(test_rgrps_write_final) { lgfs2_rgrp_t rg = lgfs2_rgrp_last(tc_rgrps); - struct lgfs2_sbd *sdp = tc_rgrps->sdp; + struct lgfs2_sbd *sdp = tc_rgrps->rgs_sdp; struct gfs2_rindex ri; struct gfs2_rgrp rgrp; uint64_t addr; diff --git a/gfs2/libgfs2/fs_ops.c b/gfs2/libgfs2/fs_ops.c index bf22c78..0da9beb 100644 --- a/gfs2/libgfs2/fs_ops.c +++ b/gfs2/libgfs2/fs_ops.c @@ -310,7 +310,7 @@ uint64_t lgfs2_space_for_data(const struct lgfs2_sbd *sdp, const unsigned bsize, int lgfs2_file_alloc(lgfs2_rgrp_t rg, uint64_t di_size, struct lgfs2_inode *ip, uint32_t flags, unsigned mode) { unsigned extlen; - struct lgfs2_sbd *sdp = rg->rgrps->sdp; + struct lgfs2_sbd *sdp = rg->rgrps->rgs_sdp; struct lgfs2_rbm rbm = { .rgd = rg, .offset = 0, .bii = 0 }; uint32_t blocks = lgfs2_space_for_data(sdp, sdp->sd_bsize, di_size); diff --git a/gfs2/libgfs2/rgrp.c b/gfs2/libgfs2/rgrp.c index 56ee571..0dd1a3e 100644 --- a/gfs2/libgfs2/rgrp.c +++ b/gfs2/libgfs2/rgrp.c @@ -108,15 +108,15 @@ struct lgfs2_rgrp_tree *lgfs2_blk2rgrpd(struct lgfs2_sbd *sdp, uint64_t blk) */ int lgfs2_rgrp_bitbuf_alloc(lgfs2_rgrp_t rg) { - struct lgfs2_sbd *sdp = rg->rgrps->sdp; + struct lgfs2_sbd *sdp = rg->rgrps->rgs_sdp; size_t len = rg->rt_length * sdp->sd_bsize; unsigned long io_align = sdp->sd_bsize; unsigned i; char *bufs; - if (rg->rgrps->align > 0) { - len = ROUND_UP(len, rg->rgrps->align * sdp->sd_bsize); - io_align = rg->rgrps->align_off * sdp->sd_bsize; + if (rg->rgrps->rgs_align > 0) { + len = ROUND_UP(len, rg->rgrps->rgs_align * sdp->sd_bsize); + io_align = rg->rgrps->rgs_align_off * sdp->sd_bsize; } if (posix_memalign((void **)&bufs, io_align, len) != 0) { errno = ENOMEM; @@ -312,7 +312,7 @@ static uint64_t align_block(const uint64_t base, const uint64_t align) */ uint64_t lgfs2_rgrp_align_addr(const lgfs2_rgrps_t rgs, uint64_t addr) { - return align_block(addr, rgs->align_off); + return align_block(addr, rgs->rgs_align_off); } /** @@ -325,7 +325,7 @@ uint64_t lgfs2_rgrp_align_addr(const lgfs2_rgrps_t rgs, uint64_t addr) */ uint32_t lgfs2_rgrp_align_len(const lgfs2_rgrps_t rgs, uint32_t len) { - return align_block(len, rgs->align) + rgs->align_off; + return align_block(len, rgs->rgs_align) + rgs->rgs_align_off; } /** @@ -344,22 +344,22 @@ uint32_t lgfs2_rgrp_align_len(const lgfs2_rgrps_t rgs, uint32_t len) */ uint32_t lgfs2_rgrps_plan(const lgfs2_rgrps_t rgs, uint64_t space, uint32_t tgtsize) { - uint32_t maxlen = (LGFS2_MAX_RGSIZE << 20) / rgs->sdp->sd_bsize; - uint32_t minlen = (LGFS2_MIN_RGSIZE << 20) / rgs->sdp->sd_bsize; - struct rg_spec *spec = rgs->plan->rg_specs; + uint32_t maxlen = (LGFS2_MAX_RGSIZE << 20) / rgs->rgs_sdp->sd_bsize; + uint32_t minlen = (LGFS2_MIN_RGSIZE << 20) / rgs->rgs_sdp->sd_bsize; + struct rg_spec *spec = rgs->rgs_plan->rg_specs; /* Apps should already have checked that the rg size is <= GFS2_MAX_RGSIZE but just in case alignment pushes it over we clamp it back down while calculating the initial rgrp length. */ do { spec[0].len = lgfs2_rgrp_align_len(rgs, tgtsize); - tgtsize -= (rgs->align + 1); + tgtsize -= (rgs->rgs_align + 1); } while (spec[0].len > maxlen); spec[0].num = space / spec[0].len; - if ((space - (spec[0].num * spec[0].len)) > rgs->align) { - unsigned adj = (rgs->align > 0) ? rgs->align : 1; + if ((space - (spec[0].num * spec[0].len)) > rgs->rgs_align) { + unsigned adj = (rgs->rgs_align > 0) ? rgs->rgs_align : 1; /* Spread the adjustment required to fit a new rgrp at the end over all of the rgrps so that we don't end with a single @@ -416,17 +416,17 @@ lgfs2_rgrps_t lgfs2_rgrps_init(struct lgfs2_sbd *sdp, uint64_t align, uint64_t o if (rgs == NULL) return NULL; - rgs->plan = calloc(1, sizeof(struct rgs_plan) + (5 * sizeof(struct rg_spec))); - if (rgs->plan == NULL) { + rgs->rgs_plan = calloc(1, sizeof(struct rgs_plan) + (5 * sizeof(struct rg_spec))); + if (rgs->rgs_plan == NULL) { free(rgs); return NULL; } - rgs->plan->length = 0; - rgs->plan->capacity = 5; - rgs->sdp = sdp; - rgs->align = align; - rgs->align_off = offset; - memset(&rgs->root, 0, sizeof(rgs->root)); + rgs->rgs_plan->length = 0; + rgs->rgs_plan->capacity = 5; + rgs->rgs_sdp = sdp; + rgs->rgs_align = align; + rgs->rgs_align_off = offset; + memset(&rgs->rgs_root, 0, sizeof(rgs->rgs_root)); return rgs; } @@ -502,7 +502,7 @@ lgfs2_rgrp_t lgfs2_rindex_read_one(struct lgfs2_inode *rip, lgfs2_rgrps_t rgs, u void lgfs2_rgrps_free(lgfs2_rgrps_t *rgs) { lgfs2_rgrp_t rg; - struct osi_root *tree = &(*rgs)->root; + struct osi_root *tree = &(*rgs)->rgs_root; while ((rg = (struct lgfs2_rgrp_tree *)osi_first(tree))) { int i; @@ -513,7 +513,7 @@ void lgfs2_rgrps_free(lgfs2_rgrps_t *rgs) osi_erase(&rg->node, tree); free(rg); } - free((*rgs)->plan); + free((*rgs)->rgs_plan); free(*rgs); *rgs = NULL; } @@ -556,7 +556,7 @@ uint32_t lgfs2_rgblocks2bitblocks(const unsigned int bsize, const uint32_t rgblo */ uint64_t lgfs2_rindex_entry_new(lgfs2_rgrps_t rgs, struct gfs2_rindex *ri, uint64_t addr, uint32_t len) { - struct rg_spec *spec = rgs->plan->rg_specs; + struct rg_spec *spec = rgs->rgs_plan->rg_specs; uint32_t ri_length, ri_data; int plan = -1; errno = EINVAL; @@ -576,10 +576,10 @@ uint64_t lgfs2_rindex_entry_new(lgfs2_rgrps_t rgs, struct gfs2_rindex *ri, uint6 spec[plan].num--; } - if (addr + len > rgs->sdp->device.length) + if (addr + len > rgs->rgs_sdp->device.length) return 0; - ri_length = lgfs2_rgblocks2bitblocks(rgs->sdp->sd_bsize, len, &ri_data); + ri_length = lgfs2_rgblocks2bitblocks(rgs->rgs_sdp->sd_bsize, len, &ri_data); ri->ri_addr = cpu_to_be64(addr); ri->ri_length = cpu_to_be32(ri_length); ri->ri_data = cpu_to_be32(ri_data); @@ -608,7 +608,7 @@ unsigned lgfs2_rgsize_for_data(uint64_t blksreq, unsigned bsize) // Temporary function to aid in API migration void lgfs2_attach_rgrps(struct lgfs2_sbd *sdp, lgfs2_rgrps_t rgs) { - sdp->rgtree.osi_node = rgs->root.osi_node; + sdp->rgtree.osi_node = rgs->rgs_root.osi_node; } /** @@ -621,8 +621,8 @@ void lgfs2_attach_rgrps(struct lgfs2_sbd *sdp, lgfs2_rgrps_t rgs) lgfs2_rgrp_t lgfs2_rgrps_append(lgfs2_rgrps_t rgs, struct gfs2_rindex *entry, uint32_t rg_skip) { lgfs2_rgrp_t rg; - struct osi_node **link = &rgs->root.osi_node; - struct osi_node *parent = osi_last(&rgs->root); + struct osi_node **link = &rgs->rgs_root.osi_node; + struct osi_node *parent = osi_last(&rgs->rgs_root); lgfs2_rgrp_t lastrg = (lgfs2_rgrp_t)parent; errno = EINVAL; @@ -642,7 +642,7 @@ lgfs2_rgrp_t lgfs2_rgrps_append(lgfs2_rgrps_t rgs, struct gfs2_rindex *entry, ui rg->bits = (struct lgfs2_bitmap *)(rg + 1); osi_link_node(&rg->node, parent, link); - osi_insert_color(&rg->node, &rgs->root); + osi_insert_color(&rg->node, &rgs->rgs_root); rg->rt_addr = be64_to_cpu(entry->ri_addr); rg->rt_length = be32_to_cpu(entry->ri_length); @@ -654,7 +654,7 @@ lgfs2_rgrp_t lgfs2_rgrps_append(lgfs2_rgrps_t rgs, struct gfs2_rindex *entry, ui rg->rt_dinodes = 0; rg->rt_skip = rg_skip; rg->rt_igeneration = 0; - compute_bitmaps(rg, rgs->sdp->sd_bsize); + compute_bitmaps(rg, rgs->rgs_sdp->sd_bsize); rg->rgrps = rgs; return rg; } @@ -665,7 +665,7 @@ lgfs2_rgrp_t lgfs2_rgrps_append(lgfs2_rgrps_t rgs, struct gfs2_rindex *entry, ui */ int lgfs2_rgrp_write(int fd, const lgfs2_rgrp_t rg) { - struct lgfs2_sbd *sdp = rg->rgrps->sdp; + struct lgfs2_sbd *sdp = rg->rgrps->rgs_sdp; unsigned int i; int freebufs = 0; ssize_t ret; @@ -686,8 +686,8 @@ int lgfs2_rgrp_write(int fd, const lgfs2_rgrp_t rg) } len = sdp->sd_bsize * rg->rt_length; - if (rg->rgrps->align > 0) - len = ROUND_UP(len, rg->rgrps->align * sdp->sd_bsize); + if (rg->rgrps->rgs_align > 0) + len = ROUND_UP(len, rg->rgrps->rgs_align * sdp->sd_bsize); ret = pwrite(fd, rg->bits[0].bi_data, len, rg->rt_addr * sdp->sd_bsize); @@ -716,7 +716,7 @@ int lgfs2_rgrps_write_final(int fd, lgfs2_rgrps_t rgs) lgfs2_rgrp_t lgfs2_rgrp_first(lgfs2_rgrps_t rgs) { - return (lgfs2_rgrp_t)osi_first(&rgs->root); + return (lgfs2_rgrp_t)osi_first(&rgs->rgs_root); } lgfs2_rgrp_t lgfs2_rgrp_next(lgfs2_rgrp_t rg) @@ -731,7 +731,7 @@ lgfs2_rgrp_t lgfs2_rgrp_prev(lgfs2_rgrp_t rg) lgfs2_rgrp_t lgfs2_rgrp_last(lgfs2_rgrps_t rgs) { - return (lgfs2_rgrp_t)osi_last(&rgs->root); + return (lgfs2_rgrp_t)osi_last(&rgs->rgs_root); } /** @@ -749,7 +749,7 @@ lgfs2_rgrp_t lgfs2_rgrp_last(lgfs2_rgrps_t rgs) int lgfs2_rbm_from_block(struct lgfs2_rbm *rbm, uint64_t block) { uint64_t rblock = block - rbm->rgd->rt_data0; - struct lgfs2_sbd *sdp = rbm->rgd->rgrps->sdp; + struct lgfs2_sbd *sdp = rbm->rgd->rgrps->rgs_sdp; if (rblock > UINT_MAX) { errno = EINVAL; @@ -880,7 +880,7 @@ static uint32_t lgfs2_free_extlen(const struct lgfs2_rbm *rrbm, uint32_t len) uint8_t *ptr, *start, *end; uint64_t block; struct lgfs2_bitmap *bi; - struct lgfs2_sbd *sdp = rbm.rgd->rgrps->sdp; + struct lgfs2_sbd *sdp = rbm.rgd->rgrps->rgs_sdp; if (n_unaligned && lgfs2_unaligned_extlen(&rbm, 4 - n_unaligned, &len)) diff --git a/gfs2/libgfs2/rgrp.h b/gfs2/libgfs2/rgrp.h index f23f25a..434c274 100644 --- a/gfs2/libgfs2/rgrp.h +++ b/gfs2/libgfs2/rgrp.h @@ -21,11 +21,11 @@ struct rgs_plan { * in an application. */ struct _lgfs2_rgrps { - struct osi_root root; - struct rgs_plan *plan; - struct lgfs2_sbd *sdp; - unsigned long align; - unsigned long align_off; + struct osi_root rgs_root; + struct rgs_plan *rgs_plan; + struct lgfs2_sbd *rgs_sdp; + unsigned long rgs_align; + unsigned long rgs_align_off; }; struct lgfs2_rbm { From 3e78a3c855741a83542e7193ac1c3ceecc636571 Mon Sep 17 00:00:00 2001 From: Andrew Price Date: Jan 16 2023 15:29:04 +0000 Subject: [PATCH 2/3] libgfs2: Make sure block_alloc() fails when out of space block_alloc() fails to fail when no resource group can satisfy the required free space, due to checking the rgt pointer which is only updated inside the loop. Check the tree node pointer for NULL instead of the rgrp pointer, which is only set inside the loop to cast the tree node pointer. Fixes: 915fb43e "libgfs2: Rework blk_alloc_i" Signed-off-by: Andrew Price --- diff --git a/gfs2/libgfs2/fs_ops.c b/gfs2/libgfs2/fs_ops.c index 0da9beb..138449e 100644 --- a/gfs2/libgfs2/fs_ops.c +++ b/gfs2/libgfs2/fs_ops.c @@ -174,7 +174,7 @@ static int block_alloc(struct lgfs2_sbd *sdp, const uint64_t blksreq, int state, if (rgt->rt_free >= blksreq) break; } - if (rgt == NULL) + if (n == NULL) return -1; if (rgt->bits[0].bi_data == NULL) { From e32fdf7f119226fc9825502b7e19614ddc9bde5e Mon Sep 17 00:00:00 2001 From: Andrew Price Date: Jan 16 2023 16:30:20 +0000 Subject: [PATCH 3/3] libgfs2: Add unit tests for lgfs2_dinode_alloc() Includes a reusable fixture for mocking up resource groups. Signed-off-by: Andrew Price --- diff --git a/gfs2/libgfs2/check_fs_ops.c b/gfs2/libgfs2/check_fs_ops.c index 0b450ec..c219c67 100644 --- a/gfs2/libgfs2/check_fs_ops.c +++ b/gfs2/libgfs2/check_fs_ops.c @@ -8,8 +8,9 @@ Suite *suite_fs_ops(void); /* Doesn't need to be big, we just need some blocks to do i/o on */ -#define MOCK_DEV_SIZE (1 << 20) +#define MOCK_DEV_SIZE ((LGFS2_MAX_RGSIZE * 2ull) << 20) #define MOCK_BSIZE (512) +#define MOCK_RGSIZE ((LGFS2_MIN_RGSIZE + 2) << 20) struct lgfs2_sbd *mock_sdp; @@ -40,6 +41,51 @@ static void teardown_mock_fs(void) free(mock_sdp); } +lgfs2_rgrps_t mock_rgs; + +/* This fixture must be added after mockup_fs */ +static void mockup_rgs(void) +{ + lgfs2_rgrps_t rgs; + uint32_t rgcount, rgcount_check = 0; + uint32_t tgtsize; + uint64_t addr = GFS2_SB_ADDR + 1; + + ck_assert(mock_sdp != NULL); + + rgs = lgfs2_rgrps_init(mock_sdp, 0, 0); + ck_assert(rgs != NULL); + + tgtsize = MOCK_RGSIZE / mock_sdp->sd_bsize; + rgcount = lgfs2_rgrps_plan(rgs, mock_sdp->device.length - GFS2_SB_ADDR, tgtsize); + ck_assert(rgcount > 0); + while (1) { + uint64_t nextaddr = 0; + lgfs2_rgrp_t rg; + struct gfs2_rindex ri; + int err; + + nextaddr = lgfs2_rindex_entry_new(rgs, &ri, addr, 0); + if (nextaddr == 0) + break; + rg = lgfs2_rgrps_append(rgs, &ri, addr - nextaddr); + ck_assert(rg != NULL); + err = lgfs2_rgrp_bitbuf_alloc(rg); + ck_assert(err == 0); + addr = nextaddr; + rgcount_check++; + } + ck_assert(rgcount_check == rgcount); + mock_rgs = rgs; + lgfs2_attach_rgrps(mock_sdp, mock_rgs); +} + +static void teardown_mock_rgs(void) +{ + lgfs2_rgrps_free(&mock_rgs); + ck_assert(mock_rgs == NULL); +} + START_TEST(test_lookupi_bad_name_size) { struct lgfs2_inode idir; @@ -113,6 +159,28 @@ START_TEST(test_lookupi_dotdot) } END_TEST +START_TEST(test_dinode_alloc) +{ + struct lgfs2_sbd *sdp = mock_sdp; + uint64_t newblock = 0; + uint64_t expected_alloced = sdp->dinodes_alloced; + int err; + + /* Fail an allocation by requesting too many blocks */ + err = lgfs2_dinode_alloc(sdp, ~(uint64_t)0, &newblock); + ck_assert(err != 0); + ck_assert(sdp->dinodes_alloced == expected_alloced); + + /* Allocate a dinode, request 1 block of space in the rg */ + err = lgfs2_dinode_alloc(sdp, 1, &newblock); + expected_alloced += 1; + ck_assert(err == 0); + ck_assert(newblock > GFS2_SB_ADDR); + ck_assert(sdp->dinodes_alloced == expected_alloced); + ck_assert(lgfs2_get_bitmap(sdp, newblock, NULL) == GFS2_BLKST_DINODE); +} +END_TEST + Suite *suite_fs_ops(void) { Suite *s = suite_create("fs_ops.c"); @@ -128,5 +196,11 @@ Suite *suite_fs_ops(void) tcase_add_test(tc, test_lookupi_dotdot); suite_add_tcase(s, tc); + tc = tcase_create("lgfs2_dinode_alloc"); + tcase_add_checked_fixture(tc, mockup_fs, teardown_mock_fs); + tcase_add_checked_fixture(tc, mockup_rgs, teardown_mock_rgs); + tcase_add_test(tc, test_dinode_alloc); + suite_add_tcase(s, tc); + return s; }