RE: [PATCH] ceph: is_root_ceph_dentry() cleanup

[Viacheslav Dubeyko <Slava.Dubeyko@xxxxxxx>]

On Tue, 2025-02-11 at 00:15 +0000, Al Viro wrote:
> On Tue, Feb 11, 2025 at 12:08:21AM +0000, Viacheslav Dubeyko wrote:
> 
> > In general, the MDS can issue NULL dentries to clients so that  they "know" the
> > direntry does not exist without having some capability (or lease) associated
> > with it. As far as I can see, if application repeatedly does stat of file, then
> > the kernel driver isn't repeatedly going out to the MDS to lookup that file. So,
> > I assume that this is the goal of this check and logic.
> 
> Er...  On repeated stat(2) you will get ->lookup() called the first time around;
> afterwards you'll be getting dentry from dcache lookup.  With ->d_revalidate()
> each time you find it in dcache, and eviction if ->d_revalidate() says it's stale.
> In which case a new dentry will be allocated and fed to ->lookup(), passed to
> it in negative-unhashed state...

After some considerations, I believe we can follow such simple logic.
Correct me if I will be wrong here. The ceph_lookup() method's responsibility is
to "look up a single dir entry". It sounds for me that if we have positive
dentry,
then it doesn't make sense to call the ceph_lookup(). And if ceph_lookup() has
been
called for the positive dentry, then something wrong is happening.

As far as I can see, currently, we have a confusing execution flow in
ceph_lookup():

if (d_really_is_negative(dentry)) {
    <do check locally>
    if (-ENOENT)
        return NULL;
}

<send request to MDS server>

But all this logic is not about negative dentry, it's about local check
before sending request to MDS server. So, I think we need to change the logic
in likewise way:

if (<we can check locally>) {
    <do check locally>
    if (-ENOENT)
        return NULL;
} else {
   <send request to MDS server>
}

Am I right here? :) Let me change the logic in this way and to test it.

Thanks,
Slava.