From 32582842cae452984f74e76a4eb69379cc48ce5f Mon Sep 17 00:00:00 2001 From: Etienne AUJAMES Date: Mon, 11 Mar 2024 18:51:57 +0100 Subject: [PATCH] LU-17583 llite: getattr/open should not revalidate dentry ll_getattr() and ll_intent_file_open() do not perform a lookup, it get the attr and ldlm locks by FID (inode). So this should not revalidate the dentry, otherwise it may produce dir cache inconsistencies (e.g: with cwd fd). Add a regression test: sanityn 31s, 31t Fixes: 14ca315 ("LU-10948 llite: Revalidate dentries in ll_intent_file_open") Fixes: 92fadf9 ("LU-15200 llite: revalidate dentry if LOOKUP lock fetched") Test-Parameters: testlist=sanityn env=ONLY=31s,ONLY_REPEAT=20 Test-Parameters: testlist=sanityn env=ONLY=31t,ONLY_REPEAT=20 Signed-off-by: Etienne AUJAMES Change-Id: Ic9823cddf37373dc95f4de3219c88c0fa0600fa7 Reviewed-on: https://review.whamcloud.com/c/fs/lustre-release/+/54354 Tested-by: jenkins Tested-by: Maloo Reviewed-by: Alex Zhuravlev Reviewed-by: James Simmons Reviewed-by: Oleg Drokin Reviewed-by: Lai Siyao --- lustre/llite/dcache.c | 16 ++++++---------- lustre/llite/file.c | 22 ++++++++-------------- lustre/tests/sanityn.sh | 44 ++++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 58 insertions(+), 24 deletions(-) diff --git a/lustre/llite/dcache.c b/lustre/llite/dcache.c index cbefe2f..83cfc6f 100644 --- a/lustre/llite/dcache.c +++ b/lustre/llite/dcache.c @@ -273,16 +273,12 @@ int ll_revalidate_it_finish(struct ptlrpc_request *request, ll_set_lock_data(ll_i2sbi(inode)->ll_md_exp, inode, it, &bits); - if (bits & MDS_INODELOCK_LOOKUP) { - if (!ll_d_setup(de, true)) - RETURN(-ENOMEM); - d_lustre_revalidate(de); - if (S_ISDIR(inode->i_mode)) { - struct dentry *parent = dget_parent(de); - - ll_update_dir_depth_dmv(d_inode(parent), de); - dput(parent); - } + if ((bits & MDS_INODELOCK_LOOKUP) && + !d_lustre_invalid(de) && S_ISDIR(inode->i_mode)) { + struct dentry *parent = dget_parent(de); + + ll_update_dir_depth_dmv(d_inode(parent), de); + dput(parent); } RETURN(rc); diff --git a/lustre/llite/file.c b/lustre/llite/file.c index 96e0d7b..4663a5a 100644 --- a/lustre/llite/file.c +++ b/lustre/llite/file.c @@ -729,25 +729,17 @@ retry: if (!rc && itp->it_lock_mode) { __u64 bits = 0; - /* If we got a lock back and it has a LOOKUP bit set, - * make sure the dentry is marked as valid so we can find it. - * We don't need to care about actual hashing since other bits - * of kernel will deal with that later. - */ - ll_set_lock_data(sbi->ll_md_exp, de->d_inode, itp, &bits); - if (bits & MDS_INODELOCK_LOOKUP) - d_lustre_revalidate(de); - /* if DoM bit returned along with LAYOUT bit then there * can be read-on-open data returned. */ + ll_set_lock_data(sbi->ll_md_exp, de->d_inode, itp, &bits); if (bits & MDS_INODELOCK_DOM && bits & MDS_INODELOCK_LAYOUT) ll_dom_finish_open(de->d_inode, req); } /* open may not fetch LOOKUP lock, update dir depth and default LMV * anyway. */ - if (!rc && S_ISDIR(de->d_inode->i_mode)) + if (!rc && !d_lustre_invalid(de) && S_ISDIR(de->d_inode->i_mode)) ll_update_dir_depth_dmv(parent->d_inode, de); out: @@ -5700,13 +5692,17 @@ static int ll_inode_revalidate(struct dentry *dentry, enum ldlm_intent_flags op) struct md_op_data *op_data; const char *name = NULL; size_t namelen = 0; + int flags = 0; int rc = 0; ENTRY; CDEBUG(D_VFSTRACE, "VFS Op:inode="DFID"(%p),name=%s\n", PFID(ll_inode2fid(inode)), inode, dentry->d_name.name); - if (exp_connect_flags2(exp) & OBD_CONNECT2_GETATTR_PFID) { + /* Call getattr by fid */ + if ((exp_connect_flags2(exp) & OBD_CONNECT2_GETATTR_PFID) && + !d_lustre_invalid(dentry)) { + flags = MF_GETATTR_BY_FID; parent = dget_parent(dentry); dir = d_inode(parent); name = dentry->d_name.name; @@ -5722,9 +5718,7 @@ static int ll_inode_revalidate(struct dentry *dentry, enum ldlm_intent_flags op) if (IS_ERR(op_data)) RETURN(PTR_ERR(op_data)); - /* Call getattr by fid */ - if (exp_connect_flags2(exp) & OBD_CONNECT2_GETATTR_PFID) - op_data->op_flags = MF_GETATTR_BY_FID; + op_data->op_flags |= flags; rc = md_intent_lock(exp, op_data, &oit, &req, &ll_md_blocking_ast, 0); ll_finish_md_op_data(op_data); if (rc < 0) { diff --git a/lustre/tests/sanityn.sh b/lustre/tests/sanityn.sh index ba1db2d..b0d8962 100755 --- a/lustre/tests/sanityn.sh +++ b/lustre/tests/sanityn.sh @@ -1208,6 +1208,50 @@ test_31r() { } run_test 31r "open-rename(replace) race" +test_31s() { + local pid ls_str + + mkdir_on_mdt0 $DIR/$tdir + touch $DIR/$tdir/$tfile || error "touch $tdir/$tfile failed" + ( cd $DIR/$tdir; tail -f $tfile || $MULTIOP . Dc) & pid=$! + + stack_trap "pkill -P $pid 2> /dev/null" ERR + + mv $DIR2/$tdir $DIR2/$tdir.old || error "mv $tdir $tdir.old failed" + mkdir_on_mdt0 $DIR2/$tdir || error "mkdir $DIR2/$tdir failed" + mv $DIR2/$tdir.old $DIR2/$tdir/ || error "mv $tdir.old $tdir/ failed" + + pkill -P $pid || error "fail to stop $pid" + wait $pid + + ls_str="$(ls $DIR/$tdir)" || error "ls $DIR/$tdir failed" + [[ "$ls_str" == "$tdir.old" ]] || + error "invalid dentry $DIR/$tdir: $ls_str != $tdir.old" +} +run_test 31s "open should not revalidate invalid dentry" + +test_31t() { + local pid i i_new + + touch $DIR/$tfile || error "touch $tfile failed" + + $MULTIOP $DIR/$tfile o_Sc & pid=$! + stack_trap "kill $pid 2> /dev/null" ERR + sleep 1 + + mv $DIR2/$tfile $DIR2/$tfile.old || error "mv $tfile $tfile.old failed" + touch $DIR2/$tfile || error "touch $tfile failed" + i_new=$(stat -c "%i" $DIR2/$tfile) || error "stat $tfile failed" + + kill -SIGUSR1 $pid || error "fail to stop $pid" + wait $pid + + i=$(stat -c "%i" $DIR/$tfile) || error "stat $tfile failed" + (( i == i_new )) || + error "invalid dentry $DIR/$tfile, not updated (inode: $i != $i_new)" +} +run_test 31t "getattr should not revalidate invalid dentry" + test_32b() { # bug 11270 remote_ost_nodsh && skip "remote OST with nodsh" && return -- 1.8.3.1