| Message ID | 1386344529-25617-1-git-send-email-jonas.jensen@gmail.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
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@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 >
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@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.)
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@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?
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@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...
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@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
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);
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(-)