[PATCH] dma: fix vchan_cookie_complete() debug print

[PATCH] dma: fix vchan_cookie_complete() debug print

[Jonas Jensen]

vd->tx.cookie is set zero on dma_cookie_complete(),
save to local before printing it.

Signed-off-by: Jonas Jensen <jonas.jensen@gmail.com>
---

Notes:
    dev_vdbg() could also be moved to happen earlier, what do you prefer?
    
    Applies to next-20131206

 drivers/dma/virt-dma.h | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/drivers/dma/virt-dma.h b/drivers/dma/virt-dma.h
index 85c19d6..181b9526 100644
--- a/drivers/dma/virt-dma.h
+++ b/drivers/dma/virt-dma.h
@@ -84,10 +84,12 @@ static inline bool vchan_issue_pending(struct virt_dma_chan *vc)
 static inline void vchan_cookie_complete(struct virt_dma_desc *vd)
 {
 	struct virt_dma_chan *vc = to_virt_chan(vd->tx.chan);
+	dma_cookie_t cookie;
 
+	cookie = vd->tx.cookie;
 	dma_cookie_complete(&vd->tx);
 	dev_vdbg(vc->chan.device->dev, "txd %p[%x]: marked complete\n",
-		vd, vd->tx.cookie);
+		 vd, cookie);
 	list_add_tail(&vd->node, &vc->desc_completed);
 
 	tasklet_schedule(&vc->task);
-- 
1.8.2.1

[PATCH] dma: fix vchan_cookie_complete() debug print

[Dan Williams]

On Fri, Dec 6, 2013 at 7:42 AM, Jonas Jensen <jonas.jensen@gmail.com> wrote:
> vd->tx.cookie is set zero on dma_cookie_complete(),
> save to local before printing it.
>
> Signed-off-by: Jonas Jensen <jonas.jensen@gmail.com>
> ---
>
> Notes:
>     dev_vdbg() could also be moved to happen earlier, what do you prefer?
>
>     Applies to next-20131206
>
>  drivers/dma/virt-dma.h | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/dma/virt-dma.h b/drivers/dma/virt-dma.h
> index 85c19d6..181b9526 100644
> --- a/drivers/dma/virt-dma.h
> +++ b/drivers/dma/virt-dma.h
> @@ -84,10 +84,12 @@ static inline bool vchan_issue_pending(struct virt_dma_chan *vc)
>  static inline void vchan_cookie_complete(struct virt_dma_desc *vd)
>  {
>         struct virt_dma_chan *vc = to_virt_chan(vd->tx.chan);
> +       dma_cookie_t cookie;
>
> +       cookie = vd->tx.cookie;
>         dma_cookie_complete(&vd->tx);
>         dev_vdbg(vc->chan.device->dev, "txd %p[%x]: marked complete\n",
> -               vd, vd->tx.cookie);
> +                vd, cookie);
>         list_add_tail(&vd->node, &vc->desc_completed);


This is a bit smaller:

