Page MenuHomeFreeBSD

l2arc: make sure that all writes honor ashift of a cache device
ClosedPublic

Authored by avg on Jun 12 2015, 12:27 PM.
Tags
None
Referenced Files
F103324765: D2789.id8858.diff
Sat, Nov 23, 1:54 PM
Unknown Object (File)
Oct 10 2024, 11:45 PM
Unknown Object (File)
Oct 9 2024, 10:22 AM
Unknown Object (File)
Oct 6 2024, 8:28 AM
Unknown Object (File)
Oct 6 2024, 3:59 AM
Unknown Object (File)
Oct 4 2024, 1:48 AM
Unknown Object (File)
Oct 3 2024, 2:01 AM
Unknown Object (File)
Oct 2 2024, 6:06 PM

Details

Summary

Previously uncompressed buffers did not obey that rule.

Type of b_asize is changed to uint64_t for consistency,
given that this is a zeta-byte filesystem.

l2arc_compress_buf is renamed to l2arc_transform_buf to better reflect
its new utility. Now not only we ensure that a compressed buffer has
a size aligned to ashift, but we also allocate a properly sized
temporary buffer if the original buffer is not compressed and it has
an odd size. This ensures that all I/O to the cache device is always
ashift-aligned, in terms of both a request offset and a request size.

If the aligned data is larger than the original data, then we have to use
a temporary buffer when reading it as well.

Also, enhance physical zio alignment checks using vdev_logical_ashift.
On FreeBSD we have this information, so we can make stricter assertions.

Diff Detail

Lint
No Lint Coverage
Unit
No Test Coverage

Event Timeline

avg retitled this revision from to l2arc: make sure that all writes honor ashift of a cache device.
avg edited the test plan for this revision. (Show Details)
avg updated this object.

Rebase after the recent massive imports from the upstream.
Plus the following small changes:

  • l2arc_transform_buf: tidy up the description
  • bump l2_compress_failures only if compression is actually requested
  • l2_padding_needed: count of uncompressed buffers that had to be padded
sys/cddl/contrib/opensolaris/uts/common/fs/zfs/arc.c
5468–5469

Logically we can assert rounded <= asize here but it's probably a good idea to use an explicit assertion here just in case?

sys/cddl/contrib/opensolaris/uts/common/fs/zfs/arc.c
5468–5469

I agree

  • arc_buf_l2_cdata_free: don't free b_tmp_cdata when l2hdr is not fully set up
  • l2arc: add a safety assertion
  • Merge branch 'master' into wip/l2arc-logical-ashift
  • arc_buf_l2_cdata_free: correct and simplify a comment
smh edited edge metadata.

One style(9) nit but otherwise this looks good to me.

sys/cddl/contrib/opensolaris/uts/common/fs/zfs/zio.c
2736–2739

style(9) these should be split to two lines ideally.

This revision is now accepted and ready to land.Jan 11 2016, 11:14 AM
mav edited edge metadata.

Except couple very small things it looks good to me.

sys/cddl/contrib/opensolaris/uts/common/fs/zfs/arc.c
5429–5430

... four possible outcomes:

5598

While it is probably not fatal here, it is somewhat odd to check for some field validity after reading it.

This revision was automatically updated to reflect the committed changes.