[Top] [All Lists]

Re: [PATCH] xfs: Improve scalability of busy extent tracking

To: Christoph Hellwig <hch@xxxxxxxxxxxxx>
Subject: Re: [PATCH] xfs: Improve scalability of busy extent tracking
From: Dave Chinner <david@xxxxxxxxxxxxx>
Date: Fri, 23 Apr 2010 13:24:15 +1000
Cc: xfs@xxxxxxxxxxx
In-reply-to: <20100422170848.GA26101@xxxxxxxxxxxxx>
References: <1271828835-2094-1-git-send-email-david@xxxxxxxxxxxxx> <20100422110143.GA21867@xxxxxxxxxxxxx> <20100422161626.GE23541@dastard> <20100422170848.GA26101@xxxxxxxxxxxxx>
User-agent: Mutt/1.5.20 (2009-06-14)
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

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...


Dave Chinner

<Prev in Thread] Current Thread [Next in Thread>