Re: [PATCH] dma: fix vchan_cookie_complete() debug print

From: Vinod Koul
Date: Sun Jan 26 2014 - 06:24:33 EST


On Mon, Jan 20, 2014 at 01:28:29PM +0000, Russell King - ARM Linux wrote:
> On Mon, Jan 20, 2014 at 05:33:01PM +0530, Vinod Koul wrote:
> > On Mon, Jan 20, 2014 at 11:28:22AM +0000, Russell King - ARM Linux wrote:
> > > On Mon, Jan 20, 2014 at 03:29:17PM +0530, Vinod Koul wrote:
> > > > On Fri, Dec 06, 2013 at 04:42:09PM +0100, Jonas Jensen wrote:
> > > > > vd->tx.cookie is set zero on dma_cookie_complete(),
> > > > > save to local before printing it.
> > > > >
> > > > > Signed-off-by: Jonas Jensen <jonas.jensen@xxxxxxxxx>
> > > > > ---
> > > > >
> > > > > Notes:
> > > > > dev_vdbg() could also be moved to happen earlier, what do you prefer?
> > > > This would be preferred IMHO. Also pls cc dmaengine@vger on this
> > >
> > > I prefer this version - it means that the verbose debug printk doesn't
> > > impact the completion timing when printk is expensive (eg, because its
> > > outputting via a serial port.)
> > But if you know your printk is costly, do you want to enable these?
>
> dev_vdbg() is for verbose debugging - you only enable it if you really
> need to. Even so, it should have _minimal_ impact where possible. That's
> why I prefer the first patch, because we mark the cookie as being
> complete _before_ we call the verbose debugging, which isn't going to add
> milliseconds to that.
Sure this version is better approach in that respect as it makes it debug
aognostic! Both mine and Dan's comment were trying to simlify by ignoring debug
option, but yes i do agree to you point here. So this patch will be applied!

> If you don't care about debugging, then getting rid of the dev_vdbg().
> But really, I could pull rank and say that this is *my* file, I get to
> choose how stuff should be done here - I'd prefer not to but...
That is not required!

--
~Vinod
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/