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

Re: [Xen-devel] [PATCH v2] xen-blkback: define pr_fmt macro to avoid the duplication of DRV_PFX





On 2015/4/1 23:04, Tao Chen wrote:
Define pr_fmt macro with {xen-blkback: } prefix, then remove all use
of DRV_PFX in the pr sentences. Replace all DPRINTK with pr sentences,
and get rid of DPRINTK macro. It will simplify the code.

And if the pr sentences miss a \n, add it in the end. If the DPRINTK
sentences have redundant \n, remove it. It will format the code.


I'm sorry, I made a mistake in this patch log. It must be changed as below.

And if the pr sentences miss a \n, add it in the end. It will format the code.

I apologize for the disturb of wrong patch log. Very looking forward
to your reply.

These all make the readability of the code become better.

Signed-off-by: Tao Chen <boby.chen@xxxxxxxxxx>
---
v2:
   - Replace all DPRINTK with pr sentences, and get rid of DPRINTK macro.
---
  drivers/block/xen-blkback/blkback.c | 62 ++++++++++++++++++-------------------
  drivers/block/xen-blkback/common.h  |  6 ----
  drivers/block/xen-blkback/xenbus.c  | 31 +++++++++++--------
  3 files changed, 49 insertions(+), 50 deletions(-)

diff --git a/drivers/block/xen-blkback/blkback.c 
b/drivers/block/xen-blkback/blkback.c
index 2a04d34..bd2b3bb 100644
--- a/drivers/block/xen-blkback/blkback.c
+++ b/drivers/block/xen-blkback/blkback.c
@@ -34,6 +34,8 @@
   * IN THE SOFTWARE.
   */

+#define pr_fmt(fmt) "xen-blkback: " fmt
+
  #include <linux/spinlock.h>
  #include <linux/kthread.h>
  #include <linux/list.h>
