On Thu, 2010-09-02 at 15:17 +1000, Dave Chinner wrote:
> From: Dave Chinner <dchinner@xxxxxxxxxx>
>
> If we attempt to preallocate more than 2^32 blocks of space in a
> single syscall, the transaction block reservation will overflow
> leading to a hangs in the superblock block accounting code. This
> is trivially reproduced with xfs_io. Fix the problem by capping the
> allocation reservation to the maximum number of blocks a single
> xfs_bmapi() call can allocate (2^21 blocks).
This looks OK, but I have two comments, below.
> Signed-off-by: Dave Chinner <dchinner@xxxxxxxxxx>
> ---
> fs/xfs/xfs_vnodeops.c | 12 +++++++++---
> 1 files changed, 9 insertions(+), 3 deletions(-)
>
> diff --git a/fs/xfs/xfs_vnodeops.c b/fs/xfs/xfs_vnodeops.c
> index 66d585c..91dd9c8 100644
> --- a/fs/xfs/xfs_vnodeops.c
> +++ b/fs/xfs/xfs_vnodeops.c
> @@ -2299,15 +2299,21 @@ xfs_alloc_file_space(
> e = allocatesize_fsb;
> }
>
> + /*
> + * we can't allocate more than @nimaps extents at a time,
> + * so prevent a 32bit overflow on the transaction reserve
> + * by trying to reserve > 16TB worth of blocks for the
> + * preallocation.
> +
This comment could use rewording. How about something like:
A 32-bit block count limits the amount of space that can
be reserved in a transaction, so we need to limit the
number of blocks reserved to avoid overflow. We can't
allocate more than @nimaps extents (whose size won't
exceed 32 bits) at a time anyway, so use that to enforce
the limit.
> */
> + resblks = min_t(xfs_fileoff_t, (e - s), (MAXEXTLEN * nimaps));
I guess it's clear that MAXEXTLEN fits in 32 bits because of
sizeof (xfs_extlen_t). And inspection shows that nimaps is
just 1, so this does the 32-bit limiting. But that just
seems indirect. (Actually, now that I've written this I
updated the above comment and it's better...)
-Alex
> if (unlikely(rt)) {
> - resrtextents = qblocks = (uint)(e - s);
> + resrtextents = qblocks = resblks;
> resrtextents /= mp->m_sb.sb_rextsize;
> resblks = XFS_DIOSTRAT_SPACE_RES(mp, 0);
> quota_flag = XFS_QMOPT_RES_RTBLKS;
> } else {
> resrtextents = 0;
> - resblks = qblocks = \
> - XFS_DIOSTRAT_SPACE_RES(mp, (uint)(e - s));
> + resblks = qblocks = XFS_DIOSTRAT_SPACE_RES(mp, resblks);
> quota_flag = XFS_QMOPT_RES_REGBLKS;
> }
>
_______________________________________________
xfs mailing list
xfs@xxxxxxxxxxx
http://oss.sgi.com/mailman/listinfo/xfs
