From: "Wachowski, Karol" <karol.wachowski@linux.intel.com>
To: Andrzej Kacprowski <andrzej.kacprowski@linux.intel.com>,
dri-devel@lists.freedesktop.org
Cc: oded.gabbay@gmail.com, jeff.hugo@oss.qualcomm.com,
lizhi.hou@amd.com, dawid.osuchowski@linux.intel.com
Subject: Re: [PATCH] accel/ivpu: Make BO free helper NULL-safe
Date: Mon, 28 Sep 2026 07:54:58 +0200 [thread overview]
Message-ID: <8f2c049b-7ef4-43f1-b2f1-892294f023a6@linux.intel.com> (raw)
In-Reply-To: <20260924105906.489378-1-andrzej.kacprowski@linux.intel.com>
On 24-Sep-26 12:59, Andrzej Kacprowski wrote:
> Handle NULL buffers in ivpu_bo_free() so callers can use it as a
> kfree-like cleanup helper.
>
> Use the NULL-safe helper to simplify error paths and teardown code in
> firmware, IPC, preemption buffer, and metric streamer cleanup. This
> removes redundant conditional frees and collapses unwind labels that only
> differed by which BOs had been allocated.
>
> Signed-off-by: Andrzej Kacprowski <andrzej.kacprowski@linux.intel.com>
> ---
> drivers/accel/ivpu/ivpu_fw.c | 65 +++++++++++++++--------------------
> drivers/accel/ivpu/ivpu_gem.c | 3 ++
> drivers/accel/ivpu/ivpu_ipc.c | 11 +++---
> drivers/accel/ivpu/ivpu_job.c | 6 ++--
> drivers/accel/ivpu/ivpu_ms.c | 17 ++++-----
> 5 files changed, 44 insertions(+), 58 deletions(-)
>
> diff --git a/drivers/accel/ivpu/ivpu_fw.c b/drivers/accel/ivpu/ivpu_fw.c
> index b78b7c35cd9b..f9ddaa2420c4 100644
> --- a/drivers/accel/ivpu/ivpu_fw.c
> +++ b/drivers/accel/ivpu/ivpu_fw.c
> @@ -364,6 +364,25 @@ static void ivpu_fw_release(struct ivpu_device *vdev)
> }
>
>
> +static void ivpu_fw_mem_fini(struct ivpu_device *vdev)
> +{
> + struct ivpu_fw_info *fw = vdev->fw;
> +
> + ivpu_bo_free(fw->mem_shave_nn);
> + ivpu_bo_free(fw->mem_log_verb);
> + ivpu_bo_free(fw->mem_log_crit);
> + ivpu_bo_free(fw->mem);
> + ivpu_bo_free(fw->mem_fw_ver);
> + ivpu_bo_free(fw->mem_bp);
> +
> + fw->mem_shave_nn = NULL;
> + fw->mem_log_verb = NULL;
> + fw->mem_log_crit = NULL;
> + fw->mem = NULL;
> + fw->mem_fw_ver = NULL;
> + fw->mem_bp = NULL;
> +}
> +
> static int ivpu_fw_mem_init(struct ivpu_device *vdev)
> {
> struct ivpu_fw_info *fw = vdev->fw;
> @@ -382,7 +401,7 @@ static int ivpu_fw_mem_init(struct ivpu_device *vdev)
> if (!fw->mem_fw_ver) {
> ivpu_err(vdev, "Failed to create firmware version memory buffer\n");
> ret = -ENOMEM;
> - goto err_free_bp;
> + goto err_free;
> }
>
> fw->mem = ivpu_bo_create_runtime(vdev, fw->runtime_addr, fw->runtime_size,
> @@ -390,14 +409,14 @@ static int ivpu_fw_mem_init(struct ivpu_device *vdev)
> if (!fw->mem) {
> ivpu_err(vdev, "Failed to create firmware runtime memory buffer\n");
> ret = -ENOMEM;
> - goto err_free_fw_ver;
> + goto err_free;
> }
>
> ret = ivpu_mmu_context_set_pages_ro(vdev, &vdev->gctx, fw->read_only_addr,
> fw->read_only_size);
> if (ret) {
> ivpu_err(vdev, "Failed to set firmware image read-only\n");
> - goto err_free_fw_mem;
> + goto err_free;
> }
>
> fw->mem_log_crit = ivpu_bo_create_global(vdev, IVPU_FW_CRITICAL_BUFFER_SIZE,
> @@ -405,7 +424,7 @@ static int ivpu_fw_mem_init(struct ivpu_device *vdev)
> if (!fw->mem_log_crit) {
> ivpu_err(vdev, "Failed to create critical log buffer\n");
> ret = -ENOMEM;
> - goto err_free_fw_mem;
> + goto err_free;
> }
>
> if (ivpu_fw_log_level <= IVPU_FW_LOG_INFO)
> @@ -418,7 +437,7 @@ static int ivpu_fw_mem_init(struct ivpu_device *vdev)
> if (!fw->mem_log_verb) {
> ivpu_err(vdev, "Failed to create verbose log buffer\n");
> ret = -ENOMEM;
> - goto err_free_log_crit;
> + goto err_free;
> }
>
> if (fw->shave_nn_size) {
> @@ -427,47 +446,17 @@ static int ivpu_fw_mem_init(struct ivpu_device *vdev)
> if (!fw->mem_shave_nn) {
> ivpu_err(vdev, "Failed to create shavenn buffer\n");
> ret = -ENOMEM;
> - goto err_free_log_verb;
> + goto err_free;
> }
> }
>
> return 0;
>
> -err_free_log_verb:
> - ivpu_bo_free(fw->mem_log_verb);
> -err_free_log_crit:
> - ivpu_bo_free(fw->mem_log_crit);
> -err_free_fw_mem:
> - ivpu_bo_free(fw->mem);
> -err_free_fw_ver:
> - ivpu_bo_free(fw->mem_fw_ver);
> -err_free_bp:
> - ivpu_bo_free(fw->mem_bp);
> +err_free:
> + ivpu_fw_mem_fini(vdev);
> return ret;
> }
>
> -static void ivpu_fw_mem_fini(struct ivpu_device *vdev)
> -{
> - struct ivpu_fw_info *fw = vdev->fw;
> -
> - if (fw->mem_shave_nn) {
> - ivpu_bo_free(fw->mem_shave_nn);
> - fw->mem_shave_nn = NULL;
> - }
> -
> - ivpu_bo_free(fw->mem_log_verb);
> - ivpu_bo_free(fw->mem_log_crit);
> - ivpu_bo_free(fw->mem);
> - ivpu_bo_free(fw->mem_fw_ver);
> - ivpu_bo_free(fw->mem_bp);
> -
> - fw->mem_log_verb = NULL;
> - fw->mem_log_crit = NULL;
> - fw->mem = NULL;
> - fw->mem_fw_ver = NULL;
> - fw->mem_bp = NULL;
> -}
> -
> int ivpu_fw_init(struct ivpu_device *vdev)
> {
> int ret;
> diff --git a/drivers/accel/ivpu/ivpu_gem.c b/drivers/accel/ivpu/ivpu_gem.c
> index 4f2005a8d496..62dbc2ef6e9d 100644
> --- a/drivers/accel/ivpu/ivpu_gem.c
> +++ b/drivers/accel/ivpu/ivpu_gem.c
> @@ -471,6 +471,9 @@ struct ivpu_bo *ivpu_bo_create_global(struct ivpu_device *vdev, u64 size, u32 fl
>
> void ivpu_bo_free(struct ivpu_bo *bo)
> {
> + if (!bo)
> + return;
> +
> struct iosys_map map = IOSYS_MAP_INIT_VADDR(bo->base.vaddr);
>
> if (bo->flags & DRM_IVPU_BO_MAPPABLE) {
> diff --git a/drivers/accel/ivpu/ivpu_ipc.c b/drivers/accel/ivpu/ivpu_ipc.c
> index 8e960293b77a..79d34f9b1f07 100644
> --- a/drivers/accel/ivpu/ivpu_ipc.c
> +++ b/drivers/accel/ivpu/ivpu_ipc.c
> @@ -509,7 +509,7 @@ int ivpu_ipc_init(struct ivpu_device *vdev)
> if (!ipc->mem_rx) {
> ivpu_err(vdev, "Failed to allocate mem_rx\n");
> ret = -ENOMEM;
> - goto err_free_tx;
> + goto err_free_bo;
> }
>
> ipc->mm_tx = devm_gen_pool_create(vdev->drm.dev, __ffs(IVPU_IPC_ALIGNMENT),
> @@ -517,13 +517,13 @@ int ivpu_ipc_init(struct ivpu_device *vdev)
> if (IS_ERR(ipc->mm_tx)) {
> ret = PTR_ERR(ipc->mm_tx);
> ivpu_err(vdev, "Failed to create gen pool, %pe\n", ipc->mm_tx);
> - goto err_free_rx;
> + goto err_free_bo;
> }
>
> ret = gen_pool_add(ipc->mm_tx, ipc->mem_tx->vpu_addr, ivpu_bo_size(ipc->mem_tx), -1);
> if (ret) {
> ivpu_err(vdev, "gen_pool_add failed, ret %d\n", ret);
> - goto err_free_rx;
> + goto err_free_bo;
> }
>
> spin_lock_init(&ipc->cons_lock);
> @@ -532,14 +532,13 @@ int ivpu_ipc_init(struct ivpu_device *vdev)
> ret = drmm_mutex_init(&vdev->drm, &ipc->lock);
> if (ret) {
> ivpu_err(vdev, "Failed to initialize ipc->lock, ret %d\n", ret);
> - goto err_free_rx;
> + goto err_free_bo;
> }
> ivpu_ipc_reset(vdev);
> return 0;
>
> -err_free_rx:
> +err_free_bo:
> ivpu_bo_free(ipc->mem_rx);
> -err_free_tx:
> ivpu_bo_free(ipc->mem_tx);
> err_destroy_cache:
> kmem_cache_destroy(ipc->rx_msg_cache);
> diff --git a/drivers/accel/ivpu/ivpu_job.c b/drivers/accel/ivpu/ivpu_job.c
> index b3de5dd29d1e..c436cd418bca 100644
> --- a/drivers/accel/ivpu/ivpu_job.c
> +++ b/drivers/accel/ivpu/ivpu_job.c
> @@ -64,10 +64,8 @@ static int ivpu_preemption_buffers_create(struct ivpu_device *vdev,
> static void ivpu_preemption_buffers_free(struct ivpu_device *vdev,
> struct ivpu_file_priv *file_priv, struct ivpu_cmdq *cmdq)
> {
> - if (cmdq->primary_preempt_buf)
> - ivpu_bo_free(cmdq->primary_preempt_buf);
> - if (cmdq->secondary_preempt_buf)
> - ivpu_bo_free(cmdq->secondary_preempt_buf);
> + ivpu_bo_free(cmdq->primary_preempt_buf);
> + ivpu_bo_free(cmdq->secondary_preempt_buf);
> }
>
> static int ivpu_preemption_job_init(struct ivpu_device *vdev, struct ivpu_file_priv *file_priv,
> diff --git a/drivers/accel/ivpu/ivpu_ms.c b/drivers/accel/ivpu/ivpu_ms.c
> index cd176e77b9a0..489ab51d3341 100644
> --- a/drivers/accel/ivpu/ivpu_ms.c
> +++ b/drivers/accel/ivpu/ivpu_ms.c
> @@ -69,7 +69,7 @@ int ivpu_ms_start_ioctl(struct drm_device *dev, void *data, struct drm_file *fil
>
> ret = ivpu_jsm_metric_streamer_info(vdev, ms->mask, 0, 0, &sample_size, NULL);
> if (ret)
> - goto err_free_ms;
> + goto err_free;
>
> buf_size = PAGE_ALIGN((u64)args->read_period_samples * sample_size *
> MS_READ_PERIOD_MULTIPLIER * MS_NUM_BUFFERS);
> @@ -77,14 +77,14 @@ int ivpu_ms_start_ioctl(struct drm_device *dev, void *data, struct drm_file *fil
> ivpu_dbg(vdev, IOCTL, "Requested MS buffer size %llu exceeds range size %llu\n",
> buf_size, ivpu_hw_range_size(&vdev->hw->ranges.global));
> ret = -EINVAL;
> - goto err_free_ms;
> + goto err_free;
> }
>
> ms->bo = ivpu_bo_create_global(vdev, buf_size, DRM_IVPU_BO_CACHED | DRM_IVPU_BO_MAPPABLE);
> if (!ms->bo) {
> ivpu_dbg(vdev, IOCTL, "Failed to allocate MS buffer (size %llu)\n", buf_size);
> ret = -ENOMEM;
> - goto err_free_ms;
> + goto err_free;
> }
>
> ms->buff_size = ivpu_bo_size(ms->bo) / MS_NUM_BUFFERS;
> @@ -96,16 +96,15 @@ int ivpu_ms_start_ioctl(struct drm_device *dev, void *data, struct drm_file *fil
> ret = ivpu_jsm_metric_streamer_start(vdev, ms->mask, args->sampling_period_ns,
> ms->active_buff_vpu_addr, ms->buff_size);
> if (ret)
> - goto err_free_bo;
> + goto err_free;
>
> args->sample_size = sample_size;
> args->max_data_size = ivpu_bo_size(ms->bo);
> list_add_tail(&ms->ms_instance_node, &file_priv->ms_instance_list);
> goto unlock;
>
> -err_free_bo:
> +err_free:
> ivpu_bo_free(ms->bo);
> -err_free_ms:
> kfree(ms);
> unlock:
> mutex_unlock(&file_priv->ms_lock);
> @@ -322,10 +321,8 @@ void ivpu_ms_cleanup(struct ivpu_file_priv *file_priv)
>
> mutex_lock(&file_priv->ms_lock);
>
> - if (file_priv->ms_info_bo) {
> - ivpu_bo_free(file_priv->ms_info_bo);
> - file_priv->ms_info_bo = NULL;
> - }
> + ivpu_bo_free(file_priv->ms_info_bo);
> + file_priv->ms_info_bo = NULL;
>
> list_for_each_entry_safe(ms, tmp, &file_priv->ms_instance_list, ms_instance_node)
> free_instance(file_priv, ms);
Reviewed-by: Karol Wachowski <karol.wachowski@linux.intel.com>
prev parent reply other threads:[~2026-09-28 5:55 UTC|newest]
Thread overview: 4+ messages / expand[flat|nested] mbox.gz Atom feed top
2026-09-24 10:59 [PATCH] accel/ivpu: Make BO free helper NULL-safe Andrzej Kacprowski
2026-09-24 11:12 ` Dawid Osuchowski
2026-09-28 6:29 ` Wachowski, Karol
2026-09-28 5:54 ` Wachowski, Karol [this message]
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=8f2c049b-7ef4-43f1-b2f1-892294f023a6@linux.intel.com \
--to=karol.wachowski@linux.intel.com \
--cc=andrzej.kacprowski@linux.intel.com \
--cc=dawid.osuchowski@linux.intel.com \
--cc=dri-devel@lists.freedesktop.org \
--cc=jeff.hugo@oss.qualcomm.com \
--cc=lizhi.hou@amd.com \
--cc=oded.gabbay@gmail.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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox