* [PATCH 00/18] xfs_scrub: remove moveon space aliens
@ 2019-09-25 21:38 Darrick J. Wong
2019-09-25 21:38 ` [PATCH 01/18] xfs_scrub: remove moveon from filemap iteration Darrick J. Wong
` (17 more replies)
0 siblings, 18 replies; 21+ messages in thread
From: Darrick J. Wong @ 2019-09-25 21:38 UTC (permalink / raw)
To: sandeen, darrick.wong; +Cc: linux-xfs
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
xfsprogs git tree:
https://git.kernel.org/cgit/linux/kernel/git/djwong/xfsprogs-dev.git/log/?h=scrub-remove-moveon
^ permalink raw reply [flat|nested] 21+ messages in thread
* [PATCH 01/18] xfs_scrub: remove moveon from filemap iteration
2019-09-25 21:38 [PATCH 00/18] xfs_scrub: remove moveon space aliens Darrick J. Wong
@ 2019-09-25 21:38 ` Darrick J. Wong
2019-09-25 21:38 ` [PATCH 02/18] xfs_scrub: remove moveon from the fscounters functions Darrick J. Wong
` (16 subsequent siblings)
17 siblings, 0 replies; 21+ messages in thread
From: Darrick J. Wong @ 2019-09-25 21:38 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 | 70 ++++++++++++++++-----------------------------
scrub/filemap.h | 12 +++-----
scrub/phase6.c | 86 ++++++++++++++++++++++++++++++-------------------------
3 files changed, 77 insertions(+), 91 deletions(-)
diff --git a/scrub/filemap.c b/scrub/filemap.c
index aaaa0521..9d157bab 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,28 +73,25 @@ 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;
}
+
map->bmv_count = min(fsx.fsx_nextents + 2, 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)
@@ -123,17 +104,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 cb331729..c2fd9cb0 100644
--- a/scrub/filemap.h
+++ b/scrub/filemap.h
@@ -7,19 +7,17 @@
#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 */
uint32_t bm_flags; /* output flags */
};
-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 7bfb856a..3c9eec09 100644
--- a/scrub/phase6.c
+++ b/scrub/phase6.c
@@ -112,9 +112,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. */
@@ -147,77 +148,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_error(ctx, descr,
+ str_error(ctx, br->descr,
_("media error in extended attribute data."));
- return true;
+ return 0;
}
/* Iterate the extent mappings of a file to report errors. */
@@ -228,18 +220,34 @@ xfs_report_verify_fd(
int fd,
void *arg)
{
- struct xfs_bmap key = {0};
- bool moveon;
+ struct badfile_report br = {
+ .ctx = ctx,
+ .vs = arg,
+ };
+ struct file_bmap key = {0};
+ char bmap_descr[DESCR_BUFSZ];
+ int ret;
+
+ br.descr = bmap_descr;
/* data fork */
- moveon = xfs_iterate_filemaps(ctx, descr, fd, XFS_DATA_FORK, &key,
- report_data_loss, arg);
- if (!moveon)
+ snprintf(bmap_descr, DESCR_BUFSZ, _("%s data"), descr);
+ ret = scrub_iterate_filemaps(ctx, fd, XFS_DATA_FORK, &key,
+ report_data_loss, &br);
+ if (ret) {
+ str_liberror(ctx, ret, bmap_descr);
return false;
+ }
/* attr fork */
- return xfs_iterate_filemaps(ctx, descr, fd, XFS_ATTR_FORK, &key,
- report_attr_loss, arg);
+ snprintf(bmap_descr, DESCR_BUFSZ, _("%s attr"), descr);
+ ret = scrub_iterate_filemaps(ctx, fd, XFS_ATTR_FORK, &key,
+ report_attr_loss, &br);
+ if (ret) {
+ str_liberror(ctx, ret, bmap_descr);
+ return false;
+ }
+ return true;
}
/* Report read verify errors in unlinked (but still open) files. */
^ permalink raw reply related [flat|nested] 21+ messages in thread
* [PATCH 02/18] xfs_scrub: remove moveon from the fscounters functions
2019-09-25 21:38 [PATCH 00/18] xfs_scrub: remove moveon space aliens Darrick J. Wong
2019-09-25 21:38 ` [PATCH 01/18] xfs_scrub: remove moveon from filemap iteration Darrick J. Wong
@ 2019-09-25 21:38 ` Darrick J. Wong
2019-09-25 21:38 ` [PATCH 03/18] xfs_scrub: remove moveon from inode iteration Darrick J. Wong
` (15 subsequent siblings)
17 siblings, 0 replies; 21+ messages in thread
From: Darrick J. Wong @ 2019-09-25 21:38 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 98aa3826..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_info(ctx, descr, _("Insufficient memory; giving up."));
- 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 3c9eec09..7607001a 100644
--- a/scrub/phase6.c
+++ b/scrub/phase6.c
@@ -779,17 +779,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] 21+ messages in thread
* [PATCH 03/18] xfs_scrub: remove moveon from inode iteration
2019-09-25 21:38 [PATCH 00/18] xfs_scrub: remove moveon space aliens Darrick J. Wong
2019-09-25 21:38 ` [PATCH 01/18] xfs_scrub: remove moveon from filemap iteration Darrick J. Wong
2019-09-25 21:38 ` [PATCH 02/18] xfs_scrub: remove moveon from the fscounters functions Darrick J. Wong
@ 2019-09-25 21:38 ` Darrick J. Wong
2019-09-25 21:38 ` [PATCH 04/18] xfs_scrub: remove moveon from vfs directory tree iteration Darrick J. Wong
` (14 subsequent siblings)
17 siblings, 0 replies; 21+ messages in thread
From: Darrick J. Wong @ 2019-09-25 21:38 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 | 131 ++++++++++++++++++++++++--------------------------------
scrub/inodes.h | 6 +--
scrub/phase3.c | 7 +--
scrub/phase5.c | 10 ++--
scrub/phase6.c | 5 +-
5 files changed, 70 insertions(+), 89 deletions(-)
diff --git a/scrub/inodes.c b/scrub/inodes.c
index bfc0a1e9..9fb9d1a5 100644
--- a/scrub/inodes.c
+++ b/scrub/inodes.c
@@ -47,7 +47,7 @@
* time (or fake it) into the bulkstat data.
*/
static void
-xfs_iterate_inodes_range_check(
+fill_in_bulkstat_holes(
struct scrub_ctx *ctx,
struct xfs_inumbers *inumbers,
struct xfs_bulkstat *bstat)
@@ -76,54 +76,66 @@ xfs_iterate_inodes_range_check(
}
}
+/* 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_info(ctx, descr, _("Insufficient memory; giving up."));
- return false;
+ str_errno(ctx, descr);
+ si->aborted = true;
+ return;
}
ireq = xfrog_inumbers_alloc_req(1, 0);
if (!ireq) {
- str_info(ctx, descr, _("Insufficient memory; giving up."));
+ 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.
@@ -141,19 +153,21 @@ xfs_iterate_inodes_ag(
errbuf, DESCR_BUFSZ));
}
- xfs_iterate_inodes_range_check(ctx, inumbers, breq->bulkstat);
+ fill_in_bulkstat_holes(ctx, inumbers, breq->bulkstat);
/* 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;
@@ -164,17 +178,16 @@ 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;
- goto out;
+ si->aborted = true;
+ return;
}
}
@@ -186,71 +199,41 @@ _("Changed too many times during scan; giving up."));
err:
if (error) {
str_liberror(ctx, error, descr);
- moveon = false;
+ si->aborted = true;
}
free(ireq);
free(breq);
-out:
- 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;
}
@@ -258,19 +241,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 3341c6d9..5bedd55b 100644
--- a/scrub/inodes.h
+++ b/scrub/inodes.h
@@ -6,13 +6,13 @@
#ifndef XFS_SCRUB_INODES_H_
#define XFS_SCRUB_INODES_H_
-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 48bcc21c..13601ed7 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 18056afd..3ee6df1b 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->errors_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 7607001a..55b5a611 100644
--- a/scrub/phase6.c
+++ b/scrub/phase6.c
@@ -273,7 +273,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)
@@ -530,7 +530,8 @@ xfs_report_verify_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] 21+ messages in thread
* [PATCH 04/18] xfs_scrub: remove moveon from vfs directory tree iteration
2019-09-25 21:38 [PATCH 00/18] xfs_scrub: remove moveon space aliens Darrick J. Wong
` (2 preceding siblings ...)
2019-09-25 21:38 ` [PATCH 03/18] xfs_scrub: remove moveon from inode iteration Darrick J. Wong
@ 2019-09-25 21:38 ` Darrick J. Wong
2019-09-25 21:38 ` [PATCH 05/18] xfs_scrub: remove moveon from spacemap Darrick J. Wong
` (13 subsequent siblings)
17 siblings, 0 replies; 21+ messages in thread
From: Darrick J. Wong @ 2019-09-25 21:38 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 | 6 +++---
3 files changed, 50 insertions(+), 36 deletions(-)
diff --git a/scrub/phase6.c b/scrub/phase6.c
index 55b5a611..dd6e29af 100644
--- a/scrub/phase6.c
+++ b/scrub/phase6.c
@@ -294,21 +294,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,
@@ -323,11 +326,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
@@ -336,8 +339,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);
@@ -348,7 +355,7 @@ xfs_report_verify_dirent(
error = close(fd);
if (error)
str_errno(ctx, path);
- return moveon;
+ return moveon ? 0 : -1;
}
/* Report an IO error resulting from read-verify based off getfsmap. */
@@ -508,7 +515,6 @@ xfs_report_verify_errors(
struct scrub_ctx *ctx,
struct media_verify_state *vs)
{
- bool moveon;
int ret;
ret = walk_ioerrs(ctx, ctx->datadev, vs);
@@ -524,9 +530,9 @@ xfs_report_verify_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 caef8969..7c518b28 100644
--- a/scrub/vfs.h
+++ b/scrub/vfs.h
@@ -6,12 +6,12 @@
#ifndef XFS_SCRUB_VFS_H_
#define XFS_SCRUB_VFS_H_
-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 *);
-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] 21+ messages in thread
* [PATCH 05/18] xfs_scrub: remove moveon from spacemap
2019-09-25 21:38 [PATCH 00/18] xfs_scrub: remove moveon space aliens Darrick J. Wong
` (3 preceding siblings ...)
2019-09-25 21:38 ` [PATCH 04/18] xfs_scrub: remove moveon from vfs directory tree iteration Darrick J. Wong
@ 2019-09-25 21:38 ` Darrick J. Wong
2019-09-25 21:38 ` [PATCH 06/18] xfs_scrub: remove moveon from unicode name collision helpers Darrick J. Wong
` (12 subsequent siblings)
17 siblings, 0 replies; 21+ messages in thread
From: Darrick J. Wong @ 2019-09-25 21:38 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 | 8 +--
4 files changed, 107 insertions(+), 102 deletions(-)
diff --git a/scrub/phase6.c b/scrub/phase6.c
index dd6e29af..49650a21 100644
--- a/scrub/phase6.c
+++ b/scrub/phase6.c
@@ -359,10 +359,9 @@ xfs_report_verify_dirent(
}
/* Report an IO error resulting from read-verify based off getfsmap. */
-static bool
+static int
ioerr_fsmap_report(
struct scrub_ctx *ctx,
- const char *descr,
struct fsmap *map,
void *arg)
{
@@ -373,7 +372,7 @@ ioerr_fsmap_report(
/* 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;
@@ -405,7 +404,7 @@ ioerr_fsmap_report(
* to find the bad file's pathname.
*/
- return true;
+ return 0;
}
static struct bitmap *
@@ -466,15 +465,10 @@ walk_ioerr(
{
struct walk_ioerr *wioerr = arg;
struct fsmap keys[2];
- char descr[DESCR_BUFSZ];
dev_t dev;
dev = xfs_disk_to_dev(wioerr->ctx, wioerr->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;
@@ -484,9 +478,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(wioerr->ctx, descr, keys, ioerr_fsmap_report,
+ return scrub_iterate_fsmap(wioerr->ctx, keys, ioerr_fsmap_report,
&start);
- return 0;
}
static int
@@ -541,10 +534,9 @@ xfs_report_verify_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)
{
@@ -574,7 +566,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. */
@@ -582,11 +574,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. */
@@ -720,8 +712,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 8f2f0a01..1291cf3c 100644
--- a/scrub/spacemap.h
+++ b/scrub/spacemap.h
@@ -6,12 +6,12 @@
#ifndef XFS_SCRUB_SPACEMAP_H_
#define XFS_SCRUB_SPACEMAP_H_
-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] 21+ messages in thread
* [PATCH 06/18] xfs_scrub: remove moveon from unicode name collision helpers
2019-09-25 21:38 [PATCH 00/18] xfs_scrub: remove moveon space aliens Darrick J. Wong
` (4 preceding siblings ...)
2019-09-25 21:38 ` [PATCH 05/18] xfs_scrub: remove moveon from spacemap Darrick J. Wong
@ 2019-09-25 21:38 ` Darrick J. Wong
2019-09-25 21:38 ` [PATCH 07/18] xfs_scrub: remove moveon from progress report helpers Darrick J. Wong
` (11 subsequent siblings)
17 siblings, 0 replies; 21+ messages in thread
From: Darrick J. Wong @ 2019-09-25 21:38 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 3ee6df1b..763685fd 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] 21+ messages in thread
* [PATCH 07/18] xfs_scrub: remove moveon from progress report helpers
2019-09-25 21:38 [PATCH 00/18] xfs_scrub: remove moveon space aliens Darrick J. Wong
` (5 preceding siblings ...)
2019-09-25 21:38 ` [PATCH 06/18] xfs_scrub: remove moveon from unicode name collision helpers Darrick J. Wong
@ 2019-09-25 21:38 ` Darrick J. Wong
2019-09-25 21:38 ` [PATCH 08/18] xfs_scrub: remove moveon from scrub ioctl wrappers Darrick J. Wong
` (10 subsequent siblings)
17 siblings, 0 replies; 21+ messages in thread
From: Darrick J. Wong @ 2019-09-25 21:38 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 89f6c96a..97482c8c 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] 21+ messages in thread
* [PATCH 08/18] xfs_scrub: remove moveon from scrub ioctl wrappers
2019-09-25 21:38 [PATCH 00/18] xfs_scrub: remove moveon space aliens Darrick J. Wong
` (6 preceding siblings ...)
2019-09-25 21:38 ` [PATCH 07/18] xfs_scrub: remove moveon from progress report helpers Darrick J. Wong
@ 2019-09-25 21:38 ` Darrick J. Wong
2019-09-25 21:39 ` [PATCH 09/18] xfs_scrub: remove moveon from repair action list helpers Darrick J. Wong
` (9 subsequent siblings)
17 siblings, 0 replies; 21+ messages in thread
From: Darrick J. Wong @ 2019-09-25 21:38 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 13601ed7..02a9a098 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 07927036..c60012b7 100644
--- a/scrub/phase4.c
+++ b/scrub/phase4.c
@@ -112,9 +112,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
@@ -122,8 +122,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 763685fd..cb79752f 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 8171082d..80a8cfdc 100644
--- a/scrub/scrub.c
+++ b/scrub/scrub.c
@@ -250,7 +250,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,
@@ -261,9 +261,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;
@@ -281,10 +282,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,
@@ -297,6 +303,7 @@ xfs_scrub_meta_type(
.sm_agno = agno,
};
enum check_outcome fix;
+ int ret;
background_sleep();
@@ -308,8 +315,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;
@@ -345,30 +353,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,
@@ -378,7 +384,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,
@@ -388,7 +394,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)
@@ -397,20 +403,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;
@@ -434,8 +437,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,
@@ -456,14 +462,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,
@@ -473,7 +479,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,
@@ -483,7 +489,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,
@@ -493,7 +499,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,
@@ -503,7 +509,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,
@@ -513,7 +519,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,
@@ -523,7 +529,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,
@@ -533,7 +539,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,
@@ -543,7 +549,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 6558bad7..4353756c 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] 21+ messages in thread
* [PATCH 09/18] xfs_scrub: remove moveon from repair action list helpers
2019-09-25 21:38 [PATCH 00/18] xfs_scrub: remove moveon space aliens Darrick J. Wong
` (7 preceding siblings ...)
2019-09-25 21:38 ` [PATCH 08/18] xfs_scrub: remove moveon from scrub ioctl wrappers Darrick J. Wong
@ 2019-09-25 21:39 ` Darrick J. Wong
2019-09-25 21:39 ` [PATCH 10/18] xfs_scrub: remove moveon from phase 7 functions Darrick J. Wong
` (8 subsequent siblings)
17 siblings, 0 replies; 21+ messages in thread
From: Darrick J. Wong @ 2019-09-25 21:39 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 3211a488..8a68a2bf 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_error(ctx, ctx->mntpoint, _("Not enough memory."));
+ error = action_lists_alloc(ctx->mnt.fsgeom.agcount,
+ &ctx->action_lists);
+ if (error) {
+ str_liberror(ctx, error, ctx->mntpoint);
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 02a9a098..a5f95004 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 c60012b7..ba60a1db 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;
@@ -141,7 +141,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 80a8cfdc..5eb1b276 100644
--- a/scrub/scrub.c
+++ b/scrub/scrub.c
@@ -253,7 +253,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;
@@ -281,7 +281,7 @@ xfs_scrub_save_repair(
break;
}
- xfs_action_list_add(alist, aitem);
+ action_list_add(alist, aitem);
return 0;
}
@@ -296,7 +296,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,
@@ -337,7 +337,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;
@@ -368,7 +368,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);
}
@@ -378,7 +378,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);
}
@@ -388,7 +388,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);
}
@@ -397,7 +397,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);
}
@@ -406,7 +406,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);
}
@@ -447,7 +447,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;
@@ -474,7 +474,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);
}
@@ -484,7 +484,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);
}
@@ -494,7 +494,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);
}
@@ -504,7 +504,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);
}
@@ -514,7 +514,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);
}
@@ -524,7 +524,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);
}
@@ -534,7 +534,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);
}
@@ -544,7 +544,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 4353756c..f997136b 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 errors_found;
^ permalink raw reply related [flat|nested] 21+ messages in thread
* [PATCH 10/18] xfs_scrub: remove moveon from phase 7 functions
2019-09-25 21:38 [PATCH 00/18] xfs_scrub: remove moveon space aliens Darrick J. Wong
` (8 preceding siblings ...)
2019-09-25 21:39 ` [PATCH 09/18] xfs_scrub: remove moveon from repair action list helpers Darrick J. Wong
@ 2019-09-25 21:39 ` Darrick J. Wong
2019-09-25 21:39 ` [PATCH 11/18] xfs_scrub: remove moveon from phase 6 functions Darrick J. Wong
` (7 subsequent siblings)
17 siblings, 0 replies; 21+ messages in thread
From: Darrick J. Wong @ 2019-09-25 21:39 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] 21+ messages in thread
* [PATCH 11/18] xfs_scrub: remove moveon from phase 6 functions
2019-09-25 21:38 [PATCH 00/18] xfs_scrub: remove moveon space aliens Darrick J. Wong
` (9 preceding siblings ...)
2019-09-25 21:39 ` [PATCH 10/18] xfs_scrub: remove moveon from phase 7 functions Darrick J. Wong
@ 2019-09-25 21:39 ` Darrick J. Wong
2019-09-25 21:39 ` [PATCH 12/18] xfs_scrub: remove moveon from phase 5 functions Darrick J. Wong
` (6 subsequent siblings)
17 siblings, 0 replies; 21+ messages in thread
From: Darrick J. Wong @ 2019-09-25 21:39 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 | 173 ++++++++++++++++++++++++++++++--------------------------
1 file changed, 92 insertions(+), 81 deletions(-)
diff --git a/scrub/phase6.c b/scrub/phase6.c
index 49650a21..9ee337f5 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)
{
@@ -95,7 +95,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;
@@ -213,8 +213,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,
@@ -236,7 +236,7 @@ xfs_report_verify_fd(
report_data_loss, &br);
if (ret) {
str_liberror(ctx, ret, bmap_descr);
- return false;
+ return ret;
}
/* attr fork */
@@ -245,23 +245,23 @@ xfs_report_verify_fd(
report_attr_loss, &br);
if (ret) {
str_liberror(ctx, ret, bmap_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)
@@ -285,26 +285,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);
}
/*
@@ -312,7 +310,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,
@@ -320,9 +318,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))
@@ -347,15 +344,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;
}
/* Report an IO error resulting from read-verify based off getfsmap. */
@@ -383,7 +380,7 @@ ioerr_fsmap_report(
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_error(ctx, buf, _("media error in %s."), type);
}
@@ -413,7 +410,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 we don't have parent pointers, save the bad extent for
@@ -457,6 +454,7 @@ struct walk_ioerr {
struct disk *disk;
};
+/* For a given badblocks extent, walk the rmap data to tell us what was lost. */
static int
walk_ioerr(
uint64_t start,
@@ -467,7 +465,7 @@ walk_ioerr(
struct fsmap keys[2];
dev_t dev;
- dev = xfs_disk_to_dev(wioerr->ctx, wioerr->disk);
+ dev = disk_to_dev(wioerr->ctx, wioerr->disk);
/* Go figure out which blocks are bad from the fsmap. */
memset(keys, 0, sizeof(struct fsmap) * 2);
@@ -482,6 +480,7 @@ walk_ioerr(
&start);
}
+/* Walk all badblocks extents to report what was lost. */
static int
walk_ioerrs(
struct scrub_ctx *ctx,
@@ -503,8 +502,8 @@ walk_ioerrs(
}
/* Given bad extent lists for the data & rtdev, find bad files. */
-static bool
-xfs_report_verify_errors(
+static int
+report_loss(
struct scrub_ctx *ctx,
struct media_verify_state *vs)
{
@@ -513,24 +512,22 @@ xfs_report_verify_errors(
ret = walk_ioerrs(ctx, ctx->datadev, vs);
if (ret) {
str_liberror(ctx, ret, _("walking datadev io errors"));
- return false;
+ return ret;
}
ret = walk_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. */
@@ -544,7 +541,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",
@@ -622,7 +619,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)
@@ -632,14 +629,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;
}
}
@@ -647,11 +644,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;
}
/*
@@ -662,18 +659,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);
@@ -711,40 +707,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 = xfs_report_verify_errors(ctx, &vs);
+ ret = report_loss(ctx, &vs);
bitmap_free(&vs.r_bad);
bitmap_free(&vs.d_bad);
- return moveon;
+ return ret;
out_rtpool:
if (vs.rvp_realtime) {
@@ -763,13 +758,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,
@@ -787,7 +788,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);
@@ -795,5 +796,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] 21+ messages in thread
* [PATCH 12/18] xfs_scrub: remove moveon from phase 5 functions
2019-09-25 21:38 [PATCH 00/18] xfs_scrub: remove moveon space aliens Darrick J. Wong
` (10 preceding siblings ...)
2019-09-25 21:39 ` [PATCH 11/18] xfs_scrub: remove moveon from phase 6 functions Darrick J. Wong
@ 2019-09-25 21:39 ` Darrick J. Wong
2019-09-25 21:39 ` [PATCH 13/18] xfs_scrub: remove moveon from phase 4 functions Darrick J. Wong
` (5 subsequent siblings)
17 siblings, 0 replies; 21+ messages in thread
From: Darrick J. Wong @ 2019-09-25 21:39 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 cb79752f..1aaa0086 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->errors_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] 21+ messages in thread
* [PATCH 13/18] xfs_scrub: remove moveon from phase 4 functions
2019-09-25 21:38 [PATCH 00/18] xfs_scrub: remove moveon space aliens Darrick J. Wong
` (11 preceding siblings ...)
2019-09-25 21:39 ` [PATCH 12/18] xfs_scrub: remove moveon from phase 5 functions Darrick J. Wong
@ 2019-09-25 21:39 ` Darrick J. Wong
2019-09-25 21:39 ` [PATCH 14/18] xfs_scrub: remove moveon from phase 3 functions Darrick J. Wong
` (4 subsequent siblings)
17 siblings, 0 replies; 21+ messages in thread
From: Darrick J. Wong @ 2019-09-25 21:39 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 | 85 +++++++++++++++++++++++++++++++++-----------------------
1 file changed, 50 insertions(+), 35 deletions(-)
diff --git a/scrub/phase4.c b/scrub/phase4.c
index ba60a1db..9c9c3e8e 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,76 +42,74 @@ 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->errors_found == 0 && ctx->unfixable_errors == 0 &&
+ if (ctx->errors_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;
@@ -124,14 +122,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,
@@ -146,5 +151,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] 21+ messages in thread
* [PATCH 14/18] xfs_scrub: remove moveon from phase 3 functions
2019-09-25 21:38 [PATCH 00/18] xfs_scrub: remove moveon space aliens Darrick J. Wong
` (12 preceding siblings ...)
2019-09-25 21:39 ` [PATCH 13/18] xfs_scrub: remove moveon from phase 4 functions Darrick J. Wong
@ 2019-09-25 21:39 ` Darrick J. Wong
2019-09-25 21:39 ` [PATCH 15/18] xfs_scrub: remove moveon from phase 2 functions Darrick J. Wong
` (3 subsequent siblings)
17 siblings, 0 replies; 21+ messages in thread
From: Darrick J. Wong @ 2019-09-25 21:39 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 a5f95004..354cc8ed 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] 21+ messages in thread
* [PATCH 15/18] xfs_scrub: remove moveon from phase 2 functions
2019-09-25 21:38 [PATCH 00/18] xfs_scrub: remove moveon space aliens Darrick J. Wong
` (13 preceding siblings ...)
2019-09-25 21:39 ` [PATCH 14/18] xfs_scrub: remove moveon from phase 3 functions Darrick J. Wong
@ 2019-09-25 21:39 ` Darrick J. Wong
2019-09-25 21:39 ` [PATCH 16/18] xfs_scrub: remove moveon from phase 1 functions Darrick J. Wong
` (2 subsequent siblings)
17 siblings, 0 replies; 21+ messages in thread
From: Darrick J. Wong @ 2019-09-25 21:39 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] 21+ messages in thread
* [PATCH 16/18] xfs_scrub: remove moveon from phase 1 functions
2019-09-25 21:38 [PATCH 00/18] xfs_scrub: remove moveon space aliens Darrick J. Wong
` (14 preceding siblings ...)
2019-09-25 21:39 ` [PATCH 15/18] xfs_scrub: remove moveon from phase 2 functions Darrick J. Wong
@ 2019-09-25 21:39 ` Darrick J. Wong
2019-09-25 21:39 ` [PATCH 17/18] xfs_scrub: remove XFS_ITERATE_INODES_ABORT from inode iterator Darrick J. Wong
2019-09-25 21:39 ` [PATCH 18/18] xfs_scrub: remove moveon from main program Darrick J. Wong
17 siblings, 0 replies; 21+ messages in thread
From: Darrick J. Wong @ 2019-09-25 21:39 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 8a68a2bf..dd6301a2 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, ctx->mntpoint);
- 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_info(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_info(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_info(ctx, ctx->mntpoint,
_("Unable to find log device path."));
- return false;
+ return ECANCELED;
}
if (ctx->mnt.fsgeom.rtblocks && ctx->fsinfo.fs_rt == NULL) {
str_info(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 97482c8c..d454cdfc 100644
--- a/scrub/xfs_scrub.c
+++ b/scrub/xfs_scrub.c
@@ -600,6 +600,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");
@@ -774,8 +775,8 @@ main(
str_error(&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 f997136b..c6fc204d 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] 21+ messages in thread
* [PATCH 17/18] xfs_scrub: remove XFS_ITERATE_INODES_ABORT from inode iterator
2019-09-25 21:38 [PATCH 00/18] xfs_scrub: remove moveon space aliens Darrick J. Wong
` (15 preceding siblings ...)
2019-09-25 21:39 ` [PATCH 16/18] xfs_scrub: remove moveon from phase 1 functions Darrick J. Wong
@ 2019-09-25 21:39 ` Darrick J. Wong
2019-09-25 21:39 ` [PATCH 18/18] xfs_scrub: remove moveon from main program Darrick J. Wong
17 siblings, 0 replies; 21+ messages in thread
From: Darrick J. Wong @ 2019-09-25 21:39 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 | 8 +++++++-
2 files changed, 8 insertions(+), 2 deletions(-)
diff --git a/scrub/inodes.c b/scrub/inodes.c
index 9fb9d1a5..142582eb 100644
--- a/scrub/inodes.c
+++ b/scrub/inodes.c
@@ -179,7 +179,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 5bedd55b..e3662c09 100644
--- a/scrub/inodes.h
+++ b/scrub/inodes.h
@@ -6,10 +6,16 @@
#ifndef XFS_SCRUB_INODES_H_
#define XFS_SCRUB_INODES_H_
+/*
+ * Return codes for the inode iterator function are 0 to continue iterating,
+ * and non-zero to stop iterating. Any non-zero value will be passed up to the
+ * iteration caller. The special 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] 21+ messages in thread
* [PATCH 18/18] xfs_scrub: remove moveon from main program
2019-09-25 21:38 [PATCH 00/18] xfs_scrub: remove moveon space aliens Darrick J. Wong
` (16 preceding siblings ...)
2019-09-25 21:39 ` [PATCH 17/18] xfs_scrub: remove XFS_ITERATE_INODES_ABORT from inode iterator Darrick J. Wong
@ 2019-09-25 21:39 ` Darrick J. Wong
17 siblings, 0 replies; 21+ messages in thread
From: Darrick J. Wong @ 2019-09-25 21:39 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 dd6301a2..0d343b03 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 354cc8ed..c3cea29a 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 9c9c3e8e..50c2dbb8 100644
--- a/scrub/phase4.c
+++ b/scrub/phase4.c
@@ -127,13 +127,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(
@@ -153,13 +146,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 1aaa0086..ece002cc 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 9ee337f5..f0977d6a 100644
--- a/scrub/phase6.c
+++ b/scrub/phase6.c
@@ -761,13 +761,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(
@@ -798,13 +791,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 d454cdfc..a880d26e 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
@@ -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 c6fc204d..537086f8 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] 21+ 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
@ 2019-10-22 18:51 ` Darrick J. Wong
0 siblings, 0 replies; 21+ 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] 21+ messages in thread
* [PATCH 06/18] xfs_scrub: remove moveon from unicode name collision helpers
2019-09-06 3:40 [PATCH 00/18] xfs_scrub: remove moveon space aliens Darrick J. Wong
@ 2019-09-06 3:41 ` Darrick J. Wong
0 siblings, 0 replies; 21+ messages in thread
From: Darrick J. Wong @ 2019-09-06 3:41 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 3ee6df1b..763685fd 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] 21+ messages in thread
end of thread, other threads:[~2019-10-22 18:51 UTC | newest]
Thread overview: 21+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-09-25 21:38 [PATCH 00/18] xfs_scrub: remove moveon space aliens Darrick J. Wong
2019-09-25 21:38 ` [PATCH 01/18] xfs_scrub: remove moveon from filemap iteration Darrick J. Wong
2019-09-25 21:38 ` [PATCH 02/18] xfs_scrub: remove moveon from the fscounters functions Darrick J. Wong
2019-09-25 21:38 ` [PATCH 03/18] xfs_scrub: remove moveon from inode iteration Darrick J. Wong
2019-09-25 21:38 ` [PATCH 04/18] xfs_scrub: remove moveon from vfs directory tree iteration Darrick J. Wong
2019-09-25 21:38 ` [PATCH 05/18] xfs_scrub: remove moveon from spacemap Darrick J. Wong
2019-09-25 21:38 ` [PATCH 06/18] xfs_scrub: remove moveon from unicode name collision helpers Darrick J. Wong
2019-09-25 21:38 ` [PATCH 07/18] xfs_scrub: remove moveon from progress report helpers Darrick J. Wong
2019-09-25 21:38 ` [PATCH 08/18] xfs_scrub: remove moveon from scrub ioctl wrappers Darrick J. Wong
2019-09-25 21:39 ` [PATCH 09/18] xfs_scrub: remove moveon from repair action list helpers Darrick J. Wong
2019-09-25 21:39 ` [PATCH 10/18] xfs_scrub: remove moveon from phase 7 functions Darrick J. Wong
2019-09-25 21:39 ` [PATCH 11/18] xfs_scrub: remove moveon from phase 6 functions Darrick J. Wong
2019-09-25 21:39 ` [PATCH 12/18] xfs_scrub: remove moveon from phase 5 functions Darrick J. Wong
2019-09-25 21:39 ` [PATCH 13/18] xfs_scrub: remove moveon from phase 4 functions Darrick J. Wong
2019-09-25 21:39 ` [PATCH 14/18] xfs_scrub: remove moveon from phase 3 functions Darrick J. Wong
2019-09-25 21:39 ` [PATCH 15/18] xfs_scrub: remove moveon from phase 2 functions Darrick J. Wong
2019-09-25 21:39 ` [PATCH 16/18] xfs_scrub: remove moveon from phase 1 functions Darrick J. Wong
2019-09-25 21:39 ` [PATCH 17/18] xfs_scrub: remove XFS_ITERATE_INODES_ABORT from inode iterator Darrick J. Wong
2019-09-25 21:39 ` [PATCH 18/18] xfs_scrub: remove moveon from main program Darrick J. Wong
-- strict thread matches above, loose matches on Subject: below --
2019-10-22 18:50 [PATCH 00/18] xfs_scrub: remove moveon space aliens Darrick J. Wong
2019-10-22 18:51 ` [PATCH 06/18] xfs_scrub: remove moveon from unicode name collision helpers Darrick J. Wong
2019-09-06 3:40 [PATCH 00/18] xfs_scrub: remove moveon space aliens Darrick J. Wong
2019-09-06 3:41 ` [PATCH 06/18] xfs_scrub: remove moveon from unicode name collision helpers Darrick J. Wong
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.