Skip site navigation (1)Skip section navigation (2) 
Date:      Tue, 12 Mar 2019 16:46:34 +0000 (UTC)
From:      Alexander Motin <mav@FreeBSD.org>
To:        src-committers@freebsd.org, svn-src-all@freebsd.org, svn-src-stable@freebsd.org, svn-src-stable-11@freebsd.org
Subject:   svn commit: r345069 - stable/11/sys/cddl/contrib/opensolaris/uts/common/fs/zfs
Message-ID:  <201903121646.x2CGkYbn049229@repo.freebsd.org>
next in thread | raw e-mail | index | archive | help 
Author: mav
Date: Tue Mar 12 16:46:34 2019
New Revision: 345069
URL: https://svnweb.freebsd.org/changeset/base/345069

Log:
  MFC r339299: Pull in a follow-on commit to resolve a deadlock in ZFS
  sequential resilver (r334844)
  
  MFV/ZoL: Fix deadlock in IO pipeline
  
  commit a76f3d0437e5e974f0f748f8735af3539443b388
  Author: Brian Behlendorf <behlendorf1@llnl.gov>
  Date:   Fri Mar 16 16:46:06 2018 -0700
  
      Fix deadlock in IO pipeline
  
      In vdev_queue_aggregate() the zio_execute() bypass should not be
      called under the vdev queue lock.  This can result in a deadlock
      as shown in the stack traces below.
  
      Drop the vdev queue lock then walk the parents of the aggregate IO
      to determine the list of component IOs to be bypassed.  This can
      be done safely without holding the io_lock since the new aggregate
      IO has not yet been returned and its parents cannot change.
  
      ---  THREAD 1 ---
      arc_read()
        zio_nowait()
          zio_vdev_io_start()
            vdev_queue_io() <--- mutex_enter(vq->vq_lock)
              vdev_queue_io_to_issue()
                vdev_queue_aggregate()
                  zio_execute()
              vdev_queue_io_to_issue()
                vdev_queue_aggregate()
                  zio_execute()
                    zio_vdev_io_assess()
                      zio_wait_for_children() <- mutex_enter(zio->io_lock)
  
      --- THREAD 2 --- (inverse order)
      arc_read()
        zio_change_priority() <- mutex_enter(zio->zio_lock)
          vdev_queue_change_io_priority() <- mutex_enter(vq->vq_lock)
  
      Reviewed-by: Tom Caputi <tcaputi@datto.com>
      Reviewed-by: Don Brady <don.brady@delphix.com>
      Signed-off-by: Brian Behlendorf <behlendorf1@llnl.gov>

Modified:
  stable/11/sys/cddl/contrib/opensolaris/uts/common/fs/zfs/vdev_queue.c

Modified: stable/11/sys/cddl/contrib/opensolaris/uts/common/fs/zfs/vdev_queue.c
==============================================================================
--- stable/11/sys/cddl/contrib/opensolaris/uts/common/fs/zfs/vdev_queue.c	Tue Mar 12 16:41:17 2019	(r345068)
+++ stable/11/sys/cddl/contrib/opensolaris/uts/common/fs/zfs/vdev_queue.c	Tue Mar 12 16:46:34 2019	(r345069)
@@ -666,6 +666,7 @@ static zio_t *
 vdev_queue_aggregate(vdev_queue_t *vq, zio_t *zio)
 {
 	zio_t *first, *last, *aio, *dio, *mandatory, *nio;
+	zio_link_t *zl = NULL;
 	uint64_t maxgap = 0;
 	uint64_t size;
 	boolean_t stretch;
@@ -809,9 +810,18 @@ vdev_queue_aggregate(vdev_queue_t *vq, zio_t *zio)
 
 		zio_add_child(dio, aio);
 		vdev_queue_io_remove(vq, dio);
+	} while (dio != last);
+
+	/*
+	 * We need to drop the vdev queue's lock to avoid a deadlock that we
+	 * could encounter since this I/O will complete immediately.
+	 */
+	mutex_exit(&vq->vq_lock);
+	while ((dio = zio_walk_parents(aio, &zl)) != NULL) {
 		zio_vdev_io_bypass(dio);
 		zio_execute(dio);
-	} while (dio != last);
+	}
+	mutex_enter(&vq->vq_lock);
 
 	return (aio);
 }

Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?201903121646.x2CGkYbn049229>