All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 0/2 v4] xfs_repair: improved secondary sb search
@ 2016-02-12 19:03 Bill O'Donnell
  2016-02-12 19:03 ` [PATCH v4 1/2] libxcmd: generalize topology functions Bill O'Donnell
  2016-02-12 19:03 ` [PATCH v4 2/2] xfs_repair: new secondary superblock search method Bill O'Donnell
  0 siblings, 2 replies; 11+ messages in thread
From: Bill O'Donnell @ 2016-02-12 19:03 UTC (permalink / raw)
  To: xfs


Hello -

New iteration (v4) of this series...

Current xfs_repair uses a brute force approach to find a valid
secondary sb. This series optimizes the secondary sb search, using
similar method to determine geometry as that of xfs_mkfs. If the
faster method fails in its search, fall back to original brute force
slower search.

version history
----
v1: http://oss.sgi.com/archives/xfs/2016-02/msg00304.html
v2: patch 2 whitespace fixups
v3: patch 2 correct functionality; style fixups
v4: patch 1,2 properly see to style and whitespace fixups
----

patch 1 has some general infrastructure adjustments, moving general
topology functions from mkfs to libxcmd.

patch 2 adds the new secondary superblock search method.

Questions, comments are welcome.

Thanks-
Bill

_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs

^ permalink raw reply	[flat|nested] 11+ messages in thread

* [PATCH v4 1/2] libxcmd: generalize topology functions
  2016-02-12 19:03 [PATCH 0/2 v4] xfs_repair: improved secondary sb search Bill O'Donnell
@ 2016-02-12 19:03 ` Bill O'Donnell
  2016-02-12 19:39   ` Christoph Hellwig
  2016-02-13  2:43   ` Eric Sandeen
  2016-02-12 19:03 ` [PATCH v4 2/2] xfs_repair: new secondary superblock search method Bill O'Donnell
  1 sibling, 2 replies; 11+ messages in thread
From: Bill O'Donnell @ 2016-02-12 19:03 UTC (permalink / raw)
  To: xfs

Move general topology functions from xfs_mkfs to new topology
collection in libxcmd.

Signed-off-by: Bill O'Donnell <billodo@redhat.com>
---
 include/Makefile   |   1 +
 include/libxcmd.h  |  56 +++++++++
 libxcmd/Makefile   |   2 +-
 libxcmd/topology.c | 341 +++++++++++++++++++++++++++++++++++++++++++++++++++++
 mkfs/Makefile      |   2 +-
 mkfs/xfs_mkfs.c    | 321 +------------------------------------------------
 6 files changed, 401 insertions(+), 322 deletions(-)
 create mode 100644 include/libxcmd.h
 create mode 100644 libxcmd/topology.c

diff --git a/include/Makefile b/include/Makefile
index 6148756..2671f07 100644
--- a/include/Makefile
+++ b/include/Makefile
@@ -20,6 +20,7 @@ include $(TOPDIR)/include/builddefs
 
 LIBHFILES = libxfs.h \
 	libxlog.h \
+	libxcmd.h \
 	atomic.h \
 	bitops.h \
 	cache.h \
diff --git a/include/libxcmd.h b/include/libxcmd.h
new file mode 100644
index 0000000..a912534
--- /dev/null
+++ b/include/libxcmd.h
@@ -0,0 +1,56 @@
+/*
+ * Copyright (c) 2000-2005 Silicon Graphics, Inc.
+ * All Rights Reserved.
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License as
+ * published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope that it would be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, write the Free Software Foundation,
+ * Inc.,  51 Franklin St, Fifth Floor, Boston, MA  02110-1301  USA
+ */
+
+#ifndef __LIBXCMD_H__
+#define __LIBXCMD_H__
+
+#include "libxfs.h"
+#include <sys/time.h>
+
+/*
+ * Device topology information.
+ */
+typedef struct fs_topology {
+	int	dsunit;		/* stripe unit - data subvolume */
+	int	dswidth;	/* stripe width - data subvolume */
+	int	rtswidth;	/* stripe width - rt subvolume */
+	int	lsectorsize;	/* logical sector size &*/
+	int	psectorsize;	/* physical sector size */
+} fs_topology_t;
+
+extern void
+get_topology(
+	libxfs_init_t		*xi,
+	struct fs_topology	*ft,
+	int			force_overwrite);
+
+extern void
+calc_default_ag_geometry(
+	int		blocklog,
+	__uint64_t	dblocks,
+	int		multidisk,
+	__uint64_t	*agsize,
+	__uint64_t	*agcount);
+
+extern int
+check_overwrite(
+	char		*device);
+
+
+
+#endif	/* __LIBXCMD_H__ */
diff --git a/libxcmd/Makefile b/libxcmd/Makefile
index 7701ed9..aab8d6d 100644
--- a/libxcmd/Makefile
+++ b/libxcmd/Makefile
@@ -10,7 +10,7 @@ LT_CURRENT = 0
 LT_REVISION = 0
 LT_AGE = 0
 
-CFILES = command.c input.c paths.c projects.c help.c quit.c
+CFILES = command.c input.c paths.c projects.c help.c quit.c topology.c
 
 ifeq ($(HAVE_GETMNTENT),yes)
 LCFLAGS += -DHAVE_GETMNTENT
diff --git a/libxcmd/topology.c b/libxcmd/topology.c
new file mode 100644
index 0000000..4cbe4b1
--- /dev/null
+++ b/libxcmd/topology.c
@@ -0,0 +1,341 @@
+/*
+ * Copyright (c) 2000-2001,2005 Silicon Graphics, Inc.
+ * All Rights Reserved.
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License as
+ * published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope that it would be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, write the Free Software Foundation,
+ * Inc.,  51 Franklin St, Fifth Floor, Boston, MA  02110-1301  USA
+ */
+
+#include "libxfs.h"
+#include "libxcmd.h"
+#ifdef ENABLE_BLKID
+#  include <blkid/blkid.h>
+#endif /* ENABLE_BLKID */
+#include "../mkfs/xfs_mkfs.h"
+
+#define TERABYTES(count, blog)	((__uint64_t)(count) << (40 - (blog)))
+#define GIGABYTES(count, blog)	((__uint64_t)(count) << (30 - (blog)))
+#define MEGABYTES(count, blog)	((__uint64_t)(count) << (20 - (blog)))
+
+void
+calc_default_ag_geometry(
+	int		blocklog,
+	__uint64_t	dblocks,
+	int		multidisk,
+	__uint64_t	*agsize,
+	__uint64_t	*agcount)
+{
+	__uint64_t	blocks = 0;
+	int		shift = 0;
+
+	/*
+	 * First handle the high extreme - the point at which we will
+	 * always use the maximum AG size.
+	 *
+	 * This applies regardless of storage configuration.
+	 */
+	if (dblocks >= TERABYTES(32, blocklog)) {
+		blocks = XFS_AG_MAX_BLOCKS(blocklog);
+		goto done;
+	}
+
+	/*
+	 * For the remainder we choose an AG size based on the
+	 * number of data blocks available, trying to keep the
+	 * number of AGs relatively small (especially compared
+	 * to the original algorithm).  AG count is calculated
+	 * based on the preferred AG size, not vice-versa - the
+	 * count can be increased by growfs, so prefer to use
+	 * smaller counts at mkfs time.
+	 *
+	 * For a single underlying storage device between 128MB
+	 * and 4TB in size, just use 4 AGs, otherwise scale up
+	 * smoothly between min/max AG sizes.
+	 */
+
+	if (!multidisk && dblocks >= MEGABYTES(128, blocklog)) {
+		if (dblocks >= TERABYTES(4, blocklog)) {
+			blocks = XFS_AG_MAX_BLOCKS(blocklog);
+			goto done;
+		}
+		shift = 2;
+	} else if (dblocks > GIGABYTES(512, blocklog))
+		shift = 5;
+	else if (dblocks > GIGABYTES(8, blocklog))
+		shift = 4;
+	else if (dblocks >= MEGABYTES(128, blocklog))
+		shift = 3;
+	else if (dblocks >= MEGABYTES(64, blocklog))
+		shift = 2;
+	else if (dblocks >= MEGABYTES(32, blocklog))
+		shift = 1;
+	else
+		shift = 0;
+	/*
+	 * If dblocks is not evenly divisible by the number of
+	 * desired AGs, round "blocks" up so we don't lose the
+	 * last bit of the filesystem. The same principle applies
+	 * to the AG count, so we don't lose the last AG!
+	 */
+	blocks = dblocks >> shift;
+	if (dblocks & xfs_mask32lo(shift)) {
+		if (blocks < XFS_AG_MAX_BLOCKS(blocklog))
+		    blocks++;
+	}
+done:
+	*agsize = blocks;
+	*agcount = dblocks / blocks + (dblocks % blocks != 0);
+}
+
+/*
+ * Check for existing filesystem or partition table on device.
+ * Returns:
+ *	 1 for existing fs or partition
+ *	 0 for nothing found
+ *	-1 for internal error
+ */
+#ifdef ENABLE_BLKID
+int
+check_overwrite(
+	char		*device)
+{
+	const char	*type;
+	blkid_probe	pr = NULL;
+	int		ret;
+	int		fd;
+	long long	size;
+	int		bsz;
+
+	if (!device || !*device)
+		return 0;
+
+	ret = -1; /* will reset on success of all setup calls */
+
+	fd = open(device, O_RDONLY);
+	if (fd < 0)
+		goto out;
+	platform_findsizes(device, fd, &size, &bsz);
+	close(fd);
+
+	/* nothing to overwrite on a 0-length device */
+	if (size == 0) {
+		ret = 0;
+		goto out;
+	}
+
+	pr = blkid_new_probe_from_filename(device);
+	if (!pr)
+		goto out;
+
+	ret = blkid_probe_enable_partitions(pr, 1);
+	if (ret < 0)
+		goto out;
+
+	ret = blkid_do_fullprobe(pr);
+	if (ret < 0)
+		goto out;
+
+	/*
+	 * Blkid returns 1 for nothing found and 0 when it finds a signature,
+	 * but we want the exact opposite, so reverse the return value here.
+	 *
+	 * In addition print some useful diagnostics about what actually is
+	 * on the device.
+	 */
+	if (ret) {
+		ret = 0;
+		goto out;
+	}
+
+	if (!blkid_probe_lookup_value(pr, "TYPE", &type, NULL)) {
+		fprintf(stderr,
+			_("%s: %s appears to contain an existing "
+			"filesystem (%s).\n"), progname, device, type);
+	} else if (!blkid_probe_lookup_value(pr, "PTTYPE", &type, NULL)) {
+		fprintf(stderr,
+			_("%s: %s appears to contain a partition "
+			"table (%s).\n"), progname, device, type);
+	} else {
+		fprintf(stderr,
+			_("%s: %s appears to contain something weird "
+			"according to blkid\n"), progname, device);
+	}
+	ret = 1;
+
+out:
+	if (pr)
+		blkid_free_probe(pr);
+	if (ret == -1)
+		fprintf(stderr,
+			_("%s: probe of %s failed, cannot detect "
+			  "existing filesystem.\n"), progname, device);
+	return ret;
+}
+
+static void
+blkid_get_topology(
+	const char	*device,
+	int		*sunit,
+	int		*swidth,
+	int		*lsectorsize,
+	int		*psectorsize,
+	int		force_overwrite)
+{
+
+	blkid_topology	tp;
+	blkid_probe 	pr;
+	unsigned long	val;
+	struct stat	statbuf;
+
+	/* can't get topology info from a file */
+	if (!stat(device, &statbuf) && S_ISREG(statbuf.st_mode))
+		return;
+
+	pr = blkid_new_probe_from_filename(device);
+	if (!pr)
+		return;
+
+	tp = blkid_probe_get_topology(pr);
+	if (!tp)
+		goto out_free_probe;
+
+	val = blkid_topology_get_logical_sector_size(tp);
+	*lsectorsize = val;
+	val = blkid_topology_get_physical_sector_size(tp);
+	*psectorsize = val;
+	val = blkid_topology_get_minimum_io_size(tp);
+	*sunit = val;
+	val = blkid_topology_get_optimal_io_size(tp);
+	*swidth = val;
+
+	/*
+	 * If the reported values are the same as the physical sector size
+	 * do not bother to report anything.  It will only cause warnings
+	 * if people specify larger stripe units or widths manually.
+	 */
+	if (*sunit == *psectorsize || *swidth == *psectorsize) {
+		*sunit = 0;
+		*swidth = 0;
+	}
+
+	/*
+	 * Blkid reports the information in terms of bytes, but we want it in
+	 * terms of 512 bytes blocks (only to convert it to bytes later..)
+	 */
+	*sunit = *sunit >> 9;
+	*swidth = *swidth >> 9;
+
+	if (blkid_topology_get_alignment_offset(tp) != 0) {
+		fprintf(stderr,
+			_("warning: device is not properly aligned %s\n"),
+			device);
+
+		if (!force_overwrite) {
+			fprintf(stderr,
+				_("Use -f to force usage of a misaligned device\n"));
+
+			exit(EXIT_FAILURE);
+		}
+		/* Do not use physical sector size if the device is misaligned */
+		*psectorsize = *lsectorsize;
+	}
+
+	blkid_free_probe(pr);
+	return;
+
+out_free_probe:
+	blkid_free_probe(pr);
+	fprintf(stderr,
+		_("warning: unable to probe device topology for device %s\n"),
+		device);
+}
+#else /* ifdef ENABLE_BLKID */
+/*
+ * Without blkid, we can't do a good check for signatures.
+ * So instead of some messy attempts, just disable any checks
+ * and always return 'nothing found'.
+ */
+#  warning BLKID is disabled, so signature detection and block device\
+ access are not working!
+
+int
+check_overwrite(
+	char		*device)
+{
+	return 1;
+}
+
+static void
+blkid_get_topology(
+	const char	*device,
+	int		*sunit,
+	int		*swidth,
+	int		*lsectorsize,
+	int		*psectorsize,
+	int		force_overwrite)
+{
+	/*
+	 * Shouldn't make any difference (no blkid = no block device access),
+	 * but make sure this dummy replacement returns with at least some
+	 * sanity.
+	 */
+	*lsectorsize = *psectorsize = 512;
+}
+
+#endif /* ENABLE_BLKID */
+
+
+void
+get_topology(
+	libxfs_init_t		*xi,
+	struct fs_topology	*ft,
+	int			force_overwrite)
+{
+	struct stat statbuf;
+	char *dfile = xi->volname ? xi->volname : xi->dname;
+
+	/*
+	 * If our target is a regular file, use platform_findsizes
+	 * to try to obtain the underlying filesystem's requirements
+	 * for direct IO; we'll set our sector size to that if possible.
+	 */
+	if (xi->disfile ||
+	    (!stat(dfile, &statbuf) && S_ISREG(statbuf.st_mode))) {
+		int fd;
+		int flags = O_RDONLY;
+		long long dummy;
+
+		/* with xi->disfile we may not have the file yet! */
+		if (xi->disfile)
+			flags |= O_CREAT;
+
+		fd = open(dfile, flags, 0666);
+		if (fd >= 0) {
+			platform_findsizes(dfile, fd, &dummy, &ft->lsectorsize);
+			close(fd);
+			ft->psectorsize = ft->lsectorsize;
+		} else
+			ft->psectorsize = ft->lsectorsize = BBSIZE;
+	} else {
+		blkid_get_topology(dfile, &ft->dsunit, &ft->dswidth,
+				   &ft->lsectorsize, &ft->psectorsize,
+				   force_overwrite);
+	}
+
+	if (xi->rtname && !xi->risfile) {
+		int sunit, lsectorsize, psectorsize;
+
+		blkid_get_topology(xi->rtname, &sunit, &ft->rtswidth,
+				   &lsectorsize, &psectorsize, force_overwrite);
+	}
+}
diff --git a/mkfs/Makefile b/mkfs/Makefile
index 570ab07..2997398 100644
--- a/mkfs/Makefile
+++ b/mkfs/Makefile
@@ -10,7 +10,7 @@ LTCOMMAND = mkfs.xfs
 HFILES = xfs_mkfs.h
 CFILES = maxtrres.c proto.c xfs_mkfs.c
 
