----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.csiden.org/r/229/#review764 -----------------------------------------------------------
Ship it! Ship It! - George Wilson On June 25, 2015, 12:34 p.m., Andriy Gapon wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.csiden.org/r/229/ > ----------------------------------------------------------- > > (Updated June 25, 2015, 12:34 p.m.) > > > Review request for OpenZFS Developer Mailing List, Justin Gibbs, George > Wilson, Matthew Ahrens, and Saso Kiselkov. > > > Bugs: 5219 > https://www.illumos.org/issues/5219 > > > Repository: illumos-gate > > > Description > ------- > > **NOTE: this review request supercedes > [#112](https://reviews.csiden.org/r/112/).** > > If we don't account for that, then we might end up overwriting disk > area of buffers that have not been evicted yet, because l2arc_evict > operates in terms of disk addresses. > > The discrepancy between the write size calculation and the actual increment > to l2ad_hand was introduced in > commit > [aad02571bc59671aa3103bb070ae365f531b0b62](https://github.com/illumos/illumos-gate/commit/aad02571bc59671aa3103bb070ae365f531b0b62) > > The change that introduced l2ad_hand alignment was almost correct > as the write size was accumulated as a sum of rounded buffer sizes. > See commit > [e14bb3258d05c1b1077e2db7cf77088924e56919](https://github.com/illumos/illumos-gate/commit/e14bb3258d05c1b1077e2db7cf77088924e56919) > > Also, we now consistently use asize / a_sz for the allocated size and > psize / p_sz for the physical size. > The latter accounts for a possible size reduction because of the compression, > whereas the former accounts for a possible subsequent size expansion > because of the alignment requirements. > > The code still assumes that either underlying storage subsystems or > hardware is able to do read-modify-write when an L2ARC buffer size is > not a multiple of a disk's block size. This is true for 4KB sector disks > that provide 512B sector emulation, but may not be true in general. > In other words, we currently do not have any code to make sure that > an L2ARC buffer, whether compressed or not, which is used for physical I/O > has a suitable size. > > Note that currently the cache device utilization is calculated based > on the physical size, not the allocated size. The same applies to l2_asize > kstat. That is wrong, but this commit does not fix that. > The accounting problem was introduced partially in > commit > [aad02571bc59671aa3103bb070ae365f531b0b62](https://github.com/illumos/illumos-gate/commit/aad02571bc59671aa3103bb070ae365f531b0b62) > and partially in > [3038a2b421b40dc5ac11cd88423696618584f85a](https://github.com/illumos/illumos-gate/commit/3038a2b421b40dc5ac11cd88423696618584f85a) > (accounting became consistent but in favour of the wrong size). > > > Diffs > ----- > > usr/src/uts/common/fs/zfs/arc.c 09d2e1dd8fdf6648d863d4c036c16b3f7981dbf5 > > Diff: https://reviews.csiden.org/r/229/diff/ > > > Testing > ------- > > FreeBSD and Linux. > > > Thanks, > > Andriy Gapon > >
_______________________________________________ developer mailing list developer@open-zfs.org http://lists.open-zfs.org/mailman/listinfo/developer
