Re: [PATCH v2] media: vb2: vmalloc-based allocator user pointer handling

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hi Sylwester,

On Thursday 08 December 2011 22:43:00 Sylwester Nawrocki wrote:
> Hi Laurent,
> 
> On 12/08/2011 11:56 AM, Laurent Pinchart wrote:
> > On Wednesday 07 December 2011 17:29:06 Marek Szyprowski wrote:
> >> From: Andrzej Pietrasiewicz <andrzej.p@xxxxxxxxxxx>
> 
> [...]
> 
> >> -	printk(KERN_DEBUG "Allocated vmalloc buffer of size %ld at
> >> vaddr=%p\n", -			buf->size, buf->vaddr);
> >> +	pr_err("Allocated vmalloc buffer of size %ld at vaddr=%p\n",
> >> buf->size, +	       buf->vaddr);
> > 
> > Turning KERN_DEBUG into pr_err() is a bit harsh :-) In my opinion even
> > KERN_DEBUG is too much here, I don't want to get messages printed to the
> > kernel log every time I allocate buffers.
> 
> Indeed, pr_err looks like an overkill:) I think pr_debug() would be fine
> here.
> 
> >>  	return buf;
> >>  
> >>  }
> >> 
> >> @@ -59,13 +63,87 @@ static void vb2_vmalloc_put(void *buf_priv)
> >> 
> >>  	struct vb2_vmalloc_buf *buf = buf_priv;
> >>  	
> >>  	if (atomic_dec_and_test(&buf->refcount)) {
> >> 
> >> -		printk(KERN_DEBUG "%s: Freeing vmalloc mem at vaddr=%p\n",
> >> -			__func__, buf->vaddr);
> >> +		pr_debug("%s: Freeing vmalloc mem at vaddr=%p\n", __func__,
> >> +			 buf->vaddr);
> > 
> > Same here. Should we get rid of those two messages, or at least
> > conditionally- compile them out of the kernel by default ?
> 
> During compilation pr_debug() will most likely be optimized away if DEBUG
> and CONFIG_DYNAMIC_DEBUG isn't defined, as it is then defined as:
> 
> static inline __printf(1, 2)
> int no_printk(const char *fmt, ...)
> {
> 	return 0;
> }
> 
> Plus it's easy with pr_debug() to enable debug trace while dynamic printk()
> is enabled in the kernel configuration.

My bad. pr_debug() is fine.

-- 
Regards,

Laurent Pinchart
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[Index of Archives]     [Linux Input]     [Video for Linux]     [Gstreamer Embedded]     [Mplayer Users]     [Linux USB Devel]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [Yosemite Backpacking]
  Powered by Linux