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

[Steven Hartland]

Looks like this could have introduced random data corruption, have you 
seen this in practice?

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;
>