On 2011-11-10 15:33:19, Jan Kara wrote:
> On Thu 10-11-11 00:45:00, Tyler Hicks wrote:
> > 9a7aa12f3911853a introduced additional logic around setting the i_mutex
> > lockdep class for directory inodes. The idea was that some filesystems
> > may want their own special lockdep class for different directory
> > inodes and calling unlock_new_inode() should not clobber one of
> > those special classes.
> >
> > I believe that the added conditional, around the *negated* return value
> > of lockdep_match_class(), caused directory inodes to be placed in the
> > wrong lockdep class.
> >
> > inode_init_always() sets the i_mutex lockdep class with i_mutex_key for
> > all inodes. If the filesystem did not change the class during inode
> > initialization, then the conditional mentioned above was false and the
> > directory inode was incorrectly left in the non-directory lockdep class.
> > If the filesystem did set a special lockdep class, then the conditional
> > mentioned above was true and that class was clobbered with
> > i_mutex_dir_key.
> >
> > This patch removes the negation from the conditional so that the i_mutex
> > lockdep class is properly set for directory inodes. Special classes are
> > preserved and directory inodes with unmodified classes are set with
> > i_mutex_dir_key.
> Duh, you are right. I wonder how come this did not trigger any lockdep
> messages during my testing with ocfs2 for which I wrote the patch...
Good question. Adding the ocfs2 maintainers and devel list to cc, as
this patch is likely to change up their lockdep warnings.
I expect this will also solve many of the lockdep issues around the
order of i_mutex and mmap_sem being taken in files verse directories.
That's why I stumbled across it.
>
> Anyway, thanks for catching this! You can add
> Reviewed-by: Jan Kara <jack@xxxxxxx>
Thanks for the review!
Tyler
>
> Honza
> > Signed-off-by: Tyler Hicks <tyhicks@xxxxxxxxxxxxx>
> > ---
> > fs/inode.c | 3 +--
> > 1 files changed, 1 insertions(+), 2 deletions(-)
> >
> > diff --git a/fs/inode.c b/fs/inode.c
> > index ee4e66b..9d01a0d 100644
> > --- a/fs/inode.c
> > +++ b/fs/inode.c
> > @@ -855,8 +855,7 @@ void lockdep_annotate_inode_mutex_key(struct inode *inode)
> > struct file_system_type *type = inode->i_sb->s_type;
> >
> > /* Set new key only if filesystem hasn't already changed it */
> > - if (!lockdep_match_class(&inode->i_mutex,
> > - &type->i_mutex_key)) {
> > + if (lockdep_match_class(&inode->i_mutex, &type->i_mutex_key)) {
> > /*
> > * ensure nobody is actually holding i_mutex
> > */
> > --
> > 1.7.5.4
> >
> --
> Jan Kara <jack@xxxxxxx>
> SUSE Labs, CR
Attachment:
signature.asc
Description: Digital signature
