linux-kernel - Re: [PATCH 1/3] dcache: add a new enum type for 'dentry_d_lock

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20191115041243.GN26530@ZenIV.linux.org.uk>
Date:   Fri, 15 Nov 2019 04:12:43 +0000
From:   Al Viro <viro@...iv.linux.org.uk>
To:     Greg KH <gregkh@...uxfoundation.org>
Cc:     yu kuai <yukuai3@...wei.com>, rafael@...nel.org,
        rostedt@...dmis.org, oleg@...hat.com, mchehab+samsung@...nel.org,
        corbet@....net, tytso@....edu, jmorris@...ei.org,
        linux-kernel@...r.kernel.org, linux-fsdevel@...r.kernel.org,
        zhengbin13@...wei.com, yi.zhang@...wei.com,
        chenxiang66@...ilicon.com, xiexiuqi@...wei.com
Subject: Re: [PATCH 1/3] dcache: add a new enum type for 'dentry_d_lock_class'

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@...wei.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 ;-/