mbox series

[v2,0/4] Improvement around mtk_vcodec_mem_free() logging and usage

Message ID 20231221092226.1395427-1-fshao@chromium.org
Headers show
Series Improvement around mtk_vcodec_mem_free() logging and usage | expand

Message

Fei Shao Dec. 21, 2023, 9:17 a.m. UTC
Hi,

This series includes some improvements around mtk_vcodec_mem_free() in
mtk_vcodec_util.c.

I noticed that mtk_vcodec_mem_free() generates a spurious error if the
target DMA buffer has been freed previously:
  mtk_vcodec_mem_free(),69: [MTK_V4L2][ERROR] 18000000.video-codec
  dma_free size=0 failed!

The warning is harmless, but it brings some confusion to our developers
and testing infra, so I sent this series to fix that with some minor
improvement for the driver alongside.

The first two patches are for aesthetic and style improvements, the
third tries to make the error logs more informative, and the last adds
back the missing checks when calling the free function.

Regards,
Fei

v1: https://lore.kernel.org/all/20231113123049.4117280-1-fshao@chromium.org/

Changes in v2:
- rebased on top of next-20231219
- revise commit message for clearer intention and rationale
- update the error messages based on the suggestion

Fei Shao (4):
  media: mediatek: vcodec: Replace dev_name in error string
  media: mediatek: vcodec: Drop unnecessary variable
  media: mediatek: vcodec: Update mtk_vcodec_mem_free() error messages
  media: mediatek: vcodec: Only free buffer VA that is not NULL

 .../mediatek/vcodec/common/mtk_vcodec_util.c  | 23 +++++++++----------
 .../vcodec/decoder/vdec/vdec_av1_req_lat_if.c | 22 ++++++++++++------
 .../vcodec/encoder/venc/venc_h264_if.c        |  5 ++--
 3 files changed, 29 insertions(+), 21 deletions(-)

Comments

AngeloGioacchino Del Regno Dec. 21, 2023, 11:43 a.m. UTC | #1
Il 21/12/23 10:17, Fei Shao ha scritto:
> In mtk_vcodec_mem_free(), there are two cases where a NULL VA is passed:
> - mem->size == 0: we are called to free no memory. This may happen when
>    we call mtk_vcodec_mem_free() twice or the memory has never been
>    allocated.
> - mem->size > 0: we are called to free memory but without VA. This means
>    that we failed to free the memory for real.
> 
> Both cases are not expected to happen, and we want to have clearer error
> messages to describe which one we just encountered.
> Update the error messages to include more information for that purpose.
> 
> Signed-off-by: Fei Shao <fshao@chromium.org>

Reviewed-by: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>
Fei Shao March 27, 2024, 9:18 a.m. UTC | #2
On Thu, Dec 21, 2023 at 5:23 PM Fei Shao <fshao@chromium.org> wrote:
>
> Hi,
>
> This series includes some improvements around mtk_vcodec_mem_free() in
> mtk_vcodec_util.c.
>
> I noticed that mtk_vcodec_mem_free() generates a spurious error if the
> target DMA buffer has been freed previously:
>   mtk_vcodec_mem_free(),69: [MTK_V4L2][ERROR] 18000000.video-codec
>   dma_free size=0 failed!
>
> The warning is harmless, but it brings some confusion to our developers
> and testing infra, so I sent this series to fix that with some minor
> improvement for the driver alongside.
>
> The first two patches are for aesthetic and style improvements, the
> third tries to make the error logs more informative, and the last adds
> back the missing checks when calling the free function.
>
> Regards,
> Fei
>
> v1: https://lore.kernel.org/all/20231113123049.4117280-1-fshao@chromium.org/
>
> Changes in v2:
> - rebased on top of next-20231219
> - revise commit message for clearer intention and rationale
> - update the error messages based on the suggestion
>
> Fei Shao (4):
>   media: mediatek: vcodec: Replace dev_name in error string
>   media: mediatek: vcodec: Drop unnecessary variable
>   media: mediatek: vcodec: Update mtk_vcodec_mem_free() error messages
>   media: mediatek: vcodec: Only free buffer VA that is not NULL
>
>  .../mediatek/vcodec/common/mtk_vcodec_util.c  | 23 +++++++++----------
>  .../vcodec/decoder/vdec/vdec_av1_req_lat_if.c | 22 ++++++++++++------
>  .../vcodec/encoder/venc/venc_h264_if.c        |  5 ++--
>  3 files changed, 29 insertions(+), 21 deletions(-)

Hi Hans,

I'd like to get some attention for this series in case it has been
overlooked. It still applies to next-20240327 without conflict.
Please let me know if you have any feedback or concerns on this.

Thank you,
Fei
>
> --
> 2.43.0.472.g3155946c3a-goog
>
Sebastian Fricke March 27, 2024, 9:57 a.m. UTC | #3
Hey Fei,

On 27.03.2024 17:18, Fei Shao wrote:
>On Thu, Dec 21, 2023 at 5:23 PM Fei Shao <fshao@chromium.org> wrote:
>>
>> Hi,
>>
>> This series includes some improvements around mtk_vcodec_mem_free() in
>> mtk_vcodec_util.c.
>>
>> I noticed that mtk_vcodec_mem_free() generates a spurious error if the
>> target DMA buffer has been freed previously:
>>   mtk_vcodec_mem_free(),69: [MTK_V4L2][ERROR] 18000000.video-codec
>>   dma_free size=0 failed!
>>
>> The warning is harmless, but it brings some confusion to our developers
>> and testing infra, so I sent this series to fix that with some minor
>> improvement for the driver alongside.
>>
>> The first two patches are for aesthetic and style improvements, the
>> third tries to make the error logs more informative, and the last adds
>> back the missing checks when calling the free function.
>>
>> Regards,
>> Fei
>>
>> v1: https://lore.kernel.org/all/20231113123049.4117280-1-fshao@chromium.org/
>>
>> Changes in v2:
>> - rebased on top of next-20231219
>> - revise commit message for clearer intention and rationale
>> - update the error messages based on the suggestion
>>
>> Fei Shao (4):
>>   media: mediatek: vcodec: Replace dev_name in error string
>>   media: mediatek: vcodec: Drop unnecessary variable
>>   media: mediatek: vcodec: Update mtk_vcodec_mem_free() error messages
>>   media: mediatek: vcodec: Only free buffer VA that is not NULL
>>
>>  .../mediatek/vcodec/common/mtk_vcodec_util.c  | 23 +++++++++----------
>>  .../vcodec/decoder/vdec/vdec_av1_req_lat_if.c | 22 ++++++++++++------
>>  .../vcodec/encoder/venc/venc_h264_if.c        |  5 ++--
>>  3 files changed, 29 insertions(+), 21 deletions(-)
>
>Hi Hans,
>
>I'd like to get some attention for this series in case it has been
>overlooked. It still applies to next-20240327 without conflict.
>Please let me know if you have any feedback or concerns on this.

Sorry this seems to have slipped through my fingers, I'll put it on my
list. Thanks for the notification.

>
>Thank you,
>Fei

Greetings,
Sebastian

>>
>> --
>> 2.43.0.472.g3155946c3a-goog
>>
>