* [PATCH 01/18] xfs_scrub: remove moveon from filemap iteration
2019-10-22 18:50 [PATCH 00/18] xfs_scrub: remove moveon space aliens Darrick J. Wong
@ 2019-10-22 18:50 ` Darrick J. Wong
2019-11-06 21:01 ` [PATCH v2 " Darrick J. Wong
2019-10-22 18:50 ` [PATCH 02/18] xfs_scrub: remove moveon from the fscounters functions Darrick J. Wong
` (17 subsequent siblings)
18 siblings, 1 reply; 25+ messages in thread
From: Darrick J. Wong @ 2019-10-22 18:50 UTC (permalink / raw)
To: sandeen, darrick.wong; +Cc: linux-xfs
From: Darrick J. Wong <darrick.wong@oracle.com>
Remove the moveon and descr clutter from filemap iteration in favor of
returning errors directly and passing error domain descriptions around
through the existing void *arg.
Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
---
scrub/filemap.c | 73 +++++++++++++++++--------------------------------
scrub/filemap.h | 16 +++++------
scrub/phase6.c | 82 +++++++++++++++++++++++++++++--------------------------
3 files changed, 75 insertions(+), 96 deletions(-)
diff --git a/scrub/filemap.c b/scrub/filemap.c
index f92e9620..bad2e9e1 100644
--- a/scrub/filemap.c
+++ b/scrub/filemap.c
@@ -23,47 +23,31 @@
#define BMAP_NR 2048
-/* Iterate all the extent block mappings between the key and fork end. */
-bool
-xfs_iterate_filemaps(
+/*
+ * Iterate all the extent block mappings between the key and fork end.
+ * Returns 0 or a positive error number.
+ */
+int
+scrub_iterate_filemaps(
struct scrub_ctx *ctx,
- const char *descr,
int fd,
int whichfork,
- struct xfs_bmap *key,
- xfs_bmap_iter_fn fn,
+ struct file_bmap *key,
+ scrub_bmap_iter_fn fn,
void *arg)
{
struct fsxattr fsx;
struct getbmapx *map;
struct getbmapx *p;
- struct xfs_bmap bmap;
- char bmap_descr[DESCR_BUFSZ];
- bool moveon = true;
+ struct file_bmap bmap;
xfs_off_t new_off;
int getxattr_type;
int i;
- int error;
-
- switch (whichfork) {
- case XFS_ATTR_FORK:
- snprintf(bmap_descr, DESCR_BUFSZ, _("%s attr"), descr);
- break;
- case XFS_COW_FORK:
- snprintf(bmap_descr, DESCR_BUFSZ, _("%s CoW"), descr);
- break;
- case XFS_DATA_FORK:
- snprintf(bmap_descr, DESCR_BUFSZ, _("%s data"), descr);
- break;
- default:
- abort();
- }
+ int ret;
map = calloc(BMAP_NR, sizeof(struct getbmapx));
- if (!map) {
- str_errno(ctx, bmap_descr);
- return false;
- }
+ if (!map)
+ return errno;
map->bmv_offset = BTOBB(key->bm_offset);
map->bmv_block = BTOBB(key->bm_physical);
@@ -89,34 +73,28 @@ xfs_iterate_filemaps(
abort();
}
- error = ioctl(fd, getxattr_type, &fsx);
- if (error < 0) {
- str_errno(ctx, bmap_descr);
- moveon = false;
+ ret = ioctl(fd, getxattr_type, &fsx);
+ if (ret < 0) {
+ ret = errno;
goto out;
}
- if (fsx.fsx_nextents == 0) {
- moveon = true;
+ if (fsx.fsx_nextents == 0)
goto out;
- }
map->bmv_count = min(fsx.fsx_nextents + 1, BMAP_NR);
- while ((error = ioctl(fd, XFS_IOC_GETBMAPX, map)) == 0) {
+ while ((ret = ioctl(fd, XFS_IOC_GETBMAPX, map)) == 0) {
for (i = 0, p = &map[i + 1]; i < map->bmv_entries; i++, p++) {
bmap.bm_offset = BBTOB(p->bmv_offset);
bmap.bm_physical = BBTOB(p->bmv_block);
bmap.bm_length = BBTOB(p->bmv_length);
bmap.bm_flags = p->bmv_oflags;
- moveon = fn(ctx, bmap_descr, fd, whichfork, &fsx,
- &bmap, arg);
- if (!moveon)
+ ret = fn(ctx, fd, whichfork, &fsx, &bmap, arg);
+ if (ret)
goto out;
- if (xfs_scrub_excessive_errors(ctx)) {
- moveon = false;
+ if (xfs_scrub_excessive_errors(ctx))
goto out;
- }
}
if (map->bmv_entries == 0)
@@ -129,17 +107,16 @@ xfs_iterate_filemaps(
map->bmv_length -= new_off - map->bmv_offset;
map->bmv_offset = new_off;
}
+ if (ret < 0)
+ ret = errno;
/*
* Pre-reflink filesystems don't know about CoW forks, so don't
* be too surprised if it fails.
*/
- if (whichfork == XFS_COW_FORK && error && errno == EINVAL)
- error = 0;
-
- if (error)
- str_errno(ctx, bmap_descr);
+ if (whichfork == XFS_COW_FORK && ret == EINVAL)
+ ret = 0;
out:
free(map);
- return moveon;
+ return ret;
}
diff --git a/scrub/filemap.h b/scrub/filemap.h
index 219be575..133e860b 100644
--- a/scrub/filemap.h
+++ b/scrub/filemap.h
@@ -7,7 +7,7 @@
#define XFS_SCRUB_FILEMAP_H_
/* inode fork block mapping */
-struct xfs_bmap {
+struct file_bmap {
uint64_t bm_offset; /* file offset of segment in bytes */
uint64_t bm_physical; /* physical starting byte */
uint64_t bm_length; /* length of segment, bytes */
@@ -15,15 +15,13 @@ struct xfs_bmap {
};
/*
- * Visit each inode fork mapping. Return true to continue iteration or false
- * to stop iterating and return to the caller.
+ * Visit each inode fork mapping. Return 0 to continue iteration or a positive
+ * error code to stop iterating and return to the caller.
*/
-typedef bool (*xfs_bmap_iter_fn)(struct scrub_ctx *ctx, const char *descr,
- int fd, int whichfork, struct fsxattr *fsx,
- struct xfs_bmap *bmap, void *arg);
+typedef int (*scrub_bmap_iter_fn)(struct scrub_ctx *ctx, int fd, int whichfork,
+ struct fsxattr *fsx, struct file_bmap *bmap, void *arg);
-bool xfs_iterate_filemaps(struct scrub_ctx *ctx, const char *descr, int fd,
- int whichfork, struct xfs_bmap *key, xfs_bmap_iter_fn fn,
- void *arg);
+int scrub_iterate_filemaps(struct scrub_ctx *ctx, int fd, int whichfork,
+ struct file_bmap *key, scrub_bmap_iter_fn fn, void *arg);
#endif /* XFS_SCRUB_FILEMAP_H_ */
diff --git a/scrub/phase6.c b/scrub/phase6.c
index 9ddddef1..0f22af59 100644
--- a/scrub/phase6.c
+++ b/scrub/phase6.c
@@ -133,9 +133,10 @@ xfs_decode_special_owner(
/* Routines to translate bad physical extents into file paths and offsets. */
struct badfile_report {
- struct scrub_ctx *ctx;
- const char *descr;
- struct xfs_bmap *bmap;
+ struct scrub_ctx *ctx;
+ const char *descr;
+ struct media_verify_state *vs;
+ struct file_bmap *bmap;
};
/* Report on bad extents found during a media scan. */
@@ -168,77 +169,68 @@ _("media error at data offset %llu length %llu."),
}
/* Report if this extent overlaps a bad region. */
-static bool
+static int
report_data_loss(
struct scrub_ctx *ctx,
- const char *descr,
int fd,
int whichfork,
struct fsxattr *fsx,
- struct xfs_bmap *bmap,
+ struct file_bmap *bmap,
void *arg)
{
- struct badfile_report br = {
- .ctx = ctx,
- .descr = descr,
- .bmap = bmap,
- };
- struct media_verify_state *vs = arg;
+ struct badfile_report *br = arg;
+ struct media_verify_state *vs = br->vs;
struct bitmap *bmp;
- int ret;
+
+ br->bmap = bmap;
/* Only report errors for real extents. */
if (scrub_data < 3 && (bmap->bm_flags & BMV_OF_PREALLOC))
- return true;
+ return 0;
if (bmap->bm_flags & BMV_OF_DELALLOC)
- return true;
+ return 0;
if (fsx->fsx_xflags & FS_XFLAG_REALTIME)
bmp = vs->r_bad;
else
bmp = vs->d_bad;
- ret = bitmap_iterate_range(bmp, bmap->bm_physical, bmap->bm_length,
- report_badfile, &br);
- if (ret) {
- str_liberror(ctx, ret, descr);
- return false;
- }
- return true;
+ return bitmap_iterate_range(bmp, bmap->bm_physical, bmap->bm_length,
+ report_badfile, br);
}
/* Report if the extended attribute data overlaps a bad region. */
-static bool
+static int
report_attr_loss(
struct scrub_ctx *ctx,
- const char *descr,
int fd,
int whichfork,
struct fsxattr *fsx,
- struct xfs_bmap *bmap,
+ struct file_bmap *bmap,
void *arg)
{
- struct media_verify_state *vs = arg;
+ struct badfile_report *br = arg;
+ struct media_verify_state *vs = br->vs;
struct bitmap *bmp = vs->d_bad;
/* Complain about attr fork extents that don't look right. */
if (bmap->bm_flags & (BMV_OF_PREALLOC | BMV_OF_DELALLOC)) {
- str_info(ctx, descr,
+ str_info(ctx, br->descr,
_("found unexpected unwritten/delalloc attr fork extent."));
- return true;
+ return 0;
}
if (fsx->fsx_xflags & FS_XFLAG_REALTIME) {
- str_info(ctx, descr,
+ str_info(ctx, br->descr,
_("found unexpected realtime attr fork extent."));
- return true;
+ return 0;
}
if (bitmap_test(bmp, bmap->bm_physical, bmap->bm_length))
- str_corrupt(ctx, descr,
+ str_corrupt(ctx, br->descr,
_("media error in extended attribute data."));
- return true;
+ return 0;
}
/* Iterate the extent mappings of a file to report errors. */
@@ -249,18 +241,30 @@ xfs_report_verify_fd(
int fd,
void *arg)
{
- struct xfs_bmap key = {0};
- bool moveon;
+ struct badfile_report br = {
+ .ctx = ctx,
+ .vs = arg,
+ .descr = descr,
+ };
+ struct file_bmap key = {0};
+ int ret;
/* data fork */
- moveon = xfs_iterate_filemaps(ctx, descr, fd, XFS_DATA_FORK, &key,
- report_data_loss, arg);
- if (!moveon)
+ ret = scrub_iterate_filemaps(ctx, fd, XFS_DATA_FORK, &key,
+ report_data_loss, &br);
+ if (ret) {
+ str_liberror(ctx, ret, descr);
return false;
+ }
/* attr fork */
- return xfs_iterate_filemaps(ctx, descr, fd, XFS_ATTR_FORK, &key,
- report_attr_loss, arg);
+ ret = scrub_iterate_filemaps(ctx, fd, XFS_ATTR_FORK, &key,
+ report_attr_loss, &br);
+ if (ret) {
+ str_liberror(ctx, ret, descr);
+ return false;
+ }
+ return true;
}
/* Report read verify errors in unlinked (but still open) files. */
^ permalink raw reply related [flat|nested] 25+ messages in thread
* [PATCH v2 01/18] xfs_scrub: remove moveon from filemap iteration
2019-10-22 18:50 ` [PATCH 01/18] xfs_scrub: remove moveon from filemap iteration Darrick J. Wong
@ 2019-11-06 21:01 ` Darrick J. Wong
2019-11-06 21:37 ` Eric Sandeen
0 siblings, 1 reply; 25+ messages in thread
From: Darrick J. Wong @ 2019-11-06 21:01 UTC (permalink / raw)
To: sandeen; +Cc: linux-xfs
From: Darrick J. Wong <darrick.wong@oracle.com>
Remove the moveon and descr clutter from filemap iteration in favor of
returning errors directly and passing error domain descriptions around
through the existing void *arg.
Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
---
v2: rebase without full-disk media scan
---
scrub/filemap.c | 73 +++++++++++++++++---------------------------------
scrub/filemap.h | 16 +++++------
scrub/phase6.c | 80 +++++++++++++++++++++++++++++--------------------------
3 files changed, 74 insertions(+), 95 deletions(-)
diff --git a/scrub/filemap.c b/scrub/filemap.c
index f92e9620..bad2e9e1 100644
--- a/scrub/filemap.c
+++ b/scrub/filemap.c
@@ -23,47 +23,31 @@
#define BMAP_NR 2048
-/* Iterate all the extent block mappings between the key and fork end. */
-bool
-xfs_iterate_filemaps(
+/*
+ * Iterate all the extent block mappings between the key and fork end.
+ * Returns 0 or a positive error number.
+ */
+int
+scrub_iterate_filemaps(
struct scrub_ctx *ctx,
- const char *descr,
int fd,
int whichfork,
- struct xfs_bmap *key,
- xfs_bmap_iter_fn fn,
+ struct file_bmap *key,
+ scrub_bmap_iter_fn fn,
void *arg)
{
struct fsxattr fsx;
struct getbmapx *map;
struct getbmapx *p;
- struct xfs_bmap bmap;
- char bmap_descr[DESCR_BUFSZ];
- bool moveon = true;
+ struct file_bmap bmap;
xfs_off_t new_off;
int getxattr_type;
int i;
- int error;
-
- switch (whichfork) {
- case XFS_ATTR_FORK:
- snprintf(bmap_descr, DESCR_BUFSZ, _("%s attr"), descr);
- break;
- case XFS_COW_FORK:
- snprintf(bmap_descr, DESCR_BUFSZ, _("%s CoW"), descr);
- break;
- case XFS_DATA_FORK:
- snprintf(bmap_descr, DESCR_BUFSZ, _("%s data"), descr);
- break;
- default:
- abort();
- }
+ int ret;
map = calloc(BMAP_NR, sizeof(struct getbmapx));
- if (!map) {
- str_errno(ctx, bmap_descr);
- return false;
- }
+ if (!map)
+ return errno;
map->bmv_offset = BTOBB(key->bm_offset);
map->bmv_block = BTOBB(key->bm_physical);
@@ -89,34 +73,28 @@ xfs_iterate_filemaps(
abort();
}
- error = ioctl(fd, getxattr_type, &fsx);
- if (error < 0) {
- str_errno(ctx, bmap_descr);
- moveon = false;
+ ret = ioctl(fd, getxattr_type, &fsx);
+ if (ret < 0) {
+ ret = errno;
goto out;
}
- if (fsx.fsx_nextents == 0) {
- moveon = true;
+ if (fsx.fsx_nextents == 0)
goto out;
- }
map->bmv_count = min(fsx.fsx_nextents + 1, BMAP_NR);
- while ((error = ioctl(fd, XFS_IOC_GETBMAPX, map)) == 0) {
+ while ((ret = ioctl(fd, XFS_IOC_GETBMAPX, map)) == 0) {
for (i = 0, p = &map[i + 1]; i < map->bmv_entries; i++, p++) {
bmap.bm_offset = BBTOB(p->bmv_offset);
bmap.bm_physical = BBTOB(p->bmv_block);
bmap.bm_length = BBTOB(p->bmv_length);
bmap.bm_flags = p->bmv_oflags;
- moveon = fn(ctx, bmap_descr, fd, whichfork, &fsx,
- &bmap, arg);
- if (!moveon)
+ ret = fn(ctx, fd, whichfork, &fsx, &bmap, arg);
+ if (ret)
goto out;
- if (xfs_scrub_excessive_errors(ctx)) {
- moveon = false;
+ if (xfs_scrub_excessive_errors(ctx))
goto out;
- }
}
if (map->bmv_entries == 0)
@@ -129,17 +107,16 @@ xfs_iterate_filemaps(
map->bmv_length -= new_off - map->bmv_offset;
map->bmv_offset = new_off;
}
+ if (ret < 0)
+ ret = errno;
/*
* Pre-reflink filesystems don't know about CoW forks, so don't
* be too surprised if it fails.
*/
- if (whichfork == XFS_COW_FORK && error && errno == EINVAL)
- error = 0;
-
- if (error)
- str_errno(ctx, bmap_descr);
+ if (whichfork == XFS_COW_FORK && ret == EINVAL)
+ ret = 0;
out:
free(map);
- return moveon;
+ return ret;
}
diff --git a/scrub/filemap.h b/scrub/filemap.h
index 219be575..133e860b 100644
--- a/scrub/filemap.h
+++ b/scrub/filemap.h
@@ -7,7 +7,7 @@
#define XFS_SCRUB_FILEMAP_H_
/* inode fork block mapping */
-struct xfs_bmap {
+struct file_bmap {
uint64_t bm_offset; /* file offset of segment in bytes */
uint64_t bm_physical; /* physical starting byte */
uint64_t bm_length; /* length of segment, bytes */
@@ -15,15 +15,13 @@ struct xfs_bmap {
};
/*
- * Visit each inode fork mapping. Return true to continue iteration or false
- * to stop iterating and return to the caller.
+ * Visit each inode fork mapping. Return 0 to continue iteration or a positive
+ * error code to stop iterating and return to the caller.
*/
-typedef bool (*xfs_bmap_iter_fn)(struct scrub_ctx *ctx, const char *descr,
- int fd, int whichfork, struct fsxattr *fsx,
- struct xfs_bmap *bmap, void *arg);
+typedef int (*scrub_bmap_iter_fn)(struct scrub_ctx *ctx, int fd, int whichfork,
+ struct fsxattr *fsx, struct file_bmap *bmap, void *arg);
-bool xfs_iterate_filemaps(struct scrub_ctx *ctx, const char *descr, int fd,
- int whichfork, struct xfs_bmap *key, xfs_bmap_iter_fn fn,
- void *arg);
+int scrub_iterate_filemaps(struct scrub_ctx *ctx, int fd, int whichfork,
+ struct file_bmap *key, scrub_bmap_iter_fn fn, void *arg);
#endif /* XFS_SCRUB_FILEMAP_H_ */
diff --git a/scrub/phase6.c b/scrub/phase6.c
index aae6b7d8..f57ce339 100644
--- a/scrub/phase6.c
+++ b/scrub/phase6.c
@@ -133,9 +133,10 @@ xfs_decode_special_owner(
/* Routines to translate bad physical extents into file paths and offsets. */
struct badfile_report {
- struct scrub_ctx *ctx;
- const char *descr;
- struct xfs_bmap *bmap;
+ struct scrub_ctx *ctx;
+ const char *descr;
+ struct media_verify_state *vs;
+ struct file_bmap *bmap;
};
/* Report on bad extents found during a media scan. */
@@ -168,75 +169,66 @@ _("media error at data offset %llu length %llu."),
}
/* Report if this extent overlaps a bad region. */
-static bool
+static int
report_data_loss(
struct scrub_ctx *ctx,
- const char *descr,
int fd,
int whichfork,
struct fsxattr *fsx,
- struct xfs_bmap *bmap,
+ struct file_bmap *bmap,
void *arg)
{
- struct badfile_report br = {
- .ctx = ctx,
- .descr = descr,
- .bmap = bmap,
- };
- struct media_verify_state *vs = arg;
+ struct badfile_report *br = arg;
+ struct media_verify_state *vs = br->vs;
struct bitmap *bmp;
- int ret;
+
+ br->bmap = bmap;
/* Only report errors for real extents. */
if (bmap->bm_flags & (BMV_OF_PREALLOC | BMV_OF_DELALLOC))
- return true;
+ return 0;
if (fsx->fsx_xflags & FS_XFLAG_REALTIME)
bmp = vs->r_bad;
else
bmp = vs->d_bad;
- ret = bitmap_iterate_range(bmp, bmap->bm_physical, bmap->bm_length,
- report_badfile, &br);
- if (ret) {
- str_liberror(ctx, ret, descr);
- return false;
- }
- return true;
+ return bitmap_iterate_range(bmp, bmap->bm_physical, bmap->bm_length,
+ report_badfile, br);
}
/* Report if the extended attribute data overlaps a bad region. */
-static bool
+static int
report_attr_loss(
struct scrub_ctx *ctx,
- const char *descr,
int fd,
int whichfork,
struct fsxattr *fsx,
- struct xfs_bmap *bmap,
+ struct file_bmap *bmap,
void *arg)
{
- struct media_verify_state *vs = arg;
+ struct badfile_report *br = arg;
+ struct media_verify_state *vs = br->vs;
struct bitmap *bmp = vs->d_bad;
/* Complain about attr fork extents that don't look right. */
if (bmap->bm_flags & (BMV_OF_PREALLOC | BMV_OF_DELALLOC)) {
- str_info(ctx, descr,
+ str_info(ctx, br->descr,
_("found unexpected unwritten/delalloc attr fork extent."));
- return true;
+ return 0;
}
if (fsx->fsx_xflags & FS_XFLAG_REALTIME) {
- str_info(ctx, descr,
+ str_info(ctx, br->descr,
_("found unexpected realtime attr fork extent."));
- return true;
+ return 0;
}
if (bitmap_test(bmp, bmap->bm_physical, bmap->bm_length))
- str_corrupt(ctx, descr,
+ str_corrupt(ctx, br->descr,
_("media error in extended attribute data."));
- return true;
+ return 0;
}
/* Iterate the extent mappings of a file to report errors. */
@@ -247,18 +239,30 @@ xfs_report_verify_fd(
int fd,
void *arg)
{
- struct xfs_bmap key = {0};
- bool moveon;
+ struct badfile_report br = {
+ .ctx = ctx,
+ .vs = arg,
+ .descr = descr,
+ };
+ struct file_bmap key = {0};
+ int ret;
/* data fork */
- moveon = xfs_iterate_filemaps(ctx, descr, fd, XFS_DATA_FORK, &key,
- report_data_loss, arg);
- if (!moveon)
+ ret = scrub_iterate_filemaps(ctx, fd, XFS_DATA_FORK, &key,
+ report_data_loss, &br);
+ if (ret) {
+ str_liberror(ctx, ret, descr);
return false;
+ }
/* attr fork */
- return xfs_iterate_filemaps(ctx, descr, fd, XFS_ATTR_FORK, &key,
- report_attr_loss, arg);
+ ret = scrub_iterate_filemaps(ctx, fd, XFS_ATTR_FORK, &key,
+ report_attr_loss, &br);
+ if (ret) {
+ str_liberror(ctx, ret, descr);
+ return false;
+ }
+ return true;
}
/* Report read verify errors in unlinked (but still open) files. */
^ permalink raw reply related [flat|nested] 25+ messages in thread
* Re: [PATCH v2 01/18] xfs_scrub: remove moveon from filemap iteration
2019-11-06 21:01 ` [PATCH v2 " Darrick J. Wong
@ 2019-11-06 21:37 ` Eric Sandeen
0 siblings, 0 replies; 25+ messages in thread
From: Eric Sandeen @ 2019-11-06 21:37 UTC (permalink / raw)
To: Darrick J. Wong; +Cc: linux-xfs
On 11/6/19 3:01 PM, Darrick J. Wong wrote:
> From: Darrick J. Wong <darrick.wong@oracle.com>
>
> Remove the moveon and descr clutter from filemap iteration in favor of
> returning errors directly and passing error domain descriptions around
> through the existing void *arg.
>
> Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
> ---
> v2: rebase without full-disk media scan
thanks for that
Looks ok,
Reviewed-by: Eric Sandeen <sandeen@redhat.com>
^ permalink raw reply [flat|nested] 25+ messages in thread
* [PATCH 02/18] xfs_scrub: remove moveon from the fscounters functions
2019-10-22 18:50 [PATCH 00/18] xfs_scrub: remove moveon space aliens Darrick J. Wong
2019-10-22 18:50 ` [PATCH 01/18] xfs_scrub: remove moveon from filemap iteration Darrick J. Wong
@ 2019-10-22 18:50 ` Darrick J. Wong
2019-10-22 18:50 ` [PATCH 03/18] xfs_scrub: remove moveon from inode iteration Darrick J. Wong
` (16 subsequent siblings)
18 siblings, 0 replies; 25+ messages in thread
From: Darrick J. Wong @ 2019-10-22 18:50 UTC (permalink / raw)
To: sandeen, darrick.wong; +Cc: linux-xfs
From: Darrick J. Wong <darrick.wong@oracle.com>
Replace the moveon returns in the fscounters functions with direct error
returns. Drop the xfs_ prefixes while we're at it.
Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
---
scrub/fscounters.c | 129 ++++++++++++++++++++--------------------------------
scrub/fscounters.h | 4 +-
scrub/phase6.c | 12 +++--
scrub/phase7.c | 15 ++++--
4 files changed, 68 insertions(+), 92 deletions(-)
diff --git a/scrub/fscounters.c b/scrub/fscounters.c
index e064c865..2581947f 100644
--- a/scrub/fscounters.c
+++ b/scrub/fscounters.c
@@ -25,8 +25,8 @@
/* Count the number of inodes in the filesystem. */
/* INUMBERS wrapper routines. */
-struct xfs_count_inodes {
- bool moveon;
+struct count_inodes {
+ int error;
uint64_t counters[0];
};
@@ -34,13 +34,14 @@ struct xfs_count_inodes {
* Count the number of inodes. Use INUMBERS to figure out how many inodes
* exist in the filesystem, assuming we've already scrubbed that.
*/
-static bool
-xfs_count_inodes_ag(
- struct scrub_ctx *ctx,
- const char *descr,
- uint32_t agno,
- uint64_t *count)
+static void
+count_ag_inodes(
+ struct workqueue *wq,
+ xfs_agnumber_t agno,
+ void *arg)
{
+ struct count_inodes *ci = arg;
+ struct scrub_ctx *ctx = (struct scrub_ctx *)wq->wq_ctx;
struct xfs_inumbers_req *ireq;
uint64_t nr = 0;
unsigned int i;
@@ -48,107 +49,78 @@ xfs_count_inodes_ag(
ireq = xfrog_inumbers_alloc_req(64, 0);
if (!ireq) {
- str_liberror(ctx, ENOMEM, _("allocating inumbers request"));
- return false;
+ ci->error = errno;
+ return;
}
xfrog_inumbers_set_ag(ireq, agno);
- while (!(error = xfrog_inumbers(&ctx->mnt, ireq))) {
+ while (!ci->error && (error = xfrog_inumbers(&ctx->mnt, ireq)) == 0) {
if (ireq->hdr.ocount == 0)
break;
for (i = 0; i < ireq->hdr.ocount; i++)
nr += ireq->inumbers[i].xi_alloccount;
}
+ if (error)
+ ci->error = error;
free(ireq);
- if (error) {
- str_liberror(ctx, error, descr);
- return false;
- }
-
- *count = nr;
- return true;
-}
-
-/* Scan all the inodes in an AG. */
-static void
-xfs_count_ag_inodes(
- struct workqueue *wq,
- xfs_agnumber_t agno,
- void *arg)
-{
- struct xfs_count_inodes *ci = arg;
- struct scrub_ctx *ctx = (struct scrub_ctx *)wq->wq_ctx;
- char descr[DESCR_BUFSZ];
- bool moveon;
-
- snprintf(descr, DESCR_BUFSZ, _("dev %d:%d AG %u inodes"),
- major(ctx->fsinfo.fs_datadev),
- minor(ctx->fsinfo.fs_datadev),
- agno);
-
- moveon = xfs_count_inodes_ag(ctx, descr, agno, &ci->counters[agno]);
- if (!moveon)
- ci->moveon = false;
+ ci->counters[agno] = nr;
}
-/* Count all the inodes in a filesystem. */
-bool
-xfs_count_all_inodes(
+/*
+ * Count all the inodes in a filesystem. Returns 0 or a positive error number.
+ */
+int
+scrub_count_all_inodes(
struct scrub_ctx *ctx,
uint64_t *count)
{
- struct xfs_count_inodes *ci;
+ struct count_inodes *ci;
xfs_agnumber_t agno;
struct workqueue wq;
- bool moveon = true;
- int ret;
+ int ret, ret2;
- ci = calloc(1, sizeof(struct xfs_count_inodes) +
+ ci = calloc(1, sizeof(struct count_inodes) +
(ctx->mnt.fsgeom.agcount * sizeof(uint64_t)));
if (!ci)
- return false;
- ci->moveon = true;
+ return errno;
ret = workqueue_create(&wq, (struct xfs_mount *)ctx,
scrub_nproc_workqueue(ctx));
- if (ret) {
- moveon = false;
- str_liberror(ctx, ret, _("creating icount workqueue"));
+ if (ret)
goto out_free;
- }
- for (agno = 0; agno < ctx->mnt.fsgeom.agcount; agno++) {
- ret = workqueue_add(&wq, xfs_count_ag_inodes, agno, ci);
- if (ret) {
- moveon = false;
- str_liberror(ctx, ret, _("queueing icount work"));
+
+ for (agno = 0; agno < ctx->mnt.fsgeom.agcount && !ci->error; agno++) {
+ ret = workqueue_add(&wq, count_ag_inodes, agno, ci);
+ if (ret)
break;
- }
}
- ret = workqueue_terminate(&wq);
- if (ret) {
- moveon = false;
- str_liberror(ctx, ret, _("finishing icount work"));
- }
+ ret2 = workqueue_terminate(&wq);
+ if (!ret && ret2)
+ ret = ret2;
workqueue_destroy(&wq);
- if (!moveon)
+ if (ci->error) {
+ ret = ci->error;
goto out_free;
+ }
for (agno = 0; agno < ctx->mnt.fsgeom.agcount; agno++)
*count += ci->counters[agno];
- moveon = ci->moveon;
out_free:
free(ci);
- return moveon;
+ return ret;
}
-/* Estimate the number of blocks and inodes in the filesystem. */
-bool
-xfs_scan_estimate_blocks(
+/*
+ * Estimate the number of blocks and inodes in the filesystem. Returns 0
+ * or a positive error number.
+ */
+int
+scrub_scan_estimate_blocks(
struct scrub_ctx *ctx,
unsigned long long *d_blocks,
unsigned long long *d_bfree,
@@ -164,17 +136,13 @@ xfs_scan_estimate_blocks(
/* Grab the fstatvfs counters, since it has to report accurately. */
error = fstatvfs(ctx->mnt.fd, &sfs);
- if (error) {
- str_errno(ctx, ctx->mntpoint);
- return false;
- }
+ if (error)
+ return errno;
/* Fetch the filesystem counters. */
error = ioctl(ctx->mnt.fd, XFS_IOC_FSCOUNTS, &fc);
- if (error) {
- str_errno(ctx, ctx->mntpoint);
- return false;
- }
+ if (error)
+ return errno;
/*
* XFS reserves some blocks to prevent hard ENOSPC, so add those
@@ -182,7 +150,8 @@ xfs_scan_estimate_blocks(
*/
error = ioctl(ctx->mnt.fd, XFS_IOC_GET_RESBLKS, &rb);
if (error)
- str_errno(ctx, ctx->mntpoint);
+ return errno;
+
sfs.f_bfree += rb.resblks_avail;
*d_blocks = sfs.f_blocks;
@@ -194,5 +163,5 @@ xfs_scan_estimate_blocks(
*f_files = sfs.f_files;
*f_free = sfs.f_ffree;
- return true;
+ return 0;
}
diff --git a/scrub/fscounters.h b/scrub/fscounters.h
index e3a79740..1fae58a6 100644
--- a/scrub/fscounters.h
+++ b/scrub/fscounters.h
@@ -6,10 +6,10 @@
#ifndef XFS_SCRUB_FSCOUNTERS_H_
#define XFS_SCRUB_FSCOUNTERS_H_
-bool xfs_scan_estimate_blocks(struct scrub_ctx *ctx,
+int scrub_scan_estimate_blocks(struct scrub_ctx *ctx,
unsigned long long *d_blocks, unsigned long long *d_bfree,
unsigned long long *r_blocks, unsigned long long *r_bfree,
unsigned long long *f_files, unsigned long long *f_free);
-bool xfs_count_all_inodes(struct scrub_ctx *ctx, uint64_t *count);
+int scrub_count_all_inodes(struct scrub_ctx *ctx, uint64_t *count);
#endif /* XFS_SCRUB_FSCOUNTERS_H_ */
diff --git a/scrub/phase6.c b/scrub/phase6.c
index 0f22af59..6a8dabe1 100644
--- a/scrub/phase6.c
+++ b/scrub/phase6.c
@@ -776,17 +776,19 @@ xfs_estimate_verify_work(
unsigned long long r_bfree;
unsigned long long f_files;
unsigned long long f_free;
- bool moveon;
+ int ret;
- moveon = xfs_scan_estimate_blocks(ctx, &d_blocks, &d_bfree,
+ ret = scrub_scan_estimate_blocks(ctx, &d_blocks, &d_bfree,
&r_blocks, &r_bfree, &f_files, &f_free);
- if (!moveon)
- return moveon;
+ if (ret) {
+ str_liberror(ctx, ret, _("estimating verify work"));
+ return false;
+ }
*items = cvt_off_fsb_to_b(&ctx->mnt, d_blocks + r_blocks);
if (scrub_data == 1)
*items -= cvt_off_fsb_to_b(&ctx->mnt, d_bfree + r_bfree);
*nr_threads = disk_heads(ctx->datadev);
*rshift = 20;
- return moveon;
+ return true;
}
diff --git a/scrub/phase7.c b/scrub/phase7.c
index 2622bc45..64e52359 100644
--- a/scrub/phase7.c
+++ b/scrub/phase7.c
@@ -156,14 +156,19 @@ xfs_scan_summary(
ptvar_free(ptvar);
/* Scan the whole fs. */
- moveon = xfs_count_all_inodes(ctx, &counted_inodes);
- if (!moveon)
+ error = scrub_count_all_inodes(ctx, &counted_inodes);
+ if (error) {
+ str_liberror(ctx, error, _("counting inodes"));
+ moveon = false;
goto out;
+ }
- moveon = xfs_scan_estimate_blocks(ctx, &d_blocks, &d_bfree, &r_blocks,
+ error = scrub_scan_estimate_blocks(ctx, &d_blocks, &d_bfree, &r_blocks,
&r_bfree, &f_files, &f_free);
- if (!moveon)
- return moveon;
+ if (error) {
+ str_liberror(ctx, error, _("estimating verify work"));
+ return false;
+ }
/*
* If we counted blocks with fsmap, then dblocks includes
^ permalink raw reply related [flat|nested] 25+ messages in thread
* [PATCH 03/18] xfs_scrub: remove moveon from inode iteration
2019-10-22 18:50 [PATCH 00/18] xfs_scrub: remove moveon space aliens Darrick J. Wong
2019-10-22 18:50 ` [PATCH 01/18] xfs_scrub: remove moveon from filemap iteration Darrick J. Wong
2019-10-22 18:50 ` [PATCH 02/18] xfs_scrub: remove moveon from the fscounters functions Darrick J. Wong
@ 2019-10-22 18:50 ` Darrick J. Wong
2019-10-22 18:50 ` [PATCH 04/18] xfs_scrub: remove moveon from vfs directory tree iteration Darrick J. Wong
` (15 subsequent siblings)
18 siblings, 0 replies; 25+ messages in thread
From: Darrick J. Wong @ 2019-10-22 18:50 UTC (permalink / raw)
To: sandeen, darrick.wong; +Cc: linux-xfs
From: Darrick J. Wong <darrick.wong@oracle.com>
Replace the moveon retuns in the inode iteration functions with a direct
integer error return. While we're at it, drop the xfs_ prefix.
Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
---
scrub/inodes.c | 124 ++++++++++++++++++++++++--------------------------------
scrub/inodes.h | 6 +--
scrub/phase3.c | 7 +--
scrub/phase5.c | 10 ++---
scrub/phase6.c | 5 +-
5 files changed, 67 insertions(+), 85 deletions(-)
diff --git a/scrub/inodes.c b/scrub/inodes.c
index 71e53bb6..a2aa6384 100644
--- a/scrub/inodes.c
+++ b/scrub/inodes.c
@@ -94,54 +94,66 @@ bulkstat_for_inumbers(
}
}
+/* BULKSTAT wrapper routines. */
+struct scan_inodes {
+ scrub_inode_iter_fn fn;
+ void *arg;
+ bool aborted;
+};
+
/*
* Call into the filesystem for inode/bulkstat information and call our
* iterator function. We'll try to fill the bulkstat information in batches,
* but we also can detect iget failures.
*/
-static bool
-xfs_iterate_inodes_ag(
- struct scrub_ctx *ctx,
- const char *descr,
- void *fshandle,
- uint32_t agno,
- xfs_inode_iter_fn fn,
+static void
+scan_ag_inodes(
+ struct workqueue *wq,
+ xfs_agnumber_t agno,
void *arg)
{
struct xfs_handle handle;
+ char descr[DESCR_BUFSZ];
struct xfs_inumbers_req *ireq;
struct xfs_bulkstat_req *breq;
- char idescr[DESCR_BUFSZ];
+ struct scan_inodes *si = arg;
+ struct scrub_ctx *ctx = (struct scrub_ctx *)wq->wq_ctx;
struct xfs_bulkstat *bs;
struct xfs_inumbers *inumbers;
- bool moveon = true;
int i;
int error;
int stale_count = 0;
- memcpy(&handle.ha_fsid, fshandle, sizeof(handle.ha_fsid));
+ snprintf(descr, DESCR_BUFSZ, _("dev %d:%d AG %u inodes"),
+ major(ctx->fsinfo.fs_datadev),
+ minor(ctx->fsinfo.fs_datadev),
+ agno);
+
+ memcpy(&handle.ha_fsid, ctx->fshandle, sizeof(handle.ha_fsid));
handle.ha_fid.fid_len = sizeof(xfs_fid_t) -
sizeof(handle.ha_fid.fid_len);
handle.ha_fid.fid_pad = 0;
breq = xfrog_bulkstat_alloc_req(XFS_INODES_PER_CHUNK, 0);
if (!breq) {
- str_liberror(ctx, ENOMEM, _("allocating bulkstat request"));
- return false;
+ str_errno(ctx, descr);
+ si->aborted = true;
+ return;
}
ireq = xfrog_inumbers_alloc_req(1, 0);
if (!ireq) {
- str_liberror(ctx, ENOMEM, _("allocating inumbers request"));
+ str_errno(ctx, descr);
free(breq);
- return false;
+ si->aborted = true;
+ return;
}
inumbers = &ireq->inumbers[0];
xfrog_inumbers_set_ag(ireq, agno);
/* Find the inode chunk & alloc mask */
error = xfrog_inumbers(&ctx->mnt, ireq);
- while (!error && ireq->hdr.ocount > 0) {
+ while (!error && !si->aborted && ireq->hdr.ocount > 0) {
/*
* We can have totally empty inode chunks on filesystems where
* there are more than 64 inodes per block. Skip these.
@@ -153,15 +165,17 @@ xfs_iterate_inodes_ag(
/* Iterate all the inodes. */
for (i = 0, bs = breq->bulkstat;
- i < inumbers->xi_alloccount;
+ !si->aborted && i < inumbers->xi_alloccount;
i++, bs++) {
handle.ha_fid.fid_ino = bs->bs_ino;
handle.ha_fid.fid_gen = bs->bs_gen;
- error = fn(ctx, &handle, bs, arg);
+ error = si->fn(ctx, &handle, bs, si->arg);
switch (error) {
case 0:
break;
- case ESTALE:
+ case ESTALE: {
+ char idescr[DESCR_BUFSZ];
+
stale_count++;
if (stale_count < 30) {
ireq->hdr.ino = inumbers->xi_startino;
@@ -172,16 +186,15 @@ xfs_iterate_inodes_ag(
str_info(ctx, idescr,
_("Changed too many times during scan; giving up."));
break;
+ }
case XFS_ITERATE_INODES_ABORT:
error = 0;
/* fall thru */
default:
- moveon = false;
- errno = error;
goto err;
}
if (xfs_scrub_excessive_errors(ctx)) {
- moveon = false;
+ si->aborted = true;
goto out;
}
}
@@ -194,71 +207,42 @@ _("Changed too many times during scan; giving up."));
err:
if (error) {
str_liberror(ctx, error, descr);
- moveon = false;
+ si->aborted = true;
}
out:
free(ireq);
free(breq);
- return moveon;
}
-/* BULKSTAT wrapper routines. */
-struct xfs_scan_inodes {
- xfs_inode_iter_fn fn;
- void *arg;
- bool moveon;
-};
-
-/* Scan all the inodes in an AG. */
-static void
-xfs_scan_ag_inodes(
- struct workqueue *wq,
- xfs_agnumber_t agno,
- void *arg)
-{
- struct xfs_scan_inodes *si = arg;
- struct scrub_ctx *ctx = (struct scrub_ctx *)wq->wq_ctx;
- char descr[DESCR_BUFSZ];
- bool moveon;
-
- snprintf(descr, DESCR_BUFSZ, _("dev %d:%d AG %u inodes"),
- major(ctx->fsinfo.fs_datadev),
- minor(ctx->fsinfo.fs_datadev),
- agno);
-
- moveon = xfs_iterate_inodes_ag(ctx, descr, ctx->fshandle, agno,
- si->fn, si->arg);
- if (!moveon)
- si->moveon = false;
-}
-
-/* Scan all the inodes in a filesystem. */
-bool
-xfs_scan_all_inodes(
+/*
+ * Scan all the inodes in a filesystem. On error, this function will log
+ * an error message and return -1.
+ */
+int
+scrub_scan_all_inodes(
struct scrub_ctx *ctx,
- xfs_inode_iter_fn fn,
+ scrub_inode_iter_fn fn,
void *arg)
{
- struct xfs_scan_inodes si;
+ struct scan_inodes si = {
+ .fn = fn,
+ .arg = arg,
+ };
xfs_agnumber_t agno;
struct workqueue wq;
int ret;
- si.moveon = true;
- si.fn = fn;
- si.arg = arg;
-
ret = workqueue_create(&wq, (struct xfs_mount *)ctx,
scrub_nproc_workqueue(ctx));
if (ret) {
str_liberror(ctx, ret, _("creating bulkstat workqueue"));
- return false;
+ return -1;
}
for (agno = 0; agno < ctx->mnt.fsgeom.agcount; agno++) {
- ret = workqueue_add(&wq, xfs_scan_ag_inodes, agno, &si);
+ ret = workqueue_add(&wq, scan_ag_inodes, agno, &si);
if (ret) {
- si.moveon = false;
+ si.aborted = true;
str_liberror(ctx, ret, _("queueing bulkstat work"));
break;
}
@@ -266,19 +250,17 @@ xfs_scan_all_inodes(
ret = workqueue_terminate(&wq);
if (ret) {
- si.moveon = false;
+ si.aborted = true;
str_liberror(ctx, ret, _("finishing bulkstat work"));
}
workqueue_destroy(&wq);
- return si.moveon;
+ return si.aborted ? -1 : 0;
}
-/*
- * Open a file by handle, or return a negative error code.
- */
+/* Open a file by handle, returning either the fd or -1 on error. */
int
-xfs_open_handle(
+scrub_open_handle(
struct xfs_handle *handle)
{
return open_by_fshandle(handle, sizeof(*handle),
diff --git a/scrub/inodes.h b/scrub/inodes.h
index e58795e7..3affaa00 100644
--- a/scrub/inodes.h
+++ b/scrub/inodes.h
@@ -12,13 +12,13 @@
* iteration will be restarted from the beginning of the inode allocation
* group. Any other non zero value will stop iteration.
*/
-typedef int (*xfs_inode_iter_fn)(struct scrub_ctx *ctx,
+typedef int (*scrub_inode_iter_fn)(struct scrub_ctx *ctx,
struct xfs_handle *handle, struct xfs_bulkstat *bs, void *arg);
#define XFS_ITERATE_INODES_ABORT (-1)
-bool xfs_scan_all_inodes(struct scrub_ctx *ctx, xfs_inode_iter_fn fn,
+int scrub_scan_all_inodes(struct scrub_ctx *ctx, scrub_inode_iter_fn fn,
void *arg);
-int xfs_open_handle(struct xfs_handle *handle);
+int scrub_open_handle(struct xfs_handle *handle);
#endif /* XFS_SCRUB_INODES_H_ */
diff --git a/scrub/phase3.c b/scrub/phase3.c
index 0d2c9019..171be3fd 100644
--- a/scrub/phase3.c
+++ b/scrub/phase3.c
@@ -78,7 +78,7 @@ xfs_scrub_inode(
/* Try to open the inode to pin it. */
if (S_ISREG(bstat->bs_mode)) {
- fd = xfs_open_handle(handle);
+ fd = scrub_open_handle(handle);
/* Stale inode means we scan the whole cluster again. */
if (fd < 0 && errno == ESTALE)
return ESTALE;
@@ -161,7 +161,6 @@ xfs_scan_inodes(
struct scrub_inode_ctx ictx;
uint64_t val;
int err;
- bool ret;
ictx.moveon = true;
err = ptcounter_alloc(scrub_nproc(ctx), &ictx.icount);
@@ -170,8 +169,8 @@ xfs_scan_inodes(
return false;
}
- ret = xfs_scan_all_inodes(ctx, xfs_scrub_inode, &ictx);
- if (!ret)
+ err = scrub_scan_all_inodes(ctx, xfs_scrub_inode, &ictx);
+ if (err)
ictx.moveon = false;
if (!ictx.moveon)
goto free;
diff --git a/scrub/phase5.c b/scrub/phase5.c
index 101a0a7a..7f4ae1a8 100644
--- a/scrub/phase5.c
+++ b/scrub/phase5.c
@@ -264,7 +264,7 @@ xfs_scrub_connections(
/* Open the dir, let the kernel try to reconnect it to the root. */
if (S_ISDIR(bstat->bs_mode)) {
- fd = xfs_open_handle(handle);
+ fd = scrub_open_handle(handle);
if (fd < 0) {
if (errno == ESTALE)
return ESTALE;
@@ -360,7 +360,7 @@ xfs_scan_connections(
struct scrub_ctx *ctx)
{
bool moveon = true;
- bool ret;
+ int ret;
if (ctx->corruptions_found || ctx->unfixable_errors) {
str_info(ctx, ctx->mntpoint,
@@ -372,9 +372,9 @@ _("Filesystem has errors, skipping connectivity checks."));
if (!moveon)
return false;
- ret = xfs_scan_all_inodes(ctx, xfs_scrub_connections, &moveon);
- if (!ret)
- moveon = false;
+ ret = scrub_scan_all_inodes(ctx, xfs_scrub_connections, &moveon);
+ if (ret)
+ return false;
if (!moveon)
return false;
xfs_scrub_report_preen_triggers(ctx);
diff --git a/scrub/phase6.c b/scrub/phase6.c
index 6a8dabe1..c4511b08 100644
--- a/scrub/phase6.c
+++ b/scrub/phase6.c
@@ -290,7 +290,7 @@ xfs_report_verify_inode(
bstat->bs_ino, bstat->bs_gen, _("(unlinked)"));
/* Try to open the inode. */
- fd = xfs_open_handle(handle);
+ fd = scrub_open_handle(handle);
if (fd < 0) {
error = errno;
if (error == ESTALE)
@@ -502,7 +502,8 @@ report_all_media_errors(
return false;
/* Scan for unlinked files. */
- return xfs_scan_all_inodes(ctx, xfs_report_verify_inode, vs);
+ ret = scrub_scan_all_inodes(ctx, xfs_report_verify_inode, vs);
+ return ret == 0;
}
/* Schedule a read-verify of a (data block) extent. */
^ permalink raw reply related [flat|nested] 25+ messages in thread
* [PATCH 04/18] xfs_scrub: remove moveon from vfs directory tree iteration
2019-10-22 18:50 [PATCH 00/18] xfs_scrub: remove moveon space aliens Darrick J. Wong
` (2 preceding siblings ...)
2019-10-22 18:50 ` [PATCH 03/18] xfs_scrub: remove moveon from inode iteration Darrick J. Wong
@ 2019-10-22 18:50 ` Darrick J. Wong
2019-10-22 18:50 ` [PATCH 05/18] xfs_scrub: remove moveon from spacemap Darrick J. Wong
` (14 subsequent siblings)
18 siblings, 0 replies; 25+ messages in thread
From: Darrick J. Wong @ 2019-10-22 18:50 UTC (permalink / raw)
To: sandeen, darrick.wong; +Cc: linux-xfs
From: Darrick J. Wong <darrick.wong@oracle.com>
Replace the moveon returns in the vfs directory tree walking functions
with a direct integer error return.
Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
---
scrub/phase6.c | 28 +++++++++++++++++-----------
scrub/vfs.c | 52 ++++++++++++++++++++++++++++++----------------------
scrub/vfs.h | 14 +++++++-------
3 files changed, 54 insertions(+), 40 deletions(-)
diff --git a/scrub/phase6.c b/scrub/phase6.c
index c4511b08..3da82e7c 100644
--- a/scrub/phase6.c
+++ b/scrub/phase6.c
@@ -311,21 +311,24 @@ _("Disappeared during read error reporting."));
}
/* Scan a directory for matches in the read verify error list. */
-static bool
+static int
xfs_report_verify_dir(
struct scrub_ctx *ctx,
const char *path,
int dir_fd,
void *arg)
{
- return xfs_report_verify_fd(ctx, path, dir_fd, arg);
+ bool moveon;
+
+ moveon = xfs_report_verify_fd(ctx, path, dir_fd, arg);
+ return moveon ? 0 : -1;
}
/*
* Scan the inode associated with a directory entry for matches with
* the read verify error list.
*/
-static bool
+static int
xfs_report_verify_dirent(
struct scrub_ctx *ctx,
const char *path,
@@ -340,11 +343,11 @@ xfs_report_verify_dirent(
/* Ignore things we can't open. */
if (!S_ISREG(sb->st_mode) && !S_ISDIR(sb->st_mode))
- return true;
+ return 0;
/* Ignore . and .. */
if (!strcmp(".", dirent->d_name) || !strcmp("..", dirent->d_name))
- return true;
+ return 0;
/*
* If we were given a dirent, open the associated file under
@@ -353,8 +356,12 @@ xfs_report_verify_dirent(
*/
fd = openat(dir_fd, dirent->d_name,
O_RDONLY | O_NOATIME | O_NOFOLLOW | O_NOCTTY);
- if (fd < 0)
- return true;
+ if (fd < 0) {
+ if (errno == ENOENT)
+ return 0;
+ str_errno(ctx, path);
+ return errno;
+ }
/* Go find the badness. */
moveon = xfs_report_verify_fd(ctx, path, fd, arg);
@@ -365,7 +372,7 @@ xfs_report_verify_dirent(
error = close(fd);
if (error)
str_errno(ctx, path);
- return moveon;
+ return moveon ? 0 : -1;
}
/* Use a fsmap to report metadata lost to a media error. */
@@ -480,7 +487,6 @@ report_all_media_errors(
struct scrub_ctx *ctx,
struct media_verify_state *vs)
{
- bool moveon;
int ret;
ret = report_disk_ioerrs(ctx, ctx->datadev, vs);
@@ -496,9 +502,9 @@ report_all_media_errors(
}
/* Scan the directory tree to get file paths. */
- moveon = scan_fs_tree(ctx, xfs_report_verify_dir,
+ ret = scan_fs_tree(ctx, xfs_report_verify_dir,
xfs_report_verify_dirent, vs);
- if (!moveon)
+ if (ret)
return false;
/* Scan for unlinked files. */
diff --git a/scrub/vfs.c b/scrub/vfs.c
index d7c40239..c807c9b9 100644
--- a/scrub/vfs.c
+++ b/scrub/vfs.c
@@ -30,7 +30,7 @@ struct scan_fs_tree {
pthread_mutex_t lock;
pthread_cond_t wakeup;
struct stat root_sb;
- bool moveon;
+ bool aborted;
scan_fs_tree_dir_fn dir_fn;
scan_fs_tree_dirent_fn dirent_fn;
void *arg;
@@ -138,8 +138,9 @@ scan_fs_dir(
}
/* Caller-specific directory checks. */
- if (!sft->dir_fn(ctx, sftd->path, dir_fd, sft->arg)) {
- sft->moveon = false;
+ error = sft->dir_fn(ctx, sftd->path, dir_fd, sft->arg);
+ if (error) {
+ sft->aborted = true;
error = close(dir_fd);
if (error)
str_errno(ctx, sftd->path);
@@ -150,11 +151,14 @@ scan_fs_dir(
dir = fdopendir(dir_fd);
if (!dir) {
str_errno(ctx, sftd->path);
+ sft->aborted = true;
close(dir_fd);
goto out;
}
rewinddir(dir);
- for (dirent = readdir(dir); dirent != NULL; dirent = readdir(dir)) {
+ for (dirent = readdir(dir);
+ !sft->aborted && dirent != NULL;
+ dirent = readdir(dir)) {
snprintf(newpath, PATH_MAX, "%s/%s", sftd->path,
dirent->d_name);
@@ -171,14 +175,15 @@ scan_fs_dir(
continue;
/* Caller-specific directory entry function. */
- if (!sft->dirent_fn(ctx, newpath, dir_fd, dirent, &sb,
- sft->arg)) {
- sft->moveon = false;
+ error = sft->dirent_fn(ctx, newpath, dir_fd, dirent, &sb,
+ sft->arg);
+ if (error) {
+ sft->aborted = true;
break;
}
if (xfs_scrub_excessive_errors(ctx)) {
- sft->moveon = false;
+ sft->aborted = true;
break;
}
@@ -189,7 +194,7 @@ scan_fs_dir(
if (error) {
str_liberror(ctx, error,
_("queueing subdirectory scan"));
- sft->moveon = false;
+ sft->aborted = true;
break;
}
}
@@ -206,8 +211,11 @@ _("queueing subdirectory scan"));
free(sftd);
}
-/* Scan the entire filesystem. */
-bool
+/*
+ * Scan the entire filesystem. This function returns 0 on success; if there
+ * are errors, this function will log them and returns nonzero.
+ */
+int
scan_fs_tree(
struct scrub_ctx *ctx,
scan_fs_tree_dir_fn dir_fn,
@@ -215,20 +223,18 @@ scan_fs_tree(
void *arg)
{
struct workqueue wq;
- struct scan_fs_tree sft;
- bool moveon = false;
+ struct scan_fs_tree sft = {
+ .root_sb = ctx->mnt_sb,
+ .dir_fn = dir_fn,
+ .dirent_fn = dirent_fn,
+ .arg = arg,
+ };
int ret;
- sft.moveon = true;
- sft.nr_dirs = 0;
- sft.root_sb = ctx->mnt_sb;
- sft.dir_fn = dir_fn;
- sft.dirent_fn = dirent_fn;
- sft.arg = arg;
ret = pthread_mutex_init(&sft.lock, NULL);
if (ret) {
str_liberror(ctx, ret, _("creating directory scan lock"));
- return false;
+ return ret;
}
ret = pthread_cond_init(&sft.wakeup, NULL);
if (ret) {
@@ -268,14 +274,16 @@ scan_fs_tree(
goto out_wq;
}
- moveon = sft.moveon;
+ if (!ret && sft.aborted)
+ ret = -1;
+
out_wq:
workqueue_destroy(&wq);
out_cond:
pthread_cond_destroy(&sft.wakeup);
out_mutex:
pthread_mutex_destroy(&sft.lock);
- return moveon;
+ return ret;
}
#ifndef FITRIM
diff --git a/scrub/vfs.h b/scrub/vfs.h
index af23674a..dc1099cf 100644
--- a/scrub/vfs.h
+++ b/scrub/vfs.h
@@ -8,20 +8,20 @@
/*
* Visit a subdirectory prior to iterating entries in that subdirectory.
- * Return true to continue iteration or false to stop iterating and return to
- * the caller.
+ * Return 0 to continue iteration or a positive error code to stop iterating
+ * and return to the caller.
*/
-typedef bool (*scan_fs_tree_dir_fn)(struct scrub_ctx *, const char *,
+typedef int (*scan_fs_tree_dir_fn)(struct scrub_ctx *, const char *,
int, void *);
/*
- * Visit each directory entry in a directory. Return true to continue
- * iteration or false to stop iterating and return to the caller.
+ * Visit each directory entry in a directory. Return 0 to continue iteration
+ * or a positive error code to stop iterating and return to the caller.
*/
-typedef bool (*scan_fs_tree_dirent_fn)(struct scrub_ctx *, const char *,
+typedef int (*scan_fs_tree_dirent_fn)(struct scrub_ctx *, const char *,
int, struct dirent *, struct stat *, void *);
-bool scan_fs_tree(struct scrub_ctx *ctx, scan_fs_tree_dir_fn dir_fn,
+int scan_fs_tree(struct scrub_ctx *ctx, scan_fs_tree_dir_fn dir_fn,
scan_fs_tree_dirent_fn dirent_fn, void *arg);
void fstrim(struct scrub_ctx *ctx);
^ permalink raw reply related [flat|nested] 25+ messages in thread
* [PATCH 05/18] xfs_scrub: remove moveon from spacemap
2019-10-22 18:50 [PATCH 00/18] xfs_scrub: remove moveon space aliens Darrick J. Wong
` (3 preceding siblings ...)
2019-10-22 18:50 ` [PATCH 04/18] xfs_scrub: remove moveon from vfs directory tree iteration Darrick J. Wong
@ 2019-10-22 18:50 ` Darrick J. Wong
2019-11-06 21:02 ` [PATCH v2 " Darrick J. Wong
2019-10-22 18:51 ` [PATCH 06/18] xfs_scrub: remove moveon from unicode name collision helpers Darrick J. Wong
` (13 subsequent siblings)
18 siblings, 1 reply; 25+ messages in thread
From: Darrick J. Wong @ 2019-10-22 18:50 UTC (permalink / raw)
To: sandeen, darrick.wong; +Cc: linux-xfs
From: Darrick J. Wong <darrick.wong@oracle.com>
Replace the moveon returns in the space map iteration code with a direct
integer return.
Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
---
scrub/phase6.c | 35 ++++++-------
scrub/phase7.c | 21 ++++----
scrub/spacemap.c | 145 +++++++++++++++++++++++++++++-------------------------
scrub/spacemap.h | 12 ++--
4 files changed, 109 insertions(+), 104 deletions(-)
diff --git a/scrub/phase6.c b/scrub/phase6.c
index 3da82e7c..805c93eb 100644
--- a/scrub/phase6.c
+++ b/scrub/phase6.c
@@ -376,10 +376,9 @@ xfs_report_verify_dirent(
}
/* Use a fsmap to report metadata lost to a media error. */
-static bool
+static int
report_ioerr_fsmap(
struct scrub_ctx *ctx,
- const char *descr,
struct fsmap *map,
void *arg)
{
@@ -390,7 +389,7 @@ report_ioerr_fsmap(
/* Don't care about unwritten extents. */
if (scrub_data < 3 && (map->fmr_flags & FMR_OF_PREALLOC))
- return true;
+ return 0;
if (err_physical > map->fmr_physical)
err_off = err_physical - map->fmr_physical;
@@ -422,7 +421,7 @@ report_ioerr_fsmap(
* to find the bad file's pathname.
*/
- return true;
+ return 0;
}
/*
@@ -436,16 +435,11 @@ report_ioerr(
void *arg)
{
struct fsmap keys[2];
- char descr[DESCR_BUFSZ];
struct disk_ioerr_report *dioerr = arg;
dev_t dev;
dev = xfs_disk_to_dev(dioerr->ctx, dioerr->disk);
- snprintf(descr, DESCR_BUFSZ,
-_("dev %d:%d ioerr @ %"PRIu64":%"PRIu64" "),
- major(dev), minor(dev), start, length);
-
/* Go figure out which blocks are bad from the fsmap. */
memset(keys, 0, sizeof(struct fsmap) * 2);
keys->fmr_device = dev;
@@ -455,9 +449,8 @@ _("dev %d:%d ioerr @ %"PRIu64":%"PRIu64" "),
(keys + 1)->fmr_owner = ULLONG_MAX;
(keys + 1)->fmr_offset = ULLONG_MAX;
(keys + 1)->fmr_flags = UINT_MAX;
- xfs_iterate_fsmap(dioerr->ctx, descr, keys, report_ioerr_fsmap,
+ return scrub_iterate_fsmap(dioerr->ctx, keys, report_ioerr_fsmap,
&start);
- return 0;
}
/* Report all the media errors found on a disk. */
@@ -513,10 +506,9 @@ report_all_media_errors(
}
/* Schedule a read-verify of a (data block) extent. */
-static bool
-xfs_check_rmap(
+static int
+check_rmap(
struct scrub_ctx *ctx,
- const char *descr,
struct fsmap *map,
void *arg)
{
@@ -546,7 +538,7 @@ xfs_check_rmap(
*/
if (map->fmr_flags & (FMR_OF_PREALLOC | FMR_OF_ATTR_FORK |
FMR_OF_EXTENT_MAP | FMR_OF_SPECIAL_OWNER))
- return true;
+ return 0;
/* XXX: Filter out directory data blocks. */
@@ -554,11 +546,11 @@ xfs_check_rmap(
ret = read_verify_schedule_io(rvp, map->fmr_physical, map->fmr_length,
vs);
if (ret) {
- str_liberror(ctx, ret, descr);
- return false;
+ str_liberror(ctx, ret, _("scheduling media verify command"));
+ return ret;
}
- return true;
+ return 0;
}
/* Wait for read/verify actions to finish, then return # bytes checked. */
@@ -717,8 +709,11 @@ xfs_scan_blocks(
if (scrub_data > 1)
moveon = verify_all_disks(ctx, &vs);
- else
- moveon = xfs_scan_all_spacemaps(ctx, xfs_check_rmap, &vs);
+ else {
+ ret = scrub_scan_all_spacemaps(ctx, check_rmap, &vs);
+ if (ret)
+ moveon = false;
+ }
if (!moveon)
goto out_rtpool;
diff --git a/scrub/phase7.c b/scrub/phase7.c
index 64e52359..4314904f 100644
--- a/scrub/phase7.c
+++ b/scrub/phase7.c
@@ -27,10 +27,9 @@ struct summary_counts {
};
/* Record block usage. */
-static bool
-xfs_record_block_summary(
+static int
+count_block_summary(
struct scrub_ctx *ctx,
- const char *descr,
struct fsmap *fsmap,
void *arg)
{
@@ -41,13 +40,13 @@ xfs_record_block_summary(
counts = ptvar_get((struct ptvar *)arg, &ret);
if (ret) {
str_liberror(ctx, ret, _("retrieving summary counts"));
- return false;
+ return ret;
}
if (fsmap->fmr_device == ctx->fsinfo.fs_logdev)
- return true;
+ return 0;
if ((fsmap->fmr_flags & FMR_OF_SPECIAL_OWNER) &&
fsmap->fmr_owner == XFS_FMR_OWN_FREE)
- return true;
+ return 0;
len = fsmap->fmr_length;
@@ -62,14 +61,14 @@ xfs_record_block_summary(
} else {
/* Count datadev extents. */
if (counts->next_phys >= fsmap->fmr_physical + len)
- return true;
+ return 0;
else if (counts->next_phys > fsmap->fmr_physical)
len = counts->next_phys - fsmap->fmr_physical;
counts->dbytes += len;
counts->next_phys = fsmap->fmr_physical + fsmap->fmr_length;
}
- return true;
+ return 0;
}
/* Add all the summaries in the per-thread counter */
@@ -145,9 +144,11 @@ xfs_scan_summary(
}
/* Use fsmap to count blocks. */
- moveon = xfs_scan_all_spacemaps(ctx, xfs_record_block_summary, ptvar);
- if (!moveon)
+ error = scrub_scan_all_spacemaps(ctx, count_block_summary, ptvar);
+ if (error) {
+ moveon = false;
goto out_free;
+ }
error = ptvar_foreach(ptvar, xfs_add_summaries, &totalcount);
if (error) {
str_liberror(ctx, error, _("counting blocks"));
diff --git a/scrub/spacemap.c b/scrub/spacemap.c
index 91e8badb..e56f090d 100644
--- a/scrub/spacemap.c
+++ b/scrub/spacemap.c
@@ -31,26 +31,25 @@
#define FSMAP_NR 65536
-/* Iterate all the fs block mappings between the two keys. */
-bool
-xfs_iterate_fsmap(
+/*
+ * Iterate all the fs block mappings between the two keys. Returns 0 or a
+ * positive error number.
+ */
+int
+scrub_iterate_fsmap(
struct scrub_ctx *ctx,
- const char *descr,
struct fsmap *keys,
- xfs_fsmap_iter_fn fn,
+ scrub_fsmap_iter_fn fn,
void *arg)
{
struct fsmap_head *head;
struct fsmap *p;
- bool moveon = true;
int i;
int error;
head = malloc(fsmap_sizeof(FSMAP_NR));
- if (!head) {
- str_errno(ctx, descr);
- return false;
- }
+ if (!head)
+ return errno;
memset(head, 0, sizeof(*head));
memcpy(head->fmh_keys, keys, sizeof(struct fsmap) * 2);
@@ -60,13 +59,11 @@ xfs_iterate_fsmap(
for (i = 0, p = head->fmh_recs;
i < head->fmh_entries;
i++, p++) {
- moveon = fn(ctx, descr, p, arg);
- if (!moveon)
+ error = fn(ctx, p, arg);
+ if (error)
goto out;
- if (xfs_scrub_excessive_errors(ctx)) {
- moveon = false;
+ if (xfs_scrub_excessive_errors(ctx))
goto out;
- }
}
if (head->fmh_entries == 0)
@@ -76,45 +73,36 @@ xfs_iterate_fsmap(
break;
fsmap_advance(head);
}
-
- if (error) {
- str_errno(ctx, descr);
- moveon = false;
- }
+ if (error)
+ error = errno;
out:
free(head);
- return moveon;
+ return error;
}
/* GETFSMAP wrappers routines. */
-struct xfs_scan_blocks {
- xfs_fsmap_iter_fn fn;
+struct scan_blocks {
+ scrub_fsmap_iter_fn fn;
void *arg;
- bool moveon;
+ bool aborted;
};
/* Iterate all the reverse mappings of an AG. */
static void
-xfs_scan_ag_blocks(
+scan_ag_rmaps(
struct workqueue *wq,
xfs_agnumber_t agno,
void *arg)
{
struct scrub_ctx *ctx = (struct scrub_ctx *)wq->wq_ctx;
- struct xfs_scan_blocks *sbx = arg;
- char descr[DESCR_BUFSZ];
+ struct scan_blocks *sbx = arg;
struct fsmap keys[2];
off64_t bperag;
- bool moveon;
+ int ret;
bperag = (off64_t)ctx->mnt.fsgeom.agblocks *
(off64_t)ctx->mnt.fsgeom.blocksize;
- snprintf(descr, DESCR_BUFSZ, _("dev %d:%d AG %u fsmap"),
- major(ctx->fsinfo.fs_datadev),
- minor(ctx->fsinfo.fs_datadev),
- agno);
-
memset(keys, 0, sizeof(struct fsmap) * 2);
keys->fmr_device = ctx->fsinfo.fs_datadev;
keys->fmr_physical = agno * bperag;
@@ -124,25 +112,32 @@ xfs_scan_ag_blocks(
(keys + 1)->fmr_offset = ULLONG_MAX;
(keys + 1)->fmr_flags = UINT_MAX;
- moveon = xfs_iterate_fsmap(ctx, descr, keys, sbx->fn, sbx->arg);
- if (!moveon)
- sbx->moveon = false;
+ if (sbx->aborted)
+ return;
+
+ ret = scrub_iterate_fsmap(ctx, keys, sbx->fn, sbx->arg);
+ if (ret) {
+ char descr[DESCR_BUFSZ];
+
+ snprintf(descr, DESCR_BUFSZ, _("dev %d:%d AG %u fsmap"),
+ major(ctx->fsinfo.fs_datadev),
+ minor(ctx->fsinfo.fs_datadev),
+ agno);
+ str_liberror(ctx, ret, descr);
+ sbx->aborted = true;
+ }
}
/* Iterate all the reverse mappings of a standalone device. */
static void
-xfs_scan_dev_blocks(
+scan_dev_rmaps(
struct scrub_ctx *ctx,
int idx,
dev_t dev,
- struct xfs_scan_blocks *sbx)
+ struct scan_blocks *sbx)
{
struct fsmap keys[2];
- char descr[DESCR_BUFSZ];
- bool moveon;
-
- snprintf(descr, DESCR_BUFSZ, _("dev %d:%d fsmap"),
- major(dev), minor(dev));
+ int ret;
memset(keys, 0, sizeof(struct fsmap) * 2);
keys->fmr_device = dev;
@@ -152,79 +147,90 @@ xfs_scan_dev_blocks(
(keys + 1)->fmr_offset = ULLONG_MAX;
(keys + 1)->fmr_flags = UINT_MAX;
- moveon = xfs_iterate_fsmap(ctx, descr, keys, sbx->fn, sbx->arg);
- if (!moveon)
- sbx->moveon = false;
+ if (sbx->aborted)
+ return;
+
+ ret = scrub_iterate_fsmap(ctx, keys, sbx->fn, sbx->arg);
+ if (ret) {
+ char descr[DESCR_BUFSZ];
+
+ snprintf(descr, DESCR_BUFSZ, _("dev %d:%d fsmap"),
+ major(dev), minor(dev));
+ str_liberror(ctx, ret, descr);
+ sbx->aborted = true;
+ }
}
/* Iterate all the reverse mappings of the realtime device. */
static void
-xfs_scan_rt_blocks(
+scan_rt_rmaps(
struct workqueue *wq,
xfs_agnumber_t agno,
void *arg)
{
struct scrub_ctx *ctx = (struct scrub_ctx *)wq->wq_ctx;
- xfs_scan_dev_blocks(ctx, agno, ctx->fsinfo.fs_rtdev, arg);
+ scan_dev_rmaps(ctx, agno, ctx->fsinfo.fs_rtdev, arg);
}
/* Iterate all the reverse mappings of the log device. */
static void
-xfs_scan_log_blocks(
+scan_log_rmaps(
struct workqueue *wq,
xfs_agnumber_t agno,
void *arg)
{
struct scrub_ctx *ctx = (struct scrub_ctx *)wq->wq_ctx;
- xfs_scan_dev_blocks(ctx, agno, ctx->fsinfo.fs_logdev, arg);
+ scan_dev_rmaps(ctx, agno, ctx->fsinfo.fs_logdev, arg);
}
-/* Scan all the blocks in a filesystem. */
-bool
-xfs_scan_all_spacemaps(
+/*
+ * Scan all the blocks in a filesystem. If errors occur, this function will
+ * log them and return nonzero.
+ */
+int
+scrub_scan_all_spacemaps(
struct scrub_ctx *ctx,
- xfs_fsmap_iter_fn fn,
+ scrub_fsmap_iter_fn fn,
void *arg)
{
struct workqueue wq;
- struct xfs_scan_blocks sbx;
+ struct scan_blocks sbx = {
+ .fn = fn,
+ .arg = arg,
+ };
xfs_agnumber_t agno;
int ret;
- sbx.moveon = true;
- sbx.fn = fn;
- sbx.arg = arg;
-
ret = workqueue_create(&wq, (struct xfs_mount *)ctx,
scrub_nproc_workqueue(ctx));
if (ret) {
str_liberror(ctx, ret, _("creating fsmap workqueue"));
- return false;
+ return ret;
}
if (ctx->fsinfo.fs_rt) {
- ret = workqueue_add(&wq, xfs_scan_rt_blocks,
+ ret = workqueue_add(&wq, scan_rt_rmaps,
ctx->mnt.fsgeom.agcount + 1, &sbx);
if (ret) {
- sbx.moveon = false;
+ sbx.aborted = true;
str_liberror(ctx, ret, _("queueing rtdev fsmap work"));
goto out;
}
}
if (ctx->fsinfo.fs_log) {
- ret = workqueue_add(&wq, xfs_scan_log_blocks,
+ ret = workqueue_add(&wq, scan_log_rmaps,
ctx->mnt.fsgeom.agcount + 2, &sbx);
if (ret) {
- sbx.moveon = false;
+ sbx.aborted = true;
str_liberror(ctx, ret, _("queueing logdev fsmap work"));
goto out;
}
}
for (agno = 0; agno < ctx->mnt.fsgeom.agcount; agno++) {
- ret = workqueue_add(&wq, xfs_scan_ag_blocks, agno, &sbx);
+ ret = workqueue_add(&wq, scan_ag_rmaps, agno, &sbx);
if (ret) {
- sbx.moveon = false;
+ sbx.aborted = true;
str_liberror(ctx, ret, _("queueing per-AG fsmap work"));
break;
}
@@ -232,10 +238,13 @@ xfs_scan_all_spacemaps(
out:
ret = workqueue_terminate(&wq);
if (ret) {
- sbx.moveon = false;
+ sbx.aborted = true;
str_liberror(ctx, ret, _("finishing fsmap work"));
}
workqueue_destroy(&wq);
- return sbx.moveon;
+ if (!ret && sbx.aborted)
+ ret = -1;
+
+ return ret;
}
diff --git a/scrub/spacemap.h b/scrub/spacemap.h
index c29c43a5..8a6d1e36 100644
--- a/scrub/spacemap.h
+++ b/scrub/spacemap.h
@@ -7,15 +7,15 @@
#define XFS_SCRUB_SPACEMAP_H_
/*
- * Visit each space mapping in the filesystem. Return true to continue
- * iteration or false to stop iterating and return to the caller.
+ * Visit each space mapping in the filesystem. Return 0 to continue iteration
+ * or a positive error code to stop iterating and return to the caller.
*/
-typedef bool (*xfs_fsmap_iter_fn)(struct scrub_ctx *ctx, const char *descr,
+typedef int (*scrub_fsmap_iter_fn)(struct scrub_ctx *ctx,
struct fsmap *fsr, void *arg);
-bool xfs_iterate_fsmap(struct scrub_ctx *ctx, const char *descr,
- struct fsmap *keys, xfs_fsmap_iter_fn fn, void *arg);
-bool xfs_scan_all_spacemaps(struct scrub_ctx *ctx, xfs_fsmap_iter_fn fn,
+int scrub_iterate_fsmap(struct scrub_ctx *ctx, struct fsmap *keys,
+ scrub_fsmap_iter_fn fn, void *arg);
+int scrub_scan_all_spacemaps(struct scrub_ctx *ctx, scrub_fsmap_iter_fn fn,
void *arg);
#endif /* XFS_SCRUB_SPACEMAP_H_ */
^ permalink raw reply related [flat|nested] 25+ messages in thread
* [PATCH v2 05/18] xfs_scrub: remove moveon from spacemap
2019-10-22 18:50 ` [PATCH 05/18] xfs_scrub: remove moveon from spacemap Darrick J. Wong
@ 2019-11-06 21:02 ` Darrick J. Wong
0 siblings, 0 replies; 25+ messages in thread
From: Darrick J. Wong @ 2019-11-06 21:02 UTC (permalink / raw)
To: sandeen; +Cc: linux-xfs
From: Darrick J. Wong <darrick.wong@oracle.com>
Replace the moveon returns in the space map iteration code with a direct
integer return.
Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
---
v2: rebase without full-disk media scan
---
scrub/phase6.c | 34 +++++--------
scrub/phase7.c | 21 ++++----
scrub/spacemap.c | 145 +++++++++++++++++++++++++++++-------------------------
scrub/spacemap.h | 12 ++--
4 files changed, 108 insertions(+), 104 deletions(-)
diff --git a/scrub/phase6.c b/scrub/phase6.c
index a0a71ce3..62629732 100644
--- a/scrub/phase6.c
+++ b/scrub/phase6.c
@@ -374,10 +374,9 @@ xfs_report_verify_dirent(
}
/* Use a fsmap to report metadata lost to a media error. */
-static bool
+static int
report_ioerr_fsmap(
struct scrub_ctx *ctx,
- const char *descr,
struct fsmap *map,
void *arg)
{
@@ -388,7 +387,7 @@ report_ioerr_fsmap(
/* Don't care about unwritten extents. */
if (map->fmr_flags & FMR_OF_PREALLOC)
- return true;
+ return 0;
if (err_physical > map->fmr_physical)
err_off = err_physical - map->fmr_physical;
@@ -420,7 +419,7 @@ report_ioerr_fsmap(
* to find the bad file's pathname.
*/
- return true;
+ return 0;
}
/*
@@ -434,16 +433,11 @@ report_ioerr(
void *arg)
{
struct fsmap keys[2];
- char descr[DESCR_BUFSZ];
struct disk_ioerr_report *dioerr = arg;
dev_t dev;
dev = xfs_disk_to_dev(dioerr->ctx, dioerr->disk);
- snprintf(descr, DESCR_BUFSZ,
-_("dev %d:%d ioerr @ %"PRIu64":%"PRIu64" "),
- major(dev), minor(dev), start, length);
-
/* Go figure out which blocks are bad from the fsmap. */
memset(keys, 0, sizeof(struct fsmap) * 2);
keys->fmr_device = dev;
@@ -453,9 +447,8 @@ _("dev %d:%d ioerr @ %"PRIu64":%"PRIu64" "),
(keys + 1)->fmr_owner = ULLONG_MAX;
(keys + 1)->fmr_offset = ULLONG_MAX;
(keys + 1)->fmr_flags = UINT_MAX;
- xfs_iterate_fsmap(dioerr->ctx, descr, keys, report_ioerr_fsmap,
+ return scrub_iterate_fsmap(dioerr->ctx, keys, report_ioerr_fsmap,
&start);
- return 0;
}
/* Report all the media errors found on a disk. */
@@ -511,10 +504,9 @@ report_all_media_errors(
}
/* Schedule a read-verify of a (data block) extent. */
-static bool
-xfs_check_rmap(
+static int
+check_rmap(
struct scrub_ctx *ctx,
- const char *descr,
struct fsmap *map,
void *arg)
{
@@ -544,7 +536,7 @@ xfs_check_rmap(
*/
if (map->fmr_flags & (FMR_OF_PREALLOC | FMR_OF_ATTR_FORK |
FMR_OF_EXTENT_MAP | FMR_OF_SPECIAL_OWNER))
- return true;
+ return 0;
/* XXX: Filter out directory data blocks. */
@@ -552,11 +544,11 @@ xfs_check_rmap(
ret = read_verify_schedule_io(rvp, map->fmr_physical, map->fmr_length,
vs);
if (ret) {
- str_liberror(ctx, ret, descr);
- return false;
+ str_liberror(ctx, ret, _("scheduling media verify command"));
+ return ret;
}
- return true;
+ return 0;
}
/* Wait for read/verify actions to finish, then return # bytes checked. */
@@ -669,9 +661,11 @@ xfs_scan_blocks(
goto out_logpool;
}
}
- moveon = xfs_scan_all_spacemaps(ctx, xfs_check_rmap, &vs);
- if (!moveon)
+ ret = scrub_scan_all_spacemaps(ctx, check_rmap, &vs);
+ if (ret) {
+ moveon = false;
goto out_rtpool;
+ }
ret = clean_pool(vs.rvp_data, &ctx->bytes_checked);
if (ret) {
diff --git a/scrub/phase7.c b/scrub/phase7.c
index 6daf75b5..28689eac 100644
--- a/scrub/phase7.c
+++ b/scrub/phase7.c
@@ -27,10 +27,9 @@ struct summary_counts {
};
/* Record block usage. */
-static bool
-xfs_record_block_summary(
+static int
+count_block_summary(
struct scrub_ctx *ctx,
- const char *descr,
struct fsmap *fsmap,
void *arg)
{
@@ -41,13 +40,13 @@ xfs_record_block_summary(
counts = ptvar_get((struct ptvar *)arg, &ret);
if (ret) {
str_liberror(ctx, ret, _("retrieving summary counts"));
- return false;
+ return ret;
}
if (fsmap->fmr_device == ctx->fsinfo.fs_logdev)
- return true;
+ return 0;
if ((fsmap->fmr_flags & FMR_OF_SPECIAL_OWNER) &&
fsmap->fmr_owner == XFS_FMR_OWN_FREE)
- return true;
+ return 0;
len = fsmap->fmr_length;
@@ -62,14 +61,14 @@ xfs_record_block_summary(
} else {
/* Count datadev extents. */
if (counts->next_phys >= fsmap->fmr_physical + len)
- return true;
+ return 0;
else if (counts->next_phys > fsmap->fmr_physical)
len = counts->next_phys - fsmap->fmr_physical;
counts->dbytes += len;
counts->next_phys = fsmap->fmr_physical + fsmap->fmr_length;
}
- return true;
+ return 0;
}
/* Add all the summaries in the per-thread counter */
@@ -144,9 +143,11 @@ xfs_scan_summary(
}
/* Use fsmap to count blocks. */
- moveon = xfs_scan_all_spacemaps(ctx, xfs_record_block_summary, ptvar);
- if (!moveon)
+ error = scrub_scan_all_spacemaps(ctx, count_block_summary, ptvar);
+ if (error) {
+ moveon = false;
goto out_free;
+ }
error = ptvar_foreach(ptvar, xfs_add_summaries, &totalcount);
if (error) {
str_liberror(ctx, error, _("counting blocks"));
diff --git a/scrub/spacemap.c b/scrub/spacemap.c
index 91e8badb..e56f090d 100644
--- a/scrub/spacemap.c
+++ b/scrub/spacemap.c
@@ -31,26 +31,25 @@
#define FSMAP_NR 65536
-/* Iterate all the fs block mappings between the two keys. */
-bool
-xfs_iterate_fsmap(
+/*
+ * Iterate all the fs block mappings between the two keys. Returns 0 or a
+ * positive error number.
+ */
+int
+scrub_iterate_fsmap(
struct scrub_ctx *ctx,
- const char *descr,
struct fsmap *keys,
- xfs_fsmap_iter_fn fn,
+ scrub_fsmap_iter_fn fn,
void *arg)
{
struct fsmap_head *head;
struct fsmap *p;
- bool moveon = true;
int i;
int error;
head = malloc(fsmap_sizeof(FSMAP_NR));
- if (!head) {
- str_errno(ctx, descr);
- return false;
- }
+ if (!head)
+ return errno;
memset(head, 0, sizeof(*head));
memcpy(head->fmh_keys, keys, sizeof(struct fsmap) * 2);
@@ -60,13 +59,11 @@ xfs_iterate_fsmap(
for (i = 0, p = head->fmh_recs;
i < head->fmh_entries;
i++, p++) {
- moveon = fn(ctx, descr, p, arg);
- if (!moveon)
+ error = fn(ctx, p, arg);
+ if (error)
goto out;
- if (xfs_scrub_excessive_errors(ctx)) {
- moveon = false;
+ if (xfs_scrub_excessive_errors(ctx))
goto out;
- }
}
if (head->fmh_entries == 0)
@@ -76,45 +73,36 @@ xfs_iterate_fsmap(
break;
fsmap_advance(head);
}
-
- if (error) {
- str_errno(ctx, descr);
- moveon = false;
- }
+ if (error)
+ error = errno;
out:
free(head);
- return moveon;
+ return error;
}
/* GETFSMAP wrappers routines. */
-struct xfs_scan_blocks {
- xfs_fsmap_iter_fn fn;
+struct scan_blocks {
+ scrub_fsmap_iter_fn fn;
void *arg;
- bool moveon;
+ bool aborted;
};
/* Iterate all the reverse mappings of an AG. */
static void
-xfs_scan_ag_blocks(
+scan_ag_rmaps(
struct workqueue *wq,
xfs_agnumber_t agno,
void *arg)
{
struct scrub_ctx *ctx = (struct scrub_ctx *)wq->wq_ctx;
- struct xfs_scan_blocks *sbx = arg;
- char descr[DESCR_BUFSZ];
+ struct scan_blocks *sbx = arg;
struct fsmap keys[2];
off64_t bperag;
- bool moveon;
+ int ret;
bperag = (off64_t)ctx->mnt.fsgeom.agblocks *
(off64_t)ctx->mnt.fsgeom.blocksize;
- snprintf(descr, DESCR_BUFSZ, _("dev %d:%d AG %u fsmap"),
- major(ctx->fsinfo.fs_datadev),
- minor(ctx->fsinfo.fs_datadev),
- agno);
-
memset(keys, 0, sizeof(struct fsmap) * 2);
keys->fmr_device = ctx->fsinfo.fs_datadev;
keys->fmr_physical = agno * bperag;
@@ -124,25 +112,32 @@ xfs_scan_ag_blocks(
(keys + 1)->fmr_offset = ULLONG_MAX;
(keys + 1)->fmr_flags = UINT_MAX;
- moveon = xfs_iterate_fsmap(ctx, descr, keys, sbx->fn, sbx->arg);
- if (!moveon)
- sbx->moveon = false;
+ if (sbx->aborted)
+ return;
+
+ ret = scrub_iterate_fsmap(ctx, keys, sbx->fn, sbx->arg);
+ if (ret) {
+ char descr[DESCR_BUFSZ];
+
+ snprintf(descr, DESCR_BUFSZ, _("dev %d:%d AG %u fsmap"),
+ major(ctx->fsinfo.fs_datadev),
+ minor(ctx->fsinfo.fs_datadev),
+ agno);
+ str_liberror(ctx, ret, descr);
+ sbx->aborted = true;
+ }
}
/* Iterate all the reverse mappings of a standalone device. */
static void
-xfs_scan_dev_blocks(
+scan_dev_rmaps(
struct scrub_ctx *ctx,
int idx,
dev_t dev,
- struct xfs_scan_blocks *sbx)
+ struct scan_blocks *sbx)
{
struct fsmap keys[2];
- char descr[DESCR_BUFSZ];
- bool moveon;
-
- snprintf(descr, DESCR_BUFSZ, _("dev %d:%d fsmap"),
- major(dev), minor(dev));
+ int ret;
memset(keys, 0, sizeof(struct fsmap) * 2);
keys->fmr_device = dev;
@@ -152,79 +147,90 @@ xfs_scan_dev_blocks(
(keys + 1)->fmr_offset = ULLONG_MAX;
(keys + 1)->fmr_flags = UINT_MAX;
- moveon = xfs_iterate_fsmap(ctx, descr, keys, sbx->fn, sbx->arg);
- if (!moveon)
- sbx->moveon = false;
+ if (sbx->aborted)
+ return;
+
+ ret = scrub_iterate_fsmap(ctx, keys, sbx->fn, sbx->arg);
+ if (ret) {
+ char descr[DESCR_BUFSZ];
+
+ snprintf(descr, DESCR_BUFSZ, _("dev %d:%d fsmap"),
+ major(dev), minor(dev));
+ str_liberror(ctx, ret, descr);
+ sbx->aborted = true;
+ }
}
/* Iterate all the reverse mappings of the realtime device. */
static void
-xfs_scan_rt_blocks(
+scan_rt_rmaps(
struct workqueue *wq,
xfs_agnumber_t agno,
void *arg)
{
struct scrub_ctx *ctx = (struct scrub_ctx *)wq->wq_ctx;
- xfs_scan_dev_blocks(ctx, agno, ctx->fsinfo.fs_rtdev, arg);
+ scan_dev_rmaps(ctx, agno, ctx->fsinfo.fs_rtdev, arg);
}
/* Iterate all the reverse mappings of the log device. */
static void
-xfs_scan_log_blocks(
+scan_log_rmaps(
struct workqueue *wq,
xfs_agnumber_t agno,
void *arg)
{
struct scrub_ctx *ctx = (struct scrub_ctx *)wq->wq_ctx;
- xfs_scan_dev_blocks(ctx, agno, ctx->fsinfo.fs_logdev, arg);
+ scan_dev_rmaps(ctx, agno, ctx->fsinfo.fs_logdev, arg);
}
-/* Scan all the blocks in a filesystem. */
-bool
-xfs_scan_all_spacemaps(
+/*
+ * Scan all the blocks in a filesystem. If errors occur, this function will
+ * log them and return nonzero.
+ */
+int
+scrub_scan_all_spacemaps(
struct scrub_ctx *ctx,
- xfs_fsmap_iter_fn fn,
+ scrub_fsmap_iter_fn fn,
void *arg)
{
struct workqueue wq;
- struct xfs_scan_blocks sbx;
+ struct scan_blocks sbx = {
+ .fn = fn,
+ .arg = arg,
+ };
xfs_agnumber_t agno;
int ret;
- sbx.moveon = true;
- sbx.fn = fn;
- sbx.arg = arg;
-
ret = workqueue_create(&wq, (struct xfs_mount *)ctx,
scrub_nproc_workqueue(ctx));
if (ret) {
str_liberror(ctx, ret, _("creating fsmap workqueue"));
- return false;
+ return ret;
}
if (ctx->fsinfo.fs_rt) {
- ret = workqueue_add(&wq, xfs_scan_rt_blocks,
+ ret = workqueue_add(&wq, scan_rt_rmaps,
ctx->mnt.fsgeom.agcount + 1, &sbx);
if (ret) {
- sbx.moveon = false;
+ sbx.aborted = true;
str_liberror(ctx, ret, _("queueing rtdev fsmap work"));
goto out;
}
}
if (ctx->fsinfo.fs_log) {
- ret = workqueue_add(&wq, xfs_scan_log_blocks,
+ ret = workqueue_add(&wq, scan_log_rmaps,
ctx->mnt.fsgeom.agcount + 2, &sbx);
if (ret) {
- sbx.moveon = false;
+ sbx.aborted = true;
str_liberror(ctx, ret, _("queueing logdev fsmap work"));
goto out;
}
}
for (agno = 0; agno < ctx->mnt.fsgeom.agcount; agno++) {
- ret = workqueue_add(&wq, xfs_scan_ag_blocks, agno, &sbx);
+ ret = workqueue_add(&wq, scan_ag_rmaps, agno, &sbx);
if (ret) {
- sbx.moveon = false;
+ sbx.aborted = true;
str_liberror(ctx, ret, _("queueing per-AG fsmap work"));
break;
}
@@ -232,10 +238,13 @@ xfs_scan_all_spacemaps(
out:
ret = workqueue_terminate(&wq);
if (ret) {
- sbx.moveon = false;
+ sbx.aborted = true;
str_liberror(ctx, ret, _("finishing fsmap work"));
}
workqueue_destroy(&wq);
- return sbx.moveon;
+ if (!ret && sbx.aborted)
+ ret = -1;
+
+ return ret;
}
diff --git a/scrub/spacemap.h b/scrub/spacemap.h
index c29c43a5..8a6d1e36 100644
--- a/scrub/spacemap.h
+++ b/scrub/spacemap.h
@@ -7,15 +7,15 @@
#define XFS_SCRUB_SPACEMAP_H_
/*
- * Visit each space mapping in the filesystem. Return true to continue
- * iteration or false to stop iterating and return to the caller.
+ * Visit each space mapping in the filesystem. Return 0 to continue iteration
+ * or a positive error code to stop iterating and return to the caller.
*/
-typedef bool (*xfs_fsmap_iter_fn)(struct scrub_ctx *ctx, const char *descr,
+typedef int (*scrub_fsmap_iter_fn)(struct scrub_ctx *ctx,
struct fsmap *fsr, void *arg);
-bool xfs_iterate_fsmap(struct scrub_ctx *ctx, const char *descr,
- struct fsmap *keys, xfs_fsmap_iter_fn fn, void *arg);
-bool xfs_scan_all_spacemaps(struct scrub_ctx *ctx, xfs_fsmap_iter_fn fn,
+int scrub_iterate_fsmap(struct scrub_ctx *ctx, struct fsmap *keys,
+ scrub_fsmap_iter_fn fn, void *arg);
+int scrub_scan_all_spacemaps(struct scrub_ctx *ctx, scrub_fsmap_iter_fn fn,
void *arg);
#endif /* XFS_SCRUB_SPACEMAP_H_ */
^ permalink raw reply related [flat|nested] 25+ messages in thread
* [PATCH 06/18] xfs_scrub: remove moveon from unicode name collision helpers
2019-10-22 18:50 [PATCH 00/18] xfs_scrub: remove moveon space aliens Darrick J. Wong
` (4 preceding siblings ...)
2019-10-22 18:50 ` [PATCH 05/18] xfs_scrub: remove moveon from spacemap Darrick J. Wong
@ 2019-10-22 18:51 ` Darrick J. Wong
2019-10-22 18:51 ` [PATCH 07/18] xfs_scrub: remove moveon from progress report helpers Darrick J. Wong
` (12 subsequent siblings)
18 siblings, 0 replies; 25+ messages in thread
From: Darrick J. Wong @ 2019-10-22 18:51 UTC (permalink / raw)
To: sandeen, darrick.wong; +Cc: linux-xfs
From: Darrick J. Wong <darrick.wong@oracle.com>
Replace the moveon returns in the unicode name collsion detector code
with a direct integer error return.
Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
---
scrub/phase5.c | 52 +++++++++++++++++++++++++++++++-------------
scrub/unicrash.c | 64 ++++++++++++++++++++++++++----------------------------
scrub/unicrash.h | 24 ++++++++++----------
3 files changed, 80 insertions(+), 60 deletions(-)
diff --git a/scrub/phase5.c b/scrub/phase5.c
index 7f4ae1a8..e752a0c4 100644
--- a/scrub/phase5.c
+++ b/scrub/phase5.c
@@ -87,23 +87,32 @@ xfs_scrub_scan_dirents(
DIR *dir;
struct dirent *dentry;
bool moveon = true;
+ int ret;
dir = fdopendir(*fd);
if (!dir) {
str_errno(ctx, descr_render(dsc));
+ moveon = false;
goto out;
}
*fd = -1; /* closedir will close *fd for us */
- moveon = unicrash_dir_init(&uc, ctx, bstat);
- if (!moveon)
+ ret = unicrash_dir_init(&uc, ctx, bstat);
+ if (ret) {
+ str_liberror(ctx, ret, descr_render(dsc));
+ moveon = false;
goto out_unicrash;
+ }
dentry = readdir(dir);
while (dentry) {
- if (uc)
- moveon = unicrash_check_dir_name(uc, dsc, dentry);
- else
+ if (uc) {
+ ret = unicrash_check_dir_name(uc, dsc, dentry);
+ if (ret) {
+ str_liberror(ctx, ret, descr_render(dsc));
+ moveon = false;
+ }
+ } else
moveon = xfs_scrub_check_name(ctx, dsc,
_("directory"), dentry->d_name);
if (!moveon)
@@ -154,9 +163,11 @@ xfs_scrub_scan_fhandle_namespace_xattrs(
int i;
int error;
- moveon = unicrash_xattr_init(&uc, ctx, bstat);
- if (!moveon)
+ error = unicrash_xattr_init(&uc, ctx, bstat);
+ if (error) {
+ str_liberror(ctx, error, descr_render(dsc));
return false;
+ }
memset(attrbuf, 0, XFS_XATTR_LIST_MAX);
memset(&cur, 0, sizeof(cur));
@@ -169,10 +180,15 @@ xfs_scrub_scan_fhandle_namespace_xattrs(
ent = ATTR_ENTRY(attrlist, i);
snprintf(keybuf, XATTR_NAME_MAX, "%s.%s", attr_ns->name,
ent->a_name);
- if (uc)
- moveon = unicrash_check_xattr_name(uc, dsc,
+ if (uc) {
+ error = unicrash_check_xattr_name(uc, dsc,
keybuf);
- else
+ if (error) {
+ str_liberror(ctx, error,
+ descr_render(dsc));
+ moveon = false;
+ }
+ } else
moveon = xfs_scrub_check_name(ctx, dsc,
_("extended attribute"),
keybuf);
@@ -321,9 +337,11 @@ xfs_scrub_fs_label(
bool moveon = true;
int error;
- moveon = unicrash_fs_label_init(&uc, ctx);
- if (!moveon)
+ error = unicrash_fs_label_init(&uc, ctx);
+ if (error) {
+ str_liberror(ctx, error, descr_render(&dsc));
return false;
+ }
descr_set(&dsc, NULL);
@@ -342,9 +360,13 @@ xfs_scrub_fs_label(
goto out;
/* Otherwise check for weirdness. */
- if (uc)
- moveon = unicrash_check_fs_label(uc, &dsc, label);
- else
+ if (uc) {
+ error = unicrash_check_fs_label(uc, &dsc, label);
+ if (error) {
+ str_liberror(ctx, error, descr_render(&dsc));
+ moveon = false;
+ }
+ } else
moveon = xfs_scrub_check_name(ctx, &dsc, _("filesystem label"),
label);
if (!moveon)
diff --git a/scrub/unicrash.c b/scrub/unicrash.c
index 9b619c02..d5d2cf20 100644
--- a/scrub/unicrash.c
+++ b/scrub/unicrash.c
@@ -145,8 +145,8 @@ is_utf8_locale(void)
}
/*
- * Generate normalized form and skeleton of the name.
- * If this fails, just forget everything; this is an advisory checker.
+ * Generate normalized form and skeleton of the name. If this fails, just
+ * forget everything and return false; this is an advisory checker.
*/
static bool
name_entry_compute_checknames(
@@ -379,7 +379,7 @@ name_entry_examine(
}
/* Initialize the collision detector. */
-static bool
+static int
unicrash_init(
struct unicrash **ucp,
struct scrub_ctx *ctx,
@@ -392,7 +392,7 @@ unicrash_init(
if (!is_utf8_locale()) {
*ucp = NULL;
- return true;
+ return 0;
}
if (nr_buckets > 65536)
@@ -402,7 +402,7 @@ unicrash_init(
p = calloc(1, UNICRASH_SZ(nr_buckets));
if (!p)
- return false;
+ return errno;
p->ctx = ctx;
p->nr_buckets = nr_buckets;
p->compare_ino = compare_ino;
@@ -418,12 +418,12 @@ unicrash_init(
p->is_only_root_writeable = is_only_root_writeable;
*ucp = p;
- return true;
+ return 0;
out_spoof:
uspoof_close(p->spoof);
out_free:
free(p);
- return false;
+ return ENOMEM;
}
/*
@@ -441,7 +441,7 @@ is_only_root_writable(
}
/* Initialize the collision detector for a directory. */
-bool
+int
unicrash_dir_init(
struct unicrash **ucp,
struct scrub_ctx *ctx,
@@ -456,7 +456,7 @@ unicrash_dir_init(
}
/* Initialize the collision detector for an extended attribute. */
-bool
+int
unicrash_xattr_init(
struct unicrash **ucp,
struct scrub_ctx *ctx,
@@ -468,7 +468,7 @@ unicrash_xattr_init(
}
/* Initialize the collision detector for a filesystem label. */
-bool
+int
unicrash_fs_label_init(
struct unicrash **ucp,
struct scrub_ctx *ctx)
@@ -608,7 +608,7 @@ _("Unicode name \"%s\" in %s could be confused with \"%s\"."),
* must be skeletonized according to Unicode TR39 to detect names that
* could be visually confused with each other.
*/
-static bool
+static void
unicrash_add(
struct unicrash *uc,
struct name_entry *new_entry,
@@ -633,7 +633,7 @@ unicrash_add(
(uc->compare_ino ? entry->ino != new_entry->ino : true)) {
*badflags |= UNICRASH_NOT_UNIQUE;
*existing_entry = entry;
- return true;
+ return;
}
/* Confusable? */
@@ -642,16 +642,14 @@ unicrash_add(
(uc->compare_ino ? entry->ino != new_entry->ino : true)) {
*badflags |= UNICRASH_CONFUSABLE;
*existing_entry = entry;
- return true;
+ return;
}
entry = entry->next;
}
-
- return true;
}
/* Check a name for unicode normalization problems or collisions. */
-static bool
+static int
__unicrash_check_name(
struct unicrash *uc,
struct descr *dsc,
@@ -660,67 +658,67 @@ __unicrash_check_name(
xfs_ino_t ino)
{
struct name_entry *dup_entry = NULL;
- struct name_entry *new_entry;
+ struct name_entry *new_entry = NULL;
unsigned int badflags = 0;
- bool moveon;
/* If we can't create entry data, just skip it. */
if (!name_entry_create(uc, name, ino, &new_entry))
- return true;
+ return 0;
name_entry_examine(new_entry, &badflags);
-
- moveon = unicrash_add(uc, new_entry, &badflags, &dup_entry);
- if (!moveon)
- return false;
-
+ unicrash_add(uc, new_entry, &badflags, &dup_entry);
if (badflags)
unicrash_complain(uc, dsc, namedescr, new_entry, badflags,
dup_entry);
- return true;
+ return 0;
}
-/* Check a directory entry for unicode normalization problems or collisions. */
-bool
+/*
+ * Check a directory entry for unicode normalization problems or collisions.
+ * If errors occur, this function will log them and return nonzero.
+ */
+int
unicrash_check_dir_name(
struct unicrash *uc,
struct descr *dsc,
struct dirent *dentry)
{
if (!uc)
- return true;
+ return 0;
return __unicrash_check_name(uc, dsc, _("directory"),
dentry->d_name, dentry->d_ino);
}
/*
* Check an extended attribute name for unicode normalization problems
- * or collisions.
+ * or collisions. If errors occur, this function will log them and return
+ * nonzero.
*/
-bool
+int
unicrash_check_xattr_name(
struct unicrash *uc,
struct descr *dsc,
const char *attrname)
{
if (!uc)
- return true;
+ return 0;
return __unicrash_check_name(uc, dsc, _("extended attribute"),
attrname, 0);
}
/*
* Check the fs label for unicode normalization problems or misleading bits.
+ * If errors occur, this function will log them and return nonzero.
*/
-bool
+int
unicrash_check_fs_label(
struct unicrash *uc,
struct descr *dsc,
const char *label)
{
if (!uc)
- return true;
+ return 0;
return __unicrash_check_name(uc, dsc, _("filesystem label"),
label, 0);
}
diff --git a/scrub/unicrash.h b/scrub/unicrash.h
index af96b230..c3a7f385 100644
--- a/scrub/unicrash.h
+++ b/scrub/unicrash.h
@@ -13,26 +13,26 @@ struct unicrash;
struct dirent;
-bool unicrash_dir_init(struct unicrash **ucp, struct scrub_ctx *ctx,
+int unicrash_dir_init(struct unicrash **ucp, struct scrub_ctx *ctx,
struct xfs_bulkstat *bstat);
-bool unicrash_xattr_init(struct unicrash **ucp, struct scrub_ctx *ctx,
+int unicrash_xattr_init(struct unicrash **ucp, struct scrub_ctx *ctx,
struct xfs_bulkstat *bstat);
-bool unicrash_fs_label_init(struct unicrash **ucp, struct scrub_ctx *ctx);
+int unicrash_fs_label_init(struct unicrash **ucp, struct scrub_ctx *ctx);
void unicrash_free(struct unicrash *uc);
-bool unicrash_check_dir_name(struct unicrash *uc, struct descr *dsc,
+int unicrash_check_dir_name(struct unicrash *uc, struct descr *dsc,
struct dirent *dirent);
-bool unicrash_check_xattr_name(struct unicrash *uc, struct descr *dsc,
+int unicrash_check_xattr_name(struct unicrash *uc, struct descr *dsc,
const char *attrname);
-bool unicrash_check_fs_label(struct unicrash *uc, struct descr *dsc,
+int unicrash_check_fs_label(struct unicrash *uc, struct descr *dsc,
const char *label);
#else
-# define unicrash_dir_init(u, c, b) (true)
-# define unicrash_xattr_init(u, c, b) (true)
-# define unicrash_fs_label_init(u, c) (true)
+# define unicrash_dir_init(u, c, b) (0)
+# define unicrash_xattr_init(u, c, b) (0)
+# define unicrash_fs_label_init(u, c) (0)
# define unicrash_free(u) do {(u) = (u);} while (0)
-# define unicrash_check_dir_name(u, d, n) (true)
-# define unicrash_check_xattr_name(u, d, n) (true)
-# define unicrash_check_fs_label(u, d, n) (true)
+# define unicrash_check_dir_name(u, d, n) (0)
+# define unicrash_check_xattr_name(u, d, n) (0)
+# define unicrash_check_fs_label(u, d, n) (0)
#endif /* HAVE_LIBICU */
#endif /* XFS_SCRUB_UNICRASH_H_ */
^ permalink raw reply related [flat|nested] 25+ messages in thread
* [PATCH 07/18] xfs_scrub: remove moveon from progress report helpers
2019-10-22 18:50 [PATCH 00/18] xfs_scrub: remove moveon space aliens Darrick J. Wong
` (5 preceding siblings ...)
2019-10-22 18:51 ` [PATCH 06/18] xfs_scrub: remove moveon from unicode name collision helpers Darrick J. Wong
@ 2019-10-22 18:51 ` Darrick J. Wong
2019-10-22 18:51 ` [PATCH 08/18] xfs_scrub: remove moveon from scrub ioctl wrappers Darrick J. Wong
` (11 subsequent siblings)
18 siblings, 0 replies; 25+ messages in thread
From: Darrick J. Wong @ 2019-10-22 18:51 UTC (permalink / raw)
To: sandeen, darrick.wong; +Cc: linux-xfs
From: Darrick J. Wong <darrick.wong@oracle.com>
Replace the moveon returns in the scrub process reporting helpers
with a direct integer error return.
Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
---
scrub/progress.c | 13 ++++++++-----
scrub/progress.h | 2 +-
scrub/xfs_scrub.c | 13 +++++++++----
3 files changed, 18 insertions(+), 10 deletions(-)
diff --git a/scrub/progress.c b/scrub/progress.c
index e93b607f..d8130ca5 100644
--- a/scrub/progress.c
+++ b/scrub/progress.c
@@ -167,8 +167,11 @@ progress_end_phase(void)
pt.fp = NULL;
}
-/* Set ourselves up to report progress. */
-bool
+/*
+ * Set ourselves up to report progress. If errors are encountered, this
+ * function will log them and return nonzero.
+ */
+int
progress_init_phase(
struct scrub_ctx *ctx,
FILE *fp,
@@ -182,7 +185,7 @@ progress_init_phase(
assert(pt.fp == NULL);
if (fp == NULL || max == 0) {
pt.fp = NULL;
- return true;
+ return 0;
}
pt.fp = fp;
pt.isatty = isatty(fileno(fp));
@@ -205,7 +208,7 @@ progress_init_phase(
goto out_ptcounter;
}
- return true;
+ return 0;
out_ptcounter:
ptcounter_free(pt.ptc);
@@ -213,5 +216,5 @@ progress_init_phase(
out_max:
pt.max = 0;
pt.fp = NULL;
- return false;
+ return ret;
}
diff --git a/scrub/progress.h b/scrub/progress.h
index 9144770e..c1a115cb 100644
--- a/scrub/progress.h
+++ b/scrub/progress.h
@@ -10,7 +10,7 @@
#define START_IGNORE '\001'
#define END_IGNORE '\002'
-bool progress_init_phase(struct scrub_ctx *ctx, FILE *progress_fp,
+int progress_init_phase(struct scrub_ctx *ctx, FILE *progress_fp,
unsigned int phase, uint64_t max, int rshift,
unsigned int nr_threads);
void progress_end_phase(void);
diff --git a/scrub/xfs_scrub.c b/scrub/xfs_scrub.c
index 839528ea..c0e60b92 100644
--- a/scrub/xfs_scrub.c
+++ b/scrub/xfs_scrub.c
@@ -423,6 +423,7 @@ run_scrub_phases(
unsigned int debug_phase = 0;
unsigned int phase;
int rshift;
+ int ret;
if (debug_tweak_on("XFS_SCRUB_PHASE"))
debug_phase = atoi(getenv("XFS_SCRUB_PHASE"));
@@ -468,15 +469,19 @@ run_scrub_phases(
* whatever other per-thread data we need to allocate.
*/
work_threads++;
- moveon = progress_init_phase(ctx, progress_fp, phase,
+ ret = progress_init_phase(ctx, progress_fp, phase,
max_work, rshift, work_threads);
- if (!moveon)
+ if (ret) {
+ moveon = false;
break;
+ }
moveon = descr_init_phase(ctx, work_threads) == 0;
} else {
- moveon = progress_init_phase(ctx, NULL, phase, 0, 0, 0);
- if (!moveon)
+ ret = progress_init_phase(ctx, NULL, phase, 0, 0, 0);
+ if (ret) {
+ moveon = false;
break;
+ }
moveon = descr_init_phase(ctx, 1) == 0;
}
if (!moveon)
^ permalink raw reply related [flat|nested] 25+ messages in thread
* [PATCH 08/18] xfs_scrub: remove moveon from scrub ioctl wrappers
2019-10-22 18:50 [PATCH 00/18] xfs_scrub: remove moveon space aliens Darrick J. Wong
` (6 preceding siblings ...)
2019-10-22 18:51 ` [PATCH 07/18] xfs_scrub: remove moveon from progress report helpers Darrick J. Wong
@ 2019-10-22 18:51 ` Darrick J. Wong
2019-10-22 18:51 ` [PATCH 09/18] xfs_scrub: remove moveon from repair action list helpers Darrick J. Wong
` (10 subsequent siblings)
18 siblings, 0 replies; 25+ messages in thread
From: Darrick J. Wong @ 2019-10-22 18:51 UTC (permalink / raw)
To: sandeen, darrick.wong; +Cc: linux-xfs
From: Darrick J. Wong <darrick.wong@oracle.com>
Replace the moveon returns in the scrub ioctl wrapper functions
with a direct integer error return.
Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
---
scrub/phase2.c | 23 ++++++++-------
scrub/phase3.c | 38 +++++++++++++-----------
scrub/phase4.c | 8 +++--
scrub/phase5.c | 4 +--
scrub/phase7.c | 4 +--
scrub/scrub.c | 84 ++++++++++++++++++++++++++++++-----------------------
scrub/scrub.h | 26 ++++++++--------
scrub/xfs_scrub.h | 2 +
8 files changed, 102 insertions(+), 87 deletions(-)
diff --git a/scrub/phase2.c b/scrub/phase2.c
index d92b7e29..016d8ec5 100644
--- a/scrub/phase2.c
+++ b/scrub/phase2.c
@@ -32,6 +32,7 @@ xfs_scan_ag_metadata(
unsigned long long broken_secondaries;
bool moveon;
char descr[DESCR_BUFSZ];
+ int ret;
xfs_action_list_init(&alist);
xfs_action_list_init(&immediate_alist);
@@ -41,8 +42,8 @@ xfs_scan_ag_metadata(
* First we scrub and fix the AG headers, because we need
* them to work well enough to check the AG btrees.
*/
- moveon = xfs_scrub_ag_headers(ctx, agno, &alist);
- if (!moveon)
+ ret = xfs_scrub_ag_headers(ctx, agno, &alist);
+ if (ret)
goto err;
/* Repair header damage. */
@@ -51,8 +52,8 @@ xfs_scan_ag_metadata(
goto err;
/* Now scrub the AG btrees. */
- moveon = xfs_scrub_ag_metadata(ctx, agno, &alist);
- if (!moveon)
+ ret = xfs_scrub_ag_metadata(ctx, agno, &alist);
+ if (ret)
goto err;
/*
@@ -100,11 +101,11 @@ xfs_scan_fs_metadata(
struct scrub_ctx *ctx = (struct scrub_ctx *)wq->wq_ctx;
bool *pmoveon = arg;
struct xfs_action_list alist;
- bool moveon;
+ int ret;
xfs_action_list_init(&alist);
- moveon = xfs_scrub_fs_metadata(ctx, &alist);
- if (!moveon)
+ ret = xfs_scrub_fs_metadata(ctx, &alist);
+ if (ret)
*pmoveon = false;
xfs_action_list_defer(ctx, agno, &alist);
@@ -134,9 +135,11 @@ xfs_scan_metadata(
* anything else.
*/
xfs_action_list_init(&alist);
- moveon = xfs_scrub_primary_super(ctx, &alist);
- if (!moveon)
+ ret = xfs_scrub_primary_super(ctx, &alist);
+ if (ret) {
+ moveon = false;
goto out;
+ }
moveon = xfs_action_list_process_or_defer(ctx, 0, &alist);
if (!moveon)
goto out;
@@ -178,7 +181,7 @@ xfs_estimate_metadata_work(
unsigned int *nr_threads,
int *rshift)
{
- *items = xfs_scrub_estimate_ag_work(ctx);
+ *items = scrub_estimate_ag_work(ctx);
*nr_threads = scrub_nproc(ctx);
*rshift = 0;
return true;
diff --git a/scrub/phase3.c b/scrub/phase3.c
index 171be3fd..4248e85f 100644
--- a/scrub/phase3.c
+++ b/scrub/phase3.c
@@ -25,10 +25,10 @@
* ensure that the inode we're checking matches what the inode scan
* told us to look at.
*/
-static bool
-xfs_scrub_fd(
+static int
+scrub_fd(
struct scrub_ctx *ctx,
- bool (*fn)(struct scrub_ctx *ctx, uint64_t ino,
+ int (*fn)(struct scrub_ctx *ctx, uint64_t ino,
uint32_t gen, struct xfs_action_list *a),
struct xfs_bulkstat *bs,
struct xfs_action_list *alist)
@@ -85,8 +85,8 @@ xfs_scrub_inode(
}
/* Scrub the inode. */
- moveon = xfs_scrub_fd(ctx, xfs_scrub_inode_fields, bstat, &alist);
- if (!moveon)
+ error = scrub_fd(ctx, xfs_scrub_inode_fields, bstat, &alist);
+ if (error)
goto out;
moveon = xfs_action_list_process_or_defer(ctx, agno, &alist);
@@ -94,14 +94,14 @@ xfs_scrub_inode(
goto out;
/* Scrub all block mappings. */
- moveon = xfs_scrub_fd(ctx, xfs_scrub_data_fork, bstat, &alist);
- if (!moveon)
+ error = scrub_fd(ctx, xfs_scrub_data_fork, bstat, &alist);
+ if (error)
goto out;
- moveon = xfs_scrub_fd(ctx, xfs_scrub_attr_fork, bstat, &alist);
- if (!moveon)
+ error = scrub_fd(ctx, xfs_scrub_attr_fork, bstat, &alist);
+ if (error)
goto out;
- moveon = xfs_scrub_fd(ctx, xfs_scrub_cow_fork, bstat, &alist);
- if (!moveon)
+ error = scrub_fd(ctx, xfs_scrub_cow_fork, bstat, &alist);
+ if (error)
goto out;
moveon = xfs_action_list_process_or_defer(ctx, agno, &alist);
@@ -110,23 +110,23 @@ xfs_scrub_inode(
if (S_ISLNK(bstat->bs_mode)) {
/* Check symlink contents. */
- moveon = xfs_scrub_symlink(ctx, bstat->bs_ino, bstat->bs_gen,
+ error = xfs_scrub_symlink(ctx, bstat->bs_ino, bstat->bs_gen,
&alist);
} else if (S_ISDIR(bstat->bs_mode)) {
/* Check the directory entries. */
- moveon = xfs_scrub_fd(ctx, xfs_scrub_dir, bstat, &alist);
+ error = scrub_fd(ctx, xfs_scrub_dir, bstat, &alist);
}
- if (!moveon)
+ if (error)
goto out;
/* Check all the extended attributes. */
- moveon = xfs_scrub_fd(ctx, xfs_scrub_attr, bstat, &alist);
- if (!moveon)
+ error = scrub_fd(ctx, xfs_scrub_attr, bstat, &alist);
+ if (error)
goto out;
/* Check parent pointers. */
- moveon = xfs_scrub_fd(ctx, xfs_scrub_parent, bstat, &alist);
- if (!moveon)
+ error = scrub_fd(ctx, xfs_scrub_parent, bstat, &alist);
+ if (error)
goto out;
/* Try to repair the file while it's open. */
@@ -135,6 +135,8 @@ xfs_scrub_inode(
goto out;
out:
+ if (error)
+ moveon = false;
error = ptcounter_add(icount, 1);
if (error) {
str_liberror(ctx, error,
diff --git a/scrub/phase4.c b/scrub/phase4.c
index a276bc32..a121df02 100644
--- a/scrub/phase4.c
+++ b/scrub/phase4.c
@@ -114,9 +114,9 @@ xfs_process_action_items(
/* Fix everything that needs fixing. */
bool
xfs_repair_fs(
- struct scrub_ctx *ctx)
+ struct scrub_ctx *ctx)
{
- bool moveon;
+ int ret;
/*
* Check the summary counters early. Normally we do this during phase
@@ -124,8 +124,8 @@ xfs_repair_fs(
* counters, so counter repairs have to be put on the list now so that
* they get fixed before we stop retrying unfixed metadata repairs.
*/
- moveon = xfs_scrub_fs_summary(ctx, &ctx->action_lists[0]);
- if (!moveon)
+ ret = xfs_scrub_fs_summary(ctx, &ctx->action_lists[0]);
+ if (ret)
return false;
return xfs_process_action_items(ctx);
diff --git a/scrub/phase5.c b/scrub/phase5.c
index e752a0c4..c2cccd76 100644
--- a/scrub/phase5.c
+++ b/scrub/phase5.c
@@ -390,8 +390,8 @@ _("Filesystem has errors, skipping connectivity checks."));
return true;
}
- moveon = xfs_scrub_fs_label(ctx);
- if (!moveon)
+ ret = xfs_scrub_fs_label(ctx);
+ if (ret)
return false;
ret = scrub_scan_all_inodes(ctx, xfs_scrub_connections, &moveon);
diff --git a/scrub/phase7.c b/scrub/phase7.c
index 4314904f..b5f77b36 100644
--- a/scrub/phase7.c
+++ b/scrub/phase7.c
@@ -121,8 +121,8 @@ xfs_scan_summary(
/* Check and fix the fs summary counters. */
xfs_action_list_init(&alist);
- moveon = xfs_scrub_fs_summary(ctx, &alist);
- if (!moveon)
+ error = xfs_scrub_fs_summary(ctx, &alist);
+ if (error)
return false;
moveon = xfs_action_list_process(ctx, ctx->mnt.fd, &alist,
ALP_COMPLAIN_IF_UNFIXED | ALP_NOPROGRESS);
diff --git a/scrub/scrub.c b/scrub/scrub.c
index d9df1e5b..2bda5054 100644
--- a/scrub/scrub.c
+++ b/scrub/scrub.c
@@ -249,7 +249,7 @@ _("Optimizations of %s are possible."), _(xfrog_scrubbers[i].descr));
}
/* Save a scrub context for later repairs. */
-static bool
+static int
xfs_scrub_save_repair(
struct scrub_ctx *ctx,
struct xfs_action_list *alist,
@@ -260,9 +260,10 @@ xfs_scrub_save_repair(
/* Schedule this item for later repairs. */
aitem = malloc(sizeof(struct action_item));
if (!aitem) {
- str_errno(ctx, _("repair list"));
- return false;
+ str_errno(ctx, _("adding item to repair list"));
+ return errno;
}
+
memset(aitem, 0, sizeof(*aitem));
aitem->type = meta->sm_type;
aitem->flags = meta->sm_flags;
@@ -280,10 +281,15 @@ xfs_scrub_save_repair(
}
xfs_action_list_add(alist, aitem);
- return true;
+ return 0;
}
-/* Scrub a single XFS_SCRUB_TYPE_*, saving corruption reports for later. */
+/*
+ * Scrub a single XFS_SCRUB_TYPE_*, saving corruption reports for later.
+ *
+ * Returns 0 for success. If errors occur, this function will log them and
+ * return a positive error code.
+ */
static int
xfs_scrub_meta_type(
struct scrub_ctx *ctx,
@@ -296,6 +302,7 @@ xfs_scrub_meta_type(
.sm_agno = agno,
};
enum check_outcome fix;
+ int ret;
background_sleep();
@@ -307,8 +314,9 @@ xfs_scrub_meta_type(
case CHECK_ABORT:
return ECANCELED;
case CHECK_REPAIR:
- if (!xfs_scrub_save_repair(ctx, alist, &meta))
- return ENOMEM;
+ ret = xfs_scrub_save_repair(ctx, alist, &meta);
+ if (ret)
+ return ret;
/* fall through */
case CHECK_DONE:
return 0;
@@ -344,30 +352,28 @@ xfs_scrub_all_types(
ret = xfs_scrub_meta_type(ctx, type, agno, alist);
if (ret)
- return false;
+ return ret;
}
- return true;
+ return 0;
}
/*
* Scrub primary superblock. This will be useful if we ever need to hook
* a filesystem-wide pre-scrub activity off of the sb 0 scrubber (which
- * currently does nothing).
+ * currently does nothing). If errors occur, this function will log them and
+ * return nonzero.
*/
-bool
+int
xfs_scrub_primary_super(
struct scrub_ctx *ctx,
struct xfs_action_list *alist)
{
- int ret;
-
- ret = xfs_scrub_meta_type(ctx, XFS_SCRUB_TYPE_SB, 0, alist);
- return ret == 0;
+ return xfs_scrub_meta_type(ctx, XFS_SCRUB_TYPE_SB, 0, alist);
}
/* Scrub each AG's header blocks. */
-bool
+int
xfs_scrub_ag_headers(
struct scrub_ctx *ctx,
xfs_agnumber_t agno,
@@ -377,7 +383,7 @@ xfs_scrub_ag_headers(
}
/* Scrub each AG's metadata btrees. */
-bool
+int
xfs_scrub_ag_metadata(
struct scrub_ctx *ctx,
xfs_agnumber_t agno,
@@ -387,7 +393,7 @@ xfs_scrub_ag_metadata(
}
/* Scrub whole-FS metadata btrees. */
-bool
+int
xfs_scrub_fs_metadata(
struct scrub_ctx *ctx,
struct xfs_action_list *alist)
@@ -396,20 +402,17 @@ xfs_scrub_fs_metadata(
}
/* Scrub FS summary metadata. */
-bool
+int
xfs_scrub_fs_summary(
struct scrub_ctx *ctx,
struct xfs_action_list *alist)
{
- int ret;
-
- ret = xfs_scrub_meta_type(ctx, XFS_SCRUB_TYPE_FSCOUNTERS, 0, alist);
- return ret == 0;
+ return xfs_scrub_meta_type(ctx, XFS_SCRUB_TYPE_FSCOUNTERS, 0, alist);
}
/* How many items do we have to check? */
unsigned int
-xfs_scrub_estimate_ag_work(
+scrub_estimate_ag_work(
struct scrub_ctx *ctx)
{
const struct xfrog_scrub_descr *sc;
@@ -433,8 +436,11 @@ xfs_scrub_estimate_ag_work(
return estimate;
}
-/* Scrub inode metadata. */
-static bool
+/*
+ * Scrub inode metadata. If errors occur, this function will log them and
+ * return nonzero.
+ */
+static int
__xfs_scrub_file(
struct scrub_ctx *ctx,
uint64_t ino,
@@ -455,14 +461,14 @@ __xfs_scrub_file(
/* Scrub the piece of metadata. */
fix = xfs_check_metadata(ctx, &meta, true);
if (fix == CHECK_ABORT)
- return false;
+ return ECANCELED;
if (fix == CHECK_DONE)
- return true;
+ return 0;
return xfs_scrub_save_repair(ctx, alist, &meta);
}
-bool
+int
xfs_scrub_inode_fields(
struct scrub_ctx *ctx,
uint64_t ino,
@@ -472,7 +478,7 @@ xfs_scrub_inode_fields(
return __xfs_scrub_file(ctx, ino, gen, XFS_SCRUB_TYPE_INODE, alist);
}
-bool
+int
xfs_scrub_data_fork(
struct scrub_ctx *ctx,
uint64_t ino,
@@ -482,7 +488,7 @@ xfs_scrub_data_fork(
return __xfs_scrub_file(ctx, ino, gen, XFS_SCRUB_TYPE_BMBTD, alist);
}
-bool
+int
xfs_scrub_attr_fork(
struct scrub_ctx *ctx,
uint64_t ino,
@@ -492,7 +498,7 @@ xfs_scrub_attr_fork(
return __xfs_scrub_file(ctx, ino, gen, XFS_SCRUB_TYPE_BMBTA, alist);
}
-bool
+int
xfs_scrub_cow_fork(
struct scrub_ctx *ctx,
uint64_t ino,
@@ -502,7 +508,7 @@ xfs_scrub_cow_fork(
return __xfs_scrub_file(ctx, ino, gen, XFS_SCRUB_TYPE_BMBTC, alist);
}
-bool
+int
xfs_scrub_dir(
struct scrub_ctx *ctx,
uint64_t ino,
@@ -512,7 +518,7 @@ xfs_scrub_dir(
return __xfs_scrub_file(ctx, ino, gen, XFS_SCRUB_TYPE_DIR, alist);
}
-bool
+int
xfs_scrub_attr(
struct scrub_ctx *ctx,
uint64_t ino,
@@ -522,7 +528,7 @@ xfs_scrub_attr(
return __xfs_scrub_file(ctx, ino, gen, XFS_SCRUB_TYPE_XATTR, alist);
}
-bool
+int
xfs_scrub_symlink(
struct scrub_ctx *ctx,
uint64_t ino,
@@ -532,7 +538,7 @@ xfs_scrub_symlink(
return __xfs_scrub_file(ctx, ino, gen, XFS_SCRUB_TYPE_SYMLINK, alist);
}
-bool
+int
xfs_scrub_parent(
struct scrub_ctx *ctx,
uint64_t ino,
@@ -542,7 +548,11 @@ xfs_scrub_parent(
return __xfs_scrub_file(ctx, ino, gen, XFS_SCRUB_TYPE_PARENT, alist);
}
-/* Test the availability of a kernel scrub command. */
+/*
+ * Test the availability of a kernel scrub command. If errors occur (or the
+ * scrub ioctl is rejected) the errors will be logged and this function will
+ * return false.
+ */
static bool
__xfs_scrub_test(
struct scrub_ctx *ctx,
diff --git a/scrub/scrub.h b/scrub/scrub.h
index d407abb0..bfb3f8e3 100644
--- a/scrub/scrub.h
+++ b/scrub/scrub.h
@@ -17,15 +17,15 @@ enum check_outcome {
struct action_item;
void xfs_scrub_report_preen_triggers(struct scrub_ctx *ctx);
-bool xfs_scrub_primary_super(struct scrub_ctx *ctx,
+int xfs_scrub_primary_super(struct scrub_ctx *ctx,
struct xfs_action_list *alist);
-bool xfs_scrub_ag_headers(struct scrub_ctx *ctx, xfs_agnumber_t agno,
+int xfs_scrub_ag_headers(struct scrub_ctx *ctx, xfs_agnumber_t agno,
struct xfs_action_list *alist);
-bool xfs_scrub_ag_metadata(struct scrub_ctx *ctx, xfs_agnumber_t agno,
+int xfs_scrub_ag_metadata(struct scrub_ctx *ctx, xfs_agnumber_t agno,
struct xfs_action_list *alist);
-bool xfs_scrub_fs_metadata(struct scrub_ctx *ctx,
+int xfs_scrub_fs_metadata(struct scrub_ctx *ctx,
struct xfs_action_list *alist);
-bool xfs_scrub_fs_summary(struct scrub_ctx *ctx,
+int xfs_scrub_fs_summary(struct scrub_ctx *ctx,
struct xfs_action_list *alist);
bool xfs_can_scrub_fs_metadata(struct scrub_ctx *ctx);
@@ -37,21 +37,21 @@ bool xfs_can_scrub_symlink(struct scrub_ctx *ctx);
bool xfs_can_scrub_parent(struct scrub_ctx *ctx);
bool xfs_can_repair(struct scrub_ctx *ctx);
-bool xfs_scrub_inode_fields(struct scrub_ctx *ctx, uint64_t ino, uint32_t gen,
+int xfs_scrub_inode_fields(struct scrub_ctx *ctx, uint64_t ino, uint32_t gen,
struct xfs_action_list *alist);
-bool xfs_scrub_data_fork(struct scrub_ctx *ctx, uint64_t ino, uint32_t gen,
+int xfs_scrub_data_fork(struct scrub_ctx *ctx, uint64_t ino, uint32_t gen,
struct xfs_action_list *alist);
-bool xfs_scrub_attr_fork(struct scrub_ctx *ctx, uint64_t ino, uint32_t gen,
+int xfs_scrub_attr_fork(struct scrub_ctx *ctx, uint64_t ino, uint32_t gen,
struct xfs_action_list *alist);
-bool xfs_scrub_cow_fork(struct scrub_ctx *ctx, uint64_t ino, uint32_t gen,
+int xfs_scrub_cow_fork(struct scrub_ctx *ctx, uint64_t ino, uint32_t gen,
struct xfs_action_list *alist);
-bool xfs_scrub_dir(struct scrub_ctx *ctx, uint64_t ino, uint32_t gen,
+int xfs_scrub_dir(struct scrub_ctx *ctx, uint64_t ino, uint32_t gen,
struct xfs_action_list *alist);
-bool xfs_scrub_attr(struct scrub_ctx *ctx, uint64_t ino, uint32_t gen,
+int xfs_scrub_attr(struct scrub_ctx *ctx, uint64_t ino, uint32_t gen,
struct xfs_action_list *alist);
-bool xfs_scrub_symlink(struct scrub_ctx *ctx, uint64_t ino, uint32_t gen,
+int xfs_scrub_symlink(struct scrub_ctx *ctx, uint64_t ino, uint32_t gen,
struct xfs_action_list *alist);
-bool xfs_scrub_parent(struct scrub_ctx *ctx, uint64_t ino, uint32_t gen,
+int xfs_scrub_parent(struct scrub_ctx *ctx, uint64_t ino, uint32_t gen,
struct xfs_action_list *alist);
/* Repair parameters are the scrub inputs and retry count. */
diff --git a/scrub/xfs_scrub.h b/scrub/xfs_scrub.h
index ed0e63b8..d8032e3d 100644
--- a/scrub/xfs_scrub.h
+++ b/scrub/xfs_scrub.h
@@ -99,7 +99,7 @@ bool xfs_repair_fs(struct scrub_ctx *ctx);
/* Progress estimator functions */
uint64_t xfs_estimate_inodes(struct scrub_ctx *ctx);
-unsigned int xfs_scrub_estimate_ag_work(struct scrub_ctx *ctx);
+unsigned int scrub_estimate_ag_work(struct scrub_ctx *ctx);
bool xfs_estimate_metadata_work(struct scrub_ctx *ctx, uint64_t *items,
unsigned int *nr_threads, int *rshift);
bool xfs_estimate_inodes_work(struct scrub_ctx *ctx, uint64_t *items,
^ permalink raw reply related [flat|nested] 25+ messages in thread
* [PATCH 09/18] xfs_scrub: remove moveon from repair action list helpers
2019-10-22 18:50 [PATCH 00/18] xfs_scrub: remove moveon space aliens Darrick J. Wong
` (7 preceding siblings ...)
2019-10-22 18:51 ` [PATCH 08/18] xfs_scrub: remove moveon from scrub ioctl wrappers Darrick J. Wong
@ 2019-10-22 18:51 ` Darrick J. Wong
2019-10-22 18:51 ` [PATCH 10/18] xfs_scrub: remove moveon from phase 7 functions Darrick J. Wong
` (9 subsequent siblings)
18 siblings, 0 replies; 25+ messages in thread
From: Darrick J. Wong @ 2019-10-22 18:51 UTC (permalink / raw)
To: sandeen, darrick.wong; +Cc: linux-xfs
From: Darrick J. Wong <darrick.wong@oracle.com>
Replace the moveon returns in the repair action list processing
functions with a direct integer error return.
Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
---
scrub/phase1.c | 9 +++---
scrub/phase2.c | 37 ++++++++++++-----------
scrub/phase3.c | 22 +++++++-------
scrub/phase4.c | 20 ++++++------
scrub/phase7.c | 10 +++---
scrub/repair.c | 85 ++++++++++++++++++++++++++++-------------------------
scrub/repair.h | 32 +++++++++-----------
scrub/scrub.c | 36 +++++++++++-----------
scrub/scrub.h | 29 ++++++++----------
scrub/xfs_scrub.h | 2 +
10 files changed, 142 insertions(+), 140 deletions(-)
diff --git a/scrub/phase1.c b/scrub/phase1.c
index 0ae368ff..5538deae 100644
--- a/scrub/phase1.c
+++ b/scrub/phase1.c
@@ -50,7 +50,7 @@ xfs_cleanup_fs(
{
int error;
- xfs_action_lists_free(&ctx->action_lists);
+ action_lists_free(&ctx->action_lists);
if (ctx->fshandle)
free_handle(ctx->fshandle, ctx->fshandle_len);
if (ctx->rtdev)
@@ -125,9 +125,10 @@ _("Not an XFS filesystem."));
return false;
}
- if (!xfs_action_lists_alloc(ctx->mnt.fsgeom.agcount,
- &ctx->action_lists)) {
- str_liberror(ctx, ENOMEM, _("allocating action lists"));
+ error = action_lists_alloc(ctx->mnt.fsgeom.agcount,
+ &ctx->action_lists);
+ if (error) {
+ str_liberror(ctx, error, _("allocating action lists"));
return false;
}
diff --git a/scrub/phase2.c b/scrub/phase2.c
index 016d8ec5..7388b8e2 100644
--- a/scrub/phase2.c
+++ b/scrub/phase2.c
@@ -26,16 +26,15 @@ xfs_scan_ag_metadata(
{
struct scrub_ctx *ctx = (struct scrub_ctx *)wq->wq_ctx;
bool *pmoveon = arg;
- struct xfs_action_list alist;
- struct xfs_action_list immediate_alist;
+ struct action_list alist;
+ struct action_list immediate_alist;
unsigned long long broken_primaries;
unsigned long long broken_secondaries;
- bool moveon;
char descr[DESCR_BUFSZ];
int ret;
- xfs_action_list_init(&alist);
- xfs_action_list_init(&immediate_alist);
+ action_list_init(&alist);
+ action_list_init(&immediate_alist);
snprintf(descr, DESCR_BUFSZ, _("AG %u"), agno);
/*
@@ -47,8 +46,8 @@ xfs_scan_ag_metadata(
goto err;
/* Repair header damage. */
- moveon = xfs_action_list_process_or_defer(ctx, agno, &alist);
- if (!moveon)
+ ret = action_list_process_or_defer(ctx, agno, &alist);
+ if (ret)
goto err;
/* Now scrub the AG btrees. */
@@ -65,7 +64,7 @@ xfs_scan_ag_metadata(
*/
broken_secondaries = 0;
broken_primaries = 0;
- xfs_action_list_find_mustfix(&alist, &immediate_alist,
+ action_list_find_mustfix(&alist, &immediate_alist,
&broken_primaries, &broken_secondaries);
if (broken_secondaries && !debug_tweak_on("XFS_SCRUB_FORCE_REPAIR")) {
if (broken_primaries)
@@ -79,12 +78,12 @@ _("Filesystem might not be repairable."));
}
/* Repair (inode) btree damage. */
- moveon = xfs_action_list_process_or_defer(ctx, agno, &immediate_alist);
- if (!moveon)
+ ret = action_list_process_or_defer(ctx, agno, &immediate_alist);
+ if (ret)
goto err;
/* Everything else gets fixed during phase 4. */
- xfs_action_list_defer(ctx, agno, &alist);
+ action_list_defer(ctx, agno, &alist);
return;
err:
@@ -100,15 +99,15 @@ xfs_scan_fs_metadata(
{
struct scrub_ctx *ctx = (struct scrub_ctx *)wq->wq_ctx;
bool *pmoveon = arg;
- struct xfs_action_list alist;
+ struct action_list alist;
int ret;
- xfs_action_list_init(&alist);
+ action_list_init(&alist);
ret = xfs_scrub_fs_metadata(ctx, &alist);
if (ret)
*pmoveon = false;
- xfs_action_list_defer(ctx, agno, &alist);
+ action_list_defer(ctx, agno, &alist);
}
/* Scan all filesystem metadata. */
@@ -116,7 +115,7 @@ bool
xfs_scan_metadata(
struct scrub_ctx *ctx)
{
- struct xfs_action_list alist;
+ struct action_list alist;
struct workqueue wq;
xfs_agnumber_t agno;
bool moveon = true;
@@ -134,15 +133,17 @@ xfs_scan_metadata(
* upgrades (followed by a full scrub), do that before we launch
* anything else.
*/
- xfs_action_list_init(&alist);
+ action_list_init(&alist);
ret = xfs_scrub_primary_super(ctx, &alist);
if (ret) {
moveon = false;
goto out;
}
- moveon = xfs_action_list_process_or_defer(ctx, 0, &alist);
- if (!moveon)
+ ret = action_list_process_or_defer(ctx, 0, &alist);
+ if (ret) {
+ moveon = false;
goto out;
+ }
for (agno = 0; moveon && agno < ctx->mnt.fsgeom.agcount; agno++) {
ret = workqueue_add(&wq, xfs_scan_ag_metadata, agno, &moveon);
diff --git a/scrub/phase3.c b/scrub/phase3.c
index 4248e85f..b35ef164 100644
--- a/scrub/phase3.c
+++ b/scrub/phase3.c
@@ -29,9 +29,9 @@ static int
scrub_fd(
struct scrub_ctx *ctx,
int (*fn)(struct scrub_ctx *ctx, uint64_t ino,
- uint32_t gen, struct xfs_action_list *a),
+ uint32_t gen, struct action_list *a),
struct xfs_bulkstat *bs,
- struct xfs_action_list *alist)
+ struct action_list *alist)
{
return fn(ctx, bs->bs_ino, bs->bs_gen, alist);
}
@@ -64,7 +64,7 @@ xfs_scrub_inode(
struct xfs_bulkstat *bstat,
void *arg)
{
- struct xfs_action_list alist;
+ struct action_list alist;
struct scrub_inode_ctx *ictx = arg;
struct ptcounter *icount = ictx->icount;
xfs_agnumber_t agno;
@@ -72,7 +72,7 @@ xfs_scrub_inode(
int fd = -1;
int error;
- xfs_action_list_init(&alist);
+ action_list_init(&alist);
agno = cvt_ino_to_agno(&ctx->mnt, bstat->bs_ino);
background_sleep();
@@ -89,8 +89,8 @@ xfs_scrub_inode(
if (error)
goto out;
- moveon = xfs_action_list_process_or_defer(ctx, agno, &alist);
- if (!moveon)
+ error = action_list_process_or_defer(ctx, agno, &alist);
+ if (error)
goto out;
/* Scrub all block mappings. */
@@ -104,8 +104,8 @@ xfs_scrub_inode(
if (error)
goto out;
- moveon = xfs_action_list_process_or_defer(ctx, agno, &alist);
- if (!moveon)
+ error = action_list_process_or_defer(ctx, agno, &alist);
+ if (error)
goto out;
if (S_ISLNK(bstat->bs_mode)) {
@@ -130,8 +130,8 @@ xfs_scrub_inode(
goto out;
/* Try to repair the file while it's open. */
- moveon = xfs_action_list_process_or_defer(ctx, agno, &alist);
- if (!moveon)
+ error = action_list_process_or_defer(ctx, agno, &alist);
+ if (error)
goto out;
out:
@@ -144,7 +144,7 @@ xfs_scrub_inode(
return false;
}
progress_add(1);
- xfs_action_list_defer(ctx, agno, &alist);
+ action_list_defer(ctx, agno, &alist);
if (fd >= 0) {
error = close(fd);
if (error)
diff --git a/scrub/phase4.c b/scrub/phase4.c
index a121df02..c6de1cd4 100644
--- a/scrub/phase4.c
+++ b/scrub/phase4.c
@@ -29,23 +29,23 @@ xfs_repair_ag(
{
struct scrub_ctx *ctx = (struct scrub_ctx *)wq->wq_ctx;
bool *pmoveon = priv;
- struct xfs_action_list *alist;
+ struct action_list *alist;
size_t unfixed;
size_t new_unfixed;
unsigned int flags = 0;
- bool moveon;
+ int ret;
alist = &ctx->action_lists[agno];
- unfixed = xfs_action_list_length(alist);
+ unfixed = action_list_length(alist);
/* Repair anything broken until we fail to make progress. */
do {
- moveon = xfs_action_list_process(ctx, ctx->mnt.fd, alist, flags);
- if (!moveon) {
+ ret = action_list_process(ctx, ctx->mnt.fd, alist, flags);
+ if (ret) {
*pmoveon = false;
return;
}
- new_unfixed = xfs_action_list_length(alist);
+ new_unfixed = action_list_length(alist);
if (new_unfixed == unfixed)
break;
unfixed = new_unfixed;
@@ -56,8 +56,8 @@ xfs_repair_ag(
/* Try once more, but this time complain if we can't fix things. */
flags |= ALP_COMPLAIN_IF_UNFIXED;
- moveon = xfs_action_list_process(ctx, ctx->mnt.fd, alist, flags);
- if (!moveon)
+ ret = action_list_process(ctx, ctx->mnt.fd, alist, flags);
+ if (ret)
*pmoveon = false;
}
@@ -78,7 +78,7 @@ xfs_process_action_items(
return false;
}
for (agno = 0; agno < ctx->mnt.fsgeom.agcount; agno++) {
- if (xfs_action_list_length(&ctx->action_lists[agno]) > 0) {
+ if (action_list_length(&ctx->action_lists[agno]) > 0) {
ret = workqueue_add(&wq, xfs_repair_ag, agno, &moveon);
if (ret) {
moveon = false;
@@ -143,7 +143,7 @@ xfs_estimate_repair_work(
size_t need_fixing = 0;
for (agno = 0; agno < ctx->mnt.fsgeom.agcount; agno++)
- need_fixing += xfs_action_list_length(&ctx->action_lists[agno]);
+ need_fixing += action_list_length(&ctx->action_lists[agno]);
need_fixing++;
*items = need_fixing;
*nr_threads = scrub_nproc(ctx) + 1;
diff --git a/scrub/phase7.c b/scrub/phase7.c
index b5f77b36..452d56ad 100644
--- a/scrub/phase7.c
+++ b/scrub/phase7.c
@@ -98,7 +98,7 @@ xfs_scan_summary(
struct scrub_ctx *ctx)
{
struct summary_counts totalcount = {0};
- struct xfs_action_list alist;
+ struct action_list alist;
struct ptvar *ptvar;
unsigned long long used_data;
unsigned long long used_rt;
@@ -120,14 +120,14 @@ xfs_scan_summary(
int error;
/* Check and fix the fs summary counters. */
- xfs_action_list_init(&alist);
+ action_list_init(&alist);
error = xfs_scrub_fs_summary(ctx, &alist);
if (error)
return false;
- moveon = xfs_action_list_process(ctx, ctx->mnt.fd, &alist,
+ error = action_list_process(ctx, ctx->mnt.fd, &alist,
ALP_COMPLAIN_IF_UNFIXED | ALP_NOPROGRESS);
- if (!moveon)
- return moveon;
+ if (error)
+ return false;
/* Flush everything out to disk before we start counting. */
error = syncfs(ctx->mnt.fd);
diff --git a/scrub/repair.c b/scrub/repair.c
index 04a9dccf..1604e252 100644
--- a/scrub/repair.c
+++ b/scrub/repair.c
@@ -112,9 +112,9 @@ xfs_action_item_compare(
* to the inode scan.
*/
void
-xfs_action_list_find_mustfix(
- struct xfs_action_list *alist,
- struct xfs_action_list *immediate_alist,
+action_list_find_mustfix(
+ struct action_list *alist,
+ struct action_list *immediate_alist,
unsigned long long *broken_primaries,
unsigned long long *broken_secondaries)
{
@@ -146,30 +146,33 @@ xfs_action_list_find_mustfix(
}
}
-/* Allocate a certain number of repair lists for the scrub context. */
-bool
-xfs_action_lists_alloc(
+/*
+ * Allocate a certain number of repair lists for the scrub context. Returns
+ * zero or a positive error number.
+ */
+int
+action_lists_alloc(
size_t nr,
- struct xfs_action_list **listsp)
+ struct action_list **listsp)
{
- struct xfs_action_list *lists;
+ struct action_list *lists;
xfs_agnumber_t agno;
- lists = calloc(nr, sizeof(struct xfs_action_list));
+ lists = calloc(nr, sizeof(struct action_list));
if (!lists)
- return false;
+ return errno;
for (agno = 0; agno < nr; agno++)
- xfs_action_list_init(&lists[agno]);
+ action_list_init(&lists[agno]);
*listsp = lists;
- return true;
+ return 0;
}
/* Free the repair lists. */
void
-xfs_action_lists_free(
- struct xfs_action_list **listsp)
+action_lists_free(
+ struct action_list **listsp)
{
free(*listsp);
*listsp = NULL;
@@ -177,8 +180,8 @@ xfs_action_lists_free(
/* Initialize repair list */
void
-xfs_action_list_init(
- struct xfs_action_list *alist)
+action_list_init(
+ struct action_list *alist)
{
INIT_LIST_HEAD(&alist->list);
alist->nr = 0;
@@ -187,16 +190,16 @@ xfs_action_list_init(
/* Number of repairs in this list. */
size_t
-xfs_action_list_length(
- struct xfs_action_list *alist)
+action_list_length(
+ struct action_list *alist)
{
return alist->nr;
};
/* Add to the list of repairs. */
void
-xfs_action_list_add(
- struct xfs_action_list *alist,
+action_list_add(
+ struct action_list *alist,
struct action_item *aitem)
{
list_add_tail(&aitem->list, &alist->list);
@@ -206,9 +209,9 @@ xfs_action_list_add(
/* Splice two repair lists. */
void
-xfs_action_list_splice(
- struct xfs_action_list *dest,
- struct xfs_action_list *src)
+action_list_splice(
+ struct action_list *dest,
+ struct action_list *src)
{
if (src->nr == 0)
return;
@@ -220,11 +223,11 @@ xfs_action_list_splice(
}
/* Repair everything on this list. */
-bool
-xfs_action_list_process(
+int
+action_list_process(
struct scrub_ctx *ctx,
int fd,
- struct xfs_action_list *alist,
+ struct action_list *alist,
unsigned int repair_flags)
{
struct action_item *aitem;
@@ -247,7 +250,7 @@ xfs_action_list_process(
free(aitem);
continue;
case CHECK_ABORT:
- return false;
+ return ECANCELED;
case CHECK_RETRY:
continue;
case CHECK_REPAIR:
@@ -255,35 +258,37 @@ xfs_action_list_process(
}
}
- return !xfs_scrub_excessive_errors(ctx);
+ if (xfs_scrub_excessive_errors(ctx))
+ return ECANCELED;
+ return 0;
}
/* Defer all the repairs until phase 4. */
void
-xfs_action_list_defer(
+action_list_defer(
struct scrub_ctx *ctx,
xfs_agnumber_t agno,
- struct xfs_action_list *alist)
+ struct action_list *alist)
{
ASSERT(agno < ctx->mnt.fsgeom.agcount);
- xfs_action_list_splice(&ctx->action_lists[agno], alist);
+ action_list_splice(&ctx->action_lists[agno], alist);
}
/* Run actions now and defer unfinished items for later. */
-bool
-xfs_action_list_process_or_defer(
+int
+action_list_process_or_defer(
struct scrub_ctx *ctx,
xfs_agnumber_t agno,
- struct xfs_action_list *alist)
+ struct action_list *alist)
{
- bool moveon;
+ int ret;
- moveon = xfs_action_list_process(ctx, ctx->mnt.fd, alist,
+ ret = action_list_process(ctx, ctx->mnt.fd, alist,
ALP_REPAIR_ONLY | ALP_NOPROGRESS);
- if (!moveon)
- return moveon;
+ if (ret)
+ return ret;
- xfs_action_list_defer(ctx, agno, alist);
- return true;
+ action_list_defer(ctx, agno, alist);
+ return 0;
}
diff --git a/scrub/repair.h b/scrub/repair.h
index c8693ccf..1994c50a 100644
--- a/scrub/repair.h
+++ b/scrub/repair.h
@@ -6,24 +6,22 @@
#ifndef XFS_SCRUB_REPAIR_H_
#define XFS_SCRUB_REPAIR_H_
-struct xfs_action_list {
+struct action_list {
struct list_head list;
size_t nr;
bool sorted;
};
-bool xfs_action_lists_alloc(size_t nr, struct xfs_action_list **listsp);
-void xfs_action_lists_free(struct xfs_action_list **listsp);
+int action_lists_alloc(size_t nr, struct action_list **listsp);
+void action_lists_free(struct action_list **listsp);
-void xfs_action_list_init(struct xfs_action_list *alist);
-size_t xfs_action_list_length(struct xfs_action_list *alist);
-void xfs_action_list_add(struct xfs_action_list *dest,
- struct action_item *item);
-void xfs_action_list_splice(struct xfs_action_list *dest,
- struct xfs_action_list *src);
+void action_list_init(struct action_list *alist);
+size_t action_list_length(struct action_list *alist);
+void action_list_add(struct action_list *dest, struct action_item *item);
+void action_list_splice(struct action_list *dest, struct action_list *src);
-void xfs_action_list_find_mustfix(struct xfs_action_list *actions,
- struct xfs_action_list *immediate_alist,
+void action_list_find_mustfix(struct action_list *actions,
+ struct action_list *immediate_alist,
unsigned long long *broken_primaries,
unsigned long long *broken_secondaries);
@@ -32,11 +30,11 @@ void xfs_action_list_find_mustfix(struct xfs_action_list *actions,
#define ALP_COMPLAIN_IF_UNFIXED (XRM_COMPLAIN_IF_UNFIXED)
#define ALP_NOPROGRESS (1U << 31)
-bool xfs_action_list_process(struct scrub_ctx *ctx, int fd,
- struct xfs_action_list *alist, unsigned int repair_flags);
-void xfs_action_list_defer(struct scrub_ctx *ctx, xfs_agnumber_t agno,
- struct xfs_action_list *alist);
-bool xfs_action_list_process_or_defer(struct scrub_ctx *ctx, xfs_agnumber_t agno,
- struct xfs_action_list *alist);
+int action_list_process(struct scrub_ctx *ctx, int fd,
+ struct action_list *alist, unsigned int repair_flags);
+void action_list_defer(struct scrub_ctx *ctx, xfs_agnumber_t agno,
+ struct action_list *alist);
+int action_list_process_or_defer(struct scrub_ctx *ctx, xfs_agnumber_t agno,
+ struct action_list *alist);
#endif /* XFS_SCRUB_REPAIR_H_ */
diff --git a/scrub/scrub.c b/scrub/scrub.c
index 2bda5054..9aac3737 100644
--- a/scrub/scrub.c
+++ b/scrub/scrub.c
@@ -252,7 +252,7 @@ _("Optimizations of %s are possible."), _(xfrog_scrubbers[i].descr));
static int
xfs_scrub_save_repair(
struct scrub_ctx *ctx,
- struct xfs_action_list *alist,
+ struct action_list *alist,
struct xfs_scrub_metadata *meta)
{
struct action_item *aitem;
@@ -280,7 +280,7 @@ xfs_scrub_save_repair(
break;
}
- xfs_action_list_add(alist, aitem);
+ action_list_add(alist, aitem);
return 0;
}
@@ -295,7 +295,7 @@ xfs_scrub_meta_type(
struct scrub_ctx *ctx,
unsigned int type,
xfs_agnumber_t agno,
- struct xfs_action_list *alist)
+ struct action_list *alist)
{
struct xfs_scrub_metadata meta = {
.sm_type = type,
@@ -336,7 +336,7 @@ xfs_scrub_all_types(
struct scrub_ctx *ctx,
enum xfrog_scrub_type scrub_type,
xfs_agnumber_t agno,
- struct xfs_action_list *alist)
+ struct action_list *alist)
{
const struct xfrog_scrub_descr *sc;
unsigned int type;
@@ -367,7 +367,7 @@ xfs_scrub_all_types(
int
xfs_scrub_primary_super(
struct scrub_ctx *ctx,
- struct xfs_action_list *alist)
+ struct action_list *alist)
{
return xfs_scrub_meta_type(ctx, XFS_SCRUB_TYPE_SB, 0, alist);
}
@@ -377,7 +377,7 @@ int
xfs_scrub_ag_headers(
struct scrub_ctx *ctx,
xfs_agnumber_t agno,
- struct xfs_action_list *alist)
+ struct action_list *alist)
{
return xfs_scrub_all_types(ctx, XFROG_SCRUB_TYPE_AGHEADER, agno, alist);
}
@@ -387,7 +387,7 @@ int
xfs_scrub_ag_metadata(
struct scrub_ctx *ctx,
xfs_agnumber_t agno,
- struct xfs_action_list *alist)
+ struct action_list *alist)
{
return xfs_scrub_all_types(ctx, XFROG_SCRUB_TYPE_PERAG, agno, alist);
}
@@ -396,7 +396,7 @@ xfs_scrub_ag_metadata(
int
xfs_scrub_fs_metadata(
struct scrub_ctx *ctx,
- struct xfs_action_list *alist)
+ struct action_list *alist)
{
return xfs_scrub_all_types(ctx, XFROG_SCRUB_TYPE_FS, 0, alist);
}
@@ -405,7 +405,7 @@ xfs_scrub_fs_metadata(
int
xfs_scrub_fs_summary(
struct scrub_ctx *ctx,
- struct xfs_action_list *alist)
+ struct action_list *alist)
{
return xfs_scrub_meta_type(ctx, XFS_SCRUB_TYPE_FSCOUNTERS, 0, alist);
}
@@ -446,7 +446,7 @@ __xfs_scrub_file(
uint64_t ino,
uint32_t gen,
unsigned int type,
- struct xfs_action_list *alist)
+ struct action_list *alist)
{
struct xfs_scrub_metadata meta = {0};
enum check_outcome fix;
@@ -473,7 +473,7 @@ xfs_scrub_inode_fields(
struct scrub_ctx *ctx,
uint64_t ino,
uint32_t gen,
- struct xfs_action_list *alist)
+ struct action_list *alist)
{
return __xfs_scrub_file(ctx, ino, gen, XFS_SCRUB_TYPE_INODE, alist);
}
@@ -483,7 +483,7 @@ xfs_scrub_data_fork(
struct scrub_ctx *ctx,
uint64_t ino,
uint32_t gen,
- struct xfs_action_list *alist)
+ struct action_list *alist)
{
return __xfs_scrub_file(ctx, ino, gen, XFS_SCRUB_TYPE_BMBTD, alist);
}
@@ -493,7 +493,7 @@ xfs_scrub_attr_fork(
struct scrub_ctx *ctx,
uint64_t ino,
uint32_t gen,
- struct xfs_action_list *alist)
+ struct action_list *alist)
{
return __xfs_scrub_file(ctx, ino, gen, XFS_SCRUB_TYPE_BMBTA, alist);
}
@@ -503,7 +503,7 @@ xfs_scrub_cow_fork(
struct scrub_ctx *ctx,
uint64_t ino,
uint32_t gen,
- struct xfs_action_list *alist)
+ struct action_list *alist)
{
return __xfs_scrub_file(ctx, ino, gen, XFS_SCRUB_TYPE_BMBTC, alist);
}
@@ -513,7 +513,7 @@ xfs_scrub_dir(
struct scrub_ctx *ctx,
uint64_t ino,
uint32_t gen,
- struct xfs_action_list *alist)
+ struct action_list *alist)
{
return __xfs_scrub_file(ctx, ino, gen, XFS_SCRUB_TYPE_DIR, alist);
}
@@ -523,7 +523,7 @@ xfs_scrub_attr(
struct scrub_ctx *ctx,
uint64_t ino,
uint32_t gen,
- struct xfs_action_list *alist)
+ struct action_list *alist)
{
return __xfs_scrub_file(ctx, ino, gen, XFS_SCRUB_TYPE_XATTR, alist);
}
@@ -533,7 +533,7 @@ xfs_scrub_symlink(
struct scrub_ctx *ctx,
uint64_t ino,
uint32_t gen,
- struct xfs_action_list *alist)
+ struct action_list *alist)
{
return __xfs_scrub_file(ctx, ino, gen, XFS_SCRUB_TYPE_SYMLINK, alist);
}
@@ -543,7 +543,7 @@ xfs_scrub_parent(
struct scrub_ctx *ctx,
uint64_t ino,
uint32_t gen,
- struct xfs_action_list *alist)
+ struct action_list *alist)
{
return __xfs_scrub_file(ctx, ino, gen, XFS_SCRUB_TYPE_PARENT, alist);
}
diff --git a/scrub/scrub.h b/scrub/scrub.h
index bfb3f8e3..161e694f 100644
--- a/scrub/scrub.h
+++ b/scrub/scrub.h
@@ -17,16 +17,13 @@ enum check_outcome {
struct action_item;
void xfs_scrub_report_preen_triggers(struct scrub_ctx *ctx);
-int xfs_scrub_primary_super(struct scrub_ctx *ctx,
- struct xfs_action_list *alist);
+int xfs_scrub_primary_super(struct scrub_ctx *ctx, struct action_list *alist);
int xfs_scrub_ag_headers(struct scrub_ctx *ctx, xfs_agnumber_t agno,
- struct xfs_action_list *alist);
+ struct action_list *alist);
int xfs_scrub_ag_metadata(struct scrub_ctx *ctx, xfs_agnumber_t agno,
- struct xfs_action_list *alist);
-int xfs_scrub_fs_metadata(struct scrub_ctx *ctx,
- struct xfs_action_list *alist);
-int xfs_scrub_fs_summary(struct scrub_ctx *ctx,
- struct xfs_action_list *alist);
+ struct action_list *alist);
+int xfs_scrub_fs_metadata(struct scrub_ctx *ctx, struct action_list *alist);
+int xfs_scrub_fs_summary(struct scrub_ctx *ctx, struct action_list *alist);
bool xfs_can_scrub_fs_metadata(struct scrub_ctx *ctx);
bool xfs_can_scrub_inode(struct scrub_ctx *ctx);
@@ -38,21 +35,21 @@ bool xfs_can_scrub_parent(struct scrub_ctx *ctx);
bool xfs_can_repair(struct scrub_ctx *ctx);
int xfs_scrub_inode_fields(struct scrub_ctx *ctx, uint64_t ino, uint32_t gen,
- struct xfs_action_list *alist);
+ struct action_list *alist);
int xfs_scrub_data_fork(struct scrub_ctx *ctx, uint64_t ino, uint32_t gen,
- struct xfs_action_list *alist);
+ struct action_list *alist);
int xfs_scrub_attr_fork(struct scrub_ctx *ctx, uint64_t ino, uint32_t gen,
- struct xfs_action_list *alist);
+ struct action_list *alist);
int xfs_scrub_cow_fork(struct scrub_ctx *ctx, uint64_t ino, uint32_t gen,
- struct xfs_action_list *alist);
+ struct action_list *alist);
int xfs_scrub_dir(struct scrub_ctx *ctx, uint64_t ino, uint32_t gen,
- struct xfs_action_list *alist);
+ struct action_list *alist);
int xfs_scrub_attr(struct scrub_ctx *ctx, uint64_t ino, uint32_t gen,
- struct xfs_action_list *alist);
+ struct action_list *alist);
int xfs_scrub_symlink(struct scrub_ctx *ctx, uint64_t ino, uint32_t gen,
- struct xfs_action_list *alist);
+ struct action_list *alist);
int xfs_scrub_parent(struct scrub_ctx *ctx, uint64_t ino, uint32_t gen,
- struct xfs_action_list *alist);
+ struct action_list *alist);
/* Repair parameters are the scrub inputs and retry count. */
struct action_item {
diff --git a/scrub/xfs_scrub.h b/scrub/xfs_scrub.h
index d8032e3d..88537b0b 100644
--- a/scrub/xfs_scrub.h
+++ b/scrub/xfs_scrub.h
@@ -71,7 +71,7 @@ struct scrub_ctx {
/* Mutable scrub state; use lock. */
pthread_mutex_t lock;
- struct xfs_action_list *action_lists;
+ struct action_list *action_lists;
unsigned long long max_errors;
unsigned long long runtime_errors;
unsigned long long corruptions_found;
^ permalink raw reply related [flat|nested] 25+ messages in thread
* [PATCH 10/18] xfs_scrub: remove moveon from phase 7 functions
2019-10-22 18:50 [PATCH 00/18] xfs_scrub: remove moveon space aliens Darrick J. Wong
` (8 preceding siblings ...)
2019-10-22 18:51 ` [PATCH 09/18] xfs_scrub: remove moveon from repair action list helpers Darrick J. Wong
@ 2019-10-22 18:51 ` Darrick J. Wong
2019-10-22 18:51 ` [PATCH 11/18] xfs_scrub: remove moveon from phase 6 functions Darrick J. Wong
` (8 subsequent siblings)
18 siblings, 0 replies; 25+ messages in thread
From: Darrick J. Wong @ 2019-10-22 18:51 UTC (permalink / raw)
To: sandeen, darrick.wong; +Cc: linux-xfs
From: Darrick J. Wong <darrick.wong@oracle.com>
Replace the moveon returns in the phase 7 code with a direct integer
error return.
Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
---
scrub/phase7.c | 40 ++++++++++++++++++++--------------------
1 file changed, 20 insertions(+), 20 deletions(-)
diff --git a/scrub/phase7.c b/scrub/phase7.c
index 452d56ad..ff51c634 100644
--- a/scrub/phase7.c
+++ b/scrub/phase7.c
@@ -73,7 +73,7 @@ count_block_summary(
/* Add all the summaries in the per-thread counter */
static int
-xfs_add_summaries(
+add_summaries(
struct ptvar *ptv,
void *data,
void *arg)
@@ -93,8 +93,8 @@ xfs_add_summaries(
* filesystem we'll be content if the summary counts are within 10% of
* what we observed.
*/
-bool
-xfs_scan_summary(
+int
+phase7_func(
struct scrub_ctx *ctx)
{
struct summary_counts totalcount = {0};
@@ -113,7 +113,6 @@ xfs_scan_summary(
unsigned long long r_bfree;
unsigned long long f_files;
unsigned long long f_free;
- bool moveon;
bool complain;
bool scrub_all = scrub_data > 1;
int ip;
@@ -123,33 +122,31 @@ xfs_scan_summary(
action_list_init(&alist);
error = xfs_scrub_fs_summary(ctx, &alist);
if (error)
- return false;
+ return error;
error = action_list_process(ctx, ctx->mnt.fd, &alist,
ALP_COMPLAIN_IF_UNFIXED | ALP_NOPROGRESS);
if (error)
- return false;
+ return error;
/* Flush everything out to disk before we start counting. */
error = syncfs(ctx->mnt.fd);
if (error) {
str_errno(ctx, ctx->mntpoint);
- return false;
+ return error;
}
error = ptvar_alloc(scrub_nproc(ctx), sizeof(struct summary_counts),
&ptvar);
if (error) {
str_liberror(ctx, error, _("setting up block counter"));
- return false;
+ return error;
}
/* Use fsmap to count blocks. */
error = scrub_scan_all_spacemaps(ctx, count_block_summary, ptvar);
- if (error) {
- moveon = false;
+ if (error)
goto out_free;
- }
- error = ptvar_foreach(ptvar, xfs_add_summaries, &totalcount);
+ error = ptvar_foreach(ptvar, add_summaries, &totalcount);
if (error) {
str_liberror(ctx, error, _("counting blocks"));
goto out_free;
@@ -160,15 +157,14 @@ xfs_scan_summary(
error = scrub_count_all_inodes(ctx, &counted_inodes);
if (error) {
str_liberror(ctx, error, _("counting inodes"));
- moveon = false;
- goto out;
+ return error;
}
error = scrub_scan_estimate_blocks(ctx, &d_blocks, &d_bfree, &r_blocks,
&r_bfree, &f_files, &f_free);
if (error) {
str_liberror(ctx, error, _("estimating verify work"));
- return false;
+ return error;
}
/*
@@ -277,11 +273,15 @@ _("%.1f%s data counted; %.1f%s file data media verified.\n"),
fflush(stdout);
}
- moveon = true;
-
-out:
- return moveon;
+ return 0;
out_free:
ptvar_free(ptvar);
- return moveon;
+ return error;
+}
+
+bool
+xfs_scan_summary(
+ struct scrub_ctx *ctx)
+{
+ return phase7_func(ctx) == 0;
}
^ permalink raw reply related [flat|nested] 25+ messages in thread
* [PATCH 11/18] xfs_scrub: remove moveon from phase 6 functions
2019-10-22 18:50 [PATCH 00/18] xfs_scrub: remove moveon space aliens Darrick J. Wong
` (9 preceding siblings ...)
2019-10-22 18:51 ` [PATCH 10/18] xfs_scrub: remove moveon from phase 7 functions Darrick J. Wong
@ 2019-10-22 18:51 ` Darrick J. Wong
2019-11-06 21:02 ` [PATCH v2 " Darrick J. Wong
2019-10-22 18:51 ` [PATCH 12/18] xfs_scrub: remove moveon from phase 5 functions Darrick J. Wong
` (7 subsequent siblings)
18 siblings, 1 reply; 25+ messages in thread
From: Darrick J. Wong @ 2019-10-22 18:51 UTC (permalink / raw)
To: sandeen, darrick.wong; +Cc: linux-xfs
From: Darrick J. Wong <darrick.wong@oracle.com>
Replace the moveon returns in the phase 6 code with a direct integer
error return.
Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
---
scrub/phase6.c | 169 +++++++++++++++++++++++++++++---------------------------
1 file changed, 89 insertions(+), 80 deletions(-)
diff --git a/scrub/phase6.c b/scrub/phase6.c
index 805c93eb..2aa3fd10 100644
--- a/scrub/phase6.c
+++ b/scrub/phase6.c
@@ -45,7 +45,7 @@ struct media_verify_state {
/* Find the fd for a given device identifier. */
static struct read_verify_pool *
-xfs_dev_to_pool(
+dev_to_pool(
struct scrub_ctx *ctx,
struct media_verify_state *vs,
dev_t dev)
@@ -61,7 +61,7 @@ xfs_dev_to_pool(
/* Find the device major/minor for a given file descriptor. */
static dev_t
-xfs_disk_to_dev(
+disk_to_dev(
struct scrub_ctx *ctx,
struct disk *disk)
{
@@ -81,7 +81,7 @@ bitmap_for_disk(
struct disk *disk,
struct media_verify_state *vs)
{
- dev_t dev = xfs_disk_to_dev(ctx, disk);
+ dev_t dev = disk_to_dev(ctx, disk);
if (dev == ctx->fsinfo.fs_datadev)
return vs->d_bad;
@@ -116,7 +116,7 @@ static const struct owner_decode special_owners[] = {
/* Decode a special owner. */
static const char *
-xfs_decode_special_owner(
+decode_special_owner(
uint64_t owner)
{
const struct owner_decode *od = special_owners;
@@ -234,8 +234,8 @@ _("media error in extended attribute data."));
}
/* Iterate the extent mappings of a file to report errors. */
-static bool
-xfs_report_verify_fd(
+static int
+report_fd_loss(
struct scrub_ctx *ctx,
const char *descr,
int fd,
@@ -254,7 +254,7 @@ xfs_report_verify_fd(
report_data_loss, &br);
if (ret) {
str_liberror(ctx, ret, descr);
- return false;
+ return ret;
}
/* attr fork */
@@ -262,23 +262,23 @@ xfs_report_verify_fd(
report_attr_loss, &br);
if (ret) {
str_liberror(ctx, ret, descr);
- return false;
+ return ret;
}
- return true;
+
+ return 0;
}
/* Report read verify errors in unlinked (but still open) files. */
static int
-xfs_report_verify_inode(
+report_inode_loss(
struct scrub_ctx *ctx,
struct xfs_handle *handle,
struct xfs_bulkstat *bstat,
void *arg)
{
char descr[DESCR_BUFSZ];
- bool moveon;
int fd;
- int error;
+ int error, err2;
/* Ignore linked files and things we can't open. */
if (bstat->bs_nlink != 0)
@@ -302,26 +302,24 @@ _("Disappeared during read error reporting."));
}
/* Go find the badness. */
- moveon = xfs_report_verify_fd(ctx, descr, fd, arg);
- error = close(fd);
- if (error)
+ error = report_fd_loss(ctx, descr, fd, arg);
+
+ err2 = close(fd);
+ if (err2)
str_errno(ctx, descr);
- return moveon ? 0 : XFS_ITERATE_INODES_ABORT;
+ return error;
}
/* Scan a directory for matches in the read verify error list. */
static int
-xfs_report_verify_dir(
+report_dir_loss(
struct scrub_ctx *ctx,
const char *path,
int dir_fd,
void *arg)
{
- bool moveon;
-
- moveon = xfs_report_verify_fd(ctx, path, dir_fd, arg);
- return moveon ? 0 : -1;
+ return report_fd_loss(ctx, path, dir_fd, arg);
}
/*
@@ -329,7 +327,7 @@ xfs_report_verify_dir(
* the read verify error list.
*/
static int
-xfs_report_verify_dirent(
+report_dirent_loss(
struct scrub_ctx *ctx,
const char *path,
int dir_fd,
@@ -337,9 +335,8 @@ xfs_report_verify_dirent(
struct stat *sb,
void *arg)
{
- bool moveon;
int fd;
- int error;
+ int error, err2;
/* Ignore things we can't open. */
if (!S_ISREG(sb->st_mode) && !S_ISDIR(sb->st_mode))
@@ -364,15 +361,15 @@ xfs_report_verify_dirent(
}
/* Go find the badness. */
- moveon = xfs_report_verify_fd(ctx, path, fd, arg);
- if (moveon)
- goto out;
+ error = report_fd_loss(ctx, path, fd, arg);
-out:
- error = close(fd);
- if (error)
+ err2 = close(fd);
+ if (err2)
str_errno(ctx, path);
- return moveon ? 0 : -1;
+ if (!error && err2)
+ error = err2;
+
+ return error;
}
/* Use a fsmap to report metadata lost to a media error. */
@@ -400,7 +397,7 @@ report_ioerr_fsmap(
if (map->fmr_flags & FMR_OF_SPECIAL_OWNER) {
snprintf(buf, DESCR_BUFSZ, _("disk offset %"PRIu64),
(uint64_t)map->fmr_physical + err_off);
- type = xfs_decode_special_owner(map->fmr_owner);
+ type = decode_special_owner(map->fmr_owner);
str_corrupt(ctx, buf, _("media error in %s."), type);
}
@@ -438,7 +435,7 @@ report_ioerr(
struct disk_ioerr_report *dioerr = arg;
dev_t dev;
- dev = xfs_disk_to_dev(dioerr->ctx, dioerr->disk);
+ dev = disk_to_dev(dioerr->ctx, dioerr->disk);
/* Go figure out which blocks are bad from the fsmap. */
memset(keys, 0, sizeof(struct fsmap) * 2);
@@ -475,7 +472,7 @@ report_disk_ioerrs(
}
/* Given bad extent lists for the data & rtdev, find bad files. */
-static bool
+static int
report_all_media_errors(
struct scrub_ctx *ctx,
struct media_verify_state *vs)
@@ -485,24 +482,22 @@ report_all_media_errors(
ret = report_disk_ioerrs(ctx, ctx->datadev, vs);
if (ret) {
str_liberror(ctx, ret, _("walking datadev io errors"));
- return false;
+ return ret;
}
ret = report_disk_ioerrs(ctx, ctx->rtdev, vs);
if (ret) {
str_liberror(ctx, ret, _("walking rtdev io errors"));
- return false;
+ return ret;
}
/* Scan the directory tree to get file paths. */
- ret = scan_fs_tree(ctx, xfs_report_verify_dir,
- xfs_report_verify_dirent, vs);
+ ret = scan_fs_tree(ctx, report_dir_loss, report_dirent_loss, vs);
if (ret)
- return false;
+ return ret;
/* Scan for unlinked files. */
- ret = scrub_scan_all_inodes(ctx, xfs_report_verify_inode, vs);
- return ret == 0;
+ return scrub_scan_all_inodes(ctx, report_inode_loss, vs);
}
/* Schedule a read-verify of a (data block) extent. */
@@ -516,7 +511,7 @@ check_rmap(
struct read_verify_pool *rvp;
int ret;
- rvp = xfs_dev_to_pool(ctx, vs, map->fmr_device);
+ rvp = dev_to_pool(ctx, vs, map->fmr_device);
dbg_printf("rmap dev %d:%d phys %"PRIu64" owner %"PRId64
" offset %"PRIu64" len %"PRIu64" flags 0x%x\n",
@@ -619,7 +614,7 @@ verify_entire_disk(
}
/* Scan every part of every disk. */
-static bool
+static int
verify_all_disks(
struct scrub_ctx *ctx,
struct media_verify_state *vs)
@@ -629,14 +624,14 @@ verify_all_disks(
ret = verify_entire_disk(vs->rvp_data, ctx->datadev, vs);
if (ret) {
str_liberror(ctx, ret, _("scheduling datadev verify"));
- return false;
+ return ret;
}
if (ctx->logdev) {
ret = verify_entire_disk(vs->rvp_log, ctx->logdev, vs);
if (ret) {
str_liberror(ctx, ret, _("scheduling logdev verify"));
- return false;
+ return ret;
}
}
@@ -644,11 +639,11 @@ verify_all_disks(
ret = verify_entire_disk(vs->rvp_realtime, ctx->rtdev, vs);
if (ret) {
str_liberror(ctx, ret, _("scheduling rtdev verify"));
- return false;
+ return ret;
}
}
- return true;
+ return 0;
}
/*
@@ -659,18 +654,17 @@ verify_all_disks(
* scan the extent maps of the entire fs tree to figure (and the unlinked
* inodes) out which files are now broken.
*/
-bool
-xfs_scan_blocks(
+int
+phase6_func(
struct scrub_ctx *ctx)
{
struct media_verify_state vs = { NULL };
- bool moveon = false;
- int ret;
+ int ret, ret2, ret3;
ret = bitmap_alloc(&vs.d_bad);
if (ret) {
str_liberror(ctx, ret, _("creating datadev badblock bitmap"));
- goto out;
+ return ret;
}
ret = bitmap_alloc(&vs.r_bad);
@@ -708,40 +702,39 @@ xfs_scan_blocks(
}
if (scrub_data > 1)
- moveon = verify_all_disks(ctx, &vs);
- else {
+ ret = verify_all_disks(ctx, &vs);
+ else
ret = scrub_scan_all_spacemaps(ctx, check_rmap, &vs);
- if (ret)
- moveon = false;
- }
- if (!moveon)
+ if (ret)
goto out_rtpool;
ret = clean_pool(vs.rvp_data, &ctx->bytes_checked);
- if (ret) {
+ if (ret)
str_liberror(ctx, ret, _("flushing datadev verify pool"));
- moveon = false;
- }
- ret = clean_pool(vs.rvp_log, &ctx->bytes_checked);
- if (ret) {
- str_liberror(ctx, ret, _("flushing logdev verify pool"));
- moveon = false;
- }
+ ret2 = clean_pool(vs.rvp_log, &ctx->bytes_checked);
+ if (ret2)
+ str_liberror(ctx, ret2, _("flushing logdev verify pool"));
- ret = clean_pool(vs.rvp_realtime, &ctx->bytes_checked);
- if (ret) {
- str_liberror(ctx, ret, _("flushing rtdev verify pool"));
- moveon = false;
- }
+ ret3 = clean_pool(vs.rvp_realtime, &ctx->bytes_checked);
+ if (ret3)
+ str_liberror(ctx, ret3, _("flushing rtdev verify pool"));
+
+ /*
+ * If the verify flush didn't work or we found no bad blocks, we're
+ * done! No errors detected.
+ */
+ if (ret || ret2 || ret3)
+ goto out_rbad;
+ if (bitmap_empty(vs.d_bad) && bitmap_empty(vs.r_bad))
+ goto out_rbad;
/* Scan the whole dir tree to see what matches the bad extents. */
- if (moveon && (!bitmap_empty(vs.d_bad) || !bitmap_empty(vs.r_bad)))
- moveon = report_all_media_errors(ctx, &vs);
+ ret = report_all_media_errors(ctx, &vs);
bitmap_free(&vs.r_bad);
bitmap_free(&vs.d_bad);
- return moveon;
+ return ret;
out_rtpool:
if (vs.rvp_realtime) {
@@ -760,13 +753,19 @@ xfs_scan_blocks(
bitmap_free(&vs.r_bad);
out_dbad:
bitmap_free(&vs.d_bad);
-out:
- return moveon;
+ return ret;
}
-/* Estimate how much work we're going to do. */
bool
-xfs_estimate_verify_work(
+xfs_scan_blocks(
+ struct scrub_ctx *ctx)
+{
+ return phase6_func(ctx) == 0;
+}
+
+/* Estimate how much work we're going to do. */
+int
+phase6_estimate(
struct scrub_ctx *ctx,
uint64_t *items,
unsigned int *nr_threads,
@@ -784,7 +783,7 @@ xfs_estimate_verify_work(
&r_blocks, &r_bfree, &f_files, &f_free);
if (ret) {
str_liberror(ctx, ret, _("estimating verify work"));
- return false;
+ return ret;
}
*items = cvt_off_fsb_to_b(&ctx->mnt, d_blocks + r_blocks);
@@ -792,5 +791,15 @@ xfs_estimate_verify_work(
*items -= cvt_off_fsb_to_b(&ctx->mnt, d_bfree + r_bfree);
*nr_threads = disk_heads(ctx->datadev);
*rshift = 20;
- return true;
+ return 0;
+}
+
+bool
+xfs_estimate_verify_work(
+ struct scrub_ctx *ctx,
+ uint64_t *items,
+ unsigned int *nr_threads,
+ int *rshift)
+{
+ return phase6_estimate(ctx, items, nr_threads, rshift) == 0;
}
^ permalink raw reply related [flat|nested] 25+ messages in thread
* [PATCH v2 11/18] xfs_scrub: remove moveon from phase 6 functions
2019-10-22 18:51 ` [PATCH 11/18] xfs_scrub: remove moveon from phase 6 functions Darrick J. Wong
@ 2019-11-06 21:02 ` Darrick J. Wong
0 siblings, 0 replies; 25+ messages in thread
From: Darrick J. Wong @ 2019-11-06 21:02 UTC (permalink / raw)
To: sandeen; +Cc: linux-xfs
From: Darrick J. Wong <darrick.wong@oracle.com>
Replace the moveon returns in the phase 6 code with a direct integer
error return.
Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
---
v2: rebase without full-disk media scan
---
scrub/phase6.c | 154 ++++++++++++++++++++++++++++++--------------------------
1 file changed, 82 insertions(+), 72 deletions(-)
diff --git a/scrub/phase6.c b/scrub/phase6.c
index 62629732..020b303d 100644
--- a/scrub/phase6.c
+++ b/scrub/phase6.c
@@ -45,7 +45,7 @@ struct media_verify_state {
/* Find the fd for a given device identifier. */
static struct read_verify_pool *
-xfs_dev_to_pool(
+dev_to_pool(
struct scrub_ctx *ctx,
struct media_verify_state *vs,
dev_t dev)
@@ -61,7 +61,7 @@ xfs_dev_to_pool(
/* Find the device major/minor for a given file descriptor. */
static dev_t
-xfs_disk_to_dev(
+disk_to_dev(
struct scrub_ctx *ctx,
struct disk *disk)
{
@@ -81,7 +81,7 @@ bitmap_for_disk(
struct disk *disk,
struct media_verify_state *vs)
{
- dev_t dev = xfs_disk_to_dev(ctx, disk);
+ dev_t dev = disk_to_dev(ctx, disk);
if (dev == ctx->fsinfo.fs_datadev)
return vs->d_bad;
@@ -116,7 +116,7 @@ static const struct owner_decode special_owners[] = {
/* Decode a special owner. */
static const char *
-xfs_decode_special_owner(
+decode_special_owner(
uint64_t owner)
{
const struct owner_decode *od = special_owners;
@@ -232,8 +232,8 @@ _("media error in extended attribute data."));
}
/* Iterate the extent mappings of a file to report errors. */
-static bool
-xfs_report_verify_fd(
+static int
+report_fd_loss(
struct scrub_ctx *ctx,
const char *descr,
int fd,
@@ -252,7 +252,7 @@ xfs_report_verify_fd(
report_data_loss, &br);
if (ret) {
str_liberror(ctx, ret, descr);
- return false;
+ return ret;
}
/* attr fork */
@@ -260,23 +260,23 @@ xfs_report_verify_fd(
report_attr_loss, &br);
if (ret) {
str_liberror(ctx, ret, descr);
- return false;
+ return ret;
}
- return true;
+
+ return 0;
}
/* Report read verify errors in unlinked (but still open) files. */
static int
-xfs_report_verify_inode(
+report_inode_loss(
struct scrub_ctx *ctx,
struct xfs_handle *handle,
struct xfs_bulkstat *bstat,
void *arg)
{
char descr[DESCR_BUFSZ];
- bool moveon;
int fd;
- int error;
+ int error, err2;
/* Ignore linked files and things we can't open. */
if (bstat->bs_nlink != 0)
@@ -300,26 +300,24 @@ _("Disappeared during read error reporting."));
}
/* Go find the badness. */
- moveon = xfs_report_verify_fd(ctx, descr, fd, arg);
- error = close(fd);
- if (error)
+ error = report_fd_loss(ctx, descr, fd, arg);
+
+ err2 = close(fd);
+ if (err2)
str_errno(ctx, descr);
- return moveon ? 0 : XFS_ITERATE_INODES_ABORT;
+ return error;
}
/* Scan a directory for matches in the read verify error list. */
static int
-xfs_report_verify_dir(
+report_dir_loss(
struct scrub_ctx *ctx,
const char *path,
int dir_fd,
void *arg)
{
- bool moveon;
-
- moveon = xfs_report_verify_fd(ctx, path, dir_fd, arg);
- return moveon ? 0 : -1;
+ return report_fd_loss(ctx, path, dir_fd, arg);
}
/*
@@ -327,7 +325,7 @@ xfs_report_verify_dir(
* the read verify error list.
*/
static int
-xfs_report_verify_dirent(
+report_dirent_loss(
struct scrub_ctx *ctx,
const char *path,
int dir_fd,
@@ -335,9 +333,8 @@ xfs_report_verify_dirent(
struct stat *sb,
void *arg)
{
- bool moveon;
int fd;
- int error;
+ int error, err2;
/* Ignore things we can't open. */
if (!S_ISREG(sb->st_mode) && !S_ISDIR(sb->st_mode))
@@ -362,15 +359,15 @@ xfs_report_verify_dirent(
}
/* Go find the badness. */
- moveon = xfs_report_verify_fd(ctx, path, fd, arg);
- if (moveon)
- goto out;
+ error = report_fd_loss(ctx, path, fd, arg);
-out:
- error = close(fd);
- if (error)
+ err2 = close(fd);
+ if (err2)
str_errno(ctx, path);
- return moveon ? 0 : -1;
+ if (!error && err2)
+ error = err2;
+
+ return error;
}
/* Use a fsmap to report metadata lost to a media error. */
@@ -398,7 +395,7 @@ report_ioerr_fsmap(
if (map->fmr_flags & FMR_OF_SPECIAL_OWNER) {
snprintf(buf, DESCR_BUFSZ, _("disk offset %"PRIu64),
(uint64_t)map->fmr_physical + err_off);
- type = xfs_decode_special_owner(map->fmr_owner);
+ type = decode_special_owner(map->fmr_owner);
str_corrupt(ctx, buf, _("media error in %s."), type);
}
@@ -436,7 +433,7 @@ report_ioerr(
struct disk_ioerr_report *dioerr = arg;
dev_t dev;
- dev = xfs_disk_to_dev(dioerr->ctx, dioerr->disk);
+ dev = disk_to_dev(dioerr->ctx, dioerr->disk);
/* Go figure out which blocks are bad from the fsmap. */
memset(keys, 0, sizeof(struct fsmap) * 2);
@@ -473,7 +470,7 @@ report_disk_ioerrs(
}
/* Given bad extent lists for the data & rtdev, find bad files. */
-static bool
+static int
report_all_media_errors(
struct scrub_ctx *ctx,
struct media_verify_state *vs)
@@ -483,24 +480,22 @@ report_all_media_errors(
ret = report_disk_ioerrs(ctx, ctx->datadev, vs);
if (ret) {
str_liberror(ctx, ret, _("walking datadev io errors"));
- return false;
+ return ret;
}
ret = report_disk_ioerrs(ctx, ctx->rtdev, vs);
if (ret) {
str_liberror(ctx, ret, _("walking rtdev io errors"));
- return false;
+ return ret;
}
/* Scan the directory tree to get file paths. */
- ret = scan_fs_tree(ctx, xfs_report_verify_dir,
- xfs_report_verify_dirent, vs);
+ ret = scan_fs_tree(ctx, report_dir_loss, report_dirent_loss, vs);
if (ret)
- return false;
+ return ret;
/* Scan for unlinked files. */
- ret = scrub_scan_all_inodes(ctx, xfs_report_verify_inode, vs);
- return ret == 0;
+ return scrub_scan_all_inodes(ctx, report_inode_loss, vs);
}
/* Schedule a read-verify of a (data block) extent. */
@@ -514,7 +509,7 @@ check_rmap(
struct read_verify_pool *rvp;
int ret;
- rvp = xfs_dev_to_pool(ctx, vs, map->fmr_device);
+ rvp = dev_to_pool(ctx, vs, map->fmr_device);
dbg_printf("rmap dev %d:%d phys %"PRIu64" owner %"PRId64
" offset %"PRIu64" len %"PRIu64" flags 0x%x\n",
@@ -614,18 +609,17 @@ remember_ioerr(
* scan the extent maps of the entire fs tree to figure (and the unlinked
* inodes) out which files are now broken.
*/
-bool
-xfs_scan_blocks(
+int
+phase6_func(
struct scrub_ctx *ctx)
{
struct media_verify_state vs = { NULL };
- bool moveon = false;
- int ret;
+ int ret, ret2, ret3;
ret = bitmap_alloc(&vs.d_bad);
if (ret) {
str_liberror(ctx, ret, _("creating datadev badblock bitmap"));
- goto out;
+ return ret;
}
ret = bitmap_alloc(&vs.r_bad);
@@ -662,36 +656,36 @@ xfs_scan_blocks(
}
}
ret = scrub_scan_all_spacemaps(ctx, check_rmap, &vs);
- if (ret) {
- moveon = false;
+ if (ret)
goto out_rtpool;
- }
ret = clean_pool(vs.rvp_data, &ctx->bytes_checked);
- if (ret) {
+ if (ret)
str_liberror(ctx, ret, _("flushing datadev verify pool"));
- moveon = false;
- }
- ret = clean_pool(vs.rvp_log, &ctx->bytes_checked);
- if (ret) {
- str_liberror(ctx, ret, _("flushing logdev verify pool"));
- moveon = false;
- }
+ ret2 = clean_pool(vs.rvp_log, &ctx->bytes_checked);
+ if (ret2)
+ str_liberror(ctx, ret2, _("flushing logdev verify pool"));
- ret = clean_pool(vs.rvp_realtime, &ctx->bytes_checked);
- if (ret) {
- str_liberror(ctx, ret, _("flushing rtdev verify pool"));
- moveon = false;
- }
+ ret3 = clean_pool(vs.rvp_realtime, &ctx->bytes_checked);
+ if (ret3)
+ str_liberror(ctx, ret3, _("flushing rtdev verify pool"));
+
+ /*
+ * If the verify flush didn't work or we found no bad blocks, we're
+ * done! No errors detected.
+ */
+ if (ret || ret2 || ret3)
+ goto out_rbad;
+ if (bitmap_empty(vs.d_bad) && bitmap_empty(vs.r_bad))
+ goto out_rbad;
/* Scan the whole dir tree to see what matches the bad extents. */
- if (moveon && (!bitmap_empty(vs.d_bad) || !bitmap_empty(vs.r_bad)))
- moveon = report_all_media_errors(ctx, &vs);
+ ret = report_all_media_errors(ctx, &vs);
bitmap_free(&vs.r_bad);
bitmap_free(&vs.d_bad);
- return moveon;
+ return ret;
out_rtpool:
if (vs.rvp_realtime) {
@@ -710,13 +704,19 @@ xfs_scan_blocks(
bitmap_free(&vs.r_bad);
out_dbad:
bitmap_free(&vs.d_bad);
-out:
- return moveon;
+ return ret;
}
-/* Estimate how much work we're going to do. */
bool
-xfs_estimate_verify_work(
+xfs_scan_blocks(
+ struct scrub_ctx *ctx)
+{
+ return phase6_func(ctx) == 0;
+}
+
+/* Estimate how much work we're going to do. */
+int
+phase6_estimate(
struct scrub_ctx *ctx,
uint64_t *items,
unsigned int *nr_threads,
@@ -734,12 +734,22 @@ xfs_estimate_verify_work(
&r_blocks, &r_bfree, &f_files, &f_free);
if (ret) {
str_liberror(ctx, ret, _("estimating verify work"));
- return false;
+ return ret;
}
*items = cvt_off_fsb_to_b(&ctx->mnt,
(d_blocks - d_bfree) + (r_blocks - r_bfree));
*nr_threads = disk_heads(ctx->datadev);
*rshift = 20;
- return true;
+ return 0;
+}
+
+bool
+xfs_estimate_verify_work(
+ struct scrub_ctx *ctx,
+ uint64_t *items,
+ unsigned int *nr_threads,
+ int *rshift)
+{
+ return phase6_estimate(ctx, items, nr_threads, rshift) == 0;
}
^ permalink raw reply related [flat|nested] 25+ messages in thread
* [PATCH 12/18] xfs_scrub: remove moveon from phase 5 functions
2019-10-22 18:50 [PATCH 00/18] xfs_scrub: remove moveon space aliens Darrick J. Wong
` (10 preceding siblings ...)
2019-10-22 18:51 ` [PATCH 11/18] xfs_scrub: remove moveon from phase 6 functions Darrick J. Wong
@ 2019-10-22 18:51 ` Darrick J. Wong
2019-10-22 18:51 ` [PATCH 13/18] xfs_scrub: remove moveon from phase 4 functions Darrick J. Wong
` (6 subsequent siblings)
18 siblings, 0 replies; 25+ messages in thread
From: Darrick J. Wong @ 2019-10-22 18:51 UTC (permalink / raw)
To: sandeen, darrick.wong; +Cc: linux-xfs
From: Darrick J. Wong <darrick.wong@oracle.com>
Replace the moveon returns in the phase 5 code with a direct integer
error return.
Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
---
scrub/phase5.c | 184 ++++++++++++++++++++++++++++++--------------------------
1 file changed, 97 insertions(+), 87 deletions(-)
diff --git a/scrub/phase5.c b/scrub/phase5.c
index c2cccd76..2641a7fb 100644
--- a/scrub/phase5.c
+++ b/scrub/phase5.c
@@ -31,9 +31,11 @@
* terminal control characters and escape sequences, since that could be used
* to do something naughty to the user's computer and/or break scripts. XFS
* doesn't consider any byte sequence invalid, so don't flag these as errors.
+ *
+ * Returns 0 for success or -1 for error. This function logs errors.
*/
-static bool
-xfs_scrub_check_name(
+static int
+simple_check_name(
struct scrub_ctx *ctx,
struct descr *dsc,
const char *namedescr,
@@ -46,7 +48,7 @@ xfs_scrub_check_name(
/* Complain about zero length names. */
if (*name == '\0' && should_warn_about_name(ctx)) {
str_warn(ctx, descr_render(dsc), _("Zero length name found."));
- return true;
+ return 0;
}
/* control characters */
@@ -61,7 +63,7 @@ xfs_scrub_check_name(
errname = string_escape(name);
if (!errname) {
str_errno(ctx, descr_render(dsc));
- return false;
+ return -1;
}
str_info(ctx, descr_render(dsc),
_("Control character found in %s name \"%s\"."),
@@ -69,15 +71,15 @@ _("Control character found in %s name \"%s\"."),
free(errname);
}
- return true;
+ return 0;
}
/*
* Iterate a directory looking for filenames with problematic
* characters.
*/
-static bool
-xfs_scrub_scan_dirents(
+static int
+check_dirent_names(
struct scrub_ctx *ctx,
struct descr *dsc,
int *fd,
@@ -86,45 +88,45 @@ xfs_scrub_scan_dirents(
struct unicrash *uc = NULL;
DIR *dir;
struct dirent *dentry;
- bool moveon = true;
int ret;
dir = fdopendir(*fd);
if (!dir) {
str_errno(ctx, descr_render(dsc));
- moveon = false;
- goto out;
+ return errno;
}
*fd = -1; /* closedir will close *fd for us */
ret = unicrash_dir_init(&uc, ctx, bstat);
if (ret) {
str_liberror(ctx, ret, descr_render(dsc));
- moveon = false;
goto out_unicrash;
}
+ errno = 0;
dentry = readdir(dir);
while (dentry) {
- if (uc) {
+ if (uc)
ret = unicrash_check_dir_name(uc, dsc, dentry);
- if (ret) {
- str_liberror(ctx, ret, descr_render(dsc));
- moveon = false;
- }
- } else
- moveon = xfs_scrub_check_name(ctx, dsc,
- _("directory"), dentry->d_name);
- if (!moveon)
+ else
+ ret = simple_check_name(ctx, dsc, _("directory"),
+ dentry->d_name);
+ if (ret) {
+ str_liberror(ctx, ret, descr_render(dsc));
break;
+ }
+ errno = 0;
dentry = readdir(dir);
}
+ if (errno) {
+ ret = errno;
+ str_liberror(ctx, ret, descr_render(dsc));
+ }
unicrash_free(uc);
out_unicrash:
closedir(dir);
-out:
- return moveon;
+ return ret;
}
#ifdef HAVE_LIBATTR
@@ -145,8 +147,8 @@ static const struct attrns_decode attr_ns[] = {
* Check all the xattr names in a particular namespace of a file handle
* for Unicode normalization problems or collisions.
*/
-static bool
-xfs_scrub_scan_fhandle_namespace_xattrs(
+static int
+check_xattr_ns_names(
struct scrub_ctx *ctx,
struct descr *dsc,
struct xfs_handle *handle,
@@ -159,14 +161,13 @@ xfs_scrub_scan_fhandle_namespace_xattrs(
struct attrlist *attrlist = (struct attrlist *)attrbuf;
struct attrlist_ent *ent;
struct unicrash *uc = NULL;
- bool moveon = true;
int i;
int error;
error = unicrash_xattr_init(&uc, ctx, bstat);
if (error) {
str_liberror(ctx, error, descr_render(dsc));
- return false;
+ return error;
}
memset(attrbuf, 0, XFS_XATTR_LIST_MAX);
@@ -180,20 +181,17 @@ xfs_scrub_scan_fhandle_namespace_xattrs(
ent = ATTR_ENTRY(attrlist, i);
snprintf(keybuf, XATTR_NAME_MAX, "%s.%s", attr_ns->name,
ent->a_name);
- if (uc) {
+ if (uc)
error = unicrash_check_xattr_name(uc, dsc,
keybuf);
- if (error) {
- str_liberror(ctx, error,
- descr_render(dsc));
- moveon = false;
- }
- } else
- moveon = xfs_scrub_check_name(ctx, dsc,
+ else
+ error = simple_check_name(ctx, dsc,
_("extended attribute"),
keybuf);
- if (!moveon)
+ if (error) {
+ str_liberror(ctx, error, descr_render(dsc));
goto out;
+ }
}
if (!attrlist->al_more)
@@ -201,37 +199,40 @@ xfs_scrub_scan_fhandle_namespace_xattrs(
error = attr_list_by_handle(handle, sizeof(*handle), attrbuf,
XFS_XATTR_LIST_MAX, attr_ns->flags, &cur);
}
- if (error && errno != ESTALE)
- str_errno(ctx, descr_render(dsc));
+ if (error) {
+ if (errno == ESTALE)
+ errno = 0;
+ if (errno)
+ str_errno(ctx, descr_render(dsc));
+ }
out:
unicrash_free(uc);
- return moveon;
+ return error;
}
/*
* Check all the xattr names in all the xattr namespaces for problematic
* characters.
*/
-static bool
-xfs_scrub_scan_fhandle_xattrs(
+static int
+check_xattr_names(
struct scrub_ctx *ctx,
struct descr *dsc,
struct xfs_handle *handle,
struct xfs_bulkstat *bstat)
{
const struct attrns_decode *ns;
- bool moveon = true;
+ int ret;
for (ns = attr_ns; ns->name; ns++) {
- moveon = xfs_scrub_scan_fhandle_namespace_xattrs(ctx, dsc,
- handle, bstat, ns);
- if (!moveon)
+ ret = check_xattr_ns_names(ctx, dsc, handle, bstat, ns);
+ if (ret)
break;
}
- return moveon;
+ return ret;
}
#else
-# define xfs_scrub_scan_fhandle_xattrs(c, d, h, b) (true)
+# define check_xattr_names(c, d, h, b) (0)
#endif /* HAVE_LIBATTR */
static int
@@ -255,26 +256,25 @@ render_ino_from_handle(
* Check for potential Unicode collisions in names.
*/
static int
-xfs_scrub_connections(
+check_inode_names(
struct scrub_ctx *ctx,
struct xfs_handle *handle,
struct xfs_bulkstat *bstat,
void *arg)
{
- bool *pmoveon = arg;
DEFINE_DESCR(dsc, ctx, render_ino_from_handle);
- bool moveon = true;
+ bool *aborted = arg;
int fd = -1;
- int error;
+ int error = 0;
+ int err2;
descr_set(&dsc, bstat);
background_sleep();
/* Warn about naming problems in xattrs. */
if (bstat->bs_xflags & FS_XFLAG_HASATTR) {
- moveon = xfs_scrub_scan_fhandle_xattrs(ctx, &dsc, handle,
- bstat);
- if (!moveon)
+ error = check_xattr_names(ctx, &dsc, handle, bstat);
+ if (error)
goto out;
}
@@ -282,7 +282,8 @@ xfs_scrub_connections(
if (S_ISDIR(bstat->bs_mode)) {
fd = scrub_open_handle(handle);
if (fd < 0) {
- if (errno == ESTALE)
+ error = errno;
+ if (error == ESTALE)
return ESTALE;
str_errno(ctx, descr_render(&dsc));
goto out;
@@ -291,21 +292,27 @@ xfs_scrub_connections(
/* Warn about naming problems in the directory entries. */
if (fd >= 0 && S_ISDIR(bstat->bs_mode)) {
- moveon = xfs_scrub_scan_dirents(ctx, &dsc, &fd, bstat);
- if (!moveon)
+ error = check_dirent_names(ctx, &dsc, &fd, bstat);
+ if (error)
goto out;
}
out:
progress_add(1);
if (fd >= 0) {
- error = close(fd);
- if (error)
+ err2 = close(fd);
+ if (err2)
str_errno(ctx, descr_render(&dsc));
+ if (!error && err2)
+ error = err2;
}
- if (!moveon)
- *pmoveon = false;
- return *pmoveon ? 0 : XFS_ITERATE_INODES_ABORT;
+
+ if (error)
+ *aborted = true;
+ if (!error && *aborted)
+ error = ECANCELED;
+
+ return error;
}
#ifndef FS_IOC_GETFSLABEL
@@ -327,20 +334,19 @@ scrub_render_mountpoint(
* Check the filesystem label for Unicode normalization problems or misleading
* sequences.
*/
-static bool
-xfs_scrub_fs_label(
+static int
+check_fs_label(
struct scrub_ctx *ctx)
{
DEFINE_DESCR(dsc, ctx, scrub_render_mountpoint);
char label[FSLABEL_MAX];
struct unicrash *uc = NULL;
- bool moveon = true;
int error;
error = unicrash_fs_label_init(&uc, ctx);
if (error) {
str_liberror(ctx, error, descr_render(&dsc));
- return false;
+ return error;
}
descr_set(&dsc, NULL);
@@ -349,7 +355,7 @@ xfs_scrub_fs_label(
error = ioctl(ctx->mnt.fd, FS_IOC_GETFSLABEL, &label);
if (error) {
if (errno != EOPNOTSUPP && errno != ENOTTY) {
- moveon = false;
+ error = errno;
perror(ctx->mntpoint);
}
goto out;
@@ -360,45 +366,49 @@ xfs_scrub_fs_label(
goto out;
/* Otherwise check for weirdness. */
- if (uc) {
+ if (uc)
error = unicrash_check_fs_label(uc, &dsc, label);
- if (error) {
- str_liberror(ctx, error, descr_render(&dsc));
- moveon = false;
- }
- } else
- moveon = xfs_scrub_check_name(ctx, &dsc, _("filesystem label"),
+ else
+ error = simple_check_name(ctx, &dsc, _("filesystem label"),
label);
- if (!moveon)
- goto out;
+ if (error)
+ str_liberror(ctx, error, descr_render(&dsc));
out:
unicrash_free(uc);
- return moveon;
+ return error;
}
/* Check directory connectivity. */
-bool
-xfs_scan_connections(
+int
+phase5_func(
struct scrub_ctx *ctx)
{
- bool moveon = true;
+ bool aborted = false;
int ret;
if (ctx->corruptions_found || ctx->unfixable_errors) {
str_info(ctx, ctx->mntpoint,
_("Filesystem has errors, skipping connectivity checks."));
- return true;
+ return 0;
}
- ret = xfs_scrub_fs_label(ctx);
+ ret = check_fs_label(ctx);
if (ret)
- return false;
+ return ret;
- ret = scrub_scan_all_inodes(ctx, xfs_scrub_connections, &moveon);
+ ret = scrub_scan_all_inodes(ctx, check_inode_names, &aborted);
if (ret)
- return false;
- if (!moveon)
- return false;
+ return ret;
+ if (aborted)
+ return ECANCELED;
+
xfs_scrub_report_preen_triggers(ctx);
- return true;
+ return 0;
+}
+
+bool
+xfs_scan_connections(
+ struct scrub_ctx *ctx)
+{
+ return phase5_func(ctx) == 0;
}
^ permalink raw reply related [flat|nested] 25+ messages in thread
* [PATCH 13/18] xfs_scrub: remove moveon from phase 4 functions
2019-10-22 18:50 [PATCH 00/18] xfs_scrub: remove moveon space aliens Darrick J. Wong
` (11 preceding siblings ...)
2019-10-22 18:51 ` [PATCH 12/18] xfs_scrub: remove moveon from phase 5 functions Darrick J. Wong
@ 2019-10-22 18:51 ` Darrick J. Wong
2019-10-22 18:51 ` [PATCH 14/18] xfs_scrub: remove moveon from phase 3 functions Darrick J. Wong
` (5 subsequent siblings)
18 siblings, 0 replies; 25+ messages in thread
From: Darrick J. Wong @ 2019-10-22 18:51 UTC (permalink / raw)
To: sandeen, darrick.wong; +Cc: linux-xfs
From: Darrick J. Wong <darrick.wong@oracle.com>
Replace the moveon returns in the phase 4 code with a direct integer
error return.
Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
---
scrub/phase4.c | 86 ++++++++++++++++++++++++++++++++------------------------
1 file changed, 49 insertions(+), 37 deletions(-)
diff --git a/scrub/phase4.c b/scrub/phase4.c
index c6de1cd4..f9dcf9c8 100644
--- a/scrub/phase4.c
+++ b/scrub/phase4.c
@@ -22,13 +22,13 @@
/* Fix all the problems in our per-AG list. */
static void
-xfs_repair_ag(
+repair_ag(
struct workqueue *wq,
xfs_agnumber_t agno,
void *priv)
{
struct scrub_ctx *ctx = (struct scrub_ctx *)wq->wq_ctx;
- bool *pmoveon = priv;
+ bool *aborted = priv;
struct action_list *alist;
size_t unfixed;
size_t new_unfixed;
@@ -42,78 +42,73 @@ xfs_repair_ag(
do {
ret = action_list_process(ctx, ctx->mnt.fd, alist, flags);
if (ret) {
- *pmoveon = false;
+ *aborted = true;
return;
}
new_unfixed = action_list_length(alist);
if (new_unfixed == unfixed)
break;
unfixed = new_unfixed;
- } while (unfixed > 0 && *pmoveon);
-
- if (!*pmoveon)
- return;
+ if (*aborted)
+ return;
+ } while (unfixed > 0);
/* Try once more, but this time complain if we can't fix things. */
flags |= ALP_COMPLAIN_IF_UNFIXED;
ret = action_list_process(ctx, ctx->mnt.fd, alist, flags);
if (ret)
- *pmoveon = false;
+ *aborted = true;
}
/* Process all the action items. */
-static bool
-xfs_process_action_items(
+static int
+repair_everything(
struct scrub_ctx *ctx)
{
struct workqueue wq;
xfs_agnumber_t agno;
- bool moveon = true;
+ bool aborted = false;
int ret;
ret = workqueue_create(&wq, (struct xfs_mount *)ctx,
scrub_nproc_workqueue(ctx));
if (ret) {
str_liberror(ctx, ret, _("creating repair workqueue"));
- return false;
+ return ret;
}
- for (agno = 0; agno < ctx->mnt.fsgeom.agcount; agno++) {
- if (action_list_length(&ctx->action_lists[agno]) > 0) {
- ret = workqueue_add(&wq, xfs_repair_ag, agno, &moveon);
- if (ret) {
- moveon = false;
- str_liberror(ctx, ret,
- _("queueing repair work"));
- break;
- }
- }
- if (!moveon)
+ for (agno = 0; !aborted && agno < ctx->mnt.fsgeom.agcount; agno++) {
+ if (action_list_length(&ctx->action_lists[agno]) == 0)
+ continue;
+
+ ret = workqueue_add(&wq, repair_ag, agno, &aborted);
+ if (ret) {
+ str_liberror(ctx, ret, _("queueing repair work"));
break;
+ }
}
ret = workqueue_terminate(&wq);
- if (ret) {
- moveon = false;
+ if (ret)
str_liberror(ctx, ret, _("finishing repair work"));
- }
workqueue_destroy(&wq);
+ if (aborted)
+ return ECANCELED;
+
pthread_mutex_lock(&ctx->lock);
- if (moveon &&
- ctx->corruptions_found == 0 &&
- ctx->unfixable_errors == 0 &&
+ if (ctx->corruptions_found == 0 && ctx->unfixable_errors == 0 &&
want_fstrim) {
fstrim(ctx);
progress_add(1);
}
pthread_mutex_unlock(&ctx->lock);
- return moveon;
+ return 0;
}
/* Fix everything that needs fixing. */
-bool
-xfs_repair_fs(
+int
+phase4_func(
struct scrub_ctx *ctx)
{
int ret;
@@ -126,14 +121,21 @@ xfs_repair_fs(
*/
ret = xfs_scrub_fs_summary(ctx, &ctx->action_lists[0]);
if (ret)
- return false;
+ return ret;
- return xfs_process_action_items(ctx);
+ return repair_everything(ctx);
}
-/* Estimate how much work we're going to do. */
bool
-xfs_estimate_repair_work(
+xfs_repair_fs(
+ struct scrub_ctx *ctx)
+{
+ return phase4_func(ctx) == 0;
+}
+
+/* Estimate how much work we're going to do. */
+int
+phase4_estimate(
struct scrub_ctx *ctx,
uint64_t *items,
unsigned int *nr_threads,
@@ -148,5 +150,15 @@ xfs_estimate_repair_work(
*items = need_fixing;
*nr_threads = scrub_nproc(ctx) + 1;
*rshift = 0;
- return true;
+ return 0;
+}
+
+bool
+xfs_estimate_repair_work(
+ struct scrub_ctx *ctx,
+ uint64_t *items,
+ unsigned int *nr_threads,
+ int *rshift)
+{
+ return phase4_estimate(ctx, items, nr_threads, rshift) == 0;
}
^ permalink raw reply related [flat|nested] 25+ messages in thread
* [PATCH 14/18] xfs_scrub: remove moveon from phase 3 functions
2019-10-22 18:50 [PATCH 00/18] xfs_scrub: remove moveon space aliens Darrick J. Wong
` (12 preceding siblings ...)
2019-10-22 18:51 ` [PATCH 13/18] xfs_scrub: remove moveon from phase 4 functions Darrick J. Wong
@ 2019-10-22 18:51 ` Darrick J. Wong
2019-10-22 18:52 ` [PATCH 15/18] xfs_scrub: remove moveon from phase 2 functions Darrick J. Wong
` (4 subsequent siblings)
18 siblings, 0 replies; 25+ messages in thread
From: Darrick J. Wong @ 2019-10-22 18:51 UTC (permalink / raw)
To: sandeen, darrick.wong; +Cc: linux-xfs
From: Darrick J. Wong <darrick.wong@oracle.com>
Replace the moveon returns in the phase 3 code with a direct integer
error return.
Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
---
scrub/phase3.c | 73 +++++++++++++++++++++++++++++++++++++-------------------
1 file changed, 48 insertions(+), 25 deletions(-)
diff --git a/scrub/phase3.c b/scrub/phase3.c
index b35ef164..eed8f46f 100644
--- a/scrub/phase3.c
+++ b/scrub/phase3.c
@@ -38,12 +38,12 @@ scrub_fd(
struct scrub_inode_ctx {
struct ptcounter *icount;
- bool moveon;
+ bool aborted;
};
/* Report a filesystem error that the vfs fed us on close. */
static void
-xfs_scrub_inode_vfs_error(
+report_close_error(
struct scrub_ctx *ctx,
struct xfs_bulkstat *bstat)
{
@@ -58,7 +58,7 @@ xfs_scrub_inode_vfs_error(
/* Verify the contents, xattrs, and extent maps of an inode. */
static int
-xfs_scrub_inode(
+scrub_inode(
struct scrub_ctx *ctx,
struct xfs_handle *handle,
struct xfs_bulkstat *bstat,
@@ -68,7 +68,6 @@ xfs_scrub_inode(
struct scrub_inode_ctx *ictx = arg;
struct ptcounter *icount = ictx->icount;
xfs_agnumber_t agno;
- bool moveon = true;
int fd = -1;
int error;
@@ -136,61 +135,75 @@ xfs_scrub_inode(
out:
if (error)
- moveon = false;
+ ictx->aborted = true;
+
error = ptcounter_add(icount, 1);
if (error) {
str_liberror(ctx, error,
_("incrementing scanned inode counter"));
- return false;
+ ictx->aborted = true;
}
progress_add(1);
action_list_defer(ctx, agno, &alist);
if (fd >= 0) {
- error = close(fd);
- if (error)
- xfs_scrub_inode_vfs_error(ctx, bstat);
+ int err2;
+
+ err2 = close(fd);
+ if (err2) {
+ report_close_error(ctx, bstat);
+ ictx->aborted = true;
+ }
}
- if (!moveon)
- ictx->moveon = false;
- return ictx->moveon ? 0 : XFS_ITERATE_INODES_ABORT;
+
+ if (!error && ictx->aborted)
+ error = ECANCELED;
+ return error;
}
/* Verify all the inodes in a filesystem. */
-bool
-xfs_scan_inodes(
+int
+phase3_func(
struct scrub_ctx *ctx)
{
- struct scrub_inode_ctx ictx;
+ struct scrub_inode_ctx ictx = { NULL };
uint64_t val;
int err;
- ictx.moveon = true;
err = ptcounter_alloc(scrub_nproc(ctx), &ictx.icount);
if (err) {
str_liberror(ctx, err, _("creating scanned inode counter"));
- return false;
+ return err;
}
- err = scrub_scan_all_inodes(ctx, xfs_scrub_inode, &ictx);
+ err = scrub_scan_all_inodes(ctx, scrub_inode, &ictx);
+ if (!err && ictx.aborted)
+ err = ECANCELED;
if (err)
- ictx.moveon = false;
- if (!ictx.moveon)
goto free;
+
xfs_scrub_report_preen_triggers(ctx);
err = ptcounter_value(ictx.icount, &val);
if (err) {
str_liberror(ctx, err, _("summing scanned inode counter"));
- return false;
+ return err;
}
+
ctx->inodes_checked = val;
free:
ptcounter_free(ictx.icount);
- return ictx.moveon;
+ return err;
}
-/* Estimate how much work we're going to do. */
bool
-xfs_estimate_inodes_work(
+xfs_scan_inodes(
+ struct scrub_ctx *ctx)
+{
+ return phase3_func(ctx) == 0;
+}
+
+/* Estimate how much work we're going to do. */
+int
+phase3_estimate(
struct scrub_ctx *ctx,
uint64_t *items,
unsigned int *nr_threads,
@@ -199,5 +212,15 @@ xfs_estimate_inodes_work(
*items = ctx->mnt_sv.f_files - ctx->mnt_sv.f_ffree;
*nr_threads = scrub_nproc(ctx);
*rshift = 0;
- return true;
+ return 0;
+}
+
+bool
+xfs_estimate_inodes_work(
+ struct scrub_ctx *ctx,
+ uint64_t *items,
+ unsigned int *nr_threads,
+ int *rshift)
+{
+ return phase3_estimate(ctx, items, nr_threads, rshift) == 0;
}
^ permalink raw reply related [flat|nested] 25+ messages in thread
* [PATCH 15/18] xfs_scrub: remove moveon from phase 2 functions
2019-10-22 18:50 [PATCH 00/18] xfs_scrub: remove moveon space aliens Darrick J. Wong
` (13 preceding siblings ...)
2019-10-22 18:51 ` [PATCH 14/18] xfs_scrub: remove moveon from phase 3 functions Darrick J. Wong
@ 2019-10-22 18:52 ` Darrick J. Wong
2019-10-22 18:52 ` [PATCH 16/18] xfs_scrub: remove moveon from phase 1 functions Darrick J. Wong
` (3 subsequent siblings)
18 siblings, 0 replies; 25+ messages in thread
From: Darrick J. Wong @ 2019-10-22 18:52 UTC (permalink / raw)
To: sandeen, darrick.wong; +Cc: linux-xfs
From: Darrick J. Wong <darrick.wong@oracle.com>
Replace the moveon returns in the phase 2 code with a direct integer
error return.
Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
---
scrub/phase2.c | 88 +++++++++++++++++++++++++++++++++++---------------------
1 file changed, 55 insertions(+), 33 deletions(-)
diff --git a/scrub/phase2.c b/scrub/phase2.c
index 7388b8e2..81b2b3dc 100644
--- a/scrub/phase2.c
+++ b/scrub/phase2.c
@@ -19,13 +19,13 @@
/* Scrub each AG's metadata btrees. */
static void
-xfs_scan_ag_metadata(
+scan_ag_metadata(
struct workqueue *wq,
xfs_agnumber_t agno,
void *arg)
{
struct scrub_ctx *ctx = (struct scrub_ctx *)wq->wq_ctx;
- bool *pmoveon = arg;
+ bool *aborted = arg;
struct action_list alist;
struct action_list immediate_alist;
unsigned long long broken_primaries;
@@ -33,6 +33,9 @@ xfs_scan_ag_metadata(
char descr[DESCR_BUFSZ];
int ret;
+ if (*aborted)
+ return;
+
action_list_init(&alist);
action_list_init(&immediate_alist);
snprintf(descr, DESCR_BUFSZ, _("AG %u"), agno);
@@ -84,48 +87,52 @@ _("Filesystem might not be repairable."));
/* Everything else gets fixed during phase 4. */
action_list_defer(ctx, agno, &alist);
-
return;
err:
- *pmoveon = false;
+ *aborted = true;
}
/* Scrub whole-FS metadata btrees. */
static void
-xfs_scan_fs_metadata(
+scan_fs_metadata(
struct workqueue *wq,
xfs_agnumber_t agno,
void *arg)
{
struct scrub_ctx *ctx = (struct scrub_ctx *)wq->wq_ctx;
- bool *pmoveon = arg;
+ bool *aborted = arg;
struct action_list alist;
int ret;
+ if (*aborted)
+ return;
+
action_list_init(&alist);
ret = xfs_scrub_fs_metadata(ctx, &alist);
- if (ret)
- *pmoveon = false;
+ if (ret) {
+ *aborted = true;
+ return;
+ }
action_list_defer(ctx, agno, &alist);
}
/* Scan all filesystem metadata. */
-bool
-xfs_scan_metadata(
+int
+phase2_func(
struct scrub_ctx *ctx)
{
struct action_list alist;
struct workqueue wq;
xfs_agnumber_t agno;
- bool moveon = true;
- int ret;
+ bool aborted = false;
+ int ret, ret2;
ret = workqueue_create(&wq, (struct xfs_mount *)ctx,
scrub_nproc_workqueue(ctx));
if (ret) {
str_liberror(ctx, ret, _("creating scrub workqueue"));
- return false;
+ return ret;
}
/*
@@ -135,48 +142,53 @@ xfs_scan_metadata(
*/
action_list_init(&alist);
ret = xfs_scrub_primary_super(ctx, &alist);
- if (ret) {
- moveon = false;
+ if (ret)
goto out;
- }
ret = action_list_process_or_defer(ctx, 0, &alist);
- if (ret) {
- moveon = false;
+ if (ret)
goto out;
- }
- for (agno = 0; moveon && agno < ctx->mnt.fsgeom.agcount; agno++) {
- ret = workqueue_add(&wq, xfs_scan_ag_metadata, agno, &moveon);
+ for (agno = 0; !aborted && agno < ctx->mnt.fsgeom.agcount; agno++) {
+ ret = workqueue_add(&wq, scan_ag_metadata, agno, &aborted);
if (ret) {
- moveon = false;
str_liberror(ctx, ret, _("queueing per-AG scrub work"));
goto out;
}
}
- if (!moveon)
+ if (aborted)
goto out;
- ret = workqueue_add(&wq, xfs_scan_fs_metadata, 0, &moveon);
+ ret = workqueue_add(&wq, scan_fs_metadata, 0, &aborted);
if (ret) {
- moveon = false;
str_liberror(ctx, ret, _("queueing per-FS scrub work"));
goto out;
}
out:
- ret = workqueue_terminate(&wq);
- if (ret) {
- moveon = false;
- str_liberror(ctx, ret, _("finishing scrub work"));
+ ret2 = workqueue_terminate(&wq);
+ if (ret2) {
+ str_liberror(ctx, ret2, _("finishing scrub work"));
+ if (!ret && ret2)
+ ret = ret2;
}
workqueue_destroy(&wq);
- return moveon;
+
+ if (!ret && aborted)
+ ret = ECANCELED;
+ return ret;
}
-/* Estimate how much work we're going to do. */
bool
-xfs_estimate_metadata_work(
+xfs_scan_metadata(
+ struct scrub_ctx *ctx)
+{
+ return phase2_func(ctx) == 0;
+}
+
+/* Estimate how much work we're going to do. */
+int
+phase2_estimate(
struct scrub_ctx *ctx,
uint64_t *items,
unsigned int *nr_threads,
@@ -185,5 +197,15 @@ xfs_estimate_metadata_work(
*items = scrub_estimate_ag_work(ctx);
*nr_threads = scrub_nproc(ctx);
*rshift = 0;
- return true;
+ return 0;
+}
+
+bool
+xfs_estimate_metadata_work(
+ struct scrub_ctx *ctx,
+ uint64_t *items,
+ unsigned int *nr_threads,
+ int *rshift)
+{
+ return phase2_estimate(ctx, items, nr_threads, rshift) == 0;
}
^ permalink raw reply related [flat|nested] 25+ messages in thread
* [PATCH 16/18] xfs_scrub: remove moveon from phase 1 functions
2019-10-22 18:50 [PATCH 00/18] xfs_scrub: remove moveon space aliens Darrick J. Wong
` (14 preceding siblings ...)
2019-10-22 18:52 ` [PATCH 15/18] xfs_scrub: remove moveon from phase 2 functions Darrick J. Wong
@ 2019-10-22 18:52 ` Darrick J. Wong
2019-10-22 18:52 ` [PATCH 17/18] xfs_scrub: remove XFS_ITERATE_INODES_ABORT from inode iterator Darrick J. Wong
` (2 subsequent siblings)
18 siblings, 0 replies; 25+ messages in thread
From: Darrick J. Wong @ 2019-10-22 18:52 UTC (permalink / raw)
To: sandeen, darrick.wong; +Cc: linux-xfs
From: Darrick J. Wong <darrick.wong@oracle.com>
Replace the moveon returns in the phase 1 code with a direct integer
error return.
Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
---
scrub/phase1.c | 49 ++++++++++++++++++++++++++++---------------------
scrub/xfs_scrub.c | 5 +++--
scrub/xfs_scrub.h | 2 +-
3 files changed, 32 insertions(+), 24 deletions(-)
diff --git a/scrub/phase1.c b/scrub/phase1.c
index 5538deae..75ae3b00 100644
--- a/scrub/phase1.c
+++ b/scrub/phase1.c
@@ -44,8 +44,8 @@ xfs_shutdown_fs(
}
/* Clean up the XFS-specific state data. */
-bool
-xfs_cleanup_fs(
+int
+scrub_cleanup(
struct scrub_ctx *ctx)
{
int error;
@@ -65,15 +65,15 @@ xfs_cleanup_fs(
str_liberror(ctx, error, _("closing mountpoint fd"));
fs_table_destroy();
- return true;
+ return error;
}
/*
* Bind to the mountpoint, read the XFS geometry, bind to the block devices.
- * Anything we've already built will be cleaned up by xfs_cleanup_fs.
+ * Anything we've already built will be cleaned up by scrub_cleanup.
*/
-bool
-xfs_setup_fs(
+int
+phase1_func(
struct scrub_ctx *ctx)
{
int error;
@@ -95,23 +95,23 @@ _("Must be root to run scrub."));
_("Not an XFS filesystem."));
else
str_liberror(ctx, error, ctx->mntpoint);
- return false;
+ return error;
}
error = fstat(ctx->mnt.fd, &ctx->mnt_sb);
if (error) {
str_errno(ctx, ctx->mntpoint);
- return false;
+ return error;
}
error = fstatvfs(ctx->mnt.fd, &ctx->mnt_sv);
if (error) {
str_errno(ctx, ctx->mntpoint);
- return false;
+ return error;
}
error = fstatfs(ctx->mnt.fd, &ctx->mnt_sf);
if (error) {
str_errno(ctx, ctx->mntpoint);
- return false;
+ return error;
}
/*
@@ -122,21 +122,21 @@ _("Not an XFS filesystem."));
error = syncfs(ctx->mnt.fd);
if (error) {
str_errno(ctx, ctx->mntpoint);
- return false;
+ return error;
}
error = action_lists_alloc(ctx->mnt.fsgeom.agcount,
&ctx->action_lists);
if (error) {
str_liberror(ctx, error, _("allocating action lists"));
- return false;
+ return error;
}
error = path_to_fshandle(ctx->mntpoint, &ctx->fshandle,
&ctx->fshandle_len);
if (error) {
str_errno(ctx, _("getting fshandle"));
- return false;
+ return error;
}
/* Do we have kernel-assisted metadata scrubbing? */
@@ -146,33 +146,33 @@ _("Not an XFS filesystem."));
!xfs_can_scrub_parent(ctx)) {
str_error(ctx, ctx->mntpoint,
_("Kernel metadata scrubbing facility is not available."));
- return false;
+ return ECANCELED;
}
/* Do we need kernel-assisted metadata repair? */
if (ctx->mode != SCRUB_MODE_DRY_RUN && !xfs_can_repair(ctx)) {
str_error(ctx, ctx->mntpoint,
_("Kernel metadata repair facility is not available. Use -n to scrub."));
- return false;
+ return ECANCELED;
}
/* Did we find the log and rt devices, if they're present? */
if (ctx->mnt.fsgeom.logstart == 0 && ctx->fsinfo.fs_log == NULL) {
str_error(ctx, ctx->mntpoint,
_("Unable to find log device path."));
- return false;
+ return ECANCELED;
}
if (ctx->mnt.fsgeom.rtblocks && ctx->fsinfo.fs_rt == NULL) {
str_error(ctx, ctx->mntpoint,
_("Unable to find realtime device path."));
- return false;
+ return ECANCELED;
}
/* Open the raw devices. */
ctx->datadev = disk_open(ctx->fsinfo.fs_name);
if (error) {
str_errno(ctx, ctx->fsinfo.fs_name);
- return false;
+ return error;
}
ctx->nr_io_threads = disk_heads(ctx->datadev);
@@ -186,14 +186,14 @@ _("Unable to find realtime device path."));
ctx->logdev = disk_open(ctx->fsinfo.fs_log);
if (error) {
str_errno(ctx, ctx->fsinfo.fs_name);
- return false;
+ return error;
}
}
if (ctx->fsinfo.fs_rt) {
ctx->rtdev = disk_open(ctx->fsinfo.fs_rt);
if (error) {
str_errno(ctx, ctx->fsinfo.fs_name);
- return false;
+ return error;
}
}
@@ -204,5 +204,12 @@ _("Unable to find realtime device path."));
*/
log_info(ctx, _("Invoking online scrub."), ctx);
ctx->scrub_setup_succeeded = true;
- return true;
+ return 0;
+}
+
+bool
+xfs_setup_fs(
+ struct scrub_ctx *ctx)
+{
+ return phase1_func(ctx) == 0;
}
diff --git a/scrub/xfs_scrub.c b/scrub/xfs_scrub.c
index c0e60b92..3751e5af 100644
--- a/scrub/xfs_scrub.c
+++ b/scrub/xfs_scrub.c
@@ -602,6 +602,7 @@ main(
int c;
int fd;
int ret = SCRUB_RET_SUCCESS;
+ int error;
fprintf(stdout, "EXPERIMENTAL xfs_scrub program in use! Use at your own risk!\n");
@@ -776,8 +777,8 @@ main(
str_info(&ctx, ctx.mntpoint, _("Injecting error."));
/* Clean up scan data. */
- moveon = xfs_cleanup_fs(&ctx);
- if (!moveon && ctx.runtime_errors == 0)
+ error = scrub_cleanup(&ctx);
+ if (error && ctx.runtime_errors == 0)
ctx.runtime_errors++;
out:
diff --git a/scrub/xfs_scrub.h b/scrub/xfs_scrub.h
index 88537b0b..5e7f94f5 100644
--- a/scrub/xfs_scrub.h
+++ b/scrub/xfs_scrub.h
@@ -88,7 +88,7 @@ struct scrub_ctx {
/* Phase helper functions */
void xfs_shutdown_fs(struct scrub_ctx *ctx);
-bool xfs_cleanup_fs(struct scrub_ctx *ctx);
+int scrub_cleanup(struct scrub_ctx *ctx);
bool xfs_setup_fs(struct scrub_ctx *ctx);
bool xfs_scan_metadata(struct scrub_ctx *ctx);
bool xfs_scan_inodes(struct scrub_ctx *ctx);
^ permalink raw reply related [flat|nested] 25+ messages in thread
* [PATCH 17/18] xfs_scrub: remove XFS_ITERATE_INODES_ABORT from inode iterator
2019-10-22 18:50 [PATCH 00/18] xfs_scrub: remove moveon space aliens Darrick J. Wong
` (15 preceding siblings ...)
2019-10-22 18:52 ` [PATCH 16/18] xfs_scrub: remove moveon from phase 1 functions Darrick J. Wong
@ 2019-10-22 18:52 ` Darrick J. Wong
2019-10-22 18:52 ` [PATCH 18/18] xfs_scrub: remove moveon from main program Darrick J. Wong
2019-11-06 22:24 ` [PATCH 00/18] xfs_scrub: remove moveon space aliens Eric Sandeen
18 siblings, 0 replies; 25+ messages in thread
From: Darrick J. Wong @ 2019-10-22 18:52 UTC (permalink / raw)
To: sandeen, darrick.wong; +Cc: linux-xfs
From: Darrick J. Wong <darrick.wong@oracle.com>
Remove the _ABORT code since nobody uses it and we're slowly moving to
ECANCELED anyway.
Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
---
scrub/inodes.c | 2 +-
scrub/inodes.h | 5 +++--
2 files changed, 4 insertions(+), 3 deletions(-)
diff --git a/scrub/inodes.c b/scrub/inodes.c
index a2aa6384..7b3284db 100644
--- a/scrub/inodes.c
+++ b/scrub/inodes.c
@@ -187,7 +187,7 @@ scan_ag_inodes(
_("Changed too many times during scan; giving up."));
break;
}
- case XFS_ITERATE_INODES_ABORT:
+ case ECANCELED:
error = 0;
/* fall thru */
default:
diff --git a/scrub/inodes.h b/scrub/inodes.h
index 3affaa00..f0318045 100644
--- a/scrub/inodes.h
+++ b/scrub/inodes.h
@@ -10,12 +10,13 @@
* Visit each space mapping of an inode fork. Return 0 to continue iteration
* or a positive error code to interrupt iteraton. If ESTALE is returned,
* iteration will be restarted from the beginning of the inode allocation
- * group. Any other non zero value will stop iteration.
+ * group. Any other non zero value will stop iteration. The special return
+ * value ECANCELED can be used to stop iteration, because the inode iteration
+ * function never generates that error code on its own.
*/
typedef int (*scrub_inode_iter_fn)(struct scrub_ctx *ctx,
struct xfs_handle *handle, struct xfs_bulkstat *bs, void *arg);
-#define XFS_ITERATE_INODES_ABORT (-1)
int scrub_scan_all_inodes(struct scrub_ctx *ctx, scrub_inode_iter_fn fn,
void *arg);
^ permalink raw reply related [flat|nested] 25+ messages in thread
* [PATCH 18/18] xfs_scrub: remove moveon from main program
2019-10-22 18:50 [PATCH 00/18] xfs_scrub: remove moveon space aliens Darrick J. Wong
` (16 preceding siblings ...)
2019-10-22 18:52 ` [PATCH 17/18] xfs_scrub: remove XFS_ITERATE_INODES_ABORT from inode iterator Darrick J. Wong
@ 2019-10-22 18:52 ` Darrick J. Wong
2019-11-06 21:03 ` [PATCH v2 " Darrick J. Wong
2019-11-06 22:24 ` [PATCH 00/18] xfs_scrub: remove moveon space aliens Eric Sandeen
18 siblings, 1 reply; 25+ messages in thread
From: Darrick J. Wong @ 2019-10-22 18:52 UTC (permalink / raw)
To: sandeen, darrick.wong; +Cc: linux-xfs
From: Darrick J. Wong <darrick.wong@oracle.com>
Replace the moveon returns in xfs_scrub.c to e with a direct integer
error return.
Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
---
scrub/phase1.c | 7 ----
scrub/phase2.c | 17 ---------
scrub/phase3.c | 17 ---------
scrub/phase4.c | 17 ---------
scrub/phase5.c | 15 ++++++--
scrub/phase6.c | 17 ---------
scrub/phase7.c | 7 ----
scrub/xfs_scrub.c | 95 +++++++++++++++++++++++++----------------------------
scrub/xfs_scrub.h | 33 +++++++++---------
9 files changed, 73 insertions(+), 152 deletions(-)
diff --git a/scrub/phase1.c b/scrub/phase1.c
index 75ae3b00..e0382b04 100644
--- a/scrub/phase1.c
+++ b/scrub/phase1.c
@@ -206,10 +206,3 @@ _("Unable to find realtime device path."));
ctx->scrub_setup_succeeded = true;
return 0;
}
-
-bool
-xfs_setup_fs(
- struct scrub_ctx *ctx)
-{
- return phase1_func(ctx) == 0;
-}
diff --git a/scrub/phase2.c b/scrub/phase2.c
index 81b2b3dc..45e0d712 100644
--- a/scrub/phase2.c
+++ b/scrub/phase2.c
@@ -179,13 +179,6 @@ phase2_func(
return ret;
}
-bool
-xfs_scan_metadata(
- struct scrub_ctx *ctx)
-{
- return phase2_func(ctx) == 0;
-}
-
/* Estimate how much work we're going to do. */
int
phase2_estimate(
@@ -199,13 +192,3 @@ phase2_estimate(
*rshift = 0;
return 0;
}
-
-bool
-xfs_estimate_metadata_work(
- struct scrub_ctx *ctx,
- uint64_t *items,
- unsigned int *nr_threads,
- int *rshift)
-{
- return phase2_estimate(ctx, items, nr_threads, rshift) == 0;
-}
diff --git a/scrub/phase3.c b/scrub/phase3.c
index eed8f46f..223f1caf 100644
--- a/scrub/phase3.c
+++ b/scrub/phase3.c
@@ -194,13 +194,6 @@ phase3_func(
return err;
}
-bool
-xfs_scan_inodes(
- struct scrub_ctx *ctx)
-{
- return phase3_func(ctx) == 0;
-}
-
/* Estimate how much work we're going to do. */
int
phase3_estimate(
@@ -214,13 +207,3 @@ phase3_estimate(
*rshift = 0;
return 0;
}
-
-bool
-xfs_estimate_inodes_work(
- struct scrub_ctx *ctx,
- uint64_t *items,
- unsigned int *nr_threads,
- int *rshift)
-{
- return phase3_estimate(ctx, items, nr_threads, rshift) == 0;
-}
diff --git a/scrub/phase4.c b/scrub/phase4.c
index f9dcf9c8..1c1de906 100644
--- a/scrub/phase4.c
+++ b/scrub/phase4.c
@@ -126,13 +126,6 @@ phase4_func(
return repair_everything(ctx);
}
-bool
-xfs_repair_fs(
- struct scrub_ctx *ctx)
-{
- return phase4_func(ctx) == 0;
-}
-
/* Estimate how much work we're going to do. */
int
phase4_estimate(
@@ -152,13 +145,3 @@ phase4_estimate(
*rshift = 0;
return 0;
}
-
-bool
-xfs_estimate_repair_work(
- struct scrub_ctx *ctx,
- uint64_t *items,
- unsigned int *nr_threads,
- int *rshift)
-{
- return phase4_estimate(ctx, items, nr_threads, rshift) == 0;
-}
diff --git a/scrub/phase5.c b/scrub/phase5.c
index 2641a7fb..540b840d 100644
--- a/scrub/phase5.c
+++ b/scrub/phase5.c
@@ -406,9 +406,16 @@ _("Filesystem has errors, skipping connectivity checks."));
return 0;
}
-bool
-xfs_scan_connections(
- struct scrub_ctx *ctx)
+/* Estimate how much work we're going to do. */
+int
+phase5_estimate(
+ struct scrub_ctx *ctx,
+ uint64_t *items,
+ unsigned int *nr_threads,
+ int *rshift)
{
- return phase5_func(ctx) == 0;
+ *items = ctx->mnt_sv.f_files - ctx->mnt_sv.f_ffree;
+ *nr_threads = scrub_nproc(ctx);
+ *rshift = 0;
+ return 0;
}
diff --git a/scrub/phase6.c b/scrub/phase6.c
index 2aa3fd10..6dbadcee 100644
--- a/scrub/phase6.c
+++ b/scrub/phase6.c
@@ -756,13 +756,6 @@ phase6_func(
return ret;
}
-bool
-xfs_scan_blocks(
- struct scrub_ctx *ctx)
-{
- return phase6_func(ctx) == 0;
-}
-
/* Estimate how much work we're going to do. */
int
phase6_estimate(
@@ -793,13 +786,3 @@ phase6_estimate(
*rshift = 20;
return 0;
}
-
-bool
-xfs_estimate_verify_work(
- struct scrub_ctx *ctx,
- uint64_t *items,
- unsigned int *nr_threads,
- int *rshift)
-{
- return phase6_estimate(ctx, items, nr_threads, rshift) == 0;
-}
diff --git a/scrub/phase7.c b/scrub/phase7.c
index ff51c634..f25a8765 100644
--- a/scrub/phase7.c
+++ b/scrub/phase7.c
@@ -278,10 +278,3 @@ _("%.1f%s data counted; %.1f%s file data media verified.\n"),
ptvar_free(ptvar);
return error;
}
-
-bool
-xfs_scan_summary(
- struct scrub_ctx *ctx)
-{
- return phase7_func(ctx) == 0;
-}
diff --git a/scrub/xfs_scrub.c b/scrub/xfs_scrub.c
index 3751e5af..55645370 100644
--- a/scrub/xfs_scrub.c
+++ b/scrub/xfs_scrub.c
@@ -245,14 +245,14 @@ struct phase_rusage {
#define REPAIR_DUMMY_FN ((void *)2)
struct phase_ops {
char *descr;
- bool (*fn)(struct scrub_ctx *);
- bool (*estimate_work)(struct scrub_ctx *, uint64_t *,
- unsigned int *, int *);
+ int (*fn)(struct scrub_ctx *ctx);
+ int (*estimate_work)(struct scrub_ctx *ctx, uint64_t *items,
+ unsigned int *threads, int *rshift);
bool must_run;
};
/* Start tracking resource usage for a phase. */
-static bool
+static int
phase_start(
struct phase_rusage *pi,
unsigned int phase,
@@ -264,14 +264,14 @@ phase_start(
error = scrub_getrusage(&pi->ruse);
if (error) {
perror(_("getrusage"));
- return false;
+ return error;
}
pi->brk_start = sbrk(0);
error = gettimeofday(&pi->time, NULL);
if (error) {
perror(_("gettimeofday"));
- return false;
+ return error;
}
pi->descr = descr;
@@ -279,11 +279,11 @@ phase_start(
fprintf(stdout, _("Phase %u: %s\n"), phase, descr);
fflush(stdout);
}
- return true;
+ return error;
}
/* Report usage stats. */
-static bool
+static int
phase_end(
struct phase_rusage *pi,
unsigned int phase)
@@ -303,19 +303,19 @@ phase_end(
int error;
if (!display_rusage)
- return true;
+ return 0;
error = gettimeofday(&time_now, NULL);
if (error) {
perror(_("gettimeofday"));
- return false;
+ return error;
}
dt = timeval_subtract(&time_now, &pi->time);
error = scrub_getrusage(&ruse_now);
if (error) {
perror(_("getrusage"));
- return false;
+ return error;
}
if (phase)
@@ -366,7 +366,7 @@ _("%sI/O rate: %.1f%s/s in, %.1f%s/s out, %.1f%s/s tot\n"),
}
fflush(stdout);
- return true;
+ return 0;
}
/* Run all the phases of the scrubber. */
@@ -379,37 +379,37 @@ run_scrub_phases(
{
{
.descr = _("Find filesystem geometry."),
- .fn = xfs_setup_fs,
+ .fn = phase1_func,
.must_run = true,
},
{
.descr = _("Check internal metadata."),
- .fn = xfs_scan_metadata,
- .estimate_work = xfs_estimate_metadata_work,
+ .fn = phase2_func,
+ .estimate_work = phase2_estimate,
},
{
.descr = _("Scan all inodes."),
- .fn = xfs_scan_inodes,
- .estimate_work = xfs_estimate_inodes_work,
+ .fn = phase3_func,
+ .estimate_work = phase3_estimate,
},
{
.descr = _("Defer filesystem repairs."),
.fn = REPAIR_DUMMY_FN,
- .estimate_work = xfs_estimate_repair_work,
+ .estimate_work = phase4_estimate,
},
{
.descr = _("Check directory tree."),
- .fn = xfs_scan_connections,
- .estimate_work = xfs_estimate_inodes_work,
+ .fn = phase5_func,
+ .estimate_work = phase5_estimate,
},
{
.descr = _("Verify data file integrity."),
.fn = DATASCAN_DUMMY_FN,
- .estimate_work = xfs_estimate_verify_work,
+ .estimate_work = phase6_estimate,
},
{
.descr = _("Check summary counters."),
- .fn = xfs_scan_summary,
+ .fn = phase7_func,
.must_run = true,
},
{
@@ -419,7 +419,6 @@ run_scrub_phases(
struct phase_rusage pi;
struct phase_ops *sp;
uint64_t max_work;
- bool moveon = true;
unsigned int debug_phase = 0;
unsigned int phase;
int rshift;
@@ -432,13 +431,13 @@ run_scrub_phases(
for (phase = 1, sp = phases; sp->fn; sp++, phase++) {
/* Turn on certain phases if user said to. */
if (sp->fn == DATASCAN_DUMMY_FN && scrub_data) {
- sp->fn = xfs_scan_blocks;
+ sp->fn = phase6_func;
if (scrub_data > 1)
sp->descr = _("Verify disk integrity.");
} else if (sp->fn == REPAIR_DUMMY_FN &&
ctx->mode == SCRUB_MODE_REPAIR) {
sp->descr = _("Repair filesystem.");
- sp->fn = xfs_repair_fs;
+ sp->fn = phase4_func;
sp->must_run = true;
}
@@ -452,15 +451,15 @@ run_scrub_phases(
continue;
/* Run this phase. */
- moveon = phase_start(&pi, phase, sp->descr);
- if (!moveon)
+ ret = phase_start(&pi, phase, sp->descr);
+ if (ret)
break;
if (sp->estimate_work) {
unsigned int work_threads;
- moveon = sp->estimate_work(ctx, &max_work,
+ ret = sp->estimate_work(ctx, &max_work,
&work_threads, &rshift);
- if (!moveon)
+ if (ret)
break;
/*
@@ -471,23 +470,19 @@ run_scrub_phases(
work_threads++;
ret = progress_init_phase(ctx, progress_fp, phase,
max_work, rshift, work_threads);
- if (ret) {
- moveon = false;
+ if (ret)
break;
- }
- moveon = descr_init_phase(ctx, work_threads) == 0;
+ ret = descr_init_phase(ctx, work_threads);
} else {
ret = progress_init_phase(ctx, NULL, phase, 0, 0, 0);
- if (ret) {
- moveon = false;
+ if (ret)
break;
- }
- moveon = descr_init_phase(ctx, 1) == 0;
+ ret = descr_init_phase(ctx, 1);
}
- if (!moveon)
+ if (ret)
break;
- moveon = sp->fn(ctx);
- if (!moveon) {
+ ret = sp->fn(ctx);
+ if (ret) {
str_info(ctx, ctx->mntpoint,
_("Scrub aborted after phase %d."),
phase);
@@ -495,17 +490,18 @@ _("Scrub aborted after phase %d."),
}
progress_end_phase();
descr_end_phase();
- moveon = phase_end(&pi, phase);
- if (!moveon)
+ ret = phase_end(&pi, phase);
+ if (ret)
break;
/* Too many errors? */
- moveon = !xfs_scrub_excessive_errors(ctx);
- if (!moveon)
+ if (xfs_scrub_excessive_errors(ctx)) {
+ ret = ECANCELED;
break;
+ }
}
- return moveon;
+ return ret;
}
static void
@@ -598,7 +594,6 @@ main(
char *mtab = NULL;
FILE *progress_fp = NULL;
struct fs_path *fsp;
- bool moveon = true;
int c;
int fd;
int ret = SCRUB_RET_SUCCESS;
@@ -713,8 +708,8 @@ main(
is_service = true;
/* Initialize overall phase stats. */
- moveon = phase_start(&all_pi, 0, NULL);
- if (!moveon)
+ error = phase_start(&all_pi, 0, NULL);
+ if (error)
return SCRUB_RET_OPERROR;
/* Find the mount record for the passed-in argument. */
@@ -761,8 +756,8 @@ main(
ctx.mode = SCRUB_MODE_REPAIR;
/* Scrub a filesystem. */
- moveon = run_scrub_phases(&ctx, progress_fp);
- if (!moveon && ctx.runtime_errors == 0)
+ error = run_scrub_phases(&ctx, progress_fp);
+ if (error && ctx.runtime_errors == 0)
ctx.runtime_errors++;
/*
diff --git a/scrub/xfs_scrub.h b/scrub/xfs_scrub.h
index 5e7f94f5..36b5bbdc 100644
--- a/scrub/xfs_scrub.h
+++ b/scrub/xfs_scrub.h
@@ -89,24 +89,25 @@ struct scrub_ctx {
/* Phase helper functions */
void xfs_shutdown_fs(struct scrub_ctx *ctx);
int scrub_cleanup(struct scrub_ctx *ctx);
-bool xfs_setup_fs(struct scrub_ctx *ctx);
-bool xfs_scan_metadata(struct scrub_ctx *ctx);
-bool xfs_scan_inodes(struct scrub_ctx *ctx);
-bool xfs_scan_connections(struct scrub_ctx *ctx);
-bool xfs_scan_blocks(struct scrub_ctx *ctx);
-bool xfs_scan_summary(struct scrub_ctx *ctx);
-bool xfs_repair_fs(struct scrub_ctx *ctx);
+int phase1_func(struct scrub_ctx *ctx);
+int phase2_func(struct scrub_ctx *ctx);
+int phase3_func(struct scrub_ctx *ctx);
+int phase4_func(struct scrub_ctx *ctx);
+int phase5_func(struct scrub_ctx *ctx);
+int phase6_func(struct scrub_ctx *ctx);
+int phase7_func(struct scrub_ctx *ctx);
/* Progress estimator functions */
-uint64_t xfs_estimate_inodes(struct scrub_ctx *ctx);
unsigned int scrub_estimate_ag_work(struct scrub_ctx *ctx);
-bool xfs_estimate_metadata_work(struct scrub_ctx *ctx, uint64_t *items,
- unsigned int *nr_threads, int *rshift);
-bool xfs_estimate_inodes_work(struct scrub_ctx *ctx, uint64_t *items,
- unsigned int *nr_threads, int *rshift);
-bool xfs_estimate_repair_work(struct scrub_ctx *ctx, uint64_t *items,
- unsigned int *nr_threads, int *rshift);
-bool xfs_estimate_verify_work(struct scrub_ctx *ctx, uint64_t *items,
- unsigned int *nr_threads, int *rshift);
+int phase2_estimate(struct scrub_ctx *ctx, uint64_t *items,
+ unsigned int *nr_threads, int *rshift);
+int phase3_estimate(struct scrub_ctx *ctx, uint64_t *items,
+ unsigned int *nr_threads, int *rshift);
+int phase4_estimate(struct scrub_ctx *ctx, uint64_t *items,
+ unsigned int *nr_threads, int *rshift);
+int phase5_estimate(struct scrub_ctx *ctx, uint64_t *items,
+ unsigned int *nr_threads, int *rshift);
+int phase6_estimate(struct scrub_ctx *ctx, uint64_t *items,
+ unsigned int *nr_threads, int *rshift);
#endif /* XFS_SCRUB_XFS_SCRUB_H_ */
^ permalink raw reply related [flat|nested] 25+ messages in thread
* [PATCH v2 18/18] xfs_scrub: remove moveon from main program
2019-10-22 18:52 ` [PATCH 18/18] xfs_scrub: remove moveon from main program Darrick J. Wong
@ 2019-11-06 21:03 ` Darrick J. Wong
0 siblings, 0 replies; 25+ messages in thread
From: Darrick J. Wong @ 2019-11-06 21:03 UTC (permalink / raw)
To: sandeen; +Cc: linux-xfs
From: Darrick J. Wong <darrick.wong@oracle.com>
Replace the moveon returns in xfs_scrub.c to e with a direct integer
error return.
Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
---
v2: rebase without full-disk media scan
---
scrub/phase1.c | 7 ----
scrub/phase2.c | 17 ---------
scrub/phase3.c | 17 ---------
scrub/phase4.c | 17 ---------
scrub/phase5.c | 15 ++++++--
scrub/phase6.c | 17 ---------
scrub/phase7.c | 7 ----
scrub/xfs_scrub.c | 95 +++++++++++++++++++++++++----------------------------
scrub/xfs_scrub.h | 33 +++++++++---------
9 files changed, 73 insertions(+), 152 deletions(-)
diff --git a/scrub/phase1.c b/scrub/phase1.c
index 75ae3b00..e0382b04 100644
--- a/scrub/phase1.c
+++ b/scrub/phase1.c
@@ -206,10 +206,3 @@ _("Unable to find realtime device path."));
ctx->scrub_setup_succeeded = true;
return 0;
}
-
-bool
-xfs_setup_fs(
- struct scrub_ctx *ctx)
-{
- return phase1_func(ctx) == 0;
-}
diff --git a/scrub/phase2.c b/scrub/phase2.c
index 81b2b3dc..45e0d712 100644
--- a/scrub/phase2.c
+++ b/scrub/phase2.c
@@ -179,13 +179,6 @@ phase2_func(
return ret;
}
-bool
-xfs_scan_metadata(
- struct scrub_ctx *ctx)
-{
- return phase2_func(ctx) == 0;
-}
-
/* Estimate how much work we're going to do. */
int
phase2_estimate(
@@ -199,13 +192,3 @@ phase2_estimate(
*rshift = 0;
return 0;
}
-
-bool
-xfs_estimate_metadata_work(
- struct scrub_ctx *ctx,
- uint64_t *items,
- unsigned int *nr_threads,
- int *rshift)
-{
- return phase2_estimate(ctx, items, nr_threads, rshift) == 0;
-}
diff --git a/scrub/phase3.c b/scrub/phase3.c
index eed8f46f..223f1caf 100644
--- a/scrub/phase3.c
+++ b/scrub/phase3.c
@@ -194,13 +194,6 @@ phase3_func(
return err;
}
-bool
-xfs_scan_inodes(
- struct scrub_ctx *ctx)
-{
- return phase3_func(ctx) == 0;
-}
-
/* Estimate how much work we're going to do. */
int
phase3_estimate(
@@ -214,13 +207,3 @@ phase3_estimate(
*rshift = 0;
return 0;
}
-
-bool
-xfs_estimate_inodes_work(
- struct scrub_ctx *ctx,
- uint64_t *items,
- unsigned int *nr_threads,
- int *rshift)
-{
- return phase3_estimate(ctx, items, nr_threads, rshift) == 0;
-}
diff --git a/scrub/phase4.c b/scrub/phase4.c
index f9dcf9c8..1c1de906 100644
--- a/scrub/phase4.c
+++ b/scrub/phase4.c
@@ -126,13 +126,6 @@ phase4_func(
return repair_everything(ctx);
}
-bool
-xfs_repair_fs(
- struct scrub_ctx *ctx)
-{
- return phase4_func(ctx) == 0;
-}
-
/* Estimate how much work we're going to do. */
int
phase4_estimate(
@@ -152,13 +145,3 @@ phase4_estimate(
*rshift = 0;
return 0;
}
-
-bool
-xfs_estimate_repair_work(
- struct scrub_ctx *ctx,
- uint64_t *items,
- unsigned int *nr_threads,
- int *rshift)
-{
- return phase4_estimate(ctx, items, nr_threads, rshift) == 0;
-}
diff --git a/scrub/phase5.c b/scrub/phase5.c
index 2641a7fb..540b840d 100644
--- a/scrub/phase5.c
+++ b/scrub/phase5.c
@@ -406,9 +406,16 @@ _("Filesystem has errors, skipping connectivity checks."));
return 0;
}
-bool
-xfs_scan_connections(
- struct scrub_ctx *ctx)
+/* Estimate how much work we're going to do. */
+int
+phase5_estimate(
+ struct scrub_ctx *ctx,
+ uint64_t *items,
+ unsigned int *nr_threads,
+ int *rshift)
{
- return phase5_func(ctx) == 0;
+ *items = ctx->mnt_sv.f_files - ctx->mnt_sv.f_ffree;
+ *nr_threads = scrub_nproc(ctx);
+ *rshift = 0;
+ return 0;
}
diff --git a/scrub/phase6.c b/scrub/phase6.c
index 020b303d..88c719b3 100644
--- a/scrub/phase6.c
+++ b/scrub/phase6.c
@@ -707,13 +707,6 @@ phase6_func(
return ret;
}
-bool
-xfs_scan_blocks(
- struct scrub_ctx *ctx)
-{
- return phase6_func(ctx) == 0;
-}
-
/* Estimate how much work we're going to do. */
int
phase6_estimate(
@@ -743,13 +736,3 @@ phase6_estimate(
*rshift = 20;
return 0;
}
-
-bool
-xfs_estimate_verify_work(
- struct scrub_ctx *ctx,
- uint64_t *items,
- unsigned int *nr_threads,
- int *rshift)
-{
- return phase6_estimate(ctx, items, nr_threads, rshift) == 0;
-}
diff --git a/scrub/phase7.c b/scrub/phase7.c
index 68912f03..191f2e4f 100644
--- a/scrub/phase7.c
+++ b/scrub/phase7.c
@@ -270,10 +270,3 @@ _("%.1f%s data counted; %.1f%s data verified.\n"),
ptvar_free(ptvar);
return error;
}
-
-bool
-xfs_scan_summary(
- struct scrub_ctx *ctx)
-{
- return phase7_func(ctx) == 0;
-}
diff --git a/scrub/xfs_scrub.c b/scrub/xfs_scrub.c
index 5030a7fa..a792768c 100644
--- a/scrub/xfs_scrub.c
+++ b/scrub/xfs_scrub.c
@@ -245,14 +245,14 @@ struct phase_rusage {
#define REPAIR_DUMMY_FN ((void *)2)
struct phase_ops {
char *descr;
- bool (*fn)(struct scrub_ctx *);
- bool (*estimate_work)(struct scrub_ctx *, uint64_t *,
- unsigned int *, int *);
+ int (*fn)(struct scrub_ctx *ctx);
+ int (*estimate_work)(struct scrub_ctx *ctx, uint64_t *items,
+ unsigned int *threads, int *rshift);
bool must_run;
};
/* Start tracking resource usage for a phase. */
-static bool
+static int
phase_start(
struct phase_rusage *pi,
unsigned int phase,
@@ -264,14 +264,14 @@ phase_start(
error = scrub_getrusage(&pi->ruse);
if (error) {
perror(_("getrusage"));
- return false;
+ return error;
}
pi->brk_start = sbrk(0);
error = gettimeofday(&pi->time, NULL);
if (error) {
perror(_("gettimeofday"));
- return false;
+ return error;
}
pi->descr = descr;
@@ -279,11 +279,11 @@ phase_start(
fprintf(stdout, _("Phase %u: %s\n"), phase, descr);
fflush(stdout);
}
- return true;
+ return error;
}
/* Report usage stats. */
-static bool
+static int
phase_end(
struct phase_rusage *pi,
unsigned int phase)
@@ -303,19 +303,19 @@ phase_end(
int error;
if (!display_rusage)
- return true;
+ return 0;
error = gettimeofday(&time_now, NULL);
if (error) {
perror(_("gettimeofday"));
- return false;
+ return error;
}
dt = timeval_subtract(&time_now, &pi->time);
error = scrub_getrusage(&ruse_now);
if (error) {
perror(_("getrusage"));
- return false;
+ return error;
}
if (phase)
@@ -366,7 +366,7 @@ _("%sI/O rate: %.1f%s/s in, %.1f%s/s out, %.1f%s/s tot\n"),
}
fflush(stdout);
- return true;
+ return 0;
}
/* Run all the phases of the scrubber. */
@@ -379,37 +379,37 @@ run_scrub_phases(
{
{
.descr = _("Find filesystem geometry."),
- .fn = xfs_setup_fs,
+ .fn = phase1_func,
.must_run = true,
},
{
.descr = _("Check internal metadata."),
- .fn = xfs_scan_metadata,
- .estimate_work = xfs_estimate_metadata_work,
+ .fn = phase2_func,
+ .estimate_work = phase2_estimate,
},
{
.descr = _("Scan all inodes."),
- .fn = xfs_scan_inodes,
- .estimate_work = xfs_estimate_inodes_work,
+ .fn = phase3_func,
+ .estimate_work = phase3_estimate,
},
{
.descr = _("Defer filesystem repairs."),
.fn = REPAIR_DUMMY_FN,
- .estimate_work = xfs_estimate_repair_work,
+ .estimate_work = phase4_estimate,
},
{
.descr = _("Check directory tree."),
- .fn = xfs_scan_connections,
- .estimate_work = xfs_estimate_inodes_work,
+ .fn = phase5_func,
+ .estimate_work = phase5_estimate,
},
{
.descr = _("Verify data file integrity."),
.fn = DATASCAN_DUMMY_FN,
- .estimate_work = xfs_estimate_verify_work,
+ .estimate_work = phase6_estimate,
},
{
.descr = _("Check summary counters."),
- .fn = xfs_scan_summary,
+ .fn = phase7_func,
.must_run = true,
},
{
@@ -419,7 +419,6 @@ run_scrub_phases(
struct phase_rusage pi;
struct phase_ops *sp;
uint64_t max_work;
- bool moveon = true;
unsigned int debug_phase = 0;
unsigned int phase;
int rshift;
@@ -432,11 +431,11 @@ run_scrub_phases(
for (phase = 1, sp = phases; sp->fn; sp++, phase++) {
/* Turn on certain phases if user said to. */
if (sp->fn == DATASCAN_DUMMY_FN && scrub_data) {
- sp->fn = xfs_scan_blocks;
+ sp->fn = phase6_func;
} else if (sp->fn == REPAIR_DUMMY_FN &&
ctx->mode == SCRUB_MODE_REPAIR) {
sp->descr = _("Repair filesystem.");
- sp->fn = xfs_repair_fs;
+ sp->fn = phase4_func;
sp->must_run = true;
}
@@ -450,15 +449,15 @@ run_scrub_phases(
continue;
/* Run this phase. */
- moveon = phase_start(&pi, phase, sp->descr);
- if (!moveon)
+ ret = phase_start(&pi, phase, sp->descr);
+ if (ret)
break;
if (sp->estimate_work) {
unsigned int work_threads;
- moveon = sp->estimate_work(ctx, &max_work,
+ ret = sp->estimate_work(ctx, &max_work,
&work_threads, &rshift);
- if (!moveon)
+ if (ret)
break;
/*
@@ -469,23 +468,19 @@ run_scrub_phases(
work_threads++;
ret = progress_init_phase(ctx, progress_fp, phase,
max_work, rshift, work_threads);
- if (ret) {
- moveon = false;
+ if (ret)
break;
- }
- moveon = descr_init_phase(ctx, work_threads) == 0;
+ ret = descr_init_phase(ctx, work_threads);
} else {
ret = progress_init_phase(ctx, NULL, phase, 0, 0, 0);
- if (ret) {
- moveon = false;
+ if (ret)
break;
- }
- moveon = descr_init_phase(ctx, 1) == 0;
+ ret = descr_init_phase(ctx, 1);
}
- if (!moveon)
+ if (ret)
break;
- moveon = sp->fn(ctx);
- if (!moveon) {
+ ret = sp->fn(ctx);
+ if (ret) {
str_info(ctx, ctx->mntpoint,
_("Scrub aborted after phase %d."),
phase);
@@ -493,17 +488,18 @@ _("Scrub aborted after phase %d."),
}
progress_end_phase();
descr_end_phase();
- moveon = phase_end(&pi, phase);
- if (!moveon)
+ ret = phase_end(&pi, phase);
+ if (ret)
break;
/* Too many errors? */
- moveon = !xfs_scrub_excessive_errors(ctx);
- if (!moveon)
+ if (xfs_scrub_excessive_errors(ctx)) {
+ ret = ECANCELED;
break;
+ }
}
- return moveon;
+ return ret;
}
static void
@@ -596,7 +592,6 @@ main(
char *mtab = NULL;
FILE *progress_fp = NULL;
struct fs_path *fsp;
- bool moveon = true;
int c;
int fd;
int ret = SCRUB_RET_SUCCESS;
@@ -711,8 +706,8 @@ main(
is_service = true;
/* Initialize overall phase stats. */
- moveon = phase_start(&all_pi, 0, NULL);
- if (!moveon)
+ error = phase_start(&all_pi, 0, NULL);
+ if (error)
return SCRUB_RET_OPERROR;
/* Find the mount record for the passed-in argument. */
@@ -759,8 +754,8 @@ main(
ctx.mode = SCRUB_MODE_REPAIR;
/* Scrub a filesystem. */
- moveon = run_scrub_phases(&ctx, progress_fp);
- if (!moveon && ctx.runtime_errors == 0)
+ error = run_scrub_phases(&ctx, progress_fp);
+ if (error && ctx.runtime_errors == 0)
ctx.runtime_errors++;
/*
diff --git a/scrub/xfs_scrub.h b/scrub/xfs_scrub.h
index 10683416..f6712d36 100644
--- a/scrub/xfs_scrub.h
+++ b/scrub/xfs_scrub.h
@@ -88,24 +88,25 @@ struct scrub_ctx {
/* Phase helper functions */
void xfs_shutdown_fs(struct scrub_ctx *ctx);
int scrub_cleanup(struct scrub_ctx *ctx);
-bool xfs_setup_fs(struct scrub_ctx *ctx);
-bool xfs_scan_metadata(struct scrub_ctx *ctx);
-bool xfs_scan_inodes(struct scrub_ctx *ctx);
-bool xfs_scan_connections(struct scrub_ctx *ctx);
-bool xfs_scan_blocks(struct scrub_ctx *ctx);
-bool xfs_scan_summary(struct scrub_ctx *ctx);
-bool xfs_repair_fs(struct scrub_ctx *ctx);
+int phase1_func(struct scrub_ctx *ctx);
+int phase2_func(struct scrub_ctx *ctx);
+int phase3_func(struct scrub_ctx *ctx);
+int phase4_func(struct scrub_ctx *ctx);
+int phase5_func(struct scrub_ctx *ctx);
+int phase6_func(struct scrub_ctx *ctx);
+int phase7_func(struct scrub_ctx *ctx);
/* Progress estimator functions */
-uint64_t xfs_estimate_inodes(struct scrub_ctx *ctx);
unsigned int scrub_estimate_ag_work(struct scrub_ctx *ctx);
-bool xfs_estimate_metadata_work(struct scrub_ctx *ctx, uint64_t *items,
- unsigned int *nr_threads, int *rshift);
-bool xfs_estimate_inodes_work(struct scrub_ctx *ctx, uint64_t *items,
- unsigned int *nr_threads, int *rshift);
-bool xfs_estimate_repair_work(struct scrub_ctx *ctx, uint64_t *items,
- unsigned int *nr_threads, int *rshift);
-bool xfs_estimate_verify_work(struct scrub_ctx *ctx, uint64_t *items,
- unsigned int *nr_threads, int *rshift);
+int phase2_estimate(struct scrub_ctx *ctx, uint64_t *items,
+ unsigned int *nr_threads, int *rshift);
+int phase3_estimate(struct scrub_ctx *ctx, uint64_t *items,
+ unsigned int *nr_threads, int *rshift);
+int phase4_estimate(struct scrub_ctx *ctx, uint64_t *items,
+ unsigned int *nr_threads, int *rshift);
+int phase5_estimate(struct scrub_ctx *ctx, uint64_t *items,
+ unsigned int *nr_threads, int *rshift);
+int phase6_estimate(struct scrub_ctx *ctx, uint64_t *items,
+ unsigned int *nr_threads, int *rshift);
#endif /* XFS_SCRUB_XFS_SCRUB_H_ */
^ permalink raw reply related [flat|nested] 25+ messages in thread
* Re: [PATCH 00/18] xfs_scrub: remove moveon space aliens
2019-10-22 18:50 [PATCH 00/18] xfs_scrub: remove moveon space aliens Darrick J. Wong
` (17 preceding siblings ...)
2019-10-22 18:52 ` [PATCH 18/18] xfs_scrub: remove moveon from main program Darrick J. Wong
@ 2019-11-06 22:24 ` Eric Sandeen
18 siblings, 0 replies; 25+ messages in thread
From: Eric Sandeen @ 2019-11-06 22:24 UTC (permalink / raw)
To: Darrick J. Wong; +Cc: linux-xfs
On 10/22/19 1:50 PM, Darrick J. Wong wrote:
> Hi all,
>
> This series replaces the unfamiliar boolean variable return values in
> xfs_scrub with the somewhat more customary "positive error value or
> zero" semantics that are used throughout the rest of xfsprogs.
>
> If you're going to start using this mess, you probably ought to just
> pull from my git trees, which are linked below.
>
> This is an extraordinary way to destroy everything. Enjoy!
> Comments and questions are, as always, welcome.
>
> --D
I had to slightly adjust a couple more patches but modulo eyes glazing
over a bit, this looks ok so for the whole series:
Reviewed-by: Eric Sandeen <sandeen@redhat.com>
^ permalink raw reply [flat|nested] 25+ messages in thread