From: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
To: Julien Massot <julien.massot@collabora.com>
Cc: "Sakari Ailus" <sakari.ailus@linux.intel.com>,
"Mauro Carvalho Chehab" <mchehab@kernel.org>,
"Tomi Valkeinen" <tomi.valkeinen@ideasonboard.com>,
"Jacopo Mondi" <jacopo+renesas@jmondi.org>,
"Kieran Bingham" <kieran.bingham+renesas@ideasonboard.com>,
"Niklas Söderlund" <niklas.soderlund+renesas@ragnatech.se>,
"Benjamin Mugnier" <benjamin.mugnier@foss.st.com>,
"Sylvain Petinot" <sylvain.petinot@foss.st.com>,
"Yong Zhi" <yong.zhi@intel.com>,
"Bingbu Cao" <bingbu.cao@intel.com>,
"Dan Scally" <djrscally@gmail.com>,
"Tianshu Qiu" <tian.shu.qiu@intel.com>,
"Eugen Hristev" <eugen.hristev@collabora.com>,
"Nicolas Ferre" <nicolas.ferre@microchip.com>,
"Alexandre Belloni" <alexandre.belloni@bootlin.com>,
"Claudiu Beznea" <claudiu.beznea@tuxon.dev>,
"Maxime Ripard" <mripard@kernel.org>,
"Rui Miguel Silva" <rmfrfs@gmail.com>,
"Martin Kepplinger" <martink@posteo.de>,
"Purism Kernel Team" <kernel@puri.sm>,
"Shawn Guo" <shawnguo@kernel.org>,
"Sascha Hauer" <s.hauer@pengutronix.de>,
"Pengutronix Kernel Team" <kernel@pengutronix.de>,
"Fabio Estevam" <festevam@gmail.com>,
"Robert Foss" <rfoss@kernel.org>,
"Todor Tomov" <todor.too@gmail.com>,
"Bryan O'Donoghue" <bryan.odonoghue@linaro.org>,
"Bjorn Andersson" <andersson@kernel.org>,
"Konrad Dybcio" <konrad.dybcio@linaro.org>,
"Fabrizio Castro" <fabrizio.castro.jz@renesas.com>,
"Dafna Hirschfeld" <dafna@fastmail.com>,
"Heiko Stuebner" <heiko@sntech.de>,
"Sylwester Nawrocki" <s.nawrocki@samsung.com>,
"Krzysztof Kozlowski" <krzysztof.kozlowski@linaro.org>,
"Alim Akhtar" <alim.akhtar@samsung.com>,
"Hugues Fruchet" <hugues.fruchet@foss.st.com>,
"Alain Volmat" <alain.volmat@foss.st.com>,
"Maxime Coquelin" <mcoquelin.stm32@gmail.com>,
"Alexandre Torgue" <alexandre.torgue@foss.st.com>,
"Chen-Yu Tsai" <wens@csie.org>,
"Jernej Skrabec" <jernej.skrabec@gmail.com>,
"Samuel Holland" <samuel@sholland.org>,
"Yong Deng" <yong.deng@magewell.com>,
"Paul Kocialkowski" <paul.kocialkowski@bootlin.com>,
"Benoit Parrot" <bparrot@ti.com>, "Jai Luthra" <j-luthra@ti.com>,
"Philipp Zabel" <p.zabel@pengutronix.de>,
"Michal Simek" <michal.simek@amd.com>,
"Greg Kroah-Hartman" <gregkh@linuxfoundation.org>,
"Thierry Reding" <thierry.reding@gmail.com>,
"Jonathan Hunter" <jonathanh@nvidia.com>,
"Sowjanya Komatineni" <skomatineni@nvidia.com>,
"Luca Ceresoli" <luca.ceresoli@bootlin.com>,
linux-media@vger.kernel.org, linux-kernel@vger.kernel.org,
linux-arm-kernel@lists.infradead.org, imx@lists.linux.dev,
linux-arm-msm@vger.kernel.org, linux-renesas-soc@vger.kernel.org,
linux-rockchip@lists.infradead.org,
linux-samsung-soc@vger.kernel.org,
linux-stm32@st-md-mailman.stormreply.com,
linux-sunxi@lists.linux.dev, linux-staging@lists.linux.dev,
linux-tegra@vger.kernel.org
Subject: Re: [PATCH 0/2] Introduce v4l2_async_nf_unregister_cleanup
Date: Thu, 2 May 2024 18:56:26 +0300 [thread overview]
Message-ID: <20240502155626.GD15807@pendragon.ideasonboard.com> (raw)
In-Reply-To: <20240502-master-v1-0-8bd109c6a3ba@collabora.com>
Hi Julien,
On Thu, May 02, 2024 at 05:22:20PM +0200, Julien Massot wrote:
> Many drivers has
> v4l2_async_nf_unregister(¬ifier);
> v4l2_async_nf_cleanup(¬ifier);
>
> Introduce a helper function to call both functions in one line.
Does this really go in the right direction ? For other objects (video
devices, media devices, ...), the unregistration should be done at
.remove() time, and the cleanup at .release() time (the operation called
when the last reference to the object is released). This is needed to
ensure proper lifetime management of the objects, and avoid a
use-after-free for objects that can be reached from userspace.
It could be argued that the notifier isn't exposed to userspace, but can
we guarantee that no driver will have a need to access the notifier in a
code path triggered by a userspace operation ? I think it would be safer
to adopt the same split for the nofifier unregistration and cleanup. In
my opinion using the same rule across different APIs also make it easier
for driver authors and for reviewers to get it right.
As shown by your series, lots of drivers call v4l2_async_nf_cleanup()
and .remove() time instead of .release(). That's because most drivers
get lifetime management wrong and don't even implement .release().
That's something Sakari is addressing with ongoing work. This patch
series seems to go in the opposite direction.
> ---
> Julien Massot (2):
> media: v4l: async: Add v4l2_async_nf_unregister_cleanup
> media: convert all drivers to use v4l2_async_nf_unregister_cleanup
>
> drivers/media/i2c/ds90ub913.c | 10 ++--------
> drivers/media/i2c/ds90ub953.c | 10 ++--------
> drivers/media/i2c/ds90ub960.c | 10 ++--------
> drivers/media/i2c/max9286.c | 3 +--
> drivers/media/i2c/st-mipid02.c | 6 ++----
> drivers/media/i2c/tc358746.c | 3 +--
> drivers/media/pci/intel/ipu3/ipu3-cio2.c | 6 ++----
> drivers/media/pci/intel/ipu6/ipu6-isys.c | 8 +-------
> drivers/media/pci/intel/ivsc/mei_csi.c | 6 ++----
> drivers/media/platform/atmel/atmel-isi.c | 3 +--
> drivers/media/platform/cadence/cdns-csi2rx.c | 6 ++----
> drivers/media/platform/intel/pxa_camera.c | 3 +--
> drivers/media/platform/marvell/mcam-core.c | 6 ++----
> drivers/media/platform/microchip/microchip-csi2dc.c | 3 +--
> drivers/media/platform/microchip/microchip-isc-base.c | 6 ++----
> drivers/media/platform/nxp/imx-mipi-csis.c | 6 ++----
> drivers/media/platform/nxp/imx7-media-csi.c | 3 +--
> drivers/media/platform/nxp/imx8-isi/imx8-isi-core.c | 3 +--
> drivers/media/platform/nxp/imx8mq-mipi-csi2.c | 6 ++----
> drivers/media/platform/qcom/camss/camss.c | 3 +--
> drivers/media/platform/renesas/rcar-csi2.c | 6 ++----
> drivers/media/platform/renesas/rcar-isp.c | 6 ++----
> drivers/media/platform/renesas/rcar-vin/rcar-core.c | 9 +++------
> drivers/media/platform/renesas/rcar_drif.c | 3 +--
> drivers/media/platform/renesas/renesas-ceu.c | 4 +---
> drivers/media/platform/renesas/rzg2l-cru/rzg2l-core.c | 3 +--
> drivers/media/platform/renesas/rzg2l-cru/rzg2l-csi2.c | 6 ++----
> drivers/media/platform/rockchip/rkisp1/rkisp1-dev.c | 3 +--
> drivers/media/platform/samsung/exynos4-is/media-dev.c | 3 +--
> drivers/media/platform/st/stm32/stm32-dcmi.c | 3 +--
> .../media/platform/st/stm32/stm32-dcmipp/dcmipp-core.c | 3 +--
> drivers/media/platform/sunxi/sun4i-csi/sun4i_csi.c | 3 +--
> .../media/platform/sunxi/sun6i-csi/sun6i_csi_bridge.c | 3 +--
> .../platform/sunxi/sun6i-mipi-csi2/sun6i_mipi_csi2.c | 3 +--
> .../sunxi/sun8i-a83t-mipi-csi2/sun8i_a83t_mipi_csi2.c | 3 +--
> drivers/media/platform/ti/am437x/am437x-vpfe.c | 3 +--
> drivers/media/platform/ti/cal/cal.c | 8 +-------
> drivers/media/platform/ti/davinci/vpif_capture.c | 3 +--
> drivers/media/platform/ti/j721e-csi2rx/j721e-csi2rx.c | 10 ++--------
> drivers/media/platform/ti/omap3isp/isp.c | 3 +--
> drivers/media/platform/video-mux.c | 3 +--
> drivers/media/platform/xilinx/xilinx-vipp.c | 3 +--
> drivers/staging/media/deprecated/atmel/atmel-isc-base.c | 6 ++----
> drivers/staging/media/sunxi/sun6i-isp/sun6i_isp_proc.c | 3 +--
> drivers/staging/media/tegra-video/vi.c | 3 +--
> include/media/v4l2-async.h | 17 +++++++++++++++++
> 46 files changed, 80 insertions(+), 153 deletions(-)
> ---
> base-commit: 843a9f4a7a85988f2f3af98adf21797c2fd05ab1
> change-id: 20240502-master-5deee133b4f5
--
Regards,
Laurent Pinchart
next prev parent reply other threads:[~2024-05-02 15:56 UTC|newest]
Thread overview: 8+ messages / expand[flat|nested] mbox.gz Atom feed top
2024-05-02 15:22 [PATCH 0/2] Introduce v4l2_async_nf_unregister_cleanup Julien Massot
2024-05-02 15:22 ` [PATCH 1/2] media: v4l: async: Add v4l2_async_nf_unregister_cleanup Julien Massot
2024-05-02 15:22 ` [PATCH 2/2] media: convert all drivers to use v4l2_async_nf_unregister_cleanup Julien Massot
2024-05-02 15:56 ` Laurent Pinchart [this message]
2024-05-02 16:01 ` [PATCH 0/2] Introduce v4l2_async_nf_unregister_cleanup Sakari Ailus
2024-05-02 16:08 ` Laurent Pinchart
2024-05-02 16:24 ` Sakari Ailus
2024-05-04 14:18 ` Laurent Pinchart
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=20240502155626.GD15807@pendragon.ideasonboard.com \
--to=laurent.pinchart@ideasonboard.com \
--cc=alain.volmat@foss.st.com \
--cc=alexandre.belloni@bootlin.com \
--cc=alexandre.torgue@foss.st.com \
--cc=alim.akhtar@samsung.com \
--cc=andersson@kernel.org \
--cc=benjamin.mugnier@foss.st.com \
--cc=bingbu.cao@intel.com \
--cc=bparrot@ti.com \
--cc=bryan.odonoghue@linaro.org \
--cc=claudiu.beznea@tuxon.dev \
--cc=dafna@fastmail.com \
--cc=djrscally@gmail.com \
--cc=eugen.hristev@collabora.com \
--cc=fabrizio.castro.jz@renesas.com \
--cc=festevam@gmail.com \
--cc=gregkh@linuxfoundation.org \
--cc=heiko@sntech.de \
--cc=hugues.fruchet@foss.st.com \
--cc=imx@lists.linux.dev \
--cc=j-luthra@ti.com \
--cc=jacopo+renesas@jmondi.org \
--cc=jernej.skrabec@gmail.com \
--cc=jonathanh@nvidia.com \
--cc=julien.massot@collabora.com \
--cc=kernel@pengutronix.de \
--cc=kernel@puri.sm \
--cc=kieran.bingham+renesas@ideasonboard.com \
--cc=konrad.dybcio@linaro.org \
--cc=krzysztof.kozlowski@linaro.org \
--cc=linux-arm-kernel@lists.infradead.org \
--cc=linux-arm-msm@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-media@vger.kernel.org \
--cc=linux-renesas-soc@vger.kernel.org \
--cc=linux-rockchip@lists.infradead.org \
--cc=linux-samsung-soc@vger.kernel.org \
--cc=linux-staging@lists.linux.dev \
--cc=linux-stm32@st-md-mailman.stormreply.com \
--cc=linux-sunxi@lists.linux.dev \
--cc=linux-tegra@vger.kernel.org \
--cc=luca.ceresoli@bootlin.com \
--cc=martink@posteo.de \
--cc=mchehab@kernel.org \
--cc=mcoquelin.stm32@gmail.com \
--cc=michal.simek@amd.com \
--cc=mripard@kernel.org \
--cc=nicolas.ferre@microchip.com \
--cc=niklas.soderlund+renesas@ragnatech.se \
--cc=p.zabel@pengutronix.de \
--cc=paul.kocialkowski@bootlin.com \
--cc=rfoss@kernel.org \
--cc=rmfrfs@gmail.com \
--cc=s.hauer@pengutronix.de \
--cc=s.nawrocki@samsung.com \
--cc=sakari.ailus@linux.intel.com \
--cc=samuel@sholland.org \
--cc=shawnguo@kernel.org \
--cc=skomatineni@nvidia.com \
--cc=sylvain.petinot@foss.st.com \
--cc=thierry.reding@gmail.com \
--cc=tian.shu.qiu@intel.com \
--cc=todor.too@gmail.com \
--cc=tomi.valkeinen@ideasonboard.com \
--cc=wens@csie.org \
--cc=yong.deng@magewell.com \
--cc=yong.zhi@intel.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;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).