From owner-svn-src-all@freebsd.org Sat Jul 4 19:02:10 2020 Return-Path: Delivered-To: svn-src-all@mailman.nyi.freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2610:1c1:1:606c::19:1]) by mailman.nyi.freebsd.org (Postfix) with ESMTP id D3945355E18; Sat, 4 Jul 2020 19:02:10 +0000 (UTC) (envelope-from cse.cem@gmail.com) Received: from mail-ot1-f44.google.com (mail-ot1-f44.google.com [209.85.210.44]) (using TLSv1.3 with cipher TLS_AES_128_GCM_SHA256 (128/128 bits) key-exchange X25519 server-signature RSA-PSS (4096 bits) server-digest SHA256 client-signature RSA-PSS (2048 bits) client-digest SHA256) (Client CN "smtp.gmail.com", Issuer "GTS CA 1O1" (verified OK)) by mx1.freebsd.org (Postfix) with ESMTPS id 49zh5t22rXz438R; Sat, 4 Jul 2020 19:02:10 +0000 (UTC) (envelope-from cse.cem@gmail.com) Received: by mail-ot1-f44.google.com with SMTP id 5so26835780oty.11; Sat, 04 Jul 2020 12:02:10 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:mime-version:references:in-reply-to:reply-to :from:date:message-id:subject:to:cc; bh=kAIPcqos0P65//5USrP/xzS1t1p4zztCxSsCDZOS9M4=; b=TdC1uOKq1nxzuaHkAkNSCzz96RzmYT58c4TyRmcWxzzbWm3DvA8Es1mzlYHU0pMcST Gc5z2qa16IfnltCo93UPtrN5eQLjDpM0Gy9ShrVLYLCaIvI7Vv9nrscv+bLB+0xEKBFN a9gEksfQIWEFEe6uArPLc7KtAmQQpcXE4xhHPOZoVXq9p8DWZfgzwwDEp/47eeLRRPwP l3XYuLRiKYXavurVi1QiW9Q9noX6PYYDg+Rwk5u++ZnjCyicddccqV67kN8qXkMTfFko 26qd6DSLVKLPok6byZedql6p8MrXBIDKN2+znRSWaw9lbipkX5OzeAL4NFKpCaCfFw7t E+7w== X-Gm-Message-State: AOAM530p9mSF+7exVUniiT3p5DIySUw+K2qzMXGsvuMaoQw3ort7i77J HkG9cbB2QwBFPYvZgY3HL31Yr+yg X-Google-Smtp-Source: ABdhPJy9wNNj8Ur8AGjgB9XTZMTKYa6JbvkXAmhH9E6d8j8esHqNqsox+BdKASusKS1jJMsUJsEvyg== X-Received: by 2002:a05:6830:138d:: with SMTP id d13mr9693538otq.298.1593889328319; Sat, 04 Jul 2020 12:02:08 -0700 (PDT) Received: from mail-ot1-f47.google.com (mail-ot1-f47.google.com. [209.85.210.47]) by smtp.gmail.com with ESMTPSA id l23sm4404114oot.41.2020.07.04.12.02.07 (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Sat, 04 Jul 2020 12:02:08 -0700 (PDT) Received: by mail-ot1-f47.google.com with SMTP id w17so20989712otl.4; Sat, 04 Jul 2020 12:02:07 -0700 (PDT) X-Received: by 2002:a9d:2224:: with SMTP id o33mr36663051ota.216.1593889327326; Sat, 04 Jul 2020 12:02:07 -0700 (PDT) MIME-Version: 1.0 References: <202007041837.064Ib4PL082461@repo.freebsd.org> In-Reply-To: <202007041837.064Ib4PL082461@repo.freebsd.org> Reply-To: cem@freebsd.org From: Conrad Meyer Date: Sat, 4 Jul 2020 12:01:56 -0700 X-Gmail-Original-Message-ID: Message-ID: Subject: Re: svn commit: r362936 - head/sbin/newfs_msdos To: Xin LI Cc: src-committers@freebsd.org, svn-src-all@freebsd.org, svn-src-head@freebsd.org X-Rspamd-Queue-Id: 49zh5t22rXz438R X-Spamd-Bar: ---- Authentication-Results: mx1.freebsd.org; none X-Spamd-Result: default: False [-4.00 / 15.00]; ASN(0.00)[asn:15169, ipnet:209.85.128.0/17, country:US]; TAGGED_FROM(0.00)[]; REPLY(-4.00)[] Content-Type: text/plain; charset="UTF-8" X-Content-Filtered-By: Mailman/MimeDel 2.1.33 X-BeenThere: svn-src-all@freebsd.org X-Mailman-Version: 2.1.33 Precedence: list List-Id: "SVN commit messages for the entire src tree \(except for " user" and " projects" \)" List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Sat, 04 Jul 2020 19:02:10 -0000 Hi Xin Li, Maybe we can use C11 static_assert instead of the CTASSERT array mechanism? Best, Conrad On Sat, Jul 4, 2020 at 11:37 Xin LI wrote: > Author: delphij > Date: Sat Jul 4 18:37:04 2020 > New Revision: 362936 > URL: https://svnweb.freebsd.org/changeset/base/362936 > > Log: > Gather writes to larger chunks (MAXPHYS) instead of issuing them in > sectors. > > On my SanDisk Cruzer Blade 16GB USB stick this made formatting much > faster: > > x before > + after > > +--------------------------------------------------------------------------+ > |+ > | > |+ > x | > |+ > x x| > |A > MA|| > > +--------------------------------------------------------------------------+ > N Min Max Median Avg > Stddev > x 3 15.89 16.38 16 16.09 > 0.2570992 > + 3 0.32 0.37 0.35 0.34666667 > 0.025166115 > Difference at 95.0% confidence > -15.7433 +/- 0.414029 > -97.8455% +/- 0.25668% > (Student's t, pooled s = 0.182665) > > Reviewed by: emaste > MFC after: 2 weeks > Differential Revision: https://reviews.freebsd.org/D24508 > > Modified: > head/sbin/newfs_msdos/mkfs_msdos.c > > Modified: head/sbin/newfs_msdos/mkfs_msdos.c > > ============================================================================== > --- head/sbin/newfs_msdos/mkfs_msdos.c Sat Jul 4 18:01:29 2020 > (r362935) > +++ head/sbin/newfs_msdos/mkfs_msdos.c Sat Jul 4 18:37:04 2020 > (r362936) > @@ -64,6 +64,7 @@ static const char rcsid[] = > > #define DOSMAGIC 0xaa55 /* DOS magic number */ > #define MINBPS 512 /* minimum bytes per sector */ > +#define MAXBPS 4096 /* maximum bytes per sector */ > #define MAXSPC 128 /* maximum sectors per cluster */ > #define MAXNFT 16 /* maximum number of FATs */ > #define DEFBLK 4096 /* default block size */ > @@ -77,6 +78,25 @@ static const char rcsid[] = > #define MAXCLS16 0xfff4U /* maximum FAT16 clusters */ > #define MAXCLS32 0xffffff4U /* maximum FAT32 clusters */ > > +#ifndef CTASSERT > +#define CTASSERT(x) _CTASSERT(x, __LINE__) > +#define _CTASSERT(x, y) __CTASSERT(x, y) > +#define __CTASSERT(x, y) typedef char __assert_ ## y [(x) ? > 1 : -1] > +#endif > + > +/* > + * For better performance, we want to write larger chunks instead of > + * individual sectors (the size can only be 512, 1024, 2048 or 4096 > + * bytes). Assert that MAXPHYS can always hold an integer number of > + * sectors by asserting that both are power of two numbers and the > + * MAXPHYS is greater than MAXBPS. > + */ > +CTASSERT(powerof2(MAXPHYS)); > +CTASSERT(powerof2(MAXBPS)); > +CTASSERT(MAXPHYS > MAXBPS); > + > +const static ssize_t chunksize = MAXPHYS; > + > #define mincls(fat) ((fat) == 12 ? MINCLS12 : \ > (fat) == 16 ? MINCLS16 : \ > MINCLS32) > @@ -243,6 +263,7 @@ mkfs_msdos(const char *fname, const char *dtype, const > struct bsx *bsx; > struct de *de; > u_int8_t *img; > + u_int8_t *physbuf, *physbuf_end; > const char *bname; > ssize_t n; > time_t now; > @@ -252,7 +273,7 @@ mkfs_msdos(const char *fname, const char *dtype, const > int fd, fd1, rv; > struct msdos_options o = *op; > > - img = NULL; > + physbuf = NULL; > rv = -1; > fd = fd1 = -1; > > @@ -343,15 +364,13 @@ mkfs_msdos(const char *fname, const char *dtype, > const > bpb.bpbSecPerClust = 64; /* otherwise 32k */ > } > } > - if (!powerof2(bpb.bpbBytesPerSec)) { > - warnx("bytes/sector (%u) is not a power of 2", bpb.bpbBytesPerSec); > + if (bpb.bpbBytesPerSec < MINBPS || > + bpb.bpbBytesPerSec > MAXBPS || > + !powerof2(bpb.bpbBytesPerSec)) { > + warnx("Invalid bytes/sector (%u): must be 512, 1024, 2048 or 4096", > + bpb.bpbBytesPerSec); > goto done; > } > - if (bpb.bpbBytesPerSec < MINBPS) { > - warnx("bytes/sector (%u) is too small; minimum is %u", > - bpb.bpbBytesPerSec, MINBPS); > - goto done; > - } > > if (o.volume_label && !oklabel(o.volume_label)) { > warnx("%s: bad volume label", o.volume_label); > @@ -621,11 +640,14 @@ mkfs_msdos(const char *fname, const char *dtype, > const > tm = localtime(&now); > } > > - > - if (!(img = malloc(bpb.bpbBytesPerSec))) { > + physbuf = malloc(chunksize); > + if (physbuf == NULL) { > warn(NULL); > goto done; > } > + physbuf_end = physbuf + chunksize; > + img = physbuf; > + > dir = bpb.bpbResSectors + (bpb.bpbFATsecs ? bpb.bpbFATsecs : > bpb.bpbBigFATsecs) * bpb.bpbFATs; > memset(&si_sa, 0, sizeof(si_sa)); > @@ -750,19 +772,37 @@ mkfs_msdos(const char *fname, const char *dtype, > const > (u_int)tm->tm_mday; > mk2(de->deMDate, x); > } > - if ((n = write(fd, img, bpb.bpbBytesPerSec)) == -1) { > - warn("%s", fname); > - goto done; > + /* > + * Issue a write of chunksize once we have collected > + * enough sectors. > + */ > + img += bpb.bpbBytesPerSec; > + if (img >= physbuf_end) { > + n = write(fd, physbuf, chunksize); > + if (n != chunksize) { > + warnx("%s: can't write sector %u", fname, lsn); > + goto done; > + } > + img = physbuf; > } > - if ((unsigned)n != bpb.bpbBytesPerSec) { > - warnx("%s: can't write sector %u", fname, lsn); > - goto done; > - } > } > + /* > + * Write remaining sectors, if the last write didn't end > + * up filling a whole chunk. > + */ > + if (img != physbuf) { > + ssize_t tailsize = img - physbuf; > + > + n = write(fd, physbuf, tailsize); > + if (n != tailsize) { > + warnx("%s: can't write sector %u", fname, lsn); > + goto done; > + } > + } > } > rv = 0; > done: > - free(img); > + free(physbuf); > if (fd != -1) > close(fd); > if (fd1 != -1) >