From owner-svn-src-head@FreeBSD.ORG Mon Nov 17 16:14:04 2014 Return-Path: Delivered-To: svn-src-head@freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:1900:2254:206a::19:1]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by hub.freebsd.org (Postfix) with ESMTPS id 3C418B8E for ; Mon, 17 Nov 2014 16:14:04 +0000 (UTC) Received: from mail-wi0-f180.google.com (mail-wi0-f180.google.com [209.85.212.180]) (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 C4814E6A for ; Mon, 17 Nov 2014 16:14:02 +0000 (UTC) Received: by mail-wi0-f180.google.com with SMTP id n3so2262407wiv.7 for ; Mon, 17 Nov 2014 08:14:01 -0800 (PST) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20130820; h=x-gm-message-state:message-id:date:from:user-agent:mime-version:to :subject:references:in-reply-to:content-type :content-transfer-encoding; bh=B5szRIx2USh2sW74sGOlu7w/XvjW1ORHfKnnoXL6Z0s=; b=JjoMY9ZR7iVW7m571fmg3BYhQVIEMW397xKK3xgau/kNG/FPu/db4In0WyqQwkaySN 7olhOkHfwiGozuDQsAy+9mGRymyUD/WyA7lg3mgPkjzTtkvw5/agzXNuHBuiNvZ1Z8aa P9aJqUvTjD1xjtx1+/LzB3m+JTe7XvXwoJsg8AFZNFh+yxgQ6P2LitLdYXqOrzsf6tRl kzSGZioUOGfikxXx4ibiKUJALdGISTeseJGBiiF6OkFd0B2dvVH6D9ll1vSN6RWoWUmH Er3jioM+hL2j4CBhBNEIyhdAYedMqu+xDtTRv4rfoXQrOG0yueJah/IZozFrOiTOVtX+ aULQ== X-Gm-Message-State: ALoCoQnxSmk8JyFVDVyG8xH4CGan+fl1ikzoIVfuflQU4/fYkWARfuhk3s/kr3yLZfDUACzMZoob X-Received: by 10.180.10.231 with SMTP id l7mr32608170wib.1.1416240841201; Mon, 17 Nov 2014 08:14:01 -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 p8sm19143187wia.1.2014.11.17.08.14.00 for (version=TLSv1.2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Mon, 17 Nov 2014 08:14:00 -0800 (PST) Message-ID: <546A1ECC.7000301@multiplay.co.uk> Date: Mon, 17 Nov 2014 16:14:04 +0000 From: Steven Hartland 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> <546A0ED8.6020104@freebsd.org> <546A0F1E.5050305@FreeBSD.org> In-Reply-To: <546A0F1E.5050305@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 16:14:04 -0000 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; >>> >