[after ARC lock-contention fixes] account for ashift when gathering buffers to be written to l2arc device #3491
+35
−12
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
kernelOfTruth
changed the title
kernelOfTruth
changed the title
kernelOfTruth
changed the title
kernelOfTruth
changed the title
|
hmpf - made a mistake |
|
/me slaps myself in the face: confusion ensues when deliberately naming the correct branch wrongly 👊 |
kernelOfTruth
changed the title
This was referenced Jun 12, 2015
kernelOfTruth
force-pushed
the
zfs_master_11.06.2015_FreeBSD_D2764
branch
2 times, most recently
from
85c6e40 to
a538055
Compare
all the things 👍 |
kernelOfTruth
force-pushed
the
zfs_master_11.06.2015_FreeBSD_D2764
branch
from
a538055 to
fbca915
Compare
The problem is that since OpenSolaris commit illumos/illumos-gate@e14bb32 l2ad_hand is kept aligned based on ashift (which is derived from the cache device's logical and physical block sizes). So, the hand could be advanced by more than the sum of b_asize-s of the written L2ARC buffers. This is because b_asize is a misnomer at the moment as it does not always represent the allocated size: if a buffer is compressed, then the compressed size is properly rounded, but if the compression fails or it is not applied, then the original size is kept and it could be smaller than what ashift requires. For the same reasons arcstat_l2_asize and the reported used space on the cache device could be smaller than the actual allocated size if ashift > 9. That problem is not fixed by this change. This change only ensures that l2ad_hand is not advanced by more than target_sz. Otherwise we would overwrite active (unevicted) L2ARC buffers. That problem is manifested as growing l2_cksum_bad and l2_io_error counters. This change also changes 'p' prefix to 'a' prefix in a few places where variables represent allocated rather than physical size. The resolved problem may also result in the reported allocated size being greater than the cache device's capacity, because of the overwritten buffers (more than one buffer claiming the same disk space). PR: 198242 PR: 195746 (possibly related) Porting notes: Rather difficult to track changes related to: Illumos 5369 - arc flags should be an enum and Illumos 5408 - managing ZFS cache devices requires lots of RAM hdr->b_l2hdr = l2hdr; changed to hdr->b_flags |= ARC_FLAG_HAS_L2HDR; list_insert_head(dev->l2ad_buflist, hdr); changed to list_insert_head(&dev->l2ad_buflist, hdr); Account for the error message: error: ISO C90 forbids mixed declarations and code [-Werror=declaration-after-statement] uint64_t stats_size = 0; References: https://reviews.freebsd.org/D2764 openzfs#3400 openzfs#3433 openzfs#3451 Ported by: kernelOfTruth <kerneloftruth@gmail.com>
This was referenced Jun 16, 2015
|
@kernelOfTruth thanks for keeping on top of this issue. I've refreshed the patch in #3521 in preparation to merge this. I just adjusted the commit message slightly. |
|
@behlendorf Awesome ! Thanks for picking it up =) |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
The problem is that since OpenSolaris commit
illumos/illumos-gate@e14bb32
l2ad_hand is kept aligned based on ashift (which is derived from
the cache device's logical and physical block sizes).
So, the hand could be advanced by more than the sum of b_asize-s
of the written L2ARC buffers. This is because b_asize is a misnomer
at the moment as it does not always represent the allocated size:
if a buffer is compressed, then the compressed size is properly rounded,
but if the compression fails or it is not applied, then the original
size is kept and it could be smaller than what ashift requires.
For the same reasons arcstat_l2_asize and the reported used space
on the cache device could be smaller than the actual allocated size
if ashift > 9. That problem is not fixed by this change.
This change only ensures that l2ad_hand is not advanced by more
than target_sz. Otherwise we would overwrite active (unevicted)
L2ARC buffers. That problem is manifested as growing l2_cksum_bad
and l2_io_error counters.
This change also changes 'p' prefix to 'a' prefix in a few places
where variables represent allocated rather than physical size.
The resolved problem may also result in the reported allocated size
being greater than the cache device's capacity, because of the
overwritten buffers (more than one buffer claiming the same disk
space).
PR: 198242
PR: 195746 (possibly related)
Porting notes:
Rather difficult to track changes related to:
Illumos 5369 - arc flags should be an enum
and
Illumos 5408 - managing ZFS cache devices requires lots of RAM
hdr->b_l2hdr = l2hdr;
changed to
hdr->b_flags |= ARC_FLAG_HAS_L2HDR;
list_insert_head(dev->l2ad_buflist, hdr);
changed to
list_insert_head(&dev->l2ad_buflist, hdr);
References:
https://reviews.freebsd.org/D2764
#3400
#3433
#3451
https://reviews.csiden.org/r/112/diff/1/#index_header [previous review]
Fixes (supposedly):
zfsonlinux#3114
zfsonlinux#3400
[and potentially several others]
Ported by: kernelOfTruth kerneloftruth@gmail.com