dmu_send: a meta-dnode range does not start where an object does

send_range_start_compare() positions a meta-dnode range, whose blkids
count dnode blocks, at the object its first dnode block covers:
objequiv becomes start_blkid * DNODES_PER_BLOCK and l0equiv is forced
to zero.  That places it correctly against the objects it covers, but
it also makes it compare equal to the first block of the object at
that exact boundary, because both sides then have the same objequiv
and an l0equiv of zero.  The two do not start at the same place; their
blkids are not even in the same units.

find_next_range() relies on that comparison meaning what it says.  The
loop which computes first_change skips DMU_META_DNODE_OBJECT, so a
meta-dnode range never lowers it, while the loop which then advances
every range that starts alongside the one being returned does not skip
it, and applies an object-relative blkid to a range counting dnode
blocks.  On a debug build that trips

  VERIFY3U(first_change, >, ranges[i]->start_blkid) failed (1 > 4)

with the meta-dnode range covering dnode block 4, which is object 128
at 32 dnode slots per block, and the redaction record for object 128
starting at block 0.  Without debug the assertion is compiled out, the
assignment below it still runs, and the range continues through the
merge with a start_blkid it does not own.

Whether that changes the emitted stream depends on where the objects
land.  In one run of the reporter's scenario a production build emits

  FREEOBJECTS firstobj = 32 numobjs = 224

where this change emits

  FREEOBJECTS firstobj = 160 numobjs = 96

object 32 being dnode block 1, the block the range was rewound to.
Other layouts which provoke the same tie emit an identical stream, and
no data loss has been demonstrated on a production build; the failure
this change fixes is the assertion above.

Order a meta-dnode range before the block ranges of the objects it
covers instead of reporting them equal, so that ranges only compare
equal when they genuinely share an object and a block.  The OBJECT and
OBJECT_RANGE tiebreaks above it are unchanged and still order those
records first.  The same distinction is applied in send_range_after(),
which asserts the ordering of consecutive merged records and would
otherwise compare those blkids across units as well.  That function is
reached only from an ASSERT3S() in get_next_range_nofree(), so it and
the invariant assert added to find_next_range() take effect on debug
builds only; the production change is the send_range_start_compare()
ordering alone.  Only the exact boundary changes; everywhere else the
comparison already ordered the meta-dnode range first.

Guarding the update loop alone is not enough.  It leaves the tie
broken by queue index, which resolves one way when the redaction list
is on the redact queue and the other when it arrives from a bookmark
on the from queue, and the second ordering then fails the merged
record assertion in get_next_range_nofree() instead.

Reaching this requires a redaction list, since the other two queues
are at end of stream for an ordinary send and an end of stream marker
never compares equal to a live range.  Both `zfs send --redact` and an
incremental from a redaction bookmark can hit it; a plain bookmark
carries no redaction object and leaves the from queue at end of
stream.

Reviewed-by: Brian Behlendorf <[email protected]>
Signed-off-by: Michael Heller <[email protected]>
Closes #18365
Closes #18901

module/zfs/dmu_send.c

@@ -1246,6 +1246,17 @@ send_range_after(const struct send_range *from, const struct send_range *to)
 	if (unlikely(cmp))
 		return (cmp);
 	cmp = TREE_CMP(to->type == OBJECT, from->type == OBJECT);
+	if (unlikely(cmp))
+		return (cmp);
+	/*
+	 * A meta-dnode range and an ordinary object's range express their
+	 * blkids in different units, dnode blocks against that object's data
+	 * blocks, so the blkid comparisons below cannot be applied to them.
+	 * Reaching here means their object ranges overlap; the meta-dnode
+	 * range sorts before the ranges of every object it covers, which is
+	 * the order send_range_start_compare() also establishes.
+	 */
+	cmp = TREE_CMP(to->object == 0, from->object == 0);
 	if (unlikely(cmp))
 		return (cmp);
 	if (from->end_blkid <= to->start_blkid)
@@ -1335,6 +1346,17 @@ send_range_start_compare(struct send_range *r1, struct send_range *r2)
 	if (unlikely(cmp))
 		return (cmp);
 	cmp = TREE_CMP(r2->type == OBJECT, r1->type == OBJECT);
+	if (unlikely(cmp))
+		return (cmp);
+	/*
+	 * A meta-dnode range covering dnode block b has the same objequiv as
+	 * the first block of object b * DNODES_PER_BLOCK, but the two do not
+	 * start at the same place: their blkids count different things.  The
+	 * merge in find_next_range() may only treat ranges as starting
+	 * together when they genuinely share an object and a block, so order
+	 * the meta-dnode range first rather than reporting them equal.
+	 */
+	cmp = TREE_CMP(r2->object == 0, r1->object == 0);
 	if (unlikely(cmp))
 		return (cmp);
 
@@ -1433,6 +1455,7 @@ find_next_range(struct send_range **ranges, bqueue_t **qs, uint64_t *out_mask)
 	for (i = 0; i < NUM_THREADS; i++) {
 		if (i == idx || (bmask & (1 << i)) == 0)
 			continue;
+		ASSERT3U(ranges[i]->object, ==, ranges[idx]->object);
 		ASSERT3U(first_change, >, ranges[i]->start_blkid);
 		ranges[i]->start_blkid = first_change;
 		ASSERT3U(ranges[i]->start_blkid, <=, ranges[i]->end_blkid);