On 17/11/2014 17:06, Steven Hartland wrote: > Looks like this could have introduced random data corruption, have you seen > this > in practice?
Which part exactly? > On 17/11/2014 14:45, Andriy Gapon wrote: >> Author: avg >> Date: Mon Nov 17 14:45:42 2014 >> New Revision: 274628 >> URL: https://svnweb.freebsd.org/changeset/base/274628 >> >> Log: >> l2arc: restore correct rounding up of asize of compressed data >> This rounding up was lost in a mismerge of illumos code. >> See r268075 MFV r267565. >> After that commit zio_compress_data() no longer performs any compressed >> size adjustment, so it needs to be done externally. On FreeBSD we round >> up the size using vdev_ashift rather than SPA_MINBLOCKSIZE so that 4KB >> devices are properly supported. >> Additionally, zero out the buffer tail only if compression succeeds. >> The compression is considered successful if the size of compressed >> data after rounding up to account for the vdev ashift is less than the >> original data size. It does not make sense to have the data compressed >> if all the savings are lost to rounding up. >> With the new zio_compress_data() it could have been possible that the >> rounded compressed size would be greater than the original size and thus >> we could zero beyond the allocated buffer if the zeroing code was kept >> at the original place. >> Discussed with: delphij, gibbs >> MFC after: 2 weeks >> X-MFC with: r274627 >> >> Modified: >> head/sys/cddl/contrib/opensolaris/uts/common/fs/zfs/arc.c >> >> Modified: head/sys/cddl/contrib/opensolaris/uts/common/fs/zfs/arc.c >> ============================================================================== >> --- head/sys/cddl/contrib/opensolaris/uts/common/fs/zfs/arc.c Mon Nov 17 >> 14:16:02 2014 (r274627) >> +++ head/sys/cddl/contrib/opensolaris/uts/common/fs/zfs/arc.c Mon Nov 17 >> 14:45:42 2014 (r274628) >> @@ -5326,12 +5326,6 @@ l2arc_compress_buf(l2arc_buf_hdr_t *l2hd >> csize = zio_compress_data(ZIO_COMPRESS_LZ4, l2hdr->b_tmp_cdata, >> cdata, l2hdr->b_asize); >> - rounded = P2ROUNDUP(csize, (size_t)SPA_MINBLOCKSIZE); >> - if (rounded > csize) { >> - bzero((char *)cdata + csize, rounded - csize); >> - csize = rounded; >> - } >> - >> if (csize == 0) { >> /* zero block, indicate that there's nothing to write */ >> zio_data_buf_free(cdata, len); >> @@ -5340,11 +5334,19 @@ l2arc_compress_buf(l2arc_buf_hdr_t *l2hd >> l2hdr->b_tmp_cdata = NULL; >> ARCSTAT_BUMP(arcstat_l2_compress_zeros); >> return (B_TRUE); >> - } else if (csize > 0 && csize < len) { >> + } >> + >> + rounded = P2ROUNDUP(csize, >> + (size_t)1 << l2hdr->b_dev->l2ad_vdev->vdev_ashift); >> + if (rounded < len) { >> /* >> * Compression succeeded, we'll keep the cdata around for >> * writing and release it afterwards. >> */ >> + if (rounded > csize) { >> + bzero((char *)cdata + csize, rounded - csize); >> + csize = rounded; >> + } >> l2hdr->b_compress = ZIO_COMPRESS_LZ4; >> l2hdr->b_asize = csize; >> l2hdr->b_tmp_cdata = cdata; >> > -- Andriy Gapon _______________________________________________ svn-src-head@freebsd.org mailing list http://lists.freebsd.org/mailman/listinfo/svn-src-head To unsubscribe, send any mail to "svn-src-head-unsubscr...@freebsd.org"