-LLDLIBS += $(LIBBLKID) $(LIBXFS) $(LIBUUID) $(LIBRT) $(LIBPTHREAD)
+LLDLIBS += $(LIBBLKID) $(LIBXFS) $(LIBUUID) $(LIBRT) $(LIBPTHREAD) $(LIBXCMD)
 LTDEPENDENCIES += $(LIBXFS)
 LLDFLAGS = -static-libtool-libs
 
diff --git a/mkfs/xfs_mkfs.c b/mkfs/xfs_mkfs.c
index 4c3a802..0a02719 100644
--- a/mkfs/xfs_mkfs.c
+++ b/mkfs/xfs_mkfs.c
@@ -22,17 +22,7 @@
 #  include <blkid/blkid.h>
 #endif /* ENABLE_BLKID */
 #include "xfs_mkfs.h"
-
-/*
- * Device topology information.
- */
-struct fs_topology {
-	int	dsunit;		/* stripe unit - data subvolume */
-	int	dswidth;	/* stripe width - data subvolume */
-	int	rtswidth;	/* stripe width - rt subvolume */
-	int	lsectorsize;	/* logical sector size &*/
-	int	psectorsize;	/* physical sector size */
-};
+#include "libxcmd.h"
 
 /*
  * Prototypes for internal functions.
@@ -295,244 +285,6 @@ calc_stripe_factors(
 	}
 }
 
-/*
- * Check for existing filesystem or partition table on device.
- * Returns:
- *	 1 for existing fs or partition
- *	 0 for nothing found
- *	-1 for internal error
- */
-#ifdef ENABLE_BLKID
-static int
-check_overwrite(
-	char		*device)
-{
-	const char	*type;
-	blkid_probe	pr = NULL;
-	int		ret;
-	int		fd;
-	long long	size;
-	int		bsz;
-
-	if (!device || !*device)
-		return 0;
-
-	ret = -1; /* will reset on success of all setup calls */
-
-	fd = open(device, O_RDONLY);
-	if (fd < 0)
-		goto out;
-	platform_findsizes(device, fd, &size, &bsz);
-	close(fd);
-
-	/* nothing to overwrite on a 0-length device */
-	if (size == 0) {
-		ret = 0;
-		goto out;
-	}
-
-	pr = blkid_new_probe_from_filename(device);
-	if (!pr)
-		goto out;
-
-	ret = blkid_probe_enable_partitions(pr, 1);
-	if (ret < 0)
-		goto out;
-
-	ret = blkid_do_fullprobe(pr);
-	if (ret < 0)
-		goto out;
-
-	/*
-	 * Blkid returns 1 for nothing found and 0 when it finds a signature,
-	 * but we want the exact opposite, so reverse the return value here.
-	 *
-	 * In addition print some useful diagnostics about what actually is
-	 * on the device.
-	 */
-	if (ret) {
-		ret = 0;
-		goto out;
-	}
-
-	if (!blkid_probe_lookup_value(pr, "TYPE", &type, NULL)) {
-		fprintf(stderr,
-			_("%s: %s appears to contain an existing "
-			"filesystem (%s).\n"), progname, device, type);
-	} else if (!blkid_probe_lookup_value(pr, "PTTYPE", &type, NULL)) {
-		fprintf(stderr,
-			_("%s: %s appears to contain a partition "
-			"table (%s).\n"), progname, device, type);
-	} else {
-		fprintf(stderr,
-			_("%s: %s appears to contain something weird "
-			"according to blkid\n"), progname, device);
-	}
-	ret = 1;
-
-out:
-	if (pr)
-		blkid_free_probe(pr);
-	if (ret == -1)
-		fprintf(stderr,
-			_("%s: probe of %s failed, cannot detect "
-			  "existing filesystem.\n"), progname, device);
-	return ret;
-}
-
-static void blkid_get_topology(
-	const char	*device,
-	int		*sunit,
-	int		*swidth,
-	int		*lsectorsize,
-	int		*psectorsize,
-	int		force_overwrite)
-{
-
-	blkid_topology tp;
-	blkid_probe pr;
-	unsigned long val;
-	struct stat statbuf;
-
-	/* can't get topology info from a file */
-	if (!stat(device, &statbuf) && S_ISREG(statbuf.st_mode))
-		return;
-
-	pr = blkid_new_probe_from_filename(device);
-	if (!pr)
-		return;
-
-	tp = blkid_probe_get_topology(pr);
-	if (!tp)
-		goto out_free_probe;
-
-	val = blkid_topology_get_logical_sector_size(tp);
-	*lsectorsize = val;
-	val = blkid_topology_get_physical_sector_size(tp);
-	*psectorsize = val;
-	val = blkid_topology_get_minimum_io_size(tp);
-	*sunit = val;
-	val = blkid_topology_get_optimal_io_size(tp);
-	*swidth = val;
-
-	/*
-	 * If the reported values are the same as the physical sector size
-	 * do not bother to report anything.  It will only cause warnings
-	 * if people specify larger stripe units or widths manually.
-	 */
-	if (*sunit == *psectorsize || *swidth == *psectorsize) {
-		*sunit = 0;
-		*swidth = 0;
-	}
-
-	/*
-	 * Blkid reports the information in terms of bytes, but we want it in
-	 * terms of 512 bytes blocks (only to convert it to bytes later..)
-	 */
-	*sunit = *sunit >> 9;
-	*swidth = *swidth >> 9;
-
-	if (blkid_topology_get_alignment_offset(tp) != 0) {
-		fprintf(stderr,
-			_("warning: device is not properly aligned %s\n"),
-			device);
-
-		if (!force_overwrite) {
-			fprintf(stderr,
-				_("Use -f to force usage of a misaligned device\n"));
-
-			exit(EXIT_FAILURE);
-		}
-		/* Do not use physical sector size if the device is misaligned */
-		*psectorsize = *lsectorsize;
-	}
-
-	blkid_free_probe(pr);
-	return;
-
-out_free_probe:
-	blkid_free_probe(pr);
-	fprintf(stderr,
-		_("warning: unable to probe device topology for device %s\n"),
-		device);
-}
-#else /* ifdef ENABLE_BLKID */
-/*
- * Without blkid, we can't do a good check for signatures.
- * So instead of some messy attempts, just disable any checks
- * and always return 'nothing found'.
- */
-#  warning BLKID is disabled, so signature detection and block device\
- access are not working!
-static int
-check_overwrite(
-	char		*device)
-{
-	return 1;
-}
-
-static void blkid_get_topology(
-	const char	*device,
-	int		*sunit,
-	int		*swidth,
-	int		*lsectorsize,
-	int		*psectorsize,
-	int		force_overwrite)
-{
-	/*
-	 * Shouldn't make any difference (no blkid = no block device access),
-	 * but make sure this dummy replacement returns with at least some
-	 * sanity.
-	 */
-	*lsectorsize = *psectorsize = 512;
-}
-
-#endif /* ENABLE_BLKID */
-
-static void get_topology(
-	libxfs_init_t		*xi,
-	struct fs_topology	*ft,
-	int			force_overwrite)
-{
-	struct stat statbuf;
-	char *dfile = xi->volname ? xi->volname : xi->dname;
-
-	/*
-	 * If our target is a regular file, use platform_findsizes
-	 * to try to obtain the underlying filesystem's requirements
-	 * for direct IO; we'll set our sector size to that if possible.
-	 */
-	if (xi->disfile ||
-	    (!stat(dfile, &statbuf) && S_ISREG(statbuf.st_mode))) {
-		int fd;
-		int flags = O_RDONLY;
-		long long dummy;
-
-		/* with xi->disfile we may not have the file yet! */
-		if (xi->disfile)
-			flags |= O_CREAT;
-
-		fd = open(dfile, flags, 0666);
-		if (fd >= 0) {
-			platform_findsizes(dfile, fd, &dummy, &ft->lsectorsize);
-			close(fd);
-			ft->psectorsize = ft->lsectorsize;
-		} else
-			ft->psectorsize = ft->lsectorsize = BBSIZE;
-	} else {
-		blkid_get_topology(dfile, &ft->dsunit, &ft->dswidth,
-				   &ft->lsectorsize, &ft->psectorsize,
-				   force_overwrite);
-	}
-
-	if (xi->rtname && !xi->risfile) {
-		int sunit, lsectorsize, psectorsize;
-
-		blkid_get_topology(xi->rtname, &sunit, &ft->rtswidth,
-				   &lsectorsize, &psectorsize, force_overwrite);
-	}
-}
-
 static void
 fixup_log_stripe_unit(
 	int		lsflag,
@@ -640,77 +392,6 @@ calc_default_imaxpct(
 	return 1;
 }
 
-
-void
-calc_default_ag_geometry(
-	int		blocklog,
-	__uint64_t	dblocks,
-	int		multidisk,
-	__uint64_t	*agsize,
-	__uint64_t	*agcount)
-{
-	__uint64_t	blocks = 0;
-	int		shift = 0;
-
-	/*
-	 * First handle the high extreme - the point at which we will
-	 * always use the maximum AG size.
-	 *
-	 * This applies regardless of storage configuration.
-	 */
-	if (dblocks >= TERABYTES(32, blocklog)) {
-		blocks = XFS_AG_MAX_BLOCKS(blocklog);
-		goto done;
-	}
-
-	/*
-	 * For the remainder we choose an AG size based on the
-	 * number of data blocks available, trying to keep the
-	 * number of AGs relatively small (especially compared
-	 * to the original algorithm).  AG count is calculated
-	 * based on the preferred AG size, not vice-versa - the
-	 * count can be increased by growfs, so prefer to use
-	 * smaller counts at mkfs time.
-	 *
-	 * For a single underlying storage device between 128MB
-	 * and 4TB in size, just use 4 AGs, otherwise scale up
-	 * smoothly between min/max AG sizes.
-	 */
-
-	if (!multidisk && dblocks >= MEGABYTES(128, blocklog)) {
-		if (dblocks >= TERABYTES(4, blocklog)) {
-			blocks = XFS_AG_MAX_BLOCKS(blocklog);
-			goto done;
-		}
-		shift = 2;
-	} else if (dblocks > GIGABYTES(512, blocklog))
-		shift = 5;
-	else if (dblocks > GIGABYTES(8, blocklog))
-		shift = 4;
-	else if (dblocks >= MEGABYTES(128, blocklog))
-		shift = 3;
-	else if (dblocks >= MEGABYTES(64, blocklog))
-		shift = 2;
-	else if (dblocks >= MEGABYTES(32, blocklog))
-		shift = 1;
-	else
-		shift = 0;
-	/*
-	 * If dblocks is not evenly divisible by the number of
-	 * desired AGs, round "blocks" up so we don't lose the
-	 * last bit of the filesystem. The same principle applies
-	 * to the AG count, so we don't lose the last AG!
-	 */
-	blocks = dblocks >> shift;
-	if (dblocks & xfs_mask32lo(shift)) {
-		if (blocks < XFS_AG_MAX_BLOCKS(blocklog))
-		    blocks++;
-	}
-done:
-	*agsize = blocks;
-	*agcount = dblocks / blocks + (dblocks % blocks != 0);
-}
-
 static void
 validate_ag_geometry(
 	int		blocklog,
-- 
2.5.0

_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs

^ permalink raw reply related	[flat|nested] 11+ messages in thread

* [PATCH v4 2/2] xfs_repair: new secondary superblock search method
  2016-02-12 19:03 [PATCH 0/2 v4] xfs_repair: improved secondary sb search Bill O'Donnell
  2016-02-12 19:03 ` [PATCH v4 1/2] libxcmd: generalize topology functions Bill O'Donnell
@ 2016-02-12 19:03 ` Bill O'Donnell
  2016-02-13  3:12   ` Eric Sandeen
  1 sibling, 1 reply; 11+ messages in thread
From: Bill O'Donnell @ 2016-02-12 19:03 UTC (permalink / raw)
  To: xfs

Optimize secondary sb search, using similar method to find
fs geometry as that of xfs_mkfs. If this faster method fails
in finding a secondary sb, fall back to original brute force
slower search.

Signed-off-by: Bill O'Donnell <billodo@redhat.com>
---
 Makefile           |  2 +-
 include/libxcmd.h  |  6 +++++-
 libxcmd/topology.c | 27 +++++++++++++++++++++++++++
 repair/Makefile    |  4 ++--
 repair/sb.c        | 49 +++++++++++++++++++++++++++++++++++++++++--------
 5 files changed, 76 insertions(+), 12 deletions(-)

diff --git a/Makefile b/Makefile
index fca0a42..1d60d9c 100644
--- a/Makefile
+++ b/Makefile
@@ -80,7 +80,7 @@ fsr: libhandle
 growfs: libxcmd
 io: libxcmd libhandle
 quota: libxcmd
-repair: libxlog
+repair: libxlog libxcmd
 copy: libxlog
 
 ifeq ($(HAVE_BUILDDEFS), yes)
diff --git a/include/libxcmd.h b/include/libxcmd.h
index a912534..eb16cd5 100644
--- a/include/libxcmd.h
+++ b/include/libxcmd.h
@@ -51,6 +51,10 @@ extern int
 check_overwrite(
 	char		*device);
 
-
+extern int
+guess_default_geometry(
+	__uint64_t	*agsize,
+	__uint64_t	*agcount,
+	libxfs_init_t	x);
 
 #endif	/* __LIBXCMD_H__ */
diff --git a/libxcmd/topology.c b/libxcmd/topology.c
index 4cbe4b1..ec9695c 100644
--- a/libxcmd/topology.c
+++ b/libxcmd/topology.c
@@ -339,3 +339,30 @@ get_topology(
 				   &lsectorsize, &psectorsize, force_overwrite);
 	}
 }
+
+int
+guess_default_geometry(
+	__uint64_t	*agsize,
+	__uint64_t	*agcount,
+	libxfs_init_t	x)
+{
+	struct fs_topology	ft;
+	int			blocklog;
+	__uint64_t		dblocks;
+	int			multidisk;
+
+	memset(&ft, 0, sizeof(ft));
+	get_topology(&x, &ft, 1);
+
+	/*
+	 * get geometry from get_topology result.
+	 * Use default block size (2^12)
+	 */
+	blocklog = 12;
+	multidisk = ft.dswidth | ft.dsunit;
+	dblocks = x.dsize >> (blocklog - BBSHIFT);
+	calc_default_ag_geometry(blocklog, dblocks, multidisk,
+				 agsize, agcount);
+
+	return blocklog;
+}
diff --git a/repair/Makefile b/repair/Makefile
index 251722b..d24ab1f 100644
--- a/repair/Makefile
+++ b/repair/Makefile
@@ -20,8 +20,8 @@ CFILES = agheader.c attr_repair.c avl.c avl64.c bmap.c btree.c \
 	progress.c prefetch.c rt.c sb.c scan.c threads.c \
 	versions.c xfs_repair.c
 
-LLDLIBS = $(LIBXFS) $(LIBXLOG) $(LIBUUID) $(LIBRT) $(LIBPTHREAD)
-LTDEPENDENCIES = $(LIBXFS) $(LIBXLOG)
+LLDLIBS = $(LIBBLKID) $(LIBXFS) $(LIBXLOG) $(LIBUUID) $(LIBRT) $(LIBPTHREAD) $(LIBXCMD)
+LTDEPENDENCIES = $(LIBXFS) $(LIBXLOG) $(LIBXCMD)
 LLDFLAGS = -static-libtool-libs
 
 default: depend $(LTCOMMAND)
diff --git a/repair/sb.c b/repair/sb.c
index 4eef14a..8dcad5f 100644
--- a/repair/sb.c
+++ b/repair/sb.c
@@ -17,6 +17,7 @@
  */
 
 #include "libxfs.h"
+#include "libxcmd.h"
 #include "libxlog.h"
 #include "agheader.h"
 #include "globals.h"
@@ -85,10 +86,15 @@ copy_sb(xfs_sb_t *source, xfs_sb_t *dest)
 }
 
 /*
- * find a secondary superblock, copy it into the sb buffer
+ * find a secondary superblock, copy it into the sb buffer.
+ * start is the point to begin reading BSIZE bytes.
+ * skip contains a byte-count of how far to advance for next read.
  */
-int
-find_secondary_sb(xfs_sb_t *rsb)
+static int
+__find_secondary_sb(
+	xfs_sb_t *rsb,
+	__uint64_t start,
+	__uint64_t skip)
 {
 	xfs_off_t	off;
 	xfs_sb_t	*sb;
@@ -101,7 +107,6 @@ find_secondary_sb(xfs_sb_t *rsb)
 	int		bsize;
 
 	do_warn(_("\nattempting to find secondary superblock...\n"));
-
 	sb = (xfs_sb_t *)memalign(libxfs_device_alignment(), BSIZE);
 	if (!sb) {
 		do_error(
@@ -117,7 +122,7 @@ find_secondary_sb(xfs_sb_t *rsb)
 	/*
 	 * skip first sector since we know that's bad
 	 */
-	for (done = 0, off = XFS_AG_MIN_BYTES; !done ; off += bsize)  {
+	for (done = 0, off = start; !done ; off += skip)  {
 		/*
 		 * read disk 1 MByte at a time.
 		 */
@@ -128,9 +133,7 @@ find_secondary_sb(xfs_sb_t *rsb)
 		if (!done && (bsize = read(x.dfd, sb, BSIZE)) <= 0)  {
 			done = 1;
 		}
-
 		do_warn(".");
-
 		/*
 		 * check the buffer 512 bytes at a time since
 		 * we don't know how big the sectors really are.
@@ -166,7 +169,37 @@ find_secondary_sb(xfs_sb_t *rsb)
 	}
 
 	free(sb);
-	return(retval);
+	return retval;
+}
+
+int
+find_secondary_sb(xfs_sb_t *rsb)
+{
+	int		retval;
+	__uint64_t	agcount;
+	__uint64_t	agsize;
+	__uint64_t	skip;
+	int		blocklog;
+
+	/*
+	 * Attempt to find secondary sb with a coarse approach,
+	 * using a large skip (agsize in bytes). Failing that,
+	 * fallback to the fine-grained approach using min agsize.
+	 */
+	blocklog = guess_default_geometry(&agsize, &agcount, x);
+
+	/*
+	 * use found ag geometry to quickly find secondary sb
+	 */
+	skip = agsize << blocklog;
+	retval = __find_secondary_sb(rsb, skip, skip);
+	if (!retval)  {
+		/*
+		 * fallback: use minimum agsize for skipsize
+		 */
+		retval = __find_secondary_sb(rsb, XFS_AG_MIN_BYTES, BSIZE);
+	}
+	return retval;
 }
 
 /*
-- 
2.5.0

_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs

^ permalink raw reply related	[flat|nested] 11+ messages in thread

* Re: [PATCH v4 1/2] libxcmd: generalize topology functions
  2016-02-12 19:03 ` [PATCH v4 1/2] libxcmd: generalize topology functions Bill O'Donnell
@ 2016-02-12 19:39   ` Christoph Hellwig
  2016-02-12 19:47     ` Bill O'Donnell
  2016-02-13  2:43   ` Eric Sandeen
  1 sibling, 1 reply; 11+ messages in thread
From: Christoph Hellwig @ 2016-02-12 19:39 UTC (permalink / raw)
  To: Bill O'Donnell; +Cc: xfs

On Fri, Feb 12, 2016 at 01:03:30PM -0600, Bill O'Donnell wrote:
> Move general topology functions from xfs_mkfs to new topology
> collection in libxcmd.

Why libxcmd and not libxfs?

_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH v4 1/2] libxcmd: generalize topology functions
  2016-02-12 19:39   ` Christoph Hellwig
@ 2016-02-12 19:47     ` Bill O'Donnell
  2016-02-13  2:22       ` Eric Sandeen
  0 siblings, 1 reply; 11+ messages in thread
From: Bill O'Donnell @ 2016-02-12 19:47 UTC (permalink / raw)
  To: Christoph Hellwig; +Cc: xfs

On Fri, Feb 12, 2016 at 11:39:42AM -0800, Christoph Hellwig wrote:
> On Fri, Feb 12, 2016 at 01:03:30PM -0600, Bill O'Donnell wrote:
> > Move general topology functions from xfs_mkfs to new topology
> > collection in libxcmd.
> 
> Why libxcmd and not libxfs?

We discussed that a bit. Since libxfs has the kernel support code, and
not generally the xfsprogs library, I put it in libxcmd.

_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH v4 1/2] libxcmd: generalize topology functions
  2016-02-12 19:47     ` Bill O'Donnell
@ 2016-02-13  2:22       ` Eric Sandeen
  0 siblings, 0 replies; 11+ messages in thread
From: Eric Sandeen @ 2016-02-13  2:22 UTC (permalink / raw)
  To: xfs



On 2/12/16 1:47 PM, Bill O'Donnell wrote:
> On Fri, Feb 12, 2016 at 11:39:42AM -0800, Christoph Hellwig wrote:
>> On Fri, Feb 12, 2016 at 01:03:30PM -0600, Bill O'Donnell wrote:
>>> Move general topology functions from xfs_mkfs to new topology
>>> collection in libxcmd.
>>
>> Why libxcmd and not libxfs?
> 
> We discussed that a bit. Since libxfs has the kernel support code, and
> not generally the xfsprogs library, I put it in libxcmd.

Yep, Dave asked for libxcmd, to keep stuff that's not shared with
the kernel away from libxfs.

-Eric

_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH v4 1/2] libxcmd: generalize topology functions
  2016-02-12 19:03 ` [PATCH v4 1/2] libxcmd: generalize topology functions Bill O'Donnell
  2016-02-12 19:39   ` Christoph Hellwig
