From: Brian Foster <bfoster@redhat.com>
To: Dave Chinner <david@fromorbit.com>
Cc: xfs@oss.sgi.com
Subject: Re: [PATCH] xfs: bulkstat doesn't release AGI buffer on error
Date: Wed, 15 Oct 2014 08:59:27 -0400 [thread overview]
Message-ID: <20141015125927.GA63003@bfoster.bfoster> (raw)
In-Reply-To: <1413324728-4008-1-git-send-email-david@fromorbit.com>
On Wed, Oct 15, 2014 at 09:12:08AM +1100, Dave Chinner wrote:
> From: Dave Chinner <dchinner@redhat.com>
>
> The recent refactoring of the bulkstat code left a small landmine in
> the code. If a inobt read fails, then the tree walk is aborted and
> returns without releasing the AGI buffer or freeing the cursor. This
> can lead to a subsequent bulkstat call hanging trying to grab the
> AGI buffer again.
>
> Signed-off-by: Dave Chinner <dchinner@redhat.com>
> ---
Reviewed-by: Brian Foster <bfoster@redhat.com>
> fs/xfs/xfs_itable.c | 16 ++++++++++------
> 1 file changed, 10 insertions(+), 6 deletions(-)
>
> diff --git a/fs/xfs/xfs_itable.c b/fs/xfs/xfs_itable.c
> index f1deb96..ef8ea05 100644
> --- a/fs/xfs/xfs_itable.c
> +++ b/fs/xfs/xfs_itable.c
> @@ -427,7 +427,7 @@ xfs_bulkstat(
>
> error = xfs_bulkstat_grab_ichunk(cur, agino, &icount, &r);
> if (error)
> - break;
> + goto del_cursor;
> if (icount) {
> irbp->ir_startino = r.ir_startino;
> irbp->ir_freecount = r.ir_freecount;
> @@ -442,7 +442,7 @@ xfs_bulkstat(
> error = xfs_inobt_lookup(cur, 0, XFS_LOOKUP_GE, &tmp);
> }
> if (error)
> - break;
> + goto del_cursor;
>
> /*
> * Loop through inode btree records in this ag,
> @@ -454,7 +454,7 @@ xfs_bulkstat(
> error = xfs_inobt_get_rec(cur, &r, &i);
> if (error || i == 0) {
> end_of_ag = 1;
> - break;
> + goto del_cursor;
> }
>
> /*
> @@ -476,13 +476,17 @@ xfs_bulkstat(
> error = xfs_btree_increment(cur, 0, &tmp);
> cond_resched();
> }
> +
> /*
> - * Drop the btree buffers and the agi buffer.
> - * We can't hold any of the locks these represent
> - * when calling iget.
> + * Drop the btree buffers and the agi buffer as we can't hold any
> + * of the locks these represent when calling iget. If there is a
> + * pending error, then we are done.
> */
> +del_cursor:
> xfs_btree_del_cursor(cur, XFS_BTREE_NOERROR);
> xfs_buf_relse(agbp);
> + if (error)
> + break;
> /*
> * Now format all the good inodes into the user's buffer.
> */
> --
> 2.0.0
>
> _______________________________________________
> xfs mailing list
> xfs@oss.sgi.com
> http://oss.sgi.com/mailman/listinfo/xfs
_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs
next prev parent reply other threads:[~2014-10-15 12:59 UTC|newest]
Thread overview: 3+ messages / expand[flat|nested] mbox.gz Atom feed top
2014-10-14 22:12 [PATCH] xfs: bulkstat doesn't release AGI buffer on error Dave Chinner
2014-10-15 12:59 ` Brian Foster [this message]
2014-10-16 13:45 ` Eric Sandeen
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20141015125927.GA63003@bfoster.bfoster \
--to=bfoster@redhat.com \
--cc=david@fromorbit.com \
--cc=xfs@oss.sgi.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
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.