From: Sakari Ailus <sakari.ailus@linux.intel.com>
To: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Cc: linux-media@vger.kernel.org, Hans Verkuil <hverkuil-cisco@xs4all.nl>
Subject: Re: [PATCH v2 19/29] media: omap3isp: Release the isp device struct by media device callback
Date: Wed, 5 Jun 2024 09:23:16 +0000 [thread overview]
Message-ID: <ZmAuhOIKX2x4SMux@kekkonen.localdomain> (raw)
In-Reply-To: <20240207142311.GQ23702@pendragon.ideasonboard.com>
Hi Laurent,
Thanks for the review.
On Wed, Feb 07, 2024 at 04:23:11PM +0200, Laurent Pinchart wrote:
> Hi Sakari,
>
> Thank you for the patch.
>
> On Wed, Dec 20, 2023 at 12:37:03PM +0200, Sakari Ailus wrote:
> > Use the media device release callback to release the isp device's data
> > structure. This approach has the benefit of not releasing memory which may
> > still be accessed through open file handles whilst the isp driver is being
> > unbound.
> >
> > Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com>
> > Acked-by: Hans Verkuil <hverkuil-cisco@xs4all.nl>
> > ---
> > drivers/media/platform/ti/omap3isp/isp.c | 24 +++++++++++++++++++-----
> > 1 file changed, 19 insertions(+), 5 deletions(-)
> >
> > diff --git a/drivers/media/platform/ti/omap3isp/isp.c b/drivers/media/platform/ti/omap3isp/isp.c
> > index 1cda23244c7b..ef6a781d6da1 100644
> > --- a/drivers/media/platform/ti/omap3isp/isp.c
> > +++ b/drivers/media/platform/ti/omap3isp/isp.c
> > @@ -649,8 +649,11 @@ static irqreturn_t isp_isr(int irq, void *_isp)
> > return IRQ_HANDLED;
> > }
> >
> > +static void isp_release(struct media_device *mdev);
> > +
>
> Forward declarations are not nice :-( Any hope to reorder functions to
> fix this ?
This depends on moving isp_cleanup_modules up, too. If you think it's the
lesser evil I'm fine with that.
>
> > static const struct media_device_ops isp_media_ops = {
> > .link_notify = v4l2_pipeline_link_notify,
> > + .release = isp_release,
> > };
> >
> > /* -----------------------------------------------------------------------------
> > @@ -1607,7 +1610,6 @@ static void isp_unregister_entities(struct isp_device *isp)
> > omap3isp_stat_unregister_entities(&isp->isp_hist);
> >
> > v4l2_device_unregister(&isp->v4l2_dev);
> > - media_device_cleanup(&isp->media_dev);
> > }
> >
> > static int isp_link_entity(
> > @@ -1955,6 +1957,19 @@ static void isp_detach_iommu(struct isp_device *isp)
> > #endif
> > }
> >
> > +static void isp_release(struct media_device *mdev)
> > +{
> > + struct isp_device *isp =
> > + container_of(mdev, struct isp_device, media_dev);
> > +
> > + isp_cleanup_modules(isp);
> > +
> > + media_entity_enum_cleanup(&isp->crashed);
> > + v4l2_async_nf_cleanup(&isp->notifier);
>
> This duplicates the call in isp_remove().
I'll drop the one in isp_remove().
>
> > +
> > + kfree(isp);
> > +}
> > +
> > static int isp_attach_iommu(struct isp_device *isp)
> > {
> > #ifdef CONFIG_ARM_DMA_USE_IOMMU
> > @@ -2004,16 +2019,15 @@ static void isp_remove(struct platform_device *pdev)
> > v4l2_async_nf_unregister(&isp->notifier);
> > v4l2_async_nf_cleanup(&isp->notifier);
> > isp_unregister_entities(isp);
> > - isp_cleanup_modules(isp);
> > +
> > isp_xclk_cleanup(isp);
> >
> > __omap3isp_get(isp, false);
> > isp_detach_iommu(isp);
> > __omap3isp_put(isp, false);
> >
> > - media_entity_enum_cleanup(&isp->crashed);
> > -
> > - kfree(isp);
> > + /* May release isp immediately */
> > + media_device_put(&isp->media_dev);
> > }
> >
> > enum isp_of_phy {
>
--
Kind regards,
Sakari Ailus
next prev parent reply other threads:[~2024-06-05 9:23 UTC|newest]
Thread overview: 95+ messages / expand[flat|nested] mbox.gz Atom feed top
2023-12-20 10:36 [PATCH v2 00/29] Media device lifetime management Sakari Ailus
2023-12-20 10:36 ` [PATCH v2 01/29] Revert "[media] media: fix media devnode ioctl/syscall and unregister race" Sakari Ailus
2023-12-20 10:36 ` [PATCH v2 02/29] Revert "media: utilize new cdev_device_add helper function" Sakari Ailus
2023-12-20 10:36 ` [PATCH v2 03/29] Revert "[media] media: fix use-after-free in cdev_put() when app exits after driver unbind" Sakari Ailus
2023-12-20 10:36 ` [PATCH v2 04/29] media: mc: utilize new cdev_device_add helper function Sakari Ailus
2024-02-07 9:38 ` Laurent Pinchart
2024-02-07 9:51 ` Laurent Pinchart
2024-02-21 12:55 ` Sakari Ailus
2023-12-20 10:36 ` [PATCH v2 05/29] Revert "media: uvcvideo: Refactor teardown of uvc on USB disconnect" Sakari Ailus
2023-12-20 10:36 ` [PATCH v2 06/29] Revert "[media] media-device: dynamically allocate struct media_devnode" Sakari Ailus
2023-12-20 10:36 ` [PATCH v2 07/29] media: uvcvideo: Refactor teardown of uvc on USB disconnect Sakari Ailus
2023-12-20 10:36 ` [PATCH v2 08/29] media: mc: Drop nop release callback Sakari Ailus
2024-02-07 9:55 ` Laurent Pinchart
2023-12-20 10:36 ` [PATCH v2 09/29] media: mc: Do not call cdev_device_del() if cdev_device_add() fails Sakari Ailus
2024-02-07 9:57 ` Laurent Pinchart
2024-03-05 8:13 ` Sakari Ailus
2023-12-20 10:36 ` [PATCH v2 10/29] media: mc: Delete character device early Sakari Ailus
2024-02-07 10:08 ` Laurent Pinchart
2024-03-05 8:52 ` Sakari Ailus
2023-12-20 10:36 ` [PATCH v2 11/29] media: mc: Split initialising and adding media devnode Sakari Ailus
2024-02-07 10:46 ` Laurent Pinchart
2024-03-05 8:59 ` Sakari Ailus
2023-12-20 10:36 ` [PATCH v2 12/29] media: mc: Shuffle functions around Sakari Ailus
2024-02-07 10:47 ` Laurent Pinchart
2023-12-20 10:36 ` [PATCH v2 13/29] media: mc: Initialise media devnode in media_device_init() Sakari Ailus
2024-02-07 10:51 ` Laurent Pinchart
2023-12-20 10:36 ` [PATCH v2 14/29] media: mc: Refactor media devnode minor clearing Sakari Ailus
2024-02-05 14:46 ` Hans Verkuil
2024-02-07 10:53 ` Laurent Pinchart
2023-12-20 10:36 ` [PATCH v2 15/29] media: mc: Unassign minor only if it has been assigned Sakari Ailus
2024-02-05 14:48 ` Hans Verkuil
2024-02-07 10:58 ` Laurent Pinchart
2024-02-21 9:24 ` Sakari Ailus
2023-12-20 10:37 ` [PATCH v2 16/29] media: mc: Refcount the media device Sakari Ailus
2024-02-07 11:08 ` Laurent Pinchart
2024-03-07 10:37 ` Sakari Ailus
2023-12-20 10:37 ` [PATCH v2 17/29] media: v4l: Acquire a reference to the media device for every video device Sakari Ailus
2024-02-05 14:56 ` Hans Verkuil
2024-02-07 11:13 ` Laurent Pinchart
2024-02-21 10:43 ` Sakari Ailus
2024-02-21 12:19 ` Laurent Pinchart
2024-02-21 12:35 ` Sakari Ailus
2024-02-21 10:40 ` Sakari Ailus
2024-02-21 10:51 ` Hans Verkuil
2024-02-21 11:44 ` Sakari Ailus
2024-03-05 7:43 ` Sakari Ailus
2024-03-05 7:46 ` Hans Verkuil
2023-12-20 10:37 ` [PATCH v2 18/29] media: mc: Postpone graph object removal until free Sakari Ailus
2024-02-07 14:18 ` Laurent Pinchart
2024-06-04 10:59 ` Sakari Ailus
2024-06-04 11:01 ` Sakari Ailus
2023-12-20 10:37 ` [PATCH v2 19/29] media: omap3isp: Release the isp device struct by media device callback Sakari Ailus
2024-02-07 14:23 ` Laurent Pinchart
2024-06-05 9:23 ` Sakari Ailus [this message]
2023-12-20 10:37 ` [PATCH v2 20/29] media: ipu3-cio2: Call v4l2_device_unregister() earlier Sakari Ailus
2024-02-07 14:24 ` Laurent Pinchart
2024-03-05 10:21 ` Sakari Ailus
2024-03-05 10:22 ` Sakari Ailus
2023-12-20 10:37 ` [PATCH v2 21/29] media: ipu3-cio2: Request IRQ earlier Sakari Ailus
2024-02-05 14:58 ` Hans Verkuil
2024-02-07 14:34 ` Laurent Pinchart
2024-02-21 10:51 ` Sakari Ailus
2023-12-20 10:37 ` [PATCH v2 22/29] media: ipu3-cio2: Release the cio2 device context by media device callback Sakari Ailus
2024-02-07 14:33 ` Laurent Pinchart
2024-03-07 12:23 ` Sakari Ailus
2023-12-20 10:37 ` [PATCH v2 23/29] media: vimc: Release resources on media device release Sakari Ailus
2024-02-05 15:02 ` Hans Verkuil
2024-02-07 14:38 ` Laurent Pinchart
2024-02-21 10:55 ` Sakari Ailus
2024-02-21 10:53 ` Sakari Ailus
2024-02-21 11:02 ` Laurent Pinchart
2024-02-21 11:38 ` Sakari Ailus
2024-02-21 11:19 ` Hans Verkuil
2024-02-21 11:40 ` Sakari Ailus
2024-02-21 11:48 ` Hans Verkuil
2024-02-21 12:02 ` Sakari Ailus
2023-12-20 10:37 ` [PATCH v2 24/29] media: Documentation: Document how Media device resources are released Sakari Ailus
2024-02-05 15:04 ` Hans Verkuil
2024-02-07 14:43 ` Laurent Pinchart
2024-02-21 11:37 ` Sakari Ailus
2023-12-20 10:37 ` [PATCH v2 25/29] media: mc: Add per-file-handle data support Sakari Ailus
2024-02-05 15:08 ` Hans Verkuil
2023-12-20 10:37 ` [PATCH v2 26/29] media: mc: Maintain a list of open file handles in a media device Sakari Ailus
2024-02-05 15:11 ` Hans Verkuil
2024-02-05 15:16 ` Laurent Pinchart
2024-02-05 15:32 ` Hans Verkuil
2024-02-05 15:41 ` Laurent Pinchart
2024-02-21 11:53 ` Sakari Ailus
2023-12-20 10:37 ` [PATCH v2 27/29] media: mc: Implement best effort media device removal safety sans refcount Sakari Ailus
2023-12-20 10:37 ` [PATCH v2 28/29] media: mc: Warn about drivers not releasing media device safely Sakari Ailus
2023-12-20 10:37 ` [PATCH v2 29/29] media: Documentation: Document media device memory safety helper Sakari Ailus
2023-12-20 10:52 ` [PATCH v2 00/29] Media device lifetime management Laurent Pinchart
2023-12-20 11:30 ` Sakari Ailus
2024-02-07 10:55 ` Laurent Pinchart
2024-03-07 10:57 ` Sakari Ailus
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=ZmAuhOIKX2x4SMux@kekkonen.localdomain \
--to=sakari.ailus@linux.intel.com \
--cc=hverkuil-cisco@xs4all.nl \
--cc=laurent.pinchart@ideasonboard.com \
--cc=linux-media@vger.kernel.org \
/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).