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 <eaujames@ddn.com>
Change-Id: Ic9823cddf37373dc95f4de3219c88c0fa0600fa7
Reviewed-on: https://review.whamcloud.com/c/fs/lustre-release/+/54354
Tested-by: jenkins <devops@whamcloud.com>
Tested-by: Maloo <maloo@whamcloud.com>
Reviewed-by: Alex Zhuravlev <bzzz@whamcloud.com>
Reviewed-by: James Simmons <jsimmons@infradead.org>
Reviewed-by: Oleg Drokin <green@whamcloud.com>
Reviewed-by: Lai Siyao <lai.siyao@whamcloud.com>

diff --git a/lustre/llite/dcache.c b/lustre/llite/dcache.c
index cbefe2f..83cfc6f 100644 (file)
--- 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 (file)
--- 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 (executable)
--- 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