| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <e5f560b2e9edd6115f5c8904e1ca5730eac1c49f.camel@collabora.com>
Date: Thu, 11 Sep 2025 08:23:28 -0400
From: Nicolas Dufresne <nicolas.dufresne@...labora.com>
To: "jackson.lee" <jackson.lee@...psnmedia.com>, "mchehab@...nel.org"
<mchehab@...nel.org>, "hverkuil-cisco@...all.nl"
<hverkuil-cisco@...all.nl>, "bob.beckett@...labora.com"
<bob.beckett@...labora.com>
Cc: "linux-media@...r.kernel.org" <linux-media@...r.kernel.org>,
"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
"lafley.kim" <lafley.kim@...psnmedia.com>, "b-brnich@...com"
<b-brnich@...com>, "hverkuil@...all.nl" <hverkuil@...all.nl>, Nas Chung
<nas.chung@...psnmedia.com>
Subject: Re: [PATCH v3 0/4] Performance improvement of decoder
Hi Jackson,
Le jeudi 11 septembre 2025 à 02:04 +0000, jackson.lee a écrit :
> Hi Nicolas
>
> > -----Original Message-----
> > From: Nicolas Dufresne <nicolas.dufresne@...labora.com>
> > Sent: Wednesday, September 10, 2025 9:57 PM
> > To: jackson.lee <jackson.lee@...psnmedia.com>; mchehab@...nel.org;
> > hverkuil-cisco@...all.nl; bob.beckett@...labora.com
> > Cc: linux-media@...r.kernel.org; linux-kernel@...r.kernel.org; lafley.kim
> > <lafley.kim@...psnmedia.com>; b-brnich@...com; hverkuil@...all.nl; Nas
> > Chung <nas.chung@...psnmedia.com>
> > Subject: Re: [PATCH v3 0/4] Performance improvement of decoder
> >
> > Hi Jackson,
> >
> > Le mercredi 10 septembre 2025 à 06:59 +0000, jackson.lee a écrit :
> > [...]
> >
> > > I have reproduced the stall problem, I can see it with the latest
> > Gstreamer version.
> > > The root cause is we checked an incorrect return value while flushing,
> > so in spite of not finished flushing, the checking loop if the flushing
> > was finished was exited.
> > > When stop streaming was called and the instance queue count was 1, the
> > checking function put infinite loop, so the stall problem happened.
> > >
> > > The below patch should be needed.
> > >
> > > diff --git a/drivers/media/platform/chips-media/wave5/wave5-vpuapi.c
> > > b/drivers/media/platform/chips-media/wave5/wave5-vpuapi.c
> > > index edbe69540ef1..2e0128cd0e4d 100644
> > > --- a/drivers/media/platform/chips-media/wave5/wave5-vpuapi.c
> > > +++ b/drivers/media/platform/chips-media/wave5/wave5-vpuapi.c
> > > @@ -52,6 +52,7 @@ int wave5_vpu_init_with_bitcode(struct device *dev,
> > > u8 *bitcode, size_t size)
> > > int wave5_vpu_flush_instance(struct vpu_instance *inst)
> > > {
> > > int ret = 0;
> > > + int mutex_ret = 0;
> > > int retry = 0;
> > >
> > > ret = mutex_lock_interruptible(&inst->dev->hw_lock);
> > > @@ -80,9 +81,9 @@ int wave5_vpu_flush_instance(struct vpu_instance
> > > *inst)
> > >
> > > mutex_unlock(&inst->dev->hw_lock);
> > > wave5_vpu_dec_get_output_info(inst,
> > > &dec_info);
> > > - ret =
> > > mutex_lock_interruptible(&inst->dev->hw_lock);
> > > - if (ret)
> > > - return ret;
> > > + mutex_ret =
> > > +mutex_lock_interruptible(&inst->dev->hw_lock);
> > > + if (mutex_ret)
> > > + return mutex_ret;
> > > if (dec_info.index_frame_display > 0)
> > > wave5_vpu_dec_set_disp_flag(inst,
> > > dec_info.index_frame_display);
> > > }
> >
> > Good catch, unfortunately it does not completely fix the problem for me.
> > You can find a the end of this message the patch I actually tested. Note
> > I ,ove the mutex_ret in a close scope, and fixed other occurence of this
> > pattern, except one that I highlighted to you with a FIXME.
> >
> > Some new information, I had this trace from GStreamer when the bug occured
> > on forward seeks (very rare):
>
> " gst-play-1.0 --audiosink=fakesink --videosink=fakevideosink long-drc.h264"
>
> When the above command is executed, to do forward seeks, I can use "shift key + '+'key " this key combination?
Note that I saw your next message stating you have managed to reproduce, this is
great.
The +/- changes the playback speed, if you did not enable instant rate changes,
this will effectively cause a seek, so its quite likely you can reproduce with
that way too.
What I was using is left/right arrows. I would just seek forward (right arrow)
close to the end, and then press and hold the left arrow till it reaches
position 0. Rince and repeat.
regards,
Nicolas
>
> Thanks
> Jackson
>
> >
> > ** (gst-play-1.0:604): WARNING **: 00:03:59.965: v4l2h264dec0: Too old
> > frames, bug in decoder -- please file a bug
> >
> > [root@...into nicolas]# echo w > /proc/sysrq-trigger [ 335.116289] sysrq:
> > Show Blocked State
> > [ 335.120054] task:typefind:sink state:D stack:0 pid:607 tgid:604
> > ppid:543 task_flags:0x40044c flags:0x00000019
> > [ 335.131147] Call trace:
> > [ 335.133584] __switch_to+0xf0/0x1c0 (T) [ 335.137442]
> > __schedule+0x35c/0x9bc [ 335.140935] schedule+0x34/0x110 [ 335.144162]
> > schedule_timeout+0x80/0x104 [ 335.148081]
> > wait_for_completion_timeout+0x74/0x158
> > [ 335.152955] wave5_vpu_wait_interrupt+0x28/0x60 [wave5] [ 335.158252]
> > wave5_vpu_dec_stop_streaming+0x68/0x28c [wave5] [ 335.163915]
> > __vb2_queue_cancel+0x2c/0x2d4 [videobuf2_common] [ 335.169668]
> > vb2_core_queue_release+0x20/0x74 [videobuf2_common] [ 335.175678]
> > vb2_queue_release+0x10/0x1c [videobuf2_v4l2] [ 335.181081]
> > v4l2_m2m_ctx_release+0x20/0x40 [v4l2_mem2mem] [ 335.186567]
> > wave5_vpu_release_device+0x44/0x150 [wave5] [ 335.191879]
> > wave5_vpu_dec_release+0x20/0x2c [wave5] [ 335.196841]
> > v4l2_release+0xb4/0xf0 [videodev] [ 335.201709] __fput+0xd0/0x2e0
> > [ 335.205090] ____fput+0x14/0x20 [ 335.208468] task_work_run+0x64/0xd4
> > [ 335.212164] do_exit+0x240/0x8e0 [ 335.215552] do_group_exit+0x30/0xa4
> > [ 335.219177] get_signal+0x790/0x860 [ 335.222676] do_signal+0x94/0x394
> > [ 335.225986] do_notify_resume+0xd0/0x14c [ 335.229910]
> > el0_svc+0xe4/0xe8 [ 335.232967] el0t_64_sync_handler+0xa0/0xe4
> > [ 335.237154] el0t_64_sync+0x198/0x19c
> >
> > regards,
> > Nicolas
> >
> > ---
> >
> > diff --git a/drivers/media/platform/chips-media/wave5/wave5-vpuapi.c
> > b/drivers/media/platform/chips-media/wave5/wave5-vpuapi.c
> > index edbe69540ef1e..2faca2eee41fe 100644
> > --- a/drivers/media/platform/chips-media/wave5/wave5-vpuapi.c
> > +++ b/drivers/media/platform/chips-media/wave5/wave5-vpuapi.c
> > @@ -77,12 +77,13 @@ int wave5_vpu_flush_instance(struct vpu_instance *inst)
> > return -ETIMEDOUT;
> > } else if (ret == -EBUSY) {
> > struct dec_output_info dec_info;
> > + int ret_mutex;
> >
> > mutex_unlock(&inst->dev->hw_lock);
> > wave5_vpu_dec_get_output_info(inst, &dec_info);
> > - ret = mutex_lock_interruptible(&inst->dev->hw_lock);
> > - if (ret)
> > - return ret;
> > + ret_mutex = mutex_lock_interruptible(&inst->dev-
> > > hw_lock);
> > + if (ret_mutex)
> > + return ret_mutex;
> > if (dec_info.index_frame_display > 0)
> > wave5_vpu_dec_set_disp_flag(inst,
> > dec_info.index_frame_display);
> > }
> > @@ -222,6 +223,8 @@ int wave5_vpu_dec_close(struct vpu_instance *inst, u32
> > *fail_res)
> > }
> >
> > do {
> > + int ret_mutex;
> > +
> > ret = wave5_vpu_dec_finish_seq(inst, fail_res);
> > if (ret < 0 && *fail_res != WAVE5_SYSERR_VPU_STILL_RUNNING) {
> > dev_warn(inst->dev->dev, "dec_finish_seq timed out\n");
> > @@ -243,10 +246,10 @@ int wave5_vpu_dec_close(struct vpu_instance *inst,
> > u32 *fail_res)
> >
> > mutex_unlock(&vpu_dev->hw_lock);
> > wave5_vpu_dec_get_output_info(inst, &dec_info);
> > - ret = mutex_lock_interruptible(&vpu_dev->hw_lock);
> > - if (ret) {
> > + ret_mutex = mutex_lock_interruptible(&vpu_dev->hw_lock);
> > + if (ret_mutex) {
> > pm_runtime_put_sync(inst->dev->dev);
> > - return ret;
> > + return ret_mutex;
> > }
> > } while (ret != 0);
> >
> > @@ -482,6 +485,7 @@ dma_addr_t wave5_vpu_dec_get_rd_ptr(struct
> > vpu_instance *inst)
> >
> > ret = mutex_lock_interruptible(&inst->dev->hw_lock);
> > if (ret)
> > + // FIXME this return type is wrong
> > return ret;
> >
> > rd_ptr = wave5_dec_get_rd_ptr(inst);
> > nicolas@...tebuilder:~/Sources/TI/jacinto/linux$ git diff diff --git
> > a/drivers/media/platform/chips-media/wave5/wave5-vpuapi.c
> > b/drivers/media/platform/chips-media/wave5/wave5-vpuapi.c
> > index edbe69540ef1e..2faca2eee41fe 100644
> > --- a/drivers/media/platform/chips-media/wave5/wave5-vpuapi.c
> > +++ b/drivers/media/platform/chips-media/wave5/wave5-vpuapi.c
> > @@ -77,12 +77,13 @@ int wave5_vpu_flush_instance(struct vpu_instance *inst)
> > return -ETIMEDOUT;
> > } else if (ret == -EBUSY) {
> > struct dec_output_info dec_info;
> > + int ret_mutex;
> >
> > mutex_unlock(&inst->dev->hw_lock);
> > wave5_vpu_dec_get_output_info(inst, &dec_info);
> > - ret = mutex_lock_interruptible(&inst->dev->hw_lock);
> > - if (ret)
> > - return ret;
> > + ret_mutex = mutex_lock_interruptible(&inst->dev-
> > > hw_lock);
> > + if (ret_mutex)
> > + return ret_mutex;
> > if (dec_info.index_frame_display > 0)
> > wave5_vpu_dec_set_disp_flag(inst,
> > dec_info.index_frame_display);
> > }
> > @@ -222,6 +223,8 @@ int wave5_vpu_dec_close(struct vpu_instance *inst, u32
> > *fail_res)
> > }
> >
> > do {
> > + int ret_mutex;
> > +
> > ret = wave5_vpu_dec_finish_seq(inst, fail_res);
> > if (ret < 0 && *fail_res != WAVE5_SYSERR_VPU_STILL_RUNNING) {
> > dev_warn(inst->dev->dev, "dec_finish_seq timed out\n");
> > @@ -243,10 +246,10 @@ int wave5_vpu_dec_close(struct vpu_instance *inst,
> > u32 *fail_res)
> >
> > mutex_unlock(&vpu_dev->hw_lock);
> > wave5_vpu_dec_get_output_info(inst, &dec_info);
> > - ret = mutex_lock_interruptible(&vpu_dev->hw_lock);
> > - if (ret) {
> > + ret_mutex = mutex_lock_interruptible(&vpu_dev->hw_lock);
> > + if (ret_mutex) {
> > pm_runtime_put_sync(inst->dev->dev);
> > - return ret;
> > + return ret_mutex;
> > }
> > } while (ret != 0);
> >
> > @@ -482,6 +485,7 @@ dma_addr_t wave5_vpu_dec_get_rd_ptr(struct
> > vpu_instance *inst)
> >
> > ret = mutex_lock_interruptible(&inst->dev->hw_lock);
> > if (ret)
> > + // FIXME this return type is wrong
> > return ret;
> >
> > rd_ptr = wave5_dec_get_rd_ptr(inst);
Download attachment "signature.asc" of type "application/pgp-signature" (229 bytes)
Powered by blists - more mailing lists