On 2020/2/17 22:52, Ilya Dryomov wrote:
On Mon, Feb 17, 2020 at 12:28 PM <xiubli@xxxxxxxxxx> wrote:
From: Xiubo Li <xiubli@xxxxxxxxxx>
For example, if dentry and inode is NULL, the log will be:
ceph: lookup result=000000007a1ca695
ceph: submit_request on 0000000041d5070e for inode 000000007a1ca695
The will be confusing without checking the corresponding code carefully.
Signed-off-by: Xiubo Li <xiubli@xxxxxxxxxx>
---
fs/ceph/dir.c | 2 +-
fs/ceph/mds_client.c | 6 +++++-
2 files changed, 6 insertions(+), 2 deletions(-)
diff --git a/fs/ceph/dir.c b/fs/ceph/dir.c
index ffeaff5bf211..245a262ec198 100644
--- a/fs/ceph/dir.c
+++ b/fs/ceph/dir.c
@@ -798,7 +798,7 @@ static struct dentry *ceph_lookup(struct inode *dir, struct dentry *dentry,
err = ceph_handle_snapdir(req, dentry, err);
dentry = ceph_finish_lookup(req, dentry, err);
ceph_mdsc_put_request(req); /* will dput(dentry) */
- dout("lookup result=%p\n", dentry);
+ dout("lookup result=%d\n", err);
return dentry;
}
diff --git a/fs/ceph/mds_client.c b/fs/ceph/mds_client.c
index b6aa357f7c61..e34f159d262b 100644
--- a/fs/ceph/mds_client.c
+++ b/fs/ceph/mds_client.c
@@ -2772,7 +2772,11 @@ int ceph_mdsc_submit_request(struct ceph_mds_client *mdsc, struct inode *dir,
ceph_get_cap_refs(ceph_inode(req->r_old_dentry_dir),
CEPH_CAP_PIN);
- dout("submit_request on %p for inode %p\n", req, dir);
+ if (dir)
+ dout("submit_request on %p for inode %p\n", req, dir);
+ else
+ dout("submit_request on %p\n", req);
Hi Xiubo,
It's been this way for a couple of years now. There are a lot more
douts in libceph, ceph and rbd that are sometimes fed NULL pointers.
I don't think replacing them with conditionals is the way forward.
I honestly don't know what security concern is addressed by hashing
NULL pointers, but that is what we have... Ultimately, douts are just
for developers, and when you find yourself having to chase individual
pointers, you usually have a large enough piece of log to figure out
what the NULL hash is.
Hi Ilya
For the ceph_lookup(). The dentry will be NULL(when the directory exists
or -ENOENT) or ERR_PTR(-errno) in most cases here, it seems for the
rename case it will be the old dentry returned.
So today I was trying to debug and get some logs from it, the
000000007a1ca695 really confused me for a long time before I dig into
the source code.
Thanks,
Thanks,
Ilya
