Date: Fri, 13 Feb 2015 12:24:15 +0100
From: Markus Pargmann <mpa@pengutronix.de>
To: Joe Perches <joe@perches.com>
Cc: nbd-general@lists.sourceforge.net, linux-kernel@vger.kernel.org
Subject: Re: [PATCH 6/9] nbd: Restructure debugging prints
Message-ID: <20150213112415.GD14438@pengutronix.de>
References: <1423774657-12748-1-git-send-email-mpa@pengutronix.de>
 <1423774657-12748-7-git-send-email-mpa@pengutronix.de>
 <1423775328.15343.38.camel@perches.com>
 <20150213095850.GA10654@pengutronix.de>
 <1423821927.2795.3.camel@perches.com>
MIME-Version: 1.0
Content-Type: multipart/signed; micalg=pgp-sha1;
	protocol="application/pgp-signature"; boundary="96YOpH+ONegL0A3E"
Content-Disposition: inline
In-Reply-To: <1423821927.2795.3.camel@perches.com>
User-Agent: Mutt/1.5.23 (2014-03-12)
Sender: linux-kernel-owner@vger.kernel.org
Content-Length: 3978
Lines: 110


--96YOpH+ONegL0A3E
Content-Type: text/plain; charset=utf-8
Content-Disposition: inline
Content-Transfer-Encoding: quoted-printable

Hi,

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. Dyn=
amic
> > > > debug offers the same amount of filtered debugging.
> > > >=20
> > > > 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.
> > >=20
> > > Perhaps add
> > >=20
> > > #define nbd_dbg(nbd, fmt, ...)					\
> > > 	dev_dbg(disk_to_dev((nbd)->disk), "%s: " fmt,		\
> > > 		nbd->disk->disk_name, ##__VA_ARGS__)
> >=20
> > I am not really happy with those custom debug print macros. What do you
> > think about an inline function 'nbd_to_dev' instead?
>=20
> 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.

>=20
> > > > 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 =3D req->errors ? -EIO : 0;
> > > >  	struct request_queue *q =3D req->q;
> > > >  	unsigned long flags;
> > > > =20
> > > > -	dprintk(DBG_BLKDEV, "%s: request %p: %s\n", req->rq_disk->disk_na=
me,
> > > > -			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");
> > >=20
> > > so this becomes
> > >=20
> > > 	nbd_dbg(nbd, "request %p: %s\n",
> > > 		req, error ? "failed" : "done");
> >=20
> > so this would be:
> >  	nbd_dbg(nbd_to_dev(nbd), "request %p: %s\n",
> >  		req, error ? "failed" : "done");
>=20
> 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.

Thanks,

Markus

--=20
Pengutronix e.K.                           |                             |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

--96YOpH+ONegL0A3E
Content-Type: application/pgp-signature; name="signature.asc"
Content-Description: Digital signature

-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1

iQIcBAEBAgAGBQJU3d7fAAoJEEpcgKtcEGQQtbcP/jld8OP99ZsOqC9qdgOMyQ2D
TdxSy2jmqOUKhAxjH9JOuU5Jo8Ee0+5ArjldB5FM865Q5/QSfCrUfZBfsdwYTzPz
+u+XkRzL7bJfoRR4ZOzMj83TTRUVWnoQazer4pDF62GYtZJJLfENO50ihUUwm4md
pkuCGQxmo1E9wHDQY+8jkD3HTdnG7W19QeTqb6AZ9fE9kl3oFP2qx5k059RV6hrU
yGli+8N5k7EOf39/LNyyAYdw4Ifu0VgGAK2/J6dRANOP9v2Oxj+N4nsH0+Ka/Dgf
K60JA7Ksb1dLzwzIPUJ6ww1rDspeb07E1yDFKN93N1ZAMT7wjFO9WIiUaDYw79B2
udlmyaDNJIZr4MQz2q4d/9kEwOhjTkkY/zU4wceCCG3T368/A2dkfd72vF/Zmlm3
xk6SsKxVtmqjbHzFFtZQ8ETdvu5ncGZJnWOY3Ct3IKGDJVOpY8Dg2vjp+DQwfKtL
FIL1hSIjjTAxGhX9hxXV9JcnFUHSNnyoKUF+5cI0lfu0wODbPLshb4KxyVDHMvzq
eTqjrfZjYEZCN4rX9oT/xMMpDLsOUmCCMR1boGouPWdJI98cqTed8oLbsDpfYVQI
QFdWM8CMxenftUeWtzooblWLShtGD+f4BKzYkDbWxuFXIgd4x/1PD28rR3GvUk90
4I6AmPTgUMSr5GO2TihR
=qraO
-----END PGP SIGNATURE-----

--96YOpH+ONegL0A3E--
--
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/