All of lore.kernel.org
 help / color / mirror / Atom feed
From: Mauro Carvalho Chehab <mchehab+huawei@kernel.org>
To: Neil Armstrong <narmstrong@baylibre.com>
Cc: Maxime Jourdan <mjourdan@baylibre.com>,
	linux-kernel@vger.kernel.org, hverkuil-cisco@xs4all.nl,
	linux-amlogic@lists.infradead.org,
	linux-arm-kernel@lists.infradead.org,
	linux-media@vger.kernel.org
Subject: Re: [PATCH 2/3] media: meson: vdec: fix another case of VP9 buffer shortage
Date: Tue, 5 May 2020 17:18:40 +0200	[thread overview]
Message-ID: <20200505171840.48d89ccc@coco.lan> (raw)
In-Reply-To: <20200428125036.9401-3-narmstrong@baylibre.com>

Em Tue, 28 Apr 2020 14:50:35 +0200
Neil Armstrong <narmstrong@baylibre.com> escreveu:

> From: Maxime Jourdan <mjourdan@baylibre.com>
> 
> - 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@baylibre.com>
> Signed-off-by: Neil Armstrong <narmstrong@baylibre.com>

> +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).

Thanks,
Mauro

_______________________________________________
linux-amlogic mailing list
linux-amlogic@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-amlogic

WARNING: multiple messages have this Message-ID (diff)
From: Mauro Carvalho Chehab <mchehab+huawei@kernel.org>
To: Neil Armstrong <narmstrong@baylibre.com>
Cc: Maxime Jourdan <mjourdan@baylibre.com>,
	linux-kernel@vger.kernel.org, hverkuil-cisco@xs4all.nl,
	linux-amlogic@lists.infradead.org,
	linux-arm-kernel@lists.infradead.org,
	linux-media@vger.kernel.org
Subject: Re: [PATCH 2/3] media: meson: vdec: fix another case of VP9 buffer shortage
Date: Tue, 5 May 2020 17:18:40 +0200	[thread overview]
Message-ID: <20200505171840.48d89ccc@coco.lan> (raw)
In-Reply-To: <20200428125036.9401-3-narmstrong@baylibre.com>

Em Tue, 28 Apr 2020 14:50:35 +0200
Neil Armstrong <narmstrong@baylibre.com> escreveu:

> From: Maxime Jourdan <mjourdan@baylibre.com>
> 
> - 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@baylibre.com>
> Signed-off-by: Neil Armstrong <narmstrong@baylibre.com>

> +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).

Thanks,
Mauro

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

WARNING: multiple messages have this Message-ID (diff)
From: Mauro Carvalho Chehab <mchehab+huawei@kernel.org>
To: Neil Armstrong <narmstrong@baylibre.com>
Cc: linux-media@vger.kernel.org, hverkuil-cisco@xs4all.nl,
	linux-amlogic@lists.infradead.org,
	linux-arm-kernel@lists.infradead.org,
	linux-kernel@vger.kernel.org,
	Maxime Jourdan <mjourdan@baylibre.com>
Subject: Re: [PATCH 2/3] media: meson: vdec: fix another case of VP9 buffer shortage
Date: Tue, 5 May 2020 17:18:40 +0200	[thread overview]
Message-ID: <20200505171840.48d89ccc@coco.lan> (raw)
In-Reply-To: <20200428125036.9401-3-narmstrong@baylibre.com>

Em Tue, 28 Apr 2020 14:50:35 +0200
Neil Armstrong <narmstrong@baylibre.com> escreveu:

> From: Maxime Jourdan <mjourdan@baylibre.com>
> 
> - 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@baylibre.com>
> Signed-off-by: Neil Armstrong <narmstrong@baylibre.com>

> +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).

Thanks,
Mauro

  reply	other threads:[~2020-05-05 15:18 UTC|newest]

Thread overview: 18+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-04-28 12:50 [PATCH 0/3] meson: vdec: vp9 & MAINTAINERS fixes Neil Armstrong
2020-04-28 12:50 ` Neil Armstrong
2020-04-28 12:50 ` Neil Armstrong
2020-04-28 12:50 ` [PATCH 1/3] media: meson: vdec: enable mcrcc for VP9 Neil Armstrong
2020-04-28 12:50   ` Neil Armstrong
2020-04-28 12:50   ` Neil Armstrong
2020-04-28 12:50 ` [PATCH 2/3] media: meson: vdec: fix another case of VP9 buffer shortage Neil Armstrong
2020-04-28 12:50   ` Neil Armstrong
2020-04-28 12:50   ` Neil Armstrong
2020-05-05 15:18   ` Mauro Carvalho Chehab [this message]
2020-05-05 15:18     ` Mauro Carvalho Chehab
2020-05-05 15:18     ` Mauro Carvalho Chehab
2020-05-05 15:38     ` Neil Armstrong
2020-05-05 15:38       ` Neil Armstrong
2020-05-05 15:38       ` Neil Armstrong
2020-04-28 12:50 ` [PATCH 3/3] MAINTAINERS: update the Amlogic VDEC driver maintainer entry Neil Armstrong
2020-04-28 12:50   ` Neil Armstrong
2020-04-28 12:50   ` Neil Armstrong

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20200505171840.48d89ccc@coco.lan \
    --to=mchehab+huawei@kernel.org \
    --cc=hverkuil-cisco@xs4all.nl \
    --cc=linux-amlogic@lists.infradead.org \
    --cc=linux-arm-kernel@lists.infradead.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-media@vger.kernel.org \
    --cc=mjourdan@baylibre.com \
    --cc=narmstrong@baylibre.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body. 
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.