xfs
[Top] [All Lists]

Re: [RFC PATCH] xfs: borrow indirect blocks from freed extent when avail

To: Brian Foster <bfoster@xxxxxxxxxx>
Subject: Re: [RFC PATCH] xfs: borrow indirect blocks from freed extent when available
From: Dave Chinner <david@xxxxxxxxxxxxx>
Date: Fri, 26 Sep 2014 11:59:31 +1000
Cc: xfs@xxxxxxxxxxx
Delivered-to: xfs@xxxxxxxxxxx
In-reply-to: <20140925150703.GB47304@xxxxxxxxxxxxxxx>
References: <1411500538-6831-1-git-send-email-bfoster@xxxxxxxxxx> <20140923215816.GC4322@dastard> <20140924122746.GA53094@xxxxxxxxxxxxxxx> <20140924233014.GB4758@dastard> <20140925150703.GB47304@xxxxxxxxxxxxxxx>
User-agent: Mutt/1.5.21 (2010-09-15)
On Thu, Sep 25, 2014 at 11:07:04AM -0400, Brian Foster wrote:
> On Thu, Sep 25, 2014 at 09:30:14AM +1000, Dave Chinner wrote:
> > I knew I'd looked at this before:
> > 
> > http://oss.sgi.com/archives/xfs/2014-03/msg00314.html
> > 
> > That got lost because I wrote it in a topic branch and not my usual
> > working branch, so when I dropped the topic branch. Guilt, however,
> > keeps all the patches from topic branches around, and so when I just
> > did a grep for da_new across .git/patch, this showed up.
> > 
> > It's basically the same "steal blocks from the deleted extent
> > reservation fix, and it was trying to address the above failure.
> > However, there are some other details in it (like changing the
> > location of delalloc accounting updates) that might be relevant.
> > 
> 
> Ah, right. I thought I had seen something like this before. In fact I
> had it in my head that we already did something like this when I
> narrowed in on the code so I was somewhat surprised, but I didn't go
> back and look through the list. That explains that. :)
> 
> This version moves the entire delalloc accounting hunk after the
> xfs_bmap_del_extent() call. I think the problem with that is the sb
> counter is the only record keeping that encompasses data blocks and
> indirect blocks, which is why I only moved that update in xfs_bunmapi().
> That's also precisely why I consider using a separate parameter rather
> than updating br_blockcount.
> 
> Let me know if you wanted to resurrect this one, otherwise I'll try to
> double check all of that when I get back to reworking mine...

It doesn't need to be resurrected if you've got a better fix. ;)

> > I'm pretty sure the test case was simply something like:
> > 
> > xfs_io -f -c "pwrite 0 1m" \
> >       -c "fzero 4k 8k" \
> >       -c "fzero 16k 8k" \
> >       -c "fzero 32k 8k" \
> >       -c "fzero 64k 8k" \
> >      .....
> > 
> > To basically split the delalloc extent repeatedly and hence drain
> > the reservation.
> > 
> 
> Yep, thanks. I assume you saw the test I posted:
> 
> http://oss.sgi.com/archives/xfs/2014-09/msg00371.html

Yup, I did see that later in the day....

Cheers,

Dave.
-- 
Dave Chinner
david@xxxxxxxxxxxxx

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