From e98c414f9a3134fe7efc56ef8f1d394b54bfd40e Mon Sep 17 00:00:00 2001 From: Christoph Hellwig Date: Wed, 23 Jun 2010 18:11:15 +1000 Subject: xfs: simplify log item descriptor tracking Currently we track log item descriptor belonging to a transaction using a complex opencoded chunk allocator. This code has been there since day one and seems to work around the lack of an efficient slab allocator. This patch replaces it with dynamically allocated log item descriptors from a dedicated slab pool, linked to the transaction by a linked list. This allows to greatly simplify the log item descriptor tracking to the point where it's just a couple hundred lines in xfs_trans.c instead of a separate file. The external API has also been simplified while we're at it - the xfs_trans_add_item and xfs_trans_del_item functions to add/ delete items from a transaction have been simplified to the bare minium, and the xfs_trans_find_item function is replaced with a direct dereference of the li_desc field. All debug code walking the list of log items in a transaction is down to a simple list_for_each_entry. Note that we could easily use a singly linked list here instead of the double linked list from list.h as the fastpath only does deletion from sequential traversal. But given that we don't have one available as a library function yet I use the list.h functions for simplicity. Signed-off-by: Christoph Hellwig Reviewed-by: Dave Chinner --- fs/xfs/xfs_trans.h | 105 ++++------------------------------------------------- 1 file changed, 7 insertions(+), 98 deletions(-) (limited to 'fs/xfs/xfs_trans.h') diff --git a/fs/xfs/xfs_trans.h b/fs/xfs/xfs_trans.h index e639e8e9a2a9..0c903eb8bbe1 100644 --- a/fs/xfs/xfs_trans.h +++ b/fs/xfs/xfs_trans.h @@ -161,105 +161,14 @@ typedef struct xfs_trans_header { * the amount of space needed to log the item it describes * once we get to commit processing (see xfs_trans_commit()). */ -typedef struct xfs_log_item_desc { +struct xfs_log_item_desc { struct xfs_log_item *lid_item; - ushort lid_size; - unsigned char lid_flags; - unsigned char lid_index; -} xfs_log_item_desc_t; + ushort lid_size; + unsigned char lid_flags; + struct list_head lid_trans; +}; #define XFS_LID_DIRTY 0x1 -#define XFS_LID_PINNED 0x2 - -/* - * This structure is used to maintain a chunk list of log_item_desc - * structures. The free field is a bitmask indicating which descriptors - * in this chunk's array are free. The unused field is the first value - * not used since this chunk was allocated. - */ -#define XFS_LIC_NUM_SLOTS 15 -typedef struct xfs_log_item_chunk { - struct xfs_log_item_chunk *lic_next; - ushort lic_free; - ushort lic_unused; - xfs_log_item_desc_t lic_descs[XFS_LIC_NUM_SLOTS]; -} xfs_log_item_chunk_t; - -#define XFS_LIC_MAX_SLOT (XFS_LIC_NUM_SLOTS - 1) -#define XFS_LIC_FREEMASK ((1 << XFS_LIC_NUM_SLOTS) - 1) - - -/* - * Initialize the given chunk. Set the chunk's free descriptor mask - * to indicate that all descriptors are free. The caller gets to set - * lic_unused to the right value (0 matches all free). The - * lic_descs.lid_index values are set up as each desc is allocated. - */ -static inline void xfs_lic_init(xfs_log_item_chunk_t *cp) -{ - cp->lic_free = XFS_LIC_FREEMASK; -} - -static inline void xfs_lic_init_slot(xfs_log_item_chunk_t *cp, int slot) -{ - cp->lic_descs[slot].lid_index = (unsigned char)(slot); -} - -static inline int xfs_lic_vacancy(xfs_log_item_chunk_t *cp) -{ - return cp->lic_free & XFS_LIC_FREEMASK; -} - -static inline void xfs_lic_all_free(xfs_log_item_chunk_t *cp) -{ - cp->lic_free = XFS_LIC_FREEMASK; -} - -static inline int xfs_lic_are_all_free(xfs_log_item_chunk_t *cp) -{ - return ((cp->lic_free & XFS_LIC_FREEMASK) == XFS_LIC_FREEMASK); -} - -static inline int xfs_lic_isfree(xfs_log_item_chunk_t *cp, int slot) -{ - return (cp->lic_free & (1 << slot)); -} - -static inline void xfs_lic_claim(xfs_log_item_chunk_t *cp, int slot) -{ - cp->lic_free &= ~(1 << slot); -} - -static inline void xfs_lic_relse(xfs_log_item_chunk_t *cp, int slot) -{ - cp->lic_free |= 1 << slot; -} - -static inline xfs_log_item_desc_t * -xfs_lic_slot(xfs_log_item_chunk_t *cp, int slot) -{ - return &(cp->lic_descs[slot]); -} - -static inline int xfs_lic_desc_to_slot(xfs_log_item_desc_t *dp) -{ - return (uint)dp->lid_index; -} - -/* - * Calculate the address of a chunk given a descriptor pointer: - * dp - dp->lid_index give the address of the start of the lic_descs array. - * From this we subtract the offset of the lic_descs field in a chunk. - * All of this yields the address of the chunk, which is - * cast to a chunk pointer. - */ -static inline xfs_log_item_chunk_t * -xfs_lic_desc_to_chunk(xfs_log_item_desc_t *dp) -{ - return (xfs_log_item_chunk_t*) \ - (((xfs_caddr_t)((dp) - (dp)->lid_index)) - \ - (xfs_caddr_t)(((xfs_log_item_chunk_t*)0)->lic_descs)); -} #define XFS_TRANS_MAGIC 0x5452414E /* 'TRAN' */ /* @@ -516,8 +425,7 @@ typedef struct xfs_trans { int64_t t_rblocks_delta;/* superblock rblocks change */ int64_t t_rextents_delta;/* superblocks rextents chg */ int64_t t_rextslog_delta;/* superblocks rextslog chg */ - unsigned int t_items_free; /* log item descs free */ - xfs_log_item_chunk_t t_items; /* first log item desc chunk */ + struct list_head t_items; /* log item descriptors */ xfs_trans_header_t t_header; /* header for in-log trans */ struct list_head t_busy; /* list of busy extents */ unsigned long t_pflags; /* saved process flags state */ @@ -595,6 +503,7 @@ int xfs_trans_ail_init(struct xfs_mount *); void xfs_trans_ail_destroy(struct xfs_mount *); extern kmem_zone_t *xfs_trans_zone; +extern kmem_zone_t *xfs_log_item_desc_zone; #endif /* __KERNEL__ */ -- cgit v1.2.3 From 9412e3181c0ef82efc3d8e88d73e583ec10c34e9 Mon Sep 17 00:00:00 2001 From: Christoph Hellwig Date: Wed, 23 Jun 2010 18:11:15 +1000 Subject: xfs: merge iop_unpin_remove into iop_unpin The unpin_remove item operation instances always share most of the implementation with the respective unpin implementation. So instead of keeping two different entry points add a remove flag to the unpin operation and share the code more easily. Signed-off-by: Christoph Hellwig Reviewed-by: Dave Chinner --- fs/xfs/quota/xfs_dquot_item.c | 33 ++++--------------- fs/xfs/xfs_buf_item.c | 74 ++++++++++++++++--------------------------- fs/xfs/xfs_extfree_item.c | 49 ++++------------------------ fs/xfs/xfs_inode_item.c | 17 ++-------- fs/xfs/xfs_trans.c | 4 +-- fs/xfs/xfs_trans.h | 6 ++-- 6 files changed, 47 insertions(+), 136 deletions(-) (limited to 'fs/xfs/xfs_trans.h') diff --git a/fs/xfs/quota/xfs_dquot_item.c b/fs/xfs/quota/xfs_dquot_item.c index fb7054c1539d..fa2b6744937e 100644 --- a/fs/xfs/quota/xfs_dquot_item.c +++ b/fs/xfs/quota/xfs_dquot_item.c @@ -97,7 +97,8 @@ xfs_qm_dquot_logitem_pin( /* ARGSUSED */ STATIC void xfs_qm_dquot_logitem_unpin( - xfs_dq_logitem_t *logitem) + xfs_dq_logitem_t *logitem, + int remove) { xfs_dquot_t *dqp = logitem->qli_dquot; @@ -106,15 +107,6 @@ xfs_qm_dquot_logitem_unpin( wake_up(&dqp->q_pinwait); } -/* ARGSUSED */ -STATIC void -xfs_qm_dquot_logitem_unpin_remove( - xfs_dq_logitem_t *logitem, - xfs_trans_t *tp) -{ - xfs_qm_dquot_logitem_unpin(logitem); -} - /* * Given the logitem, this writes the corresponding dquot entry to disk * asynchronously. This is called with the dquot entry securely locked; @@ -318,9 +310,7 @@ static struct xfs_item_ops xfs_dquot_item_ops = { .iop_format = (void(*)(xfs_log_item_t*, xfs_log_iovec_t*)) xfs_qm_dquot_logitem_format, .iop_pin = (void(*)(xfs_log_item_t*))xfs_qm_dquot_logitem_pin, - .iop_unpin = (void(*)(xfs_log_item_t*))xfs_qm_dquot_logitem_unpin, - .iop_unpin_remove = (void(*)(xfs_log_item_t*, xfs_trans_t*)) - xfs_qm_dquot_logitem_unpin_remove, + .iop_unpin = (void(*)(xfs_log_item_t*, int))xfs_qm_dquot_logitem_unpin, .iop_trylock = (uint(*)(xfs_log_item_t*)) xfs_qm_dquot_logitem_trylock, .iop_unlock = (void(*)(xfs_log_item_t*))xfs_qm_dquot_logitem_unlock, @@ -413,14 +403,7 @@ xfs_qm_qoff_logitem_pin(xfs_qoff_logitem_t *qf) */ /*ARGSUSED*/ STATIC void -xfs_qm_qoff_logitem_unpin(xfs_qoff_logitem_t *qf) -{ - return; -} - -/*ARGSUSED*/ -STATIC void -xfs_qm_qoff_logitem_unpin_remove(xfs_qoff_logitem_t *qf, xfs_trans_t *tp) +xfs_qm_qoff_logitem_unpin(xfs_qoff_logitem_t *qf, int remove) { return; } @@ -524,9 +507,7 @@ static struct xfs_item_ops xfs_qm_qoffend_logitem_ops = { .iop_format = (void(*)(xfs_log_item_t*, xfs_log_iovec_t*)) xfs_qm_qoff_logitem_format, .iop_pin = (void(*)(xfs_log_item_t*))xfs_qm_qoff_logitem_pin, - .iop_unpin = (void(*)(xfs_log_item_t*))xfs_qm_qoff_logitem_unpin, - .iop_unpin_remove = (void(*)(xfs_log_item_t*,xfs_trans_t*)) - xfs_qm_qoff_logitem_unpin_remove, + .iop_unpin = (void(*)(xfs_log_item_t*, int))xfs_qm_qoff_logitem_unpin, .iop_trylock = (uint(*)(xfs_log_item_t*))xfs_qm_qoff_logitem_trylock, .iop_unlock = (void(*)(xfs_log_item_t*))xfs_qm_qoff_logitem_unlock, .iop_committed = (xfs_lsn_t(*)(xfs_log_item_t*, xfs_lsn_t)) @@ -545,9 +526,7 @@ static struct xfs_item_ops xfs_qm_qoff_logitem_ops = { .iop_format = (void(*)(xfs_log_item_t*, xfs_log_iovec_t*)) xfs_qm_qoff_logitem_format, .iop_pin = (void(*)(xfs_log_item_t*))xfs_qm_qoff_logitem_pin, - .iop_unpin = (void(*)(xfs_log_item_t*))xfs_qm_qoff_logitem_unpin, - .iop_unpin_remove = (void(*)(xfs_log_item_t*,xfs_trans_t*)) - xfs_qm_qoff_logitem_unpin_remove, + .iop_unpin = (void(*)(xfs_log_item_t*, int))xfs_qm_qoff_logitem_unpin, .iop_trylock = (uint(*)(xfs_log_item_t*))xfs_qm_qoff_logitem_trylock, .iop_unlock = (void(*)(xfs_log_item_t*))xfs_qm_qoff_logitem_unlock, .iop_committed = (xfs_lsn_t(*)(xfs_log_item_t*, xfs_lsn_t)) diff --git a/fs/xfs/xfs_buf_item.c b/fs/xfs/xfs_buf_item.c index 711f69abbbe4..93899953c603 100644 --- a/fs/xfs/xfs_buf_item.c +++ b/fs/xfs/xfs_buf_item.c @@ -388,20 +388,25 @@ xfs_buf_item_pin( * Also drop the reference to the buf item for the current transaction. * If the XFS_BLI_STALE flag is set and we are the last reference, * then free up the buf log item and unlock the buffer. + * + * If the remove flag is set we are called from uncommit in the + * forced-shutdown path. If that is true and the reference count on + * the log item is going to drop to zero we need to free the item's + * descriptor in the transaction. */ STATIC void xfs_buf_item_unpin( - xfs_buf_log_item_t *bip) + xfs_buf_log_item_t *bip, + int remove) { struct xfs_ail *ailp; - xfs_buf_t *bp; + xfs_buf_t *bp = bip->bli_buf; int freed; int stale = bip->bli_flags & XFS_BLI_STALE; - bp = bip->bli_buf; - ASSERT(bp != NULL); ASSERT(XFS_BUF_FSPRIVATE(bp, xfs_buf_log_item_t *) == bip); ASSERT(atomic_read(&bip->bli_refcount) > 0); + trace_xfs_buf_item_unpin(bip); freed = atomic_dec_and_test(&bip->bli_refcount); @@ -413,8 +418,26 @@ xfs_buf_item_unpin( ASSERT(!(XFS_BUF_ISDELAYWRITE(bp))); ASSERT(XFS_BUF_ISSTALE(bp)); ASSERT(bip->bli_format.blf_flags & XFS_BLF_CANCEL); + trace_xfs_buf_item_unpin_stale(bip); + if (remove) { + /* + * We have to remove the log item from the transaction + * as we are about to release our reference to the + * buffer. If we don't, the unlock that occurs later + * in xfs_trans_uncommit() will ry to reference the + * buffer which we no longer have a hold on. + */ + xfs_trans_del_item(&bip->bli_item); + + /* + * Since the transaction no longer refers to the buffer, + * the buffer should no longer refer to the transaction. + */ + XFS_BUF_SET_FSPRIVATE2(bp, NULL); + } + /* * If we get called here because of an IO error, we may * or may not have the item on the AIL. xfs_trans_ail_delete() @@ -435,45 +458,6 @@ xfs_buf_item_unpin( } } -/* - * this is called from uncommit in the forced-shutdown path. - * we need to check to see if the reference count on the log item - * is going to drop to zero. If so, unpin will free the log item - * so we need to free the item's descriptor (that points to the item) - * in the transaction. - */ -STATIC void -xfs_buf_item_unpin_remove( - xfs_buf_log_item_t *bip, - xfs_trans_t *tp) -{ - /* will xfs_buf_item_unpin() call xfs_buf_item_relse()? */ - if ((atomic_read(&bip->bli_refcount) == 1) && - (bip->bli_flags & XFS_BLI_STALE)) { - /* - * yes -- We can safely do some work here and then call - * buf_item_unpin to do the rest because we are - * are holding the buffer locked so no one else will be - * able to bump up the refcount. We have to remove the - * log item from the transaction as we are about to release - * our reference to the buffer. If we don't, the unlock that - * occurs later in the xfs_trans_uncommit() will try to - * reference the buffer which we no longer have a hold on. - */ - ASSERT(XFS_BUF_VALUSEMA(bip->bli_buf) <= 0); - trace_xfs_buf_item_unpin_stale(bip); - - xfs_trans_del_item(&bip->bli_item); - - /* - * Since the transaction no longer refers to the buffer, the - * buffer should no longer refer to the transaction. - */ - XFS_BUF_SET_FSPRIVATE2(bip->bli_buf, NULL); - } - xfs_buf_item_unpin(bip); -} - /* * This is called to attempt to lock the buffer associated with this * buf log item. Don't sleep on the buffer lock. If we can't get @@ -669,9 +653,7 @@ static struct xfs_item_ops xfs_buf_item_ops = { .iop_format = (void(*)(xfs_log_item_t*, xfs_log_iovec_t*)) xfs_buf_item_format, .iop_pin = (void(*)(xfs_log_item_t*))xfs_buf_item_pin, - .iop_unpin = (void(*)(xfs_log_item_t*))xfs_buf_item_unpin, - .iop_unpin_remove = (void(*)(xfs_log_item_t*, xfs_trans_t *)) - xfs_buf_item_unpin_remove, + .iop_unpin = (void(*)(xfs_log_item_t*, int))xfs_buf_item_unpin, .iop_trylock = (uint(*)(xfs_log_item_t*))xfs_buf_item_trylock, .iop_unlock = (void(*)(xfs_log_item_t*))xfs_buf_item_unlock, .iop_committed = (xfs_lsn_t(*)(xfs_log_item_t*, xfs_lsn_t)) diff --git a/fs/xfs/xfs_extfree_item.c b/fs/xfs/xfs_extfree_item.c index 8d0e543ca3c0..6ac7e596c54c 100644 --- a/fs/xfs/xfs_extfree_item.c +++ b/fs/xfs/xfs_extfree_item.c @@ -103,32 +103,8 @@ xfs_efi_item_pin(xfs_efi_log_item_t *efip) * Here we coordinate with xfs_efi_cancel() to determine who gets to * free the EFI. */ -/*ARGSUSED*/ -STATIC void -xfs_efi_item_unpin(xfs_efi_log_item_t *efip) -{ - struct xfs_ail *ailp = efip->efi_item.li_ailp; - - spin_lock(&ailp->xa_lock); - if (efip->efi_flags & XFS_EFI_CANCELED) { - /* xfs_trans_ail_delete() drops the AIL lock. */ - xfs_trans_ail_delete(ailp, (xfs_log_item_t *)efip); - xfs_efi_item_free(efip); - } else { - efip->efi_flags |= XFS_EFI_COMMITTED; - spin_unlock(&ailp->xa_lock); - } -} - -/* - * like unpin only we have to also clear the xaction descriptor - * pointing the log item if we free the item. This routine duplicates - * unpin because efi_flags is protected by the AIL lock. Freeing - * the descriptor and then calling unpin would force us to drop the AIL - * lock which would open up a race condition. - */ STATIC void -xfs_efi_item_unpin_remove(xfs_efi_log_item_t *efip, xfs_trans_t *tp) +xfs_efi_item_unpin(xfs_efi_log_item_t *efip, int remove) { struct xfs_ail *ailp = efip->efi_item.li_ailp; @@ -136,10 +112,8 @@ xfs_efi_item_unpin_remove(xfs_efi_log_item_t *efip, xfs_trans_t *tp) if (efip->efi_flags & XFS_EFI_CANCELED) { struct xfs_log_item *lip = &efip->efi_item; - /* - * free the xaction descriptor pointing to this item - */ - xfs_trans_del_item(lip); + if (remove) + xfs_trans_del_item(lip); /* xfs_trans_ail_delete() drops the AIL lock. */ xfs_trans_ail_delete(ailp, lip); @@ -223,9 +197,7 @@ static struct xfs_item_ops xfs_efi_item_ops = { .iop_format = (void(*)(xfs_log_item_t*, xfs_log_iovec_t*)) xfs_efi_item_format, .iop_pin = (void(*)(xfs_log_item_t*))xfs_efi_item_pin, - .iop_unpin = (void(*)(xfs_log_item_t*))xfs_efi_item_unpin, - .iop_unpin_remove = (void(*)(xfs_log_item_t*, xfs_trans_t *)) - xfs_efi_item_unpin_remove, + .iop_unpin = (void(*)(xfs_log_item_t*, int))xfs_efi_item_unpin, .iop_trylock = (uint(*)(xfs_log_item_t*))xfs_efi_item_trylock, .iop_unlock = (void(*)(xfs_log_item_t*))xfs_efi_item_unlock, .iop_committed = (xfs_lsn_t(*)(xfs_log_item_t*, xfs_lsn_t)) @@ -424,14 +396,7 @@ xfs_efd_item_pin(xfs_efd_log_item_t *efdp) */ /*ARGSUSED*/ STATIC void -xfs_efd_item_unpin(xfs_efd_log_item_t *efdp) -{ - return; -} - -/*ARGSUSED*/ -STATIC void -xfs_efd_item_unpin_remove(xfs_efd_log_item_t *efdp, xfs_trans_t *tp) +xfs_efd_item_unpin(xfs_efd_log_item_t *efdp, int remove) { return; } @@ -514,9 +479,7 @@ static struct xfs_item_ops xfs_efd_item_ops = { .iop_format = (void(*)(xfs_log_item_t*, xfs_log_iovec_t*)) xfs_efd_item_format, .iop_pin = (void(*)(xfs_log_item_t*))xfs_efd_item_pin, - .iop_unpin = (void(*)(xfs_log_item_t*))xfs_efd_item_unpin, - .iop_unpin_remove = (void(*)(xfs_log_item_t*, xfs_trans_t*)) - xfs_efd_item_unpin_remove, + .iop_unpin = (void(*)(xfs_log_item_t*, int))xfs_efd_item_unpin, .iop_trylock = (uint(*)(xfs_log_item_t*))xfs_efd_item_trylock, .iop_unlock = (void(*)(xfs_log_item_t*))xfs_efd_item_unlock, .iop_committed = (xfs_lsn_t(*)(xfs_log_item_t*, xfs_lsn_t)) diff --git a/fs/xfs/xfs_inode_item.c b/fs/xfs/xfs_inode_item.c index 4b97d7754b83..a01990dbb945 100644 --- a/fs/xfs/xfs_inode_item.c +++ b/fs/xfs/xfs_inode_item.c @@ -544,10 +544,10 @@ xfs_inode_item_pin( * * Also wake up anyone in xfs_iunpin_wait() if the count goes to 0. */ -/* ARGSUSED */ STATIC void xfs_inode_item_unpin( - xfs_inode_log_item_t *iip) + xfs_inode_log_item_t *iip, + int remove) { struct xfs_inode *ip = iip->ili_inode; @@ -557,15 +557,6 @@ xfs_inode_item_unpin( wake_up(&ip->i_ipin_wait); } -/* ARGSUSED */ -STATIC void -xfs_inode_item_unpin_remove( - xfs_inode_log_item_t *iip, - xfs_trans_t *tp) -{ - xfs_inode_item_unpin(iip); -} - /* * This is called to attempt to lock the inode associated with this * inode log item, in preparation for the push routine which does the actual @@ -829,9 +820,7 @@ static struct xfs_item_ops xfs_inode_item_ops = { .iop_format = (void(*)(xfs_log_item_t*, xfs_log_iovec_t*)) xfs_inode_item_format, .iop_pin = (void(*)(xfs_log_item_t*))xfs_inode_item_pin, - .iop_unpin = (void(*)(xfs_log_item_t*))xfs_inode_item_unpin, - .iop_unpin_remove = (void(*)(xfs_log_item_t*, xfs_trans_t*)) - xfs_inode_item_unpin_remove, + .iop_unpin = (void(*)(xfs_log_item_t*, int))xfs_inode_item_unpin, .iop_trylock = (uint(*)(xfs_log_item_t*))xfs_inode_item_trylock, .iop_unlock = (void(*)(xfs_log_item_t*))xfs_inode_item_unlock, .iop_committed = (xfs_lsn_t(*)(xfs_log_item_t*, xfs_lsn_t)) diff --git a/fs/xfs/xfs_trans.c b/fs/xfs/xfs_trans.c index 9c41efccf728..213792e1ad02 100644 --- a/fs/xfs/xfs_trans.c +++ b/fs/xfs/xfs_trans.c @@ -1375,7 +1375,7 @@ xfs_trans_item_committed( * log item flags, if anyone else stales the buffer we do not want to * pay any attention to it. */ - IOP_UNPIN(lip); + IOP_UNPIN(lip, 0); } /* @@ -1422,7 +1422,7 @@ xfs_trans_uncommit( * Unpin all but those that aren't dirty. */ if (lidp->lid_flags & XFS_LID_DIRTY) - IOP_UNPIN_REMOVE(lidp->lid_item, tp); + IOP_UNPIN(lidp->lid_item, 1); } xfs_trans_unreserve_and_mod_sb(tp); diff --git a/fs/xfs/xfs_trans.h b/fs/xfs/xfs_trans.h index 0c903eb8bbe1..37c0ce1ccd48 100644 --- a/fs/xfs/xfs_trans.h +++ b/fs/xfs/xfs_trans.h @@ -347,8 +347,7 @@ typedef struct xfs_item_ops { uint (*iop_size)(xfs_log_item_t *); void (*iop_format)(xfs_log_item_t *, struct xfs_log_iovec *); void (*iop_pin)(xfs_log_item_t *); - void (*iop_unpin)(xfs_log_item_t *); - void (*iop_unpin_remove)(xfs_log_item_t *, struct xfs_trans *); + void (*iop_unpin)(xfs_log_item_t *, int remove); uint (*iop_trylock)(xfs_log_item_t *); void (*iop_unlock)(xfs_log_item_t *); xfs_lsn_t (*iop_committed)(xfs_log_item_t *, xfs_lsn_t); @@ -360,8 +359,7 @@ typedef struct xfs_item_ops { #define IOP_SIZE(ip) (*(ip)->li_ops->iop_size)(ip) #define IOP_FORMAT(ip,vp) (*(ip)->li_ops->iop_format)(ip, vp) #define IOP_PIN(ip) (*(ip)->li_ops->iop_pin)(ip) -#define IOP_UNPIN(ip) (*(ip)->li_ops->iop_unpin)(ip) -#define IOP_UNPIN_REMOVE(ip,tp) (*(ip)->li_ops->iop_unpin_remove)(ip, tp) +#define IOP_UNPIN(ip, remove) (*(ip)->li_ops->iop_unpin)(ip, remove) #define IOP_TRYLOCK(ip) (*(ip)->li_ops->iop_trylock)(ip) #define IOP_UNLOCK(ip) (*(ip)->li_ops->iop_unlock)(ip) #define IOP_COMMITTED(ip, lsn) (*(ip)->li_ops->iop_committed)(ip, lsn) -- cgit v1.2.3 From 898621d5a72c6799a9a13fce20443b4b6699899c Mon Sep 17 00:00:00 2001 From: Christoph Hellwig Date: Thu, 24 Jun 2010 11:36:58 +1000 Subject: xfs: simplify inode to transaction joining Currently we need to either call IHOLD or xfs_trans_ihold on an inode when joining it to a transaction via xfs_trans_ijoin. This patches instead makes xfs_trans_ijoin usable on it's own by doing an implicity xfs_trans_ihold, which also allows us to drop the third argument. For the case where we want to hold a reference on the inode a xfs_trans_ijoin_ref wrapper is added which does the IHOLD and marks the inode for needing an xfs_iput. In addition to the cleaner interface to the caller this also simplifies the implementation. Signed-off-by: Christoph Hellwig Reviewed-by: Dave Chinner --- fs/xfs/linux-2.6/xfs_file.c | 3 +- fs/xfs/linux-2.6/xfs_ioctl.c | 3 +- fs/xfs/linux-2.6/xfs_super.c | 3 +- fs/xfs/linux-2.6/xfs_sync.c | 3 +- fs/xfs/quota/xfs_dquot.c | 9 +----- fs/xfs/xfs_attr.c | 75 +++++++++++++++----------------------------- fs/xfs/xfs_bmap.c | 5 +-- fs/xfs/xfs_dfrag.c | 7 ++--- fs/xfs/xfs_fsops.c | 3 +- fs/xfs/xfs_inode.c | 14 +++------ fs/xfs/xfs_inode_item.c | 42 +++---------------------- fs/xfs/xfs_inode_item.h | 8 +---- fs/xfs/xfs_iomap.c | 9 ++---- fs/xfs/xfs_rename.c | 26 +++++---------- fs/xfs/xfs_trans.c | 3 +- fs/xfs/xfs_trans.h | 4 +-- fs/xfs/xfs_trans_inode.c | 60 ++++++++++++++--------------------- fs/xfs/xfs_utils.c | 4 +-- fs/xfs/xfs_vnodeops.c | 69 +++++++++++----------------------------- 19 files changed, 103 insertions(+), 247 deletions(-) (limited to 'fs/xfs/xfs_trans.h') diff --git a/fs/xfs/linux-2.6/xfs_file.c b/fs/xfs/linux-2.6/xfs_file.c index 8d26d93648b4..9a9b446a58a7 100644 --- a/fs/xfs/linux-2.6/xfs_file.c +++ b/fs/xfs/linux-2.6/xfs_file.c @@ -158,8 +158,7 @@ xfs_file_fsync( * transaction. So we play it safe and fire off the * transaction anyway. */ - xfs_trans_ijoin(tp, ip, XFS_ILOCK_EXCL); - xfs_trans_ihold(tp, ip); + xfs_trans_ijoin(tp, ip); xfs_trans_log_inode(tp, ip, XFS_ILOG_CORE); xfs_trans_set_sync(tp); error = _xfs_trans_commit(tp, 0, &log_flushed); diff --git a/fs/xfs/linux-2.6/xfs_ioctl.c b/fs/xfs/linux-2.6/xfs_ioctl.c index 8aa54f0eec15..a12dddad126e 100644 --- a/fs/xfs/linux-2.6/xfs_ioctl.c +++ b/fs/xfs/linux-2.6/xfs_ioctl.c @@ -1034,8 +1034,7 @@ xfs_ioctl_setattr( } } - xfs_trans_ijoin(tp, ip, lock_flags); - xfs_trans_ihold(tp, ip); + xfs_trans_ijoin(tp, ip); /* * Change file ownership. Must be the owner or privileged. diff --git a/fs/xfs/linux-2.6/xfs_super.c b/fs/xfs/linux-2.6/xfs_super.c index 4b90e4b531b7..b8ad17e730b6 100644 --- a/fs/xfs/linux-2.6/xfs_super.c +++ b/fs/xfs/linux-2.6/xfs_super.c @@ -1023,8 +1023,7 @@ xfs_log_inode( * an inode in another recent transaction. So we play it safe and * fire off the transaction anyway. */ - xfs_trans_ijoin(tp, ip, XFS_ILOCK_EXCL); - xfs_trans_ihold(tp, ip); + xfs_trans_ijoin(tp, ip); xfs_trans_log_inode(tp, ip, XFS_ILOG_CORE); xfs_trans_set_sync(tp); error = xfs_trans_commit(tp, 0); diff --git a/fs/xfs/linux-2.6/xfs_sync.c b/fs/xfs/linux-2.6/xfs_sync.c index 850b4198bf60..0283b88bc16c 100644 --- a/fs/xfs/linux-2.6/xfs_sync.c +++ b/fs/xfs/linux-2.6/xfs_sync.c @@ -362,8 +362,7 @@ xfs_commit_dummy_trans( xfs_ilock(ip, XFS_ILOCK_EXCL); - xfs_trans_ijoin(tp, ip, XFS_ILOCK_EXCL); - xfs_trans_ihold(tp, ip); + xfs_trans_ijoin(tp, ip); xfs_trans_log_inode(tp, ip, XFS_ILOG_CORE); error = xfs_trans_commit(tp, 0); xfs_iunlock(ip, XFS_ILOCK_EXCL); diff --git a/fs/xfs/quota/xfs_dquot.c b/fs/xfs/quota/xfs_dquot.c index 6526e87cade0..56f366e327f3 100644 --- a/fs/xfs/quota/xfs_dquot.c +++ b/fs/xfs/quota/xfs_dquot.c @@ -378,14 +378,7 @@ xfs_qm_dqalloc( return (ESRCH); } - /* - * xfs_trans_commit normally decrements the vnode ref count - * when it unlocks the inode. Since we want to keep the quota - * inode around, we bump the vnode ref count now. - */ - IHOLD(quotip); - - xfs_trans_ijoin(tp, quotip, XFS_ILOCK_EXCL); + xfs_trans_ijoin_ref(tp, quotip, XFS_ILOCK_EXCL); nmaps = 1; if ((error = xfs_bmapi(tp, quotip, offset_fsb, XFS_DQUOT_CLUSTER_SIZE_FSB, diff --git a/fs/xfs/xfs_attr.c b/fs/xfs/xfs_attr.c index 8bde79785a75..f3ca7186155a 100644 --- a/fs/xfs/xfs_attr.c +++ b/fs/xfs/xfs_attr.c @@ -319,8 +319,7 @@ xfs_attr_set_int( return (error); } - xfs_trans_ijoin(args.trans, dp, XFS_ILOCK_EXCL); - xfs_trans_ihold(args.trans, dp); + xfs_trans_ijoin(args.trans, dp); /* * If the attribute list is non-existent or a shortform list, @@ -390,10 +389,8 @@ xfs_attr_set_int( * bmap_finish() may have committed the last trans and started * a new one. We need the inode to be in all transactions. */ - if (committed) { - xfs_trans_ijoin(args.trans, dp, XFS_ILOCK_EXCL); - xfs_trans_ihold(args.trans, dp); - } + if (committed) + xfs_trans_ijoin(args.trans, dp); /* * Commit the leaf transformation. We'll need another (linked) @@ -538,8 +535,7 @@ xfs_attr_remove_int(xfs_inode_t *dp, struct xfs_name *name, int flags) * No need to make quota reservations here. We expect to release some * blocks not allocate in the common case. */ - xfs_trans_ijoin(args.trans, dp, XFS_ILOCK_EXCL); - xfs_trans_ihold(args.trans, dp); + xfs_trans_ijoin(args.trans, dp); /* * Decide on what work routines to call based on the inode size. @@ -815,8 +811,7 @@ xfs_attr_inactive(xfs_inode_t *dp) * No need to make quota reservations here. We expect to release some * blocks, not allocate, in the common case. */ - xfs_trans_ijoin(trans, dp, XFS_ILOCK_EXCL); - xfs_trans_ihold(trans, dp); + xfs_trans_ijoin(trans, dp); /* * Decide on what work routines to call based on the inode size. @@ -975,10 +970,8 @@ xfs_attr_leaf_addname(xfs_da_args_t *args) * bmap_finish() may have committed the last trans and started * a new one. We need the inode to be in all transactions. */ - if (committed) { - xfs_trans_ijoin(args->trans, dp, XFS_ILOCK_EXCL); - xfs_trans_ihold(args->trans, dp); - } + if (committed) + xfs_trans_ijoin(args->trans, dp); /* * Commit the current trans (including the inode) and start @@ -1079,10 +1072,8 @@ xfs_attr_leaf_addname(xfs_da_args_t *args) * and started a new one. We need the inode to be * in all transactions. */ - if (committed) { - xfs_trans_ijoin(args->trans, dp, XFS_ILOCK_EXCL); - xfs_trans_ihold(args->trans, dp); - } + if (committed) + xfs_trans_ijoin(args->trans, dp); } else xfs_da_buf_done(bp); @@ -1155,10 +1146,8 @@ xfs_attr_leaf_removename(xfs_da_args_t *args) * bmap_finish() may have committed the last trans and started * a new one. We need the inode to be in all transactions. */ - if (committed) { - xfs_trans_ijoin(args->trans, dp, XFS_ILOCK_EXCL); - xfs_trans_ihold(args->trans, dp); - } + if (committed) + xfs_trans_ijoin(args->trans, dp); } else xfs_da_buf_done(bp); return(0); @@ -1311,10 +1300,8 @@ restart: * and started a new one. We need the inode to be * in all transactions. */ - if (committed) { - xfs_trans_ijoin(args->trans, dp, XFS_ILOCK_EXCL); - xfs_trans_ihold(args->trans, dp); - } + if (committed) + xfs_trans_ijoin(args->trans, dp); /* * Commit the node conversion and start the next @@ -1350,10 +1337,8 @@ restart: * bmap_finish() may have committed the last trans and started * a new one. We need the inode to be in all transactions. */ - if (committed) { - xfs_trans_ijoin(args->trans, dp, XFS_ILOCK_EXCL); - xfs_trans_ihold(args->trans, dp); - } + if (committed) + xfs_trans_ijoin(args->trans, dp); } else { /* * Addition succeeded, update Btree hashvals. @@ -1464,10 +1449,8 @@ restart: * and started a new one. We need the inode to be * in all transactions. */ - if (committed) { - xfs_trans_ijoin(args->trans, dp, XFS_ILOCK_EXCL); - xfs_trans_ihold(args->trans, dp); - } + if (committed) + xfs_trans_ijoin(args->trans, dp); } /* @@ -1598,10 +1581,8 @@ xfs_attr_node_removename(xfs_da_args_t *args) * bmap_finish() may have committed the last trans and started * a new one. We need the inode to be in all transactions. */ - if (committed) { - xfs_trans_ijoin(args->trans, dp, XFS_ILOCK_EXCL); - xfs_trans_ihold(args->trans, dp); - } + if (committed) + xfs_trans_ijoin(args->trans, dp); /* * Commit the Btree join operation and start a new trans. @@ -1652,10 +1633,8 @@ xfs_attr_node_removename(xfs_da_args_t *args) * and started a new one. We need the inode to be * in all transactions. */ - if (committed) { - xfs_trans_ijoin(args->trans, dp, XFS_ILOCK_EXCL); - xfs_trans_ihold(args->trans, dp); - } + if (committed) + xfs_trans_ijoin(args->trans, dp); } else xfs_da_brelse(args->trans, bp); } @@ -2093,10 +2072,8 @@ xfs_attr_rmtval_set(xfs_da_args_t *args) * bmap_finish() may have committed the last trans and started * a new one. We need the inode to be in all transactions. */ - if (committed) { - xfs_trans_ijoin(args->trans, dp, XFS_ILOCK_EXCL); - xfs_trans_ihold(args->trans, dp); - } + if (committed) + xfs_trans_ijoin(args->trans, dp); ASSERT(nmap == 1); ASSERT((map.br_startblock != DELAYSTARTBLOCK) && @@ -2249,10 +2226,8 @@ xfs_attr_rmtval_remove(xfs_da_args_t *args) * bmap_finish() may have committed the last trans and started * a new one. We need the inode to be in all transactions. */ - if (committed) { - xfs_trans_ijoin(args->trans, args->dp, XFS_ILOCK_EXCL); - xfs_trans_ihold(args->trans, args->dp); - } + if (committed) + xfs_trans_ijoin(args->trans, args->dp); /* * Close out trans and start the next one in the chain. diff --git a/fs/xfs/xfs_bmap.c b/fs/xfs/xfs_bmap.c index ff8675b41973..e0389656ad2c 100644 --- a/fs/xfs/xfs_bmap.c +++ b/fs/xfs/xfs_bmap.c @@ -3751,9 +3751,10 @@ xfs_bmap_add_attrfork( ip->i_d.di_aformat = XFS_DINODE_FMT_EXTENTS; } ASSERT(ip->i_d.di_anextents == 0); - IHOLD(ip); - xfs_trans_ijoin(tp, ip, XFS_ILOCK_EXCL); + + xfs_trans_ijoin_ref(tp, ip, XFS_ILOCK_EXCL); xfs_trans_log_inode(tp, ip, XFS_ILOG_CORE); + switch (ip->i_d.di_format) { case XFS_DINODE_FMT_DEV: ip->i_d.di_forkoff = roundup(sizeof(xfs_dev_t), 8) >> 3; diff --git a/fs/xfs/xfs_dfrag.c b/fs/xfs/xfs_dfrag.c index 7b11dc0494c2..3b9582c60a22 100644 --- a/fs/xfs/xfs_dfrag.c +++ b/fs/xfs/xfs_dfrag.c @@ -416,11 +416,8 @@ xfs_swap_extents( } - IHOLD(ip); - xfs_trans_ijoin(tp, ip, XFS_ILOCK_EXCL | XFS_IOLOCK_EXCL); - - IHOLD(tip); - xfs_trans_ijoin(tp, tip, XFS_ILOCK_EXCL | XFS_IOLOCK_EXCL); + xfs_trans_ijoin_ref(tp, ip, XFS_ILOCK_EXCL | XFS_IOLOCK_EXCL); + xfs_trans_ijoin_ref(tp, tip, XFS_ILOCK_EXCL | XFS_IOLOCK_EXCL); xfs_trans_log_inode(tp, ip, ilf_fields); xfs_trans_log_inode(tp, tip, tilf_fields); diff --git a/fs/xfs/xfs_fsops.c b/fs/xfs/xfs_fsops.c index ade96922fc8c..dbca5f5c37ba 100644 --- a/fs/xfs/xfs_fsops.c +++ b/fs/xfs/xfs_fsops.c @@ -622,8 +622,7 @@ xfs_fs_log_dummy( ip = mp->m_rootip; xfs_ilock(ip, XFS_ILOCK_EXCL); - xfs_trans_ijoin(tp, ip, XFS_ILOCK_EXCL); - xfs_trans_ihold(tp, ip); + xfs_trans_ijoin(tp, ip); xfs_trans_log_inode(tp, ip, XFS_ILOG_CORE); xfs_trans_set_sync(tp); error = xfs_trans_commit(tp, 0); diff --git a/fs/xfs/xfs_inode.c b/fs/xfs/xfs_inode.c index c7c48da97ad4..d22b580162cc 100644 --- a/fs/xfs/xfs_inode.c +++ b/fs/xfs/xfs_inode.c @@ -1456,7 +1456,7 @@ xfs_itruncate_finish( ASSERT((*tp)->t_flags & XFS_TRANS_PERM_LOG_RES); ASSERT(ip->i_transp == *tp); ASSERT(ip->i_itemp != NULL); - ASSERT(ip->i_itemp->ili_flags & XFS_ILI_HOLD); + ASSERT(ip->i_itemp->ili_lock_flags == 0); ntp = *tp; @@ -1608,12 +1608,8 @@ xfs_itruncate_finish( */ error = xfs_bmap_finish(tp, &free_list, &committed); ntp = *tp; - if (committed) { - /* link the inode into the next xact in the chain */ - xfs_trans_ijoin(ntp, ip, - XFS_ILOCK_EXCL | XFS_IOLOCK_EXCL); - xfs_trans_ihold(ntp, ip); - } + if (committed) + xfs_trans_ijoin(ntp, ip); if (error) { /* @@ -1642,9 +1638,7 @@ xfs_itruncate_finish( error = xfs_trans_commit(*tp, 0); *tp = ntp; - /* link the inode into the next transaction in the chain */ - xfs_trans_ijoin(ntp, ip, XFS_ILOCK_EXCL | XFS_IOLOCK_EXCL); - xfs_trans_ihold(ntp, ip); + xfs_trans_ijoin(ntp, ip); if (error) return error; diff --git a/fs/xfs/xfs_inode_item.c b/fs/xfs/xfs_inode_item.c index c7e70d708345..ad050c618e62 100644 --- a/fs/xfs/xfs_inode_item.c +++ b/fs/xfs/xfs_inode_item.c @@ -628,19 +628,10 @@ xfs_inode_item_unlock( { struct xfs_inode_log_item *iip = INODE_ITEM(lip); struct xfs_inode *ip = iip->ili_inode; - uint hold; - uint iolocked; - uint lock_flags; + unsigned short lock_flags; - ASSERT(iip != NULL); ASSERT(iip->ili_inode->i_itemp != NULL); ASSERT(xfs_isilocked(iip->ili_inode, XFS_ILOCK_EXCL)); - ASSERT((!(iip->ili_inode->i_itemp->ili_flags & - XFS_ILI_IOLOCKED_EXCL)) || - xfs_isilocked(iip->ili_inode, XFS_IOLOCK_EXCL)); - ASSERT((!(iip->ili_inode->i_itemp->ili_flags & - XFS_ILI_IOLOCKED_SHARED)) || - xfs_isilocked(iip->ili_inode, XFS_IOLOCK_SHARED)); /* * Clear the transaction pointer in the inode. @@ -668,35 +659,10 @@ xfs_inode_item_unlock( iip->ili_aextents_buf = NULL; } - /* - * Figure out if we should unlock the inode or not. - */ - hold = iip->ili_flags & XFS_ILI_HOLD; - - /* - * Before clearing out the flags, remember whether we - * are holding the inode's IO lock. - */ - iolocked = iip->ili_flags & XFS_ILI_IOLOCKED_ANY; - - /* - * Clear out the fields of the inode log item particular - * to the current transaction. - */ - iip->ili_flags = 0; - - /* - * Unlock the inode if XFS_ILI_HOLD was not set. - */ - if (!hold) { - lock_flags = XFS_ILOCK_EXCL; - if (iolocked & XFS_ILI_IOLOCKED_EXCL) { - lock_flags |= XFS_IOLOCK_EXCL; - } else if (iolocked & XFS_ILI_IOLOCKED_SHARED) { - lock_flags |= XFS_IOLOCK_SHARED; - } + lock_flags = iip->ili_lock_flags; + iip->ili_lock_flags = 0; + if (lock_flags) xfs_iput(iip->ili_inode, lock_flags); - } } /* diff --git a/fs/xfs/xfs_inode_item.h b/fs/xfs/xfs_inode_item.h index b6a97ff1c3ab..d3dee61e6d91 100644 --- a/fs/xfs/xfs_inode_item.h +++ b/fs/xfs/xfs_inode_item.h @@ -103,12 +103,6 @@ typedef struct xfs_inode_log_format_64 { XFS_ILOG_ADATA | XFS_ILOG_AEXT | \ XFS_ILOG_ABROOT) -#define XFS_ILI_HOLD 0x1 -#define XFS_ILI_IOLOCKED_EXCL 0x2 -#define XFS_ILI_IOLOCKED_SHARED 0x4 - -#define XFS_ILI_IOLOCKED_ANY (XFS_ILI_IOLOCKED_EXCL | XFS_ILI_IOLOCKED_SHARED) - static inline int xfs_ilog_fbroot(int w) { return (w == XFS_DATA_FORK ? XFS_ILOG_DBROOT : XFS_ILOG_ABROOT); @@ -137,7 +131,7 @@ typedef struct xfs_inode_log_item { struct xfs_inode *ili_inode; /* inode ptr */ xfs_lsn_t ili_flush_lsn; /* lsn at last flush */ xfs_lsn_t ili_last_lsn; /* lsn at last transaction */ - unsigned short ili_flags; /* misc flags */ + unsigned short ili_lock_flags; /* lock flags */ unsigned short ili_logged; /* flushed logged data */ unsigned int ili_last_fields; /* fields when flushed */ struct xfs_bmbt_rec *ili_extents_buf; /* array of logged diff --git a/fs/xfs/xfs_iomap.c b/fs/xfs/xfs_iomap.c index 772f3e791ebe..aeac00294a18 100644 --- a/fs/xfs/xfs_iomap.c +++ b/fs/xfs/xfs_iomap.c @@ -329,8 +329,7 @@ xfs_iomap_write_direct( if (error) goto error1; - xfs_trans_ijoin(tp, ip, XFS_ILOCK_EXCL); - xfs_trans_ihold(tp, ip); + xfs_trans_ijoin(tp, ip); bmapi_flag = XFS_BMAPI_WRITE; if ((flags & BMAPI_DIRECT) && (offset < ip->i_size || extsz)) @@ -597,8 +596,7 @@ xfs_iomap_write_allocate( return XFS_ERROR(error); } xfs_ilock(ip, XFS_ILOCK_EXCL); - xfs_trans_ijoin(tp, ip, XFS_ILOCK_EXCL); - xfs_trans_ihold(tp, ip); + xfs_trans_ijoin(tp, ip); xfs_bmap_init(&free_list, &first_block); @@ -761,8 +759,7 @@ xfs_iomap_write_unwritten( } xfs_ilock(ip, XFS_ILOCK_EXCL); - xfs_trans_ijoin(tp, ip, XFS_ILOCK_EXCL); - xfs_trans_ihold(tp, ip); + xfs_trans_ijoin(tp, ip); /* * Modify the unwritten extent state of the buffer. diff --git a/fs/xfs/xfs_rename.c b/fs/xfs/xfs_rename.c index 8edb1074847a..778c87a8ebfc 100644 --- a/fs/xfs/xfs_rename.c +++ b/fs/xfs/xfs_rename.c @@ -169,26 +169,14 @@ xfs_rename( /* * Join all the inodes to the transaction. From this point on, * we can rely on either trans_commit or trans_cancel to unlock - * them. Note that we need to add a vnode reference to the - * directories since trans_commit & trans_cancel will decrement - * them when they unlock the inodes. Also, we need to be careful - * not to add an inode to the transaction more than once. + * them. */ - IHOLD(src_dp); - xfs_trans_ijoin(tp, src_dp, XFS_ILOCK_EXCL); - - if (new_parent) { - IHOLD(target_dp); - xfs_trans_ijoin(tp, target_dp, XFS_ILOCK_EXCL); - } - - IHOLD(src_ip); - xfs_trans_ijoin(tp, src_ip, XFS_ILOCK_EXCL); - - if (target_ip) { - IHOLD(target_ip); - xfs_trans_ijoin(tp, target_ip, XFS_ILOCK_EXCL); - } + xfs_trans_ijoin_ref(tp, src_dp, XFS_ILOCK_EXCL); + if (new_parent) + xfs_trans_ijoin_ref(tp, target_dp, XFS_ILOCK_EXCL); + xfs_trans_ijoin_ref(tp, src_ip, XFS_ILOCK_EXCL); + if (target_ip) + xfs_trans_ijoin_ref(tp, target_ip, XFS_ILOCK_EXCL); /* * If we are using project inheritance, we only allow renames diff --git a/fs/xfs/xfs_trans.c b/fs/xfs/xfs_trans.c index 213792e1ad02..f2065ccb6c2d 100644 --- a/fs/xfs/xfs_trans.c +++ b/fs/xfs/xfs_trans.c @@ -1892,7 +1892,6 @@ xfs_trans_roll( if (error) return error; - xfs_trans_ijoin(trans, dp, XFS_ILOCK_EXCL); - xfs_trans_ihold(trans, dp); + xfs_trans_ijoin(trans, dp); return 0; } diff --git a/fs/xfs/xfs_trans.h b/fs/xfs/xfs_trans.h index 37c0ce1ccd48..aa6f422f0361 100644 --- a/fs/xfs/xfs_trans.h +++ b/fs/xfs/xfs_trans.h @@ -475,8 +475,8 @@ void xfs_trans_dquot_buf(xfs_trans_t *, struct xfs_buf *, uint); void xfs_trans_inode_alloc_buf(xfs_trans_t *, struct xfs_buf *); int xfs_trans_iget(struct xfs_mount *, xfs_trans_t *, xfs_ino_t , uint, uint, struct xfs_inode **); -void xfs_trans_ijoin(xfs_trans_t *, struct xfs_inode *, uint); -void xfs_trans_ihold(xfs_trans_t *, struct xfs_inode *); +void xfs_trans_ijoin_ref(struct xfs_trans *, struct xfs_inode *, uint); +void xfs_trans_ijoin(struct xfs_trans *, struct xfs_inode *); void xfs_trans_log_buf(xfs_trans_t *, struct xfs_buf *, uint, uint); void xfs_trans_log_inode(xfs_trans_t *, struct xfs_inode *, uint); struct xfs_efi_log_item *xfs_trans_get_efi(xfs_trans_t *, uint); diff --git a/fs/xfs/xfs_trans_inode.c b/fs/xfs/xfs_trans_inode.c index 865eeb63ce16..cdc53a1050c5 100644 --- a/fs/xfs/xfs_trans_inode.c +++ b/fs/xfs/xfs_trans_inode.c @@ -33,6 +33,7 @@ #include "xfs_btree.h" #include "xfs_trans_priv.h" #include "xfs_inode_item.h" +#include "xfs_trace.h" #ifdef XFS_TRANS_DEBUG STATIC void @@ -42,7 +43,6 @@ xfs_trans_inode_broot_debug( #define xfs_trans_inode_broot_debug(ip) #endif - /* * Get an inode and join it to the transaction. */ @@ -58,32 +58,31 @@ xfs_trans_iget( int error; error = xfs_iget(mp, tp, ino, flags, lock_flags, ipp); - if (!error && tp) - xfs_trans_ijoin(tp, *ipp, lock_flags); + if (!error && tp) { + xfs_trans_ijoin(tp, *ipp); + (*ipp)->i_itemp->ili_lock_flags = lock_flags; + } return error; } /* - * Add the locked inode to the transaction. - * The inode must be locked, and it cannot be associated with any - * transaction. The caller must specify the locks already held - * on the inode. + * Add a locked inode to the transaction. + * + * The inode must be locked, and it cannot be associated with any transaction. */ void xfs_trans_ijoin( - xfs_trans_t *tp, - xfs_inode_t *ip, - uint lock_flags) + struct xfs_trans *tp, + struct xfs_inode *ip) { xfs_inode_log_item_t *iip; ASSERT(ip->i_transp == NULL); ASSERT(xfs_isilocked(ip, XFS_ILOCK_EXCL)); - ASSERT(lock_flags & XFS_ILOCK_EXCL); if (ip->i_itemp == NULL) xfs_inode_item_init(ip, ip->i_mount); iip = ip->i_itemp; - ASSERT(iip->ili_flags == 0); + ASSERT(iip->ili_lock_flags == 0); /* * Get a log_item_desc to point at the new item. @@ -92,15 +91,6 @@ xfs_trans_ijoin( xfs_trans_inode_broot_debug(ip); - /* - * If the IO lock is already held, mark that in the inode log item. - */ - if (lock_flags & XFS_IOLOCK_EXCL) { - iip->ili_flags |= XFS_ILI_IOLOCKED_EXCL; - } else if (lock_flags & XFS_IOLOCK_SHARED) { - iip->ili_flags |= XFS_ILI_IOLOCKED_SHARED; - } - /* * Initialize i_transp so we can find it with xfs_inode_incore() * in xfs_trans_iget() above. @@ -108,27 +98,25 @@ xfs_trans_ijoin( ip->i_transp = tp; } - - /* - * Mark the inode as not needing to be unlocked when the inode item's - * IOP_UNLOCK() routine is called. The inode must already be locked - * and associated with the given transaction. + * Add a locked inode to the transaction. + * + * + * Grabs a reference to the inode which will be dropped when the transaction + * is commited. The inode will also be unlocked at that point. The inode + * must be locked, and it cannot be associated with any transaction. */ -/*ARGSUSED*/ void -xfs_trans_ihold( - xfs_trans_t *tp, - xfs_inode_t *ip) +xfs_trans_ijoin_ref( + struct xfs_trans *tp, + struct xfs_inode *ip, + uint lock_flags) { - ASSERT(ip->i_transp == tp); - ASSERT(ip->i_itemp != NULL); - ASSERT(xfs_isilocked(ip, XFS_ILOCK_EXCL)); - - ip->i_itemp->ili_flags |= XFS_ILI_HOLD; + xfs_trans_ijoin(tp, ip); + IHOLD(ip); + ip->i_itemp->ili_lock_flags = lock_flags; } - /* * This is called to mark the fields indicated in fieldmask as needing * to be logged when the transaction is committed. The inode must diff --git a/fs/xfs/xfs_utils.c b/fs/xfs/xfs_utils.c index 8965887d26b1..102ce4898ab7 100644 --- a/fs/xfs/xfs_utils.c +++ b/fs/xfs/xfs_utils.c @@ -374,8 +374,8 @@ xfs_truncate_file( * of references will stay constant. */ xfs_ilock(ip, XFS_ILOCK_EXCL); - xfs_trans_ijoin(tp, ip, XFS_ILOCK_EXCL | XFS_IOLOCK_EXCL); - xfs_trans_ihold(tp, ip); + xfs_trans_ijoin(tp, ip); + /* * Signal a sync xaction. The only case where that isn't * the case is if we're truncating an already unlinked file diff --git a/fs/xfs/xfs_vnodeops.c b/fs/xfs/xfs_vnodeops.c index 161444e768b6..130343a5d22d 100644 --- a/fs/xfs/xfs_vnodeops.c +++ b/fs/xfs/xfs_vnodeops.c @@ -268,8 +268,7 @@ xfs_setattr( commit_flags = XFS_TRANS_RELEASE_LOG_RES; xfs_ilock(ip, XFS_ILOCK_EXCL); - xfs_trans_ijoin(tp, ip, lock_flags); - xfs_trans_ihold(tp, ip); + xfs_trans_ijoin(tp, ip); /* * Only change the c/mtime if we are changing the size @@ -319,8 +318,7 @@ xfs_setattr( xfs_iflags_set(ip, XFS_ITRUNCATED); } } else if (tp) { - xfs_trans_ijoin(tp, ip, lock_flags); - xfs_trans_ihold(tp, ip); + xfs_trans_ijoin(tp, ip); } /* @@ -653,10 +651,7 @@ xfs_free_eofblocks( } xfs_ilock(ip, XFS_ILOCK_EXCL); - xfs_trans_ijoin(tp, ip, - XFS_IOLOCK_EXCL | - XFS_ILOCK_EXCL); - xfs_trans_ihold(tp, ip); + xfs_trans_ijoin(tp, ip); error = xfs_itruncate_finish(&tp, ip, ip->i_size, @@ -728,8 +723,7 @@ xfs_inactive_symlink_rmt( xfs_ilock(ip, XFS_IOLOCK_EXCL | XFS_ILOCK_EXCL); size = (int)ip->i_d.di_size; ip->i_d.di_size = 0; - xfs_trans_ijoin(tp, ip, XFS_ILOCK_EXCL | XFS_IOLOCK_EXCL); - xfs_trans_ihold(tp, ip); + xfs_trans_ijoin(tp, ip); xfs_trans_log_inode(tp, ip, XFS_ILOG_CORE); /* * Find the block(s) so we can inval and unmap them. @@ -773,8 +767,7 @@ xfs_inactive_symlink_rmt( * Mark it dirty so it will be logged and moved forward in the log as * part of every commit. */ - xfs_trans_ijoin(tp, ip, XFS_ILOCK_EXCL | XFS_IOLOCK_EXCL); - xfs_trans_ihold(tp, ip); + xfs_trans_ijoin(tp, ip); xfs_trans_log_inode(tp, ip, XFS_ILOG_CORE); /* * Get a new, empty transaction to return to our caller. @@ -907,8 +900,7 @@ xfs_inactive_attrs( goto error_cancel; xfs_ilock(ip, XFS_ILOCK_EXCL); - xfs_trans_ijoin(tp, ip, XFS_IOLOCK_EXCL | XFS_ILOCK_EXCL); - xfs_trans_ihold(tp, ip); + xfs_trans_ijoin(tp, ip); xfs_idestroy_fork(ip, XFS_ATTR_FORK); ASSERT(ip->i_d.di_anextents == 0); @@ -1095,8 +1087,7 @@ xfs_inactive( } xfs_ilock(ip, XFS_ILOCK_EXCL); - xfs_trans_ijoin(tp, ip, XFS_IOLOCK_EXCL | XFS_ILOCK_EXCL); - xfs_trans_ihold(tp, ip); + xfs_trans_ijoin(tp, ip); /* * normally, we have to run xfs_itruncate_finish sync. @@ -1129,8 +1120,7 @@ xfs_inactive( return VN_INACTIVE_CACHE; } - xfs_trans_ijoin(tp, ip, XFS_IOLOCK_EXCL | XFS_ILOCK_EXCL); - xfs_trans_ihold(tp, ip); + xfs_trans_ijoin(tp, ip); } else { error = xfs_trans_reserve(tp, 0, XFS_IFREE_LOG_RES(mp), @@ -1143,8 +1133,7 @@ xfs_inactive( } xfs_ilock(ip, XFS_ILOCK_EXCL | XFS_IOLOCK_EXCL); - xfs_trans_ijoin(tp, ip, XFS_IOLOCK_EXCL | XFS_ILOCK_EXCL); - xfs_trans_ihold(tp, ip); + xfs_trans_ijoin(tp, ip); } /* @@ -1392,8 +1381,7 @@ xfs_create( * the transaction cancel unlocking dp so don't do it explicitly in the * error path. */ - IHOLD(dp); - xfs_trans_ijoin(tp, dp, XFS_ILOCK_EXCL); + xfs_trans_ijoin_ref(tp, dp, XFS_ILOCK_EXCL); unlock_dp_on_error = B_FALSE; error = xfs_dir_createname(tp, dp, name, ip->i_ino, @@ -1730,15 +1718,8 @@ xfs_remove( xfs_lock_two_inodes(dp, ip, XFS_ILOCK_EXCL); - /* - * At this point, we've gotten both the directory and the entry - * inodes locked. - */ - IHOLD(ip); - xfs_trans_ijoin(tp, dp, XFS_ILOCK_EXCL); - - IHOLD(dp); - xfs_trans_ijoin(tp, ip, XFS_ILOCK_EXCL); + xfs_trans_ijoin_ref(tp, dp, XFS_ILOCK_EXCL); + xfs_trans_ijoin_ref(tp, ip, XFS_ILOCK_EXCL); /* * If we're removing a directory perform some additional validation. @@ -1884,15 +1865,8 @@ xfs_link( xfs_lock_two_inodes(sip, tdp, XFS_ILOCK_EXCL); - /* - * Increment vnode ref counts since xfs_trans_commit & - * xfs_trans_cancel will both unlock the inodes and - * decrement the associated ref counts. - */ - IHOLD(sip); - IHOLD(tdp); - xfs_trans_ijoin(tp, sip, XFS_ILOCK_EXCL); - xfs_trans_ijoin(tp, tdp, XFS_ILOCK_EXCL); + xfs_trans_ijoin_ref(tp, sip, XFS_ILOCK_EXCL); + xfs_trans_ijoin_ref(tp, tdp, XFS_ILOCK_EXCL); /* * If the source has too many links, we can't make any more to it. @@ -2087,8 +2061,7 @@ xfs_symlink( * transaction cancel unlocking dp so don't do it explicitly in the * error path. */ - IHOLD(dp); - xfs_trans_ijoin(tp, dp, XFS_ILOCK_EXCL); + xfs_trans_ijoin_ref(tp, dp, XFS_ILOCK_EXCL); unlock_dp_on_error = B_FALSE; /* @@ -2227,13 +2200,12 @@ xfs_set_dmattrs( return error; } xfs_ilock(ip, XFS_ILOCK_EXCL); - xfs_trans_ijoin(tp, ip, XFS_ILOCK_EXCL); + xfs_trans_ijoin_ref(tp, ip, XFS_ILOCK_EXCL); ip->i_d.di_dmevmask = evmask; ip->i_d.di_dmstate = state; xfs_trans_log_inode(tp, ip, XFS_ILOG_CORE); - IHOLD(ip); error = xfs_trans_commit(tp, 0); return error; @@ -2366,8 +2338,7 @@ xfs_alloc_file_space( if (error) goto error1; - xfs_trans_ijoin(tp, ip, XFS_ILOCK_EXCL); - xfs_trans_ihold(tp, ip); + xfs_trans_ijoin(tp, ip); /* * Issue the xfs_bmapi() call to allocate the blocks @@ -2668,8 +2639,7 @@ xfs_free_file_space( if (error) goto error1; - xfs_trans_ijoin(tp, ip, XFS_ILOCK_EXCL); - xfs_trans_ihold(tp, ip); + xfs_trans_ijoin(tp, ip); /* * issue the bunmapi() call to free the blocks @@ -2839,8 +2809,7 @@ xfs_change_file_space( xfs_ilock(ip, XFS_ILOCK_EXCL); - xfs_trans_ijoin(tp, ip, XFS_ILOCK_EXCL); - xfs_trans_ihold(tp, ip); + xfs_trans_ijoin(tp, ip); if ((attr_flags & XFS_ATTR_DMI) == 0) { ip->i_d.di_mode &= ~S_ISUID; -- cgit v1.2.3 From a59f55703c3725b2fa582924f51db62b58be05bb Mon Sep 17 00:00:00 2001 From: Christoph Hellwig Date: Wed, 23 Jun 2010 18:11:15 +1000 Subject: xfs: remove the unused XFS_TRANS_NOSLEEP/XFS_TRANS_WAIT flags Signed-off-by: Christoph Hellwig Reviewed-by: Dave Chinner --- fs/xfs/xfs_trans.h | 2 -- 1 file changed, 2 deletions(-) (limited to 'fs/xfs/xfs_trans.h') diff --git a/fs/xfs/xfs_trans.h b/fs/xfs/xfs_trans.h index aa6f422f0361..c13c0f97b494 100644 --- a/fs/xfs/xfs_trans.h +++ b/fs/xfs/xfs_trans.h @@ -184,8 +184,6 @@ struct xfs_log_item_desc { /* * Values for call flags parameter. */ -#define XFS_TRANS_NOSLEEP 0x1 -#define XFS_TRANS_WAIT 0x2 #define XFS_TRANS_RELEASE_LOG_RES 0x4 #define XFS_TRANS_ABORT 0x8 -- cgit v1.2.3