Print this page
8040 NFSv4 client: 3-way deadlock between nfs4_bio(), nfs4_do_delegreturn(), and nfs4_flush_pages()
Reviewed by: Arne Jansen <arne@die-jansens.de>
Reviewed by: Vitaliy Gusev <gusev.vitaliy@icloud.com>
Approved by: Gordon Ross <gordon.w.ross@gmail.com>

Split Close
Expand all
Collapse all
          --- old/usr/src/uts/common/fs/nfs/nfs4_client.c
          +++ new/usr/src/uts/common/fs/nfs/nfs4_client.c
↓ open down ↓ 16 lines elided ↑ open up ↑
  17   17   * information: Portions Copyright [yyyy] [name of copyright owner]
  18   18   *
  19   19   * CDDL HEADER END
  20   20   */
  21   21  /*
  22   22   * Copyright (c) 1986, 2010, Oracle and/or its affiliates. All rights reserved.
  23   23   * Copyright (c) 2017 by Delphix. All rights reserved.
  24   24   */
  25   25  
  26   26  /*
  27      - *      Copyright (c) 1983,1984,1985,1986,1987,1988,1989  AT&T.
       27 + *      Copyright (c) 1983,1984,1985,1986,1987,1988,1989  AT&T.
  28   28   *      All Rights Reserved
  29   29   */
  30   30  
  31   31  #include <sys/param.h>
  32   32  #include <sys/types.h>
  33   33  #include <sys/systm.h>
  34   34  #include <sys/thread.h>
  35   35  #include <sys/t_lock.h>
  36   36  #include <sys/time.h>
  37   37  #include <sys/vnode.h>
↓ open down ↓ 419 lines elided ↑ open up ↑
 457  457          ASSERT(mi->mi_vfsp->vfs_dev == garp->n4g_va.va_fsid);
 458  458  
 459  459          /* Is curthread the recovery thread? */
 460  460          mutex_enter(&mi->mi_lock);
 461  461          recov = (VTOMI4(vp)->mi_recovthread == curthread);
 462  462          mutex_exit(&mi->mi_lock);
 463  463  
 464  464          rp = VTOR4(vp);
 465  465          mutex_enter(&rp->r_statelock);
 466  466          was_serial = (rp->r_serial == curthread);
 467      -        if (rp->r_serial && !was_serial) {
 468      -                klwp_t *lwp = ttolwp(curthread);
 469      -
      467 +        if (rp->r_serial != NULL && !was_serial) {
 470  468                  /*
 471      -                 * If we're the recovery thread, then purge current attrs
 472      -                 * and bail out to avoid potential deadlock between another
 473      -                 * thread caching attrs (r_serial thread), recov thread,
 474      -                 * and an async writer thread.
      469 +                 * Purge current attrs and bail out to avoid potential deadlock
      470 +                 * between another thread caching attrs (r_serial thread), this
      471 +                 * thread, and a thread trying to read or write pages.
 475  472                   */
 476      -                if (recov) {
 477      -                        PURGE_ATTRCACHE4_LOCKED(rp);
 478      -                        mutex_exit(&rp->r_statelock);
 479      -                        return;
 480      -                }
 481      -
 482      -                if (lwp != NULL)
 483      -                        lwp->lwp_nostop++;
 484      -                while (rp->r_serial != NULL) {
 485      -                        if (!cv_wait_sig(&rp->r_cv, &rp->r_statelock)) {
 486      -                                mutex_exit(&rp->r_statelock);
 487      -                                if (lwp != NULL)
 488      -                                        lwp->lwp_nostop--;
 489      -                                return;
 490      -                        }
 491      -                }
 492      -                if (lwp != NULL)
 493      -                        lwp->lwp_nostop--;
      473 +                PURGE_ATTRCACHE4_LOCKED(rp);
      474 +                mutex_exit(&rp->r_statelock);
      475 +                return;
 494  476          }
 495  477  
 496  478          /*
 497  479           * If there is a page flush thread, the current thread needs to
 498  480           * bail out, to prevent a possible deadlock between the current
 499  481           * thread (which might be in a start_op/end_op region), the
 500  482           * recovery thread, and the page flush thread.  Expire the
 501  483           * attribute cache, so that any attributes the current thread was
 502  484           * going to set are not lost.
 503  485           */
↓ open down ↓ 2556 lines elided ↑ open up ↑
3060 3042          return (ret);
3061 3043  }
3062 3044  
3063 3045  void
3064 3046  nfs_free_mi4(mntinfo4_t *mi)
3065 3047  {
3066 3048          nfs4_open_owner_t       *foop;
3067 3049          nfs4_oo_hash_bucket_t   *bucketp;
3068 3050          nfs4_debug_msg_t        *msgp;
3069 3051          int i;
3070      -        servinfo4_t             *svp;
     3052 +        servinfo4_t             *svp;
3071 3053  
3072 3054          /*
3073 3055           * Code introduced here should be carefully evaluated to make
3074 3056           * sure none of the freed resources are accessed either directly
3075 3057           * or indirectly after freeing them. For eg: Introducing calls to
3076 3058           * NFS4_DEBUG that use mntinfo4_t structure member after freeing
3077 3059           * the structure members or other routines calling back into NFS
3078 3060           * accessing freed mntinfo4_t structure member.
3079 3061           */
3080 3062          mutex_enter(&mi->mi_lock);
↓ open down ↓ 1320 lines elided ↑ open up ↑
XXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXX