HomeFreeBSD

dnode_is_dirty: check dnode and its data for dirtiness

Description

dnode_is_dirty: check dnode and its data for dirtiness

Over its history this the dirty dnode test has been changed between
checking for a dnodes being on os_dirty_dnodes (dn_dirty_link) and
dn_dirty_record.

  de198f2d9 Fix lseek(SEEK_DATA/SEEK_HOLE) mmap consistency
  2531ce372 Revert "Report holes when there are only metadata changes"
  ec4f9b8f3 Report holes when there are only metadata changes
  454365bba Fix dirty check in dmu_offset_next()
  66aca2473 SEEK_HOLE should not block on txg_wait_synced()

Also illumos/illumos-gate@c543ec060d illumos/illumos-gate@2bcf0248e9

It turns out both are actually required.

In the case of appending data to a newly created file, the dnode proper
is dirtied (at least to change the blocksize) and dirty records are
added. Thus, a single logical operation is represented by separate
dirty indicators, and must not be separated.

The incorrect dirty check becomes a problem when the first block of a
file is being appended to while another process is calling lseek to skip
holes. There is a small window where the dnode part is undirtied while
there are still dirty records. In this case, lseek(fd, 0, SEEK_DATA)
would not know that the file is dirty, and would go to
dnode_next_offset(). Since the object has no data blocks yet, it
returns ESRCH, indicating no data found, which results in ENXIO
being returned to lseek()'s caller.

Since coreutils 9.2, cp performs sparse copies by default, that is, it
uses SEEK_DATA and SEEK_HOLE against the source file and attempts to
replicate the holes in the target. When it hits the bug, its initial
search for data fails, and it goes on to call fallocate() to create a
hole over the entire destination file.

This has come up more recently as users upgrade their systems, getting
OpenZFS 2.2 as well as a newer coreutils. However, this problem has been
reproduced against 2.1, as well as on FreeBSD 13 and 14.

This change simply updates the dirty check to check both types of dirty.
If there's anything dirty at all, we immediately go to the "wait for
sync" stage, It doesn't really matter after that; both changes are on
disk, so the dirty fields should be correct.

Sponsored-by: Klara, Inc.
Sponsored-by: Wasabi Technology, Inc.
Reviewed-by: Brian Behlendorf <behlendorf1@llnl.gov>
Reviewed-by: Alexander Motin <mav@FreeBSD.org>
Reviewed-by: Rich Ercolani <rincebrain@gmail.com>
Signed-off-by: Rob Norris <rob.norris@klarasystems.com>
Closes #15571
Closes #15526

Details

Provenance
rob.norris_klarasystems.comAuthored on Nov 28 2023, 5:07 PM
GitHub <noreply@github.com>Committed on Nov 28 2023, 5:07 PM
Parents
rGacb33ee1c169: FreeBSD: Fix ZFS so that snapshots under .zfs/snapshot are NFS visible
Branches
Unknown
Tags
Unknown