xfs
[Top] [All Lists]

[PATCH] mkfs.xfs: fix protofile name create block reservation

To: Boris Ranto <branto@xxxxxxxxxx>
Subject: [PATCH] mkfs.xfs: fix protofile name create block reservation
From: Eric Sandeen <sandeen@xxxxxxxxxx>
Date: Wed, 07 Aug 2013 01:00:43 -0500
Cc: xfs-oss <xfs@xxxxxxxxxxx>
Delivered-to: xfs@xxxxxxxxxxx
User-agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10.8; rv:17.0) Gecko/20130620 Thunderbird/17.0.7
A large protofile which creates a large directory and requires
a a dir tree split, can fail:

  mkfs.xfs: directory createname error [28 - No space left on device]

This is because when we've split a block once, we decrement args->total:
(see kernel commit a7444053fb3ebd3d905e3c7a7bd5ea80a54b083a for the
rationale)

       /* account for newly allocated blocks in reserved blocks total */
       args->total -= dp->i_d.di_nblocks - nblks;

but every call into this path from proto file parsing started
reserved / args->total as only "1" as passed tro newdirent() -
so if we allocate a block, args->total hits 0, and then in
xfs_dir2_node_addname():

        /*
         * Add the new leaf entry.
         */
        rval = xfs_dir2_leafn_add(blk->bp, args, blk->index);
        if (rval == 0) {
                ...
        } else {
                /*
                 * It didn't work, we need to split the leaf block.
                 */
                if (args->total == 0) {
                        ASSERT(rval == ENOSPC);
                        goto done;
                }
                /*
                 * Split the leaf block and insert the new entry.
                 */

we hit the args->total == 0 special case, and don't do the next
split, and ENOSPC gets returned all the way up, and we fail.

So rather than calling newdirent with a total of "1" in every case,
which doesn't account for possible tree splits, we should call it
with a more appropriate value: XFS_DIRENTER_SPACE_RES(mp, name->len),
which will handle the maximum nr of block allocations that might be
needed during a directory entry insert.

Since the reservation required doesn't depend on entry type,
just push this down a level, into newdirent() itself.

Reported-by: Boris Ranto <branto@xxxxxxxxxx>
Signed-off-by: Eric Sandeen <sandeen@xxxxxxxxxx>
---

diff --git a/mkfs/proto.c b/mkfs/proto.c
index f201096..7d96b46 100644
--- a/mkfs/proto.c
+++ b/mkfs/proto.c
@@ -306,12 +306,14 @@ newdirent(
        struct xfs_name *name,
        xfs_ino_t       inum,
        xfs_fsblock_t   *first,
-       xfs_bmap_free_t *flist,
-       xfs_extlen_t    total)
+       xfs_bmap_free_t *flist)
 {
        int     error;
+       int     rsv;
+
+       rsv = XFS_DIRENTER_SPACE_RES(mp, name->len);
 
-       error = libxfs_dir_createname(tp, pip, name, inum, first, flist, total);
+       error = libxfs_dir_createname(tp, pip, name, inum, first, flist, rsv);
        if (error)
                fail(_("directory createname error"), error);
 }
@@ -449,7 +451,7 @@ parseproto(
                if (buf)
                        free(buf);
                libxfs_trans_ijoin(tp, pip, 0);
-               newdirent(mp, tp, pip, &xname, ip->i_ino, &first, &flist, 1);
+               newdirent(mp, tp, pip, &xname, ip->i_ino, &first, &flist);
                libxfs_trans_ihold(tp, pip);
                break;
 
@@ -465,7 +467,7 @@ parseproto(
 
                libxfs_trans_ijoin(tp, pip, 0);
 
-               newdirent(mp, tp, pip, &xname, ip->i_ino, &first, &flist, 1);
+               newdirent(mp, tp, pip, &xname, ip->i_ino, &first, &flist);
                libxfs_trans_ihold(tp, pip);
                libxfs_trans_log_inode(tp, ip, flags);
 
@@ -486,7 +488,7 @@ parseproto(
                        fail(_("Inode allocation failed"), error);
                }
                libxfs_trans_ijoin(tp, pip, 0);
-               newdirent(mp, tp, pip, &xname, ip->i_ino, &first, &flist, 1);
+               newdirent(mp, tp, pip, &xname, ip->i_ino, &first, &flist);
                libxfs_trans_ihold(tp, pip);
                flags |= XFS_ILOG_DEV;
                break;
@@ -500,7 +502,7 @@ parseproto(
                if (error)
                        fail(_("Inode allocation failed"), error);
                libxfs_trans_ijoin(tp, pip, 0);
-               newdirent(mp, tp, pip, &xname, ip->i_ino, &first, &flist, 1);
+               newdirent(mp, tp, pip, &xname, ip->i_ino, &first, &flist);
                libxfs_trans_ihold(tp, pip);
                flags |= XFS_ILOG_DEV;
                break;
@@ -512,7 +514,7 @@ parseproto(
                if (error)
                        fail(_("Inode allocation failed"), error);
                libxfs_trans_ijoin(tp, pip, 0);
-               newdirent(mp, tp, pip, &xname, ip->i_ino, &first, &flist, 1);
+               newdirent(mp, tp, pip, &xname, ip->i_ino, &first, &flist);
                libxfs_trans_ihold(tp, pip);
                break;
        case IF_SYMLINK:
@@ -525,7 +527,7 @@ parseproto(
                        fail(_("Inode allocation failed"), error);
                flags |= newfile(tp, ip, &flist, &first, 1, 1, buf, len);
                libxfs_trans_ijoin(tp, pip, 0);
-               newdirent(mp, tp, pip, &xname, ip->i_ino, &first, &flist, 1);
+               newdirent(mp, tp, pip, &xname, ip->i_ino, &first, &flist);
                libxfs_trans_ihold(tp, pip);
                break;
        case IF_DIRECTORY:
@@ -544,7 +546,7 @@ parseproto(
                } else {
                        libxfs_trans_ijoin(tp, pip, 0);
                        newdirent(mp, tp, pip, &xname, ip->i_ino,
-                                 &first, &flist, 1);
+                                 &first, &flist);
                        pip->i_d.di_nlink++;
                        libxfs_trans_ihold(tp, pip);
                        libxfs_trans_log_inode(tp, pip, XFS_ILOG_CORE);


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