Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752364AbbDBBfM (ORCPT ); Wed, 1 Apr 2015 21:35:12 -0400 Received: from szxga03-in.huawei.com ([119.145.14.66]:6255 "EHLO szxga03-in.huawei.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751379AbbDBBfI (ORCPT ); Wed, 1 Apr 2015 21:35:08 -0400 Message-ID: <551C9C42.6020604@huawei.com> Date: Thu, 2 Apr 2015 09:32:50 +0800 From: "Chentao (Boby)" User-Agent: Mozilla/5.0 (Windows NT 6.1; rv:24.0) Gecko/20100101 Thunderbird/24.2.0 MIME-Version: 1.0 To: , , CC: , , , , Subject: Re: [PATCH v2] xen-blkback: define pr_fmt macro to avoid the duplication of DRV_PFX References: <1427900662-25419-1-git-send-email-boby.chen@huawei.com> In-Reply-To: <1427900662-25419-1-git-send-email-boby.chen@huawei.com> Content-Type: text/plain; charset="ISO-8859-1"; format=flowed Content-Transfer-Encoding: 7bit X-Originating-IP: [10.177.25.98] X-CFilter-Loop: Reflected X-Mirapoint-Virus-RAPID-Raw: score=unknown(0), refid=str=0001.0A020203.551C9C52.0174,ss=1,re=0.000,recu=0.000,reip=0.000,cl=1,cld=1,fgs=0, ip=0.0.0.0, so=2013-05-26 15:14:31, dmn=2013-03-21 17:37:32 X-Mirapoint-Loop-Id: 632131684431a51a273be32e9fbce2a4 Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 17123 Lines: 439 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 > --- > 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 > #include > #include > @@ -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 > #include > > -#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 > #include > #include > @@ -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" : ""); > > -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/