Rob's feedback

Signed-off-by: Paul Dagnelie <[email protected]>

Author: pcd1193182

include/sys/zio.h

@@ -59,21 +59,37 @@ typedef struct zio_eck {
 
 /*
  * Gang block headers are self-checksumming and contain an array
- * of block pointers.
+ * of block pointers. The old gang block size has enough room for 3 blkptrs,
+ * while new gang blocks can store more.
+ *
+ * Layout:
+ * +--------+--------+--------+-----+---------+-----------+
+ * |        |        |        |     |         |           |
+ * | blkptr | blkptr | blkptr | ... | padding | zio_eck_t |
+ * |   1    |   2    |   3    |     |         |           |
+ * +--------+--------+--------+-----+---------+-----------+
+ *   128B     128B     128B             88B        40B
  */
 #define	SPA_OLD_GANGBLOCKSIZE	SPA_MINBLOCKSIZE
 typedef void zio_gbh_phys_t;
 
 static inline uint64_t
 gbh_nblkptrs(uint64_t size) {
+	ASSERT(ISP2(size));
 	return ((size - sizeof (zio_eck_t)) / sizeof (blkptr_t));
 }
 
 static inline zio_eck_t *
 gbh_eck(zio_gbh_phys_t *gbh, uint64_t size) {
+	ASSERT(ISP2(size));
 	return ((zio_eck_t *)((uintptr_t)gbh + size - sizeof (zio_eck_t)));
 }
 
+static inline blkptr_t *
+gbh_bp(zio_gbh_phys_t *gbh, int bp) {
+	return (&((blkptr_t *)gbh)[bp]);
+}
+
 enum zio_checksum {
 	ZIO_CHECKSUM_INHERIT = 0,
 	ZIO_CHECKSUM_ON,

module/zfs/zio.c

@@ -2751,13 +2751,12 @@ zio_resume_wait(spa_t *spa)
  *
  * A gang block consists of a a gang header and up to gbh_nblkptrs(size)
  * gang members. The gang header is like an indirect block: it's an array
- * of block pointers, though the header has a small tail (zio_gb_tail_t)
- * that stores a version number (for future compatibility) and an embedded
- * checksum. It is allocated using only a single sector as the requested
- * size, and hence is allocatable regardless of fragmentation. Its size
- * is determined by the smallest allocatable asize of the vdevs it was
- * allocated on. The gang header's bps point to its gang members,
- * which hold the data.
+ * of block pointers, though the header has a small tail (a zio_eck_t)
+ * that stores an embedded checksum. It is allocated using only a single
+ * sector as the requested size, and hence is allocatable regardless of
+ * fragmentation. Its size is determined by the smallest allocatable
+ * asize of the vdevs it was allocated on. The gang header's bps point
+ * to its gang members, which hold the data.
  *
  * Gang blocks are self-checksumming, using the bp's <vdev, offset, txg>
  * as the verifier to ensure uniqueness of the SHA256 checksum.
@@ -3010,7 +3009,7 @@ zio_gang_tree_assemble_done(zio_t *zio)
 	abd_free(zio->io_abd);
 
 	for (int g = 0; g < gbh_nblkptrs(gn->gn_gangblocksize); g++) {
-		blkptr_t *gbp = &((blkptr_t *)gn->gn_gbh)[g];
+		blkptr_t *gbp = gbh_bp(gn->gn_gbh, g);
 		if (!BP_IS_GANG(gbp))
 			continue;
 		zio_gang_tree_assemble(gio, gbp, &gn->gn_child[g]);
@@ -3039,7 +3038,7 @@ zio_gang_tree_issue(zio_t *pio, zio_gang_node_t *gn, blkptr_t *bp, abd_t *data,
 		    gn->gn_gangblocksize)->zec_magic, ==, ZEC_MAGIC);
 
 		for (int g = 0; g < gbh_nblkptrs(gn->gn_gangblocksize); g++) {
-			blkptr_t *gbp = &((blkptr_t *)gn->gn_gbh)[g];
+			blkptr_t *gbp = gbh_bp(gn->gn_gbh, g);
 			if (BP_IS_HOLE(gbp))
 				continue;
 			zio_gang_tree_issue(zio, gn->gn_child[g], gbp, data,
@@ -4399,7 +4398,7 @@ zio_dva_unallocate(zio_t *zio, zio_gang_node_t *gn, blkptr_t *bp)
 	if (gn != NULL) {
 		for (int g = 0; g < gbh_nblkptrs(gn->gn_gangblocksize); g++) {
 			zio_dva_unallocate(zio, gn->gn_child[g],
-			    &((blkptr_t *)gn->gn_gbh)[g]);
+			    gbh_bp(gn->gn_gbh, g));
 		}
 	}
 }
@@ -5344,6 +5343,7 @@ zio_dva_throttle_done(zio_t *zio)
 	 * Parents of gang children can have two flavors -- ones that allocated
 	 * the gang header (will have ZIO_FLAG_IO_REWRITE set) and ones that
 	 * allocated the constituent blocks.  The first use their parent as tag.
+	 * We set the size to match the original allocation call for that case.
 	 */
 	if (pio->io_child_type == ZIO_CHILD_GANG &&
 	    (pio->io_flags & ZIO_FLAG_IO_REWRITE)) {