On Wed, Dec 05, 2012 at 11:47:56AM -0500, Brian Foster wrote: > The round down occurs towards the beginning of the function. Push > it down after throttling has occurred. This is to support adding > further transformations to 'alloc_blocks' that might not preserve > power-of-two alignment (and thus could lead to rounding down > multiple times). > > Signed-off-by: Brian Foster <bfoster@xxxxxxxxxx> > --- > fs/xfs/xfs_iomap.c | 12 ++++++------ > 1 files changed, 6 insertions(+), 6 deletions(-) > > diff --git a/fs/xfs/xfs_iomap.c b/fs/xfs/xfs_iomap.c > index bd7c060..d381326 100644 > --- a/fs/xfs/xfs_iomap.c > +++ b/fs/xfs/xfs_iomap.c > @@ -329,13 +329,11 @@ xfs_iomap_prealloc_size( > goto check_writeio; > > /* > - * rounddown_pow_of_two() returns an undefined result > - * if we pass in alloc_blocks = 0. Hence the "+ 1" to > - * ensure we always pass in a non-zero value. > + * MAXEXTLEN is 21 bits, add one to protect against the rounddown > + * further down. > */ > - alloc_blocks = XFS_B_TO_FSB(mp, XFS_ISIZE(ip)) + 1; > - alloc_blocks = XFS_FILEOFF_MIN(MAXEXTLEN, > - rounddown_pow_of_two(alloc_blocks)); > + alloc_blocks = XFS_FILEOFF_MIN(MAXEXTLEN + 1, > + XFS_B_TO_FSB(mp, XFS_ISIZE(ip))); I suspect this is will lead to a bug - if the round down doesn't modify the value when it is MAXEXTLEN + 1, then we are returning a value greater than MAXEXTLEN to the caller.... > xfs_icsb_sync_counters(mp, XFS_ICSB_LAZY_COUNT); > freesp = mp->m_sb.sb_fdblocks; > @@ -352,6 +350,8 @@ xfs_iomap_prealloc_size( > } > if (shift) > alloc_blocks >>= shift; > + if (alloc_blocks) > + alloc_blocks = rounddown_pow_of_two(alloc_blocks); This needs the comment about rounddown_pow_of_two() and zero values. It then needs to cap alloc_blocks to MAXEXTLEN, because it can clearly be larger thanks to the above (MAXEXTLEN + 1) code. Cheers, Dave. -- Dave Chinner david@xxxxxxxxxxxxx _______________________________________________ xfs mailing list xfs@xxxxxxxxxxx http://oss.sgi.com/mailman/listinfo/xfs
