Re: [PATCH 1/3] dcache: add a new enum type for 'dentry_d_lock_class'

Al Viro
2019-11-15T04:13:13Z

On Fri, Nov 15, 2019 at 11:27:59AM +0800, Greg KH wrote:
> On Fri, Nov 15, 2019 at 11:27:50AM +0800, yu kuai wrote:
> > 'dentry_d_lock_class' can be used for spin_lock_nested in case lockdep
> > confused about two different dentry take the 'd_lock'.
> >
> > However, a single 'DENTRY_D_LOCK_NESTED' may not be enough if more than
> > two dentry are involed. So, and in 'DENTRY_D_LOCK_NESTED_2'
> >
> > Signed-off-by: yu kuai <yukuai3@huawei.com>
> > ---
> > include/linux/dcache.h | 3 ++-
> > 1 file changed, 2 insertions(+), 1 deletion(-)
> >
> > diff --git a/include/linux/dcache.h b/include/linux/dcache.h
> > index 10090f1..8eb84ef 100644
> > --- a/include/linux/dcache.h
> > +++ b/include/linux/dcache.h
> > @@ -129,7 +129,8 @@ struct dentry {
> > enum dentry_d_lock_class
> > {
> > DENTRY_D_LOCK_NORMAL, /* implicitly used by plain spin_lock() APIs. */
> > - DENTRY_D_LOCK_NESTED
> > + DENTRY_D_LOCK_NESTED,
> > + DENTRY_D_LOCK_NESTED_2
>
> You should document this, as "_2" does not make much sense to anyone
> only looking at the code :(
>
> Or rename it better.

FWIW, I'm not sure it's a good solution. What are the rules for callers
of that thing, anyway? If it can be called when somebody is creating
more files in that subtree, we almost certainly will have massive
problems with the lifetimes of underlying objects...

Could somebody familiar with debugfs explain how is that thing actually
used and what is required from/promised to its callers? I can try and
grep through the tree and guess what the rules are, but I've way too
much on my platter right now and I don't want to get sidetracked into yet
another tree-wide search and analysis session ;-/