On Thu, Jun 17, 2004 at 03:53:13AM -0700, Andrew Morton wrote:
> "Adam J. Richter" <adam@yggdrasil.com> wrote:
> >
> > 	In the near future, I expect to try to eliminate dn_lock by
> >  using parent_inode->i_sem instead, as the kmem_cache_t in dnotify.c
> >  does not need to be protected by a separate lock.
> 
> inode->i_lock would be better.  Take care to keep it an "innermost" VFS
> lock though.

	Thank you for the suggestion.  I was not aware of inode->i_lock.

	Looking at other users of inode->i_lock, I believe that using
inode->i_lock should not cause any conflict.  The lock for
inode->i_dnotify is only taken when someone calls the dnotify ioctl
or if there actually is a match to some dnotify event, so the change
in lock contention between a single dn_lock and inode->i_lock (which
is obviously used elsewhere) should be minimal.  The text + data of
the .o file generate on x86 was actually 32 bytes smaller when I
switched to inode->i_lock, and, of course, it made the source code
1 line shorter.

>Move kmem_cache_free() outside the lock altoghter.

	Per your suggestion, I've done that in fcntl_dirnotify().
This wipes out the trivial space saving from switching to
inode->i_lock, but it's probably more important to avoid holding
inode->i_lock unnecessarily anyhow.  My removal of one of the
goto labels had no effect on object code size on my x86
configuration.

	The other places that call kmem_cache_free() with the lock
held do so because they are iterating through inode->i_dnotify, and
may have to call kmem_cache_free() repeatedly.

	Here is an updated patch.


-- 
                    __     ______________
Adam J. Richter        \ /
adam@yggdrasil.com      | g g d r a s i l