@ 2016-02-13  2:43   ` Eric Sandeen
  1 sibling, 0 replies; 11+ messages in thread
From: Eric Sandeen @ 2016-02-13  2:43 UTC (permalink / raw)
  To: xfs

On 2/12/16 1:03 PM, Bill O'Donnell wrote:
> Move general topology functions from xfs_mkfs to new topology
> collection in libxcmd.
> 
> Signed-off-by: Bill O'Donnell <billodo@redhat.com>

Reviewed-by: Eric Sandeen <sandeen@redhat.com>

> ---
>  include/Makefile   |   1 +
>  include/libxcmd.h  |  56 +++++++++
>  libxcmd/Makefile   |   2 +-
>  libxcmd/topology.c | 341 +++++++++++++++++++++++++++++++++++++++++++++++++++++
>  mkfs/Makefile      |   2 +-
>  mkfs/xfs_mkfs.c    | 321 +------------------------------------------------
>  6 files changed, 401 insertions(+), 322 deletions(-)
>  create mode 100644 include/libxcmd.h
>  create mode 100644 libxcmd/topology.c
> 
> diff --git a/include/Makefile b/include/Makefile
> index 6148756..2671f07 100644
> --- a/include/Makefile
> +++ b/include/Makefile
> @@ -20,6 +20,7 @@ include $(TOPDIR)/include/builddefs
>  
>  LIBHFILES = libxfs.h \
>  	libxlog.h \
> +	libxcmd.h \
>  	atomic.h \
>  	bitops.h \
>  	cache.h \
> diff --git a/include/libxcmd.h b/include/libxcmd.h
> new file mode 100644
> index 0000000..a912534
> --- /dev/null
> +++ b/include/libxcmd.h
> @@ -0,0 +1,56 @@
> +/*
> + * Copyright (c) 2000-2005 Silicon Graphics, Inc.
> + * All Rights Reserved.
> + *
> + * This program is free software; you can redistribute it and/or
> + * modify it under the terms of the GNU General Public License as
> + * published by the Free Software Foundation.
> + *
> + * This program is distributed in the hope that it would be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> + * GNU General Public License for more details.
> + *
> + * You should have received a copy of the GNU General Public License
> + * along with this program; if not, write the Free Software Foundation,
> + * Inc.,  51 Franklin St, Fifth Floor, Boston, MA  02110-1301  USA
> + */
> +
> +#ifndef __LIBXCMD_H__
> +#define __LIBXCMD_H__
> +
> +#include "libxfs.h"
> +#include <sys/time.h>
> +
> +/*
> + * Device topology information.
> + */
> +typedef struct fs_topology {
> +	int	dsunit;		/* stripe unit - data subvolume */
> +	int	dswidth;	/* stripe width - data subvolume */
> +	int	rtswidth;	/* stripe width - rt subvolume */
> +	int	lsectorsize;	/* logical sector size &*/
> +	int	psectorsize;	/* physical sector size */
> +} fs_topology_t;
> +
> +extern void
> +get_topology(
> +	libxfs_init_t		*xi,
> +	struct fs_topology	*ft,
> +	int			force_overwrite);
> +
> +extern void
> +calc_default_ag_geometry(
> +	int		blocklog,
> +	__uint64_t	dblocks,
> +	int		multidisk,
> +	__uint64_t	*agsize,
> +	__uint64_t	*agcount);
> +
> +extern int
> +check_overwrite(
> +	char		*device);
> +
> +
> +
> +#endif	/* __LIBXCMD_H__ */
> diff --git a/libxcmd/Makefile b/libxcmd/Makefile
> index 7701ed9..aab8d6d 100644
> --- a/libxcmd/Makefile
> +++ b/libxcmd/Makefile
> @@ -10,7 +10,7 @@ LT_CURRENT = 0
>  LT_REVISION = 0
>  LT_AGE = 0
>  
> -CFILES = command.c input.c paths.c projects.c help.c quit.c
> +CFILES = command.c input.c paths.c projects.c help.c quit.c topology.c
>  
>  ifeq ($(HAVE_GETMNTENT),yes)
>  LCFLAGS += -DHAVE_GETMNTENT
> diff --git a/libxcmd/topology.c b/libxcmd/topology.c
> new file mode 100644
> index 0000000..4cbe4b1
> --- /dev/null
> +++ b/libxcmd/topology.c
> @@ -0,0 +1,341 @@
> +/*
> + * Copyright (c) 2000-2001,2005 Silicon Graphics, Inc.
> + * All Rights Reserved.
> + *
> + * This program is free software; you can redistribute it and/or
> + * modify it under the terms of the GNU General Public License as
> + * published by the Free Software Foundation.
> + *
> + * This program is distributed in the hope that it would be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> + * GNU General Public License for more details.
> + *
> + * You should have received a copy of the GNU General Public License
> + * along with this program; if not, write the Free Software Foundation,
> + * Inc.,  51 Franklin St, Fifth Floor, Boston, MA  02110-1301  USA
> + */
> +
> +#include "libxfs.h"
> +#include "libxcmd.h"
> +#ifdef ENABLE_BLKID
> +#  include <blkid/blkid.h>
> +#endif /* ENABLE_BLKID */
> +#include "../mkfs/xfs_mkfs.h"
> +
> +#define TERABYTES(count, blog)	((__uint64_t)(count) << (40 - (blog)))
> +#define GIGABYTES(count, blog)	((__uint64_t)(count) << (30 - (blog)))
> +#define MEGABYTES(count, blog)	((__uint64_t)(count) << (20 - (blog)))
> +
> +void
> +calc_default_ag_geometry(
> +	int		blocklog,
> +	__uint64_t	dblocks,
> +	int		multidisk,
> +	__uint64_t	*agsize,
> +	__uint64_t	*agcount)
> +{
> +	__uint64_t	blocks = 0;
> +	int		shift = 0;
> +
> +	/*
> +	 * First handle the high extreme - the point at which we will
> +	 * always use the maximum AG size.
> +	 *
> +	 * This applies regardless of storage configuration.
> +	 */
> +	if (dblocks >= TERABYTES(32, blocklog)) {
> +		blocks = XFS_AG_MAX_BLOCKS(blocklog);
> +		goto done;
> +	}
> +
> +	/*
> +	 * For the remainder we choose an AG size based on the
> +	 * number of data blocks available, trying to keep the
> +	 * number of AGs relatively small (especially compared
> +	 * to the original algorithm).  AG count is calculated
> +	 * based on the preferred AG size, not vice-versa - the
> +	 * count can be increased by growfs, so prefer to use
> +	 * smaller counts at mkfs time.
> +	 *
> +	 * For a single underlying storage device between 128MB
> +	 * and 4TB in size, just use 4 AGs, otherwise scale up
> +	 * smoothly between min/max AG sizes.
> +	 */
> +
> +	if (!multidisk && dblocks >= MEGABYTES(128, blocklog)) {
> +		if (dblocks >= TERABYTES(4, blocklog)) {
> +			blocks = XFS_AG_MAX_BLOCKS(blocklog);
> +			goto done;
> +		}
> +		shift = 2;
> +	} else if (dblocks > GIGABYTES(512, blocklog))
> +		shift = 5;
> +	else if (dblocks > GIGABYTES(8, blocklog))
> +		shift = 4;
> +	else if (dblocks >= MEGABYTES(128, blocklog))
> +		shift = 3;
> +	else if (dblocks >= MEGABYTES(64, blocklog))
> +		shift = 2;
> +	else if (dblocks >= MEGABYTES(32, blocklog))
> +		shift = 1;
> +	else
> +		shift = 0;
> +	/*
> +	 * If dblocks is not evenly divisible by the number of
> +	 * desired AGs, round "blocks" up so we don't lose the
> +	 * last bit of the filesystem. The same principle applies
> +	 * to the AG count, so we don't lose the last AG!
> +	 */
> +	blocks = dblocks >> shift;
> +	if (dblocks & xfs_mask32lo(shift)) {
> +		if (blocks < XFS_AG_MAX_BLOCKS(blocklog))
> +		    blocks++;
> +	}
> +done:
> +	*agsize = blocks;
> +	*agcount = dblocks / blocks + (dblocks % blocks != 0);
> +}
> +
> +/*
> + * Check for existing filesystem or partition table on device.
> + * Returns:
> + *	 1 for existing fs or partition
> + *	 0 for nothing found
> + *	-1 for internal error
> + */
> +#ifdef ENABLE_BLKID
> +int
> +check_overwrite(
> +	char		*device)
> +{
> +	const char	*type;
> +	blkid_probe	pr = NULL;
> +	int		ret;
> +	int		fd;
> +	long long	size;
> +	int		bsz;
> +
> +	if (!device || !*device)
> +		return 0;
> +
> +	ret = -1; /* will reset on success of all setup calls */
> +
> +	fd = open(device, O_RDONLY);
> +	if (fd < 0)
> +		goto out;
> +	platform_findsizes(device, fd, &size, &bsz);
> +	close(fd);
> +
> +	/* nothing to overwrite on a 0-length device */
> +	if (size == 0) {
> +		ret = 0;
> +		goto out;
> +	}
> +
> +	pr = blkid_new_probe_from_filename(device);
> +	if (!pr)
> +		goto out;
> +
> +	ret = blkid_probe_enable_partitions(pr, 1);
> +	if (ret < 0)
> +		goto out;
> +
> +	ret = blkid_do_fullprobe(pr);
> +	if (ret < 0)
> +		goto out;
> +
> +	/*
> +	 * Blkid returns 1 for nothing found and 0 when it finds a signature,
> +	 * but we want the exact opposite, so reverse the return value here.
> +	 *
> +	 * In addition print some useful diagnostics about what actually is
> +	 * on the device.
> +	 */
> +	if (ret) {
> +		ret = 0;
> +		goto out;
> +	}
> +
> +	if (!blkid_probe_lookup_value(pr, "TYPE", &type, NULL)) {
> +		fprintf(stderr,
> +			_("%s: %s appears to contain an existing "
> +			"filesystem (%s).\n"), progname, device, type);
> +	} else if (!blkid_probe_lookup_value(pr, "PTTYPE", &type, NULL)) {
> +		fprintf(stderr,
> +			_("%s: %s appears to contain a partition "
> +			"table (%s).\n"), progname, device, type);
> +	} else {
> +		fprintf(stderr,
> +			_("%s: %s appears to contain something weird "
> +			"according to blkid\n"), progname, device);
> +	}
> +	ret = 1;
> +
> +out:
> +	if (pr)
> +		blkid_free_probe(pr);
> +	if (ret == -1)
> +		fprintf(stderr,
> +			_("%s: probe of %s failed, cannot detect "
> +			  "existing filesystem.\n"), progname, device);
> +	return ret;
> +}
> +
> +static void
> +blkid_get_topology(
> +	const char	*device,
> +	int		*sunit,
> +	int		*swidth,
> +	int		*lsectorsize,
> +	int		*psectorsize,
> +	int		force_overwrite)
> +{
> +
> +	blkid_topology	tp;
> +	blkid_probe 	pr;
> +	unsigned long	val;
> +	struct stat	statbuf;
> +
> +	/* can't get topology info from a file */
> +	if (!stat(device, &statbuf) && S_ISREG(statbuf.st_mode))
> +		return;
> +
> +	pr = blkid_new_probe_from_filename(device);
> +	if (!pr)
> +		return;
> +
> +	tp = blkid_probe_get_topology(pr);
> +	if (!tp)
> +		goto out_free_probe;
> +
> +	val = blkid_topology_get_logical_sector_size(tp);
> +	*lsectorsize = val;
> +	val = blkid_topology_get_physical_sector_size(tp);
> +	*psectorsize = val;
> +	val = blkid_topology_get_minimum_io_size(tp);
> +	*sunit = val;
> +	val = blkid_topology_get_optimal_io_size(tp);
> +	*swidth = val;
> +
> +	/*
> +	 * If the reported values are the same as the physical sector size
> +	 * do not bother to report anything.  It will only cause warnings
> +	 * if people specify larger stripe units or widths manually.
> +	 */
> +	if (*sunit == *psectorsize || *swidth == *psectorsize) {
> +		*sunit = 0;
> +		*swidth = 0;
> +	}
> +
> +	/*
> +	 * Blkid reports the information in terms of bytes, but we want it in
> +	 * terms of 512 bytes blocks (only to convert it to bytes later..)
> +	 */
> +	*sunit = *sunit >> 9;
> +	*swidth = *swidth >> 9;
> +
> +	if (blkid_topology_get_alignment_offset(tp) != 0) {
> +		fprintf(stderr,
> +			_("warning: device is not properly aligned %s\n"),
> +			device);
> +
> +		if (!force_overwrite) {
> +			fprintf(stderr,
> +				_("Use -f to force usage of a misaligned device\n"));
> +
> +			exit(EXIT_FAILURE);
> +		}
> +		/* Do not use physical sector size if the device is misaligned */
> +		*psectorsize = *lsectorsize;
> +	}
> +
> +	blkid_free_probe(pr);
> +	return;
> +
> +out_free_probe:
> +	blkid_free_probe(pr);
> +	fprintf(stderr,
> +		_("warning: unable to probe device topology for device %s\n"),
> +		device);
> +}
> +#else /* ifdef ENABLE_BLKID */
> +/*
> + * Without blkid, we can't do a good check for signatures.
> + * So instead of some messy attempts, just disable any checks
> + * and always return 'nothing found'.
> + */
> +#  warning BLKID is disabled, so signature detection and block device\
> + access are not working!
> +
> +int
> +check_overwrite(
> +	char		*device)
> +{
> +	return 1;
> +}
> +
> +static void
> +blkid_get_topology(
> +	const char	*device,
> +	int		*sunit,
> +	int		*swidth,
> +	int		*lsectorsize,
> +	int		*psectorsize,
> +	int		force_overwrite)
> +{
> +	/*
> +	 * Shouldn't make any difference (no blkid = no block device access),
> +	 * but make sure this dummy replacement returns with at least some
> +	 * sanity.
> +	 */
> +	*lsectorsize = *psectorsize = 512;
> +}
> +
> +#endif /* ENABLE_BLKID */
> +
> +
> +void
> +get_topology(
> +	libxfs_init_t		*xi,
> +	struct fs_topology	*ft,
> +	int			force_overwrite)
> +{
> +	struct stat statbuf;
> +	char *dfile = xi->volname ? xi->volname : xi->dname;
> +
> +	/*
> +	 * If our target is a regular file, use platform_findsizes
> +	 * to try to obtain the underlying filesystem's requirements
> +	 * for direct IO; we'll set our sector size to that if possible.
> +	 */
> +	if (xi->disfile ||
> +	    (!stat(dfile, &statbuf) && S_ISREG(statbuf.st_mode))) {
> +		int fd;
> +		int flags = O_RDONLY;
> +		long long dummy;
> +
> +		/* with xi->disfile we may not have the file yet! */
> +		if (xi->disfile)
> +			flags |= O_CREAT;
> +
> +		fd = open(dfile, flags, 0666);
> +		if (fd >= 0) {
> +			platform_findsizes(dfile, fd, &dummy, &ft->lsectorsize);
> +			close(fd);
> +			ft->psectorsize = ft->lsectorsize;
> +		} else
> +			ft->psectorsize = ft->lsectorsize = BBSIZE;
> +	} else {
> +		blkid_get_topology(dfile, &ft->dsunit, &ft->dswidth,
> +				   &ft->lsectorsize, &ft->psectorsize,
> +				   force_overwrite);
> +	}
> +
> +	if (xi->rtname && !xi->risfile) {
> +		int sunit, lsectorsize, psectorsize;
> +
> +		blkid_get_topology(xi->rtname, &sunit, &ft->rtswidth,
> +				   &lsectorsize, &psectorsize, force_overwrite);
> +	}
> +}
> diff --git a/mkfs/Makefile b/mkfs/Makefile
> index 570ab07..2997398 100644
> --- a/mkfs/Makefile
> +++ b/mkfs/Makefile
> @@ -10,7 +10,7 @@ LTCOMMAND = mkfs.xfs
>  HFILES = xfs_mkfs.h
>  CFILES = maxtrres.c proto.c xfs_mkfs.c
>  
> -LLDLIBS += $(LIBBLKID) $(LIBXFS) $(LIBUUID) $(LIBRT) $(LIBPTHREAD)
> +LLDLIBS += $(LIBBLKID) $(LIBXFS) $(LIBUUID) $(LIBRT) $(LIBPTHREAD) $(LIBXCMD)
>  LTDEPENDENCIES += $(LIBXFS)
>  LLDFLAGS = -static-libtool-libs
>  
> diff --git a/mkfs/xfs_mkfs.c b/mkfs/xfs_mkfs.c
> index 4c3a802..0a02719 100644
> --- a/mkfs/xfs_mkfs.c
> +++ b/mkfs/xfs_mkfs.c
> @@ -22,17 +22,7 @@
>  #  include <blkid/blkid.h>
>  #endif /* ENABLE_BLKID */
>  #include "xfs_mkfs.h"
> -
> -/*
> - * Device topology information.
> - */
> -struct fs_topology {
> -	int	dsunit;		/* stripe unit - data subvolume */
> -	int	dswidth;	/* stripe width - data subvolume */
> -	int	rtswidth;	/* stripe width - rt subvolume */
> -	int	lsectorsize;	/* logical sector size &*/
> -	int	psectorsize;	/* physical sector size */
> -};
> +#include "libxcmd.h"
>  
>  /*
>   * Prototypes for internal functions.
> @@ -295,244 +285,6 @@ calc_stripe_factors(
>  	}
>  }
>  
> -/*
> - * Check for existing filesystem or partition table on device.
> - * Returns:
> - *	 1 for existing fs or partition
> - *	 0 for nothing found
> - *	-1 for internal error
> - */
> -#ifdef ENABLE_BLKID
> -static int
> -check_overwrite(
> -	char		*device)
> -{
> -	const char	*type;
> -	blkid_probe	pr = NULL;
> -	int		ret;
> -	int		fd;
> -	long long	size;
> -	int		bsz;
> -
> -	if (!device || !*device)
> -		return 0;
> -
> -	ret = -1; /* will reset on success of all setup calls */
> -
> -	fd = open(device, O_RDONLY);
> -	if (fd < 0)
> -		goto out;
> -	platform_findsizes(device, fd, &size, &bsz);
> -	close(fd);
> -
> -	/* nothing to overwrite on a 0-length device */
> -	if (size == 0) {
> -		ret = 0;
> -		goto out;
> -	}
> -
> -	pr = blkid_new_probe_from_filename(device);
> -	if (!pr)
> -		goto out;
> -
> -	ret = blkid_probe_enable_partitions(pr, 1);
> -	if (ret < 0)
> -		goto out;
> -
> -	ret = blkid_do_fullprobe(pr);
> -	if (ret < 0)
> -		goto out;
> -
> -	/*
> -	 * Blkid returns 1 for nothing found and 0 when it finds a signature,
> -	 * but we want the exact opposite, so reverse the return value here.
> -	 *
> -	 * In addition print some useful diagnostics about what actually is
> -	 * on the device.
> -	 */
> -	if (ret) {
> -		ret = 0;
> -		goto out;
> -	}
> -
> -	if (!blkid_probe_lookup_value(pr, "TYPE", &type, NULL)) {
> -		fprintf(stderr,
> -			_("%s: %s appears to contain an existing "
> -			"filesystem (%s).\n"), progname, device, type);
> -	} else if (!blkid_probe_lookup_value(pr, "PTTYPE", &type, NULL)) {
> -		fprintf(stderr,
> -			_("%s: %s appears to contain a partition "
> -			"table (%s).\n"), progname, device, type);
> -	} else {
> -		fprintf(stderr,
> -			_("%s: %s appears to contain something weird "
> -			"according to blkid\n"), progname, device);
> -	}
> -	ret = 1;
> -
> -out:
> -	if (pr)
> -		blkid_free_probe(pr);
> -	if (ret == -1)
> -		fprintf(stderr,
> -			_("%s: probe of %s failed, cannot detect "
> -			  "existing filesystem.\n"), progname, device);
> -	return ret;
> -}
> -
> -static void blkid_get_topology(
> -	const char	*device,
> -	int		*sunit,
> -	int		*swidth,
> -	int		*lsectorsize,
> -	int		*psectorsize,
> -	int		force_overwrite)
> -{
> -
> -	blkid_topology tp;
> -	blkid_probe pr;
> -	unsigned long val;
> -	struct stat statbuf;
> -
> -	/* can't get topology info from a file */
> -	if (!stat(device, &statbuf) && S_ISREG(statbuf.st_mode))
> -		return;
> -
> -	pr = blkid_new_probe_from_filename(device);
> -	if (!pr)
> -		return;
> -
> -	tp = blkid_probe_get_topology(pr);
> -	if (!tp)
> -		goto out_free_probe;
> -
> -	val = blkid_topology_get_logical_sector_size(tp);
> -	*lsectorsize = val;
> -	val = blkid_topology_get_physical_sector_size(tp);
> -	*psectorsize = val;
> -	val = blkid_topology_get_minimum_io_size(tp);
> -	*sunit = val;
> -	val = blkid_topology_get_optimal_io_size(tp);
> -	*swidth = val;
> -
> -	/*
> -	 * If the reported values are the same as the physical sector size
> -	 * do not bother to report anything.  It will only cause warnings
> -	 * if people specify larger stripe units or widths manually.
> -	 */
> -	if (*sunit == *psectorsize || *swidth == *psectorsize) {
> -		*sunit = 0;
> -		*swidth = 0;
> -	}
> -
> -	/*
> -	 * Blkid reports the information in terms of bytes, but we want it in
> -	 * terms of 512 bytes blocks (only to convert it to bytes later..)
> -	 */
> -	*sunit = *sunit >> 9;
> -	*swidth = *swidth >> 9;
> -
> -	if (blkid_topology_get_alignment_offset(tp) != 0) {
> -		fprintf(stderr,
> -			_("warning: device is not properly aligned %s\n"),
> -			device);
> -
> -		if (!force_overwrite) {
> -			fprintf(stderr,
> -				_("Use -f to force usage of a misaligned device\n"));
> -
> -			exit(EXIT_FAILURE);
> -		}
> -		/* Do not use physical sector size if the device is misaligned */
> -		*psectorsize = *lsectorsize;
> -	}
> -
> -	blkid_free_probe(pr);
> -	return;
> -
> -out_free_probe:
> -	blkid_free_probe(pr);
> -	fprintf(stderr,
> -		_("warning: unable to probe device topology for device %s\n"),
> -		device);
> -}
> -#else /* ifdef ENABLE_BLKID */
> -/*
> - * Without blkid, we can't do a good check for signatures.
> - * So instead of some messy attempts, just disable any checks
> - * and always return 'nothing found'.
> - */
> -#  warning BLKID is disabled, so signature detection and block device\
> - access are not working!
> -static int
> -check_overwrite(
> -	char		*device)
> -{
> -	return 1;
> -}
> -
> -static void blkid_get_topology(
> -	const char	*device,
> -	int		*sunit,
> -	int		*swidth,
> -	int		*lsectorsize,
> -	int		*psectorsize,
> -	int		force_overwrite)
> -{
> -	/*
> -	 * Shouldn't make any difference (no blkid = no block device access),
> -	 * but make sure this dummy replacement returns with at least some
> -	 * sanity.
> -	 */
> -	*lsectorsize = *psectorsize = 512;
> -}
> -
> -#endif /* ENABLE_BLKID */
> -
> -static void get_topology(
> -	libxfs_init_t		*xi,
> -	struct fs_topology	*ft,
> -	int			force_overwrite)
> -{
> -	struct stat statbuf;
> -	char *dfile = xi->volname ? xi->volname : xi->dname;
> -
> -	/*
> -	 * If our target is a regular file, use platform_findsizes
> -	 * to try to obtain the underlying filesystem's requirements
> -	 * for direct IO; we'll set our sector size to that if possible.
> -	 */
> -	if (xi->disfile ||
> -	    (!stat(dfile, &statbuf) && S_ISREG(statbuf.st_mode))) {
> -		int fd;
> -		int flags = O_RDONLY;
> -		long long dummy;
> -
> -		/* with xi->disfile we may not have the file yet! */
> -		if (xi->disfile)
> -			flags |= O_CREAT;
> -
> -		fd = open(dfile, flags, 0666);
> -		if (fd >= 0) {
> -			platform_findsizes(dfile, fd, &dummy, &ft->lsectorsize);
> -			close(fd);
> -			ft->psectorsize = ft->lsectorsize;
> -		} else
> -			ft->psectorsize = ft->lsectorsize = BBSIZE;
> -	} else {
> -		blkid_get_topology(dfile, &ft->dsunit, &ft->dswidth,
> -				   &ft->lsectorsize, &ft->psectorsize,
> -				   force_overwrite);
> -	}
> -
> -	if (xi->rtname && !xi->risfile) {
> -		int sunit, lsectorsize, psectorsize;
> -
> -		blkid_get_topology(xi->rtname, &sunit, &ft->rtswidth,
> -				   &lsectorsize, &psectorsize, force_overwrite);
> -	}
> -}
> -
>  static void
>  fixup_log_stripe_unit(
>  	int		lsflag,
> @@ -640,77 +392,6 @@ calc_default_imaxpct(
>  	return 1;
>  }
>  
> -
> -void
> -calc_default_ag_geometry(
> -	int		blocklog,
> -	__uint64_t	dblocks,
> -	int		multidisk,
> -	__uint64_t	*agsize,
> -	__uint64_t	*agcount)
> -{
> -	__uint64_t	blocks = 0;
> -	int		shift = 0;
> -
> -	/*
> -	 * First handle the high extreme - the point at which we will
> -	 * always use the maximum AG size.
> -	 *
> -	 * This applies regardless of storage configuration.
> -	 */
> -	if (dblocks >= TERABYTES(32, blocklog)) {
> -		blocks = XFS_AG_MAX_BLOCKS(blocklog);
> -		goto done;
> -	}
> -
> -	/*
> -	 * For the remainder we choose an AG size based on the
> -	 * number of data blocks available, trying to keep the
> -	 * number of AGs relatively small (especially compared
> -	 * to the original algorithm).  AG count is calculated
> -	 * based on the preferred AG size, not vice-versa - the
> -	 * count can be increased by growfs, so prefer to use
> -	 * smaller counts at mkfs time.
> -	 *
> -	 * For a single underlying storage device between 128MB
> -	 * and 4TB in size, just use 4 AGs, otherwise scale up
> -	 * smoothly between min/max AG sizes.
> -	 */
> -
> -	if (!multidisk && dblocks >= MEGABYTES(128, blocklog)) {
> -		if (dblocks >= TERABYTES(4, blocklog)) {
> -			blocks = XFS_AG_MAX_BLOCKS(blocklog);
> -			goto done;
> -		}
> -		shift = 2;
> -	} else if (dblocks > GIGABYTES(512, blocklog))
> -		shift = 5;
> -	else if (dblocks > GIGABYTES(8, blocklog))
> -		shift = 4;
> -	else if (dblocks >= MEGABYTES(128, blocklog))
> -		shift = 3;
> -	else if (dblocks >= MEGABYTES(64, blocklog))
> -		shift = 2;
> -	else if (dblocks >= MEGABYTES(32, blocklog))
> -		shift = 1;
> -	else
> -		shift = 0;
> -	/*
> -	 * If dblocks is not evenly divisible by the number of
> -	 * desired AGs, round "blocks" up so we don't lose the
> -	 * last bit of the filesystem. The same principle applies
> -	 * to the AG count, so we don't lose the last AG!
> -	 */
> -	blocks = dblocks >> shift;
> -	if (dblocks & xfs_mask32lo(shift)) {
> -		if (blocks < XFS_AG_MAX_BLOCKS(blocklog))
> -		    blocks++;
> -	}
> -done:
> -	*agsize = blocks;
> -	*agcount = dblocks / blocks + (dblocks % blocks != 0);
> -}
> -
>  static void
>  validate_ag_geometry(
>  	int		blocklog,
> 

_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH v4 2/2] xfs_repair: new secondary superblock search method
  2016-02-12 19:03 ` [PATCH v4 2/2] xfs_repair: new secondary superblock search method Bill O'Donnell
