libxfs: dont free xfs_inode until complete

Originally, the xfs_inode are released upon the first
call to xfs_trans_cancel, xfs_trans_commit, or
inode_item_done. This code used the log item lock field
to prevent the release of the inode on the next call to
one of the above functions. This is a unusual use of the
log item lock field which is suppose to specify which lock
is to be release on transaction commit or cancel. User
space does not perform locking in transactions..

Unfortunately, this breaks any code that relies on multiple
transaction operations. For example, adding an extended
attribute to an inode that does not have an attribute fork
will fail:

 # xfs_db -x XFS_DEVICE
 xfs_db> inode INO_NUM
 xfs_db> attr_set newattribute

This patch does the following:
 1) Removes the iput from the transaction completion and
    requires that the xfs_inode allocators call IRELE()
    when they are done with the pointer. The real time
    inodes are pointed to by the xfs_mount and have a longer
    lifetime.
 2) Removes libxfs_trans_iput() because transaction entries
    are removed in transaction commit and cancel.
 3) Removes libxfs_trans_ihold() which is an obsolete interface.
 4) Removes the now unneeded ili_ilock_flags from the
    xfs_inode_log_item structure.

Signed-off-by: Mark Tinguely <tinguely@sgi.com>
Reviewed-by: Christoph Hellwig <hch@lst.de>
Signed-off-by: Dave Chinner <david@fromorbit.com>

