| Message ID | 156685448524.2840069.582566075645213965.stgit@magnolia (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | xfs_scrub: fix bugs in vfs tree walk code | expand |
On Mon, Aug 26, 2019 at 02:21:25PM -0700, Darrick J. Wong wrote: > From: Darrick J. Wong <darrick.wong@oracle.com> > > When we're scanning the directory tree, we bump nr_dirs every time we > think we're going to queue a new directory to process, and we decrement > it every time we're finished doing something with a directory > (successful or not). We forgot to undo a counter increment when > workqueue_add fails, so refactor the code into helpers and call them > as necessary for correct operation. > > Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com> > --- > scrub/vfs.c | 38 +++++++++++++++++++++++++++++--------- > 1 file changed, 29 insertions(+), 9 deletions(-) > > > diff --git a/scrub/vfs.c b/scrub/vfs.c > index ea2866d9..b358ab4a 100644 > --- a/scrub/vfs.c > +++ b/scrub/vfs.c > @@ -45,6 +45,32 @@ struct scan_fs_tree_dir { > > static void scan_fs_dir(struct workqueue *wq, xfs_agnumber_t agno, void *arg); > > +/* Increment the number of directories that are queued for processing. */ > +static void > +inc_nr_dirs( > + struct scan_fs_tree *sft) > +{ > + pthread_mutex_lock(&sft->lock); > + sft->nr_dirs++; > + pthread_mutex_unlock(&sft->lock); > +} > + > +/* > + * Decrement the number of directories that are queued for processing and if > + * we ran out of dirs to process, wake up anyone who was waiting for processing > + * to finish. > + */ > +static void > +dec_nr_dirs( > + struct scan_fs_tree *sft) > +{ > + pthread_mutex_lock(&sft->lock); > + sft->nr_dirs--; > + if (sft->nr_dirs == 0) > + pthread_cond_signal(&sft->wakeup); > + pthread_mutex_unlock(&sft->lock); > +} > + > /* Queue a directory for scanning. */ > static bool > queue_subdir( > @@ -73,11 +99,10 @@ queue_subdir( > new_sftd->sft = sft; > new_sftd->rootdir = is_rootdir; > > - pthread_mutex_lock(&sft->lock); > - sft->nr_dirs++; > - pthread_mutex_unlock(&sft->lock); > + inc_nr_dirs(sft); > error = workqueue_add(wq, scan_fs_dir, 0, new_sftd); > if (error) { > + dec_nr_dirs(sft); > str_info(ctx, ctx->mntpoint, > _("Could not queue subdirectory scan work.")); > return false; Ok, that's the bug fix for the previous patch. Potentially should be a separate patch, but right now there is so much outstanding that I don't think it's worthwhile to respin the series just to fix that. Reviewed-by: Dave Chinner <dchinner@redhat.com> Cheers, Dave.
diff --git a/scrub/vfs.c b/scrub/vfs.c index ea2866d9..b358ab4a 100644 --- a/scrub/vfs.c +++ b/scrub/vfs.c @@ -45,6 +45,32 @@ struct scan_fs_tree_dir { static void scan_fs_dir(struct workqueue *wq, xfs_agnumber_t agno, void *arg); +/* Increment the number of directories that are queued for processing. */ +static void +inc_nr_dirs( + struct scan_fs_tree *sft) +{ + pthread_mutex_lock(&sft->lock); + sft->nr_dirs++; + pthread_mutex_unlock(&sft->lock); +} + +/* + * Decrement the number of directories that are queued for processing and if + * we ran out of dirs to process, wake up anyone who was waiting for processing + * to finish. + */ +static void +dec_nr_dirs( + struct scan_fs_tree *sft) +{ + pthread_mutex_lock(&sft->lock); + sft->nr_dirs--; + if (sft->nr_dirs == 0) + pthread_cond_signal(&sft->wakeup); + pthread_mutex_unlock(&sft->lock); +} + /* Queue a directory for scanning. */ static bool queue_subdir( @@ -73,11 +99,10 @@ queue_subdir( new_sftd->sft = sft; new_sftd->rootdir = is_rootdir; - pthread_mutex_lock(&sft->lock); - sft->nr_dirs++; - pthread_mutex_unlock(&sft->lock); + inc_nr_dirs(sft); error = workqueue_add(wq, scan_fs_dir, 0, new_sftd); if (error) { + dec_nr_dirs(sft); str_info(ctx, ctx->mntpoint, _("Could not queue subdirectory scan work.")); return false; @@ -172,12 +197,7 @@ scan_fs_dir( str_errno(ctx, sftd->path); out: - pthread_mutex_lock(&sft->lock); - sft->nr_dirs--; - if (sft->nr_dirs == 0) - pthread_cond_signal(&sft->wakeup); - pthread_mutex_unlock(&sft->lock); - + dec_nr_dirs(sft); free(sftd->path); free(sftd); }