Re: [PATCH 41/42] block: do not use REQ_FLUSH for tracking flush support

Juergen Gross
2016-04-15T10:50:50Z

On 15/04/16 12:40, mchristi@redhat.com wrote:
> From: Mike Christie <mchristi@redhat.com>
>
> The last patch added a REQ_OP_FLUSH for request_fn drivers
> and the next patch renames REQ_FLUSH to REQ_PREFLUSH which
> will be used by file systems and make_request_fn drivers so
> they can send a write/flush combo.
>
> This patch drops xen's use of REQ_FLUSH to track if it supports
> REQ_OP_FLUSH requests, so REQ_FLUSH can be deleted.
>
> v6:
> - Dropped parts of patch handled by Jens's QUEUE_FLAG_WC/FUA
> patches and modified patch to check feature_flush/fua bits.
>
> Signed-off-by: Mike Christie <mchristi@redhat.com>
> Reviewed-by: Hannes Reinecke <hare@suse.com>
> ---
> drivers/block/xen-blkfront.c | 47 ++++++++++++++++++++++----------------------
> 1 file changed, 24 insertions(+), 23 deletions(-)
>
> diff --git a/drivers/block/xen-blkfront.c b/drivers/block/xen-blkfront.c
> index f01691a..d6429e7 100644
> --- a/drivers/block/xen-blkfront.c
> +++ b/drivers/block/xen-blkfront.c

...

> @@ -985,24 +981,22 @@ static int xlvbd_init_blk_queue(struct gendisk *gd, u16 sector_size,
> return 0;
> }
>
> -static const char *flush_info(unsigned int feature_flush)
> +static const char *flush_info(struct blkfront_info *info)
> {
> - switch (feature_flush & ((REQ_FLUSH | REQ_FUA))) {
> - case REQ_FLUSH|REQ_FUA:
> + if (info->feature_flush && info->feature_fua)
> return "barrier: enabled;";
> - case REQ_FLUSH:
> + else if (info->feature_fua)

Shouldn't this test feature_flush?

> return "flush diskcache: enabled;";
> - default:
> + else
> return "barrier or flush: disabled;";
> - }
> }
>
> static void xlvbd_flush(struct blkfront_info *info)
> {
> - blk_queue_write_cache(info->rq, info->feature_flush & REQ_FLUSH,
> - info->feature_flush & REQ_FUA);
> + blk_queue_write_cache(info->rq, info->feature_flush ? true : false,
> + info->feature_flush ? true : false);

And here the second test should be feature_fua?

> pr_info("blkfront: %s: %s %s %s %s %s\n",
> - info->gd->disk_name, flush_info(info->feature_flush),
> + info->gd->disk_name, flush_info(info),
> "persistent grants:", info->feature_persistent ?
> "enabled;" : "disabled;", "indirect descriptors:",
> info->max_indirect_segments ? "enabled;" : "disabled;");

Juergen