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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox