From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-8.2 required=3.0 tests=HEADER_FROM_DIFFERENT_DOMAINS, INCLUDES_PATCH,MAILING_LIST_MULTI,SIGNED_OFF_BY,SPF_HELO_NONE,SPF_PASS, URIBL_BLOCKED,USER_AGENT_SANE_1 autolearn=unavailable autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id C7FC1C33CB7 for ; Wed, 29 Jan 2020 16:45:55 +0000 (UTC) Received: from gabe.freedesktop.org (gabe.freedesktop.org [131.252.210.177]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mail.kernel.org (Postfix) with ESMTPS id A54D1207FD for ; Wed, 29 Jan 2020 16:45:55 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org A54D1207FD Authentication-Results: mail.kernel.org; dmarc=none (p=none dis=none) header.from=ravnborg.org Authentication-Results: mail.kernel.org; spf=none smtp.mailfrom=dri-devel-bounces@lists.freedesktop.org Received: from gabe.freedesktop.org (localhost [127.0.0.1]) by gabe.freedesktop.org (Postfix) with ESMTP id 455776E400; Wed, 29 Jan 2020 16:45:53 +0000 (UTC) Received: from asavdk4.altibox.net (asavdk4.altibox.net [109.247.116.15]) by gabe.freedesktop.org (Postfix) with ESMTPS id 511C56E3F9; Wed, 29 Jan 2020 16:45:51 +0000 (UTC) Received: from ravnborg.org (unknown [158.248.194.18]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by asavdk4.altibox.net (Postfix) with ESMTPS id 30107804E7; Wed, 29 Jan 2020 17:45:46 +0100 (CET) Date: Wed, 29 Jan 2020 17:45:45 +0100 From: Sam Ravnborg To: Daniel Vetter Subject: Re: [PATCH 4/5] drm: Push drm_global_mutex locking in drm_open Message-ID: <20200129164545.GA22331@ravnborg.org> References: <20200129082410.1691996-1-daniel.vetter@ffwll.ch> <20200129082410.1691996-5-daniel.vetter@ffwll.ch> MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: <20200129082410.1691996-5-daniel.vetter@ffwll.ch> User-Agent: Mutt/1.10.1 (2018-07-13) X-CMAE-Score: 0 X-CMAE-Analysis: v=2.3 cv=VcLZwmh9 c=1 sm=1 tr=0 a=UWs3HLbX/2nnQ3s7vZ42gw==:117 a=UWs3HLbX/2nnQ3s7vZ42gw==:17 a=jpOVt7BSZ2e4Z31A5e1TngXxSK0=:19 a=kj9zAlcOel0A:10 a=taGs_qngAAAA:8 a=QyXUC8HyAAAA:8 a=e5mUnYsNAAAA:8 a=mkuqXOTpuORTtGt2Qt4A:9 a=llfO7MrySwjLa4GP:21 a=mdBBOxew0v2lbHCs:21 a=CjuIK1q_8ugA:10 a=DM_PlaNYpjARcMQr2apF:22 a=Vxmtnl_E_bksehYqCbjh:22 X-BeenThere: dri-devel@lists.freedesktop.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Direct Rendering Infrastructure - Development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: Daniel Vetter , Intel Graphics Development , DRI Development Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Errors-To: dri-devel-bounces@lists.freedesktop.org Sender: "dri-devel" Hi Daniel. On Wed, Jan 29, 2020 at 09:24:09AM +0100, Daniel Vetter wrote: > We want to only take the BKL on crap drivers, but to know whether The BKL was killed long time ago.. In other words I was confused until I realized that - BKL - drm_global_mutex BKL - drm_global_mutex Was all the same. At least my OCD color me confused as is. > we have a crap driver we first need to look it up. Split this shuffle > out from the main BKL-disabling patch, for more clarity. > > Since the minors are refcounted drm_minor_acquire is purely internal > and this does not have a driver visible effect. > > v2: Push the locking even further into drm_open(), suggested by Chris. > This gives us more symmetry with drm_release(), and maybe a futuer > avenue where we make drm_globale_mutex locking (partially) opt-in like s/drm_globale_mutex/drm_global_mutex/ > with drm_release_noglobal(). > > Reviewed-by: Chris Wilson > Cc: Chris Wilson > Signed-off-by: Daniel Vetter Above is IMO fix-while-committing stuff. Sam > --- > drivers/gpu/drm/drm_drv.c | 14 +++++--------- > drivers/gpu/drm/drm_file.c | 6 ++++++ > 2 files changed, 11 insertions(+), 9 deletions(-) > > diff --git a/drivers/gpu/drm/drm_drv.c b/drivers/gpu/drm/drm_drv.c > index 8deff75b484c..05bdf0b9d2b3 100644 > --- a/drivers/gpu/drm/drm_drv.c > +++ b/drivers/gpu/drm/drm_drv.c > @@ -1085,17 +1085,14 @@ static int drm_stub_open(struct inode *inode, struct file *filp) > > DRM_DEBUG("\n"); > > - mutex_lock(&drm_global_mutex); > minor = drm_minor_acquire(iminor(inode)); > - if (IS_ERR(minor)) { > - err = PTR_ERR(minor); > - goto out_unlock; > - } > + if (IS_ERR(minor)) > + return PTR_ERR(minor); > > new_fops = fops_get(minor->dev->driver->fops); > if (!new_fops) { > err = -ENODEV; > - goto out_release; > + goto out; > } > > replace_fops(filp, new_fops); > @@ -1104,10 +1101,9 @@ static int drm_stub_open(struct inode *inode, struct file *filp) > else > err = 0; > > -out_release: > +out: > drm_minor_release(minor); > -out_unlock: > - mutex_unlock(&drm_global_mutex); > + > return err; > } > > diff --git a/drivers/gpu/drm/drm_file.c b/drivers/gpu/drm/drm_file.c > index 1075b3a8b5b1..d36cb74ebe0c 100644 > --- a/drivers/gpu/drm/drm_file.c > +++ b/drivers/gpu/drm/drm_file.c > @@ -378,6 +378,8 @@ int drm_open(struct inode *inode, struct file *filp) > if (IS_ERR(minor)) > return PTR_ERR(minor); > > + mutex_unlock(&drm_global_mutex); > + > dev = minor->dev; > if (!atomic_fetch_inc(&dev->open_count)) > need_setup = 1; > @@ -395,10 +397,14 @@ int drm_open(struct inode *inode, struct file *filp) > goto err_undo; > } > } > + > + mutex_unlock(&drm_global_mutex); > + > return 0; > > err_undo: > atomic_dec(&dev->open_count); > + mutex_unlock(&drm_global_mutex); > drm_minor_release(minor); > return retcode; > } > -- > 2.24.1 > > _______________________________________________ > dri-devel mailing list > dri-devel@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/dri-devel _______________________________________________ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-8.2 required=3.0 tests=HEADER_FROM_DIFFERENT_DOMAINS, INCLUDES_PATCH,MAILING_LIST_MULTI,SIGNED_OFF_BY,SPF_HELO_NONE,SPF_PASS, URIBL_BLOCKED,USER_AGENT_SANE_1 autolearn=ham autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id 17172C2D0DB for ; Wed, 29 Jan 2020 16:45:54 +0000 (UTC) Received: from gabe.freedesktop.org (gabe.freedesktop.org [131.252.210.177]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mail.kernel.org (Postfix) with ESMTPS id ECC8320732 for ; Wed, 29 Jan 2020 16:45:53 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org ECC8320732 Authentication-Results: mail.kernel.org; dmarc=none (p=none dis=none) header.from=ravnborg.org Authentication-Results: mail.kernel.org; spf=none smtp.mailfrom=intel-gfx-bounces@lists.freedesktop.org Received: from gabe.freedesktop.org (localhost [127.0.0.1]) by gabe.freedesktop.org (Postfix) with ESMTP id 064856E3F9; Wed, 29 Jan 2020 16:45:53 +0000 (UTC) Received: from asavdk4.altibox.net (asavdk4.altibox.net [109.247.116.15]) by gabe.freedesktop.org (Postfix) with ESMTPS id 511C56E3F9; Wed, 29 Jan 2020 16:45:51 +0000 (UTC) Received: from ravnborg.org (unknown [158.248.194.18]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by asavdk4.altibox.net (Postfix) with ESMTPS id 30107804E7; Wed, 29 Jan 2020 17:45:46 +0100 (CET) Date: Wed, 29 Jan 2020 17:45:45 +0100 From: Sam Ravnborg To: Daniel Vetter Message-ID: <20200129164545.GA22331@ravnborg.org> References: <20200129082410.1691996-1-daniel.vetter@ffwll.ch> <20200129082410.1691996-5-daniel.vetter@ffwll.ch> MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: <20200129082410.1691996-5-daniel.vetter@ffwll.ch> User-Agent: Mutt/1.10.1 (2018-07-13) X-CMAE-Score: 0 X-CMAE-Analysis: v=2.3 cv=VcLZwmh9 c=1 sm=1 tr=0 a=UWs3HLbX/2nnQ3s7vZ42gw==:117 a=UWs3HLbX/2nnQ3s7vZ42gw==:17 a=jpOVt7BSZ2e4Z31A5e1TngXxSK0=:19 a=kj9zAlcOel0A:10 a=taGs_qngAAAA:8 a=QyXUC8HyAAAA:8 a=e5mUnYsNAAAA:8 a=mkuqXOTpuORTtGt2Qt4A:9 a=llfO7MrySwjLa4GP:21 a=mdBBOxew0v2lbHCs:21 a=CjuIK1q_8ugA:10 a=DM_PlaNYpjARcMQr2apF:22 a=Vxmtnl_E_bksehYqCbjh:22 Subject: Re: [Intel-gfx] [PATCH 4/5] drm: Push drm_global_mutex locking in drm_open X-BeenThere: intel-gfx@lists.freedesktop.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Intel graphics driver community testing & development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: Daniel Vetter , Intel Graphics Development , DRI Development Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Errors-To: intel-gfx-bounces@lists.freedesktop.org Sender: "Intel-gfx" Hi Daniel. On Wed, Jan 29, 2020 at 09:24:09AM +0100, Daniel Vetter wrote: > We want to only take the BKL on crap drivers, but to know whether The BKL was killed long time ago.. In other words I was confused until I realized that - BKL - drm_global_mutex BKL - drm_global_mutex Was all the same. At least my OCD color me confused as is. > we have a crap driver we first need to look it up. Split this shuffle > out from the main BKL-disabling patch, for more clarity. > > Since the minors are refcounted drm_minor_acquire is purely internal > and this does not have a driver visible effect. > > v2: Push the locking even further into drm_open(), suggested by Chris. > This gives us more symmetry with drm_release(), and maybe a futuer > avenue where we make drm_globale_mutex locking (partially) opt-in like s/drm_globale_mutex/drm_global_mutex/ > with drm_release_noglobal(). > > Reviewed-by: Chris Wilson > Cc: Chris Wilson > Signed-off-by: Daniel Vetter Above is IMO fix-while-committing stuff. Sam > --- > drivers/gpu/drm/drm_drv.c | 14 +++++--------- > drivers/gpu/drm/drm_file.c | 6 ++++++ > 2 files changed, 11 insertions(+), 9 deletions(-) > > diff --git a/drivers/gpu/drm/drm_drv.c b/drivers/gpu/drm/drm_drv.c > index 8deff75b484c..05bdf0b9d2b3 100644 > --- a/drivers/gpu/drm/drm_drv.c > +++ b/drivers/gpu/drm/drm_drv.c > @@ -1085,17 +1085,14 @@ static int drm_stub_open(struct inode *inode, struct file *filp) > > DRM_DEBUG("\n"); > > - mutex_lock(&drm_global_mutex); > minor = drm_minor_acquire(iminor(inode)); > - if (IS_ERR(minor)) { > - err = PTR_ERR(minor); > - goto out_unlock; > - } > + if (IS_ERR(minor)) > + return PTR_ERR(minor); > > new_fops = fops_get(minor->dev->driver->fops); > if (!new_fops) { > err = -ENODEV; > - goto out_release; > + goto out; > } > > replace_fops(filp, new_fops); > @@ -1104,10 +1101,9 @@ static int drm_stub_open(struct inode *inode, struct file *filp) > else > err = 0; > > -out_release: > +out: > drm_minor_release(minor); > -out_unlock: > - mutex_unlock(&drm_global_mutex); > + > return err; > } > > diff --git a/drivers/gpu/drm/drm_file.c b/drivers/gpu/drm/drm_file.c > index 1075b3a8b5b1..d36cb74ebe0c 100644 > --- a/drivers/gpu/drm/drm_file.c > +++ b/drivers/gpu/drm/drm_file.c > @@ -378,6 +378,8 @@ int drm_open(struct inode *inode, struct file *filp) > if (IS_ERR(minor)) > return PTR_ERR(minor); > > + mutex_unlock(&drm_global_mutex); > + > dev = minor->dev; > if (!atomic_fetch_inc(&dev->open_count)) > need_setup = 1; > @@ -395,10 +397,14 @@ int drm_open(struct inode *inode, struct file *filp) > goto err_undo; > } > } > + > + mutex_unlock(&drm_global_mutex); > + > return 0; > > err_undo: > atomic_dec(&dev->open_count); > + mutex_unlock(&drm_global_mutex); > drm_minor_release(minor); > return retcode; > } > -- > 2.24.1 > > _______________________________________________ > dri-devel mailing list > dri-devel@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/dri-devel _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx