svn commit: r274628 - head/sys/cddl/contrib/opensolaris/uts/common/fs/zfs

[Steven Hartland]

On 17/11/2014 15:07, Andriy Gapon wrote:
> 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?
The part where it zeroed beyond the allocated buffer.
>> 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;
>>>
>