@ 2016-02-13  3:12   ` Eric Sandeen
  2016-02-15 17:00     ` [PATCH v5 " Bill O'Donnell
  0 siblings, 1 reply; 11+ messages in thread
From: Eric Sandeen @ 2016-02-13  3:12 UTC (permalink / raw)
  To: xfs

On 2/12/16 1:03 PM, Bill O'Donnell wrote:
> Optimize secondary sb search, using similar method to find
> fs geometry as that of xfs_mkfs. If this faster method fails
> in finding a secondary sb, fall back to original brute force
> slower search.

Hey Bill, looks like this is pretty much there.  I do have a few
comments, things which I really should have seen earlier, so I'm
very sorry about that.  Inline below.

I think you can just send a V5 in reply to this one to keep it in
the thread.

Dave's up to V6 on his latest patchset so don't feel too bad.  :D

-Eric
 
> Signed-off-by: Bill O'Donnell <billodo@redhat.com>
> ---
>  Makefile           |  2 +-
>  include/libxcmd.h  |  6 +++++-
>  libxcmd/topology.c | 27 +++++++++++++++++++++++++++
>  repair/Makefile    |  4 ++--
>  repair/sb.c        | 49 +++++++++++++++++++++++++++++++++++++++++--------
>  5 files changed, 76 insertions(+), 12 deletions(-)
> 
> diff --git a/Makefile b/Makefile
> index fca0a42..1d60d9c 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -80,7 +80,7 @@ fsr: libhandle
>  growfs: libxcmd
>  io: libxcmd libhandle
>  quota: libxcmd
> -repair: libxlog
> +repair: libxlog libxcmd
>  copy: libxlog
>  
>  ifeq ($(HAVE_BUILDDEFS), yes)
> diff --git a/include/libxcmd.h b/include/libxcmd.h
> index a912534..eb16cd5 100644
> --- a/include/libxcmd.h
> +++ b/include/libxcmd.h
> @@ -51,6 +51,10 @@ extern int
>  check_overwrite(
>  	char		*device);
>  
> -
> +extern int
> +guess_default_geometry(
> +	__uint64_t	*agsize,
> +	__uint64_t	*agcount,
> +	libxfs_init_t	x);

This function is only used in repair/sb.c, so it can just
be a static in that function, no need to have it in the library.

But more importantly, passing the libxfs_init_t structure by
value is pretty unusual; it works in this case because it's
not modified, but it would be safer and make more sense to
pass in a pointer.
 
>  #endif	/* __LIBXCMD_H__ */
> diff --git a/libxcmd/topology.c b/libxcmd/topology.c
> index 4cbe4b1..ec9695c 100644
> --- a/libxcmd/topology.c
> +++ b/libxcmd/topology.c
> @@ -339,3 +339,30 @@ get_topology(
>  				   &lsectorsize, &psectorsize, force_overwrite);
>  	}
>  }
> +
> +int
> +guess_default_geometry(
> +	__uint64_t	*agsize,
> +	__uint64_t	*agcount,
> +	libxfs_init_t	x)

