Skip to contentSkip to conversation

make inode reclaim wait for log I/O to complete

PATCH In review Lachlan McIlroy 2 participants 4 messages May 23, 2008 – Jun 18, 2008

lore
Share & tools
lore
#0 lore raw

PATCH v2 1/1 make inode reclaim wait for log I/O to complete

During a forced shutdown a 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. Version 2 cleans up the code a bit by relying on xfs_iflush() to do the unpinning and forced shutdown check.

xfs/xfs_inode.c renamed from xfs/xfs_inode.c_1.504 +0 -0
@@ -3082,8 +3082,6 @@ xfs_iflush(
	 * flush lock and do nothing.
	 */
	if (xfs_inode_clean(ip)) {
-		ASSERT((iip != NULL) ?
-			 !(iip->ili_item.li_flags & XFS_LI_IN_AIL) : 1);
		xfs_ifunlock(ip);
		return 0;
	}
xfs/xfs_vnodeops.c renamed from xfs/xfs_vnodeops.c_1.760 +0 -0
@@ -3260,7 +3260,6 @@ xfs_finish_reclaim(
{
	xfs_perag_t	*pag = xfs_get_perag(ip->i_mount, ip->i_ino);
	bhv_vnode_t	*vp = XFS_ITOV_NULL(ip);

- int error;

	if (vp && VN_BAD(vp))
		goto reclaim;
@@ -3303,29 +3302,16 @@ xfs_finish_reclaim(
		xfs_iflock(ip);
	}

- if (!XFS_FORCED_SHUTDOWN(ip->i_mount)) {
- if (ip->i_update_core ||
- ((ip->i_itemp != NULL) &&
- (ip->i_itemp->ili_format.ilf_fields != 0))) {
- error = xfs_iflush(ip, sync_mode);
- /*
- * If we hit an error, typically because of filesystem
- * shutdown, we don't need to let vn_reclaim to know
- * because we're gonna reclaim the inode anyway.
- */
- if (error) {
- xfs_iunlock(ip, XFS_ILOCK_EXCL);
- goto reclaim;
- }
- xfs_iflock(ip); /* synchronize with xfs_iflush_done */
- }
-
- ASSERT(ip->i_update_core == 0);
- ASSERT(ip->i_itemp == NULL ||
- ip->i_itemp->ili_format.ilf_fields == 0);
+ /*
+ * In the case of a forced shutdown we rely on xfs_iflush() to
+ * wait for the inode to be unpinned before returning an error.
+ */
+ if (xfs_iflush(ip, sync_mode) == 0) {
+ /* synchronize with xfs_iflush_done */
+ xfs_iflock(ip);

+		xfs_ifunlock(ip);
	}
-	xfs_ifunlock(ip);
	xfs_iunlock(ip, XFS_ILOCK_EXCL);

reclaim:

#1 lore raw

ping.

Quoted 68 lines · Lachlan McIlroy

During a forced shutdown a 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. Version 2 cleans up the code a bit by relying on xfs_iflush() to do the unpinning and forced shutdown check.

xfs/xfs_inode.c 2008-05-23 11:21:14.000000000 +1000 renamed from xfs/xfs_inode.c_1.504 2008-05-23 11:19:21.000000000 +1000 +0 -2
@@ -3082,8 +3082,6 @@ xfs_iflush(
     * flush lock and do nothing.
     */
    if (xfs_inode_clean(ip)) {
-        ASSERT((iip != NULL) ?
-             !(iip->ili_item.li_flags & XFS_LI_IN_AIL) : 1);
        xfs_ifunlock(ip);
        return 0;
    }
xfs/xfs_vnodeops.c 2008-05-23 11:52:54.000000000 +1000 renamed from xfs/xfs_vnodeops.c_1.760 2008-05-22 16:01:09.000000000 +1000 +8 -22
@@ -3260,7 +3260,6 @@ xfs_finish_reclaim(
 {
    xfs_perag_t    *pag = xfs_get_perag(ip->i_mount, ip->i_ino);
    bhv_vnode_t    *vp = XFS_ITOV_NULL(ip);
-    int        error;
 
    if (vp && VN_BAD(vp))
        goto reclaim;
@@ -3303,29 +3302,16 @@ xfs_finish_reclaim(
        xfs_iflock(ip);
    }
 
-    if (!XFS_FORCED_SHUTDOWN(ip->i_mount)) {
-        if (ip->i_update_core ||
-            ((ip->i_itemp != NULL) &&
-             (ip->i_itemp->ili_format.ilf_fields != 0))) {
-            error = xfs_iflush(ip, sync_mode);
-            /*
-             * If we hit an error, typically because of filesystem
-             * shutdown, we don't need to let vn_reclaim to know
-             * because we're gonna reclaim the inode anyway.
-             */
-            if (error) {
-                xfs_iunlock(ip, XFS_ILOCK_EXCL);
-                goto reclaim;
-            }
-            xfs_iflock(ip); /* synchronize with xfs_iflush_done */
-        }
-
-        ASSERT(ip->i_update_core == 0);
-        ASSERT(ip->i_itemp == NULL ||
-               ip->i_itemp->ili_format.ilf_fields == 0);
+    /*
+     * In the case of a forced shutdown we rely on xfs_iflush() to
+     * wait for the inode to be unpinned before returning an error.
+     */
+    if (xfs_iflush(ip, sync_mode) == 0) {
+        /* synchronize with xfs_iflush_done */
+        xfs_iflock(ip);
+        xfs_ifunlock(ip);
    }
 
-    xfs_ifunlock(ip);
    xfs_iunlock(ip, XFS_ILOCK_EXCL);
 
 reclaim:
#2 lore raw
Quoted 8 lines · Lachlan McIlroy

ping.

Quoted 5 lines · Lachlan McIlroy

During a forced shutdown a 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. Version 2 cleans up the code a bit by relying on xfs_iflush() to do the unpinning and forced shutdown check.

Sorry I missed this when you first posted it. This one looks ok.

Cheers,

Dave.

Signature · 2 lines
Dave Chinner
[email protected]
#3 lore raw
Quoted 12 lines · Dave Chinner
Quoted 8 lines · Lachlan McIlroy

ping.

Quoted 5 lines · Lachlan McIlroy

During a forced shutdown a 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. Version 2 cleans up the code a bit by relying on xfs_iflush() to do the unpinning and forced shutdown check.

Sorry I missed this when you first posted it. This one looks ok.

Thanks Dave.