public inbox for linux-kernel@vger.kernel.org
 help / color / mirror / Atom feed
* Re: [PATCH] xen-blkback: define pr_fmt macro to avoid the duplication of DRV_PFX
  2015-03-31 21:14 [PATCH] xen-blkback: define pr_fmt macro to avoid the duplication of DRV_PFX Tao Chen
@ 2015-03-31 14:57 ` Roger Pau Monné
  2015-03-31 16:53   ` Joe Perches
  2015-04-01  6:40   ` Chentao (Boby)
  0 siblings, 2 replies; 4+ messages in thread
From: Roger Pau Monné @ 2015-03-31 14:57 UTC (permalink / raw)
  To: Tao Chen, konrad.wilk; +Cc: xen-devel, linux-kernel, wu.wubin, rudy.zhangmin

El 31/03/15 a les 23.14, Tao Chen ha escrit:
> Define pr_fmt macro with {xen-blkback: } prefix, then remove all use
> of DRV_PFX in the pr and DPRINTK sentences. 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.
> 
> These all make the readability of the code become better.

Thanks for the patch.

> Signed-off-by: Tao Chen <boby.chen@huawei.com>
> ---
>  drivers/block/xen-blkback/blkback.c | 62 ++++++++++++++++++-------------------
>  drivers/block/xen-blkback/common.h  |  6 ----
>  drivers/block/xen-blkback/xenbus.c  | 18 ++++++-----
>  3 files changed, 42 insertions(+), 44 deletions(-)
> 
[...]
> 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..0dbbfeb 100644
> --- a/drivers/block/xen-blkback/xenbus.c
> +++ b/drivers/block/xen-blkback/xenbus.c
> @@ -14,6 +14,11 @@
>  
>  */
>  
> +#define pr_fmt(fmt) "xen-blkback: " fmt
> +#define DPRINTK(fmt, args...)			\
> +	pr_debug("(%s:%d) " fmt ".\n",		\
> +		__func__, __LINE__, ##args)
> +
>  #include <stdarg.h>
>  #include <linux/module.h>
>  #include <linux/kthread.h>
> @@ -426,14 +431,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",
> +		DPRINTK("xen_vbd_create: device %08x could not be opened",
>  			vbd->pdevice);
>  		return -ENOENT;
>  	}
>  
>  	vbd->bdev = bdev;
>  	if (vbd->bdev->bd_disk == NULL) {
> -		DPRINTK("xen_vbd_create: device %08x doesn't exist.\n",
> +		DPRINTK("xen_vbd_create: device %08x doesn't exist",
>  			vbd->pdevice);

IMHO this two above should be made a pr_warn probably...

>  		xen_vbd_free(vbd);
>  		return -ENOENT;
> @@ -452,7 +457,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",
> +	DPRINTK("Successful creation of handle=%04x (dom=%u)",
>  		handle, blkif->domid);

...and this should be turned into a plain pr_debug. And with that we can
get rid of DPRINTK.

Roger.


^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH] xen-blkback: define pr_fmt macro to avoid the duplication of DRV_PFX
  2015-03-31 14:57 ` Roger Pau Monné
@ 2015-03-31 16:53   ` Joe Perches
  2015-04-01  6:40   ` Chentao (Boby)
  1 sibling, 0 replies; 4+ messages in thread
From: Joe Perches @ 2015-03-31 16:53 UTC (permalink / raw)
  To: Roger Pau Monné
  Cc: Tao Chen, konrad.wilk, xen-devel, linux-kernel, wu.wubin,
	rudy.zhangmin