@@ -211,7 +213,7 @@ static int add_persistent_gnt(struct xen_blkif *blkif,
                else if (persistent_gnt->gnt > this->gnt)
                        new = &((*new)->rb_right);
                else {
-                       pr_alert_ratelimited(DRV_PFX " trying to add a gref that's 
already in the tree\n");
+                       pr_alert_ratelimited("trying to add a gref that's already in 
the tree\n");
                        return -EINVAL;
                }
        }
@@ -242,7 +244,7 @@ static struct persistent_gnt *get_persistent_gnt(struct 
xen_blkif *blkif,
                        node = node->rb_right;
                else {
                        if(test_bit(PERSISTENT_GNT_ACTIVE, data->flags)) {
-                               pr_alert_ratelimited(DRV_PFX " requesting a grant 
already in use\n");
+                               pr_alert_ratelimited("requesting a grant already in 
use\n");
                                return NULL;
                        }
                        set_bit(PERSISTENT_GNT_ACTIVE, data->flags);
@@ -257,7 +259,7 @@ static void put_persistent_gnt(struct xen_blkif *blkif,
                                 struct persistent_gnt *persistent_gnt)
  {
        if(!test_bit(PERSISTENT_GNT_ACTIVE, persistent_gnt->flags))
-                 pr_alert_ratelimited(DRV_PFX " freeing a grant already 
unused");
+               pr_alert_ratelimited("freeing a grant already unused\n");
        set_bit(PERSISTENT_GNT_WAS_ACTIVE, persistent_gnt->flags);
        clear_bit(PERSISTENT_GNT_ACTIVE, persistent_gnt->flags);
        atomic_dec(&blkif->persistent_gnt_in_use);
@@ -374,7 +376,7 @@ static void purge_persistent_gnt(struct xen_blkif *blkif)
        }

        if (work_pending(&blkif->persistent_purge_work)) {
-               pr_alert_ratelimited(DRV_PFX "Scheduled work from previous purge is 
still pending, cannot purge list\n");
+               pr_alert_ratelimited("Scheduled work from previous purge is still 
pending, cannot purge list\n");
                return;
        }

@@ -396,7 +398,7 @@ static void purge_persistent_gnt(struct xen_blkif *blkif)

        total = num_clean;

-       pr_debug(DRV_PFX "Going to purge %u persistent grants\n", num_clean);
+       pr_debug("Going to purge %u persistent grants\n", num_clean);

        BUG_ON(!list_empty(&blkif->persistent_purge_list));
        root = &blkif->persistent_gnts;
@@ -428,13 +430,13 @@ purge_list:
         * with the requested num
         */
        if (!scan_used && !clean_used) {
-               pr_debug(DRV_PFX "Still missing %u purged frames\n", num_clean);
+               pr_debug("Still missing %u purged frames\n", num_clean);
                scan_used = true;
                goto purge_list;
        }
  finished:
        if (!clean_used) {
-               pr_debug(DRV_PFX "Finished scanning for grants to clean, removing 
used flag\n");
+               pr_debug("Finished scanning for grants to clean, removing used 
flag\n");
                clean_used = true;
                goto purge_list;
        }
@@ -444,7 +446,7 @@ finished:

        /* We can defer this work */
        schedule_work(&blkif->persistent_purge_work);
-       pr_debug(DRV_PFX "Purged %u/%u\n", (total - num_clean), total);
+       pr_debug("Purged %u/%u\n", (total - num_clean), total);
        return;
  }

@@ -520,20 +522,20 @@ static void xen_vbd_resize(struct xen_blkif *blkif)
        struct xenbus_device *dev = xen_blkbk_xenbus(blkif->be);
        unsigned long long new_size = vbd_sz(vbd);

-       pr_info(DRV_PFX "VBD Resize: Domid: %d, Device: (%d, %d)\n",
+       pr_info("VBD Resize: Domid: %d, Device: (%d, %d)\n",
                blkif->domid, MAJOR(vbd->pdevice), MINOR(vbd->pdevice));
-       pr_info(DRV_PFX "VBD Resize: new size %llu\n", new_size);
+       pr_info("VBD Resize: new size %llu\n", new_size);
        vbd->size = new_size;
  again:
        err = xenbus_transaction_start(&xbt);
        if (err) {
-               pr_warn(DRV_PFX "Error starting transaction");
+               pr_warn("Error starting transaction\n");
                return;
        }
        err = xenbus_printf(xbt, dev->nodename, "sectors", "%llu",
                            (unsigned long long)vbd_sz(vbd));
        if (err) {
-               pr_warn(DRV_PFX "Error writing new size");
+               pr_warn("Error writing new size\n");
                goto abort;
        }
        /*
@@ -543,7 +545,7 @@ again:
         */
        err = xenbus_printf(xbt, dev->nodename, "state", "%d", dev->state);
        if (err) {
-               pr_warn(DRV_PFX "Error writing the state");
+               pr_warn("Error writing the state\n");
                goto abort;
        }

@@ -551,7 +553,7 @@ again:
        if (err == -EAGAIN)
                goto again;
        if (err)
-               pr_warn(DRV_PFX "Error ending transaction");
+               pr_warn("Error ending transaction\n");
        return;
  abort:
        xenbus_transaction_end(xbt, 1);
@@ -578,7 +580,7 @@ irqreturn_t xen_blkif_be_int(int irq, void *dev_id)

  static void print_stats(struct xen_blkif *blkif)
  {
-       pr_info("xen-blkback (%s): oo %3llu  |  rd %4llu  |  wr %4llu  |  f 
%4llu"
+       pr_info("(%s): oo %3llu  |  rd %4llu  |  wr %4llu  |  f %4llu"
                 "  |  ds %4llu | pg: %4u/%4d\n",
                 current->comm, blkif->st_oo_req,
                 blkif->st_rd_req, blkif->st_wr_req,
@@ -855,7 +857,7 @@ again:
                        /* This is a newly mapped grant */
                        BUG_ON(new_map_idx >= segs_to_map);
                        if (unlikely(map[new_map_idx].status != 0)) {
-                               pr_debug(DRV_PFX "invalid buffer -- could not remap 
it\n");
+                               pr_debug("invalid buffer -- could not remap 
it\n");
                                put_free_pages(blkif, &pages[seg_idx]->page, 1);
                                pages[seg_idx]->handle = BLKBACK_INVALID_HANDLE;
                                ret |= 1;
@@ -891,14 +893,14 @@ again:
                                goto next;
                        }
                        pages[seg_idx]->persistent_gnt = persistent_gnt;
-                       pr_debug(DRV_PFX " grant %u added to the tree of persistent 
grants, using %u/%u\n",
+                       pr_debug("grant %u added to the tree of persistent grants, 
using %u/%u\n",
                                 persistent_gnt->gnt, blkif->persistent_gnt_c,
                                 xen_blkif_max_pgrants);
                        goto next;
                }
                if (use_persistent_gnts && !blkif->vbd.overflow_max_grants) {
                        blkif->vbd.overflow_max_grants = 1;
-                       pr_debug(DRV_PFX " domain %u, device %#x is using maximum 
number of persistent grants\n",
+                       pr_debug("domain %u, device %#x is using maximum number of 
persistent grants\n",
                                 blkif->domid, blkif->vbd.handle);
                }
                /*
@@ -916,7 +918,7 @@ next:
        return ret;

  out_of_memory:
-       pr_alert(DRV_PFX "%s: out of memory\n", __func__);
+       pr_alert("%s: out of memory\n", __func__);
        put_free_pages(blkif, pages_to_gnt, segs_to_map);
        return -ENOMEM;
  }
@@ -996,7 +998,7 @@ static int dispatch_discard_io(struct xen_blkif *blkif,

        err = xen_vbd_translate(&preq, blkif, WRITE);
        if (err) {
-               pr_warn(DRV_PFX "access denied: DISCARD [%llu->%llu] on 
dev=%04x\n",
+               pr_warn("access denied: DISCARD [%llu->%llu] on dev=%04x\n",
                        preq.sector_number,
                        preq.sector_number + preq.nr_sects, blkif->vbd.pdevice);
                goto fail_response;
@@ -1012,7 +1014,7 @@ static int dispatch_discard_io(struct xen_blkif *blkif,
                                   GFP_KERNEL, secure);
  fail_response:
        if (err == -EOPNOTSUPP) {
-               pr_debug(DRV_PFX "discard op failed, not supported\n");
+               pr_debug("discard op failed, not supported\n");
                status = BLKIF_RSP_EOPNOTSUPP;
        } else if (err)
                status = BLKIF_RSP_ERROR;
@@ -1056,16 +1058,16 @@ static void __end_block_io_op(struct pending_req 
*pending_req, int error)
        /* An error fails the entire request. */
        if ((pending_req->operation == BLKIF_OP_FLUSH_DISKCACHE) &&
            (error == -EOPNOTSUPP)) {
-               pr_debug(DRV_PFX "flush diskcache op failed, not supported\n");
+               pr_debug("flush diskcache op failed, not supported\n");
                xen_blkbk_flush_diskcache(XBT_NIL, pending_req->blkif->be, 0);
                pending_req->status = BLKIF_RSP_EOPNOTSUPP;
        } else if ((pending_req->operation == BLKIF_OP_WRITE_BARRIER) &&
                    (error == -EOPNOTSUPP)) {
-               pr_debug(DRV_PFX "write barrier op failed, not supported\n");
+               pr_debug("write barrier op failed, not supported\n");
                xen_blkbk_barrier(XBT_NIL, pending_req->blkif->be, 0);
                pending_req->status = BLKIF_RSP_EOPNOTSUPP;
        } else if (error) {
-               pr_debug(DRV_PFX "Buffer not up-to-date at end of operation,"
+               pr_debug("Buffer not up-to-date at end of operation,"
                         " error=%d\n", error);
                pending_req->status = BLKIF_RSP_ERROR;
        }
@@ -1110,7 +1112,7 @@ __do_block_io_op(struct xen_blkif *blkif)

        if (RING_REQUEST_PROD_OVERFLOW(&blk_rings->common, rp)) {
                rc = blk_rings->common.rsp_prod_pvt;
-               pr_warn(DRV_PFX "Frontend provided bogus ring requests (%d - %d = 
%d). Halting ring processing on dev=%04x\n",
+               pr_warn("Frontend provided bogus ring requests (%d - %d = %d). 
Halting ring processing on dev=%04x\n",
                        rp, rc, rp - rc, blkif->vbd.pdevice);
                return -EACCES;
        }
@@ -1217,8 +1219,7 @@ static int dispatch_rw_block_io(struct xen_blkif *blkif,
        if ((req->operation == BLKIF_OP_INDIRECT) &&
            (req_operation != BLKIF_OP_READ) &&
            (req_operation != BLKIF_OP_WRITE)) {
-               pr_debug(DRV_PFX "Invalid indirect operation (%u)\n",
-                        req_operation);
+               pr_debug("Invalid indirect operation (%u)\n", req_operation);
                goto fail_response;
        }

@@ -1252,8 +1253,7 @@ static int dispatch_rw_block_io(struct xen_blkif *blkif,
                     (nseg > BLKIF_MAX_SEGMENTS_PER_REQUEST)) ||
            unlikely((req->operation == BLKIF_OP_INDIRECT) &&
                     (nseg > MAX_INDIRECT_SEGMENTS))) {
-               pr_debug(DRV_PFX "Bad number of segments in request (%d)\n",
-                        nseg);
+               pr_debug("Bad number of segments in request (%d)\n", nseg);
                /* Haven't submitted any bio's yet. */
                goto fail_response;
        }
@@ -1288,7 +1288,7 @@ static int dispatch_rw_block_io(struct xen_blkif *blkif,
        }

        if (xen_vbd_translate(&preq, blkif, operation) != 0) {
-               pr_debug(DRV_PFX "access denied: %s of [%llu,%llu] on 
dev=%04x\n",
+               pr_debug("access denied: %s of [%llu,%llu] on dev=%04x\n",
                         operation == READ ? "read" : "write",
                         preq.sector_number,
                         preq.sector_number + preq.nr_sects,
@@ -1303,7 +1303,7 @@ static int dispatch_rw_block_io(struct xen_blkif *blkif,
        for (i = 0; i < nseg; i++) {
                if (((int)preq.sector_number|(int)seg[i].nsec) &
                    ((bdev_logical_block_size(preq.bdev) >> 9) - 1)) {
-                       pr_debug(DRV_PFX "Misaligned I/O request from domain 
%d",
+                       pr_debug("Misaligned I/O request from domain %d\n",
                                 blkif->domid);
                        goto fail_response;
                }
diff --git a/drivers/block/xen-blkback/common.h 
b/drivers/block/xen-blkback/common.h
index 375d288..f620b5d 100644
--- a/drivers/block/xen-blkback/common.h
+++ b/drivers/block/xen-blkback/common.h
@@ -44,12 +44,6 @@
  #include <xen/interface/io/blkif.h>
  #include <xen/interface/io/protocols.h>

-#define DRV_PFX "xen-blkback:"
-#define DPRINTK(fmt, args...)                          \
-       pr_debug(DRV_PFX "(%s:%d) " fmt ".\n",              \
-                __func__, __LINE__, ##args)
-
-
  /*
   * This is the maximum number of segments that would be allowed in indirect
   * requests. This value will also be passed to the frontend.
diff --git a/drivers/block/xen-blkback/xenbus.c 
b/drivers/block/xen-blkback/xenbus.c
index b33083e..5448dc8 100644
--- a/drivers/block/xen-blkback/xenbus.c
+++ b/drivers/block/xen-blkback/xenbus.c
@@ -14,6 +14,8 @@

  */

+#define pr_fmt(fmt) "xen-blkback: " fmt
+
  #include <stdarg.h>
  #include <linux/module.h>
  #include <linux/kthread.h>
@@ -426,14 +428,14 @@ static int xen_vbd_create(struct xen_blkif *blkif, 
blkif_vdev_t handle,
                                 FMODE_READ : FMODE_WRITE, NULL);

        if (IS_ERR(bdev)) {
-               DPRINTK("xen_vbd_create: device %08x could not be opened.\n",
+               pr_warn("xen_vbd_create: device %08x could not be opened\n",
                        vbd->pdevice);
                return -ENOENT;
        }

        vbd->bdev = bdev;
        if (vbd->bdev->bd_disk == NULL) {
-               DPRINTK("xen_vbd_create: device %08x doesn't exist.\n",
+               pr_warn("xen_vbd_create: device %08x doesn't exist\n",
                        vbd->pdevice);
                xen_vbd_free(vbd);
                return -ENOENT;
@@ -452,7 +454,7 @@ static int xen_vbd_create(struct xen_blkif *blkif, 
blkif_vdev_t handle,
        if (q && blk_queue_secdiscard(q))
                vbd->discard_secure = true;

-       DPRINTK("Successful creation of handle=%04x (dom=%u)\n",
+       pr_debug("Successful creation of handle=%04x (dom=%u)\n",
                handle, blkif->domid);
        return 0;
  }
@@ -460,7 +462,7 @@ static int xen_blkbk_remove(struct xenbus_device *dev)
  {
        struct backend_info *be = dev_get_drvdata(&dev->dev);

-       DPRINTK("");
+       pr_debug("%s %p %d\n", __func__, dev, dev->otherend_id);

        if (be->major || be->minor)
                xenvbd_sysfs_delif(dev);
@@ -566,6 +568,10 @@ static int xen_blkbk_probe(struct xenbus_device *dev,
        int err;
        struct backend_info *be = kzalloc(sizeof(struct backend_info),
                                          GFP_KERNEL);
+
+       /* match the pr_debug in xen_blkbk_remove */
+       pr_debug("%s %p %d\n", __func__, dev, dev->otherend_id);
+
        if (!be) {
                xenbus_dev_fatal(dev, -ENOMEM,
                                 "allocating backend structure");
@@ -597,7 +603,7 @@ static int xen_blkbk_probe(struct xenbus_device *dev,
        return 0;

  fail:
-       DPRINTK("failed");
+       pr_warn("%s failed\n", __func__);
        xen_blkbk_remove(dev);
        return err;
  }
@@ -621,7 +627,7 @@ static void backend_changed(struct xenbus_watch *watch,
        unsigned long handle;
        char *device_type;

-       DPRINTK("");
+       pr_debug("%s %p %d\n", __func__, dev, dev->otherend_id);

        err = xenbus_scanf(XBT_NIL, dev->nodename, "physical-device", "%x:%x",
                           &major, &minor);
@@ -640,7 +646,7 @@ static void backend_changed(struct xenbus_watch *watch,

        if (be->major | be->minor) {
                if (be->major != major || be->minor != minor)
-                       pr_warn(DRV_PFX "changing physical device (from %x:%x to 
%x:%x) not supported.\n",
+                       pr_warn("changing physical device (from %x:%x to %x:%x) not 
supported.\n",
                                be->major, be->minor, major, minor);
                return;
        }
@@ -701,13 +707,12 @@ static void frontend_changed(struct xenbus_device *dev,
        struct backend_info *be = dev_get_drvdata(&dev->dev);
        int err;

-       DPRINTK("%s", xenbus_strstate(frontend_state));
+       pr_debug("%s %p %s\n", __func__, dev, xenbus_strstate(frontend_state));

        switch (frontend_state) {
        case XenbusStateInitialising:
                if (dev->state == XenbusStateClosed) {
-                       pr_info(DRV_PFX "%s: prepare for reconnect\n",
-                               dev->nodename);
+                       pr_info("%s: prepare for reconnect\n", dev->nodename);
                        xenbus_switch_state(dev, XenbusStateInitWait);
                }
                break;
@@ -774,7 +779,7 @@ static void connect(struct backend_info *be)
        int err;
        struct xenbus_device *dev = be->dev;

-       DPRINTK("%s", dev->otherend);
+       pr_debug("%s %s\n", __func__, dev->otherend);

        /* Supply the information about the device the frontend needs */
  again:
@@ -860,7 +865,7 @@ static int connect_ring(struct backend_info *be)
        char protocol[64] = "";
        int err;

-       DPRINTK("%s", dev->otherend);
+       pr_debug("%s %s\n", __func__, dev->otherend);

        err = xenbus_gather(XBT_NIL, dev->otherend, "ring-ref", "%lu",
                            &ring_ref, "event-channel", "%u", &evtchn, NULL);
@@ -895,7 +900,7 @@ static int connect_ring(struct backend_info *be)
        be->blkif->vbd.feature_gnt_persistent = pers_grants;
        be->blkif->vbd.overflow_max_grants = 0;

-       pr_info(DRV_PFX "ring-ref %ld, event-channel %d, protocol %d (%s) %s\n",
+       pr_info("ring-ref %ld, event-channel %d, protocol %d (%s) %s\n",
                ring_ref, evtchn, be->blkif->blk_protocol, protocol,
                pers_grants ? "persistent grants" : "");




_______________________________________________
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®.