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=-6.5 required=3.0 tests=DKIM_INVALID,DKIM_SIGNED, HEADER_FROM_DIFFERENT_DOMAINS,INCLUDES_PATCH,MAILING_LIST_MULTI,SIGNED_OFF_BY, SPF_HELO_NONE,SPF_PASS 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 EF42EC2D0DB for ; Wed, 29 Jan 2020 17:05:38 +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 8C4B52070E for ; Wed, 29 Jan 2020 17:05:38 +0000 (UTC) Authentication-Results: mail.kernel.org; dkim=fail reason="signature verification failed" (1024-bit key) header.d=ffwll.ch header.i=@ffwll.ch header.b="gJZfwLMP" DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org 8C4B52070E Authentication-Results: mail.kernel.org; dmarc=none (p=none dis=none) header.from=ffwll.ch 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 BC62C6F5F0; Wed, 29 Jan 2020 17:05:37 +0000 (UTC) Received: from mail-wm1-x341.google.com (mail-wm1-x341.google.com [IPv6:2a00:1450:4864:20::341]) by gabe.freedesktop.org (Postfix) with ESMTPS id 210086F5F0 for ; Wed, 29 Jan 2020 17:05:37 +0000 (UTC) Received: by mail-wm1-x341.google.com with SMTP id q9so495880wmj.5 for ; Wed, 29 Jan 2020 09:05:37 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=ffwll.ch; s=google; h=date:from:to:cc:subject:message-id:references:mime-version :content-disposition:in-reply-to; bh=xkI97nwb8UhKTzo9RfVn3ZtAzZCPu347U5qNCO04TsU=; b=gJZfwLMP1Tx7L/mQahukNxW87QP1aSZfZxwe9AoDX0kIycLiK/Rt6b44oXyTUJhCNs KIe7M1ovgojtS+ZxQbSAc5tWutt+8RnL3TLYdfzUm1wXmWECs8IGjGo9VpHQs1ZXeNyz 8FM16dIjVVbD8XQkmkpvIG96XuKWkCj4o8EQE= X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:date:from:to:cc:subject:message-id:references :mime-version:content-disposition:in-reply-to; bh=xkI97nwb8UhKTzo9RfVn3ZtAzZCPu347U5qNCO04TsU=; b=U1BL93S3KF9MIhNXpRXSbM0PUq/XATk8kNOSfs+wn8yvavi27kGzfC5sme25s9S1nU 7AJzbnLnNYrfVzBjiffHiqzkUdo+sA4p3YfL70hoHsdRZBSQZegaa0I7qY6VGaYovEWE O2NLabsvNq5tl7pBdsOEtM13O3XAKSB5R/mXPhYgM2Epj028G0P3qM5zxqT1zCEuzLcE GioXpupDcPhHqDjlLABSncXLsdLAUQkwnVUDi9VZgejoINBYoqaeFFiPn7eA4mjwJNhm vtlWiLcJVDTDnV99IcxDWMaAJF6lrrXBSNKJUUT5iuaHZd4chjvQctkUiskT/124g11+ n6bA== X-Gm-Message-State: APjAAAXiU0Mre3joWnCJbxoY19DytJGiHWBLSszcj4DWT1XReM9ag4O3 YN4jo5BY3x6rMcjFtJcDiCvyeJxEqrZdYQ== X-Google-Smtp-Source: APXvYqwVD0lTgdC9GL5ZqYIoufEDvBesiynTTGbu0wtY95KC4cFr6YAt+nHasqUhGwzquylL/MxK7w== X-Received: by 2002:a05:600c:54e:: with SMTP id k14mr173051wmc.115.1580317535740; Wed, 29 Jan 2020 09:05:35 -0800 (PST) Received: from phenom.ffwll.local ([2a02:168:57f4:0:efd0:b9e5:5ae6:c2fa]) by smtp.gmail.com with ESMTPSA id t9sm2776350wmj.28.2020.01.29.09.05.34 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Wed, 29 Jan 2020 09:05:35 -0800 (PST) Date: Wed, 29 Jan 2020 18:05:33 +0100 From: Daniel Vetter To: Sam Ravnborg Subject: Re: [PATCH 4/5] drm: Push drm_global_mutex locking in drm_open Message-ID: <20200129170533.GM43062@phenom.ffwll.local> References: <20200129082410.1691996-1-daniel.vetter@ffwll.ch> <20200129082410.1691996-5-daniel.vetter@ffwll.ch> <20200129164545.GA22331@ravnborg.org> MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: <20200129164545.GA22331@ravnborg.org> X-Operating-System: Linux phenom 5.3.0-3-amd64 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 , Daniel Vetter Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Errors-To: dri-devel-bounces@lists.freedesktop.org Sender: "dri-devel" On Wed, Jan 29, 2020 at 05:45:45PM +0100, Sam Ravnborg wrote: > 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. The Real BKL was converted into drm_global_mutex for drm when the BKL was killed. Which is kinda relevant, because the BKL locking horrors (at least in drm) have been inherited by drm_global_mutex (and the conversion broke a few things that had to be papered over). Hence the motivation to finally clean up the locking and figure out what exactly is still protected by this lock. If you're bored, all this is at least in modern git history afaics. -Daniel > > > 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 -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch _______________________________________________ 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=-6.5 required=3.0 tests=DKIM_INVALID,DKIM_SIGNED, HEADER_FROM_DIFFERENT_DOMAINS,INCLUDES_PATCH,MAILING_LIST_MULTI,SIGNED_OFF_BY, SPF_HELO_NONE,SPF_PASS 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 797BBC33CB7 for ; Wed, 29 Jan 2020 17:05:40 +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 4B5892070E for ; Wed, 29 Jan 2020 17:05:40 +0000 (UTC) Authentication-Results: mail.kernel.org; dkim=fail reason="signature verification failed" (1024-bit key) header.d=ffwll.ch header.i=@ffwll.ch header.b="gJZfwLMP" DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org 4B5892070E Authentication-Results: mail.kernel.org; dmarc=none (p=none dis=none) header.from=ffwll.ch 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 4DE1B6F5F8; Wed, 29 Jan 2020 17:05:38 +0000 (UTC) Received: from mail-wm1-x343.google.com (mail-wm1-x343.google.com [IPv6:2a00:1450:4864:20::343]) by gabe.freedesktop.org (Postfix) with ESMTPS id 2E84A6F5F2 for ; Wed, 29 Jan 2020 17:05:37 +0000 (UTC) Received: by mail-wm1-x343.google.com with SMTP id c84so601533wme.4 for ; Wed, 29 Jan 2020 09:05:37 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=ffwll.ch; s=google; h=date:from:to:cc:subject:message-id:references:mime-version :content-disposition:in-reply-to; bh=xkI97nwb8UhKTzo9RfVn3ZtAzZCPu347U5qNCO04TsU=; b=gJZfwLMP1Tx7L/mQahukNxW87QP1aSZfZxwe9AoDX0kIycLiK/Rt6b44oXyTUJhCNs KIe7M1ovgojtS+ZxQbSAc5tWutt+8RnL3TLYdfzUm1wXmWECs8IGjGo9VpHQs1ZXeNyz 8FM16dIjVVbD8XQkmkpvIG96XuKWkCj4o8EQE= X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:date:from:to:cc:subject:message-id:references :mime-version:content-disposition:in-reply-to; bh=xkI97nwb8UhKTzo9RfVn3ZtAzZCPu347U5qNCO04TsU=; b=r1CzQK6qI8mn5+fQAQp64SM8im1xsTw6w3Wq8SYp5SB3KtUuWv/fMNhmSX6j+kiLp2 VoK/qRmLOKW6+jsx7DRT68lKJ2dzf9rfke9diynLUx2C6MZaiaonytefNDHtIeb/2qwl giU09K6vF/JaVffIFcivgCjqozUXBue3tk4mrrhFkC1MBVRo7GvwZZl9r0HO7sUsYzw3 NupNF2FfsrrDQHO3vlGxY/K4OwWts/Kpkf8wI6ae3EZLvOawzvG+QFtanh1bQVkviHcY f9hUR+nw9BcJuTRwuZpR0jKlezIFWktFhPnYOuS8nDVFvQtvoa1Vll2xlmpnCwtfIX+X u11A== X-Gm-Message-State: APjAAAW3fc14rutfUJnYYKWmkC93IKydEt9g2uMX8T0cYLspI/Budr9d nuTW+fp4O+BsEkllk1TavAS9gw== X-Google-Smtp-Source: APXvYqwVD0lTgdC9GL5ZqYIoufEDvBesiynTTGbu0wtY95KC4cFr6YAt+nHasqUhGwzquylL/MxK7w== X-Received: by 2002:a05:600c:54e:: with SMTP id k14mr173051wmc.115.1580317535740; Wed, 29 Jan 2020 09:05:35 -0800 (PST) Received: from phenom.ffwll.local ([2a02:168:57f4:0:efd0:b9e5:5ae6:c2fa]) by smtp.gmail.com with ESMTPSA id t9sm2776350wmj.28.2020.01.29.09.05.34 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Wed, 29 Jan 2020 09:05:35 -0800 (PST) Date: Wed, 29 Jan 2020 18:05:33 +0100 From: Daniel Vetter To: Sam Ravnborg Message-ID: <20200129170533.GM43062@phenom.ffwll.local> References: <20200129082410.1691996-1-daniel.vetter@ffwll.ch> <20200129082410.1691996-5-daniel.vetter@ffwll.ch> <20200129164545.GA22331@ravnborg.org> MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: <20200129164545.GA22331@ravnborg.org> X-Operating-System: Linux phenom 5.3.0-3-amd64 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 , Daniel Vetter Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Errors-To: intel-gfx-bounces@lists.freedesktop.org Sender: "Intel-gfx" On Wed, Jan 29, 2020 at 05:45:45PM +0100, Sam Ravnborg wrote: > 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. The Real BKL was converted into drm_global_mutex for drm when the BKL was killed. Which is kinda relevant, because the BKL locking horrors (at least in drm) have been inherited by drm_global_mutex (and the conversion broke a few things that had to be papered over). Hence the motivation to finally clean up the locking and figure out what exactly is still protected by this lock. If you're bored, all this is at least in modern git history afaics. -Daniel > > > 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 -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx