Hey Jeff, On Fri, Oct 25, 2013 at 02:52:44PM +0800, Jeff Liu wrote: > From: Jie Liu <jeff.liu@xxxxxxxxxx> > > 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. > > For now, there is no existing test case can demonstrates a problem with > the er_extcount being set incorrectly here, but it obviously like a bug. > > Signed-off-by: Jie Liu <jeff.liu@xxxxxxxxxx> > --- > v2: * Declare count to uint as it will be decreased to 0 and XFS_LINEAR_EXTS > can be uint because of a case in the macro. > * Convert MIN() to min(). > * Revise the commits log to indicate there is no existing test case can > reflect this issue for future tracking up. > > fs/xfs/xfs_inode_fork.c | 9 ++++----- > 1 file changed, 4 insertions(+), 5 deletions(-) > > diff --git a/fs/xfs/xfs_inode_fork.c b/fs/xfs/xfs_inode_fork.c > index 22c9837..cfee14a 100644 > --- a/fs/xfs/xfs_inode_fork.c > +++ b/fs/xfs/xfs_inode_fork.c > @@ -1021,15 +1021,14 @@ xfs_iext_add( > * the next index needed in the indirection array. > */ > else { > - int count = ext_diff; > + uint count = ext_diff; > > 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, XFS_LINEAR_EXTS); > + count -= erp->er_extcount; > + if (count) > erp_idx++; > - } > } > } > } Really nice find. So there is potential for incorrect er_extcount and er_extoff when adding > 256 extents to the end of the indirection array. You'd think we'd be seeing some side effects since xfs_iext_idx_to_irec uses them in it's binary search. Reviewed-by: Ben Myers <bpm@xxxxxxx> _______________________________________________ xfs mailing list xfs@xxxxxxxxxxx http://oss.sgi.com/mailman/listinfo/xfs