All of lore.kernel.org
 help / color / mirror / Atom feed
From: Dave Chinner <david@fromorbit.com>
To: Mark Tinguely <tinguely@sgi.com>
Cc: xfs@oss.sgi.com
Subject: Re: [PATCH] xfs: fix buffer shudown reference count mismatch
Date: Thu, 1 Nov 2012 12:26:38 +1100	[thread overview]
Message-ID: <20121101012638.GN29378@dastard> (raw)
In-Reply-To: <50915B51.3050309@sgi.com>

On Wed, Oct 31, 2012 at 12:09:37PM -0500, Mark Tinguely wrote:
> On 10/29/12 19:20, Dave Chinner wrote:
> >From: Dave Chinner<dchinner@redhat.com>
> >
> >When we shut down the filesystem, we have to unpin and free all the
> >buffers currently active in the CIL. To do this we unpin and remove
> >them in one operation as a result of a failed iclogbuf write. For
> >buffers, we do this removal via a simultated IO completion of after
> >marking the buffer stale.
> >
> >At the time we do this, we have two references to the buffer - the
> >active LRU reference and the buf log item.  The LRU reference is
> >removed by marking the buffer stale, and the active CIL reference is
> >by the xfs_buf_iodone() callback that is run by
> >xfs_buf_do_callbacks() during ioend processing (via the bp->b_iodone
> >callback).
> >
> >However, ioend processing requires one more reference - that of the
> >IO that it is completing. We don't have this reference, so we free
> >the buffer prematurely and use it after it is freed. This leads to
> >assert failures in xfs_buf_rele() on debug kernels because the
> >b_hold count is zero.
> >
> >Fix this by making sure we take the necessary IO reference before
> >starting IO completion processing on the stale buffer.
> >
> >Cc:<stable@vger.kernel.org>
> >Signed-off-by: Dave Chinner<dchinner@redhat.com>
> 
> This seems to take care of one of the ASSERT that I experienced after
> the worker move series.

It should - it's the shutdown failure you've reported for some time
now ;)

> With this patch applied, there is a new ASSERT that the perag is not
> empty in filesystem unmount in test 179. I think this is related to
> the worker move series and not this patch. I will send the information
> in a different thread.

That's been around for a long, long time. I've never been able to
reproduce it reliably - I see it maybe once every couple of months -
so I've never been able to get to the bottom of it....

> Reviewed-by: Mark Tinguely <tinguely@sgi.com>

Thanks,

Dave.

-- 
Dave Chinner
david@fromorbit.com

_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs

      reply	other threads:[~2012-11-01  1:24 UTC|newest]

Thread overview: 5+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2012-10-30  0:20 [PATCH] xfs: fix buffer shudown reference count mismatch Dave Chinner
2012-10-30 19:26 ` Christoph Hellwig
2012-10-30 20:03 ` Carlos Maiolino
2012-10-31 17:09 ` Mark Tinguely
2012-11-01  1:26   ` Dave Chinner [this message]

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=20121101012638.GN29378@dastard \
    --to=david@fromorbit.com \
    --cc=tinguely@sgi.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.