bcache: read retry improvements

5 files changed, 67 insertions, 60 deletions

diff --git a/drivers/md/bcache/bcache.h b/drivers/md/bcache/bcache.h
index be5e0d943729..b53bcd7ae2dc 100644
--- a/drivers/md/bcache/bcache.h
+++ b/drivers/md/bcache/bcache.h
@@ -721,9 +721,9 @@ struct cache_set {
 	struct work_struct	bio_submit_work;
 	spinlock_t		bio_submit_lock;
 
-	struct bio_list		read_race_list;
-	struct work_struct	read_race_work;
-	spinlock_t		read_race_lock;
+	struct bio_list		read_retry_list;
+	struct work_struct	read_retry_work;
+	spinlock_t		read_retry_lock;
 
 	/* FILESYSTEM */
 	atomic_long_t		nr_inodes;
diff --git a/drivers/md/bcache/io.c b/drivers/md/bcache/io.c
index 5b1f09226479..a9b61b8643cf 100644
--- a/drivers/md/bcache/io.c
+++ b/drivers/md/bcache/io.c
@@ -1374,16 +1374,6 @@ out_submit:
 
 /* Read */
 
-static void bch_read_requeue(struct cache_set *c, struct bio *bio)
-{
-	unsigned long flags;
-
-	spin_lock_irqsave(&c->read_race_lock, flags);
-	bio_list_add(&c->read_race_list, bio);
-	spin_unlock_irqrestore(&c->read_race_lock, flags);
-	queue_work(c->wq, &c->read_race_work);
-}
-
 static int bio_uncompress_lz4(struct cache_set *c,
 			      struct bio *dst, struct bvec_iter dst_iter,
 			      struct bio *src, struct bvec_iter src_iter,
@@ -1551,6 +1541,41 @@ static void bch_rbio_free(struct bch_read_bio *rbio)
 	bio_put(bio);
 }
 
+static void bch_read_bio_done(struct bch_read_bio *rbio)
+{
+	bio_endio(rbio->parent);
+	bch_rbio_free(rbio);
+}
+
+/*
+ * Decide if we want to retry the read - returns true if read is being retried,
+ * false if caller should pass error on up
+ */
+static void bch_read_error_maybe_retry(struct bch_read_bio *rbio, int error)
+{
+	struct cache_set *c = rbio->c;
+	unsigned long flags;
+
+	if ((error == -EINTR) &&
+	    (rbio->flags & BCH_READ_RETRY_IF_STALE)) {
+		atomic_long_inc(&rbio->c->cache_read_races);
+		goto retry;
+	}
+
+	if (error == -EIO) {
+		/* io error - do we have another replica? */
+	}
+
+	rbio->parent->bi_error = error;
+	bch_read_bio_done(rbio);
+	return;
+retry:
+	spin_lock_irqsave(&c->read_retry_lock, flags);
+	bio_list_add(&c->read_retry_list, &rbio->bio.bio);
+	spin_unlock_irqrestore(&c->read_retry_lock, flags);
+	queue_work(c->wq, &c->read_retry_work);
+}
+
 static void cache_promote_done(struct closure *cl)
 {
 	struct cache_promote_op *op =
@@ -1565,20 +1590,24 @@ static void __bch_read_endio(struct bch_read_bio *rbio)
 	int ret;
 
 	ret = bio_checksum_uncompress(rbio);
-	if (ret)
-		rbio->parent->bi_error = ret;
-	bio_endio(rbio->parent);
+	if (ret) {
+		bch_read_error_maybe_retry(rbio, ret);
+		return;
+	}
 
-	if (!ret && rbio->promote &&
+	if (rbio->promote &&
 	    !test_bit(CACHE_SET_RO, &rbio->c->flags) &&
 	    !test_bit(CACHE_SET_STOPPING, &rbio->c->flags)) {
 		struct closure *cl = &rbio->promote->cl;
 
+		bio_endio(rbio->parent);
+		rbio->parent = NULL;
+
 		closure_init(cl, &rbio->c->cl);
 		closure_call(&rbio->promote->iop.cl, bch_write, rbio->c->wq, cl);
 		closure_return_with_destructor(cl, cache_promote_done);
 	} else {
-		bch_rbio_free(rbio);
+		bch_read_bio_done(rbio);
 	}
 }
 
@@ -1604,21 +1633,20 @@ static void bch_read_endio(struct bio *bio)
 {
 	struct bch_read_bio *rbio =
 		container_of(bio, struct bch_read_bio, bio.bio);
-	bool stale = race_fault() ||
-		ptr_stale(rbio->bio.ca, &rbio->bio.ptr);
-	int error = bio->bi_error;
+	int stale = race_fault() ||
+		ptr_stale(rbio->bio.ca, &rbio->bio.ptr) ? -EINTR : 0;
+	int error = bio->bi_error ?: stale;
 
 	bch_account_bbio_completion(&rbio->bio);
 
-	cache_nonfatal_io_err_on(error, rbio->bio.ca, "data read");
+	cache_nonfatal_io_err_on(bio->bi_error, rbio->bio.ca, "data read");
 
 	percpu_ref_put(&rbio->bio.ca->ref);
 
-	if (error)
-		goto out;
-
-	if (stale)
-		goto stale;
+	if (error) {
+		bch_read_error_maybe_retry(rbio, error);
+		return;
+	}
 
 	if (rbio->compression_type != BCH_COMPRESSION_NONE) {
 		struct bio_decompress_worker *d;
@@ -1631,28 +1659,6 @@ static void bch_read_endio(struct bio *bio)
 	} else {
 		__bch_read_endio(rbio);
 	}
-
-	return;
-stale:
-	if (rbio->promote)
-		kfree(rbio->promote);
-	rbio->promote = NULL;
-
-	/* Raced with the bucket being reused and invalidated: */
-	if (rbio->flags & BCH_READ_RETRY_IF_STALE) {
-		atomic_long_inc(&rbio->c->cache_read_races);
-		bch_read_requeue(rbio->c, bio);
-		return;
-	}
-
-	error = -EINTR;
-out:
-	if (rbio->promote)
-		kfree(rbio->promote);
-	if (error)
-		rbio->parent->bi_error = error;
-	bio_endio(rbio->parent);
-	bio_put(bio);
 }
 
 void bch_read_extent_iter(struct cache_set *c, struct bio *orig,
@@ -1854,18 +1860,18 @@ static void bch_read_retry(struct cache_set *c, struct bch_read_bio *rbio)
 	bch_read_iter(c, parent, iter, inode);
 }
 
-void bch_read_race_work(struct work_struct *work)
+void bch_read_retry_work(struct work_struct *work)
 {
 	struct cache_set *c = container_of(work, struct cache_set,
-					   read_race_work);
+					   read_retry_work);
 	struct bch_read_bio *rbio;
 	struct bio *bio;
 	unsigned long flags;
 
 	while (1) {
-		spin_lock_irqsave(&c->read_race_lock, flags);
-		bio = bio_list_pop(&c->read_race_list);
-		spin_unlock_irqrestore(&c->read_race_lock, flags);
+		spin_lock_irqsave(&c->read_retry_lock, flags);
+		bio = bio_list_pop(&c->read_retry_list);
+		spin_unlock_irqrestore(&c->read_retry_lock, flags);
 
 		if (!bio)
 			break;
diff --git a/drivers/md/bcache/io.h b/drivers/md/bcache/io.h
index 147ccd969586..8be6a2a9eed3 100644
--- a/drivers/md/bcache/io.h
+++ b/drivers/md/bcache/io.h
@@ -79,7 +79,7 @@ void __cache_promote(struct cache_set *, struct bbio *,
 		     struct bkey_s_c, struct bkey_s_c, unsigned);
 bool cache_promote(struct cache_set *, struct bbio *, struct bkey_s_c);
 
-void bch_read_race_work(struct work_struct *);
+void bch_read_retry_work(struct work_struct *);
 void bch_wake_delayed_writes(unsigned long data);
 
 void bch_bio_decompress_work(struct work_struct *);
diff --git a/drivers/md/bcache/io_types.h b/drivers/md/bcache/io_types.h
index f0a58401295a..22f3b7d3a5e6 100644
--- a/drivers/md/bcache/io_types.h
+++ b/drivers/md/bcache/io_types.h
@@ -40,10 +40,11 @@ struct bch_read_bio {
 	unsigned		flags;
 
 	/* fields align with bch_extent_crc64 */
-	u64			bounce:3,
+	u64			bounce:1,
+				pad:2,
+				offset:17,
 				compressed_size:18,
 				uncompressed_size:18,
-				offset:17,
 				csum_type:4,
 				compression_type:4;
 	u64			csum;
diff --git a/drivers/md/bcache/super.c b/drivers/md/bcache/super.c
index c44f44c44712..e7a929cb0795 100644
--- a/drivers/md/bcache/super.c
+++ b/drivers/md/bcache/super.c
@@ -1073,9 +1073,9 @@ static struct cache_set *bch_cache_set_alloc(struct cache_sb *sb,
 	mutex_init(&c->bio_bounce_pages_lock);
 	INIT_WORK(&c->bio_submit_work, bch_bio_submit_work);
 	spin_lock_init(&c->bio_submit_lock);
-	bio_list_init(&c->read_race_list);
-	spin_lock_init(&c->read_race_lock);
-	INIT_WORK(&c->read_race_work, bch_read_race_work);
+	bio_list_init(&c->read_retry_list);
+	spin_lock_init(&c->read_retry_lock);
+	INIT_WORK(&c->read_retry_work, bch_read_retry_work);
 
 	seqcount_init(&c->gc_pos_lock);