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

Dave Chinner
2013-02-19T23:19:58Z

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@redhat.com>
> >> ---
> >> 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@fromorbit.com