Whamcloud - gitweb
LU-13017 tests: disable statahead_agl for sanity test_56ra 67/39667/5
authorMr NeilBrown <neilb@suse.de>
Thu, 13 Aug 2020 05:26:54 +0000 (15:26 +1000)
committerOleg Drokin <green@whamcloud.com>
Mon, 12 Oct 2020 05:44:40 +0000 (05:44 +0000)
The sanity test_56ra can fail because statahead_agl can cause extra
glimpse request.

If a stat() systemcall is made after an AGL glimpse request is sent,
but before the reply has been received, the code handling the stat
cannot see that glimpse request and so will send another.  This
elevates the number of requests counted.

There is a parameter (statahead_agl) which make it easy to disable the
AGL, but it isn't implemented properly.  Specifically, inodes can
still be added to the sai_agls list when agl is disabled.  They will
never be removed, which causes an assertion to fail.

To clean this up, remove the sai_agl_valid flag, and use a test on
sai_task being non-NULL instead.  Also check agl_should_run() while
locked against ->sai_task changing, and before adding anything
to lli_agl_list.

We don't need the 'added' variable.  It is perfectly OK to wake_up the
sai_agl_task *before* adding to the list as long is that is all done
under the lock.  The task will wait for the lock before checking the
list, so it won't see it being empty.

Signed-off-by: Mr NeilBrown <neilb@suse.de>
Change-Id: I12c4e447104a86b3f48eaf57b6cf7ce4b41cc6de
Reviewed-on: https://review.whamcloud.com/39667
Reviewed-by: Lai Siyao <lai.siyao@whamcloud.com>
Reviewed-by: Yingjin Qian <qian@ddn.com>
Tested-by: jenkins <devops@whamcloud.com>
Tested-by: Maloo <maloo@whamcloud.com>
Reviewed-by: James Simmons <jsimmons@infradead.org>
Reviewed-by: Oleg Drokin <green@whamcloud.com>
lustre/llite/llite_internal.h
lustre/llite/statahead.c
lustre/tests/sanity.sh

index 2cb186f..ad2a7f5 100644 (file)
@@ -1436,7 +1436,6 @@ struct ll_statahead_info {
                                                 */
        unsigned int            sai_ls_all:1,   /* "ls -al", do stat-ahead for
                                                 * hidden entries */
-                               sai_agl_valid:1,/* AGL is valid for the dir */
                                sai_in_readpage:1;/* statahead is in readdir()*/
        wait_queue_head_t       sai_waitq;      /* stat-ahead wait queue */
        struct task_struct      *sai_task;      /* stat-ahead thread */
index 7cd9df5..e0e008b 100644 (file)
@@ -132,7 +132,7 @@ sa_unhash(struct ll_statahead_info *sai, struct sa_entry *entry)
 static inline int agl_should_run(struct ll_statahead_info *sai,
                                 struct inode *inode)
 {
-       return (inode && S_ISREG(inode->i_mode) && sai->sai_agl_valid);
+       return inode && S_ISREG(inode->i_mode) && sai->sai_agl_task;
 }
 
 static inline struct ll_inode_info *
@@ -432,7 +432,6 @@ static void ll_agl_add(struct ll_statahead_info *sai,
 {
        struct ll_inode_info *child  = ll_i2info(inode);
        struct ll_inode_info *parent = ll_i2info(sai->sai_dentry->d_inode);
-       int                   added  = 0;
 
        spin_lock(&child->lli_agl_lock);
        if (child->lli_agl_index == 0) {
@@ -441,13 +440,15 @@ static void ll_agl_add(struct ll_statahead_info *sai,
 
                LASSERT(list_empty(&child->lli_agl_list));
 
-               igrab(inode);
                spin_lock(&parent->lli_agl_lock);
-               if (agl_list_empty(sai))
-                       added = 1;
-               list_add_tail(&child->lli_agl_list, &sai->sai_agls);
-               if (added && sai->sai_agl_task)
-                       wake_up_process(sai->sai_agl_task);
+               /* Re-check under the lock */
+               if (agl_should_run(sai, inode)) {
+                       if (agl_list_empty(sai))
+                               wake_up_process(sai->sai_agl_task);
+                       igrab(inode);
+                       list_add_tail(&child->lli_agl_list, &sai->sai_agls);
+               } else
+                       child->lli_agl_index = 0;
                spin_unlock(&parent->lli_agl_lock);
        } else {
                spin_unlock(&child->lli_agl_lock);
@@ -957,7 +958,6 @@ static void ll_stop_agl(struct ll_statahead_info *sai)
        kthread_stop(agl_task);
 
        spin_lock(&plli->lli_agl_lock);
-       sai->sai_agl_valid = 0;
        while (!agl_list_empty(sai)) {
                clli = agl_first_entry(sai);
                list_del_init(&clli->lli_agl_list);
@@ -989,11 +989,9 @@ static void ll_start_agl(struct dentry *parent, struct ll_statahead_info *sai)
                                      plli->lli_opendir_pid);
        if (IS_ERR(task)) {
                CERROR("can't start ll_agl thread, rc: %ld\n", PTR_ERR(task));
-               sai->sai_agl_valid = 0;
                RETURN_EXIT;
        }
        sai->sai_agl_task = task;
-       LASSERT(sai->sai_agl_valid == 1);
        atomic_inc(&ll_i2sbi(d_inode(parent))->ll_agl_total);
        /* Get an extra reference that the thread holds */
        ll_sai_get(d_inode(parent));
@@ -1588,7 +1586,6 @@ static int start_statahead_thread(struct inode *dir, struct dentry *dentry,
                GOTO(out, rc = -ENOMEM);
 
        sai->sai_ls_all = (first == LS_FIRST_DOT_DE);
-       sai->sai_agl_valid = agl;
 
        /*
         * if current lli_opendir_key was deauthorized, or dir re-opened by
@@ -1659,10 +1656,11 @@ static inline bool ll_statahead_started(struct inode *dir, bool agl)
 
        spin_lock(&lli->lli_sa_lock);
        sai = lli->lli_sai;
-       if (sai && sai->sai_agl_valid != agl)
+       if (sai && (sai->sai_agl_task != NULL) != agl)
                CDEBUG(D_READA,
                       "%s: Statahead AGL hint changed from %d to %d\n",
-                      ll_i2sbi(dir)->ll_fsname, sai->sai_agl_valid, agl);
+                      ll_i2sbi(dir)->ll_fsname,
+                      sai->sai_agl_task != NULL, agl);
        spin_unlock(&lli->lli_sa_lock);
 
        return !!sai;
index 52699a6..079ac70 100755 (executable)
@@ -6257,8 +6257,13 @@ test_56ra() {
        [[ $MDS1_VERSION -gt $(version_code 2.12.58) ]] ||
                skip "MDS < 2.12.58 doesn't return LSOM data"
        local dir=$DIR/$tdir
+       local old_agl=$($LCTL get_param -n llite.*.statahead_agl)
 
-       [[ $OSC == "mdc" ]] && skip "DoM files" && return
+       [[ $OSC == "mdc" ]] && skip "statahead not needed for DoM files"
+
+       # statahead_agl may cause extra glimpse which confuses results. LU-13017
+       $LCTL set_param -n llite.*.statahead_agl=0
+       stack_trap "$LCTL set_param -n llite.*.statahead_agl=$old_agl"
 
        setup_56 $dir $NUMFILES $NUMDIRS "-c 1"
        # open and close all files to ensure LSOM is updated