make xfs_idestroy() wait for log I/O to complete
PATCH 1/1 make xfs_idestroy() wait for log I/O to complete
An xfs inode can be destroyed before log I/O involving that inode is complete. We need to wait for the inode to be unpinned before tearing it down. The patch looks big but the only real change is adding a call to xfs_iunpin_wait() to the start of xfs_idestroy(). The rest of the patch is moving xfs_idestroy() after the pinning routines.
Lachlan
xfs/xfs_inode.c renamed from xfs/xfs_inode.c_1.489 +68 -65
@@ -2733,71 +2733,6 @@ xfs_idestroy_fork( }/* -* This is called free all the memory associated with an inode. -* It must free the inode itself and any buffers allocated for -* if_extents/if_data and if_broot. It must also free the lock -* associated with the inode. -*/ -void -xfs_idestroy ( -xfs_inode_t *ip) -{ -switch (ip->i_d.di_mode &S_IFMT ) { -case S_IFREG : -case S_IFDIR : -case S_IFLNK : -xfs_idestroy_fork (ip,XFS_DATA_FORK ); -break ; - } -if (ip->i_afp) -xfs_idestroy_fork (ip,XFS_ATTR_FORK ); -mrfree (&ip->i_lock); -mrfree (&ip->i_iolock); -freesema (&ip->i_flock); - -#ifdef XFS_INODE_TRACE -ktrace_free (ip->i_trace); -#endif -#ifdef XFS_BMAP_TRACE -ktrace_free (ip->i_xtrace); -#endif -#ifdef XFS_BMBT_TRACE -ktrace_free (ip->i_btrace); -#endif -#ifdef XFS_RW_TRACE -ktrace_free (ip->i_rwtrace); -#endif -#ifdef XFS_ILOCK_TRACE -ktrace_free (ip->i_lock_trace); -#endif -#ifdef XFS_DIR2_TRACE -ktrace_free (ip->i_dir_trace); -#endif -if (ip->i_itemp) { -/* -* Only if we are shutting down the fs will we see an -* inode still in the AIL. If it is there, we should remove -* it to prevent a use-after-free from occurring. -*/ -xfs_mount_t *mp = ip->i_mount; -xfs_log_item_t *lip = &ip->i_itemp->ili_item; - -ASSERT (((lip->li_flags &XFS_LI_IN_AIL ) ==0 ) || -XFS_FORCED_SHUTDOWN (ip->i_mount)); -if (lip->li_flags &XFS_LI_IN_AIL ) { -spin_lock (&mp->m_ail_lock); -if (lip->li_flags &XFS_LI_IN_AIL ) -xfs_trans_delete_ail (mp, lip); -else -spin_unlock (&mp->m_ail_lock); - } -xfs_inode_item_destroy (ip); - } -kmem_zone_free (xfs_inode_zone, ip); -} - - -/* * Increment the pin count of the given buffer. * This value is protected by ipinlock spinlock in the mount structure. */
@@ -2860,6 +2795,74 @@ xfs_iunpin_wait(wait_event (ip->i_ipin_wait, (atomic_read (&ip->i_pincount) ==0 )); } +/* +* This is called free all the memory associated with an inode. +* It must free the inode itself and any buffers allocated for +* if_extents/if_data and if_broot. It must also free the lock +* associated with the inode. +*/ +void +xfs_idestroy ( +xfs_inode_t *ip) +{ +/* +* Wait for any log writes referencing this inode to complete. +*/ +xfs_iunpin_wait (ip); + +switch (ip->i_d.di_mode &S_IFMT ) { +case S_IFREG : +case S_IFDIR : +case S_IFLNK : +xfs_idestroy_fork (ip,XFS_DATA_FORK ); +break ; + } +if (ip->i_afp) +xfs_idestroy_fork (ip,XFS_ATTR_FORK ); +mrfree (&ip->i_lock); +mrfree (&ip->i_iolock); +freesema (&ip->i_flock); + +#ifdef XFS_INODE_TRACE +ktrace_free (ip->i_trace); +#endif +#ifdef XFS_BMAP_TRACE +ktrace_free (ip->i_xtrace); +#endif +#ifdef XFS_BMBT_TRACE +ktrace_free (ip->i_btrace); +#endif +#ifdef XFS_RW_TRACE +ktrace_free (ip->i_rwtrace); +#endif +#ifdef XFS_ILOCK_TRACE +ktrace_free (ip->i_lock_trace); +#endif +#ifdef XFS_DIR2_TRACE +ktrace_free (ip->i_dir_trace); +#endif +if (ip->i_itemp) { +/* +* Only if we are shutting down the fs will we see an +* inode still in the AIL. If it is there, we should remove +* it to prevent a use-after-free from occurring. +*/ +xfs_mount_t *mp = ip->i_mount; +xfs_log_item_t *lip = &ip->i_itemp->ili_item; + +ASSERT (((lip->li_flags &XFS_LI_IN_AIL ) ==0 ) || +XFS_FORCED_SHUTDOWN (ip->i_mount)); +if (lip->li_flags &XFS_LI_IN_AIL ) { +spin_lock (&mp->m_ail_lock); +if (lip->li_flags &XFS_LI_IN_AIL ) +xfs_trans_delete_ail (mp, lip); +else +spin_unlock (&mp->m_ail_lock); + } +xfs_inode_item_destroy (ip); + } +kmem_zone_free (xfs_inode_zone, ip); +}/* * xfs_iextents_copy()
Quoted 6 lines · Lachlan McIlroy
An xfs inode can be destroyed before log I/O involving that inode is complete. We need to wait for the inode to be unpinned before tearing it down. The patch looks big but the only real change is adding a call to
xfs_iunpin_wait()to the start ofxfs_idestroy(). The rest of the patch is movingxfs_idestroy()after the pinning routines.
Making sure the inode is unpinned before it's destroyed definitvely sounds useful. I can't think of any harm this might cause either.
I'd prefer to have to commits, one to move the function around and one to add the call to xfs_iunpin_wait, though.