From e292092b4b725f11961a23cbabf4d4ad30fc848e Mon Sep 17 00:00:00 2001 From: Ashley Pittman Date: Fri, 14 Jul 2023 12:53:11 +0000 Subject: [PATCH 1/3] DAOS-12751 dfuse: Improve evict command. Improve error cases where path is not backed by dfuse. Lock forget commands against inode query. Print and accept inodes as integers only. Evict the build tree as part of the DaosBuild test. Required-githooks: true Signed-off-by: Ashley Pittman --- src/client/dfuse/dfuse.h | 21 ++++++++++++ src/client/dfuse/dfuse_core.c | 4 +++ src/client/dfuse/ops/forget.c | 53 +++++++++++++---------------- src/client/dfuse/ops/ioctl.c | 15 ++++---- src/control/cmd/daos/filesystem.go | 4 +-- src/tests/ftest/dfuse/daos_build.py | 3 ++ src/utils/daos_hdlr.c | 20 ++++++++--- utils/node_local_test.py | 2 +- 8 files changed, 77 insertions(+), 45 deletions(-) diff --git a/src/client/dfuse/dfuse.h b/src/client/dfuse/dfuse.h index 31e9331e0c9..58abf0ed9f5 100644 --- a/src/client/dfuse/dfuse.h +++ b/src/client/dfuse/dfuse.h @@ -43,6 +43,9 @@ struct dfuse_info { */ pthread_spinlock_t di_lock; + /* RW lock used for force filesystem query ioctl to block for pending forget calls. */ + pthread_rwlock_t di_forget_lock; + /** Hash table of open inodes, this matches kernel ref counts */ struct d_hash_table dpi_iet; /** Hash table of open pools */ @@ -791,6 +794,24 @@ struct dfuse_inode_entry { bool ie_unlinked; }; +static inline struct dfuse_inode_entry * +dfuse_inode_lookup(struct dfuse_info *dfuse_info, fuse_ino_t ino) +{ + d_list_t *rlink; + + rlink = d_hash_rec_find(&dfuse_info->dpi_iet, &ino, sizeof(ino)); + if (!rlink) + return NULL; + + return container_of(rlink, struct dfuse_inode_entry, ie_htl); +} + +static inline void +dfuse_inode_decref(struct dfuse_info *dfuse_info, struct dfuse_inode_entry *ie) +{ + d_hash_rec_decref(&dfuse_info->dpi_iet, &ie->ie_htl); +} + extern char *duns_xattr_name; /* Generate the inode to use for this dfs object. This is generating a single diff --git a/src/client/dfuse/dfuse_core.c b/src/client/dfuse/dfuse_core.c index efab59965eb..6c8839b6386 100644 --- a/src/client/dfuse/dfuse_core.c +++ b/src/client/dfuse/dfuse_core.c @@ -1042,6 +1042,8 @@ dfuse_fs_init(struct dfuse_info *fs_handle) D_SPIN_INIT(&fs_handle->di_lock, 0); + D_RWLOCK_INIT(&fs_handle->di_forget_lock, 0); + for (i = 0; i < fs_handle->di_eq_count; i++) { struct dfuse_eq *eqt = &fs_handle->di_eqt[i]; @@ -1071,6 +1073,7 @@ dfuse_fs_init(struct dfuse_info *fs_handle) err_eq: D_SPIN_DESTROY(&fs_handle->di_lock); + D_RWLOCK_DESTROY(&fs_handle->di_forget_lock); for (i = 0; i < fs_handle->di_eq_count; i++) { struct dfuse_eq *eqt = &fs_handle->di_eqt[i]; @@ -1529,6 +1532,7 @@ dfuse_fs_fini(struct dfuse_info *dfuse_info) int i; D_SPIN_DESTROY(&dfuse_info->di_lock); + D_RWLOCK_DESTROY(&dfuse_info->di_forget_lock); for (i = 0; i < dfuse_info->di_eq_count; i++) { struct dfuse_eq *eqt = &dfuse_info->di_eqt[i]; diff --git a/src/client/dfuse/ops/forget.c b/src/client/dfuse/ops/forget.c index 62689714da7..0f4dbf6ac5a 100644 --- a/src/client/dfuse/ops/forget.c +++ b/src/client/dfuse/ops/forget.c @@ -1,5 +1,5 @@ /** - * (C) Copyright 2016-2022 Intel Corporation. + * (C) Copyright 2016-2023 Intel Corporation. * * SPDX-License-Identifier: BSD-2-Clause-Patent */ @@ -8,58 +8,51 @@ #include "dfuse.h" static void -dfuse_forget_one(struct dfuse_projection_info *fs_handle, - fuse_ino_t ino, uintptr_t nlookup) +dfuse_forget_one(struct dfuse_info *dfuse_info, fuse_ino_t ino, uintptr_t nlookup) { - d_list_t *rlink; - int rc; + struct dfuse_inode_entry *ie; + int rc; - /* One additional reference is needed because the rec_find() itself - * acquires one - */ + /* One additional reference is needed because the rec_find() itself acquires one */ nlookup++; - rlink = d_hash_rec_find(&fs_handle->dpi_iet, &ino, sizeof(ino)); - if (!rlink) { - DFUSE_TRA_WARNING(fs_handle, "Unable to find ref for %#lx %lu", - ino, nlookup); + ie = dfuse_inode_lookup(dfuse_info, ino); + if (!ie) { + DFUSE_TRA_WARNING(dfuse_info, "Unable to find ref for %#lx %lu", ino, nlookup); return; } - DFUSE_TRA_DEBUG(container_of(rlink, struct dfuse_inode_entry, ie_htl), - "inode %#lx count %lu", - ino, nlookup); + DFUSE_TRA_DEBUG(ie, "inode %#lx count %lu", ino, nlookup); - rc = d_hash_rec_ndecref(&fs_handle->dpi_iet, nlookup, rlink); - if (rc != -DER_SUCCESS) { - DFUSE_TRA_ERROR(fs_handle, "Invalid refcount %lu on %p", - nlookup, - container_of(rlink, struct dfuse_inode_entry, - ie_htl)); - } + rc = d_hash_rec_ndecref(&dfuse_info->dpi_iet, nlookup, &ie->ie_htl); + if (rc != -DER_SUCCESS) + DFUSE_TRA_ERROR(dfuse_info, "Invalid refcount %lu on %p", nlookup, ie); } void dfuse_cb_forget(fuse_req_t req, fuse_ino_t ino, uintptr_t nlookup) { - struct dfuse_projection_info *fs_handle = fuse_req_userdata(req); + struct dfuse_info *dfuse_info = fuse_req_userdata(req); fuse_reply_none(req); - dfuse_forget_one(fs_handle, ino, nlookup); + dfuse_forget_one(dfuse_info, ino, nlookup); } void -dfuse_cb_forget_multi(fuse_req_t req, size_t count, - struct fuse_forget_data *forgets) +dfuse_cb_forget_multi(fuse_req_t req, size_t count, struct fuse_forget_data *forgets) { - struct dfuse_projection_info *fs_handle = fuse_req_userdata(req); - int i; + struct dfuse_info *dfuse_info = fuse_req_userdata(req); + int i; fuse_reply_none(req); - DFUSE_TRA_DEBUG(fs_handle, "Forgetting %zi", count); + DFUSE_TRA_DEBUG(dfuse_info, "Forgetting %zi", count); + + D_RWLOCK_RDLOCK(&dfuse_info->di_forget_lock); for (i = 0; i < count; i++) - dfuse_forget_one(fs_handle, forgets[i].ino, forgets[i].nlookup); + dfuse_forget_one(dfuse_info, forgets[i].ino, forgets[i].nlookup); + + D_RWLOCK_UNLOCK(&dfuse_info->di_forget_lock); } diff --git a/src/client/dfuse/ops/ioctl.c b/src/client/dfuse/ops/ioctl.c index 03f6536cc76..cdf28caad4d 100644 --- a/src/client/dfuse/ops/ioctl.c +++ b/src/client/dfuse/ops/ioctl.c @@ -304,14 +304,16 @@ handle_cont_qe_ioctl_helper(fuse_req_t req, const struct dfuse_mem_query *in_que struct dfuse_mem_query query = {}; if (in_query && in_query->ino) { - d_list_t *rlink; + struct dfuse_inode_entry *ie; - rlink = - d_hash_rec_find(&dfuse_info->dpi_iet, &in_query->ino, sizeof(in_query->ino)); - if (rlink) { + D_RWLOCK_WRLOCK(&dfuse_info->di_forget_lock); + + ie = dfuse_inode_lookup(dfuse_info, in_query->ino); + if (ie) { query.found = true; - d_hash_rec_decref(&dfuse_info->dpi_iet, rlink); + dfuse_inode_decref(dfuse_info, ie); } + D_RWLOCK_UNLOCK(&dfuse_info->di_forget_lock); } query.inode_count = atomic_load_relaxed(&dfuse_info->di_inode_count); @@ -326,11 +328,10 @@ static void handle_cont_query_ioctl(fuse_req_t req, const void *in_buf, size_t in_bufsz) { struct dfuse_info *dfuse_info = fuse_req_userdata(req); - struct dfuse_mem_query query = {}; const struct dfuse_mem_query *in_query = in_buf; int rc; - if (in_bufsz != sizeof(query)) + if (in_bufsz != sizeof(struct dfuse_mem_query)) D_GOTO(err, rc = EIO); handle_cont_qe_ioctl_helper(req, in_query); diff --git a/src/control/cmd/daos/filesystem.go b/src/control/cmd/daos/filesystem.go index 0191a54a8f0..02344d91cb2 100644 --- a/src/control/cmd/daos/filesystem.go +++ b/src/control/cmd/daos/filesystem.go @@ -499,9 +499,9 @@ func (cmd *fsDfuseQueryCmd) Execute(_ []string) error { cmd.Infof(" Open files: %d", ap.dfuse_mem.fh_count) if cmd.Ino != 0 { if ap.dfuse_mem.found { - cmd.Infof(" Inode %#lx resident", cmd.Ino) + cmd.Infof(" Inode %d resident", cmd.Ino) } else { - cmd.Infof(" Inode %#lx not resident", cmd.Ino) + cmd.Infof(" Inode %d not resident", cmd.Ino) } } diff --git a/src/tests/ftest/dfuse/daos_build.py b/src/tests/ftest/dfuse/daos_build.py index 3e0076e24e8..f869d396f57 100644 --- a/src/tests/ftest/dfuse/daos_build.py +++ b/src/tests/ftest/dfuse/daos_build.py @@ -203,6 +203,9 @@ def run_build_test(self, cache_mode, intercept=False, dfuse_namespace=None): 'python3 -m pip install pip --upgrade', 'python3 -m pip install -r {}/requirements.txt'.format(build_dir), 'scons -C {} --jobs {} --build-deps=only'.format(build_dir, build_jobs), + 'daos filesystem query {}'.format(mount_dir), + 'daos filesystem evict {}'.format(build_dir), + 'daos filesystem query {}'.format(mount_dir), 'scons -C {} --jobs {}'.format(build_dir, intercept_jobs)] for cmd in cmds: command = '{};{}'.format(preload_cmd, cmd) diff --git a/src/utils/daos_hdlr.c b/src/utils/daos_hdlr.c index 2d8b87050be..15e89948767 100644 --- a/src/utils/daos_hdlr.c +++ b/src/utils/daos_hdlr.c @@ -2441,7 +2441,8 @@ dfuse_count_query(struct cmd_args_s *ap) fd = open(ap->path, O_NOFOLLOW, O_RDONLY); if (fd < 0) { rc = errno; - DH_PERROR_SYS(ap, rc, "Failed to open path"); + if (rc != ENOENT) + DH_PERROR_SYS(ap, rc, "Failed to open path"); return daos_errno2der(rc); } @@ -2450,8 +2451,13 @@ dfuse_count_query(struct cmd_args_s *ap) rc = ioctl(fd, DFUSE_IOCTL_COUNT_QUERY, &query); if (rc < 0) { rc = errno; - DH_PERROR_SYS(ap, rc, "ioctl failed"); - rc = daos_errno2der(errno); + if (rc == ENOTTY) { + /* TODO: What is the correct error here? */ + rc = -DER_INVAL; + } else { + DH_PERROR_SYS(ap, rc, "ioctl failed"); + rc = daos_errno2der(errno); + } goto close; } @@ -2542,8 +2548,12 @@ dfuse_evict(struct cmd_args_s *ap) rc = ioctl(fd, DFUSE_IOCTL_DFUSE_EVICT, &query); if (rc < 0) { rc = errno; - DH_PERROR_SYS(ap, rc, "ioctl failed"); - rc = daos_errno2der(errno); + if (rc == ENOTTY) { + rc = -DER_INVAL; + } else { + DH_PERROR_SYS(ap, rc, "ioctl failed"); + rc = daos_errno2der(errno); + } goto close; } diff --git a/utils/node_local_test.py b/utils/node_local_test.py index 0b2d151b6de..c59f69da649 100755 --- a/utils/node_local_test.py +++ b/utils/node_local_test.py @@ -5424,7 +5424,7 @@ def get_cmd(cont_id): os.symlink('broken', join(sub_dir, 'broken_s')) os.symlink('file.0', join(sub_dir, 'link')) - rc = run_daos_cmd(conf, ['filesystem', 'copy', '--src', src_dir, + rc = run_daos_cmd(conf, ['filesystem', 'copy', '--src', sub_dir, '--dst', f'daos://{pool.id()}/aft_base']) assert rc.returncode == 0, rc From edcab4190595882f96191df981859f743f885f00 Mon Sep 17 00:00:00 2001 From: Ashley Pittman Date: Fri, 14 Jul 2023 16:36:47 +0000 Subject: [PATCH 2/3] use -DER_NOTAPPLICABLE Required-githooks: true Signed-off-by: Ashley Pittman --- src/utils/daos_hdlr.c | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/src/utils/daos_hdlr.c b/src/utils/daos_hdlr.c index 15e89948767..f79d009e909 100644 --- a/src/utils/daos_hdlr.c +++ b/src/utils/daos_hdlr.c @@ -2452,8 +2452,7 @@ dfuse_count_query(struct cmd_args_s *ap) if (rc < 0) { rc = errno; if (rc == ENOTTY) { - /* TODO: What is the correct error here? */ - rc = -DER_INVAL; + rc = -DER_NOTAPPLICABLE; } else { DH_PERROR_SYS(ap, rc, "ioctl failed"); rc = daos_errno2der(errno); @@ -2549,7 +2548,7 @@ dfuse_evict(struct cmd_args_s *ap) if (rc < 0) { rc = errno; if (rc == ENOTTY) { - rc = -DER_INVAL; + rc = -DER_NOTAPPLICABLE; } else { DH_PERROR_SYS(ap, rc, "ioctl failed"); rc = daos_errno2der(errno); From ebb26684b62c32fe65c1e39aff0dff09331a65a8 Mon Sep 17 00:00:00 2001 From: Ashley Pittman Date: Mon, 17 Jul 2023 11:32:17 +0000 Subject: [PATCH 3/3] Switch back to DER_MISC Test-tag: dfuse Required-githooks: true Signed-off-by: Ashley Pittman --- src/utils/daos_hdlr.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/utils/daos_hdlr.c b/src/utils/daos_hdlr.c index 116d3812b49..0a486cd63b7 100644 --- a/src/utils/daos_hdlr.c +++ b/src/utils/daos_hdlr.c @@ -2450,7 +2450,7 @@ dfuse_count_query(struct cmd_args_s *ap) if (rc < 0) { rc = errno; if (rc == ENOTTY) { - rc = -DER_NOTAPPLICABLE; + rc = -DER_MISC; } else { DH_PERROR_SYS(ap, rc, "ioctl failed"); rc = daos_errno2der(errno); @@ -2546,7 +2546,7 @@ dfuse_evict(struct cmd_args_s *ap) if (rc < 0) { rc = errno; if (rc == ENOTTY) { - rc = -DER_NOTAPPLICABLE; + rc = -DER_MISC; } else { DH_PERROR_SYS(ap, rc, "ioctl failed"); rc = daos_errno2der(errno);