diff --git a/drivers/dma/virt-dma.h b/drivers/dma/virt-dma.h
index 85c19d63f9fb..7d6f611c7654 100644
--- a/drivers/dma/virt-dma.h
+++ b/drivers/dma/virt-dma.h
@@ -85,9 +85,9 @@ static inline void vchan_cookie_complete(struct
virt_dma_desc *vd)
 {
        struct virt_dma_chan *vc = to_virt_chan(vd->tx.chan);

-       dma_cookie_complete(&vd->tx);
        dev_vdbg(vc->chan.device->dev, "txd %p[%x]: marked complete\n",
                vd, vd->tx.cookie);
+       dma_cookie_complete(&vd->tx);
        list_add_tail(&vd->node, &vc->desc_completed);

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

[Jonas Jensen]

vchan_cookie_complete() is supposed to print the cookie value
but it's always zero because:

dma_cookie_complete() is called prior to dev_vdbg()

Move the call, place it after dev_vdbg().

Signed-off-by: Jonas Jensen <jonas.jensen@gmail.com>
---

Notes:
    Changes since v1:
    
    1. remove local dma_cookie_t
    2. call dma_cookie_complete() after dev_vdbg()
    
    Applies to next-20131209

 drivers/dma/virt-dma.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/dma/virt-dma.h b/drivers/dma/virt-dma.h
index 85c19d6..7d6f611 100644
--- a/drivers/dma/virt-dma.h
+++ b/drivers/dma/virt-dma.h
@@ -85,9 +85,9 @@ static inline void vchan_cookie_complete(struct virt_dma_desc *vd)
 {
 	struct virt_dma_chan *vc = to_virt_chan(vd->tx.chan);
 
-	dma_cookie_complete(&vd->tx);
 	dev_vdbg(vc->chan.device->dev, "txd %p[%x]: marked complete\n",
 		vd, vd->tx.cookie);
+	dma_cookie_complete(&vd->tx);
 	list_add_tail(&vd->node, &vc->desc_completed);
 
 	tasklet_schedule(&vc->task);
-- 
1.8.2.1

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

[Vinod Koul]

On Mon, Dec 09, 2013 at 10:25:23AM +0100, Jonas Jensen wrote:
> vchan_cookie_complete() is supposed to print the cookie value
> but it's always zero because:
> 
> dma_cookie_complete() is called prior to dev_vdbg()
> 
> Move the call, place it after dev_vdbg().
> 
> Signed-off-by: Jonas Jensen <jonas.jensen@gmail.com>
Applied, thanks

--
~Vinod
> ---
> 
> Notes:
>     Changes since v1:
>     
>     1. remove local dma_cookie_t
>     2. call dma_cookie_complete() after dev_vdbg()
>     
>     Applies to next-20131209
> 
>  drivers/dma/virt-dma.h | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/dma/virt-dma.h b/drivers/dma/virt-dma.h
> index 85c19d6..7d6f611 100644
> --- a/drivers/dma/virt-dma.h
> +++ b/drivers/dma/virt-dma.h
> @@ -85,9 +85,9 @@ static inline void vchan_cookie_complete(struct virt_dma_desc *vd)
>  {
>  	struct virt_dma_chan *vc = to_virt_chan(vd->tx.chan);
>  
> -	dma_cookie_complete(&vd->tx);
>  	dev_vdbg(vc->chan.device->dev, "txd %p[%x]: marked complete\n",
>  		vd, vd->tx.cookie);
> +	dma_cookie_complete(&vd->tx);
>  	list_add_tail(&vd->node, &vc->desc_completed);
>  
>  	tasklet_schedule(&vc->task);
> -- 
> 1.8.2.1
> 

-- 

[PATCH] dma: fix vchan_cookie_complete() debug print

[Vinod Koul]

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@gmail.com>
> ---
> 
> Notes:
>     dev_vdbg() could also be moved to happen earlier, what do you prefer?
This would be preferred IMHO. Also pls cc dmaengine at vger on this

--
~Vinod
>     
>     Applies to next-20131206
> 
>  drivers/dma/virt-dma.h | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/dma/virt-dma.h b/drivers/dma/virt-dma.h
> index 85c19d6..181b9526 100644
> --- a/drivers/dma/virt-dma.h
> +++ b/drivers/dma/virt-dma.h
> @@ -84,10 +84,12 @@ static inline bool vchan_issue_pending(struct virt_dma_chan *vc)
>  static inline void vchan_cookie_complete(struct virt_dma_desc *vd)
>  {
>  	struct virt_dma_chan *vc = to_virt_chan(vd->tx.chan);
> +	dma_cookie_t cookie;
>  
> +	cookie = vd->tx.cookie;
>  	dma_cookie_complete(&vd->tx);
>  	dev_vdbg(vc->chan.device->dev, "txd %p[%x]: marked complete\n",
> -		vd, vd->tx.cookie);
> +		 vd, cookie);
>  	list_add_tail(&vd->node, &vc->desc_completed);
>  
>  	tasklet_schedule(&vc->task);
> -- 
> 1.8.2.1
> 

-- 

[PATCH] dma: fix vchan_cookie_complete() debug print

[Russell King - ARM Linux]

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@gmail.com>
> > ---
> > 
> > Notes:
> >     dev_vdbg() could also be moved to happen earlier, what do you prefer?
> This would be preferred IMHO. Also pls cc dmaengine at 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.)

-- 
FTTC broadband for 0.8mile line: 5.8Mbps down 500kbps up.  Estimation
in database were 13.1 to 19Mbit for a good line, about 7.5+ for a bad.
Estimate before purchase was "up to 13.2Mbit".

[PATCH] dma: fix vchan_cookie_complete() debug print

[Vinod Koul]

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@gmail.com>
> > > ---
> > > 
> > > Notes:
> > >     dev_vdbg() could also be moved to happen earlier, what do you prefer?
> > This would be preferred IMHO. Also pls cc dmaengine at 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?

-- 
~Vinod

[PATCH] dma: fix vchan_cookie_complete() debug print

[Russell King - ARM Linux]

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@gmail.com>
> > > > ---
> > > > 
> > > > Notes:
> > > >     dev_vdbg() could also be moved to happen earlier, what do you prefer?
> > > This would be preferred IMHO. Also pls cc dmaengine at 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.

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...

-- 
FTTC broadband for 0.8mile line: 5.8Mbps down 500kbps up.  Estimation
in database were 13.1 to 19Mbit for a good line, about 7.5+ for a bad.
Estimate before purchase was "up to 13.2Mbit".

[PATCH] dma: fix vchan_cookie_complete() debug print

[Vinod Koul]

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@gmail.com>
> > > > > ---
> > > > > 
> > > > > Notes:
> > > > >     dev_vdbg() could also be moved to happen earlier, what do you prefer?
> > > > This would be preferred IMHO. Also pls cc dmaengine at 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