From: Alex Elder <aelder@sgi.com>
To: Dave Chinner <david@fromorbit.com>
Cc: xfs@oss.sgi.com
Subject: Re: [PATCH 03/12] xfs: modify buffer item reference counting
Date: Wed, 19 May 2010 13:35:58 -0500 [thread overview]
Message-ID: <1274294158.2140.35.camel@doink> (raw)
In-Reply-To: <1274138668-1662-4-git-send-email-david@fromorbit.com>
On Tue, 2010-05-18 at 09:24 +1000, Dave Chinner wrote:
> From: Dave Chinner <dchinner@redhat.com>
>
> The buffer log item reference counts used to take referenceѕ for every
> transaction, similar to the pin counting. This is symmetric (like the
> pin/unpin) with respect to transaction completion, but with dleayed logging
> becomes assymetric as the pinning becomes assymetric w.r.t. transaction
> completion.
Doing the reference counts this way is an improvement.
I have a question below.
. . .
> --- a/fs/xfs/xfs_buf_item.c
> +++ b/fs/xfs/xfs_buf_item.c
. . .
> STATIC void
> xfs_buf_item_unlock(
> @@ -514,73 +523,54 @@ xfs_buf_item_unlock(
>
> bp = bip->bli_buf;
>
> - /*
> - * Clear the buffer's association with this transaction.
> - */
> + /* Clear the buffer's association with this transaction. */
> XFS_BUF_SET_FSPRIVATE2(bp, NULL);
>
> /*
> - * If this is a transaction abort, don't return early.
> - * Instead, allow the brelse to happen.
> - * Normally it would be done for stale (cancelled) buffers
> - * at unpin time, but we'll never go through the pin/unpin
> - * cycle if we abort inside commit.
> + * If this is a transaction abort, don't return early. Instead, allow
> + * the brelse to happen. Normally it would be done for stale
> + * (cancelled) buffers at unpin time, but we'll never go through the
> + * pin/unpin cycle if we abort inside commit.
> */
> aborted = (bip->bli_item.li_flags & XFS_LI_ABORTED) != 0;
>
> /*
> - * If the buf item is marked stale, then don't do anything.
> - * We'll unlock the buffer and free the buf item when the
> - * buffer is unpinned for the last time.
> + * Before possibly freeing the buf item, determine if we should
> + * release the buffer at the end of this routine.
> + */
> + hold = bip->bli_flags & XFS_BLI_HOLD;
> +
> + /* Clear the per transaction state. */
> + bip->bli_flags &= ~(XFS_BLI_LOGGED | XFS_BLI_HOLD);
> +
> + /*
> + * If the buf item is marked stale, then don't do anything. We'll
> + * unlock the buffer and free the buf item when the buffer is unpinned
> + * for the last time.
> */
> if (bip->bli_flags & XFS_BLI_STALE) {
> - bip->bli_flags &= ~XFS_BLI_LOGGED;
> trace_xfs_buf_item_unlock_stale(bip);
> ASSERT(bip->bli_format.blf_flags & XFS_BLI_CANCEL);
> - if (!aborted)
> + if (!aborted) {
> + atomic_dec(&bip->bli_refcount);
I notice that, unlike before, if you return via this path
the XFS_BLI_HOLD flag will have been turned off. I guess
I don't know off hand whether this is an issue. Can you
explain why it is not?
> return;
> + }
> }
>
. . .
_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs
next prev parent reply other threads:[~2010-05-19 18:33 UTC|newest]
Thread overview: 27+ messages / expand[flat|nested] mbox.gz Atom feed top
2010-05-17 23:24 [PATCH 0/12] xfs: delayed logging V6 Dave Chinner
2010-05-17 23:24 ` [PATCH 01/12] xfs: Don't reuse the same transaction ID for duplicated transactions Dave Chinner
2010-05-17 23:24 ` [PATCH 02/12] xfs: allow log ticket allocation to take allocation flags Dave Chinner
2010-05-17 23:24 ` [PATCH 03/12] xfs: modify buffer item reference counting Dave Chinner
2010-05-19 18:35 ` Alex Elder [this message]
2010-05-19 22:37 ` Dave Chinner
2010-05-17 23:24 ` [PATCH 04/12] xfs: Clean up XFS_BLI_* flag namespace Dave Chinner
2010-05-19 19:09 ` Alex Elder
2010-05-17 23:24 ` [PATCH 05/12] xfs: clean up log ticket overrun debug output Dave Chinner
2010-05-19 19:16 ` Alex Elder
2010-05-19 22:41 ` Dave Chinner
2010-05-17 23:24 ` [PATCH 06/12] xfs: make the log ticket ID available outside the log infrastructure Dave Chinner
2010-05-17 23:24 ` [PATCH 07/12] xfs: Improve scalability of busy extent tracking Dave Chinner
2010-05-20 20:15 ` Alex Elder
2010-05-21 2:16 ` Dave Chinner
2010-05-21 20:59 ` Alex Elder
2010-05-17 23:24 ` [PATCH 08/12] xfs: Delayed logging design documentation Dave Chinner
2010-05-21 21:02 ` Alex Elder
2010-05-17 23:24 ` [PATCH 09/12] xfs: Introduce delayed logging core code Dave Chinner
2010-05-21 21:06 ` Alex Elder
2010-05-17 23:24 ` [PATCH 10/12] xfs: forced unmounts need to push the CIL Dave Chinner
2010-05-17 23:24 ` [PATCH 11/12] xfs: enable background pushing of " Dave Chinner
2010-05-17 23:24 ` [PATCH 12/12] xfs: Ensure inode allocation buffers are fully replayed Dave Chinner
2010-05-21 21:21 ` [PATCH 0/12] xfs: delayed logging V6 Alex Elder
2010-05-22 0:39 ` Dave Chinner
2010-05-24 0:30 ` Dave Chinner
-- strict thread matches above, loose matches on Subject: below --
2010-05-07 5:40 [PATCH 0/12] xfs: delayed logging V5 Dave Chinner
2010-05-07 5:40 ` [PATCH 03/12] xfs: modify buffer item reference counting Dave Chinner
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=1274294158.2140.35.camel@doink \
--to=aelder@sgi.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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox