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

2016-04-17 Thread Juergen Gross 
On 15/04/16 21:16, mchri...@redhat.com wrote:
> From: Mike Christie 
> 
> 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.
> 
> v7:
> - Fix feature_flush/fua use.
> 
> 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 
> Reviewed-by: Hannes Reinecke 
> Cc: Juergen Gross 

Acked-by: Juergen Gross 


Juergen
--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

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

2016-04-15 Thread mchristi 
From: Mike Christie 

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.

v7:
- Fix feature_flush/fua use.

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 
Reviewed-by: Hannes Reinecke 
Cc: Juergen Gross 

---
 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
@@ -196,6 +196,7 @@ struct blkfront_info
unsigned int nr_ring_pages;
struct request_queue *rq;
unsigned int feature_flush;
+   unsigned int feature_fua;
unsigned int feature_discard:1;
unsigned int feature_secdiscard:1;
unsigned int discard_granularity;
@@ -763,19 +764,14 @@ static int blkif_queue_rw_req(struct request *req, struct 
blkfront_ring_info *ri
 * implement it the same way.  (It's also a FLUSH+FUA,
 * since it is guaranteed ordered WRT previous writes.)
 */
-   switch (info->feature_flush &
-   ((REQ_FLUSH|REQ_FUA))) {
-   case REQ_FLUSH|REQ_FUA:
+   if (info->feature_flush && info->feature_fua)
ring_req->operation =
BLKIF_OP_WRITE_BARRIER;
-   break;
-   case REQ_FLUSH:
+   else if (info->feature_flush)
ring_req->operation =
BLKIF_OP_FLUSH_DISKCACHE;
-   break;
-   default:
+   else
ring_req->operation = 0;
-   }
}
ring_req->u.rw.nr_segments = num_grant;
if (unlikely(require_extra_req)) {
@@ -866,9 +862,9 @@ static inline bool blkif_request_flush_invalid(struct 
request *req,
 {
return ((req->cmd_type != REQ_TYPE_FS) ||
((req->op == REQ_OP_FLUSH) &&
-!(info->feature_flush & REQ_FLUSH)) ||
+!info->feature_flush) ||
((req->cmd_flags & REQ_FUA) &&
-!(info->feature_flush & REQ_FUA)));
+!info->feature_fua));
 }
 
 static int blkif_queue_rq(struct blk_mq_hw_ctx *hctx,
@@ -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_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_fua ? true : false);
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;");
@@ -1621,6 +1615,7 @@ static irqreturn_t blkif_interrupt(int irq, void *dev_id)
if (unlikely(error)) {
if (error == -EOPNOTSUPP)
error = 0;
+   info->feature_fua = 0;
info->feature_flush = 0;
xlvbd_flush(info);
}
@@ -2315,6 +2310,7 @@ static void blkfront_gather_backend_features(struct 
blkfront_info *info)
unsigned int indirect_segments;
 
info->feature_flush = 0;
+   info->feature_fua = 0;
 
err = xenbus_gather(XBT_NIL,

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

2016-04-15 Thread Mike Christie 
On 04/15/2016 05:50 AM, Juergen Gross wrote:
> On 15/04/16 12:40, mchri...@redhat.com wrote:
>> From: Mike Christie 
>>
>> 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 
>> Reviewed-by: Hannes Reinecke 
>> ---
>>  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?
> 

You are right. Will fix up and resend.

--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

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

2016-04-15 Thread Juergen Gross 
On 15/04/16 12:40, mchri...@redhat.com wrote:
> From: Mike Christie 
> 
> 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 
> Reviewed-by: Hannes Reinecke 
> ---
>  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

--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

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

2016-04-15 Thread mchristi 
From: Mike Christie 

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 
Reviewed-by: Hannes Reinecke 
---
 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
@@ -196,6 +196,7 @@ struct blkfront_info
unsigned int nr_ring_pages;
struct request_queue *rq;
unsigned int feature_flush;
+   unsigned int feature_fua;
unsigned int feature_discard:1;
unsigned int feature_secdiscard:1;
unsigned int discard_granularity;
@@ -763,19 +764,14 @@ static int blkif_queue_rw_req(struct request *req, struct 
blkfront_ring_info *ri
 * implement it the same way.  (It's also a FLUSH+FUA,
 * since it is guaranteed ordered WRT previous writes.)
 */
-   switch (info->feature_flush &
-   ((REQ_FLUSH|REQ_FUA))) {
-   case REQ_FLUSH|REQ_FUA:
+   if (info->feature_flush && info->feature_fua)
ring_req->operation =
BLKIF_OP_WRITE_BARRIER;
-   break;
-   case REQ_FLUSH:
+   else if (info->feature_flush)
ring_req->operation =
BLKIF_OP_FLUSH_DISKCACHE;
-   break;
-   default:
+   else
ring_req->operation = 0;
-   }
}
ring_req->u.rw.nr_segments = num_grant;
if (unlikely(require_extra_req)) {
@@ -866,9 +862,9 @@ static inline bool blkif_request_flush_invalid(struct 
request *req,
 {
return ((req->cmd_type != REQ_TYPE_FS) ||
((req->op == REQ_OP_FLUSH) &&
-!(info->feature_flush & REQ_FLUSH)) ||
+!info->feature_flush) ||
((req->cmd_flags & REQ_FUA) &&
-!(info->feature_flush & REQ_FUA)));
+!info->feature_fua));
 }
 
 static int blkif_queue_rq(struct blk_mq_hw_ctx *hctx,
@@ -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)
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);
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;");
@@ -1621,6 +1615,7 @@ static irqreturn_t blkif_interrupt(int irq, void *dev_id)
if (unlikely(error)) {
if (error == -EOPNOTSUPP)
error = 0;
+   info->feature_fua = 0;
info->feature_flush = 0;
xlvbd_flush(info);
}
@@ -2315,6 +2310,7 @@ static void blkfront_gather_backend_features(struct 
blkfront_info *info)
unsigned int indirect_segments;
 
info->feature_flush = 0;
+   info->feature_fua = 0;
 
err = xenbus_gather(XBT_NIL, info->xbdev->otherend,
"feature-barrier", "%d", ,
@@ -2327,8