+	__uint64_t		*agsize,
+	__uint64_t		*agcount,
+	libxfs_init_t		*x)

tab out to match below, and take pointer for x

> +{
> +	struct fs_topology	ft;
> +	int			blocklog;
> +	__uint64_t		dblocks;
> +	int			multidisk;
> +
> +	memset(&ft, 0, sizeof(ft));
> +	get_topology(&x, &ft, 1);

+	get_topology(x, &ft,1);

> +
> +	/*
> +	 * get geometry from get_topology result.
> +	 * Use default block size (2^12)
> +	 */
> +	blocklog = 12;
> +	multidisk = ft.dswidth | ft.dsunit;
> +	dblocks = x.dsize >> (blocklog - BBSHIFT);

+	dblocks = x->dsize >> (blocklog - BBSHIFT);

> +	calc_default_ag_geometry(blocklog, dblocks, multidisk,
> +				 agsize, agcount);
> +
> +	return blocklog;
> +}
> diff --git a/repair/Makefile b/repair/Makefile
> index 251722b..d24ab1f 100644
> --- a/repair/Makefile
> +++ b/repair/Makefile
> @@ -20,8 +20,8 @@ CFILES = agheader.c attr_repair.c avl.c avl64.c bmap.c btree.c \
>  	progress.c prefetch.c rt.c sb.c scan.c threads.c \
>  	versions.c xfs_repair.c
>  
> -LLDLIBS = $(LIBXFS) $(LIBXLOG) $(LIBUUID) $(LIBRT) $(LIBPTHREAD)
> -LTDEPENDENCIES = $(LIBXFS) $(LIBXLOG)
> +LLDLIBS = $(LIBBLKID) $(LIBXFS) $(LIBXLOG) $(LIBUUID) $(LIBRT) $(LIBPTHREAD) $(LIBXCMD)
> +LTDEPENDENCIES = $(LIBXFS) $(LIBXLOG) $(LIBXCMD)
>  LLDFLAGS = -static-libtool-libs
>  
>  default: depend $(LTCOMMAND)
> diff --git a/repair/sb.c b/repair/sb.c
> index 4eef14a..8dcad5f 100644
> --- a/repair/sb.c
> +++ b/repair/sb.c
> @@ -17,6 +17,7 @@
>   */
>  
>  #include "libxfs.h"
> +#include "libxcmd.h"
>  #include "libxlog.h"
>  #include "agheader.h"
>  #include "globals.h"
> @@ -85,10 +86,15 @@ copy_sb(xfs_sb_t *source, xfs_sb_t *dest)
>  }
>  
>  /*
> - * find a secondary superblock, copy it into the sb buffer
> + * find a secondary superblock, copy it into the sb buffer.
> + * start is the point to begin reading BSIZE bytes.
> + * skip contains a byte-count of how far to advance for next read.
>   */
> -int
> -find_secondary_sb(xfs_sb_t *rsb)
> +static int
> +__find_secondary_sb(
> +	xfs_sb_t *rsb,
> +	__uint64_t start,
> +	__uint64_t skip)

