[Top] [All Lists]

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

To: Dave Chinner <david@xxxxxxxxxxxxx>
Subject: Re: [PATCH v3 3/7] xfs: cap prealloc size to free space before shift
From: Brian Foster <bfoster@xxxxxxxxxx>
Date: Tue, 19 Feb 2013 17:29:11 -0500
Cc: xfs@xxxxxxxxxxx
Delivered-to: xfs@xxxxxxxxxxx
In-reply-to: <20130219214842.GG10731@dastard>
References: <1361291851-24714-1-git-send-email-bfoster@xxxxxxxxxx> <1361291851-24714-4-git-send-email-bfoster@xxxxxxxxxx> <20130219214842.GG10731@dastard>
User-agent: Mozilla/5.0 (X11; Linux x86_64; rv:17.0) Gecko/20130110 Thunderbird/17.0.2
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...


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

In particular, is the "metadata overhead" referred to in your original
explanation accounted against an associated quota, 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...


> Cheers,
> Dave.

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