* Tejun Heo <[email protected]> [170211 12:34]: > The release callback may be called from two places - file release > operation and kernfs open file draining. kernfs_open_file->mutex is > used to synchronize the two callsites. This unfortunately leads to > possible circular locking because of->mutex is used to protect the > usual kernfs operations which may use locking constructs which are > held while removing and thus draining kernfs files. > > @of->mutex is for synchronizing concurrent kernfs access operations > and all we need here is synchronization between the releaes and drain > paths. As the drain path has to grab kernfs_open_file_mutex anyway, > let's use the mutex to synchronize the release operation instead. > > Signed-off-by: Tejun Heo <[email protected]> > Reported-by: Tony Lindgren <[email protected]> > Fixes: 0e67db2f9fe9 ("kernfs: add kernfs_ops->open/release() callbacks") > --- > Hello, > > Tony, can you please verify that this resolves the lockdep warnings > that you've been seeing on linux-next?
Yes thanks this fixes the issue I was seeing: Tested-by: Tony Lindgren <[email protected]> > Greg, this is a fix for the kernfs patches which are being routed > through the cgroup tree. Once Tony confirms, I'll apply this patch on > top. > > Thanks! > > fs/kernfs/file.c | 19 ++++++++++++++----- > 1 file changed, 14 insertions(+), 5 deletions(-) > > --- a/fs/kernfs/file.c > +++ b/fs/kernfs/file.c > @@ -747,10 +747,15 @@ err_out: > static void kernfs_release_file(struct kernfs_node *kn, > struct kernfs_open_file *of) > { > - if (!(kn->flags & KERNFS_HAS_RELEASE)) > - return; > + /* > + * @of is guaranteed to have no other file operations in flight and > + * we just want to synchronize release and drain paths. > + * @kernfs_open_file_mutex is enough. @of->mutex can't be used > + * here because drain path may be called from places which can > + * cause circular dependency. > + */ > + lockdep_assert_held(&kernfs_open_file_mutex); > > - mutex_lock(&of->mutex); > if (!of->released) { > /* > * A file is never detached without being released and we > @@ -760,7 +765,6 @@ static void kernfs_release_file(struct k > kn->attr.ops->release(of); > of->released = true; > } > - mutex_unlock(&of->mutex); > } > > static int kernfs_fop_release(struct inode *inode, struct file *filp) > @@ -768,7 +772,12 @@ static int kernfs_fop_release(struct ino > struct kernfs_node *kn = filp->f_path.dentry->d_fsdata; > struct kernfs_open_file *of = kernfs_of(filp); > > - kernfs_release_file(kn, of); > + if (kn->flags & KERNFS_HAS_RELEASE) { > + mutex_lock(&kernfs_open_file_mutex); > + kernfs_release_file(kn, of); > + mutex_unlock(&kernfs_open_file_mutex); > + } > + > kernfs_put_open_node(kn, of); > seq_release(inode, filp); > kfree(of->prealloc_buf);
