Wait for txg sync if the last DRR_FREEOBJECTS might result in a hole

[dhedberg]

Motivation, Context and Description

If we receive a DRR_FREEOBJECTS as the first entry in an object range, this might end up producing a hole if the freed objects were the only existing objects in the block.

If the txg starts syncing before we've processed any following DRR_OBJECT records, this leads to a possible race where the backing arc_buf_t gets its psize set to 0 in the arc_write_ready() callback while still being referenced from a dirty record in the open txg.

To prevent this, we insert a txg_wait_synced call if the first record in the range was a DRR_FREEOBJECTS that actually resulted in one or more freed objects.

Sponsored by: Findity AB
Closes: #11893

How Has This Been Tested?

The patch has been tested in a VM running a cloud image of Ubuntu 22.04 with kernel 5.15.0-56. The issue can be triggered either by simply by starting a few zfs receive/zfs rollback in a loop with an affected incremental stream and waiting, or by inserting a busy loop before dmu_tx_create() in object_receive():

     // Something like
     if (drro->drr_object == <some-affected-object>) {
         uint64_t wait_txg = 0;
         wait_txg = dmu_objset_spa(rwa->os)->spa_syncing_txg;
         while (dmu_objset_spa(rwa->os)->spa_syncing_txg <= wait_txg) {}
     }

The patch has been tested both by leaving the hacky trigger in place, and also by just letting the final patch run with a few receive threads looping for a while without detecting any issues.

Types of changes

• Bug fix (non-breaking change which fixes an issue)
• New feature (non-breaking change which adds functionality)
• Performance enhancement (non-breaking change which improves efficiency)
• Code cleanup (non-breaking change which makes code smaller or more readable)
• Breaking change (fix or feature that would cause existing functionality to change)
• Library ABI change (libzfs, libzfs_core, libnvpair, libuutil and libzfsbootenv)
• Documentation (a change to man pages or other documentation)

Checklist:

• My code follows the OpenZFS code style requirements.
• I have updated the documentation accordingly.
• I have read the contributing document.
• I have added tests to cover my changes.
• I have run the ZFS Test Suite with this change applied.
• All commit messages are properly formatted and contain Signed-off-by.

[behlendorf]

If you can construct a small stream which triggers this it would be nice to add a test case for this.

[dhedberg]

I have added a test with the potential of triggering the issue, which can be verified by running it without the fix applied and with the following hack in its stead:

diff --git a/module/zfs/dmu_recv.c b/module/zfs/dmu_recv.c
index f2ff415aa..39d8ae45f 100644
--- a/module/zfs/dmu_recv.c
+++ b/module/zfs/dmu_recv.c
@@ -1959,6 +1959,11 @@ receive_object(struct receive_writer_arg *rwa, struct drr_object *drro,
                        txg_wait_synced(dmu_objset_pool(rwa->os), 0);
        }

+        if (drro->drr_object == 129) {
+                uint64_t wait_txg = dmu_objset_spa(rwa->os)->spa_syncing_txg;
+                while (dmu_objset_spa(rwa->os)->spa_syncing_txg <= wait_txg) {}
+        }
+
        tx = dmu_tx_create(rwa->os);
        dmu_tx_hold_bonus(tx, object_to_hold);
        dmu_tx_hold_write(tx, object_to_hold, 0, 0);

Given the nature of the issue I'm not sure if there's a way to write a test that reliably (or perhaps even likely) triggers it under real world conditions, but this will at least exercise the related code paths.

Does this seem OK?

[behlendorf]

I think exercising the relevant code paths should be sufficient. It'd be nice to be able to hit it every time, but I agree it's probably not worth adding additional code to the kmod to be able to trigger exactly this issue. This looks good to me. Thanks.

[behlendorf]

I'm a little concerned this won't be entirely reliable since it depends on some assumptions about how object numbers are allocated. But as you pointed out we already do this same trick in send_freeobjects.ksh so I'm okay with doing the same thing here.