[Top] [All Lists]

Re: [PATCH 4/8] xfs: limit extent length for allocation to AG size

To: Alex Elder <aelder@xxxxxxx>
Subject: Re: [PATCH 4/8] xfs: limit extent length for allocation to AG size
From: Dave Chinner <david@xxxxxxxxxxxxx>
Date: Thu, 27 Jan 2011 11:38:52 +1100
Cc: xfs@xxxxxxxxxxx
In-reply-to: <1296076968.1980.941.camel@doink>
References: <1295945444-29488-1-git-send-email-david@xxxxxxxxxxxxx> <1295945444-29488-5-git-send-email-david@xxxxxxxxxxxxx> <1296076968.1980.941.camel@doink>
User-agent: Mutt/1.5.20 (2009-06-14)
On Wed, Jan 26, 2011 at 03:22:48PM -0600, Alex Elder wrote:
> On Tue, 2011-01-25 at 19:50 +1100, Dave Chinner wrote:
> > From: Dave Chinner <dchinner@xxxxxxxxxx>
> > 
> > Delayed allocation extents can be larger than AGs, so when trying to
> > convert a large range we may scan every AG inside
> > xfs_bmap_alloc_nullfb() trying to find an AG with a size larger than
> > an AG. We should stop when we find the first AG with a maximum
> > possible allocation size. This causes excessive CPU usage when there
> > are lots of AGs.
> > 
> > The same problem occurs when doing preallocation of a range larger
> > than an AG.
> > 
> > Fix the problem by limiting real allocation lengths to the maximum
> > that an AG can support. This means if we have empty AGs, we'll stop
> > the search at the first of them. If there are no empty AGs, we'll
> > still scan them all, but that is a different problem....
> Maybe I'm wrong but I think you need to change a "+"
> to a "-" (shown below).

Good catch. That's what I get for cleaning up code ;)

> And I have a few really minor suggestions:
> - You should update a comment (which I point
>   out below) to match your change.

Will fix.

> - Maybe make use of a local variable, at least
>   in xfs_bmap_btalloc_nullfb(), such as:
>     xfs_extlen_t requested = args->maxlen;

All the allocation code is coded that way to avoid putting local
variables on the stack. The allocation-in-writeback path is the
critical stack usage path in XFS, so I'd prefer to keep using
args->maxlen here....


Dave Chinner

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