xfs
[Top] [All Lists]

Re: [PATCH v3 3/7] xfs: cap prealloc size to free space before shift

To: Brian Foster <bfoster@xxxxxxxxxx>
Subject: Re: [PATCH v3 3/7] xfs: cap prealloc size to free space before shift
From: Dave Chinner <david@xxxxxxxxxxxxx>
Date: Wed, 20 Feb 2013 10:19:54 +1100
Cc: xfs@xxxxxxxxxxx
Delivered-to: xfs@xxxxxxxxxxx
In-reply-to: <5123FCB7.9050309@xxxxxxxxxx>
References: <1361291851-24714-1-git-send-email-bfoster@xxxxxxxxxx> <1361291851-24714-4-git-send-email-bfoster@xxxxxxxxxx> <20130219214842.GG10731@dastard> <5123FCB7.9050309@xxxxxxxxxx>
User-agent: Mutt/1.5.21 (2010-09-15)
On Tue, Feb 19, 2013 at 05:29:11PM -0500, Brian Foster wrote:
> On 02/19/2013 04:48 PM, Dave Chinner wrote:
> > On Tue, Feb 19, 2013 at 11:37:27AM -0500, Brian Foster wrote:
> >> With the addition of quota preallocation throttling, we want to
> >> support a general algorithm that considers the maximum allowable
> >> prealloc size and recommended shift modifier from various sources
> >> (i.e., global fs state and all applicable quotas for an inode).
> >>
> >> Update the current global free space throttle algorithm to cap the
> >> preallocation size to the free space available in the filesystem.
> >>
> >> Signed-off-by: Brian Foster <bfoster@xxxxxxxxxx>
> >> ---
> >>  fs/xfs/xfs_iomap.c |    3 +++
> >>  1 files changed, 3 insertions(+), 0 deletions(-)
> >>
> >> diff --git a/fs/xfs/xfs_iomap.c b/fs/xfs/xfs_iomap.c
> >> index daa08f6..3b41c18 100644
> >> --- a/fs/xfs/xfs_iomap.c
> >> +++ b/fs/xfs/xfs_iomap.c
> >> @@ -412,6 +412,9 @@ xfs_iomap_prealloc_size(
> >>            if (freesp < mp->m_low_space[XFS_LOWSP_1_PCNT])
> >>                    shift++;
> >>    }
> >> +  if (alloc_blocks > freesp)
> >> +          alloc_blocks = freesp;
> >> +
> >>    if (shift)
> >>            alloc_blocks >>= shift;
> > 
> > This is redundant with the previous additions of the trailing
> > 
> >     while (alloc_blocks >= freesp)
> >             alloc_blocks >>= 4;
> > 
> > code. Effectively adding the check will result in preventing the
> > existing loop from working as alloc_blocks will be brought down to
> > just under freespc by things like power of 2 rounding, rather than
> > being thottled to a small fraction of the remaining free space...
> > 
> 
> Ah, yes. For starters, this set was more of a logical add-on to make the
> throttling consistent between global free space and quota limits (e.g.,
> start with free space available and throttle down) as opposed to a
> functional dependency, so it should be safe to just drop this patch from
> the set.
> 
> Tracking back to that discussion...
> 
> http://oss.sgi.com/archives/xfs/2013-01/msg00392.html
> 
> ... my understanding is that at the moment, the condition addressed by
> the previous change is not relevant to quota since we have no
> flush/retry cycle (e.g., we just fail early). The intended follow up set
> to this (eofblocks scan, retry) would introduce such a cycle. What I'm
> wondering is if we'll need something similar longer term within the
> quota throttling code.

There is a retry cycle for EDQUOT - we simply turn off preallocation
and try again. So, if we ask for all the free blocks in the
quota....

> 
> In particular, is the "metadata overhead" referred to in your original
> explanation accounted against an associated quota,

... this *may* trigger an EDQUOT and turn off preallocation.

> such that it still
> isn't enough to simply start the prealloc capped at the quota free space
> limit? If so, perhaps as part of that set I'll need to modify this code
> to carry a minimum 'qfreesp' through each of the quotas and add that to
> the squashing loop...

Probably.

i.e. it doesn't matter alloc_blocks is over freesp or dquot limits
at the end of the prealloc size calculation. If it is, we just keep
squashing it by >>= 4 until it is under all relevant thresholds...

Cheers,

Dave.
-- 
Dave Chinner
david@xxxxxxxxxxxxx

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