Blob Blame History Raw
From: NeilBrown <neilb@suse.com>
Date: Wed, 21 Jun 2017 08:56:56 +1000
Subject: [PATCH] md: use a separate bio_set for synchronous IO.
Patch-mainline: v4.13
Git-commit: 5a85071c2cbcc7d8d8f764b33bf64c76e47d268d
References: bsc#1040351

md devices allocate a bio_set and use it for two
distinct purposes.
mddev->bio_set is used to clone bios as part of sending
upper level requests down to lower level devices,
and it is also use for synchronous IO such as superblock
and bitmap updates, for correcting read errors.

This multiple usage can lead to deadlocks.  It is likely
that cloned bios might be queued for write and to be
waiting for a metadata update before the write can be permitted.
If the cloning exhausted mddev->bio_set, the metadata update
may not be able to proceed.

This scenario has been seen during heavy testing, with lots of IO and
lots of memory pressure.

Address this by adding a new bio_set specifically for synchronous IO.
All synchronous IO goes directly to the underlying device and is not
queued at the md level, so request using entries from the new
mddev->sync_set will complete in a timely fashion.
Requests that use mddev->bio_set will sometimes need to wait
for synchronous IO, but will no longer risk deadlocking that iO.

Also: small simplification in mddev_put(): there is no need to
wait until the spinlock is released before calling bioset_free().

Signed-off-by: NeilBrown <neilb@suse.com>

---
 drivers/md/md.c |   27 ++++++++++++++++++++-------
 drivers/md/md.h |    3 +++
 2 files changed, 23 insertions(+), 7 deletions(-)

--- a/drivers/md/md.c
+++ b/drivers/md/md.c
@@ -203,6 +203,14 @@ struct bio *bio_alloc_mddev(gfp_t gfp_ma
 }
 EXPORT_SYMBOL_GPL(bio_alloc_mddev);
 
+static struct bio *md_bio_alloc_sync(struct mddev *mddev)
+{
+	if (!mddev->sync_set)
+		return bio_alloc(GFP_NOIO, 1);
+
+	return bio_alloc_bioset(GFP_NOIO, 1, mddev->sync_set);
+}
+
 /*
  * We have a system wide 'event count' that is incremented
  * on any 'interesting' event, and readers of /proc/mdstat
@@ -467,8 +475,6 @@ static void mddev_delayed_delete(struct
 
 static void mddev_put(struct mddev *mddev)
 {
-	struct bio_set *bs = NULL;
-
 	if (!atomic_dec_and_lock(&mddev->active, &all_mddevs_lock))
 		return;
 	if (!mddev->raid_disks && list_empty(&mddev->disks) &&
@@ -476,8 +482,12 @@ static void mddev_put(struct mddev *mdde
 		/* Array is not configured at all, and not held active,
 		 * so destroy it */
 		list_del_init(&mddev->all_mddevs);
-		bs = mddev->bio_set;
+		if (mddev->bio_set)
+			bioset_free(mddev->bio_set);
+		if (mddev->sync_set)
+			bioset_free(mddev->sync_set);
 		mddev->bio_set = NULL;
+		mddev->sync_set = NULL;
 		if (mddev->gendisk) {
 			/* We did a probe so need to clean up.  Call
 			 * queue_work inside the spinlock so that
@@ -490,8 +500,6 @@ static void mddev_put(struct mddev *mdde
 			kfree(mddev);
 	}
 	spin_unlock(&all_mddevs_lock);
-	if (bs)
-		bioset_free(bs);
 }
 
 static void md_safemode_timeout(unsigned long data);
@@ -756,7 +764,7 @@ void md_super_write(struct mddev *mddev,
 	if (test_bit(Faulty, &rdev->flags))
 		return;
 
-	bio = bio_alloc_mddev(GFP_NOIO, 1, mddev);
+	bio = md_bio_alloc_sync(mddev);
 
 	atomic_inc(&rdev->nr_pending);
 
@@ -788,7 +796,7 @@ int md_super_wait(struct mddev *mddev)
 int sync_page_io(struct md_rdev *rdev, sector_t sector, int size,
 		 struct page *page, int op, int op_flags, bool metadata_op)
 {
-	struct bio *bio = bio_alloc_mddev(GFP_NOIO, 1, rdev->mddev);
+	struct bio *bio = md_bio_alloc_sync(rdev->mddev);
 	int ret;
 
 	bio->bi_bdev = (metadata_op && rdev->meta_bdev) ?
@@ -5612,6 +5620,11 @@ static int do_md_run(struct mddev *mddev
 		bitmap_destroy(mddev);
 		goto out;
 	}
+	if (mddev->sync_set == NULL) {
+		mddev->sync_set = bioset_create(BIO_POOL_SIZE, 0, BIOSET_NEED_BVECS);
+		if (!mddev->sync_set)
+			return -ENOMEM;
+	}
 
 	if (mddev_is_clustered(mddev))
 		md_allow_write(mddev);
--- a/drivers/md/md.h
+++ b/drivers/md/md.h
@@ -444,6 +444,9 @@ struct mddev {
 	struct attribute_group		*to_remove;
 
 	struct bio_set			*bio_set;
+	struct bio_set			*sync_set; /* for sync operations like
+						   * metadata and bitmap writes
+						   */
 
 	/* Generic flush handling.
 	 * The last to finish preflush schedules a worker to submit