Message ID | 20231220103713.113386-11-sakari.ailus@linux.intel.com (mailing list archive) |
---|---|
State | Changes Requested |
Delegated to: | Sakari Ailus |
Headers |
Received: from ny.mirrors.kernel.org ([147.75.199.223]) by www.linuxtv.org with esmtps (TLS1.3:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.92) (envelope-from <linux-media+bounces-2771-patchwork=linuxtv.org@vger.kernel.org>) id 1rFtxm-00H48S-4m for patchwork@linuxtv.org; Wed, 20 Dec 2023 10:38:06 +0000 Received: from smtp.subspace.kernel.org (wormhole.subspace.kernel.org [52.25.139.140]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by ny.mirrors.kernel.org (Postfix) with ESMTPS id 3C3E51C224FF for <patchwork@linuxtv.org>; Wed, 20 Dec 2023 10:38:03 +0000 (UTC) Received: from localhost.localdomain (localhost.localdomain [127.0.0.1]) by smtp.subspace.kernel.org (Postfix) with ESMTP id EC5A121109; Wed, 20 Dec 2023 10:37:27 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=intel.com header.i=@intel.com header.b="RkTbjYp+" X-Original-To: linux-media@vger.kernel.org Received: from mgamail.intel.com (mgamail.intel.com [198.175.65.10]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id 6783220DF3 for <linux-media@vger.kernel.org>; Wed, 20 Dec 2023 10:37:26 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=linux.intel.com Authentication-Results: smtp.subspace.kernel.org; spf=none smtp.mailfrom=linux.intel.com DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=intel.com; i=@intel.com; q=dns/txt; s=Intel; t=1703068646; x=1734604646; h=from:to:cc:subject:date:message-id:in-reply-to: references:mime-version:content-transfer-encoding; bh=05/GurQC+tFTFg0EilvY/mjumAa9KUU6C9I+HxauEVg=; b=RkTbjYp+x0HZOB9kwu/p8s3P33KvqvDhOs4PZtjO2kXwctQbpuUPTtB7 vnioeZnuRU5KlehkPR9ttlacqE2Wmv3r32bvCeWKy+KUGz0YE1F/Ssq1f UDWThwHHxzqd+kVLTXq7qFV3ap4KLYgGZulJDCFGP/ahUOjLRniURpYSB CwqYZgeG5M5gCVuOa5Ukhzp0Gz/D919zpf0npSzjorj+Mw9LAxtqA1xAC 17r8AQC9r+NHy0DLwXznTakty+Aj/QPGdQv+icHJd43nXn2IrTGRp/dBP X2oY2FCpQjrT4Sfh3oj4oUDvkimHYOVAsTwhfj7V2Ht5L8PH70w1lmh/J A==; X-IronPort-AV: E=McAfee;i="6600,9927,10929"; a="9174336" X-IronPort-AV: E=Sophos;i="6.04,291,1695711600"; d="scan'208";a="9174336" Received: from orsmga007.jf.intel.com ([10.7.209.58]) by orvoesa102.jf.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 20 Dec 2023 02:37:24 -0800 X-ExtLoop1: 1 X-IronPort-AV: E=McAfee;i="6600,9927,10929"; a="769544250" X-IronPort-AV: E=Sophos;i="6.04,291,1695711600"; d="scan'208";a="769544250" Received: from turnipsi.fi.intel.com (HELO kekkonen.fi.intel.com) ([10.237.72.44]) by orsmga007-auth.jf.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 20 Dec 2023 02:37:23 -0800 Received: from svinhufvud.ger.corp.intel.com (localhost [IPv6:::1]) by kekkonen.fi.intel.com (Postfix) with ESMTP id 6F8C61201D0; Wed, 20 Dec 2023 12:37:20 +0200 (EET) From: Sakari Ailus <sakari.ailus@linux.intel.com> To: linux-media@vger.kernel.org Cc: laurent.pinchart@ideasonboard.com, Hans Verkuil <hverkuil-cisco@xs4all.nl> Subject: [PATCH v2 10/29] media: mc: Delete character device early Date: Wed, 20 Dec 2023 12:36:54 +0200 Message-Id: <20231220103713.113386-11-sakari.ailus@linux.intel.com> X-Mailer: git-send-email 2.39.2 In-Reply-To: <20231220103713.113386-1-sakari.ailus@linux.intel.com> References: <20231220103713.113386-1-sakari.ailus@linux.intel.com> Precedence: bulk X-Mailing-List: linux-media@vger.kernel.org List-Id: <linux-media.vger.kernel.org> List-Subscribe: <mailto:linux-media+subscribe@vger.kernel.org> List-Unsubscribe: <mailto:linux-media+unsubscribe@vger.kernel.org> MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-LSpam-Score: -4.7 (----) X-LSpam-Report: No, score=-4.7 required=5.0 tests=BAYES_00=-1.9,DKIMWL_WL_HIGH=0.001,DKIM_SIGNED=0.1,DKIM_VALID=-0.1,HEADER_FROM_DIFFERENT_DOMAINS=0.5,MAILING_LIST_MULTI=-1,RCVD_IN_DNSWL_MED=-2.3 autolearn=ham autolearn_force=no |
Series |
Media device lifetime management
|
|
Commit Message
Sakari Ailus
Dec. 20, 2023, 10:36 a.m. UTC
The parent of the character device related to the media devnode is the media devnode. Thus the character device needs to be released before the media devnode's release function. Move it to unregistering of the media devnode, which mirrors adding the character device in conjunction with registering the media devnode. Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com> Acked-by: Hans Verkuil <hverkuil-cisco@xs4all.nl> --- drivers/media/mc/mc-devnode.c | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-)
Comments
Hi Sakari, Thank you for the patch. On Wed, Dec 20, 2023 at 12:36:54PM +0200, Sakari Ailus wrote: > The parent of the character device related to the media devnode is the > media devnode. Thus the character device needs to be released before the > media devnode's release function. Move it to unregistering of the media > devnode, which mirrors adding the character device in conjunction with > registering the media devnode. > > Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com> > Acked-by: Hans Verkuil <hverkuil-cisco@xs4all.nl> > --- > drivers/media/mc/mc-devnode.c | 4 +--- > 1 file changed, 1 insertion(+), 3 deletions(-) > > diff --git a/drivers/media/mc/mc-devnode.c b/drivers/media/mc/mc-devnode.c > index 7e22938dfd81..8bc7450ac144 100644 > --- a/drivers/media/mc/mc-devnode.c > +++ b/drivers/media/mc/mc-devnode.c > @@ -51,9 +51,6 @@ static void media_devnode_release(struct device *cd) > > mutex_lock(&media_devnode_lock); > > - /* Delete the cdev on this minor as well */ > - cdev_del(&devnode->cdev); > - > /* Mark device node number as free */ > clear_bit(devnode->minor, media_devnode_nums); Should this be moved to media_devnode_unregister() too ? It can be done in a separate patch. > > @@ -270,6 +267,7 @@ void media_devnode_unregister(struct media_devnode *devnode) > clear_bit(MEDIA_FLAG_REGISTERED, &devnode->flags); > mutex_unlock(&media_devnode_lock); > > + cdev_del(&devnode->cdev); I initially wondered if this could raise with the cdev access in media_open(), as the media_devnode_lock is released just before calling cdev_dev(), but my understanding is that the dev/open race is properly handled in the cdev layer. Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> I wonder if a similar change in v4l2-dev.c would be beneficial. > device_unregister(&devnode->dev); > } >
Hi Laurent, On Wed, Feb 07, 2024 at 12:08:10PM +0200, Laurent Pinchart wrote: > Hi Sakari, > > Thank you for the patch. > > On Wed, Dec 20, 2023 at 12:36:54PM +0200, Sakari Ailus wrote: > > The parent of the character device related to the media devnode is the > > media devnode. Thus the character device needs to be released before the > > media devnode's release function. Move it to unregistering of the media > > devnode, which mirrors adding the character device in conjunction with > > registering the media devnode. > > > > Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com> > > Acked-by: Hans Verkuil <hverkuil-cisco@xs4all.nl> > > --- > > drivers/media/mc/mc-devnode.c | 4 +--- > > 1 file changed, 1 insertion(+), 3 deletions(-) > > > > diff --git a/drivers/media/mc/mc-devnode.c b/drivers/media/mc/mc-devnode.c > > index 7e22938dfd81..8bc7450ac144 100644 > > --- a/drivers/media/mc/mc-devnode.c > > +++ b/drivers/media/mc/mc-devnode.c > > @@ -51,9 +51,6 @@ static void media_devnode_release(struct device *cd) > > > > mutex_lock(&media_devnode_lock); > > > > - /* Delete the cdev on this minor as well */ > > - cdev_del(&devnode->cdev); > > - > > /* Mark device node number as free */ > > clear_bit(devnode->minor, media_devnode_nums); > > Should this be moved to media_devnode_unregister() too ? It can be done > in a separate patch. Good question. Yes, I think that seems reasonable. The minor isn't needed after unregistering the device. > > > > > @@ -270,6 +267,7 @@ void media_devnode_unregister(struct media_devnode *devnode) > > clear_bit(MEDIA_FLAG_REGISTERED, &devnode->flags); > > mutex_unlock(&media_devnode_lock); > > > > + cdev_del(&devnode->cdev); > > I initially wondered if this could raise with the cdev access in > media_open(), as the media_devnode_lock is released just before calling > cdev_dev(), but my understanding is that the dev/open race is properly > handled in the cdev layer. > > Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> Thanks! > > I wonder if a similar change in v4l2-dev.c would be beneficial. Quite possibly. Let's see...
diff --git a/drivers/media/mc/mc-devnode.c b/drivers/media/mc/mc-devnode.c index 7e22938dfd81..8bc7450ac144 100644 --- a/drivers/media/mc/mc-devnode.c +++ b/drivers/media/mc/mc-devnode.c @@ -51,9 +51,6 @@ static void media_devnode_release(struct device *cd) mutex_lock(&media_devnode_lock); - /* Delete the cdev on this minor as well */ - cdev_del(&devnode->cdev); - /* Mark device node number as free */ clear_bit(devnode->minor, media_devnode_nums); @@ -270,6 +267,7 @@ void media_devnode_unregister(struct media_devnode *devnode) clear_bit(MEDIA_FLAG_REGISTERED, &devnode->flags); mutex_unlock(&media_devnode_lock); + cdev_del(&devnode->cdev); device_unregister(&devnode->dev); }