* [PATCH 1/3] xfs_io: refactor numlen into a library function
2017-06-15 20:34 [PATCH v9 0/3] xfsprogs 4.12: GETFSMAP support Darrick J. Wong
@ 2017-06-15 20:35 ` Darrick J. Wong
2017-06-15 20:35 ` [PATCH 2/3] libxcmd: add cvt{int, long} to convert strings to int and long Darrick J. Wong
` (2 subsequent siblings)
3 siblings, 0 replies; 6+ messages in thread
From: Darrick J. Wong @ 2017-06-15 20:35 UTC (permalink / raw)
To: sandeen, darrick.wong; +Cc: linux-xfs
From: Darrick J. Wong <darrick.wong@oracle.com>
Refactor the competing numlen implementations into a single library function.
Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
---
include/input.h | 1 +
io/bmap.c | 19 ++++---------------
io/fiemap.c | 14 +-------------
libxcmd/input.c | 13 +++++++++++++
4 files changed, 19 insertions(+), 28 deletions(-)
diff --git a/include/input.h b/include/input.h
index 221678e..82cd2f4 100644
--- a/include/input.h
+++ b/include/input.h
@@ -28,6 +28,7 @@ extern char **breakline(char *input, int *count);
extern void doneline(char *input, char **vec);
extern char *fetchline(void);
+extern size_t numlen(uint64_t val, size_t base);
extern long long cvtnum(size_t blocksize, size_t sectorsize, char *s);
extern void cvtstr(double value, char *str, size_t sz);
extern unsigned long cvttime(char *s);
diff --git a/io/bmap.c b/io/bmap.c
index 2333244..2e4ff7b 100644
--- a/io/bmap.c
+++ b/io/bmap.c
@@ -18,6 +18,7 @@
#include "platform_defs.h"
#include "command.h"
+#include "input.h"
#include "init.h"
#include "io.h"
@@ -53,18 +54,6 @@ bmap_help(void)
"\n"));
}
-static int
-numlen(
- off64_t val)
-{
- off64_t tmp;
- int len;
-
- for (len = 0, tmp = val; tmp > 0; tmp = tmp/10)
- len++;
- return (len == 0 ? 1 : len);
-}
-
int
bmap_f(
int argc,
@@ -323,7 +312,7 @@ bmap_f(
if (map[i + 1].bmv_block == -1) {
foff_w = max(foff_w, strlen(rbuf));
tot_w = max(tot_w,
- numlen(map[i+1].bmv_length));
+ numlen(map[i+1].bmv_length, 10));
} else {
snprintf(bbuf, sizeof(bbuf), "%lld..%lld",
(long long) map[i + 1].bmv_block,
@@ -344,10 +333,10 @@ bmap_f(
aoff_w = 0;
foff_w = max(foff_w, strlen(rbuf));
tot_w = max(tot_w,
- numlen(map[i+1].bmv_length));
+ numlen(map[i+1].bmv_length, 10));
}
}
- agno_w = is_rt ? 0 : max(MINAG_WIDTH, numlen(fsgeo.agcount));
+ agno_w = is_rt ? 0 : max(MINAG_WIDTH, numlen(fsgeo.agcount, 10));
printf("%4s: %-*s %-*s %*s %-*s %*s%s\n",
_("EXT"),
foff_w, _("FILE-OFFSET"),
diff --git a/io/fiemap.c b/io/fiemap.c
index bcbae49..75e8820 100644
--- a/io/fiemap.c
+++ b/io/fiemap.c
@@ -18,6 +18,7 @@
#include "platform_defs.h"
#include "command.h"
+#include "input.h"
#include <linux/fiemap.h>
#include "init.h"
#include "io.h"
@@ -48,19 +49,6 @@ fiemap_help(void)
"\n"));
}
-static int
-numlen(
- __u64 val,
- int base)
-{
- __u64 tmp;
- int len;
-
- for (len = 0, tmp = val; tmp > 0; tmp = tmp/base)
- len++;
- return (len == 0 ? 1 : len);
-}
-
static void
print_verbose(
struct fiemap_extent *extent,
diff --git a/libxcmd/input.c b/libxcmd/input.c
index 8aeb3b0..9437be3 100644
--- a/libxcmd/input.c
+++ b/libxcmd/input.c
@@ -136,6 +136,19 @@ doneline(
free(vec);
}
+size_t
+numlen(
+ uint64_t val,
+ size_t base)
+{
+ uint64_t tmp;
+ size_t len;
+
+ for (len = 0, tmp = val; tmp > 0; tmp = tmp / base)
+ len++;
+ return len == 0 ? 1 : len;
+}
+
#define EXABYTES(x) ((long long)(x) << 60)
#define PETABYTES(x) ((long long)(x) << 50)
#define TERABYTES(x) ((long long)(x) << 40)
^ permalink raw reply related [flat|nested] 6+ messages in thread* [PATCH 2/3] libxcmd: add cvt{int, long} to convert strings to int and long
2017-06-15 20:34 [PATCH v9 0/3] xfsprogs 4.12: GETFSMAP support Darrick J. Wong
2017-06-15 20:35 ` [PATCH 1/3] xfs_io: refactor numlen into a library function Darrick J. Wong
@ 2017-06-15 20:35 ` Darrick J. Wong
2017-06-15 20:35 ` [PATCH 3/3] libxfs: use crc32c slice-by-8 variant by default Darrick J. Wong
2017-06-15 20:37 ` [PATCH v9 0/3] xfsprogs 4.12: GETFSMAP support Darrick J. Wong
3 siblings, 0 replies; 6+ messages in thread
From: Darrick J. Wong @ 2017-06-15 20:35 UTC (permalink / raw)
To: sandeen, darrick.wong; +Cc: linux-xfs
From: Darrick J. Wong <darrick.wong@oracle.com>
Create some helper functions to convert strings to int or long
in a standard way and work around problems in the C library
atoi/strtol functions.
Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
---
include/input.h | 8 +++
libxcmd/input.c | 134 +++++++++++++++++++++++++++++++++++++++++++++++++++++++
2 files changed, 142 insertions(+)
diff --git a/include/input.h b/include/input.h
index 82cd2f4..145114b 100644
--- a/include/input.h
+++ b/include/input.h
@@ -28,6 +28,14 @@ extern char **breakline(char *input, int *count);
extern void doneline(char *input, char **vec);
extern char *fetchline(void);
+extern int64_t cvt_s64(char *s, int base);
+extern int32_t cvt_s32(char *s, int base);
+extern int16_t cvt_s16(char *s, int base);
+
+extern uint64_t cvt_u64(char *s, int base);
+extern uint32_t cvt_u32(char *s, int base);
+extern uint16_t cvt_u16(char *s, int base);
+
extern size_t numlen(uint64_t val, size_t base);
extern long long cvtnum(size_t blocksize, size_t sectorsize, char *s);
extern void cvtstr(double value, char *str, size_t sz);
diff --git a/libxcmd/input.c b/libxcmd/input.c
index 9437be3..8dd232c 100644
--- a/libxcmd/input.c
+++ b/libxcmd/input.c
@@ -149,6 +149,140 @@ numlen(
return len == 0 ? 1 : len;
}
+/*
+ * Convert string to int64_t, set errno if the conversion fails or
+ * doesn't fit. Does not allow unit specifiers. Sets errno to zero
+ * prior to conversion.
+ */
+int64_t
+cvt_s64(
+ char *s,
+ int base)
+{
+ long long i;
+ char *sp;
+ int c;
+
+ errno = 0;
+ i = strtoll(s, &sp, base);
+ if (*sp == '\0' && sp != s)
+ return i;
+bad:
+ errno = -ERANGE;
+ return INT64_MIN;
+}
+
+/*
+ * Convert string to int32_t, set errno if the conversion fails or
+ * doesn't fit. Does not allow unit specifiers. Sets errno to zero
+ * prior to conversion.
+ */
+int32_t
+cvt_s32(
+ char *s,
+ int base)
+{
+ int64_t i;
+
+ i = cvt_s64(s, base);
+ if (errno)
+ return i;
+ if (i > INT32_MAX || i < INT32_MIN) {
+ errno = -ERANGE;
+ return INT32_MIN;
+ }
+ return i;
+}
+
+/*
+ * Convert string to int16_t, set errno if the conversion fails or
+ * doesn't fit. Does not allow unit specifiers. Sets errno to zero
+ * prior to conversion.
+ */
+int16_t
+cvt_s16(
+ char *s,
+ int base)
+{
+ int64_t i;
+
+ i = cvt_s64(s, base);
+ if (errno)
+ return i;
+ if (i > INT16_MAX || i < INT16_MIN) {
+ errno = -ERANGE;
+ return INT16_MIN;
+ }
+ return i;
+}
+
+/*
+ * Convert string to uint64_t, set errno if the conversion fails or
+ * doesn't fit. Does not allow unit specifiers. Sets errno to zero
+ * prior to conversion.
+ */
+uint64_t
+cvt_u64(
+ char *s,
+ int base)
+{
+ long long i;
+ char *sp;
+ int c;
+
+ errno = 0;
+ i = strtoll(s, &sp, base);
+ if (*sp == '\0' && sp != s)
+ return i;
+bad:
+ errno = -ERANGE;
+ return UINT64_MAX;
+}
+
+/*
+ * Convert string to uint32_t, set errno if the conversion fails or
+ * doesn't fit. Does not allow unit specifiers. Sets errno to zero
+ * prior to conversion.
+ */
+uint32_t
+cvt_u32(
+ char *s,
+ int base)
+{
+ uint64_t i;
+
+ i = cvt_u64(s, base);
+ if (errno)
+ return i;
+ if (i > UINT32_MAX) {
+ errno = -ERANGE;
+ return UINT32_MAX;
+ }
+ return i;
+}
+
+/*
+ * Convert string to uint16_t, set errno if the conversion fails or
+ * doesn't fit. Does not allow unit specifiers. Sets errno to zero
+ * prior to conversion.
+ */
+uint16_t
+cvt_u16(
+ char *s,
+ int base)
+{
+ uint64_t i;
+
+ i = cvt_u64(s, base);
+ if (errno)
+ return i;
+ if (i > UINT16_MAX) {
+ errno = -ERANGE;
+ return UINT16_MAX;
+ }
+ return i;
+}
+
#define EXABYTES(x) ((long long)(x) << 60)
#define PETABYTES(x) ((long long)(x) << 50)
#define TERABYTES(x) ((long long)(x) << 40)
^ permalink raw reply related [flat|nested] 6+ messages in thread* [PATCH 3/3] libxfs: use crc32c slice-by-8 variant by default
2017-06-15 20:34 [PATCH v9 0/3] xfsprogs 4.12: GETFSMAP support Darrick J. Wong
2017-06-15 20:35 ` [PATCH 1/3] xfs_io: refactor numlen into a library function Darrick J. Wong
2017-06-15 20:35 ` [PATCH 2/3] libxcmd: add cvt{int, long} to convert strings to int and long Darrick J. Wong
@ 2017-06-15 20:35 ` Darrick J. Wong
2017-06-21 19:14 ` Eric Sandeen
2017-06-15 20:37 ` [PATCH v9 0/3] xfsprogs 4.12: GETFSMAP support Darrick J. Wong
3 siblings, 1 reply; 6+ messages in thread
From: Darrick J. Wong @ 2017-06-15 20:35 UTC (permalink / raw)
To: sandeen, darrick.wong; +Cc: linux-xfs
From: Darrick J. Wong <darrick.wong@oracle.com>
The crc32c code used in xfsprogs was copied directly from the Linux
kernel. However, that code selects slice-by-4 by default, which isn't
the fastest -- that's slice-by-8, which trades table size for speed.
Fix some makefile dependency problems and explicitly select the
algorithm we want. With this patch applied, I see about a 10% drop in
CPU time running xfs_repair.
Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
---
libxfs/Makefile | 4 ++--
libxfs/crc32defs.h | 34 ++++++++++++++++++++++++++++++++++
2 files changed, 36 insertions(+), 2 deletions(-)
diff --git a/libxfs/Makefile b/libxfs/Makefile
index baba02f..d248c1f 100644
--- a/libxfs/Makefile
+++ b/libxfs/Makefile
@@ -122,7 +122,7 @@ LDIRT = gen_crc32table crc32table.h crc32selftest
default: crc32selftest ltdepend $(LTLIBRARY)
-crc32table.h: gen_crc32table.c
+crc32table.h: gen_crc32table.c crc32defs.h
@echo " [CC] gen_crc32table"
$(Q) $(BUILD_CC) $(BUILD_CFLAGS) -o gen_crc32table $<
@echo " [GENERATE] $@"
@@ -133,7 +133,7 @@ crc32table.h: gen_crc32table.c
# systems/architectures. Hence we make sure that xfsprogs will never use a
# busted CRC calculation at build time and hence avoid putting bad CRCs down on
# disk.
-crc32selftest: gen_crc32table.c crc32table.h crc32.c
+crc32selftest: gen_crc32table.c crc32table.h crc32.c crc32defs.h
@echo " [TEST] CRC32"
$(Q) $(BUILD_CC) $(BUILD_CFLAGS) -D CRC32_SELFTEST=1 crc32.c -o $@
$(Q) ./$@
diff --git a/libxfs/crc32defs.h b/libxfs/crc32defs.h
index 64cba2c..2999782 100644
--- a/libxfs/crc32defs.h
+++ b/libxfs/crc32defs.h
@@ -1,4 +1,38 @@
/*
+ * Use slice-by-8, which is the fastest variant.
+ *
+ * Calculate checksum 8 bytes at a time with a clever slicing algorithm.
+ * This is the fastest algorithm, but comes with a 8KiB lookup table.
+ * Most modern processors have enough cache to hold this table without
+ * thrashing the cache.
+ *
+ * The Linux kernel uses this as the default implementation "unless you
+ * have a good reason not to". The reason why Kconfig urges you to pick
+ * SLICEBY8 is because people challenged the assertion that we should
+ * always use slice by 8, so Darrick wrote a crc microbenchmark utility
+ * and ran it on as many machines as he could get his hands on to show
+ * that sb8 was the fastest.
+ *
+ * Every 64-bit machine (and most of the 32-bit ones too) saw the best
+ * results with sb8. Any machine with more than 4K of cache saw better
+ * results. The spreadsheet still exists today[1]; note that
+ * 'crc32-kern-le' corresponds to the slice by 4 algorithm which is the
+ * default unless CRC_LE_BITS is defined explicitly.
+ *
+ * FWIW, there are a handful of board defconfigs in the kernel that
+ * don't pick sliceby8. These are all embedded 32-bit mips/ppc systems
+ * with very small cache sizes which experience cache thrashing with the
+ * slice by 8 algorithm, and therefore chose to pick defaults that are
+ * saner for their particular board configuration. For nearly all of
+ * XFS' perceived userbase (which we assume are 32 and 64-bit machines
+ * with sufficiently large CPU cache and largeish storage devices) slice
+ * by 8 is the right choice.
+ *
+ * [1] https://goo.gl/0LSzsG ("crc32c_bench")
+ */
+#define CRC_LE_BITS 64
+
+/*
* There are multiple 16-bit CRC polynomials in common use, but this is
* *the* standard CRC-32 polynomial, first popularized by Ethernet.
* x^32+x^26+x^23+x^22+x^16+x^12+x^11+x^10+x^8+x^7+x^5+x^4+x^2+x^1+x^0
^ permalink raw reply related [flat|nested] 6+ messages in thread* Re: [PATCH 3/3] libxfs: use crc32c slice-by-8 variant by default
2017-06-15 20:35 ` [PATCH 3/3] libxfs: use crc32c slice-by-8 variant by default Darrick J. Wong
@ 2017-06-21 19:14 ` Eric Sandeen
0 siblings, 0 replies; 6+ messages in thread
From: Eric Sandeen @ 2017-06-21 19:14 UTC (permalink / raw)
To: Darrick J. Wong, sandeen; +Cc: linux-xfs, Dave Chinner
On 6/15/17 3:35 PM, Darrick J. Wong wrote:
> From: Darrick J. Wong <darrick.wong@oracle.com>
>
> The crc32c code used in xfsprogs was copied directly from the Linux
> kernel. However, that code selects slice-by-4 by default, which isn't
> the fastest -- that's slice-by-8, which trades table size for speed.
> Fix some makefile dependency problems and explicitly select the
> algorithm we want. With this patch applied, I see about a 10% drop in
> CPU time running xfs_repair.
>
> Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
Reviewed-by: Eric Sandeen <sandeen@redhat.com>
dchinner: are you ok with this now? You had questions earlier,
but Darrick's explanations satisfied me, at least...
> ---
> libxfs/Makefile | 4 ++--
> libxfs/crc32defs.h | 34 ++++++++++++++++++++++++++++++++++
> 2 files changed, 36 insertions(+), 2 deletions(-)
>
>
> diff --git a/libxfs/Makefile b/libxfs/Makefile
> index baba02f..d248c1f 100644
> --- a/libxfs/Makefile
> +++ b/libxfs/Makefile
> @@ -122,7 +122,7 @@ LDIRT = gen_crc32table crc32table.h crc32selftest
>
> default: crc32selftest ltdepend $(LTLIBRARY)
>
> -crc32table.h: gen_crc32table.c
> +crc32table.h: gen_crc32table.c crc32defs.h
> @echo " [CC] gen_crc32table"
> $(Q) $(BUILD_CC) $(BUILD_CFLAGS) -o gen_crc32table $<
> @echo " [GENERATE] $@"
> @@ -133,7 +133,7 @@ crc32table.h: gen_crc32table.c
> # systems/architectures. Hence we make sure that xfsprogs will never use a
> # busted CRC calculation at build time and hence avoid putting bad CRCs down on
> # disk.
> -crc32selftest: gen_crc32table.c crc32table.h crc32.c
> +crc32selftest: gen_crc32table.c crc32table.h crc32.c crc32defs.h
> @echo " [TEST] CRC32"
> $(Q) $(BUILD_CC) $(BUILD_CFLAGS) -D CRC32_SELFTEST=1 crc32.c -o $@
> $(Q) ./$@
> diff --git a/libxfs/crc32defs.h b/libxfs/crc32defs.h
> index 64cba2c..2999782 100644
> --- a/libxfs/crc32defs.h
> +++ b/libxfs/crc32defs.h
> @@ -1,4 +1,38 @@
> /*
> + * Use slice-by-8, which is the fastest variant.
> + *
> + * Calculate checksum 8 bytes at a time with a clever slicing algorithm.
> + * This is the fastest algorithm, but comes with a 8KiB lookup table.
> + * Most modern processors have enough cache to hold this table without
> + * thrashing the cache.
> + *
> + * The Linux kernel uses this as the default implementation "unless you
> + * have a good reason not to". The reason why Kconfig urges you to pick
> + * SLICEBY8 is because people challenged the assertion that we should
> + * always use slice by 8, so Darrick wrote a crc microbenchmark utility
> + * and ran it on as many machines as he could get his hands on to show
> + * that sb8 was the fastest.
> + *
> + * Every 64-bit machine (and most of the 32-bit ones too) saw the best
> + * results with sb8. Any machine with more than 4K of cache saw better
> + * results. The spreadsheet still exists today[1]; note that
> + * 'crc32-kern-le' corresponds to the slice by 4 algorithm which is the
> + * default unless CRC_LE_BITS is defined explicitly.
> + *
> + * FWIW, there are a handful of board defconfigs in the kernel that
> + * don't pick sliceby8. These are all embedded 32-bit mips/ppc systems
> + * with very small cache sizes which experience cache thrashing with the
> + * slice by 8 algorithm, and therefore chose to pick defaults that are
> + * saner for their particular board configuration. For nearly all of
> + * XFS' perceived userbase (which we assume are 32 and 64-bit machines
> + * with sufficiently large CPU cache and largeish storage devices) slice
> + * by 8 is the right choice.
> + *
> + * [1] https://goo.gl/0LSzsG ("crc32c_bench")
> + */
> +#define CRC_LE_BITS 64
> +
> +/*
> * There are multiple 16-bit CRC polynomials in common use, but this is
> * *the* standard CRC-32 polynomial, first popularized by Ethernet.
> * x^32+x^26+x^23+x^22+x^16+x^12+x^11+x^10+x^8+x^7+x^5+x^4+x^2+x^1+x^0
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-xfs" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
>
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH v9 0/3] xfsprogs 4.12: GETFSMAP support
2017-06-15 20:34 [PATCH v9 0/3] xfsprogs 4.12: GETFSMAP support Darrick J. Wong
` (2 preceding siblings ...)
2017-06-15 20:35 ` [PATCH 3/3] libxfs: use crc32c slice-by-8 variant by default Darrick J. Wong
@ 2017-06-15 20:37 ` Darrick J. Wong
3 siblings, 0 replies; 6+ messages in thread
From: Darrick J. Wong @ 2017-06-15 20:37 UTC (permalink / raw)
To: sandeen; +Cc: linux-xfs
Sorry, I mistyped the ending patch name in stg mail, so please ignore
this series. The 12-patch series is the correct one.
--D
On Thu, Jun 15, 2017 at 01:34:49PM -0700, Darrick J. Wong wrote:
> Hi all,
>
> This patchset implements the userspace components of the new GETFSMAP
> ioctl that is now in upstream for 4.12. This release contains various
> cleanups that came up in the discussion of the v8 patchset; it should
> apply directly to for-next.
>
> The first two patches refactor string-to-number conversion routines.
>
> The third patch chooses a faster crc32c algorithm, which provides
> a small (~5%) decrease in xfs_repair runtime for metadata-heavy fses.
>
> The fourth patch adds some autoconf magic so that client programs can
> detect a platform supporting FSMAP (currently Linux) and stuff in our
> own header if the system headers do not provide it.
>
> The fifth patch introduce a 'fsmap' command to xfs_io so that users can
> query the raw reverse mapping data that is exported by fsmap. If the
> experimental rmapbt feature is enabled, the returned results will be
> straight out of the rmapbt; otherwise, the extent information is
> synthesized out of the free space btres.
>
> The sixth patch delegates xfs_repair's rmap comparison function to the
> new libxfs version that was introduced as a part of the getfsmap kernel
> patches.
>
> The remainder of the patch set introduces xfs_spaceman, which is a new
> tool to manage disk space on a mounted XFS filesystem. The new tool can
> call FITRIM, manage space reservations, and display summary information
> about the free space on the filesystem. This information is probably
> most useful for developers and people surveying free space
> fragmentation.
>
> Note: The primary user of the GETFSMAP information will be the media
> scan phase of the upcoming xfs_scrub tool, which will be posted later.
>
> Questions? Comments?
>
> --D
> --
> To unsubscribe from this list: send the line "unsubscribe linux-xfs" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply [flat|nested] 6+ messages in thread