From: Dave Chinner <david@fromorbit.com>
To: Brian Foster <bfoster@redhat.com>
Cc: xfs@oss.sgi.com
Subject: Re: [PATCH v3 3/7] xfs: cap prealloc size to free space before shift
Date: Wed, 20 Feb 2013 10:19:54 +1100 [thread overview]
Message-ID: <20130219231954.GI10731@dastard> (raw)
In-Reply-To: <5123FCB7.9050309@redhat.com>
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
_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs
next prev parent reply other threads:[~2013-02-19 23:19 UTC|newest]
Thread overview: 14+ messages / expand[flat|nested] mbox.gz Atom feed top
2013-02-19 16:37 [PATCH v3 0/7] speculative preallocation quota throttling Brian Foster
2013-02-19 16:37 ` [PATCH v3 1/7] xfs: reorganize xfs_iomap_prealloc_size to remove indentation Brian Foster
2013-02-19 16:37 ` [PATCH v3 2/7] xfs: push rounddown_pow_of_two() to after prealloc throttle Brian Foster
2013-02-19 16:37 ` [PATCH v3 3/7] xfs: cap prealloc size to free space before shift Brian Foster
2013-02-19 21:48 ` Dave Chinner
2013-02-19 22:29 ` Brian Foster
2013-02-19 23:19 ` Dave Chinner [this message]
2013-02-20 13:17 ` Brian Foster
2013-02-19 16:37 ` [PATCH v3 4/7] xfs: pass xfs_dquot to xfs_qm_adjust_dqlimits() instead of xfs_disk_dquot_t Brian Foster
2013-02-19 16:37 ` [PATCH v3 5/7] xfs: xfs_dquot prealloc throttling watermarks and low free space Brian Foster
2013-02-19 23:08 ` Dave Chinner
2013-02-20 13:17 ` Brian Foster
2013-02-19 16:37 ` [PATCH v3 6/7] xfs: add quota-driven speculative preallocation throttling Brian Foster
2013-02-19 16:37 ` [PATCH v3 7/7] xfs: xfs_iomap_prealloc_size() tracepoint Brian Foster
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20130219231954.GI10731@dastard \
--to=david@fromorbit.com \
--cc=bfoster@redhat.com \
--cc=xfs@oss.sgi.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox