Re: Race in rcar-v4l2.c

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 06/09/2024 16:46, Niklas Söderlund wrote:
Hello Tomi,

Thanks for testing that series.

On 2024-09-06 15:28:21 +0300, Tomi Valkeinen wrote:
On 06/09/2024 14:27, Tomi Valkeinen wrote:
Hi,

On 06/09/2024 14:14, Tomi Valkeinen wrote:
Hi Niklas,

On 06/09/2024 13:14, Niklas Söderlund wrote:
Hi Tomi,

Thanks for your report.

I have an on-going series trying to clean this all up [1]. The one
change in the v4l-async core I proposed was however rejected and I have
yet to circle back to figure out a different solution.

Could you give it a try and see if it also solves this issue?

1. [PATCH 0/6] media: rcar-vin: Make use of multiple connections
in v4l-async
     https://lore.kernel.org/linux-renesas-
soc/20240129202254.1126012-1-niklas.soderlund+renesas@xxxxxxxxxxxx/

The compilation fails (broken in media: rcar-vin: Simplify remote
source type detection) with:

drivers/media/platform/renesas/rcar-vin/rcar-dma.c:767:24: error:
‘struct rvin_dev’ has no member named ‘is_csi’

If I hack past that, I don't see the warnings anymore. But if I'm
not mistaken, rvin_release() is not called at all anymore. I can
also see plenty of leaks with kmemleak. Those seem to originate from
max96712, but... I don't see max96712's remove() getting called
either when I remove the module.

FWIW module unloading is tricky and AFIK not supported upstream. There
is a reason the VIN capture pipeline have .suppress_bind_attrs = true
;-)

Module unloading is tricky, but only if you define "supported" as supported in production, with all possible scenarios working fine.

Unloading modules, at least in my use cases (mostly DRM and V4L2, on many different platforms) works very well, if you just make sure to unload in the correct order. It's a very powerful development time option to use, speeding up development a lot, and highlighting resource management issues.

So, anyone breaking module unloading will be getting emails from me... =)


Sorry, never mind that. I had the debug print in max96714... =). So
max96712 remove() gets called, but it's missing:

+       v4l2_subdev_cleanup(&priv->sd);
+       media_entity_cleanup(&priv->sd.entity);
+       v4l2_ctrl_handler_free(&priv->ctrl_handler);

But now I'm seeing rvin_release() getting called (no warnings). I'm not
sure what changed. Maybe I had some extra changes lying around. Let me
test the series a bit more...

I haven't seen the warning anymore, so I believe your series indeed fixes
the issue.

Awesome! I will try to find some time soon and rebase that series, I
think I have an idea on how to avoid having to mock around in the
v4l-async core and still achieve the same thing.

Hmm, do you mean "[PATCH] media: v4l: async: Fix completion of chained subnotifiers"? I only now noticed that. I did not apply that... Should I have?

 Tomi





[Index of Archives]     [Linux Input]     [Video for Linux]     [Gstreamer Embedded]     [Mplayer Users]     [Linux USB Devel]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [Yosemite Backpacking]

  Powered by Linux