* [PATCH 1/5] media: vb2: don't go out of the buffer range
2017-12-28 16:29 [PATCH 0/5] some VB2 bug fixes and improvements Mauro Carvalho Chehab
@ 2017-12-28 16:29 ` Mauro Carvalho Chehab
2017-12-28 18:18 ` Sakari Ailus
2017-12-28 16:29 ` [PATCH 2/5] media: vb2: Fix a bug about unnecessary calls to queue cancel and free Mauro Carvalho Chehab
` (3 subsequent siblings)
4 siblings, 1 reply; 9+ messages in thread
From: Mauro Carvalho Chehab @ 2017-12-28 16:29 UTC (permalink / raw)
To: Linux Media Mailing List
Cc: Mauro Carvalho Chehab, Mauro Carvalho Chehab, Hans Verkuil,
Sakari Ailus, Christophe JAILLET, Satendra Singh Thakur,
Stanimir Varbanov
Currently, there's no check if an invalid buffer range
is passed. However, while testing DVB memory mapped apps,
I got this:
videobuf2_core: VB: num_buffers -2143943680, buffer 33, index -2143943647
unable to handle kernel paging request at ffff888b773c0890
IP: __vb2_queue_alloc+0x134/0x4e0 [videobuf2_core]
PGD 4142c7067 P4D 4142c7067 PUD 0
Oops: 0002 [#1] SMP
Modules linked in: xt_CHECKSUM iptable_mangle ipt_MASQUERADE nf_nat_masquerade_ipv4 iptable_nat nf_nat_ipv4 nf_nat nf_conntrack_ipv4 nf_defrag_ipv4 xt_conntrack nf_conntrack tun bridge stp llc ebtable_filter ebtables ip6table_filter ip6_tables bluetooth rfkill ecdh_generic binfmt_misc rc_dvbsky sp2 ts2020 intel_rapl x86_pkg_temp_thermal dvb_usb_dvbsky intel_powerclamp dvb_usb_v2 coretemp m88ds3103 kvm_intel i2c_mux dvb_core snd_hda_codec_hdmi crct10dif_pclmul crc32_pclmul videobuf2_vmalloc videobuf2_memops snd_hda_intel ghash_clmulni_intel videobuf2_core snd_hda_codec rc_core mei_me intel_cstate snd_hwdep snd_hda_core videodev intel_uncore snd_pcm mei media tpm_tis tpm_tis_core intel_rapl_perf tpm snd_timer lpc_ich snd soundcore kvm irqbypass libcrc32c i915 i2c_algo_bit drm_kms_helper
e1000e ptp drm crc32c_intel video pps_core
CPU: 3 PID: 1776 Comm: dvbv5-zap Not tainted 4.14.0+ #78
Hardware name: /NUC5i7RYB, BIOS RYBDWi35.86A.0364.2017.0511.0949 05/11/2017
task: ffff88877c73bc80 task.stack: ffffb7c402418000
RIP: 0010:__vb2_queue_alloc+0x134/0x4e0 [videobuf2_core]
RSP: 0018:ffffb7c40241bc60 EFLAGS: 00010246
RAX: 0000000080360421 RBX: 0000000000000021 RCX: 000000000000000a
RDX: ffffb7c40241bcf4 RSI: ffff888780362c60 RDI: ffff888796d8e130
RBP: ffffb7c40241bcc8 R08: 0000000000000316 R09: 0000000000000004
R10: ffff888780362c00 R11: 0000000000000001 R12: 000000000002f000
R13: ffff8887758be700 R14: 0000000000021000 R15: 0000000000000001
FS: 00007f2849024740(0000) GS:ffff888796d80000(0000) knlGS:0000000000000000
CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
CR2: ffff888b773c0890 CR3: 000000043beb2005 CR4: 00000000003606e0
Call Trace:
vb2_core_reqbufs+0x226/0x420 [videobuf2_core]
dvb_vb2_reqbufs+0x2d/0xc0 [dvb_core]
dvb_dvr_do_ioctl+0x98/0x1d0 [dvb_core]
dvb_usercopy+0x53/0x1b0 [dvb_core]
? dvb_demux_ioctl+0x20/0x20 [dvb_core]
? tty_ldisc_deref+0x16/0x20
? tty_write+0x1f9/0x310
? process_echoes+0x70/0x70
dvb_dvr_ioctl+0x15/0x20 [dvb_core]
do_vfs_ioctl+0xa5/0x600
SyS_ioctl+0x79/0x90
entry_SYSCALL_64_fastpath+0x1a/0xa5
RIP: 0033:0x7f28486f7ea7
RSP: 002b:00007ffc13b2db18 EFLAGS: 00000246 ORIG_RAX: 0000000000000010
RAX: ffffffffffffffda RBX: 000055b10fc06130 RCX: 00007f28486f7ea7
RDX: 00007ffc13b2db48 RSI: 00000000c0086f3c RDI: 0000000000000007
RBP: 0000000000000203 R08: 000055b10df1e02c R09: 000000000000002e
R10: 0036b42415108357 R11: 0000000000000246 R12: 0000000000000000
R13: 00007f2849062f60 R14: 00000000000001f1 R15: 00007ffc13b2da54
Code: 74 0a 60 8b 0a 48 83 c0 30 48 83 c2 04 89 48 d0 89 48 d4 48 39 f0 75 eb 41 8b 42 08 83 7d d4 01 41 c7 82 ec 01 00 00 ff ff ff ff <4d> 89 94 c5 88 00 00 00 74 14 83 c3 01 41 39 dc 0f 85 f1 fe ff
RIP: __vb2_queue_alloc+0x134/0x4e0 [videobuf2_core] RSP: ffffb7c40241bc60
CR2: ffff888b773c0890
So, add a sanity check in order to prevent going past array.
Signed-off-by: Mauro Carvalho Chehab <mchehab@s-opensource.com>
---
drivers/media/common/videobuf/videobuf2-core.c | 3 +++
1 file changed, 3 insertions(+)
diff --git a/drivers/media/common/videobuf/videobuf2-core.c b/drivers/media/common/videobuf/videobuf2-core.c
index a8589d96ef72..a3b4836fc41d 100644
--- a/drivers/media/common/videobuf/videobuf2-core.c
+++ b/drivers/media/common/videobuf/videobuf2-core.c
@@ -332,6 +332,9 @@ static int __vb2_queue_alloc(struct vb2_queue *q, enum vb2_memory memory,
struct vb2_buffer *vb;
int ret;
+ /* Ensure that q->num_buffers+num_buffers is below VB2_MAX_FRAME */
+ num_buffers = min_t(unsigned int, num_buffers, VB2_MAX_FRAME - q->num_buffers);
+
for (buffer = 0; buffer < num_buffers; ++buffer) {
/* Allocate videobuf buffer structures */
vb = kzalloc(q->buf_struct_size, GFP_KERNEL);
--
2.14.3
^ permalink raw reply related [flat|nested] 9+ messages in thread* Re: [PATCH 1/5] media: vb2: don't go out of the buffer range
2017-12-28 16:29 ` [PATCH 1/5] media: vb2: don't go out of the buffer range Mauro Carvalho Chehab
@ 2017-12-28 18:18 ` Sakari Ailus
0 siblings, 0 replies; 9+ messages in thread
From: Sakari Ailus @ 2017-12-28 18:18 UTC (permalink / raw)
To: Mauro Carvalho Chehab
Cc: Linux Media Mailing List, Mauro Carvalho Chehab, Hans Verkuil,
Christophe JAILLET, Satendra Singh Thakur, Stanimir Varbanov
Hi Mauro,
On Thu, Dec 28, 2017 at 02:29:34PM -0200, Mauro Carvalho Chehab wrote:
> Currently, there's no check if an invalid buffer range
> is passed. However, while testing DVB memory mapped apps,
> I got this:
>
> videobuf2_core: VB: num_buffers -2143943680, buffer 33, index -2143943647
> unable to handle kernel paging request at ffff888b773c0890
> IP: __vb2_queue_alloc+0x134/0x4e0 [videobuf2_core]
> PGD 4142c7067 P4D 4142c7067 PUD 0
> Oops: 0002 [#1] SMP
> Modules linked in: xt_CHECKSUM iptable_mangle ipt_MASQUERADE nf_nat_masquerade_ipv4 iptable_nat nf_nat_ipv4 nf_nat nf_conntrack_ipv4 nf_defrag_ipv4 xt_conntrack nf_conntrack tun bridge stp llc ebtable_filter ebtables ip6table_filter ip6_tables bluetooth rfkill ecdh_generic binfmt_misc rc_dvbsky sp2 ts2020 intel_rapl x86_pkg_temp_thermal dvb_usb_dvbsky intel_powerclamp dvb_usb_v2 coretemp m88ds3103 kvm_intel i2c_mux dvb_core snd_hda_codec_hdmi crct10dif_pclmul crc32_pclmul videobuf2_vmalloc videobuf2_memops snd_hda_intel ghash_clmulni_intel videobuf2_core snd_hda_codec rc_core mei_me intel_cstate snd_hwdep snd_hda_core videodev intel_uncore snd_pcm mei media tpm_tis tpm_tis_core intel_rapl_perf tpm snd_timer lpc_ich snd soundcore kvm irqbypass libcrc32c i915 i2c_algo_bit drm_kms_helper
> e1000e ptp drm crc32c_intel video pps_core
> CPU: 3 PID: 1776 Comm: dvbv5-zap Not tainted 4.14.0+ #78
> Hardware name: /NUC5i7RYB, BIOS RYBDWi35.86A.0364.2017.0511.0949 05/11/2017
> task: ffff88877c73bc80 task.stack: ffffb7c402418000
> RIP: 0010:__vb2_queue_alloc+0x134/0x4e0 [videobuf2_core]
> RSP: 0018:ffffb7c40241bc60 EFLAGS: 00010246
> RAX: 0000000080360421 RBX: 0000000000000021 RCX: 000000000000000a
> RDX: ffffb7c40241bcf4 RSI: ffff888780362c60 RDI: ffff888796d8e130
> RBP: ffffb7c40241bcc8 R08: 0000000000000316 R09: 0000000000000004
> R10: ffff888780362c00 R11: 0000000000000001 R12: 000000000002f000
> R13: ffff8887758be700 R14: 0000000000021000 R15: 0000000000000001
> FS: 00007f2849024740(0000) GS:ffff888796d80000(0000) knlGS:0000000000000000
> CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
> CR2: ffff888b773c0890 CR3: 000000043beb2005 CR4: 00000000003606e0
> Call Trace:
> vb2_core_reqbufs+0x226/0x420 [videobuf2_core]
> dvb_vb2_reqbufs+0x2d/0xc0 [dvb_core]
> dvb_dvr_do_ioctl+0x98/0x1d0 [dvb_core]
> dvb_usercopy+0x53/0x1b0 [dvb_core]
> ? dvb_demux_ioctl+0x20/0x20 [dvb_core]
> ? tty_ldisc_deref+0x16/0x20
> ? tty_write+0x1f9/0x310
> ? process_echoes+0x70/0x70
> dvb_dvr_ioctl+0x15/0x20 [dvb_core]
> do_vfs_ioctl+0xa5/0x600
> SyS_ioctl+0x79/0x90
> entry_SYSCALL_64_fastpath+0x1a/0xa5
> RIP: 0033:0x7f28486f7ea7
> RSP: 002b:00007ffc13b2db18 EFLAGS: 00000246 ORIG_RAX: 0000000000000010
> RAX: ffffffffffffffda RBX: 000055b10fc06130 RCX: 00007f28486f7ea7
> RDX: 00007ffc13b2db48 RSI: 00000000c0086f3c RDI: 0000000000000007
> RBP: 0000000000000203 R08: 000055b10df1e02c R09: 000000000000002e
> R10: 0036b42415108357 R11: 0000000000000246 R12: 0000000000000000
> R13: 00007f2849062f60 R14: 00000000000001f1 R15: 00007ffc13b2da54
> Code: 74 0a 60 8b 0a 48 83 c0 30 48 83 c2 04 89 48 d0 89 48 d4 48 39 f0 75 eb 41 8b 42 08 83 7d d4 01 41 c7 82 ec 01 00 00 ff ff ff ff <4d> 89 94 c5 88 00 00 00 74 14 83 c3 01 41 39 dc 0f 85 f1 fe ff
> RIP: __vb2_queue_alloc+0x134/0x4e0 [videobuf2_core] RSP: ffffb7c40241bc60
> CR2: ffff888b773c0890
>
> So, add a sanity check in order to prevent going past array.
>
> Signed-off-by: Mauro Carvalho Chehab <mchehab@s-opensource.com>
> ---
> drivers/media/common/videobuf/videobuf2-core.c | 3 +++
> 1 file changed, 3 insertions(+)
>
> diff --git a/drivers/media/common/videobuf/videobuf2-core.c b/drivers/media/common/videobuf/videobuf2-core.c
> index a8589d96ef72..a3b4836fc41d 100644
> --- a/drivers/media/common/videobuf/videobuf2-core.c
> +++ b/drivers/media/common/videobuf/videobuf2-core.c
> @@ -332,6 +332,9 @@ static int __vb2_queue_alloc(struct vb2_queue *q, enum vb2_memory memory,
> struct vb2_buffer *vb;
> int ret;
>
> + /* Ensure that q->num_buffers+num_buffers is below VB2_MAX_FRAME */
> + num_buffers = min_t(unsigned int, num_buffers, VB2_MAX_FRAME - q->num_buffers);
Could you wrap the above line? It's over 80 characters.
With that,
Acked-by: Sakari Ailus <sakari.ailus@linux.intel.com>
> +
> for (buffer = 0; buffer < num_buffers; ++buffer) {
> /* Allocate videobuf buffer structures */
> vb = kzalloc(q->buf_struct_size, GFP_KERNEL);
> --
> 2.14.3
>
--
Sakari Ailus
sakari.ailus@linux.intel.com
^ permalink raw reply [flat|nested] 9+ messages in thread
* [PATCH 2/5] media: vb2: Fix a bug about unnecessary calls to queue cancel and free
2017-12-28 16:29 [PATCH 0/5] some VB2 bug fixes and improvements Mauro Carvalho Chehab
2017-12-28 16:29 ` [PATCH 1/5] media: vb2: don't go out of the buffer range Mauro Carvalho Chehab
@ 2017-12-28 16:29 ` Mauro Carvalho Chehab
2017-12-28 18:24 ` Sakari Ailus
2017-12-28 16:29 ` [PATCH 3/5] media: vb2: Enforce VB2_MAX_FRAME in vb2_core_reqbufs better Mauro Carvalho Chehab
` (2 subsequent siblings)
4 siblings, 1 reply; 9+ messages in thread
From: Mauro Carvalho Chehab @ 2017-12-28 16:29 UTC (permalink / raw)
To: Linux Media Mailing List
Cc: Satendra Singh Thakur, Mauro Carvalho Chehab, Hans Verkuil,
Sakari Ailus, Christophe JAILLET, Gustavo Padovan,
Mauro Carvalho Chehab
From: Satendra Singh Thakur <satendra.t@samsung.com>
Currently, there's a logic with checks if *count is non-zero,
q->num_buffers is zero and q->memory is different than memory.
That's flawed when the device is initialized, or after the
queues are freed, as it does, unnecessary calls to
__vb2_queue_cancel() and __vb2_queue_free().
That can be avoided by making sure that q->memory is set to
VB2_MEMORY_UNKNOWN at vb2_core_queue_init(), and adding such
check at the loop.
[mchehab@s-opensource.com: fix checkpatch issues and improve the
patch, by setting q->memory to zero at vb2_core_queue_init]
Signed-off-by: Satendra Singh Thakur <satendra.t@samsung.com>
Signed-off-by: Mauro Carvalho Chehab <mchehab@s-opensource.com>
---
drivers/media/common/videobuf/videobuf2-core.c | 7 +++++--
1 file changed, 5 insertions(+), 2 deletions(-)
diff --git a/drivers/media/common/videobuf/videobuf2-core.c b/drivers/media/common/videobuf/videobuf2-core.c
index a3b4836fc41d..1793bdb1fe54 100644
--- a/drivers/media/common/videobuf/videobuf2-core.c
+++ b/drivers/media/common/videobuf/videobuf2-core.c
@@ -523,7 +523,7 @@ static int __vb2_queue_free(struct vb2_queue *q, unsigned int buffers)
q->num_buffers -= buffers;
if (!q->num_buffers) {
- q->memory = 0;
+ q->memory = VB2_MEMORY_UNKNOWN;
INIT_LIST_HEAD(&q->queued_list);
}
return 0;
@@ -665,7 +665,8 @@ int vb2_core_reqbufs(struct vb2_queue *q, enum vb2_memory memory,
return -EBUSY;
}
- if (*count == 0 || q->num_buffers != 0 || q->memory != memory) {
+ if (*count == 0 || q->num_buffers != 0 ||
+ (q->memory != VB2_MEMORY_UNKNOWN && q->memory != memory)) {
/*
* We already have buffers allocated, so first check if they
* are not in use and can be freed.
@@ -1997,6 +1998,8 @@ int vb2_core_queue_init(struct vb2_queue *q)
mutex_init(&q->mmap_lock);
init_waitqueue_head(&q->done_wq);
+ q->memory = VB2_MEMORY_UNKNOWN;
+
if (q->buf_struct_size == 0)
q->buf_struct_size = sizeof(struct vb2_buffer);
--
2.14.3
^ permalink raw reply related [flat|nested] 9+ messages in thread* Re: [PATCH 2/5] media: vb2: Fix a bug about unnecessary calls to queue cancel and free
2017-12-28 16:29 ` [PATCH 2/5] media: vb2: Fix a bug about unnecessary calls to queue cancel and free Mauro Carvalho Chehab
@ 2017-12-28 18:24 ` Sakari Ailus
0 siblings, 0 replies; 9+ messages in thread
From: Sakari Ailus @ 2017-12-28 18:24 UTC (permalink / raw)
To: Mauro Carvalho Chehab
Cc: Linux Media Mailing List, Satendra Singh Thakur,
Mauro Carvalho Chehab, Hans Verkuil, Christophe JAILLET,
Gustavo Padovan
On Thu, Dec 28, 2017 at 02:29:35PM -0200, Mauro Carvalho Chehab wrote:
> From: Satendra Singh Thakur <satendra.t@samsung.com>
>
> Currently, there's a logic with checks if *count is non-zero,
> q->num_buffers is zero and q->memory is different than memory.
>
> That's flawed when the device is initialized, or after the
> queues are freed, as it does, unnecessary calls to
> __vb2_queue_cancel() and __vb2_queue_free().
>
> That can be avoided by making sure that q->memory is set to
> VB2_MEMORY_UNKNOWN at vb2_core_queue_init(), and adding such
> check at the loop.
>
> [mchehab@s-opensource.com: fix checkpatch issues and improve the
> patch, by setting q->memory to zero at vb2_core_queue_init]
> Signed-off-by: Satendra Singh Thakur <satendra.t@samsung.com>
> Signed-off-by: Mauro Carvalho Chehab <mchehab@s-opensource.com>
Acked-by: Sakari Ailus <sakari.ailus@linux.intel.com>
--
Sakari Ailus
sakari.ailus@linux.intel.com
^ permalink raw reply [flat|nested] 9+ messages in thread
* [PATCH 3/5] media: vb2: Enforce VB2_MAX_FRAME in vb2_core_reqbufs better
2017-12-28 16:29 [PATCH 0/5] some VB2 bug fixes and improvements Mauro Carvalho Chehab
2017-12-28 16:29 ` [PATCH 1/5] media: vb2: don't go out of the buffer range Mauro Carvalho Chehab
2017-12-28 16:29 ` [PATCH 2/5] media: vb2: Fix a bug about unnecessary calls to queue cancel and free Mauro Carvalho Chehab
@ 2017-12-28 16:29 ` Mauro Carvalho Chehab
2017-12-28 16:29 ` [PATCH 4/5] media: vb2: add pr_fmt() macro Mauro Carvalho Chehab
2017-12-28 16:29 ` [PATCH 5/5] media: vb2: add a new warning about pending buffers Mauro Carvalho Chehab
4 siblings, 0 replies; 9+ messages in thread
From: Mauro Carvalho Chehab @ 2017-12-28 16:29 UTC (permalink / raw)
To: Linux Media Mailing List
Cc: Sakari Ailus, Mauro Carvalho Chehab, Hans Verkuil,
Christophe JAILLET, Hirokazu Honda, Stanimir Varbanov,
Satendra Singh Thakur, Mauro Carvalho Chehab
From: Sakari Ailus <sakari.ailus@linux.intel.com>
The check for the number of buffers requested against the maximum,
VB2_MAX_FRAME, was performed before checking queue's minimum number of
buffers. Reverse the order, thus ensuring that under no circumstances
num_buffers exceeds VB2_MAX_FRAME here.
Also add a warning of the condition.
Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com>
Signed-off-by: Mauro Carvalho Chehab <mchehab@s-opensource.com>
---
drivers/media/common/videobuf/videobuf2-core.c | 5 +++--
1 file changed, 3 insertions(+), 2 deletions(-)
diff --git a/drivers/media/common/videobuf/videobuf2-core.c b/drivers/media/common/videobuf/videobuf2-core.c
index 1793bdb1fe54..ba04103f2f32 100644
--- a/drivers/media/common/videobuf/videobuf2-core.c
+++ b/drivers/media/common/videobuf/videobuf2-core.c
@@ -700,8 +700,9 @@ int vb2_core_reqbufs(struct vb2_queue *q, enum vb2_memory memory,
/*
* Make sure the requested values and current defaults are sane.
*/
- num_buffers = min_t(unsigned int, *count, VB2_MAX_FRAME);
- num_buffers = max_t(unsigned int, num_buffers, q->min_buffers_needed);
+ WARN_ON(q->min_buffers_needed > VB2_MAX_FRAME);
+ num_buffers = max_t(unsigned int, *count, q->min_buffers_needed);
+ num_buffers = min_t(unsigned int, num_buffers, VB2_MAX_FRAME);
memset(q->alloc_devs, 0, sizeof(q->alloc_devs));
q->memory = memory;
--
2.14.3
^ permalink raw reply related [flat|nested] 9+ messages in thread
* [PATCH 4/5] media: vb2: add pr_fmt() macro
2017-12-28 16:29 [PATCH 0/5] some VB2 bug fixes and improvements Mauro Carvalho Chehab
` (2 preceding siblings ...)
2017-12-28 16:29 ` [PATCH 3/5] media: vb2: Enforce VB2_MAX_FRAME in vb2_core_reqbufs better Mauro Carvalho Chehab
@ 2017-12-28 16:29 ` Mauro Carvalho Chehab
2017-12-28 18:26 ` Sakari Ailus
2017-12-28 16:29 ` [PATCH 5/5] media: vb2: add a new warning about pending buffers Mauro Carvalho Chehab
4 siblings, 1 reply; 9+ messages in thread
From: Mauro Carvalho Chehab @ 2017-12-28 16:29 UTC (permalink / raw)
To: Linux Media Mailing List
Cc: Mauro Carvalho Chehab, Mauro Carvalho Chehab, Hans Verkuil,
Sakari Ailus, Christophe JAILLET, Satendra Singh Thakur,
Gustavo Padovan, Stanimir Varbanov
Simplify the pr_foo() macros by adding a pr_fmt() macro.
Signed-off-by: Mauro Carvalho Chehab <mchehab@s-opensource.com>
---
drivers/media/common/videobuf/videobuf2-core.c | 30 ++++++++++++++------------
1 file changed, 16 insertions(+), 14 deletions(-)
diff --git a/drivers/media/common/videobuf/videobuf2-core.c b/drivers/media/common/videobuf/videobuf2-core.c
index ba04103f2f32..195942bf8fde 100644
--- a/drivers/media/common/videobuf/videobuf2-core.c
+++ b/drivers/media/common/videobuf/videobuf2-core.c
@@ -14,6 +14,8 @@
* the Free Software Foundation.
*/
+#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
+
#include <linux/err.h>
#include <linux/kernel.h>
#include <linux/module.h>
@@ -32,10 +34,10 @@
static int debug;
module_param(debug, int, 0644);
-#define dprintk(level, fmt, arg...) \
- do { \
- if (debug >= level) \
- pr_info("vb2-core: %s: " fmt, __func__, ## arg); \
+#define dprintk(level, fmt, arg...) \
+ do { \
+ if (debug >= level) \
+ pr_info("%s: " fmt, __func__, ## arg); \
} while (0)
#ifdef CONFIG_VIDEO_ADV_DEBUG
@@ -463,12 +465,12 @@ static int __vb2_queue_free(struct vb2_queue *q, unsigned int buffers)
q->cnt_wait_prepare != q->cnt_wait_finish;
if (unbalanced || debug) {
- pr_info("vb2: counters for queue %p:%s\n", q,
+ pr_info("counters for queue %p:%s\n", q,
unbalanced ? " UNBALANCED!" : "");
- pr_info("vb2: setup: %u start_streaming: %u stop_streaming: %u\n",
+ pr_info(" setup: %u start_streaming: %u stop_streaming: %u\n",
q->cnt_queue_setup, q->cnt_start_streaming,
q->cnt_stop_streaming);
- pr_info("vb2: wait_prepare: %u wait_finish: %u\n",
+ pr_info(" wait_prepare: %u wait_finish: %u\n",
q->cnt_wait_prepare, q->cnt_wait_finish);
}
q->cnt_queue_setup = 0;
@@ -489,23 +491,23 @@ static int __vb2_queue_free(struct vb2_queue *q, unsigned int buffers)
vb->cnt_buf_init != vb->cnt_buf_cleanup;
if (unbalanced || debug) {
- pr_info("vb2: counters for queue %p, buffer %d:%s\n",
+ pr_info(" counters for queue %p, buffer %d:%s\n",
q, buffer, unbalanced ? " UNBALANCED!" : "");
- pr_info("vb2: buf_init: %u buf_cleanup: %u buf_prepare: %u buf_finish: %u\n",
+ pr_info(" buf_init: %u buf_cleanup: %u buf_prepare: %u buf_finish: %u\n",
vb->cnt_buf_init, vb->cnt_buf_cleanup,
vb->cnt_buf_prepare, vb->cnt_buf_finish);
- pr_info("vb2: buf_queue: %u buf_done: %u\n",
+ pr_info(" buf_queue: %u buf_done: %u\n",
vb->cnt_buf_queue, vb->cnt_buf_done);
- pr_info("vb2: alloc: %u put: %u prepare: %u finish: %u mmap: %u\n",
+ pr_info(" alloc: %u put: %u prepare: %u finish: %u mmap: %u\n",
vb->cnt_mem_alloc, vb->cnt_mem_put,
vb->cnt_mem_prepare, vb->cnt_mem_finish,
vb->cnt_mem_mmap);
- pr_info("vb2: get_userptr: %u put_userptr: %u\n",
+ pr_info(" get_userptr: %u put_userptr: %u\n",
vb->cnt_mem_get_userptr, vb->cnt_mem_put_userptr);
- pr_info("vb2: attach_dmabuf: %u detach_dmabuf: %u map_dmabuf: %u unmap_dmabuf: %u\n",
+ pr_info(" attach_dmabuf: %u detach_dmabuf: %u map_dmabuf: %u unmap_dmabuf: %u\n",
vb->cnt_mem_attach_dmabuf, vb->cnt_mem_detach_dmabuf,
vb->cnt_mem_map_dmabuf, vb->cnt_mem_unmap_dmabuf);
- pr_info("vb2: get_dmabuf: %u num_users: %u vaddr: %u cookie: %u\n",
+ pr_info(" get_dmabuf: %u num_users: %u vaddr: %u cookie: %u\n",
vb->cnt_mem_get_dmabuf,
vb->cnt_mem_num_users,
vb->cnt_mem_vaddr,
--
2.14.3
^ permalink raw reply related [flat|nested] 9+ messages in thread* Re: [PATCH 4/5] media: vb2: add pr_fmt() macro
2017-12-28 16:29 ` [PATCH 4/5] media: vb2: add pr_fmt() macro Mauro Carvalho Chehab
@ 2017-12-28 18:26 ` Sakari Ailus
0 siblings, 0 replies; 9+ messages in thread
From: Sakari Ailus @ 2017-12-28 18:26 UTC (permalink / raw)
To: Mauro Carvalho Chehab
Cc: Linux Media Mailing List, Mauro Carvalho Chehab, Hans Verkuil,
Christophe JAILLET, Satendra Singh Thakur, Gustavo Padovan,
Stanimir Varbanov
On Thu, Dec 28, 2017 at 02:29:37PM -0200, Mauro Carvalho Chehab wrote:
> Simplify the pr_foo() macros by adding a pr_fmt() macro.
>
> Signed-off-by: Mauro Carvalho Chehab <mchehab@s-opensource.com>
On 4th and 5th:
Acked-by: Sakari Ailus <sakari.ailus@linux.intel.com>
--
Sakari Ailus
sakari.ailus@linux.intel.com
^ permalink raw reply [flat|nested] 9+ messages in thread
* [PATCH 5/5] media: vb2: add a new warning about pending buffers
2017-12-28 16:29 [PATCH 0/5] some VB2 bug fixes and improvements Mauro Carvalho Chehab
` (3 preceding siblings ...)
2017-12-28 16:29 ` [PATCH 4/5] media: vb2: add pr_fmt() macro Mauro Carvalho Chehab
@ 2017-12-28 16:29 ` Mauro Carvalho Chehab
4 siblings, 0 replies; 9+ messages in thread
From: Mauro Carvalho Chehab @ 2017-12-28 16:29 UTC (permalink / raw)
To: Linux Media Mailing List
Cc: Mauro Carvalho Chehab, Mauro Carvalho Chehab, Hans Verkuil,
Sakari Ailus, Christophe JAILLET, Gustavo Padovan, Hirokazu Honda
There's a logic at the VB2 core that produces a WARN_ON if
there are still buffers waiting to be filled. However, it doesn't
indicate what buffers are still opened, with makes harder to
identify issues inside caller drivers.
So, add a new pr_warn() pointing to such buffers. That, together
with debug instrumentation inside the drivers can make easier to
identify where the problem is.
Signed-off-by: Mauro Carvalho Chehab <mchehab@s-opensource.com>
---
drivers/media/common/videobuf/videobuf2-core.c | 5 ++++-
1 file changed, 4 insertions(+), 1 deletion(-)
diff --git a/drivers/media/common/videobuf/videobuf2-core.c b/drivers/media/common/videobuf/videobuf2-core.c
index 195942bf8fde..c82c1e3157a4 100644
--- a/drivers/media/common/videobuf/videobuf2-core.c
+++ b/drivers/media/common/videobuf/videobuf2-core.c
@@ -1657,8 +1657,11 @@ static void __vb2_queue_cancel(struct vb2_queue *q)
*/
if (WARN_ON(atomic_read(&q->owned_by_drv_count))) {
for (i = 0; i < q->num_buffers; ++i)
- if (q->bufs[i]->state == VB2_BUF_STATE_ACTIVE)
+ if (q->bufs[i]->state == VB2_BUF_STATE_ACTIVE) {
+ pr_warn("driver bug: stop_streaming operation is leaving buf %p in active state\n",
+ q->bufs[i]);
vb2_buffer_done(q->bufs[i], VB2_BUF_STATE_ERROR);
+ }
/* Must be zero now */
WARN_ON(atomic_read(&q->owned_by_drv_count));
}
--
2.14.3
^ permalink raw reply related [flat|nested] 9+ messages in thread