From 260c85e85e5ffb250c210f32d8740cadf6b720b2 Mon Sep 17 00:00:00 2001 From: Mark Tinguely Date: Tue, 20 May 2014 18:30:11 +1000 Subject: [PATCH] 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 Reviewed-by: Christoph Hellwig Signed-off-by: Dave Chinner --- include/libxfs.h | 3 --- libxfs/trans.c | 59 +++++------------------------------------------- libxfs/util.c | 1 - libxfs/xfs.h | 1 - mkfs/proto.c | 16 ++----------- repair/phase6.c | 13 ++++------- repair/phase7.c | 2 +- 7 files changed, 13 insertions(+), 82 deletions(-) diff --git a/include/libxfs.h b/include/libxfs.h index f97ab6587..7203d7993 100644 --- 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 49af4d187..13d21b245 100644 --- 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 4c4032480..9504e3368 100644 --- 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 5a21590b1..30a316d32 100644 --- 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 5b68b23d0..72068f024 100644 --- 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 0c8a0ba1e..9b10f1669 100644 --- 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 26446d3d8..d3fe95a88 100644 --- 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 -- 2.39.5