On 6/11/24 2:19 PM, Christoph Hellwig wrote:
> blkfront always had a robust negotiation protocol for detecting a write
> cache. Stop simply disabling cache flushes when they fail as that is
> a grave error.
>
> Signed-off-by: Christoph Hellwig <hch@xxxxxx>
Looks good to me but maybe mention that removal of xlvbd_flush() as well ?
And it feels like the "stop disabling cache flushes when they fail" part should
be a fix patch sent separately...
Anyway, for both parts, feel free to add:
Reviewed-by: Damien Le Moal <dlemoal@xxxxxxxxxx>
> ---
> drivers/block/xen-blkfront.c | 29 +++++++++--------------------
> 1 file changed, 9 insertions(+), 20 deletions(-)
>
> diff --git a/drivers/block/xen-blkfront.c b/drivers/block/xen-blkfront.c
> index 9b4ec3e4908cce..9794ac2d3299d1 100644
> --- a/drivers/block/xen-blkfront.c
> +++ b/drivers/block/xen-blkfront.c
> @@ -982,18 +982,6 @@ static const char *flush_info(struct blkfront_info *info)
> return "barrier or flush: disabled;";
> }
>
> -static void xlvbd_flush(struct blkfront_info *info)
> -{
> - blk_queue_write_cache(info->rq, info->feature_flush ? true : false,
> - info->feature_fua ? true : false);
> - pr_info("blkfront: %s: %s %s %s %s %s %s %s\n",
> - info->gd->disk_name, flush_info(info),
> - "persistent grants:", info->feature_persistent ?
> - "enabled;" : "disabled;", "indirect descriptors:",
> - info->max_indirect_segments ? "enabled;" : "disabled;",
> - "bounce buffer:", info->bounce ? "enabled" : "disabled;");
> -}
> -
> static int xen_translate_vdev(int vdevice, int *minor, unsigned int *offset)
> {
> int major;
> @@ -1162,7 +1150,15 @@ static int xlvbd_alloc_gendisk(blkif_sector_t capacity,
> info->sector_size = sector_size;
> info->physical_sector_size = physical_sector_size;
>
> - xlvbd_flush(info);
> + blk_queue_write_cache(info->rq, info->feature_flush ? true : false,
> + info->feature_fua ? true : false);
> +
> + pr_info("blkfront: %s: %s %s %s %s %s %s %s\n",
> + info->gd->disk_name, flush_info(info),
> + "persistent grants:", info->feature_persistent ?
> + "enabled;" : "disabled;", "indirect descriptors:",
> + info->max_indirect_segments ? "enabled;" : "disabled;",
> + "bounce buffer:", info->bounce ? "enabled" : "disabled;");
>
> if (info->vdisk_info & VDISK_READONLY)
> set_disk_ro(gd, 1);
> @@ -1622,13 +1618,6 @@ static irqreturn_t blkif_interrupt(int irq, void *dev_id)
> info->gd->disk_name, op_name(bret.operation));
> blkif_req(req)->error = BLK_STS_NOTSUPP;
> }
> - if (unlikely(blkif_req(req)->error)) {
> - if (blkif_req(req)->error == BLK_STS_NOTSUPP)
> - blkif_req(req)->error = BLK_STS_OK;
> - info->feature_fua = 0;
> - info->feature_flush = 0;
> - xlvbd_flush(info);
> - }
> fallthrough;
> case BLKIF_OP_READ:
> case BLKIF_OP_WRITE:
--
Damien Le Moal
Western Digital Research
