public inbox for linux-xfs@vger.kernel.org
 help / color / mirror / Atom feed
From: Dave Chinner <david@fromorbit.com>
To: Christoph Hellwig <hch@infradead.org>
Cc: xfs@oss.sgi.com
Subject: Re: [PATCH] xfs: Improve scalability of busy extent tracking
Date: Fri, 23 Apr 2010 13:24:15 +1000	[thread overview]
Message-ID: <20100423032415.GD10390@dastard> (raw)
In-Reply-To: <20100422170848.GA26101@infradead.org>

On Thu, Apr 22, 2010 at 01:08:49PM -0400, Christoph Hellwig wrote:
> Been looking at this a bit and I have a theory:
> 
>  - a tid is not actually unique to a xfs_trans structure, if
>    we call xfs_trans_dup a single xlog_ticket, and with that the
>    tid is re-used by multiple transaction structure.

Good point, and an angle I had missed.

>  - because of that the major semantic change in the version vs
>    the previous one is that we now do not force the synchronous
>    transaction for the case where we re-used a block in
>    the rolled over transaction.

The problem case is not re-allocation of the busy extent, it's the
subsequent freeing of that extent that is already busy.  i.e. we've
done "free - alloc - free" before the transaction containing the
alloc has hit the disk and cleared the original busy extent from the
tree.

In a single transaction case, the alloc should mark the transaction
synchronous, so the second free should match the tid and assert that
it is synchronous.

The mutliple transaction case can be split up:

	{free} {alloc} {free}: mismatched tids -> log force
	{free - alloc} {free}: mismatched tids -> log force
	{free} {alloc - free}: same as the single transaction case

But as you point out, the mutiple transaction case could have
duplicate tids, so the first two cases could be getting it wrong.
However, if the tids match, then the ASSERT should fire if the
transactions were not sycnhronous.

Hence all I can conclude is that xfsqa is not triggering the first
two cases from duplicated transactions, but we need to change
tid of the log ticket when we dup transactions.

FWIW, now that you've pointed this out, this appears to be another
existing source of duplicate TIDs in the log that I did not identify
when I first saw them in the log.  It seems to me that the best
approach to fixing this is that when we regrant the log space on a
permanent log ticket we need to regenerate the ticket tid so that
individual components of rolling transactions are not confused in
log replay. Does this seem reasonable?

Doing this would also handle the above two cases correctly as well.
Despite this, I still can't see how such a scenario would cause the
problems with transaction pointer matching because the "same
transaction pointer but different transactions" situation still
can't occur even with duplicated transactions....

> Still not quite sure about the implications of this.

I'll change it and run some tests. It'll be next week before I can
get to this, though...

Cheers,

Dave.
-- 
Dave Chinner
david@fromorbit.com

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

  reply	other threads:[~2010-04-23  3:22 UTC|newest]

Thread overview: 8+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2010-04-21  5:47 [PATCH] xfs: Improve scalability of busy extent tracking Dave Chinner
2010-04-22 11:01 ` Christoph Hellwig
2010-04-22 16:16   ` Dave Chinner
2010-04-22 17:08     ` Christoph Hellwig
2010-04-23  3:24       ` Dave Chinner [this message]
2010-04-22 11:07 ` Christoph Hellwig
2010-04-22 17:10   ` Christoph Hellwig
2010-04-23  3:25     ` 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=20100423032415.GD10390@dastard \
    --to=david@fromorbit.com \
    --cc=hch@infradead.org \
    --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