linux-kernel - Re: fs: break out inode operations from inode

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: <20101029093421.GJ2715@dastard>
Date:	Fri, 29 Oct 2010 20:34:21 +1100
From:	Dave Chinner <david@...morbit.com>
To:	Al Viro <viro@...IV.linux.org.uk>
Cc:	linux-fsdevel@...r.kernel.org, linux-kernel@...r.kernel.org,
	Nick Piggin <npiggin@...nel.dk>
Subject: Re: fs: break out inode operations from inode_lock V4

On Fri, Oct 29, 2010 at 10:29:30AM +0100, Al Viro wrote:
> On Fri, Oct 29, 2010 at 07:59:55PM +1100, Dave Chinner wrote:
> > Hi Al,
> > 
> > Another update to the inode_lock splitting patch set. It's still
> > based on your merge-stem branch. I'm going to be out all weekend, so
> > any further changes will take a couple of days to turn around.
> > 
> > Version 4:
> > - whitespace cleanup
> > - moved setting state on new inodes till after the hash search fails
> >   in insert_inode_locked
> > - made hash insert operations atomic with state changes by holding
> >   inode->i_lock while doing hash inserts
> > - made inode hash removals atomic with state changes by taking the
> >   inode_lock (later inode_hash_lock) and inode->i_lock. Combined
> >   with the insert changes, this means the inode_unhashed check in
> >   ->drop_inode is safely protected by just holding the
> >   inode->i_lock.
> > - protect inode_unhashed() checks in insert_inode_locked with
> >   inode->i_lock
> 
> The last one is not needed at all; look at what's getting done there - we
> drop that ->i_lock immediately after the check, so it doesn't buy us anything.
> The stuff before that *is* a race fix; namely, the race with BS iget()
> triggered by nfsd.  This check is just verifying that it was a race and not
> a badly confused filesystem.  IOW, no need to lock anything and no _point_
> locking anything.  We are repeating the hash walk anyway; this is just making
> sure that we hadn't run into infinite retries.

Oh, I forgot to remove that from the changelog - I removed the
locking from the code, though....

> Other than that I'm OK with that set; could you add "lift ->i_lock from
> the beginning of writeback_single_inode()" to the series and post your
> current RCU-for-i_hash patch for review?

It'll be Monday before I get to this. It's way past beer o'clock
here....

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/