xfs
[Top] [All Lists]

Re: [PATCH 5/8] xfs: prevent extsize alignment from exceeding maximum ex

To: Dave Chinner <david@xxxxxxxxxxxxx>
Subject: Re: [PATCH 5/8] xfs: prevent extsize alignment from exceeding maximum extent size
From: Alex Elder <aelder@xxxxxxx>
Date: Wed, 26 Jan 2011 15:22:55 -0600
Cc: xfs@xxxxxxxxxxx
In-reply-to: <1295945444-29488-6-git-send-email-david@xxxxxxxxxxxxx>
References: <1295945444-29488-1-git-send-email-david@xxxxxxxxxxxxx> <1295945444-29488-6-git-send-email-david@xxxxxxxxxxxxx>
Reply-to: aelder@xxxxxxx
On Tue, 2011-01-25 at 19:50 +1100, Dave Chinner wrote:
> From: Dave Chinner <dchinner@xxxxxxxxxx>
> 
> 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@xxxxxxx>

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


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