linux-kernel - Re: [PATCH 02/11] inode: add IOP_NOTHASHED to avoid inode hash lock in evict

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Date:	Fri, 2 Aug 2013 07:32:34 -0700
From:	Christoph Hellwig <hch@...radead.org>
To:	Dave Chinner <david@...morbit.com>
Cc:	Christoph Hellwig <hch@...radead.org>,
	linux-fsdevel@...r.kernel.org, linux-kernel@...r.kernel.org,
	akpm@...ux-foundation.org, davej@...hat.com,
	viro@...iv.linux.org.uk, jack@...e.cz, glommer@...allels.com
Subject: Re: [PATCH 02/11] inode: add IOP_NOTHASHED to avoid inode hash lock
 in evict

On Fri, Aug 02, 2013 at 11:11:33AM +1000, Dave Chinner wrote:
> But that doesn't fix the problem of taking the hash lock in evict()
> when it is not necessary. If everything sets I_NEEDS_WRITEBACK, and
> we still fake hashing the inode, how are do we know that we don't
> need to unhash it in evict()?

The idea is to keep evict as-is but make the flag that an inode needs
writeback entirely separate from the is hashed check.

I've attached a draft patch implementing my idea belo.  It does survive
booting and data copies to ext4 or xfs filesystems is persistent with
it, so it works for some defintion of working.

Bedxies splitting and proper documentation we probably also want to get
rid of the inode_unhashed check in generic_drop_inode and replace it
with an is_bad_inode, just special casing unhashed inodes in coda to
keep the behaviour for it the same.


diff --git a/fs/fs-writeback.c b/fs/fs-writeback.c
index 68851ff..e36cad9 100644
--- a/fs/fs-writeback.c
+++ b/fs/fs-writeback.c
@@ -1147,22 +1147,12 @@ void __mark_inode_dirty(struct inode *inode, int flags)
 		inode->i_state |= flags;
 
 		/*
-		 * If the inode is being synced, just update its dirty state.
-		 * The unlocker will place the inode on the appropriate
-		 * superblock list, based upon its state.
+		 * If an inode isn't fully constructed yet or being torn
+		 * down let it on the list it is on currently.  For inodes
+		 * undergoing sync we will move it to the right list once
+		 * syncing it has been finished.
 		 */
-		if (inode->i_state & I_SYNC)
-			goto out_unlock_inode;
-
-		/*
-		 * Only add valid (hashed) inodes to the superblock's
-		 * dirty list.  Add blockdev inodes as well.
-		 */
-		if (!S_ISBLK(inode->i_mode)) {
-			if (inode_unhashed(inode))
-				goto out_unlock_inode;
-		}
-		if (inode->i_state & I_FREEING)
+		if (inode->i_state & (I_NEW | I_SYNC | I_FREEING))
 			goto out_unlock_inode;
 
 		/*
diff --git a/fs/xfs/xfs_iops.c b/fs/xfs/xfs_iops.c
index 96dda62..4bd3be0 100644
--- a/fs/xfs/xfs_iops.c
+++ b/fs/xfs/xfs_iops.c
@@ -1168,8 +1168,6 @@ xfs_setup_inode(
 	inode->i_state = I_NEW;
 
 	inode_sb_list_add(inode);
-	/* make the inode look hashed for the writeback code */
-	hlist_add_fake(&inode->i_hash);
 
 	inode->i_mode	= ip->i_d.di_mode;
 	set_nlink(inode, ip->i_d.di_nlink);
diff --git a/fs/xfs/xfs_super.c b/fs/xfs/xfs_super.c
index 19922eb..c5fdf48 100644
--- a/fs/xfs/xfs_super.c
+++ b/fs/xfs/xfs_super.c
@@ -1026,9 +1026,7 @@ STATIC int
 xfs_fs_drop_inode(
 	struct inode		*inode)
 {
-	struct xfs_inode	*ip = XFS_I(inode);
-
-	return generic_drop_inode(inode) || (ip->i_flags & XFS_IDONTCACHE);
+	return !inode->i_nlink || (XFS_I(inode)->i_flags & XFS_IDONTCACHE);
 }
 
 STATIC void
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/