ahrens
commented
4 years ago Here's an idea of how this could be happening:
arc_write() does (annotated):
zio = zio_write(...,
abd_get_from_buf(buf->b_data, HDR_GET_LSIZE(hdr)), //abd
HDR_GET_LSIZE(hdr), //lsize
arc_buf_size(buf), //psizeIf this is a dedup,verify write, this zio will go through zio_ddt_collision().
The problem comes when we are writing already-compressed data, due to receiving from zfs send send --compressed. In this case, the abd that arc_write() creates is of size HDR_GET_LSIZE(hdr), the logical (uncompressed) size. But the buffer is actually only arc_buf_size(), which is the smaller, "physical", already-compressed size. So the abd is referencing past the end of the buffer, into potentially unmapped virtual addresses, but for the most part that doesn't matter because we don't read past the psize (arc_buf_size()).
However, zio_ddt_collision() has this code:
zio_t *lio = dde->dde_lead_zio[p];
...
return (lio->io_orig_size != zio->io_orig_size ||
abd_cmp(zio->io_orig_abd, lio->io_orig_abd) != 0);What's happening here is that we are doing a dedup,verify write, and we've found a hit in the DDT, so we are going to verify that the data referenced by the entry in the dedup table (the DDE) is the same as the data we are writing. Typically we will read the data off disk and then compare it to what we are writing. But in this case, we are still in the middle of writing out the block that the DDE will reference (in the currently-syncing txg, while we are writing in the next (open) txg). So rather than reading the data off disk, we compare with the data that the earlier zio is writing.
This code assumes that if the two zio's io_orig_size (i.e. psize) is the same, then their abd's will be the same size. In this case of arc_write() using a wrong-size abd for an already-compressed write, typically both the old and new zio's would have gone through the same code paths and so both of them would have io_orig_size being the compressed size and io_orig_abd->abd_size being the uncompressed size. This would lead to incorrectly thinking that there is a collision (same hash but different buffer contents) because we are comparing past the end of the actual (compressed) buffer.
I'm not yet sure how the later zio ends up with the correct (smaller, compressed) io_orig_abd->abd_size.
FYI, the earlier zio (dde_lead_zio) is created by zio_ddt_write (paraphrasing):
dde->dde_lead_zio[p] = zio_write(...,
zio->io_orig_abd, //abd
zio->io_orig_size, //lsize
zio->io_orig_size, //psizeSo it preserves the incorrect abd size. It does seem odd to me that it sets the lsize of the new zio to be the same as the psize, but maybe that doesn't matter.
Is there any chance you could get a crash dump that we could use to check if this hypothesis is correct, and see if we can explain the difference in abd sizes of the two zio's?
Incidentally, what is your use case for dedup=edonr,verify, as opposed to using a stronger hash function and not verifying? Dedup verification is not widely used and is a feature that we would potentially consider deprecating and eventually removing, so I'd like to understand if there are workloads for which it is much better than the alternatives.
dioni21
stale[bot]
scineram
System information
Everything compiled with configuration:
configure --enable-silent-rules --enable-dependency-tracking --enable-asan --enable-debuginfo --enable-debug --enable-debug-kmem --enable-debug-kmem-tracking --enable-code-coverageDescribe the problem you're observing
While doing a zfs send/recv, got a panic kernel message, and now almost everything zpool related is now frozen. zfs recv process is blocked in D state.
Describe how to reproduce the problem
Not sure yet. Why try again later.
Pool was created with the following command:
And some others dataset that were not yet initialized.
Command to transfer data:
The first transfer ran with any problems, but I think it has no deduped data at all.
While doing the second I decided to stop it and change the script. But the
zfs recvnever returned from the^CBTW:
tankis an old pool in a failing disk. Config difference is bsicallychecksum=onanddedup=verify, so I assume the "new" stuff isedonr.Will reboot now to see if I can solve this.
Include any warning/errors/backtraces from the system logs
Kernel logs from dmesg: