c/mirror_zfs
1
0
Fork 0
mirror of https://git.proxmox.com/git/mirror_zfs.git synced 2024-12-24 18:39:34 +03:00
Code Issues Packages Projects Releases Wiki Activity

ddt_addref: remove unnecessary phys fill when refcount is 0

Browse Source 
The previous comment wondered if this case could happen; it turns out
that it really can't.

This block can only be entered if dde_type and dde_class are "real";
that only happens when a ddt entry has been previously synced to a ddt
store, that is, it was created on a previous txg. Since its gone through
that sync, its dde_refcount must be >0.

ddt_addref() is called from brt_pending_apply(), which is called at the
beginning of spa_sync(), before pending DMU writes/frees are issued.
Freeing a dedup block is the only thing that can decrement dde_refcount,
so there's no way for it to drop to zero before applying the clone bumps
it.

Further, even if it _could_ go to zero, it wouldn't be necessary to fill
the entry from the block. The phys content is not cleared until the free
is issued, which happens when the refcount goes to zero, when the last
real free comes through. The cloned block should be identical to what's
in the phys already, so the fill should be a no-op anyway.

I've replaced this with an assertion because this is all very dependent
on the ordering in which BRT and DDT changes are applied, and that might
change in the future.

Reviewed-by: Brian Behlendorf <behlendorf1@llnl.gov>
Signed-off-by: Rob Norris <rob.norris@klarasystems.com>
Sponsored-By: Klara, Inc.
Closes #15004
This commit is contained in:
Rob N 2023-07-01 02:01:58 +10:00 committed by GitHub
parent 233425a153
commit 61ab05cac7
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
1 changed files with 13 additions and 4 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

17
module/zfs/ddt.c
View File

@ -1209,10 +1209,19 @@ ddt_addref(spa_t *spa, const blkptr_t *bp)
ASSERT3S(dde->dde_class, <, DDT_CLASSES);
ddp = &dde->dde_phys[BP_GET_NDVAS(bp)];
if (ddp->ddp_refcnt == 0) {
/* This should never happen? */
ddt_phys_fill(ddp, bp);
}
/*
* This entry already existed (dde_type is real), so it must
* have refcnt >0 at the start of this txg. We are called from
* brt_pending_apply(), before frees are issued, so the refcnt
* can't be lowered yet. Therefore, it must be >0. We assert
* this because if the order of BRT and DDT interactions were
* ever to change and the refcnt was ever zero here, then
* likely further action is required to fill out the DDT entry,
* and this is a place that is likely to be missed in testing.
*/
ASSERT3U(ddp->ddp_refcnt, >, 0);
ddt_phys_addref(ddp);
result = B_TRUE;
} else {