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 <[email protected]>
>> ---
>> 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.
>
>
> .
>