again, tab those out to match:

+	xfs_sb_t 	*rsb,
+	__uint64_t 	start,
+	__uint64_t 	skip)

>  {
>  	xfs_off_t	off;
>  	xfs_sb_t	*sb;
> @@ -101,7 +107,6 @@ find_secondary_sb(xfs_sb_t *rsb)
>  	int		bsize;
>  
>  	do_warn(_("\nattempting to find secondary superblock...\n"));
> -

No good reason to remove this line, really; sometimes we clean up
whitespace if it makes sense and we're in the area, but this hunk
is nothing but a line removal; generally don't want unrelated whitespace
changes to sneak in.

>  	sb = (xfs_sb_t *)memalign(libxfs_device_alignment(), BSIZE);
>  	if (!sb) {
>  		do_error(
> @@ -117,7 +122,7 @@ find_secondary_sb(xfs_sb_t *rsb)
>  	/*
>  	 * skip first sector since we know that's bad
>  	 */
> -	for (done = 0, off = XFS_AG_MIN_BYTES; !done ; off += bsize)  {
> +	for (done = 0, off = start; !done ; off += skip)  {
>  		/*
>  		 * read disk 1 MByte at a time.
>  		 */
> @@ -128,9 +133,7 @@ find_secondary_sb(xfs_sb_t *rsb)
>  		if (!done && (bsize = read(x.dfd, sb, BSIZE)) <= 0)  {
>  			done = 1;
>  		}
> -
>  		do_warn(".");
> -

While we're at it - no reason to remove these lines, it makes the
dot-generator stand out a little more. ;)

>  		/*
>  		 * check the buffer 512 bytes at a time since
>  		 * we don't know how big the sectors really are.
> @@ -166,7 +169,37 @@ find_secondary_sb(xfs_sb_t *rsb)
>  	}
>  
>  	free(sb);
> -	return(retval);
> +	return retval;
> +}

You can put guess_default_geometry() here, as a static
function.

> +
> +int
> +find_secondary_sb(xfs_sb_t *rsb)
> +{
> +	int		retval;
> +	__uint64_t	agcount;
> +	__uint64_t	agsize;
> +	__uint64_t	skip;
> +	int		blocklog;
> +
> +	/*
> +	 * Attempt to find secondary sb with a coarse approach,
> +	 * using a large skip (agsize in bytes). Failing that,

"using a large start and skip," I guess.

> +	 * fallback to the fine-grained approach using min agsize.

"fallback to the fine-grained approach, starting at min agsize."

(it's not using min agsize for skip, so just to keep that clear)

> +	 */
> +	blocklog = guess_default_geometry(&agsize, &agcount, x);
> +
> +	/*
> +	 * use found ag geometry to quickly find secondary sb
> +	 */
> +	skip = agsize << blocklog;
> +	retval = __find_secondary_sb(rsb, skip, skip);
> +	if (!retval)  {
> +		/*
> +		 * fallback: use minimum agsize for skipsize

actually that's the start, not skip; so

	"fallback: Start at min agsize and scan all blocks"

> +		 */
> +		retval = __find_secondary_sb(rsb, XFS_AG_MIN_BYTES, BSIZE);
> +	}
> +	return retval;
>  }
>  
>  /*
> 

_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH v5 2/2] xfs_repair: new secondary superblock search method
  2016-02-13  3:12   ` Eric Sandeen
@ 2016-02-15 17:00     ` Bill O'Donnell
  2016-02-15 22:11       ` Eric Sandeen
  0 siblings, 1 reply; 11+ messages in thread
From: Bill O'Donnell @ 2016-02-15 17:00 UTC (permalink / raw)
  To: Eric Sandeen; +Cc: xfs

xfs_repair: new secondary superblock search method

Optimize secondary sb search, using similar method to find
fs geometry as that of xfs_mkfs. If this faster method fails
in finding a secondary sb, fall back to original brute force
slower search.

version history:
----
v1: http://oss.sgi.com/archives/xfs/2016-02/msg00304.html
v2: patch 2 whitespace fixups
v3: patch 2 correct functionality; style fixups
v4: patch 1,2 properly see to style and whitespace fixups
v5: patch 2 move guess_default_geometry() to sb.c plus
    very minor style/whitespace fixups.
----

Signed-off-by: Bill O'Donnell <billodo@redhat.com>
---
 Makefile        |  2 +-
 repair/Makefile |  4 ++--
 repair/sb.c     | 72 +++++++++++++++++++++++++++++++++++++++++++++++++++++----
 3 files changed, 70 insertions(+), 8 deletions(-)

diff --git a/Makefile b/Makefile
index fca0a42..1d60d9c 100644
--- a/Makefile
+++ b/Makefile
@@ -80,7 +80,7 @@ fsr: libhandle
 growfs: libxcmd
 io: libxcmd libhandle
 quota: libxcmd
-repair: libxlog
+repair: libxlog libxcmd
 copy: libxlog
 
 ifeq ($(HAVE_BUILDDEFS), yes)
diff --git a/repair/Makefile b/repair/Makefile
index 251722b..d24ab1f 100644
--- a/repair/Makefile
+++ b/repair/Makefile
@@ -20,8 +20,8 @@ CFILES = agheader.c attr_repair.c avl.c avl64.c bmap.c btree.c \
 	progress.c prefetch.c rt.c sb.c scan.c threads.c \
 	versions.c xfs_repair.c
 
-LLDLIBS = $(LIBXFS) $(LIBXLOG) $(LIBUUID) $(LIBRT) $(LIBPTHREAD)
-LTDEPENDENCIES = $(LIBXFS) $(LIBXLOG)
+LLDLIBS = $(LIBBLKID) $(LIBXFS) $(LIBXLOG) $(LIBUUID) $(LIBRT) $(LIBPTHREAD) $(LIBXCMD)
+LTDEPENDENCIES = $(LIBXFS) $(LIBXLOG) $(LIBXCMD)
 LLDFLAGS = -static-libtool-libs
 
 default: depend $(LTCOMMAND)
diff --git a/repair/sb.c b/repair/sb.c
index 4eef14a..7e4708c 100644
--- a/repair/sb.c
+++ b/repair/sb.c
@@ -17,6 +17,7 @@
  */
 
 #include "libxfs.h"
+#include "libxcmd.h"
 #include "libxlog.h"
 #include "agheader.h"
 #include "globals.h"
@@ -85,10 +86,15 @@ copy_sb(xfs_sb_t *source, xfs_sb_t *dest)
 }
 
 /*
- * find a secondary superblock, copy it into the sb buffer
+ * find a secondary superblock, copy it into the sb buffer.
+ * start is the point to begin reading BSIZE bytes.
+ * skip contains a byte-count of how far to advance for next read.
  */
-int
-find_secondary_sb(xfs_sb_t *rsb)
+static int
+__find_secondary_sb(
+	xfs_sb_t	*rsb,
+	__uint64_t	start,
+	__uint64_t	skip)
 {
 	xfs_off_t	off;
 	xfs_sb_t	*sb;
@@ -117,7 +123,7 @@ find_secondary_sb(xfs_sb_t *rsb)
 	/*
 	 * skip first sector since we know that's bad
 	 */
-	for (done = 0, off = XFS_AG_MIN_BYTES; !done ; off += bsize)  {
+	for (done = 0, off = start; !done ; off += skip)  {
 		/*
 		 * read disk 1 MByte at a time.
 		 */
@@ -166,7 +172,63 @@ find_secondary_sb(xfs_sb_t *rsb)
 	}
 
 	free(sb);
-	return(retval);
+	return retval;
+}
+
+static int
+guess_default_geometry(
+	__uint64_t		*agsize,
+	__uint64_t		*agcount,
+	libxfs_init_t		*x)
+{
+	struct fs_topology	ft;
+	int			blocklog;
+	__uint64_t		dblocks;
+	int			multidisk;
+
+	memset(&ft, 0, sizeof(ft));
+	get_topology(x, &ft, 1);
+
+	/*
+	 * get geometry from get_topology result.
+	 * Use default block size (2^12)
+	 */
+	blocklog = 12;
+	multidisk = ft.dswidth | ft.dsunit;
+	dblocks = x->dsize >> (blocklog - BBSHIFT);
+	calc_default_ag_geometry(blocklog, dblocks, multidisk,
+				 agsize, agcount);
+
+	return blocklog;
+}
+
+int
+find_secondary_sb(xfs_sb_t *rsb)
+{
+	int		retval;
+	__uint64_t	agcount;
+	__uint64_t	agsize;
+	__uint64_t	skip;
+	int		blocklog;
+
+	/*
+	 * Attempt to find secondary sb with a coarse approach.
+	 * Failing that, fallback to a fine-grained approach.
+	 */
+	blocklog = guess_default_geometry(&agsize, &agcount, &x);
+
+	/*
+	 * use found ag geometry to quickly find secondary sb
+	 */
+	skip = agsize << blocklog;
+	retval = __find_secondary_sb(rsb, skip, skip);
+	if (!retval)  {
+		/*
+		 * fallback: Start at min agsize and scan all blocks
+		 */
+		retval = __find_secondary_sb(rsb, XFS_AG_MIN_BYTES, BSIZE);
+	}
+	return retval;
 }
 
 /*
-- 
2.5.0

_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs

^ permalink raw reply related	[flat|nested] 11+ messages in thread

* Re: [PATCH v5 2/2] xfs_repair: new secondary superblock search method
  2016-02-15 17:00     ` [PATCH v5 " Bill O'Donnell
@ 2016-02-15 22:11       ` Eric Sandeen
  0 siblings, 0 replies; 11+ messages in thread
From: Eric Sandeen @ 2016-02-15 22:11 UTC (permalink / raw)
  To: xfs

On 2/15/16 11:00 AM, Bill O'Donnell wrote:
> xfs_repair: new secondary superblock search method
> 
> Optimize secondary sb search, using similar method to find
> fs geometry as that of xfs_mkfs. If this faster method fails
> in finding a secondary sb, fall back to original brute force
> slower search.
> 
> version history:
> ----
> v1: http://oss.sgi.com/archives/xfs/2016-02/msg00304.html
> v2: patch 2 whitespace fixups
> v3: patch 2 correct functionality; style fixups
> v4: patch 1,2 properly see to style and whitespace fixups
> v5: patch 2 move guess_default_geometry() to sb.c plus
>     very minor style/whitespace fixups.
> ----
> 
> Signed-off-by: Bill O'Donnell <billodo@redhat.com>

Looks good to me, thanks Bill!

Reviewed-by: Eric Sandeen <sandeen@redhat.com>

> ---
>  Makefile        |  2 +-
>  repair/Makefile |  4 ++--
>  repair/sb.c     | 72 +++++++++++++++++++++++++++++++++++++++++++++++++++++----
>  3 files changed, 70 insertions(+), 8 deletions(-)
> 
> diff --git a/Makefile b/Makefile
> index fca0a42..1d60d9c 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -80,7 +80,7 @@ fsr: libhandle
>  growfs: libxcmd
>  io: libxcmd libhandle
>  quota: libxcmd
> -repair: libxlog
> +repair: libxlog libxcmd
>  copy: libxlog
>  
>  ifeq ($(HAVE_BUILDDEFS), yes)
> diff --git a/repair/Makefile b/repair/Makefile
> index 251722b..d24ab1f 100644
> --- a/repair/Makefile
> +++ b/repair/Makefile
> @@ -20,8 +20,8 @@ CFILES = agheader.c attr_repair.c avl.c avl64.c bmap.c btree.c \
>  	progress.c prefetch.c rt.c sb.c scan.c threads.c \
>  	versions.c xfs_repair.c
>  
> -LLDLIBS = $(LIBXFS) $(LIBXLOG) $(LIBUUID) $(LIBRT) $(LIBPTHREAD)
> -LTDEPENDENCIES = $(LIBXFS) $(LIBXLOG)
> +LLDLIBS = $(LIBBLKID) $(LIBXFS) $(LIBXLOG) $(LIBUUID) $(LIBRT) $(LIBPTHREAD) $(LIBXCMD)
> +LTDEPENDENCIES = $(LIBXFS) $(LIBXLOG) $(LIBXCMD)
>  LLDFLAGS = -static-libtool-libs
>  
>  default: depend $(LTCOMMAND)
> diff --git a/repair/sb.c b/repair/sb.c
> index 4eef14a..7e4708c 100644
> --- a/repair/sb.c
> +++ b/repair/sb.c
> @@ -17,6 +17,7 @@
>   */
>  
>  #include "libxfs.h"
> +#include "libxcmd.h"
>  #include "libxlog.h"
>  #include "agheader.h"
>  #include "globals.h"
> @@ -85,10 +86,15 @@ copy_sb(xfs_sb_t *source, xfs_sb_t *dest)
>  }
>  
>  /*
> - * find a secondary superblock, copy it into the sb buffer
> + * find a secondary superblock, copy it into the sb buffer.
> + * start is the point to begin reading BSIZE bytes.
> + * skip contains a byte-count of how far to advance for next read.
>   */
> -int
> -find_secondary_sb(xfs_sb_t *rsb)
> +static int
> +__find_secondary_sb(
> +	xfs_sb_t	*rsb,
> +	__uint64_t	start,
> +	__uint64_t	skip)
>  {
>  	xfs_off_t	off;
>  	xfs_sb_t	*sb;
> @@ -117,7 +123,7 @@ find_secondary_sb(xfs_sb_t *rsb)
>  	/*
>  	 * skip first sector since we know that's bad
>  	 */
> -	for (done = 0, off = XFS_AG_MIN_BYTES; !done ; off += bsize)  {
> +	for (done = 0, off = start; !done ; off += skip)  {
>  		/*
>  		 * read disk 1 MByte at a time.
>  		 */
> @@ -166,7 +172,63 @@ find_secondary_sb(xfs_sb_t *rsb)
>  	}
>  
>  	free(sb);
> -	return(retval);
> +	return retval;
> +}
> +
> +static int
> +guess_default_geometry(
> +	__uint64_t		*agsize,
> +	__uint64_t		*agcount,
> +	libxfs_init_t		*x)
> +{
> +	struct fs_topology	ft;
> +	int			blocklog;
> +	__uint64_t		dblocks;
> +	int			multidisk;
> +
> +	memset(&ft, 0, sizeof(ft));
> +	get_topology(x, &ft, 1);
> +
> +	/*
> +	 * get geometry from get_topology result.
> +	 * Use default block size (2^12)
> +	 */
> +	blocklog = 12;
> +	multidisk = ft.dswidth | ft.dsunit;
> +	dblocks = x->dsize >> (blocklog - BBSHIFT);
> +	calc_default_ag_geometry(blocklog, dblocks, multidisk,
> +				 agsize, agcount);
> +
> +	return blocklog;
> +}
> +
> +int
> +find_secondary_sb(xfs_sb_t *rsb)
> +{
> +	int		retval;
> +	__uint64_t	agcount;
> +	__uint64_t	agsize;
> +	__uint64_t	skip;
> +	int		blocklog;
> +
> +	/*
> +	 * Attempt to find secondary sb with a coarse approach.
> +	 * Failing that, fallback to a fine-grained approach.
> +	 */
> +	blocklog = guess_default_geometry(&agsize, &agcount, &x);
> +
> +	/*
> +	 * use found ag geometry to quickly find secondary sb
> +	 */
> +	skip = agsize << blocklog;
> +	retval = __find_secondary_sb(rsb, skip, skip);
> +	if (!retval)  {
> +		/*
> +		 * fallback: Start at min agsize and scan all blocks
> +		 */
> +		retval = __find_secondary_sb(rsb, XFS_AG_MIN_BYTES, BSIZE);
> +	}
> +	return retval;
>  }
>  
>  /*
> 

_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs

^ permalink raw reply	[flat|nested] 11+ messages in thread

* [PATCH v5 2/2] xfs_repair: new secondary superblock search method
  2016-05-11 22:45 [PATCH v5 0/2] xfs_repair: improved secondary sb search Bill O'Donnell
@ 2016-05-11 22:45 ` Bill O'Donnell
  0 siblings, 0 replies; 11+ messages in thread
From: Bill O'Donnell @ 2016-05-11 22:45 UTC (permalink / raw)
  To: xfs

Optimize secondary sb search, using similar method to find
fs geometry as that of xfs_mkfs. If this faster method fails
in finding a secondary sb, fall back to original brute force
slower search.

Signed-off-by: Bill O'Donnell <billodo@redhat.com>
---
 Makefile        |  2 +-
 repair/Makefile |  4 ++--
 repair/sb.c     | 72 +++++++++++++++++++++++++++++++++++++++++++++++++++++----
 3 files changed, 70 insertions(+), 8 deletions(-)

diff --git a/Makefile b/Makefile
index fca0a42..1d60d9c 100644
--- a/Makefile
+++ b/Makefile
@@ -80,7 +80,7 @@ fsr: libhandle
 growfs: libxcmd
 io: libxcmd libhandle
 quota: libxcmd
-repair: libxlog
+repair: libxlog libxcmd
 copy: libxlog
 
 ifeq ($(HAVE_BUILDDEFS), yes)
diff --git a/repair/Makefile b/repair/Makefile
index 251722b..d24ab1f 100644
--- a/repair/Makefile
+++ b/repair/Makefile
@@ -20,8 +20,8 @@ CFILES = agheader.c attr_repair.c avl.c avl64.c bmap.c btree.c \
 	progress.c prefetch.c rt.c sb.c scan.c threads.c \
 	versions.c xfs_repair.c
 
-LLDLIBS = $(LIBXFS) $(LIBXLOG) $(LIBUUID) $(LIBRT) $(LIBPTHREAD)
-LTDEPENDENCIES = $(LIBXFS) $(LIBXLOG)
+LLDLIBS = $(LIBBLKID) $(LIBXFS) $(LIBXLOG) $(LIBUUID) $(LIBRT) $(LIBPTHREAD) $(LIBXCMD)
+LTDEPENDENCIES = $(LIBXFS) $(LIBXLOG) $(LIBXCMD)
 LLDFLAGS = -static-libtool-libs
 
 default: depend $(LTCOMMAND)
diff --git a/repair/sb.c b/repair/sb.c
index 4eef14a..7e4708c 100644
--- a/repair/sb.c
+++ b/repair/sb.c
@@ -17,6 +17,7 @@
  */
 
 #include "libxfs.h"
+#include "libxcmd.h"
 #include "libxlog.h"
 #include "agheader.h"
 #include "globals.h"
@@ -85,10 +86,15 @@ copy_sb(xfs_sb_t *source, xfs_sb_t *dest)
 }
 
 /*
- * find a secondary superblock, copy it into the sb buffer
+ * find a secondary superblock, copy it into the sb buffer.
+ * start is the point to begin reading BSIZE bytes.
+ * skip contains a byte-count of how far to advance for next read.
  */
-int
-find_secondary_sb(xfs_sb_t *rsb)
+static int
+__find_secondary_sb(
+	xfs_sb_t	*rsb,
+	__uint64_t	start,
+	__uint64_t	skip)
 {
 	xfs_off_t	off;
 	xfs_sb_t	*sb;
@@ -117,7 +123,7 @@ find_secondary_sb(xfs_sb_t *rsb)
 	/*
 	 * skip first sector since we know that's bad
 	 */
-	for (done = 0, off = XFS_AG_MIN_BYTES; !done ; off += bsize)  {
+	for (done = 0, off = start; !done ; off += skip)  {
 		/*
 		 * read disk 1 MByte at a time.
 		 */
@@ -166,7 +172,63 @@ find_secondary_sb(xfs_sb_t *rsb)
 	}
 
 	free(sb);
-	return(retval);
+	return retval;
+}
+
+static int
+guess_default_geometry(
+	__uint64_t		*agsize,
+	__uint64_t		*agcount,
+	libxfs_init_t		*x)
+{
+	struct fs_topology	ft;
+	int			blocklog;
+	__uint64_t		dblocks;
+	int			multidisk;
+
+	memset(&ft, 0, sizeof(ft));
+	get_topology(x, &ft, 1);
+
+	/*
+	 * get geometry from get_topology result.
+	 * Use default block size (2^12)
+	 */
+	blocklog = 12;
+	multidisk = ft.dswidth | ft.dsunit;
+	dblocks = x->dsize >> (blocklog - BBSHIFT);
+	calc_default_ag_geometry(blocklog, dblocks, multidisk,
+				 agsize, agcount);
+
+	return blocklog;
+}
+
+int
+find_secondary_sb(xfs_sb_t *rsb)
+{
+	int		retval;
+	__uint64_t	agcount;
+	__uint64_t	agsize;
+	__uint64_t	skip;
+	int		blocklog;
+
+	/*
+	 * Attempt to find secondary sb with a coarse approach.
+	 * Failing that, fallback to a fine-grained approach.
+	 */
+	blocklog = guess_default_geometry(&agsize, &agcount, &x);
+
+	/*
+	 * use found ag geometry to quickly find secondary sb
+	 */
+	skip = agsize << blocklog;
+	retval = __find_secondary_sb(rsb, skip, skip);
+	if (!retval)  {
+		/*
+		 * fallback: Start at min agsize and scan all blocks
+		 */
+		retval = __find_secondary_sb(rsb, XFS_AG_MIN_BYTES, BSIZE);
+	}
+	return retval;
 }
 
 /*
-- 
2.5.5

_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs

^ permalink raw reply related	[flat|nested] 11+ messages in thread

end of thread, other threads:[~2016-05-11 22:45 UTC | newest]

Thread overview: 11+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-02-12 19:03 [PATCH 0/2 v4] xfs_repair: improved secondary sb search Bill O'Donnell
2016-02-12 19:03 ` [PATCH v4 1/2] libxcmd: generalize topology functions Bill O'Donnell
2016-02-12 19:39   ` Christoph Hellwig
2016-02-12 19:47     ` Bill O'Donnell
2016-02-13  2:22       ` Eric Sandeen
2016-02-13  2:43   ` Eric Sandeen
2016-02-12 19:03 ` [PATCH v4 2/2] xfs_repair: new secondary superblock search method Bill O'Donnell
2016-02-13  3:12   ` Eric Sandeen
2016-02-15 17:00     ` [PATCH v5 " Bill O'Donnell
2016-02-15 22:11       ` Eric Sandeen
2016-05-11 22:45 [PATCH v5 0/2] xfs_repair: improved secondary sb search Bill O'Donnell
2016-05-11 22:45 ` [PATCH v5 2/2] xfs_repair: new secondary superblock search method Bill O'Donnell

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.