linuxppc-dev.lists.ozlabs.org archive mirror
 help / color / mirror / Atom feed
From: Christoph Hellwig <hch@lst.de>
To: Jens Axboe <axboe@kernel.dk>
Cc: nvdimm@lists.linux.dev, "Michael S. Tsirkin" <mst@redhat.com>,
	"Jason Wang" <jasowang@redhat.com>,
	linux-nvme@lists.infradead.org, "Song Liu" <song@kernel.org>,
	linux-mtd@lists.infradead.org,
	"Vineeth Vijayan" <vneethv@linux.ibm.com>,
	"Alasdair Kergon" <agk@redhat.com>,
	drbd-dev@lists.linbit.com, linux-s390@vger.kernel.org,
	linux-scsi@vger.kernel.org, "Richard Weinberger" <richard@nod.at>,
	"Geert Uytterhoeven" <geert@linux-m68k.org>,
	"Yu Kuai" <yukuai3@huawei.com>,
	dm-devel@lists.linux.dev, linux-um@lists.infradead.org,
	"Mike Snitzer" <snitzer@kernel.org>,
	"Josef Bacik" <josef@toxicpanda.com>,
	nbd@other.debian.org, linux-raid@vger.kernel.org,
	linux-m68k@lists.linux-m68k.org,
	"Mikulas Patocka" <mpatocka@redhat.com>,
	xen-devel@lists.xenproject.org, ceph-devel@vger.kernel.org,
	"Ming Lei" <ming.lei@redhat.com>,
	linux-bcache@vger.kernel.org, linux-block@vger.kernel.org,
	"Martin K. Petersen" <martin.petersen@oracle.com>,
	linux-mmc@vger.kernel.org,
	"Philipp Reisner" <philipp.reisner@linbit.com>,
	"Christoph Böhmwalder" <christoph.boehmwalder@linbit.com>,
	virtualization@lists.linux.dev,
	"Lars Ellenberg" <lars.ellenberg@linbit.com>,
	linuxppc-dev@lists.ozlabs.org,
	"Roger Pau Monné" <roger.pau@citrix.com>
Subject: [PATCH 01/26] xen-blkfront: don't disable cache flushes when they fail
Date: Mon, 17 Jun 2024 08:04:28 +0200	[thread overview]
Message-ID: <20240617060532.127975-2-hch@lst.de> (raw)
In-Reply-To: <20240617060532.127975-1-hch@lst.de>

blkfront always had a robust negotiation protocol for detecting a write
cache.  Stop simply disabling cache flushes in the block layer as the
flags handling is moving to the atomic queue limits API that needs
user context to freeze the queue for that.  Instead handle the case
of the feature flags cleared inside of blkfront.  This removes old
debug code to check for such a mismatch which was previously impossible
to hit, including the check for passthrough requests that blkfront
never used to start with.

Signed-off-by: Christoph Hellwig <hch@lst.de>
---
 drivers/block/xen-blkfront.c | 44 +++++++++++++++++++-----------------
 1 file changed, 23 insertions(+), 21 deletions(-)