On Tue, 2015-03-31 at 16:57 +0200, Roger Pau Monné wrote:
> El 31/03/15 a les 23.14, Tao Chen ha escrit:
> > Define pr_fmt macro with {xen-blkback: } prefix, then remove all use
> > of DRV_PFX in the pr and DPRINTK sentences. It will simplify the code.
[]
> > diff --git a/drivers/block/xen-blkback/xenbus.c b/drivers/block/xen-blkback/xenbus.c
[]
> > @@ -14,6 +14,11 @@
> >  
> >  */
> >  
> > +#define pr_fmt(fmt) "xen-blkback: " fmt
> > +#define DPRINTK(fmt, args...)			\
> > +	pr_debug("(%s:%d) " fmt ".\n",		\
> > +		__func__, __LINE__, ##args)

As dynamic debug can emit __func__ and __LINE__
I suggest converting DPRINTK to pr_debug.



^ permalink raw reply	[flat|nested] 4+ messages in thread

* [PATCH] xen-blkback: define pr_fmt macro to avoid the duplication of DRV_PFX
@ 2015-03-31 21:14 Tao Chen
  2015-03-31 14:57 ` Roger Pau Monné
  0 siblings, 1 reply; 4+ messages in thread
From: Tao Chen @ 2015-03-31 21:14 UTC (permalink / raw)
  To: konrad.wilk, roger.pau
  Cc: xen-devel, linux-kernel, boby.chen, wu.wubin, rudy.zhangmin

Define pr_fmt macro with {xen-blkback: } prefix, then remove all use
of DRV_PFX in the pr and DPRINTK sentences. 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.

These all make the readability of the code become better.

Signed-off-by: Tao Chen <boby.chen@huawei.com>
---
 drivers/block/xen-blkback/blkback.c | 62 ++++++++++++++++++-------------------
 drivers/block/xen-blkback/common.h  |  6 ----
 drivers/block/xen-blkback/xenbus.c  | 18 ++++++-----
 3 files changed, 42 insertions(+), 44 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..0dbbfeb 100644
--- a/drivers/block/xen-blkback/xenbus.c
+++ b/drivers/block/xen-blkback/xenbus.c
@@ -14,6 +14,11 @@
 
 */
 
+#define pr_fmt(fmt) "xen-blkback: " fmt
+#define DPRINTK(fmt, args...)			\
+	pr_debug("(%s:%d) " fmt ".\n",		\
+		__func__, __LINE__, ##args)
+
 #include <stdarg.h>
 #include <linux/module.h>
 #include <linux/kthread.h>
@@ -426,14 +431,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",
+		DPRINTK("xen_vbd_create: device %08x could not be opened",
 			vbd->pdevice);
 		return -ENOENT;
 	}
 
 	vbd->bdev = bdev;
 	if (vbd->bdev->bd_disk == NULL) {
-		DPRINTK("xen_vbd_create: device %08x doesn't exist.\n",
+		DPRINTK("xen_vbd_create: device %08x doesn't exist",
 			vbd->pdevice);
 		xen_vbd_free(vbd);
 		return -ENOENT;
@@ -452,7 +457,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",
+	DPRINTK("Successful creation of handle=%04x (dom=%u)",
 		handle, blkif->domid);
 	return 0;
 }
@@ -640,7 +645,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;
 	}
@@ -706,8 +711,7 @@ static void frontend_changed(struct xenbus_device *dev,
 	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;
@@ -895,7 +899,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" : "");
 
-- 
1.8.5


^ permalink raw reply related	[flat|nested] 4+ messages in thread

* Re: [PATCH] xen-blkback: define pr_fmt macro to avoid the duplication of DRV_PFX
  2015-03-31 14:57 ` Roger Pau Monné
  2015-03-31 16:53   ` Joe Perches
@ 2015-04-01  6:40   ` Chentao (Boby)
  1 sibling, 0 replies; 4+ messages in thread
From: Chentao (Boby) @ 2015-04-01  6:40 UTC (permalink / raw)
  To: Roger Pau Monné, Joe Perches
  Cc: konrad.wilk, xen-devel, linux-kernel, wu.wubin, rudy.zhangmin

Thanks roger and joe.

I will adopt your suggestions in my v2 patch.

On 2015/3/31 22:57, Roger Pau Monné wrote:
> El 31/03/15 a les 23.14, Tao Chen ha escrit:
>> Define pr_fmt macro with {xen-blkback: } prefix, then remove all use
>> of DRV_PFX in the pr and DPRINTK sentences. 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.
>>
>> These all make the readability of the code become better.
>
> Thanks for the patch.
>
>> Signed-off-by: Tao Chen <boby.chen@huawei.com>
>> ---
>>   drivers/block/xen-blkback/blkback.c | 62 ++++++++++++++++++-------------------
>>   drivers/block/xen-blkback/common.h  |  6 ----
>>   drivers/block/xen-blkback/xenbus.c  | 18 ++++++-----
>>   3 files changed, 42 insertions(+), 44 deletions(-)
>>
> [...]
>> 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..0dbbfeb 100644
>> --- a/drivers/block/xen-blkback/xenbus.c
>> +++ b/drivers/block/xen-blkback/xenbus.c
>> @@ -14,6 +14,11 @@
>>
>>   */
>>
>> +#define pr_fmt(fmt) "xen-blkback: " fmt
>> +#define DPRINTK(fmt, args...)			\
>> +	pr_debug("(%s:%d) " fmt ".\n",		\
>> +		__func__, __LINE__, ##args)
>> +
>>   #include <stdarg.h>
>>   #include <linux/module.h>
>>   #include <linux/kthread.h>
>> @@ -426,14 +431,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",
>> +		DPRINTK("xen_vbd_create: device %08x could not be opened",
>>   			vbd->pdevice);
>>   		return -ENOENT;
>>   	}
>>
>>   	vbd->bdev = bdev;
>>   	if (vbd->bdev->bd_disk == NULL) {
>> -		DPRINTK("xen_vbd_create: device %08x doesn't exist.\n",
>> +		DPRINTK("xen_vbd_create: device %08x doesn't exist",
>>   			vbd->pdevice);
>
> IMHO this two above should be made a pr_warn probably...
>
>>   		xen_vbd_free(vbd);
>>   		return -ENOENT;
>> @@ -452,7 +457,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",
>> +	DPRINTK("Successful creation of handle=%04x (dom=%u)",
>>   		handle, blkif->domid);
>
> ...and this should be turned into a plain pr_debug. And with that we can
> get rid of DPRINTK.
>
> Roger.
>
>
> .
>


^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2015-04-01  6:40 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2015-03-31 21:14 [PATCH] xen-blkback: define pr_fmt macro to avoid the duplication of DRV_PFX Tao Chen
2015-03-31 14:57 ` Roger Pau Monné
2015-03-31 16:53   ` Joe Perches
2015-04-01  6:40   ` Chentao (Boby)

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox