Re: [PATCH] xfs: fix the extent count when allocating an new indirection array entry

[Dave Chinner <david@fromorbit.com>]

On Sat, Sep 21, 2013 at 10:38:31PM +0800, Jeff Liu wrote:
> From: Jie Liu <jeff.liu@oracle.com>
> 
> At xfs_iext_add(), if extent(s) are being appended to the last
> page in the indirection array and the new extent(s) don't fit
> in the page, the number of extents(erp->er_extcount) in a new
> allocated entry should be the minimum value between count and
> XFS_LINEAR_EXTS, instead of count.

Definitely looks like a bug, but what are the symptoms of it and how
did you find the problem? Is there any test case that demonstrates a
problem with the er_extcount being set incorrectly here?

> Signed-off-by: Jie Liu <jeff.liu@oracle.com>
> ---
>  fs/xfs/xfs_inode_fork.c |    8 ++++----
>  1 file changed, 4 insertions(+), 4 deletions(-)
> 
> diff --git a/fs/xfs/xfs_inode_fork.c b/fs/xfs/xfs_inode_fork.c
> index 02f1083..dfb4226 100644
> --- a/fs/xfs/xfs_inode_fork.c
> +++ b/fs/xfs/xfs_inode_fork.c
> @@ -1035,11 +1035,11 @@ xfs_iext_add(
>  
>  			while (count) {
>  				erp = xfs_iext_irec_new(ifp, erp_idx);
> -				erp->er_extcount = count;
> -				count -= MIN(count, (int)XFS_LINEAR_EXTS);
> -				if (count) {
> +				erp->er_extcount = MIN(count,
> +						       (int)XFS_LINEAR_EXTS);
> +				count -= erp->er_extcount;

count is declared as an int, whereas XFS_LINEAR_EXTS probably ends
up with a type of uint because of a cast in the macro. because we
are decrementing to zero, the count can be declared as a uint, too,
and the cast in the MIN() can go away. Indeed, MIN() should be
converted to min() seeing as we are touching the code here, and if
you want to retain the current types, the min_t() is appropriate,
not min(x, (some cast)y)....

Cheers,

Dave.
-- 
Dave Chinner
david@fromorbit.com

_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs