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; 10+ 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] 10+ 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; 10+ 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] 10+ 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; 10+ 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] 10+ 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; 10+ 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] 10+ 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; 10+ 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] 10+ 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; 10+ 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] 10+ 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; 10+ 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] 10+ 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; 10+ 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] 10+ 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; 10+ 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] 10+ 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; 10+ 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] 10+ messages in thread

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

Thread overview: 10+ 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

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.