On Tue, Jan 28, 2025 at 11:27:05PM +0000, Viacheslav Dubeyko wrote:
> I assume that you imply this code:
>
> /* can we conclude ENOENT locally? */
> if (d_really_is_negative(dentry)) {
> struct ceph_inode_info *ci = ceph_inode(dir);
> struct ceph_dentry_info *di = ceph_dentry(dentry);
>
> spin_lock(&ci->i_ceph_lock);
> doutc(cl, " dir %llx.%llx flags are 0x%lx\n",
> ceph_vinop(dir), ci->i_ceph_flags);
> if (strncmp(dentry->d_name.name,
> fsc->mount_options->snapdir_name,
> dentry->d_name.len) &&
> !is_root_ceph_dentry(dir, dentry) &&
> ceph_test_mount_opt(fsc, DCACHE) &&
> __ceph_dir_is_complete(ci) &&
> __ceph_caps_issued_mask_metric(ci, CEPH_CAP_FILE_SHARED,
> 1)) {
> __ceph_touch_fmode(ci, mdsc, CEPH_FILE_MODE_RD);
> spin_unlock(&ci->i_ceph_lock);
> doutc(cl, " dir %llx.%llx complete, -ENOENT\n",
> ceph_vinop(dir));
> d_add(dentry, NULL);
> di->lease_shared_gen = atomic_read(&ci->i_shared_gen);
> return NULL;
> }
> spin_unlock(&ci->i_ceph_lock);
> }
>
> Am I correct? So, how can we rework this code if it's wrong? What is your
> vision? Do you mean that it's dead code? How can we check it?
I mean that ->lookup() is called *ONLY* for a negative unhashed dentries.
In other words, on a call from VFS that condition will always be true.
That part is easily provable; what is harder to reason about is the
direct call of ceph_lookup() from ceph_handle_notrace_create().
The callers of that thing (ceph_mknod(), ceph_symlink() and ceph_mkdir())
are all guaranteed that dentry will be negative when they are called.
The hard-to-reason-about part is the call of ceph_mdsc_do_request()
directly preceding the calls of ceph_handle_notrace_create().
Can ceph_mdsc_do_request() return 0, with req->r_reply_info.head->is_dentry
being false *AND* a call of splice_dentry() made by ceph_fill_trace() called
by ceph_mdsc_do_request()?
AFAICS, there are 3 calls of splice_dentry(); two of them happen under
explicit check for ->is_dentry and thus are not interesting for our
purposes. The third one, though, could be hit with ->is_dentry being
false and ->r_op being CEPH_MDS_OP_MKSNAP. That is not impossible
from ceph_mkdir(), as far as I can tell, and I don't understand the
details well enough to tell whether it can actually happen.
Is it actually possible to hit ceph_handle_notrace_create() with
a positive dentry?
