From 771866c3bc1c4a3ca724aea069b6bc3e2422436d Mon Sep 17 00:00:00 2001 From: Paul Dagnelie Date: Tue, 9 Aug 2016 11:02:36 -0400 Subject: [PATCH] 7176 Yet another hole birth issue This is another bug in the long line of hole-birth related issues. In this particular case, it was discovered that a previous hole-birth fix (illumos bug 6513) did not cover as many cases as we thought it did. While the issue worked in the case of hole-punching (writing zeroes to a large part of a file), it did not deal with truncation, and then writing beyond the new end of the file. The problem is that dbuf_findbp will return ENOENT if the block it's trying to find is beyond the end of the file. If that happens, we assume there is no birth time, and so we lose that information when we write out new blkptrs. We should teach dbuf_findbp to look for things that are beyond the current end, but not beyond the absolute end of the file. Reviewed by: Matthew Ahrens Reviewed by: George Wilson Ported by: Boris Protopopov --- include/sys/dnode.h | 8 +++++++- module/zfs/dbuf.c | 36 +++++++++++++++++++++++++++--------- 2 files changed, 34 insertions(+), 10 deletions(-) diff --git a/include/sys/dnode.h b/include/sys/dnode.h index cee4ea783810..147175683041 100644 --- a/include/sys/dnode.h +++ b/include/sys/dnode.h @@ -58,7 +58,13 @@ extern "C" { */ #define DNODE_SHIFT 9 /* 512 bytes */ #define DN_MIN_INDBLKSHIFT 12 /* 4k */ -#define DN_MAX_INDBLKSHIFT 14 /* 16k */ +/* + * If we ever increase this value beyond 20, we need to revisit all logic that + * does x << level * ebps to handle overflow. With a 1M indirect block size, + * 4 levels of indirect blocks would not be able to guarantee addressing an + * entire object, so 5 levels will be used, but 5 * (20 - 7) = 65. + */ +#define DN_MAX_INDBLKSHIFT 17 /* 128k */ #define DNODE_BLOCK_SHIFT 14 /* 16k */ #define DNODE_CORE_SIZE 64 /* 64 bytes for dnode sans blkptrs */ #define DN_MAX_OBJECT_SHIFT 48 /* 256 trillion (zfs_fid_t limit) */ diff --git a/module/zfs/dbuf.c b/module/zfs/dbuf.c index 1728694ac2e5..51f08102c878 100644 --- a/module/zfs/dbuf.c +++ b/module/zfs/dbuf.c @@ -1926,8 +1926,6 @@ static inline int dbuf_findbp(dnode_t *dn, int level, uint64_t blkid, int fail_sparse, dmu_buf_impl_t **parentp, blkptr_t **bpp, struct dbuf_hold_impl_data *dh) { - int nlevels, epbs; - *parentp = NULL; *bpp = NULL; @@ -1946,17 +1944,35 @@ dbuf_findbp(dnode_t *dn, int level, uint64_t blkid, int fail_sparse, return (0); } - if (dn->dn_phys->dn_nlevels == 0) - nlevels = 1; - else - nlevels = dn->dn_phys->dn_nlevels; - - epbs = dn->dn_indblkshift - SPA_BLKPTRSHIFT; + int nlevels = + (dn->dn_phys->dn_nlevels == 0) ? 1 : dn->dn_phys->dn_nlevels; + int epbs = dn->dn_indblkshift - SPA_BLKPTRSHIFT; ASSERT3U(level * epbs, <, 64); ASSERT(RW_LOCK_HELD(&dn->dn_struct_rwlock)); + /* + * This assertion shouldn't trip as long as the max indirect block size + * is less than 1M. The reason for this is that up to that point, + * the number of levels required to address an entire object with blocks + * of size SPA_MINBLOCKSIZE satisfies nlevels * epbs + 1 <= 64. In + * other words, if N * epbs + 1 > 64, then if (N-1) * epbs + 1 > 55 + * (i.e. we can address the entire object), objects will all use at most + * N-1 levels and the assertion won't overflow. However, once epbs is + * 13, 4 * 13 + 1 = 53, but 5 * 13 + 1 = 66. Then, 4 levels will not be + * enough to address an entire object, so objects will have 5 levels, + * but then this assertion will overflow. + * + * All this is to say that if we ever increase DN_MAX_INDBLKSHIFT, we + * need to redo this logic to handle overflows. + */ + ASSERT(level >= nlevels || + ((nlevels - level - 1) * epbs) + + highbit64(dn->dn_phys->dn_nblkptr) <= 64); if (level >= nlevels || - (blkid > (dn->dn_phys->dn_maxblkid >> (level * epbs)))) { + blkid >= ((uint64_t)dn->dn_phys->dn_nblkptr << + ((nlevels - level - 1) * epbs)) || + (fail_sparse && + blkid > (dn->dn_phys->dn_maxblkid >> (level * epbs)))) { /* the buffer has no parent yet */ return (SET_ERROR(ENOENT)); } else if (level < nlevels-1) { @@ -1982,6 +1998,8 @@ dbuf_findbp(dnode_t *dn, int level, uint64_t blkid, int fail_sparse, } *bpp = ((blkptr_t *)(*parentp)->db.db_data) + (blkid & ((1ULL << epbs) - 1)); + if (blkid > (dn->dn_phys->dn_maxblkid >> (level * epbs))) + ASSERT(BP_IS_HOLE(*bpp)); return (0); } else { /* the block is referenced from the dnode */