From owner-svn-src-head@FreeBSD.ORG Mon Nov 17 15:06:08 2014 Return-Path: Delivered-To: svn-src-head@freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [8.8.178.115]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by hub.freebsd.org (Postfix) with ESMTPS id 1D3C818F for ; Mon, 17 Nov 2014 15:06:08 +0000 (UTC) Received: from mail-wi0-f177.google.com (mail-wi0-f177.google.com [209.85.212.177]) (using TLSv1 with cipher ECDHE-RSA-RC4-SHA (128/128 bits)) (Client CN "smtp.gmail.com", Issuer "Google Internet Authority G2" (verified OK)) by mx1.freebsd.org (Postfix) with ESMTPS id A143F11C for ; Mon, 17 Nov 2014 15:06:07 +0000 (UTC) Received: by mail-wi0-f177.google.com with SMTP id l15so9409819wiw.10 for ; Mon, 17 Nov 2014 07:06:05 -0800 (PST) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20130820; h=x-gm-message-state:from:message-id:date:user-agent:mime-version:to :subject:references:in-reply-to:content-type :content-transfer-encoding; bh=GnDdf2rdIYTJfRloTzkXi8DOKW6+Rs9Dws66OIpSct8=; b=ibUJVjm9CP1INXblZUOYwf02CqFI5kSDsjegmpC5dxL+I6Sol7gxRo+71GYvVBrW9m ER4i3O4i/KJedM1Ykw3FoIOpA2UalmZisFAIK4gLRmP52NjbfsloqHCDoMbwmRbliWqA XzNbeQ81cvLa6pIt2aJbuC4mE0dJKOMkr/PHv6ZSGfZ1jYJnUZG2mz5fgyTmzzX4UT2k XtAhoONiCXR/xXZXI8UGgk8W/uNncJtqDEaTi9XP32hqsnKv3b3w/omU9/we9YkGasek 8WMxKv8KTMFhu7YtTrzTH5noEZ8176DzSLBFCs/IrXqN2T+WoAnCYQKe47itTDqpOjZM Y6Fg== X-Gm-Message-State: ALoCoQn8yFy3k2xNrzfvEzwH5k3s/PeX4CFYxiBs3Iab/10BmoywPpEvCwjS3jJDTtoxbieOOLEs X-Received: by 10.194.87.34 with SMTP id u2mr34942205wjz.42.1416236765132; Mon, 17 Nov 2014 07:06:05 -0800 (PST) Received: from [10.10.1.68] (82-69-141-170.dsl.in-addr.zen.co.uk. [82.69.141.170]) by mx.google.com with ESMTPSA id s2sm15629437wia.3.2014.11.17.07.05.56 for (version=TLSv1.2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Mon, 17 Nov 2014 07:05:56 -0800 (PST) From: Steven Hartland X-Google-Original-From: Steven Hartland Message-ID: <546A0ED8.6020104@freebsd.org> Date: Mon, 17 Nov 2014 15:06:00 +0000 User-Agent: Mozilla/5.0 (Windows NT 5.1; rv:31.0) Gecko/20100101 Thunderbird/31.2.0 MIME-Version: 1.0 To: Andriy Gapon , src-committers@freebsd.org, svn-src-all@freebsd.org, svn-src-head@freebsd.org Subject: Re: svn commit: r274628 - head/sys/cddl/contrib/opensolaris/uts/common/fs/zfs References: <201411171445.sAHEjgbE096937@svn.freebsd.org> In-Reply-To: <201411171445.sAHEjgbE096937@svn.freebsd.org> Content-Type: text/plain; charset=utf-8; format=flowed Content-Transfer-Encoding: 7bit X-BeenThere: svn-src-head@freebsd.org X-Mailman-Version: 2.1.18-1 Precedence: list List-Id: SVN commit messages for the src tree for head/-current List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Mon, 17 Nov 2014 15:06:08 -0000 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; >