Re: [PATCH 2/3] media: meson: vdec: fix another case of VP9 buffer shortage

From: Neil Armstrong
Date: Tue May 05 2020 - 11:38:44 EST


On 05/05/2020 17:18, Mauro Carvalho Chehab wrote:
> Em Tue, 28 Apr 2020 14:50:35 +0200
> Neil Armstrong <narmstrong@xxxxxxxxxxxx> escreveu:
>
>> From: Maxime Jourdan <mjourdan@xxxxxxxxxxxx>
>>
>> - Redo the logic where VP9 gets fresh CAPTURE buffers. The previous code
>> could lead to a hardlock.
>> - Reserve 4 margin buffers instead of 3, as apparently there are corner
>> cases where 3 is not enough.
>>
>> Fixes: e9a3eb4819ca ("media: meson: vdec: add VP9 input support")
>> Fixes: 00c43088aa68 ("media: meson: vdec: add VP9 decoder support")
>> Signed-off-by: Maxime Jourdan <mjourdan@xxxxxxxxxxxx>
>> Signed-off-by: Neil Armstrong <narmstrong@xxxxxxxxxxxx>
>
>> +static struct vb2_v4l2_buffer *get_free_vbuf(struct amvdec_session *sess)
>> +{
>> + struct codec_vp9 *vp9 = sess->priv;
>> + struct vb2_v4l2_buffer *vbuf = v4l2_m2m_dst_buf_remove(sess->m2m_ctx);
>> + struct vb2_v4l2_buffer *vbuf2;
>> +
>> + if (!vbuf)
>> + return NULL;
>> +
>> + if (!codec_vp9_get_frame_by_idx(vp9, vbuf->vb2_buf.index))
>> + return vbuf;
>> +
>> + vbuf2 = get_free_vbuf(sess);
>
> Huh!!!!
>
> Never use recursive functions inside the Kernel! Kernel stack is too
> limited.
>
> Also, even if Kernel stack would be unlimited, the above logic
> would endlessly be calling get_free_vbuf(sess).

Will rework this correctly, thanks for the review.

Neil

>
> Thanks,
> Mauro
>