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] [thread-next>] [day] [month] [year] [list]

Message-ID: <20130802011133.GT7118@dastard>
Date:	Fri, 2 Aug 2013 11:11:33 +1000
From:	Dave Chinner <david@...morbit.com>
To:	Christoph Hellwig <hch@...radead.org>
Cc:	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 Thu, Aug 01, 2013 at 01:12:35AM -0700, Christoph Hellwig wrote:
> On Wed, Jul 31, 2013 at 02:15:41PM +1000, Dave Chinner wrote:
> > From: Dave Chinner <dchinner@...hat.com>
> > 
> > Some filesystems don't use the VFS inode hash and fake the fact they
> > are hashed so that all the writeback code works correctly. However,
> > this means the evict() path still tries to remove the inode from the
> > hash, meaning that the inode_hash_lock() needs to be taken
> > unnecessarily. Hence under certain workloads the inode_hash_lock can
> > be contended even if the inode is never actually hashed.
> > 
> > To avoid this, add an inode opflag to allow inode_hash_remove() to
> > avoid taking the hash lock on inodes have never actually been
> > hashed.
> 
> Good idea, but I don't like how it's implemented.
> 
> First a formality: i_opflags really is for flags showing that inode
> operations exist, not for addional bits.  Just use i_flags for it.
> 
> Second this is a hack hacking around a hack.  We just mark the inode
> hashed so that writeback doesn't ignore it, and not we need to work
> around the fact that we don't want an inode marked hashed from the
> hashlist.
> 
> As the most simple version I'd suggest to just add an I_NEEDS_WRITEBACK
> flag which gets set by __insert_inode_hash, and all the current users
> of hlist_add_fake on i_hash, as well as the block devices that currently
> have another special case in the writeback code.

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()?

Cheers,

Dave.
-- 
Dave Chinner
david@...morbit.com
--
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/