Skip site navigation (1)Skip section navigation (2)
Date:      Thu, 17 Jan 2019 12:23:12 -0800
From:      Conrad Meyer <cem@freebsd.org>
To:        Eugene Grosbein <eugen@freebsd.org>
Cc:        src-committers <src-committers@freebsd.org>, svn-src-all@freebsd.org,  svn-src-head@freebsd.org
Subject:   Re: svn commit: r343118 - in head/usr.sbin: . trim
Message-ID:  <CAG6CVpVBsFDgksLEWG_U%2B4k6tH3SnaZkNFgd77D8NfLDf21Pdg@mail.gmail.com>
In-Reply-To: <201901171808.x0HI80jC068564@repo.freebsd.org>
References:  <201901171808.x0HI80jC068564@repo.freebsd.org>

next in thread | previous in thread | raw e-mail | index | archive | help
Please back it out; stop attributing code review to "hackers@," which
can not (it's a list, not individuals) and did not review this
changeset; and put it on phabricator for actual review.  I think
summary backout is warranted on the basis of your handling of this
last time around and lack of attempt to solicit review for this time.

On Thu, Jan 17, 2019 at 10:08 AM Eugene Grosbein <eugen@freebsd.org> wrote:
>
> Author: eugen
> Date: Thu Jan 17 18:07:59 2019
> New Revision: 343118
> URL: https://svnweb.freebsd.org/changeset/base/343118
>
> Log:
>   Re-add new small tool trim(8) to delete contents for blocks
>   on devices using wear-leveling algorithms as a few weeks passed
>   after review and discussion of trim(8) ceased and
>   we still have no utility to perform the job.
>
>   Reviewed by:  hackers@
>   MFC after:    2 weeks
>
> Added:
>   head/usr.sbin/trim/
>   head/usr.sbin/trim/Makefile   (contents, props changed)
>   head/usr.sbin/trim/trim.8   (contents, props changed)
>   head/usr.sbin/trim/trim.c   (contents, props changed)
> Modified:
>   head/usr.sbin/Makefile
>
> Modified: head/usr.sbin/Makefile
> ==============================================================================
> --- head/usr.sbin/Makefile      Thu Jan 17 17:36:18 2019        (r343117)
> +++ head/usr.sbin/Makefile      Thu Jan 17 18:07:59 2019        (r343118)
> @@ -92,6 +92,7 @@ SUBDIR=       adduser \
>         tcpdrop \
>         tcpdump \
>         traceroute \
> +       trim \
>         trpt \
>         tzsetup \
>         ugidfw \
>
> Added: head/usr.sbin/trim/Makefile
> ==============================================================================
> --- /dev/null   00:00:00 1970   (empty, because file is newly added)
> +++ head/usr.sbin/trim/Makefile Thu Jan 17 18:07:59 2019        (r343118)
> @@ -0,0 +1,8 @@
> +# $FreeBSD$
> +
> +PROG=  trim
> +MAN=   trim.8
> +LIBADD=        util
> +LDFLAGS=       -lutil
> +
> +.include <bsd.prog.mk>
>
> Added: head/usr.sbin/trim/trim.8
> ==============================================================================
> --- /dev/null   00:00:00 1970   (empty, because file is newly added)
> +++ head/usr.sbin/trim/trim.8   Thu Jan 17 18:07:59 2019        (r343118)
> @@ -0,0 +1,167 @@
> +.\"
> +.\" Copyright (c) 2019 Eugene Grosbein <eugen@FreeBSD.org>.
> +.\" All rights reserved.
> +.\"
> +.\" Redistribution and use in source and binary forms, with or without
> +.\" modification, are permitted provided that the following conditions
> +.\" are met:
> +.\" 1. Redistributions of source code must retain the above copyright
> +.\"    notice, this list of conditions and the following disclaimer.
> +.\" 2. Redistributions in binary form must reproduce the above copyright
> +.\"    notice, this list of conditions and the following disclaimer in the
> +.\"    documentation and/or other materials provided with the distribution.
> +.\"
> +.\" THIS SOFTWARE IS PROVIDED BY THE AUTHOR AND CONTRIBUTORS ``AS IS'' AND
> +.\" ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE
> +.\" IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE
> +.\" ARE DISCLAIMED.  IN NO EVENT SHALL THE AUTHOR OR CONTRIBUTORS BE LIABLE
> +.\" FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL
> +.\" DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS
> +.\" OR SERVICES; LOSS OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION)
> +.\" HOWEVER CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT
> +.\" LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY
> +.\" OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF
> +.\" SUCH DAMAGE.
> +.\"
> +.\" $FreeBSD$
> +.\"
> +.Dd January 18, 2019
> +.Dt TRIM 8
> +.Os
> +.Sh NAME
> +.Nm trim
> +.Nd erase device blocks that have no needed contents
> +.Sh SYNOPSIS
> +.Nm
> +.Op Fl Nfqv
> +.Fl [ [lo] Xo
> +.Bk -words
> +.Sm off
> +.Ar offset
> +.Op Cm K | k | M | m | G | g | T | t ]
> +.Sm on
> +.Xc
> +.Ek
> +.Bk -words
> +.Op Fl r Ar rfile
> +.Ek
> +.Ar device ...
> +.Sh DESCRIPTION
> +The
> +.Nm
> +utility erases specified region of the device.
> +It is mostly relevant for storage that implement trim (like flash based,
> +or thinly provisioned storage).
> +.Sy All erased data is lost.
> +.Pp
> +The following options are available:
> +.Bl -tag -width indent
> +.It Fl N
> +Do not actually erase anything but show what it would do (dry run).
> +Implies
> +.Fl v .
> +This is the default. Overrides
> +.Fl f .
> +.It Fl f
> +Perform the operation. Overrides
> +.Fl N .
> +.It Fl l Xo
> +.Sm off
> +.Ar offset
> +.Op Cm K | k | M | m | G | g | T | t
> +.Sm on
> +.Xc
> +.It Fl o Xo
> +.Sm off
> +.Ar offset
> +.Op Cm K | k | M | m | G | g | T | t
> +.Sm on
> +.Xc
> +Specify the length
> +.Fl l
> +of the region to trim or its offset
> +.Fl o
> +from the beginning of the device.
> +.Sy The whole device is erased by default
> +unless one or both of these options are presented.
> +.Pp
> +The argument may be suffixed with one of
> +.Cm K ,
> +.Cm M ,
> +.Cm G
> +or
> +.Cm T
> +(either upper or lower case) to indicate a multiple of
> +Kilobytes, Megabytes, Gigabytes or Terabytes
> +respectively.
> +.It Fl q
> +Do not output anything except of possible error messages (quiet mode).
> +Overrides
> +.Fl v .
> +.It Fl r Ar rfile
> +Uses the length of given
> +.Ar rfile
> +as length of the region to erase.
> +.Sy The whole device is erased by default.
> +.It Fl v
> +Show offset and length of actual region being erased, in bytes.
> +.El
> +.Pp
> +Later options override previous ones.
> +.Pp
> +Note that actual success of the operation depends of underlying
> +device driver such as
> +.Xr ada 4 ,
> +.Xr da 4
> +and others.
> +Refer to corresponding manual pages for detail on possible caveats
> +in low level support for ATA TRIM or SCSI UNMAP commands.
> +.Sh EXIT STATUS
> +.Ex -std
> +If the final erase operation fails for an argument, the
> +.Nm
> +utility returns exit code 1.
> +It can also return one of the exit codes defined in
> +.Xr sysexits 3 ,
> +as follows:
> +.Bl -tag -width ".Dv EX_UNAVAILABLE"
> +.It Dv EX_USAGE
> +The specified offset or length of the region is incorrect.
> +.It Dv EX_OSERR
> +There is no enough memory to proceed.
> +.It Dv EX_NOINPUT
> +The specified
> +.Ar rfile
> +cannot be opened (perhaps, it does not exist).
> +.It Dv EX_IOERR
> +The specified
> +.Ar rfile
> +cannot be examined for its size due to some system input/output error.
> +.It Dv EX_DATAERR
> +The specified
> +.Ar rfile
> +is not regular file, directory nor special device, so its size
> +cannot be examined.
> +.It Dv EX_UNAVAILABLE
> +The specified
> +.Ar rfile
> +is special device file not supporting DIOCGMEDIASIZE
> +.Xr ioctl 2
> +(probably not a disk), so its size cannot be examined.
> +.El
> +.Sh SEE ALSO
> +.Xr ada 4 ,
> +.Xr da 4 ,
> +.Xr ioctl 2 ,
> +.Xr nda 4 ,
> +.Xr sysexits 3
> +.Sh HISTORY
> +The
> +.Nm
> +utility first appeared in
> +.Fx 12.1 .
> +.Sh AUTHORS
> +The
> +.Nm
> +utility was written by
> +.An Eugene Grosbein Aq Mt eugen@FreeBSD.org .
>
> Added: head/usr.sbin/trim/trim.c
> ==============================================================================
> --- /dev/null   00:00:00 1970   (empty, because file is newly added)
> +++ head/usr.sbin/trim/trim.c   Thu Jan 17 18:07:59 2019        (r343118)
> @@ -0,0 +1,210 @@
> +/*-
> + * SPDX-License-Identifier: BSD-2-Clause-FreeBSD
> + *
> + * Copyright (c) 2019 Eugene Grosbein <eugen@FreeBSD.org>.
> + * All rights reserved.
> + *
> + * Redistribution and use in source and binary forms, with or without
> + * modification, are permitted provided that the following conditions
> + * are met:
> + * 1. Redistributions of source code must retain the above copyright
> + *    notice, this list of conditions and the following disclaimer.
> + * 2. Redistributions in binary form must reproduce the above copyright
> + *    notice, this list of conditions and the following disclaimer in the
> + *    documentation and/or other materials provided with the distribution.
> + *
> + * THIS SOFTWARE IS PROVIDED BY THE AUTHOR AND CONTRIBUTORS ``AS IS'' AND
> + * ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE
> + * IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE
> + * ARE DISCLAIMED.  IN NO EVENT SHALL THE AUTHOR OR CONTRIBUTORS BE LIABLE
> + * FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL
> + * DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS
> + * OR SERVICES; LOSS OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION)
> + * HOWEVER CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT
> + * LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY
> + * OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF
> + * SUCH DAMAGE.
> + *
> + */
> +
> +#include <sys/disk.h>
> +#include <sys/ioctl.h>
> +#include <sys/stat.h>
> +
> +#include <err.h>
> +#include <errno.h>
> +#include <fcntl.h>
> +#include <libutil.h>
> +#include <limits.h>
> +#include <paths.h>
> +#include <stdbool.h>
> +#include <stdio.h>
> +#include <stdlib.h>
> +#include <sysexits.h>
> +#include <unistd.h>
> +
> +#include <sys/cdefs.h>
> +__FBSDID("$FreeBSD$");
> +
> +static off_t   getsize(const char *path);
> +static int     opendev(const char *path, int flags);
> +static int     trim(const char *path, off_t offset, off_t length, bool dryrun, bool verbose);
> +static void    usage(const char *name);
> +
> +int
> +main(int argc, char **argv)
> +{
> +       off_t offset, length;
> +       uint64_t usz;
> +       int ch, error;
> +       bool dryrun, verbose;
> +       char *fname, *name;
> +
> +       error = 0;
> +       length = offset = 0;
> +       name = argv[0];
> +       dryrun = verbose = true;
> +
> +       while ((ch = getopt(argc, argv, "Nfl:o:qr:v")) != -1)
> +               switch (ch) {
> +               case 'N':
> +                       dryrun = true;
> +                       verbose = true;
> +                       break;
> +               case 'f':
> +                       dryrun = false;
> +                       break;
> +               case 'l':
> +               case 'o':
> +                       if (expand_number(optarg, &usz) == -1 ||
> +                                       (off_t)usz < 0 || (usz == 0 && ch == 'l'))
> +                               errx(EX_USAGE,
> +                                       "invalid %s of the region: %s",
> +                                       ch == 'o' ? "offset" : "length",
> +                                       optarg);
> +                       if (ch == 'o')
> +                               offset = (off_t)usz;
> +                       else
> +                               length = (off_t)usz;
> +                       break;
> +               case 'q':
> +                       verbose = false;
> +                       break;
> +               case 'r':
> +                       if ((length = getsize(optarg)) == 0)
> +                               errx(EX_USAGE,
> +                                       "invalid zero length reference file"
> +                                       " for the region: %s", optarg);
> +                       break;
> +               case 'v':
> +                       verbose = true;
> +                       break;
> +               default:
> +                       usage(name);
> +                       /* NOTREACHED */
> +               }
> +
> +       argv += optind;
> +       argc -= optind;
> +
> +       if (argc < 1)
> +               usage(name);
> +
> +       while ((fname = *argv++) != NULL)
> +               if (trim(fname, offset, length, dryrun, verbose) < 0)
> +                       error++;
> +
> +       return (error ? EXIT_FAILURE : EXIT_SUCCESS);
> +}
> +
> +static int
> +opendev(const char *path, int flags)
> +{
> +       int fd;
> +       char *tstr;
> +
> +       if ((fd = open(path, flags)) < 0) {
> +               if (errno == ENOENT && path[0] != '/') {
> +                       if (asprintf(&tstr, "%s%s", _PATH_DEV, path) < 0)
> +                               errx(EX_OSERR, "no memory");
> +                       fd = open(tstr, flags);
> +                       free(tstr);
> +               }
> +       }
> +
> +       if (fd < 0)
> +               err(EX_NOINPUT, "open failed: %s", path);
> +
> +       return (fd);
> +}
> +
> +static off_t
> +getsize(const char *path)
> +{
> +       struct stat sb;
> +       off_t mediasize;
> +       int fd;
> +
> +       fd = opendev(path, O_RDONLY | O_DIRECT);
> +
> +       if (fstat(fd, &sb) < 0)
> +               err(EX_IOERR, "fstat failed: %s", path);
> +
> +       if (S_ISREG(sb.st_mode) || S_ISDIR(sb.st_mode)) {
> +               close(fd);
> +               return (sb.st_size);
> +       }
> +
> +       if (!S_ISCHR(sb.st_mode) && !S_ISBLK(sb.st_mode))
> +               errx(EX_DATAERR,
> +                       "invalid type of the file "
> +                       "(not regular, directory nor special device): %s",
> +                       path);
> +
> +       if (ioctl(fd, DIOCGMEDIASIZE, &mediasize) < 0)
> +               err(EX_UNAVAILABLE,
> +                       "ioctl(DIOCGMEDIASIZE) failed, probably not a disk: "
> +                       "%s", path);
> +
> +       close(fd);
> +       return (mediasize);
> +}
> +
> +static int
> +trim(const char *path, off_t offset, off_t length, bool dryrun, bool verbose)
> +{
> +       off_t arg[2];
> +       int error, fd;
> +
> +       if (length == 0)
> +               length = getsize(path);
> +
> +       if (verbose)
> +               printf("trim %s offset %ju length %ju\n",
> +                   path, (uintmax_t)offset, (uintmax_t)length);
> +
> +       if (dryrun) {
> +               printf("dry run: add -f to actually perform the operation\n");
> +               return (0);
> +       }
> +
> +       fd = opendev(path, O_WRONLY | O_DIRECT);
> +       arg[0] = offset;
> +       arg[1] = length;
> +
> +       error = ioctl(fd, DIOCGDELETE, arg);
> +       if (error < 0)
> +               warn("ioctl(DIOCGDELETE) failed: %s", path);
> +
> +       close(fd);
> +       return (error);
> +}
> +
> +static void
> +usage(const char *name)
> +{
> +       (void)fprintf(stderr,
> +           "usage: %s [-[lo] offset[K|k|M|m|G|g|T|t]] [-r rfile] [-Nfqv] device ...\n",
> +           name);
> +       exit(EX_USAGE);
> +}
>



Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?CAG6CVpVBsFDgksLEWG_U%2B4k6tH3SnaZkNFgd77D8NfLDf21Pdg>