From: Christoph Hellwig <hch@infradead.org>
To: Dave Chinner <david@fromorbit.com>
Cc: xfs@oss.sgi.com
Subject: Re: [PATCH 2/2] xfs: fix efi item leak on forced shutdown
Date: Thu, 20 Jan 2011 08:27:38 -0500 [thread overview]
Message-ID: <20110120132738.GA1061@infradead.org> (raw)
In-Reply-To: <20110118233346.GA28803@dastard>
On Wed, Jan 19, 2011 at 10:33:46AM +1100, Dave Chinner wrote:
> > Hmm, this is not symmetric with the non-delaylog path.
> > xfs_trans_item_committed never sets the remove flag to IOP_UNPIN,
> > even if the transaction commit was aborted.
>
> Right, because the delaylog and non-delaylog paths are not symmetric
> w.r.t. log write failures.
>
> > It seems like the CIL code is missing an equivalent to
> > xfs_trans_uncommit for the case that xfs_log_write or xfs_log_done
> > fail.
>
> There isn't an equivalent. In the delaylog case, we don't have a
> transaction to "uncommit" when a log write failure occurs - we are
> aborting the checkpoint of the CIL, not a transaction. As the items
> have already gone through IOP_COMMITTING and IOP_UNLOCK, we have to
> treat the failures like they came from the log IO completion
> handler.
True.
> In the case of non-delaylog, neither IOP_COMMITTING or IOP_UNLOCK
> has been called on the items when the xfs_log_write() fails. They
> are still linked into the xfs_trans structure, so they can be
> handled by xfs_trans_uncommit() which simply needs to walk the items
> in the transaction and IOP_UNPIN(lip, abort), IOP_UNLOCK and free
> the items.
You're right. Care to extend the comment to explain this a bit better
in the code?
_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs
prev parent reply other threads:[~2011-01-20 13:25 UTC|newest]
Thread overview: 7+ messages / expand[flat|nested] mbox.gz Atom feed top
2011-01-14 13:07 [PATCH 0/2] xfs: fix memory leaks on shutdown w/ delayed logging Dave Chinner
2011-01-14 13:07 ` [PATCH 1/2] xfs: fix log ticket leak on forced shutdown Dave Chinner
2011-01-18 12:35 ` Christoph Hellwig
2011-01-14 13:07 ` [PATCH 2/2] xfs: fix efi item " Dave Chinner
2011-01-18 12:46 ` Christoph Hellwig
2011-01-18 23:33 ` Dave Chinner
2011-01-20 13:27 ` Christoph Hellwig [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=20110120132738.GA1061@infradead.org \
--to=hch@infradead.org \
--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.