Worth to mention we got this issue happened also for v3d (a fix was
already submitted).
Reviewed-by: Juan A. Suarez <jasuarez@xxxxxxxxxx>
On Fri, 2024-10-04 at 09:36 -0300, Maíra Canal wrote:
> Upon closing the file descriptor, the active performance monitor is
> not
> stopped. Although all perfmons are destroyed in
> `vc4_perfmon_close_file()`,
> the active performance monitor's pointer (`vc4->active_perfmon`) is
> still
> retained.
>
> If we open a new file descriptor and submit a few jobs with
> performance
> monitors, the driver will attempt to stop the active performance
> monitor
> using the stale pointer in `vc4->active_perfmon`. However, this
> pointer
> is no longer valid because the previous process has already
> terminated,
> and all performance monitors associated with it have been destroyed
> and
> freed.
>
> To fix this, when the active performance monitor belongs to a given
> process, explicitly stop it before destroying and freeing it.
>
> Cc: <stable@xxxxxxxxxxxxxxx> # v4.17+
> Cc: Boris Brezillon <bbrezillon@xxxxxxxxxx>
> Cc: Juan A. Suarez Romero <jasuarez@xxxxxxxxxx>
> Fixes: 65101d8c9108 ("drm/vc4: Expose performance counters to
> userspace")
> Signed-off-by: Maíra Canal <mcanal@xxxxxxxxxx>
> ---
> drivers/gpu/drm/vc4/vc4_perfmon.c | 7 ++++++-
> 1 file changed, 6 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/vc4/vc4_perfmon.c
> b/drivers/gpu/drm/vc4/vc4_perfmon.c
> index f2e56d0f6298..f1342f917cf7 100644
> --- a/drivers/gpu/drm/vc4/vc4_perfmon.c
> +++ b/drivers/gpu/drm/vc4/vc4_perfmon.c
> @@ -116,6 +116,11 @@ void vc4_perfmon_open_file(struct vc4_file
> *vc4file)
> static int vc4_perfmon_idr_del(int id, void *elem, void *data)
> {
> struct vc4_perfmon *perfmon = elem;
> + struct vc4_dev *vc4 = (struct vc4_dev *)data;
> +
> + /* If the active perfmon is being destroyed, stop it first
> */
> + if (perfmon == vc4->active_perfmon)
> + vc4_perfmon_stop(vc4, perfmon, false);
>
> vc4_perfmon_put(perfmon);
>
> @@ -130,7 +135,7 @@ void vc4_perfmon_close_file(struct vc4_file
> *vc4file)
> return;
>
> mutex_lock(&vc4file->perfmon.lock);
> - idr_for_each(&vc4file->perfmon.idr, vc4_perfmon_idr_del,
> NULL);
> + idr_for_each(&vc4file->perfmon.idr, vc4_perfmon_idr_del,
> vc4);
> idr_destroy(&vc4file->perfmon.idr);
> mutex_unlock(&vc4file->perfmon.lock);
> mutex_destroy(&vc4file->perfmon.lock);
