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

[Andriy Gapon]

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