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. -- 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