From: Dave Chinner <david@fromorbit.com>
To: Eric Sandeen <sandeen@sandeen.net>
Cc: linux-xfs <linux-xfs@vger.kernel.org>
Subject: Re: [PATCH V2] libxfs: use FALLOC_FL_ZERO_RANGE in libxfs_device_zero
Date: Fri, 14 Feb 2020 12:34:57 +1100 [thread overview]
Message-ID: <20200214013457.GX10776@dread.disaster.area> (raw)
In-Reply-To: <3ebe2d29-7943-b0a2-db5c-196610537bca@sandeen.net>
On Thu, Feb 13, 2020 at 07:05:50PM -0600, Eric Sandeen wrote:
> I had a request from someone who cared about mkfs speed(!)
> over a slower network block device to look into using faster
> zeroing methods, particularly for the log, during mkfs.
>
> Using FALLOC_FL_ZERO_RANGE is faster in this case than writing
> a bunch of zeros across a wire.
>
> Signed-off-by: Eric Sandeen <sandeen@redhat.com>
> ---
>
> V2: Clean up all the nasty stuff I'd flung out there as a wild first
> cut, thanks Dave.
>
> diff --git a/include/builddefs.in b/include/builddefs.in
> index 4700b527..1dd27f76 100644
> --- a/include/builddefs.in
> +++ b/include/builddefs.in
> @@ -144,6 +144,9 @@ endif
> ifeq ($(HAVE_GETFSMAP),yes)
> PCFLAGS+= -DHAVE_GETFSMAP
> endif
> +ifeq ($(HAVE_FALLOCATE),yes)
> +PCFLAGS += -DHAVE_FALLOCATE
> +endif
>
> LIBICU_LIBS = @libicu_LIBS@
> LIBICU_CFLAGS = @libicu_CFLAGS@
> diff --git a/include/linux.h b/include/linux.h
> index 8f3c32b0..8d5c4584 100644
> --- a/include/linux.h
> +++ b/include/linux.h
> @@ -20,6 +20,10 @@
> #include <stdio.h>
> #include <asm/types.h>
> #include <mntent.h>
> +#include <fcntl.h>
> +#if defined(HAVE_FALLOCATE)
> +#include <linux/falloc.h>
> +#endif
> #ifdef OVERRIDE_SYSTEM_FSXATTR
> # define fsxattr sys_fsxattr
> #endif
> @@ -164,6 +168,24 @@ static inline void platform_mntent_close(struct mntent_cursor * cursor)
> endmntent(cursor->mtabp);
> }
>
> +#if defined(FALLOC_FL_ZERO_RANGE)
> +static inline int
> +platform_zero_range(
> + int fd,
> + xfs_off_t start,
> + size_t len)
> +{
> + int ret;
> +
> + ret = fallocate(fd, FALLOC_FL_ZERO_RANGE, start, len);
> + if (!ret)
> + return 0;
> + return -errno;
> +}
> +#else
> +#define platform_zero_range(fd, s, l) (-EOPNOTSUPP)
> +#endif
That all looks good. :)
> /*
> * Check whether we have to define FS_IOC_FS[GS]ETXATTR ourselves. These
> * are a copy of the definitions moved to linux/uapi/fs.h in the 4.5 kernel,
> diff --git a/libxfs/rdwr.c b/libxfs/rdwr.c
> index 0d9d7202..2f6a3eb3 100644
> --- a/libxfs/rdwr.c
> +++ b/libxfs/rdwr.c
> @@ -60,9 +60,19 @@ int
> libxfs_device_zero(struct xfs_buftarg *btp, xfs_daddr_t start, uint len)
> {
> xfs_off_t start_offset, end_offset, offset;
> - ssize_t zsize, bytes;
> + ssize_t zsize, bytes, len_bytes;
len_bytes should be size_t, right?
> char *z;
> - int fd;
> + int error, fd;
> +
> + fd = libxfs_device_to_fd(btp->dev);
> + start_offset = LIBXFS_BBTOOFF64(start);
> + end_offset = LIBXFS_BBTOOFF64(start + len) - start_offset;
> +
> + /* try to use special zeroing methods, fall back to writes if needed */
> + len_bytes = LIBXFS_BBTOOFF64(len);
> + error = platform_zero_range(fd, start_offset, len_bytes);
This is a bit ... convoluted, and doesn't end_offset = len_bytes?
i.e.
start_offset = start << BBSHIFT
len_bytes = len << BBSHIFT
end_offset = (start + len) << BBSHIFT - start_offset
= (start << BBSHIFT) + (len << BBSHIFT) - start_offset
= start_offset + len_bytes - start_offset
= len_bytes
Cheers,
Dave.
--
Dave Chinner
david@fromorbit.com
next prev parent reply other threads:[~2020-02-14 1:35 UTC|newest]
Thread overview: 14+ messages / expand[flat|nested] mbox.gz Atom feed top
2020-02-13 21:12 [PATCH, RFC] libxfs: use FALLOC_FL_ZERO_RANGE in libxfs_device_zero Eric Sandeen
2020-02-13 23:48 ` Dave Chinner
2020-02-13 23:57 ` Eric Sandeen
2020-02-14 0:25 ` Dave Chinner
2020-02-14 1:05 ` [PATCH V2] " Eric Sandeen
2020-02-14 1:34 ` Dave Chinner [this message]
2020-02-14 1:43 ` Eric Sandeen
2020-02-22 3:22 ` [PATCH V3] " Eric Sandeen
2020-02-22 7:24 ` Darrick J. Wong
2020-02-22 15:23 ` Eric Sandeen
2020-02-25 18:13 ` [PATCH V4] " Eric Sandeen
2020-02-25 18:46 ` Christoph Hellwig
2020-02-25 19:16 ` Darrick J. Wong
2020-02-25 23:33 ` Eric Sandeen
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20200214013457.GX10776@dread.disaster.area \
--to=david@fromorbit.com \
--cc=linux-xfs@vger.kernel.org \
--cc=sandeen@sandeen.net \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.