On Wed, 30 Jun 2010 22:05:02 +1000 Nick Piggin <npiggin@xxxxxxx> wrote: > On Wed, Jun 30, 2010 at 05:27:02PM +1000, Dave Chinner wrote: > > On Thu, Jun 24, 2010 at 01:02:41PM +1000, npiggin@xxxxxxx wrote: > > > Protect inode->i_count with i_lock, rather than having it atomic. > > > Next step should also be to move things together (eg. the refcount increment > > > into d_instantiate, which will remove a lock/unlock cycle on i_lock). > > ..... > > > Index: linux-2.6/fs/inode.c > > > =================================================================== > > > --- linux-2.6.orig/fs/inode.c > > > +++ linux-2.6/fs/inode.c > > > @@ -33,14 +33,13 @@ > > > * inode_hash_lock protects: > > > * inode hash table, i_hash > > > * inode->i_lock protects: > > > - * i_state > > > + * i_state, i_count > > > * > > > * Ordering: > > > * inode_lock > > > * sb_inode_list_lock > > > * inode->i_lock > > > - * inode_lock > > > - * inode_hash_lock > > > + * inode_hash_lock > > > */ > > > > I thought that the rule governing the use of inode->i_lock was that > > it can be used anywhere as long as it is the innermost lock. > > > > Hmmm, no references in the code or documentation. Google gives a > > pretty good reference: > > > > http://www.mail-archive.com/linux-ext4@xxxxxxxxxxxxxxx/msg02584.html > > > > Perhaps a different/new lock needs to be used here? > > Well I just changed the order (and documented it to boot :)). It's > pretty easy to verify that LOR is no problem. inode hash is only > taken in a very few places so other code outside inode.c is fine to > use i_lock as an innermost lock. um, nesting a kernel-wide singleton lock inside a per-inode lock is plain nutty. -- To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
