xfs
[Top] [All Lists]

Re: [PATCH 4/6] xfs: allow reusing busy extents where safe

To: Christoph Hellwig <hch@xxxxxxxxxxxxx>
Subject: Re: [PATCH 4/6] xfs: allow reusing busy extents where safe
From: Dave Chinner <david@xxxxxxxxxxxxx>
Date: Wed, 23 Mar 2011 11:20:24 +1100
Cc: xfs@xxxxxxxxxxx
In-reply-to: <20110322200137.837735220@xxxxxxxxxxxxxxxxxxxxxx>
References: <20110322195550.260682574@xxxxxxxxxxxxxxxxxxxxxx> <20110322200137.837735220@xxxxxxxxxxxxxxxxxxxxxx>
User-agent: Mutt/1.5.20 (2009-06-14)
On Tue, Mar 22, 2011 at 03:55:54PM -0400, Christoph Hellwig wrote:
> Allow reusing any busy extent for metadata allocations, and reusing busy
> userdata extents for userdata allocations.  Most of the complexity is
> propagating the userdata information from the XFS_BMAPI_METADATA flag
> to xfs_bunmapi into the low-level extent freeing routines.  After that
> we can just track what type of busy extent we have and treat it accordingly.
> 
> Signed-off-by: Christoph Hellwig <hch@xxxxxx>
.....

> @@ -2717,7 +2723,7 @@ restart:
>  
>               overlap = xfs_alloc_busy_try_reuse(pag, busyp,
>                                                  fbno, fbno + flen);
> -             if (overlap) {
> +             if (overlap == -1 || (overlap && userdata)) {
>                       spin_unlock(&pag->pagb_lock);
>                       xfs_log_force(tp->t_mountp, XFS_LOG_SYNC);
>                       goto restart;

Ok, so the only time we'll do a log force now is on an complete
overlap or a partial userdata overlap?

> @@ -2754,6 +2760,7 @@ xfs_alloc_busy_trim(
>  
>       ASSERT(flen > 0);
>  
> +restart:
>       spin_lock(&args->pag->pagb_lock);
>       rbp = args->pag->pagb_tree.rb_node;
>       while (rbp && flen >= args->minlen) {
> @@ -2771,6 +2778,31 @@ xfs_alloc_busy_trim(
>                       continue;
>               }
>  
> +             if (!args->userdata ||
> +                 (busyp->flags & XFS_ALLOC_BUSY_USERDATA)) {
> +                     int overlap;
> +
> +                     overlap = xfs_alloc_busy_try_reuse(args->pag, busyp,
> +                                                        fbno, fbno + flen);
> +                     if (unlikely(overlap == -1)) {
> +                             spin_unlock(&args->pag->pagb_lock);
> +                             xfs_log_force(args->mp, XFS_LOG_SYNC);
> +                             goto restart;
> +                     }

Hmmmm - I'm not so sure we can reuse overlapped data extents for
data allocations without a log force at all as there is no guarantee
that the data will not be overwritten before the original free
transaction is on disk.

That is, recovery may not replay the original data extent free
transaction or the new allocation transaction, but there is nothing
stopping us from having written the new data into the extent before
the crash occurred, especially as delayed allocation places the
allocation very close the data IO issue. e.g.:

        thread X                thread Y
        free data extent ABC
                                allocate data extent BCD
                                partial overlap, no log force
                                issue data IO
                                .....

                 <crash>

That leads to corruption of the data in the original file because
neither transaction is written to disk, but new data is....

Cheers,

Dave.
-- 
Dave Chinner
david@xxxxxxxxxxxxx

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