diff --git a/include/libxfs.h b/include/libxfs.h
index f97ab65875d3e150f86cf2f5c03d128dfbf82b56..7203d7993bba35a3ba7f0ac7ccd530ad04ef6747 100644 (file)
--- a/include/libxfs.h
+++ b/include/libxfs.h
@@ -480,7 +480,6 @@ typedef struct xfs_inode_log_item {
        unsigned int            ili_fields;             /* fields to be logged */
        unsigned int            ili_last_fields;        /* fields when flushed*/
        xfs_inode_log_format_t  ili_format;             /* logged structure */
-       int                     ili_lock_flags;
 } xfs_inode_log_item_t;
 
 typedef struct xfs_buf_log_item {
@@ -535,9 +534,7 @@ extern xfs_buf_t    *libxfs_trans_getsb (xfs_trans_t *, xfs_mount_t *, int);
 
 extern int     libxfs_trans_iget (xfs_mount_t *, xfs_trans_t *, xfs_ino_t,
                                uint, uint, struct xfs_inode **);
-extern void    libxfs_trans_iput(xfs_trans_t *, struct xfs_inode *);
 extern void    libxfs_trans_ijoin (xfs_trans_t *, struct xfs_inode *, uint);
-extern void    libxfs_trans_ihold (xfs_trans_t *, struct xfs_inode *);
 extern void    libxfs_trans_ijoin_ref(xfs_trans_t *, struct xfs_inode *, int);
 extern void    libxfs_trans_log_inode (xfs_trans_t *, struct xfs_inode *,
                                uint);

diff --git a/libxfs/trans.c b/libxfs/trans.c
index 49af4d187f32fb5da30c01501c83c221434bbc4b..13d21b24568ceb3deae2b2d1fe7cf4c20231dfe4 100644 (file)
--- a/libxfs/trans.c
+++ b/libxfs/trans.c
@@ -110,7 +110,7 @@ libxfs_trans_roll(
        /*
         * Commit the current transaction.
         * If this commit failed, then it'd just unlock those items that
-        * are not marked ihold. That also means that a filesystem shutdown
+        * are marked to be released. That also means that a filesystem shutdown
         * is in progress. The caller takes the responsibility to cancel
         * the duplicate transaction that gets returned.
         */
@@ -247,26 +247,6 @@ libxfs_trans_iget(
        return 0;
 }
 
-void
-libxfs_trans_iput(
-       xfs_trans_t             *tp,
-       xfs_inode_t             *ip)
-{
-       xfs_inode_log_item_t    *iip;
-
-       if (tp == NULL) {
-               IRELE(ip);
-               return;
-       }
-
-       ASSERT(ip->i_transp == tp);
-       iip = ip->i_itemp;
-       ASSERT(iip != NULL);
-       xfs_trans_del_item(&iip->ili_item);
-
-       IRELE(ip);
-}
-
 void
 libxfs_trans_ijoin(
        xfs_trans_t             *tp,
@@ -300,28 +280,12 @@ libxfs_trans_ijoin_ref(
        ASSERT(ip->i_itemp != NULL);
 
        xfs_trans_ijoin(tp, ip, lock_flags);
-       ip->i_itemp->ili_lock_flags = lock_flags;
 
 #ifdef XACT_DEBUG
        fprintf(stderr, "ijoin_ref'd inode %llu, transaction %p\n", ip->i_ino, tp);
 #endif
 }
 
-void
-libxfs_trans_ihold(
-       xfs_trans_t             *tp,
-       xfs_inode_t             *ip)
-{
-       ASSERT(ip->i_transp == tp);
-       ASSERT(ip->i_itemp != NULL);
-
-       ip->i_itemp->ili_lock_flags = 1;
-
-#ifdef XACT_DEBUG
-       fprintf(stderr, "ihold'd inode %llu, transaction %p\n", ip->i_ino, tp);
-#endif
-}
-
 void
 libxfs_trans_inode_alloc_buf(
        xfs_trans_t             *tp,
@@ -701,7 +665,7 @@ inode_item_done(
        if (!(iip->ili_fields & XFS_ILOG_ALL)) {
                ip->i_transp = NULL;    /* disassociate from transaction */
                iip->ili_flags = 0;     /* reset all flags */
-               goto ili_done;
+               return;
        }
 
        /*
@@ -711,7 +675,7 @@ inode_item_done(
        if (error) {
                fprintf(stderr, _("%s: warning - imap_to_bp failed (%d)\n"),
                        progname, error);
-               goto ili_done;
+               return;
        }
 
        XFS_BUF_SET_FSPRIVATE(bp, iip);
@@ -719,7 +683,7 @@ inode_item_done(
        if (error) {
                fprintf(stderr, _("%s: warning - iflush_int failed (%d)\n"),
                        progname, error);
-               goto ili_done;
+               return;
        }
 
        ip->i_transp = NULL;    /* disassociate from transaction */
@@ -727,16 +691,9 @@ inode_item_done(
        XFS_BUF_SET_FSPRIVATE2(bp, NULL);       /* remove xact ptr */
        libxfs_writebuf(bp, 0);
 #ifdef XACT_DEBUG
-       fprintf(stderr, "flushing dirty inode %llu, buffer %p (hold=%u)\n",
-                       ip->i_ino, bp, iip->ili_lock_flags);
+       fprintf(stderr, "flushing dirty inode %llu, buffer %p\n",
+                       ip->i_ino, bp);
 #endif
-ili_done:
-       if (iip->ili_lock_flags) {
-               iip->ili_lock_flags = 0;
-               return;
-       }
-       /* free the inode */
-       IRELE(ip);
 }
 
 static void
@@ -817,10 +774,6 @@ inode_item_unlock(
        ip->i_transp = NULL;
 
        iip->ili_flags = 0;
-       if (!iip->ili_lock_flags)
-               IRELE(ip);
-       else
-               iip->ili_lock_flags = 0;
 }
 
 /*

diff --git a/libxfs/util.c b/libxfs/util.c
index 4c40324806d7a1783fd09a80b2e0ccecccd65d07..9504e3368f0528c63a2e2345aa01341ada4d6338 100644 (file)
--- a/libxfs/util.c
+++ b/libxfs/util.c
@@ -595,7 +595,6 @@ libxfs_alloc_file_space(
                        break;
                }
                xfs_trans_ijoin(tp, ip, 0);
-               xfs_trans_ihold(tp, ip);
 
                xfs_bmap_init(&free_list, &firstfsb);
                error = xfs_bmapi_write(tp, ip, startoffset_fsb, allocatesize_fsb,

diff --git a/libxfs/xfs.h b/libxfs/xfs.h
index 5a21590b139f2afd95d7aa22d1bd6af98465423b..30a316d3268c2bb252d7eaffb2b9bfed3db079ef 100644 (file)
--- a/libxfs/xfs.h
+++ b/libxfs/xfs.h
@@ -292,7 +292,6 @@ roundup_64(__uint64_t x, __uint32_t y)
 #define xfs_trans_get_buf              libxfs_trans_get_buf
 #define xfs_trans_getsb                        libxfs_trans_getsb
 #define xfs_trans_iget                 libxfs_trans_iget
-#define xfs_trans_ihold                        libxfs_trans_ihold
 #define xfs_trans_ijoin                        libxfs_trans_ijoin
 #define xfs_trans_ijoin_ref            libxfs_trans_ijoin_ref
 #define xfs_trans_init                 libxfs_trans_init

diff --git a/mkfs/proto.c b/mkfs/proto.c
index 5b68b23d07918ce470f84915fde9a1a3c131d645..72068f0242fe2d16647f570e822c8eb3a9ace1b4 100644 (file)
--- a/mkfs/proto.c
+++ b/mkfs/proto.c
@@ -197,7 +197,6 @@ rsvfile(
        tp = libxfs_trans_alloc(mp, 0);
 
        libxfs_trans_ijoin(tp, ip, 0);
-       libxfs_trans_ihold(tp, ip);
 
        ip->i_d.di_mode &= ~S_ISUID;
 
@@ -464,7 +463,6 @@ parseproto(
                libxfs_trans_ijoin(tp, pip, 0);
                xname.type = XFS_DIR3_FT_REG_FILE;
                newdirent(mp, tp, pip, &xname, ip->i_ino, &first, &flist);
-               libxfs_trans_ihold(tp, pip);
                break;
 
        case IF_RESERVED:                       /* pre-allocated space only */
@@ -481,7 +479,6 @@ parseproto(
 
                xname.type = XFS_DIR3_FT_REG_FILE;
                newdirent(mp, tp, pip, &xname, ip->i_ino, &first, &flist);
-               libxfs_trans_ihold(tp, pip);
                libxfs_trans_log_inode(tp, ip, flags);
 
                error = libxfs_bmap_finish(&tp, &flist, &committed);
@@ -489,6 +486,7 @@ parseproto(
                        fail(_("Pre-allocated file creation failed"), error);
                libxfs_trans_commit(tp, 0);
                rsvfile(mp, ip, llen);
+               IRELE(ip);
                return;
 
        case IF_BLOCK:
@@ -503,7 +501,6 @@ parseproto(
                libxfs_trans_ijoin(tp, pip, 0);
                xname.type = XFS_DIR3_FT_BLKDEV;
                newdirent(mp, tp, pip, &xname, ip->i_ino, &first, &flist);
-               libxfs_trans_ihold(tp, pip);
                flags |= XFS_ILOG_DEV;
                break;
 
@@ -518,7 +515,6 @@ parseproto(
                libxfs_trans_ijoin(tp, pip, 0);
                xname.type = XFS_DIR3_FT_CHRDEV;
                newdirent(mp, tp, pip, &xname, ip->i_ino, &first, &flist);
-               libxfs_trans_ihold(tp, pip);
                flags |= XFS_ILOG_DEV;
                break;
 
@@ -531,7 +527,6 @@ parseproto(
                libxfs_trans_ijoin(tp, pip, 0);
                xname.type = XFS_DIR3_FT_FIFO;
                newdirent(mp, tp, pip, &xname, ip->i_ino, &first, &flist);
-               libxfs_trans_ihold(tp, pip);
                break;
        case IF_SYMLINK:
                buf = getstr(pp);
@@ -545,7 +540,6 @@ parseproto(
                libxfs_trans_ijoin(tp, pip, 0);
                xname.type = XFS_DIR3_FT_SYMLINK;
                newdirent(mp, tp, pip, &xname, ip->i_ino, &first, &flist);
-               libxfs_trans_ihold(tp, pip);
                break;
        case IF_DIRECTORY:
                getres(tp, 0);
@@ -565,7 +559,6 @@ parseproto(
                        newdirent(mp, tp, pip, &xname, ip->i_ino,
                                  &first, &flist);
                        pip->i_d.di_nlink++;
-                       libxfs_trans_ihold(tp, pip);
                        libxfs_trans_log_inode(tp, pip, XFS_ILOG_CORE);
                }
                newdirectory(mp, tp, ip, pip);
@@ -573,7 +566,6 @@ parseproto(
                error = libxfs_bmap_finish(&tp, &flist, &committed);
                if (error)
                        fail(_("Directory creation failed"), error);
-               libxfs_trans_ihold(tp, ip);
                libxfs_trans_commit(tp, 0);
                /*
                 * RT initialization.  Do this here to ensure that
@@ -603,6 +595,7 @@ parseproto(
                        error);
        }
        libxfs_trans_commit(tp, 0);
+       IRELE(ip);
 }
 
 void
@@ -665,7 +658,6 @@ rtinit(
        *(__uint64_t *)&rbmip->i_d.di_atime = 0;
        libxfs_trans_log_inode(tp, rbmip, XFS_ILOG_CORE);
        libxfs_mod_sb(tp, XFS_SB_RBMINO);
-       libxfs_trans_ihold(tp, rbmip);
        mp->m_rbmip = rbmip;
        error = libxfs_inode_alloc(&tp, NULL, S_IFREG, 1, 0,
                                        &creds, &fsxattrs, &rsumip);
@@ -676,7 +668,6 @@ rtinit(
        rsumip->i_d.di_size = mp->m_rsumsize;
        libxfs_trans_log_inode(tp, rsumip, XFS_ILOG_CORE);
        libxfs_mod_sb(tp, XFS_SB_RSUMINO);
-       libxfs_trans_ihold(tp, rsumip);
        libxfs_trans_commit(tp, 0);
        mp->m_rsumip = rsumip;
        /*
@@ -689,7 +680,6 @@ rtinit(
                res_failed(i);
 
        libxfs_trans_ijoin(tp, rbmip, 0);
-       libxfs_trans_ihold(tp, rbmip);
        bno = 0;
        xfs_bmap_init(&flist, &first);
        while (bno < mp->m_sb.sb_rbmblocks) {
@@ -726,7 +716,6 @@ rtinit(
        if (i)
                res_failed(i);
        libxfs_trans_ijoin(tp, rsumip, 0);
-       libxfs_trans_ihold(tp, rsumip);
        bno = 0;
        xfs_bmap_init(&flist, &first);
        while (bno < nsumblocks) {
@@ -762,7 +751,6 @@ rtinit(
                if (i)
                        res_failed(i);
                libxfs_trans_ijoin(tp, rbmip, 0);
-               libxfs_trans_ihold(tp, rbmip);
                xfs_bmap_init(&flist, &first);
                ebno = XFS_RTMIN(mp->m_sb.sb_rextents,
                        bno + NBBY * mp->m_sb.sb_blocksize);

diff --git a/repair/phase6.c b/repair/phase6.c
index 0c8a0ba1e4951eb0b9f6c1c076f5e75a70406eae..9b10f1669a11d16114e0f3231b4cedada73e8dd2 100644 (file)
--- a/repair/phase6.c
+++ b/repair/phase6.c
@@ -544,7 +544,6 @@ mk_rbmino(xfs_mount_t *mp)
         * commit changes
         */
        libxfs_trans_log_inode(tp, ip, XFS_ILOG_CORE);
-       libxfs_trans_ihold(tp, ip);
        libxfs_trans_commit(tp, XFS_TRANS_RELEASE_LOG_RES|XFS_TRANS_SYNC);
 
        /*
@@ -801,7 +800,6 @@ mk_rsumino(xfs_mount_t *mp)
         * commit changes
         */
        libxfs_trans_log_inode(tp, ip, XFS_ILOG_CORE);
-       libxfs_trans_ihold(tp, ip);
        libxfs_trans_commit(tp, XFS_TRANS_RELEASE_LOG_RES|XFS_TRANS_SYNC);
 
        /*
@@ -1063,6 +1061,8 @@ mk_orphanage(xfs_mount_t *mp)
 
 
        libxfs_trans_commit(tp, XFS_TRANS_RELEASE_LOG_RES|XFS_TRANS_SYNC);
+       IRELE(ip);
+       IRELE(pip);
        add_inode_reached(irec,ino_offset);
 
        return(ino);
@@ -1260,6 +1260,8 @@ mv_orphanage(
 
                libxfs_trans_commit(tp, XFS_TRANS_RELEASE_LOG_RES|XFS_TRANS_SYNC);
        }
+       IRELE(ino_p);
+       IRELE(orphanage_ip);
 }
 
 static int
@@ -1330,7 +1332,6 @@ longform_dir2_rebuild(
        if (error)
                res_failed(error);
        libxfs_trans_ijoin(tp, ip, 0);
-       libxfs_trans_ihold(tp, ip);
 
        if ((error = libxfs_bmap_last_offset(tp, ip, &lastblock,
                                                XFS_DATA_FORK)))
@@ -1370,7 +1371,6 @@ longform_dir2_rebuild(
                        res_failed(error);
 
                libxfs_trans_ijoin(tp, ip, 0);
-               libxfs_trans_ihold(tp, ip);
 
                xfs_bmap_init(&flist, &firstblock);
                error = libxfs_dir_createname(tp, ip, &p->name, p->inum,
@@ -1428,7 +1428,6 @@ dir2_kill_block(
        if (error)
                res_failed(error);
        libxfs_trans_ijoin(tp, ip, 0);
-       libxfs_trans_ihold(tp, ip);
        libxfs_trans_bjoin(tp, bp);
        memset(&args, 0, sizeof(args));
        xfs_bmap_init(&flist, &firstblock);
@@ -1616,7 +1615,6 @@ longform_dir2_entry_check_data(
        if (error)
                res_failed(error);
        libxfs_trans_ijoin(tp, ip, 0);
-       libxfs_trans_ihold(tp, ip);
        libxfs_trans_bjoin(tp, bp);
        libxfs_trans_bhold(tp, bp);
        xfs_bmap_init(&flist, &firstblock);
@@ -2800,7 +2798,6 @@ process_dir_inode(
                                res_failed(error);
 
                        libxfs_trans_ijoin(tp, ip, 0);
-                       libxfs_trans_ihold(tp, ip);
 
                        shortform_dir2_entry_check(mp, ino, ip, &dirty,
                                                irec, ino_offset,
@@ -2848,7 +2845,6 @@ process_dir_inode(
                        res_failed(error);
 
                libxfs_trans_ijoin(tp, ip, 0);
-               libxfs_trans_ihold(tp, ip);
 
                xfs_bmap_init(&flist, &first);
 
@@ -2910,7 +2906,6 @@ process_dir_inode(
                                res_failed(error);
 
                        libxfs_trans_ijoin(tp, ip, 0);
-                       libxfs_trans_ihold(tp, ip);
 
                        xfs_bmap_init(&flist, &first);
 

diff --git a/repair/phase7.c b/repair/phase7.c
index 26446d3d86cceeb1a1e6f412c3b47785277b6f73..d3fe95a8887421b12d87e1a0c8433fc698bf05d0 100644 (file)
--- a/repair/phase7.c
+++ b/repair/phase7.c
@@ -99,7 +99,6 @@ update_inode_nlinks(
        set_nlinks(&ip->i_d, ino, nlinks, &dirty);
 
        if (!dirty)  {
-               libxfs_trans_iput(tp, ip);
                libxfs_trans_cancel(tp, XFS_TRANS_RELEASE_LOG_RES);
        } else  {
                libxfs_trans_log_inode(tp, ip, XFS_ILOG_CORE);
@@ -113,6 +112,7 @@ update_inode_nlinks(
 
                ASSERT(error == 0);
        }
+       IRELE(ip);
 }
 
 void