Subject: Re: [PATCH 5/8] xfs: prevent extsize alignment from exceeding
 maximum extent size
From: Alex Elder <aelder@sgi.com>
Reply-To: aelder@sgi.com
To: Dave Chinner <david@fromorbit.com>
Cc: xfs@oss.sgi.com
In-Reply-To: <1295945444-29488-6-git-send-email-david@fromorbit.com>
References: <1295945444-29488-1-git-send-email-david@fromorbit.com>
	 <1295945444-29488-6-git-send-email-david@fromorbit.com>
Content-Type: text/plain; charset="UTF-8"
Date: Wed, 26 Jan 2011 15:22:55 -0600
Message-ID: <1296076975.1980.942.camel@doink>
Mime-Version: 1.0
Content-Transfer-Encoding: 7bit

On Tue, 2011-01-25 at 19:50 +1100, Dave Chinner wrote:
> From: Dave Chinner <dchinner@redhat.com>
> 
> When doing delayed allocation, if the allocation size is for a
> maximally sized extent, extent size alignment can push it over this
> limit. This results in an assert failure in xfs_bmbt_set_allf() as
> the extent length is too large to find in the extent record.
> 
> Fix this by ensuring that we allow for space that extent size
> alignment requires (up to 2 * (extsize -1) blocks as we have to
> handle both head and tail alignment) when limiting the maximum size
> of the extent.

I think this is OK, however...

It seems to me that the XFS_FILBLKS_MIN() call you're making is
sort of magical because it pre-supposes exactly what the following
xfs_bmap_extsize_align() actually does.  And because of that, I
would rather see that logic built into xfs_bmap_extsize_align()
itself.  I haven't looked at it closely, but I presume the other
two spots that call xfs_bmap_extsize_align() would be subject to
the same MAXEXTLEN limit.

OK with me if you disagree though.

Reviewed-by: Alex Elder <aelder@sgi.com>

> Signed-off-by: Dave Chinner <dchinner@redhat.com>
> ---
>  fs/xfs/xfs_bmap.c |   10 ++++++++++
>  1 files changed, 10 insertions(+), 0 deletions(-)