Skip to contentSkip to conversation

Remove unnecessary assertion

PATCH In review Lachlan McIlroy 2 participants 2 messages Dec 4, 2008 XFS filesystem

lore
Share & tools
lore
#0 lore raw

PATCH 1/1 Remove unnecessary assertion

Hit this assert because an inode was tagged with XFS_ICI_RECLAIM_TAG but not XFS_IRECLAIMABLE|XFS_IRECLAIM. This is because xfs_iget_cache_hit() first clears XFS_IRECLAIMABLE and then calls __xfs_inode_clear_reclaim_tag() while only holding the pag_ici_lock in read mode so we can race with xfs_reclaim_inodes_ag(). Looks like xfs_reclaim_inodes_ag() will do the right thing anyway so just remove the assert.

Thanks to Christoph for pointing out where the problem was.

fs/xfs/linux-2.6/xfs_sync.c +0 -2
@@ -707,8 +707,6 @@ restart:
  			break;
  		}
 
-		ASSERT(xfs_iflags_test(ip, (XFS_IRECLAIMABLE|XFS_IRECLAIM)));
-
  		/* ignore if already under reclaim */
  		if (xfs_iflags_test(ip, XFS_IRECLAIM)) {
  			read_unlock(&pag->pag_ici_lock);

_______________________________________________
xfs mailing list
[email protected]
http://oss.sgi.com/mailman/listinfo/xfs

#1 lore raw
Quoted 8 lines · Lachlan McIlroy

Hit this assert because an inode was tagged with XFS_ICI_RECLAIM_TAG but not XFS_IRECLAIMABLE|XFS_IRECLAIM. This is because xfs_iget_cache_hit() first clears XFS_IRECLAIMABLE and then calls __xfs_inode_clear_reclaim_tag() while only holding the pag_ici_lock in read mode so we can race with xfs_reclaim_inodes_ag(). Looks like xfs_reclaim_inodes_ag() will do the right thing anyway so just remove the assert.

Thanks to Christoph for pointing out where the problem was.

Yeah, given that I ran with this patch for a while you have my ACK for it.

_______________________________________________
xfs mailing list
[email protected]
http://oss.sgi.com/mailman/listinfo/xfs