Hi Paulo,
Thanks for your replies.
On Mon, Jan 13, 2025 at 8:55 PM Paulo Alcantara <pc@xxxxxxxxxxxxx> wrote:
>
> Shyam Prasad N <nspmangalore@xxxxxxxxx> writes:
>
> > Ideally, negative dentries should allow a filename lookup to happen
> > entirely from the dentry cache if the lookup had happened once
> > already. But I noticed that the SMB client goes to the server every
> > time we do a stat of a file that does not exist.
>
> This is a network filesystem. If the last lookup ended up with a
> negative dentry in dcache, that doesn't mean the file won't exist the
> next time we look it up again. The file could have been created by a
> different client, so we need to query it on server.
I agree. But we do have tools to trade performance for accuracy using
parameters like actimeo/acdirmax/acregmax.
So we can avoid going to the server each time if it's within some interval.
If the server gives us dir leases, we can be sure that the dentries
have not changed without us knowing. So we can definitely cache the
negative dentries till as long as we have the lease.
>
> > I investigated this more and it looks like vfs_getattr does make use
> > of negative dentry, but the revalidate always comes to
> > cifs_d_revalidate even for negative dentries. And we do not have the
> > code necessary to deal with it.
>
> I think we do. Check the places where we return 0 from
> cifs_d_revalidate(), meaning that the dentry will need to be looked up
> again and hopefully instantiated (e.g. file was created on server).
I was thinking so too. But I could see that a revalidate loop that
repeats every second is calling cifs_revalidate_dentry in
cifs_d_revalidate, which can only happen if d_really_is_positive
returns success.
if (d_really_is_positive(direntry)) {
inode = d_inode(direntry);
if ((flags & LOOKUP_REVAL) && !CIFS_CACHE_READ(CIFS_I(inode)))
CIFS_I(inode)->time = 0; /* force reval */
rc = cifs_revalidate_dentry(direntry); <<<<<<<<<<<<<<<<<<<
if (rc) {
cifs_dbg(FYI, "cifs_revalidate_dentry failed
with rc=%d", rc);
....
}
else {
....
return 1;
}
>
> > We do use d_really_is_positive before we do the dentry validation, but
> > it looks like that comes to us as success, even in case of
> > non-existent dentries. Is this expected?
>
> I don't think so.
>
> Al, am I missing someting?
--
Regards,
Shyam
