Bug #26650
Correctly report ZFS dataset quota overflows
Description
When writing to a particular ZFS dataset and that dataset being below quota when the files are initially opened for write, but reaching its quota while the clients are actively writing, eventually something in zfs slows to a crawl.
This is trivially reproducible. Running TrueNAS 11.0-U4
1.zfs create cargo/quota-test
2.zfs set compression=off cargo/quota-test
3.zfs set refquota=50m cargo/quota-test
4.cd /mnt/cargo/quota-test
5.dd if=/dev/zero of=file.out bs=1m
If you perform these steps, you will notice that the "dd" command will continue issuing writes to the dataset but will never stop even though the quota has been met. I have procstat -kk -a | fgrep dd and attached the output to this ticket.
NOTE I have tried this on proper freeBSD 11.0, 11.1, and CURRENT and all produce the same results.
| 1. start-dd.PNG (5.7 KB) 1. start-dd.PNG | |||
| 2. no-more-space.PNG (3.94 KB) 2. no-more-space.PNG | |||
| procstat (1.29 KB) procstat |
Associated revisions
make illumos uiocopy use vn_io_fault_uiomove
uiocopy() is currently unused, its purpose is copy data from a uio
without modifying the uio. It was in use before the vn_io_fault support
was added to ZFS, at which point our code diverged from the illumos code
a little bit. Because ZFS is the only (potential) user of the function
we are free to modify it to better suit ZFS needs.
The intention behind this change is to remove the differences introduced
earlier in zfs_write().
While here, re-implement uioskip() using uiomove() with
uio_segflg == UIO_NOCOPY.
The story of uioskip is the same as with uiocopy.
Reviewed by: mav
MFC after: 1 week
Ticket: #26650
(cherry picked from commit 0e4af542392ca3d738baa5191f0e842506a0d5f5)
zfs_write: fix problem with writes appearing to succeed when over quota
The problem happens when the writes have offsets and sizes aligned with
a filesystem's recordsize (maximum block size). In this scenario
dmu_tx_assign() would fail because of being over the quota, but the uio
would already be modified in the code path where we copy data from the
uio into a borrowed ARC buffer. That makes an appearance of a partial
write, so zfs_write() would return success and the uio would be modified
consistently with writing a single block.
That bug can result in a data loss because the writes over the quota
would appear to succeed while the actual data is being discarded.
This commit fixes the bug by ensuring that the uio is not changed until
after all error checks are done. To achieve that the code now uses
uiocopy() + uioskip() as in the original illumos design. We can do that
now that uiocopy() has been updated in r326067 to use
vn_io_fault_uiomove().
Reported by: mav
Analyzed by: mav
Reviewed by: mav
Pointyhat to: avg (myself)
MFC after: 1 week
X-MFC after: r326067
X-Erratum: wanted
Ticket: #26650
(cherry picked from commit 5c5e6af72cd1243dfc44da1dbb8ec8f0bda3cfd6)
zfs_write: fix problem with writes appearing to succeed when over quota
The problem happens when the writes have offsets and sizes aligned with
a filesystem's recordsize (maximum block size). In this scenario
dmu_tx_assign() would fail because of being over the quota, but the uio
would already be modified in the code path where we copy data from the
uio into a borrowed ARC buffer. That makes an appearance of a partial
write, so zfs_write() would return success and the uio would be modified
consistently with writing a single block.
That bug can result in a data loss because the writes over the quota
would appear to succeed while the actual data is being discarded.
This commit fixes the bug by ensuring that the uio is not changed until
after all error checks are done. To achieve that the code now uses
uiocopy() + uioskip() as in the original illumos design. We can do that
now that uiocopy() has been updated in r326067 to use
vn_io_fault_uiomove().
Reported by: mav
Analyzed by: mav
Reviewed by: mav
Pointyhat to: avg (myself)
MFC after: 1 week
X-MFC after: r326067
X-Erratum: wanted
Ticket: #26650
(cherry picked from commit 5c5e6af72cd1243dfc44da1dbb8ec8f0bda3cfd6)
make illumos uiocopy use vn_io_fault_uiomove
uiocopy() is currently unused, its purpose is copy data from a uio
without modifying the uio. It was in use before the vn_io_fault support
was added to ZFS, at which point our code diverged from the illumos code
a little bit. Because ZFS is the only (potential) user of the function
we are free to modify it to better suit ZFS needs.
The intention behind this change is to remove the differences introduced
earlier in zfs_write().
While here, re-implement uioskip() using uiomove() with
uio_segflg == UIO_NOCOPY.
The story of uioskip is the same as with uiocopy.
Reviewed by: mav
MFC after: 1 week
Ticket: #26650
(cherry picked from commit 0e4af542392ca3d738baa5191f0e842506a0d5f5)
(cherry picked from commit c601305d70ac23601972729521f2dabc3fa0a8ac)
zfs_write: fix problem with writes appearing to succeed when over quota
The problem happens when the writes have offsets and sizes aligned with
a filesystem's recordsize (maximum block size). In this scenario
dmu_tx_assign() would fail because of being over the quota, but the uio
would already be modified in the code path where we copy data from the
uio into a borrowed ARC buffer. That makes an appearance of a partial
write, so zfs_write() would return success and the uio would be modified
consistently with writing a single block.
That bug can result in a data loss because the writes over the quota
would appear to succeed while the actual data is being discarded.
This commit fixes the bug by ensuring that the uio is not changed until
after all error checks are done. To achieve that the code now uses
uiocopy() + uioskip() as in the original illumos design. We can do that
now that uiocopy() has been updated in r326067 to use
vn_io_fault_uiomove().
Reported by: mav
Analyzed by: mav
Reviewed by: mav
Pointyhat to: avg (myself)
MFC after: 1 week
X-MFC after: r326067
X-Erratum: wanted
Ticket: #26650
(cherry picked from commit 5c5e6af72cd1243dfc44da1dbb8ec8f0bda3cfd6)
(cherry picked from commit 2fc7a5c98fe68e41c1a34412beb2e65d8f2261ce)
make illumos uiocopy use vn_io_fault_uiomove
uiocopy() is currently unused, its purpose is copy data from a uio
without modifying the uio. It was in use before the vn_io_fault support
was added to ZFS, at which point our code diverged from the illumos code
a little bit. Because ZFS is the only (potential) user of the function
we are free to modify it to better suit ZFS needs.
The intention behind this change is to remove the differences introduced
earlier in zfs_write().
While here, re-implement uioskip() using uiomove() with
uio_segflg == UIO_NOCOPY.
The story of uioskip is the same as with uiocopy.
Reviewed by: mav
MFC after: 1 week
Ticket: #26650
(cherry picked from commit 0e4af542392ca3d738baa5191f0e842506a0d5f5)
(cherry picked from commit c601305d70ac23601972729521f2dabc3fa0a8ac)
zfs_write: fix problem with writes appearing to succeed when over quota
The problem happens when the writes have offsets and sizes aligned with
a filesystem's recordsize (maximum block size). In this scenario
dmu_tx_assign() would fail because of being over the quota, but the uio
would already be modified in the code path where we copy data from the
uio into a borrowed ARC buffer. That makes an appearance of a partial
write, so zfs_write() would return success and the uio would be modified
consistently with writing a single block.
That bug can result in a data loss because the writes over the quota
would appear to succeed while the actual data is being discarded.
This commit fixes the bug by ensuring that the uio is not changed until
after all error checks are done. To achieve that the code now uses
uiocopy() + uioskip() as in the original illumos design. We can do that
now that uiocopy() has been updated in r326067 to use
vn_io_fault_uiomove().
Reported by: mav
Analyzed by: mav
Reviewed by: mav
Pointyhat to: avg (myself)
MFC after: 1 week
X-MFC after: r326067
X-Erratum: wanted
Ticket: #26650
(cherry picked from commit 5c5e6af72cd1243dfc44da1dbb8ec8f0bda3cfd6)
(cherry picked from commit 2fc7a5c98fe68e41c1a34412beb2e65d8f2261ce)
History
#1
Updated by Alexander Motin 11 months ago
Updated by Alexander Motin - Subject changed from zfs refquota deadlock to ZFS dataset quota overflows not reported correctly
- Status changed from Unscreened to Fix In Progress
- Priority changed from Important to Critical
- Target version set to TrueNAS 11.1-U1
Investigation of the problem brought me to FreeBSD-specific change r298105 by avg@ on 2016-04-16. If quota overflow detected during write, the write will fail, but the error status can be lost, falsely reporting partial completion. As result written data are flowing to nowhere and indefinitely, as fast as CPU can handle the loop.
Updated by Dru Lavigne 
Updated by Nick Wolff 
make illumos uiocopy use vn_io_fault_uiomove
uiocopy() is currently unused, its purpose is copy data from a uio
without modifying the uio. It was in use before the vn_io_fault support
was added to ZFS, at which point our code diverged from the illumos code
a little bit. Because ZFS is the only (potential) user of the function
we are free to modify it to better suit ZFS needs.
The intention behind this change is to remove the differences introduced
earlier in zfs_write().
While here, re-implement uioskip() using uiomove() with
uio_segflg == UIO_NOCOPY.
The story of uioskip is the same as with uiocopy.
Reviewed by: mav
MFC after: 1 week
Ticket: #26650
(cherry picked from commit 0e4af542392ca3d738baa5191f0e842506a0d5f5)