[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Xen-devel] [PATCH] xen/blkfront: remove redundant flush_op



On 12/08/2014 09:17 AM, Vitaly Kuznetsov wrote:
flush_op is unambiguously defined by feature_flush:
     REQ_FUA | REQ_FLUSH -> BLKIF_OP_WRITE_BARRIER
     REQ_FLUSH -> BLKIF_OP_FLUSH_DISKCACHE
     0 -> 0
and thus can be removed. This is just a cleanup.

The patch was suggested by Boris Ostrovsky.

Signed-off-by: Vitaly Kuznetsov <vkuznets@xxxxxxxxxx>
---
The patch is supposed to be applied after "xen/blkfront: improve protection
against issuing unsupported REQ_FUA".
---
  drivers/block/xen-blkfront.c | 24 ++++++++++++------------
  1 file changed, 12 insertions(+), 12 deletions(-)

diff --git a/drivers/block/xen-blkfront.c b/drivers/block/xen-blkfront.c
index 2e6c103..d1ee233 100644
--- a/drivers/block/xen-blkfront.c
+++ b/drivers/block/xen-blkfront.c
@@ -126,7 +126,6 @@ struct blkfront_info
        unsigned int persistent_gnts_c;
        unsigned long shadow_free;
        unsigned int feature_flush;
-       unsigned int flush_op;
        unsigned int feature_discard:1;
        unsigned int feature_secdiscard:1;
        unsigned int discard_granularity;
@@ -479,7 +478,14 @@ static int blkif_queue_request(struct request *req)
                                 * way.  (It's also a FLUSH+FUA, since it is
                                 * guaranteed ordered WRT previous writes.)
                                 */
-                               ring_req->operation = info->flush_op;
+                               if (unlikely(info->feature_flush & REQ_FUA))
+                                       ring_req->operation =
+                                               BLKIF_OP_WRITE_BARRIER;
+                               else if (likely(info->feature_flush))
+                                       ring_req->operation =
+                                               BLKIF_OP_FLUSH_DISKCACHE;

To better future-proof it against new flags maybe something like

    switch ( info->feature_flush & (REQ_FLUSH|REQ_FUA) ) {
           case REQ_FLUSH|REQ_FUA: ...
           case REQ_FLUSH: ...
           default: ...

or the if/else equivalent?


+                               else
+                                       ring_req->operation = 0;
                        }
                        ring_req->u.rw.nr_segments = nseg;
                }
@@ -691,8 +697,8 @@ static void xlvbd_flush(struct blkfront_info *info)
        blk_queue_flush(info->rq, info->feature_flush);
        printk(KERN_INFO "blkfront: %s: %s: %s %s %s %s %s\n",
               info->gd->disk_name,
-              info->flush_op == BLKIF_OP_WRITE_BARRIER ?
-               "barrier" : (info->flush_op == BLKIF_OP_FLUSH_DISKCACHE ?
+              info->feature_flush == (REQ_FLUSH | REQ_FUA) ?
+               "barrier" : (info->feature_flush == REQ_FLUSH ?

And something similar here?

Thanks.
-boris


                "flush diskcache" : "barrier or flush"),
               info->feature_flush ? "enabled;" : "disabled;",
               "persistent grants:",
@@ -1190,7 +1196,6 @@ static irqreturn_t blkif_interrupt(int irq, void *dev_id)
                                if (error == -EOPNOTSUPP)
                                        error = 0;
                                info->feature_flush = 0;
-                               info->flush_op = 0;
                                xlvbd_flush(info);
                        }
                        /* fall through */
@@ -1810,7 +1815,6 @@ static void blkfront_connect(struct blkfront_info *info)
                physical_sector_size = sector_size;
info->feature_flush = 0;
-       info->flush_op = 0;
err = xenbus_gather(XBT_NIL, info->xbdev->otherend,
                            "feature-barrier", "%d", &barrier,
@@ -1823,10 +1827,8 @@ static void blkfront_connect(struct blkfront_info *info)
         *
         * If there are barriers, then we use flush.
         */
-       if (!err && barrier) {
+       if (!err && barrier)
                info->feature_flush = REQ_FLUSH | REQ_FUA;
-               info->flush_op = BLKIF_OP_WRITE_BARRIER;
-       }
        /*
         * And if there is "feature-flush-cache" use that above
         * barriers.
@@ -1835,10 +1837,8 @@ static void blkfront_connect(struct blkfront_info *info)
                            "feature-flush-cache", "%d", &flush,
                            NULL);
- if (!err && flush) {
+       if (!err && flush)
                info->feature_flush = REQ_FLUSH;
-               info->flush_op = BLKIF_OP_FLUSH_DISKCACHE;
-       }
err = xenbus_gather(XBT_NIL, info->xbdev->otherend,
                            "feature-discard", "%d", &discard,


_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel


 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.