diff --git a/drivers/block/xen-blkfront.c b/drivers/block/xen-blkfront.c
index 9b4ec3e4908cce..851b03844edd13 100644
--- a/drivers/block/xen-blkfront.c
+++ b/drivers/block/xen-blkfront.c
@@ -788,6 +788,11 @@ static int blkif_queue_rw_req(struct request *req, struct blkfront_ring_info *ri
 			 * A barrier request a superset of FUA, so we can
 			 * implement it the same way.  (It's also a FLUSH+FUA,
 			 * since it is guaranteed ordered WRT previous writes.)
+			 *
+			 * Note that can end up here with a FUA write and the
+			 * flags cleared.  This happens when the flag was
+			 * run-time disabled after a failing I/O, and we'll
+			 * simplify submit it as a normal write.
 			 */
 			if (info->feature_flush && info->feature_fua)
 				ring_req->operation =
@@ -795,8 +800,6 @@ static int blkif_queue_rw_req(struct request *req, struct blkfront_ring_info *ri
 			else if (info->feature_flush)
 				ring_req->operation =
 					BLKIF_OP_FLUSH_DISKCACHE;
-			else
-				ring_req->operation = 0;
 		}
 		ring_req->u.rw.nr_segments = num_grant;
 		if (unlikely(require_extra_req)) {
@@ -887,16 +890,6 @@ static inline void flush_requests(struct blkfront_ring_info *rinfo)
 		notify_remote_via_irq(rinfo->irq);
 }
 
-static inline bool blkif_request_flush_invalid(struct request *req,
-					       struct blkfront_info *info)
-{
-	return (blk_rq_is_passthrough(req) ||
-		((req_op(req) == REQ_OP_FLUSH) &&
-		 !info->feature_flush) ||
-		((req->cmd_flags & REQ_FUA) &&
-		 !info->feature_fua));
-}
-
 static blk_status_t blkif_queue_rq(struct blk_mq_hw_ctx *hctx,
 			  const struct blk_mq_queue_data *qd)
 {
@@ -908,12 +901,22 @@ static blk_status_t blkif_queue_rq(struct blk_mq_hw_ctx *hctx,
 	rinfo = get_rinfo(info, qid);
 	blk_mq_start_request(qd->rq);
 	spin_lock_irqsave(&rinfo->ring_lock, flags);
-	if (RING_FULL(&rinfo->ring))
-		goto out_busy;
 
-	if (blkif_request_flush_invalid(qd->rq, rinfo->dev_info))
-		goto out_err;
+	/*
+	 * Check if the backend actually supports flushes.
+	 *
+	 * While the block layer won't send us flushes if we don't claim to
+	 * support them, the Xen protocol allows the backend to revoke support
+	 * at any time.  That is of course a really bad idea and dangerous, but
+	 * has been allowed for 10+ years.  In that case we simply clear the
+	 * flags, and directly return here for an empty flush and ignore the
+	 * FUA flag later on.
+	 */
+	if (unlikely(req_op(qd->rq) == REQ_OP_FLUSH && !info->feature_flush))
+		goto complete;
 
+	if (RING_FULL(&rinfo->ring))
+		goto out_busy;
 	if (blkif_queue_request(qd->rq, rinfo))
 		goto out_busy;
 
@@ -921,14 +924,14 @@ static blk_status_t blkif_queue_rq(struct blk_mq_hw_ctx *hctx,
 	spin_unlock_irqrestore(&rinfo->ring_lock, flags);
 	return BLK_STS_OK;
 
-out_err:
-	spin_unlock_irqrestore(&rinfo->ring_lock, flags);
-	return BLK_STS_IOERR;
-
 out_busy:
 	blk_mq_stop_hw_queue(hctx);
 	spin_unlock_irqrestore(&rinfo->ring_lock, flags);
 	return BLK_STS_DEV_RESOURCE;
+complete:
+	spin_unlock_irqrestore(&rinfo->ring_lock, flags);
+	blk_mq_end_request(qd->rq, BLK_STS_OK);
+	return BLK_STS_OK;
 }
 
 static void blkif_complete_rq(struct request *rq)
@@ -1627,7 +1630,6 @@ static irqreturn_t blkif_interrupt(int irq, void *dev_id)
 					blkif_req(req)->error = BLK_STS_OK;
 				info->feature_fua = 0;
 				info->feature_flush = 0;
-				xlvbd_flush(info);
 			}
 			fallthrough;
 		case BLKIF_OP_READ:
-- 
2.43.0


  reply	other threads:[~2024-06-17  6:06 UTC|newest]

Thread overview: 63+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2024-06-17  6:04 move features flags into queue_limits v2 Christoph Hellwig
2024-06-17  6:04 ` Christoph Hellwig [this message]
2024-06-17  7:55   ` [PATCH 01/26] xen-blkfront: don't disable cache flushes when they fail Roger Pau Monné
2024-06-17 10:03   ` Hannes Reinecke
2024-06-17  6:04 ` [PATCH 02/26] sd: remove sd_is_zoned Christoph Hellwig
2024-06-18  1:39   ` Chaitanya Kulkarni
2024-06-17  6:04 ` [PATCH 03/26] sd: move zone limits setup out of sd_read_block_characteristics Christoph Hellwig
2024-06-17  6:12   ` Damien Le Moal
2024-06-17 10:04   ` Hannes Reinecke
2024-06-17  6:04 ` [PATCH 04/26] loop: stop using loop_reconfigure_limits in __loop_clr_fd Christoph Hellwig
2024-06-17  6:04 ` [PATCH 05/26] loop: always update discard settings in loop_reconfigure_limits Christoph Hellwig
2024-06-17  6:04 ` [PATCH 06/26] loop: regularize upgrading the block size for direct I/O Christoph Hellwig
2024-06-17  6:13   ` Damien Le Moal
2024-06-17  6:04 ` [PATCH 07/26] loop: also use the default block size from an underlying block device Christoph Hellwig
2024-06-17  6:14   ` Damien Le Moal
2024-06-17  6:04 ` [PATCH 08/26] loop: fold loop_update_rotational into loop_reconfigure_limits Christoph Hellwig
2024-06-17  6:04 ` [PATCH 09/26] virtio_blk: remove virtblk_update_cache_mode Christoph Hellwig
2024-06-18  1:39   ` Chaitanya Kulkarni
2024-06-17  6:04 ` [PATCH 10/26] nbd: move setting the cache control flags to __nbd_set_size Christoph Hellwig
2024-06-17  6:04 ` [PATCH 11/26] block: freeze the queue in queue_attr_store Christoph Hellwig
2024-06-18  1:41   ` Chaitanya Kulkarni
2024-06-17  6:04 ` [PATCH 12/26] block: remove blk_flush_policy Christoph Hellwig
2024-06-18  1:42   ` Chaitanya Kulkarni
2024-06-17  6:04 ` [PATCH 13/26] block: move cache control settings out of queue->flags Christoph Hellwig
2024-06-17  6:23   ` Damien Le Moal
2024-06-17 10:36   ` Hannes Reinecke
2024-06-17  6:04 ` [PATCH 14/26] block: move the nonrot flag to queue_limits Christoph Hellwig
2024-06-17 10:36   ` Hannes Reinecke
2024-06-24 17:08   ` Keith Busch
2024-06-24 17:24     ` Christoph Hellwig
2024-07-04 11:11     ` Simon Fernandez
2024-07-25 11:35   ` Wouter Verhelst
2024-07-25 13:00     ` Christoph Hellwig
2024-06-17  6:04 ` [PATCH 15/26] block: move the add_random " Christoph Hellwig
2024-06-17 10:38   ` Hannes Reinecke
2024-06-17  6:04 ` [PATCH 16/26] block: move the io_stat flag setting " Christoph Hellwig
2024-06-17  6:25   ` Damien Le Moal
2024-06-17 10:38   ` Hannes Reinecke
2024-06-17  6:04 ` [PATCH 17/26] block: move the stable_writes flag " Christoph Hellwig
2024-06-17 10:40   ` Hannes Reinecke
2024-06-17  6:04 ` [PATCH 18/26] block: move the synchronous " Christoph Hellwig
2024-06-17 10:40   ` Hannes Reinecke
2024-06-17  6:04 ` [PATCH 19/26] block: move the nowait " Christoph Hellwig
2024-06-17  6:26   ` Damien Le Moal
2024-06-17 10:41   ` Hannes Reinecke
2024-06-17  6:04 ` [PATCH 20/26] block: move the dax " Christoph Hellwig
2024-06-17 10:42   ` Hannes Reinecke
2024-06-17  6:04 ` [PATCH 21/26] block: move the poll " Christoph Hellwig
2024-06-17 10:42   ` Hannes Reinecke
2024-06-17  6:04 ` [PATCH 22/26] block: move the zoned flag into the features field Christoph Hellwig
2024-06-17 10:43   ` Hannes Reinecke
2024-06-17  6:04 ` [PATCH 23/26] block: move the zone_resetall flag to queue_limits Christoph Hellwig
2024-06-17 10:43   ` Hannes Reinecke
2024-06-17  6:04 ` [PATCH 24/26] block: move the pci_p2pdma " Christoph Hellwig
2024-06-17 10:44   ` Hannes Reinecke
2024-06-17  6:04 ` [PATCH 25/26] block: move the skip_tagset_quiesce " Christoph Hellwig
2024-06-17 10:45   ` Hannes Reinecke
2024-06-17  6:04 ` [PATCH 26/26] block: move the bounce flag into the features field Christoph Hellwig
2024-06-17 10:45   ` Hannes Reinecke
2024-06-17 14:35   ` Keith Busch
2024-06-19 14:18 ` move features flags into queue_limits v2 Jens Axboe
2024-06-19 14:21   ` Jens Axboe
2024-06-19 14:23     ` Christoph Hellwig

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20240617060532.127975-2-hch@lst.de \
    --to=hch@lst.de \
    --cc=agk@redhat.com \
    --cc=axboe@kernel.dk \
    --cc=ceph-devel@vger.kernel.org \
    --cc=christoph.boehmwalder@linbit.com \
    --cc=dm-devel@lists.linux.dev \
    --cc=drbd-dev@lists.linbit.com \
    --cc=geert@linux-m68k.org \
    --cc=jasowang@redhat.com \
    --cc=josef@toxicpanda.com \
    --cc=lars.ellenberg@linbit.com \
    --cc=linux-bcache@vger.kernel.org \
    --cc=linux-block@vger.kernel.org \
    --cc=linux-m68k@lists.linux-m68k.org \
    --cc=linux-mmc@vger.kernel.org \
    --cc=linux-mtd@lists.infradead.org \
    --cc=linux-nvme@lists.infradead.org \
    --cc=linux-raid@vger.kernel.org \
    --cc=linux-s390@vger.kernel.org \
    --cc=linux-scsi@vger.kernel.org \
    --cc=linux-um@lists.infradead.org \
    --cc=linuxppc-dev@lists.ozlabs.org \
    --cc=martin.petersen@oracle.com \
    --cc=ming.lei@redhat.com \
    --cc=mpatocka@redhat.com \
    --cc=mst@redhat.com \
    --cc=nbd@other.debian.org \
    --cc=nvdimm@lists.linux.dev \
    --cc=philipp.reisner@linbit.com \
    --cc=richard@nod.at \
    --cc=roger.pau@citrix.com \
    --cc=snitzer@kernel.org \
    --cc=song@kernel.org \
    --cc=virtualization@lists.linux.dev \
    --cc=vneethv@linux.ibm.com \
    --cc=xen-devel@lists.xenproject.org \
    --cc=yukuai3@huawei.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).