From owner-svn-src-head@freebsd.org Fri Feb 19 06:12:57 2016 Return-Path: Delivered-To: svn-src-head@mailman.ysv.freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:1900:2254:206a::19:1]) by mailman.ysv.freebsd.org (Postfix) with ESMTP id 7BCB9AACB85 for ; Fri, 19 Feb 2016 06:12:57 +0000 (UTC) (envelope-from sobomax@sippysoft.com) Received: from mail-wm0-x233.google.com (mail-wm0-x233.google.com [IPv6:2a00:1450:400c:c09::233]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (Client CN "smtp.gmail.com", Issuer "Google Internet Authority G2" (verified OK)) by mx1.freebsd.org (Postfix) with ESMTPS id F3DE317A0 for ; Fri, 19 Feb 2016 06:12:56 +0000 (UTC) (envelope-from sobomax@sippysoft.com) Received: by mail-wm0-x233.google.com with SMTP id g62so54817287wme.0 for ; Thu, 18 Feb 2016 22:12:56 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=sippysoft-com.20150623.gappssmtp.com; s=20150623; h=mime-version:sender:in-reply-to:references:date:message-id:subject :from:to:cc; bh=m0xwitiG7BzDp40iyuM6B7AhbX2HZngdbWx1vcAGqq4=; b=ZJhIwDa8lecXVR7za67V/0ZbDrvG7bvJQGgkNWLwhNF+dOoSV+BBjY8n/xsK1SgoZ/ m5+1y1t2KeZ+eSTUeLi9t3KhTa9/fTRvddms+Ib7BYI9lJfVgf7ZOR76Mpl/eJ3B5OvM buc5c28NG9DSSQMYHNRx+qPk83yLuPgdrSRKHeLIddCgZm25EKeZFT+xLYkQLrQ22ADH bNyZrF1E0gO2e5ty59vYVYgghuvevxuE7sX+vnop0R4/l89uuFJZbYU68jRaloVp9GSF x0kFahUWi8ZMlfVlCZV1wCX/Ze7h0MiKrrg8dSQd/ufLRIP8Ki711kNutwu/NC5SfANc jyvg== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20130820; h=x-gm-message-state:mime-version:sender:in-reply-to:references:date :message-id:subject:from:to:cc; bh=m0xwitiG7BzDp40iyuM6B7AhbX2HZngdbWx1vcAGqq4=; b=aKFH5UyjlNu6zqGZBEc8i6X1PgATn/hVtBxSEaR1tf8RyxvsQu3Pj4zi4xASfac+j+ SeK+52secb9UTTro8qso3DRiy5xBNQOx8EGu7AGdCaozjbT0r62HZEkkSvZ77kuqAegq iiC9G1XVfI39C0VgTzAx8X1UFUEjIokOuraCbvzRavaaujmTj/MwjUUF1s1QHeomJ3z8 jyNppcQQLB4AUJJRgnlPFeA2IXxipXJzEgONMahrxaTqTpAXVu+y/WZMTZGyC8VyT/oo p67Tr1V2WdyzhvPVh4wp4SsxrNKHzkCTe1zPV/CGcaAFZtiL+ewwtb2uwKBpzRelCoEy oH7Q== X-Gm-Message-State: AG10YOR+xSZy6/TuSxF0aK0S387MhLpSFjgDlJBXXQpT48WoLNIHQhUsqW1X2vfCfX8aLWTITDf/VyQb3HZvG9qc MIME-Version: 1.0 X-Received: by 10.28.225.8 with SMTP id y8mr7636444wmg.94.1455862375314; Thu, 18 Feb 2016 22:12:55 -0800 (PST) Sender: sobomax@sippysoft.com Received: by 10.27.218.12 with HTTP; Thu, 18 Feb 2016 22:12:55 -0800 (PST) In-Reply-To: <201602180844.u1I8iGTN082641@repo.freebsd.org> References: <201602180844.u1I8iGTN082641@repo.freebsd.org> Date: Thu, 18 Feb 2016 22:12:55 -0800 X-Google-Sender-Auth: E0ydCEq61P9m4kTu26d_3k-IoZc Message-ID: Subject: Re: svn: head/bin/dd From: Maxim Sobolev To: Thomas Quinot Cc: src-committers@freebsd.org, svn-src-all@freebsd.org, svn-src-head@freebsd.org Content-Type: text/plain; charset=UTF-8 X-Content-Filtered-By: Mailman/MimeDel 2.1.20 X-BeenThere: svn-src-head@freebsd.org X-Mailman-Version: 2.1.20 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: Fri, 19 Feb 2016 06:12:57 -0000 Thanks, Thomas. IMHO this is quite serious bug in one of the core utilities, so I suggest it gets faster MFC before 10.3 goes into beta stage on Feb 26. The old code in RELENG_10 is just plain broken anyway now, so it could not make it any worse. Thanks! On Thu, Feb 18, 2016 at 12:44 AM, Thomas Quinot wrote: > Author: thomas > Date: Thu Feb 18 08:44:16 2016 > New Revision: 295749 > URL: https://svnweb.freebsd.org/changeset/base/295749 > > Log: > Reorganize the handling all-zeroes terminal block in sparse mode > > The intent of the previous code in that case was to force > an explicit write, but the implementation was incorrect, and > as a result the write was never performed. This new implementation > instead uses ftruncate(2) to extend the file with a trailing hole. > > Also introduce regression tests for these cases. > > PR: 189284 > (original PR whose fix introduced this bug) > > PR: 207092 > > Differential Revision: D5248 > Reviewed by: sobomax,kib > MFC after: 2 weeks > > Added: > head/bin/dd/ref.obs_zeroes (contents, props changed) > Modified: > head/bin/dd/Makefile > head/bin/dd/dd.c > head/bin/dd/dd.h > head/bin/dd/gen.c > > Modified: head/bin/dd/Makefile > > ============================================================================== > --- head/bin/dd/Makefile Thu Feb 18 07:44:14 2016 (r295748) > +++ head/bin/dd/Makefile Thu Feb 18 08:44:16 2016 (r295749) > @@ -24,7 +24,18 @@ test: ${PROG} gen > LC_ALL=en_US.US-ASCII hexdump -C | \ > diff -I FreeBSD - ${.CURDIR}/ref.${conv} > .endfor > - @rm -f gen > + @${ECHO} "testing sparse file (obs zeroes)" > + @./gen 189284 | ./dd ibs=16 obs=8 conv=sparse of=obs_zeroes 2> > /dev/null > + @hexdump -C obs_zeroes | diff -I FreeBSD - > ${.CURDIR}/ref.obs_zeroes > + > + @${ECHO} "testing spase file (all zeroes)" > + @./dd if=/dev/zero of=1M_zeroes bs=1048576 count=1 2> /dev/null > + @./dd if=1M_zeroes of=1M_zeroes.1 bs=1048576 conv=sparse 2> > /dev/null > + @./dd if=1M_zeroes of=1M_zeroes.2 bs=1048576 2> /dev/null > + @diff 1M_zeroes 1M_zeroes.1 > + @diff 1M_zeroes 1M_zeroes.2 > + > + @rm -f gen 1M_zeroes* obs_zeroes > > .if ${MK_TESTS} != "no" > SUBDIR+= tests > > Modified: head/bin/dd/dd.c > > ============================================================================== > --- head/bin/dd/dd.c Thu Feb 18 07:44:14 2016 (r295748) > +++ head/bin/dd/dd.c Thu Feb 18 08:44:16 2016 (r295749) > @@ -77,7 +77,6 @@ STAT st; /* statistics */ > void (*cfunc)(void); /* conversion function */ > uintmax_t cpy_cnt; /* # of blocks to copy */ > static off_t pending = 0; /* pending seek if sparse */ > -static off_t last_sp = 0; /* size of last added sparse block */ > u_int ddflags = 0; /* conversion options */ > size_t cbsz; /* conversion block size */ > uintmax_t files_cnt = 1; /* # of files to copy */ > @@ -409,6 +408,15 @@ dd_close(void) > } > if (out.dbcnt || pending) > dd_out(1); > + > + /* > + * If the file ends with a hole, ftruncate it to extend its size > + * up to the end of the hole (without having to write any data). > + */ > + if (out.seek_offset > 0 && (out.flags & ISTRUNC)) { > + if (ftruncate(out.fd, out.seek_offset) == -1) > + err(1, "truncating %s", out.name); > + } > } > > void > @@ -457,29 +465,27 @@ dd_out(int force) > } > if (sparse && !force) { > pending += cnt; > - last_sp = cnt; > nw = cnt; > } else { > if (pending != 0) { > - /* If forced to write, and we have > no > - * data left, we need to write the > last > - * sparse block explicitly. > + /* > + * Seek past hole. Note that we > need to record the > + * reached offset, because we > might have no more data > + * to write, in which case we'll > need to call > + * ftruncate to extend the file > size. > */ > - if (force && cnt == 0) { > - pending -= last_sp; > - assert(outp == out.db); > - memset(outp, 0, cnt); > - } > - if (lseek(out.fd, pending, > SEEK_CUR) == > - -1) > + out.seek_offset = lseek(out.fd, > pending, SEEK_CUR); > + if (out.seek_offset == -1) > err(2, "%s: seek error > creating sparse file", > out.name); > - pending = last_sp = 0; > + pending = 0; > } > - if (cnt) > + if (cnt) { > nw = write(out.fd, outp, cnt); > - else > + out.seek_offset = 0; > + } else { > return; > + } > } > > if (nw <= 0) { > > Modified: head/bin/dd/dd.h > > ============================================================================== > --- head/bin/dd/dd.h Thu Feb 18 07:44:14 2016 (r295748) > +++ head/bin/dd/dd.h Thu Feb 18 08:44:16 2016 (r295749) > @@ -54,6 +54,7 @@ typedef struct { > const char *name; /* name */ > int fd; /* file descriptor */ > off_t offset; /* # of blocks to skip */ > + off_t seek_offset; /* offset of last seek past output > hole */ > } IO; > > typedef struct { > > Modified: head/bin/dd/gen.c > > ============================================================================== > --- head/bin/dd/gen.c Thu Feb 18 07:44:14 2016 (r295748) > +++ head/bin/dd/gen.c Thu Feb 18 08:44:16 2016 (r295749) > @@ -5,13 +5,20 @@ > */ > > #include > +#include > > int > -main(int argc __unused, char **argv __unused) > +main(int argc, char **argv) > { > int i; > > - for (i = 0; i < 256; i++) > - putchar(i); > + if (argc > 1 && !strcmp(argv[1], "189284")) { > + fputs("ABCDEFGH", stdout); > + for (i = 0; i < 8; i++) > + putchar(0); > + } else { > + for (i = 0; i < 256; i++) > + putchar(i); > + } > return (0); > } > > Added: head/bin/dd/ref.obs_zeroes > > ============================================================================== > --- /dev/null 00:00:00 1970 (empty, because file is newly added) > +++ head/bin/dd/ref.obs_zeroes Thu Feb 18 08:44:16 2016 (r295749) > @@ -0,0 +1,3 @@ > +$FreeBSD$ > +00000000 41 42 43 44 45 46 47 48 00 00 00 00 00 00 00 00 > |ABCDEFGH........| > +00000010 > >