All of lore.kernel.org
 help / color / mirror / Atom feed
From: Al Viro <viro@zeniv.linux.org.uk>
To: Viacheslav Dubeyko <Slava.Dubeyko@ibm.com>
Cc: "idryomov@gmail.com" <idryomov@gmail.com>,
	Alex Markuze <amarkuze@redhat.com>,
	"slava@dubeyko.com" <slava@dubeyko.com>,
	"ceph-devel@vger.kernel.org" <ceph-devel@vger.kernel.org>,
	"linux-fsdevel@vger.kernel.org" <linux-fsdevel@vger.kernel.org>,
	Patrick Donnelly <pdonnell@redhat.com>
Subject: Re: [PATCH] ceph: is_root_ceph_dentry() cleanup
Date: Tue, 11 Feb 2025 19:01:11 +0000	[thread overview]
Message-ID: <20250211190111.GH1977892@ZenIV> (raw)
In-Reply-To: <01dc18199e660f7f9b9ea78c89aa0c24ba09a173.camel@ibm.com>

On Tue, Feb 11, 2025 at 06:01:21PM +0000, Viacheslav Dubeyko wrote:

> 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.

VFS never calls it that way; ceph itself might, if ceph_handle_notrace_create()
is called with positive dentry.

> 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.

First of all, returning NULL does *not* mean "it's negative"; d_add(dentry, NULL)
does that.  What would "we can check locally" be?  AFAICS, the checks in
__ceph_dir_is_complete() and near it are doing just that...

The really unpleasant question is whether ceph_handle_notrace_create() could
end up feeding an already-positive dentry to direct call of ceph_lookup()...

  reply	other threads:[~2025-02-11 19:01 UTC|newest]

Thread overview: 10+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2025-01-28  1:10 [PATCH] ceph: is_root_ceph_dentry() cleanup Viacheslav Dubeyko
2025-01-28  3:07 ` Al Viro
2025-01-28 23:27   ` Viacheslav Dubeyko
2025-01-29  1:12     ` Al Viro
2025-02-11  0:08       ` Viacheslav Dubeyko
2025-02-11  0:15         ` Al Viro
2025-02-11 18:01           ` Viacheslav Dubeyko
2025-02-11 19:01             ` Al Viro [this message]
2025-02-11 19:32               ` Viacheslav Dubeyko
2025-02-11 21:40                 ` Al Viro

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20250211190111.GH1977892@ZenIV \
    --to=viro@zeniv.linux.org.uk \
    --cc=Slava.Dubeyko@ibm.com \
    --cc=amarkuze@redhat.com \
    --cc=ceph-devel@vger.kernel.org \
    --cc=idryomov@gmail.com \
    --cc=linux-fsdevel@vger.kernel.org \
    --cc=pdonnell@redhat.com \
    --cc=slava@dubeyko.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.