On Fri, 2015-02-13 at 12:24 +0100, Markus Pargmann wrote:
> On Fri, Feb 13, 2015 at 02:05:27AM -0800, Joe Perches wrote:
> > On Fri, 2015-02-13 at 10:58 +0100, Markus Pargmann wrote:
> > > On Thu, Feb 12, 2015 at 01:08:48PM -0800, Joe Perches wrote:
> > > > On Thu, 2015-02-12 at 21:57 +0100, Markus Pargmann wrote:
> > > > > dprintk has some name collisions with other frameworks and drivers. It
> > > > > is also not necessary to have these custom debug print filters.
> > > > > Dynamic
> > > > > debug offers the same amount of filtered debugging.
> > > > >
> > > > > This patch replaces all dprintks with dev_dbg(). It also removes the
> > > > > ioctl dprintk which prints the ingoing ioctls which should be
> > > > > replaceable by strace or similar stuff.
> > > >
> > > > Perhaps add
> > > >
> > > > #define nbd_dbg(nbd, fmt, ...) \
> > > > dev_dbg(disk_to_dev((nbd)->disk), "%s: " fmt, \
> > > > nbd->disk->disk_name, ##__VA_ARGS__)
> > >
> > > I am not really happy with those custom debug print macros. What do you
> > > think about an inline function 'nbd_to_dev' instead?
> >
> > Wouldn't that change the output?
> > What would the output look like?
>
> I meant a function that just translates struct nbd_device* to struct
> device*. That wouldn't change the output.
> > > > > diff --git a/drivers/block/nbd.c b/drivers/block/nbd.c
> > > > []
> > > > > +static void nbd_end_request(struct nbd_device *nbd, struct request
> > > > > *req)
> > > > > {
> > > > > int error = req->errors ? -EIO : 0;
> > > > > struct request_queue *q = req->q;
> > > > > unsigned long flags;
> > > > >
> > > > > - dprintk(DBG_BLKDEV, "%s: request %p: %s\n",
> > > > > req->rq_disk->disk_name,
> > > > > - req, error ? "failed" : "done");
> > > > > + dev_dbg(disk_to_dev(nbd->disk), "%s: request %p: %s\n",
> > > > > + req->rq_disk->disk_name, req, error ? "failed" :
> > > > > "done");
> > > >
> > > > so this becomes
> > > >
> > > > nbd_dbg(nbd, "request %p: %s\n",
> > > > req, error ? "failed" : "done");
> > >
> > > so this would be:
> > > nbd_dbg(nbd_to_dev(nbd), "request %p: %s\n",
> > > req, error ? "failed" : "done");
> >
> > I don't see much value in that style,
> > but I don't manage the code either.
>
> Oh sorry, I meant to write:
> dev_dbg(nbd_to_dev(nbd), "request %p: %s\n",
> req, error ? "failed" : "done");
>
> So the normal dev_dbg call is still there and the expression to get the
> device from a nbd_device struct is shorter.
Is nbd->disk->disk_name the same string as
disk_to_dev((nbd)->disk)->name?
What's the output of the conversion in this patch?
- dprintk(DBG_BLKDEV, "%s: request %p: %s\n", req->rq_disk->disk_name,
- req, error ? "failed" : "done");
+ dev_dbg(disk_to_dev(nbd->disk), "%s: request %p: %s\n",
+ req->rq_disk->disk_name, req, error ? "failed" : "done");
Should this conversion be as you wrote above
dev_dbg(nbd_to_dev(nbd), "request %p: %s\n",
req, error ? "failed" : "done");
If so, then there's probably not much use in a
custom nbd_dbg macro.
There is some value though in classifying blocks of
debugging output akin to the use of netif_msg_<type>.
That is lost with these conversions.
------------------------------------------------------------------------------
Dive into the World of Parallel Programming. The Go Parallel Website,
sponsored by Intel and developed in partnership with Slashdot Media, is your
hub for all things parallel software development, from weekly thought
leadership blogs to news, videos, case studies, tutorials and more. Take a
look and join the conversation now. http://goparallel.sourceforge.net/
_______________________________________________
Nbd-general mailing list
[email protected]
https://lists.sourceforge.net/lists/listinfo/nbd-general
