All of lore.kernel.org
 help / color / mirror / Atom feed
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

  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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.