Message ID | 20221129200242.298120-3-robdclark@gmail.com |
---|---|
State | New |
Headers |
Return-Path: <linux-kernel-owner@vger.kernel.org> Delivered-To: ouuuleilei@gmail.com Received: by 2002:adf:f944:0:0:0:0:0 with SMTP id q4csp544558wrr; Tue, 29 Nov 2022 12:09:11 -0800 (PST) X-Google-Smtp-Source: AA0mqf4hSwm15xuAWeDse/n8yykSffNwGrmbTyrERBUHZVoRMbpbeB57SD4Qj0DbscF0glitp5vm X-Received: by 2002:a17:902:aa0c:b0:188:d7f5:20df with SMTP id be12-20020a170902aa0c00b00188d7f520dfmr38121512plb.120.1669752550735; Tue, 29 Nov 2022 12:09:10 -0800 (PST) ARC-Seal: i=1; a=rsa-sha256; t=1669752550; cv=none; d=google.com; s=arc-20160816; b=LWS3oBLiv/ajGQZPERi94QOsf6YANtaXso37cX1z1YtP4VXe8MFxxQpeTLTX/lShmO TJpojG5hPICNHreboyZcQj6uOpLTYcztQivwsYXf4qtecJ2liKCsg7JtZF9KOxwNevj6 pCdiNc5Zt6gzE5jCJzAm8m/JH2LV/Sfmx4g3ABwzOkAB9pSLdhkw9XkDIXwTO28NKvPS 3VFAq0KpWPss309sm6tx6bKm4pOqGzBYtdmyddPoYIKzeIMQqz5EI2z59JxuzxYkbRfl hKchjE1WzL3CjvXkrgwUGrZ+pgSQt2k2vB61WcjZw33zPFqodX5wBoZXiuBMdZqI42O8 rspg== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=arc-20160816; h=list-id:precedence:content-transfer-encoding:mime-version :references:in-reply-to:message-id:date:subject:cc:to:from :dkim-signature; bh=KmLZiaEVkjJr8RiYF6yToHXQoLPPBZhOHiujuwoMMv8=; b=sZBU0S89cxWBZqAXhSBk58jMXChinTlT9YjbOrNRzVm2W6fSEbDDnLly/AGeKYyMQR +o8JNDYcXQeOaZ3R8LK1jgpVYdNoJfKPVPscAsqjZhAiRWGvP5savjuJy43neVDREm7j x7DZRjY8kymKBGdEl828m6XkgwhZXoSbYnz1fwhweQ1r4is10IJp3hTP10qk0TLmL3Lp 6C6HEgUA7bSFpHqHC4+47qUygO/ZdWONlvkMqrH/A6ayyGREz5nHZpKXVHZ7tirdWiK5 FODMLgyoeEBHB0AxgI9at+Yk/2Y9Xmq0z9urELuyTN5ZpvTXJspjv9kBbmjrMZt0JZNB TKCQ== ARC-Authentication-Results: i=1; mx.google.com; dkim=pass header.i=@gmail.com header.s=20210112 header.b=Dz7C3cal; spf=pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org; dmarc=pass (p=NONE sp=QUARANTINE dis=NONE) header.from=gmail.com Received: from out1.vger.email (out1.vger.email. [2620:137:e000::1:20]) by mx.google.com with ESMTP id v13-20020a63464d000000b0047702d2fe08si14790575pgk.493.2022.11.29.12.08.47; Tue, 29 Nov 2022 12:09:10 -0800 (PST) Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20; Authentication-Results: mx.google.com; dkim=pass header.i=@gmail.com header.s=20210112 header.b=Dz7C3cal; spf=pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org; dmarc=pass (p=NONE sp=QUARANTINE dis=NONE) header.from=gmail.com Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S237189AbiK2UDG (ORCPT <rfc822;rbbytesnap@gmail.com> + 99 others); Tue, 29 Nov 2022 15:03:06 -0500 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:46042 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S237192AbiK2UDB (ORCPT <rfc822;linux-kernel@vger.kernel.org>); Tue, 29 Nov 2022 15:03:01 -0500 Received: from mail-pl1-x62c.google.com (mail-pl1-x62c.google.com [IPv6:2607:f8b0:4864:20::62c]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id D6AB817E22; Tue, 29 Nov 2022 12:02:59 -0800 (PST) Received: by mail-pl1-x62c.google.com with SMTP id y17so6617091plp.3; Tue, 29 Nov 2022 12:02:59 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20210112; h=content-transfer-encoding:mime-version:references:in-reply-to :message-id:date:subject:cc:to:from:from:to:cc:subject:date :message-id:reply-to; bh=KmLZiaEVkjJr8RiYF6yToHXQoLPPBZhOHiujuwoMMv8=; b=Dz7C3calMnoC2yh0ZC4zZ18Y3+Fg658tq9lsj9p0WciuPXjgUd+tVvWl0Lc62p6jVW r2Bd5aY2VRPWULHTa6j95JU+cnKHKPHnhokZ8HHRUkXMxDRfKyho4TJOBtfLMgnN+jtk 6qX+OzHN0jn37opP0t8b0WA97hE2SX19L9NufPseOkLxo9fx3ln3D27ws/2/FmFfLWBs 2QiVp247MYChRznqD8zQlY1Lq6+md5jBcmE2ngOip1Im7cI9Rx6uQL1xuibovEpkTLY9 POfy1S6VtEZK9vsQvB5mLbG+0CGKZwNmlz/ChDoSF5CaH+QBXV5gMYFobvOGJ+Jp5ez7 mLzQ== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=content-transfer-encoding:mime-version:references:in-reply-to :message-id:date:subject:cc:to:from:x-gm-message-state:from:to:cc :subject:date:message-id:reply-to; bh=KmLZiaEVkjJr8RiYF6yToHXQoLPPBZhOHiujuwoMMv8=; b=S7pxbw4PUyLUtUIrEQdYroomjl8+jdR330+KaDLk3ZXch6HlLkKZMtuLz0NacRhPqC K+ms40Z34BlsnJr8zBJB8dm3I0IIvhFANENJL6zynEn1qPIYDb1xeWwMloOHh8bgD+d1 JP4qShSSr2lF6pM4zT9C8SEdXuTzB8TLleyO1134nyjDVLRrb3BDwaPM5q8j913jl3da EklmTXYrLUuplEqkwsawQlf+LdgBYv1wGRZLmkYSnGIyxOIU3UMlAbn5V0tYo4q+1N5C wDteGR6Ny1MMzd7leSZRWrGqY0N+PGHdxy0uLSIyjCcRPYeloi4vVdf8IGBysDnWs/r4 IGEQ== X-Gm-Message-State: ANoB5plFTbpgktu9inR6h4pOe/tJUios2seTBqvCFq8EVMV/SOmLIQGO 3U9rNcKeYAWJ7wihGcpMAa4= X-Received: by 2002:a17:902:dac6:b0:189:7105:59e8 with SMTP id q6-20020a170902dac600b00189710559e8mr19846111plx.50.1669752179326; Tue, 29 Nov 2022 12:02:59 -0800 (PST) Received: from localhost ([2a00:79e1:abd:4a00:2703:3c72:eb1a:cffd]) by smtp.gmail.com with ESMTPSA id q5-20020a17090311c500b00186985198a4sm11337838plh.169.2022.11.29.12.02.58 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Tue, 29 Nov 2022 12:02:58 -0800 (PST) From: Rob Clark <robdclark@gmail.com> To: dri-devel@lists.freedesktop.org Cc: Daniel Vetter <daniel@ffwll.ch>, Rob Clark <robdclark@chromium.org>, stable@vger.kernel.org, Maarten Lankhorst <maarten.lankhorst@linux.intel.com>, Maxime Ripard <mripard@kernel.org>, Thomas Zimmermann <tzimmermann@suse.de>, David Airlie <airlied@gmail.com>, Eric Anholt <eric@anholt.net>, =?utf-8?q?Noralf_Tr=C3=B8nnes?= <noralf@tronnes.org>, Rob Herring <robh@kernel.org>, linux-kernel@vger.kernel.org (open list) Subject: [PATCH 2/2] drm/shmem-helper: Avoid vm_open error paths Date: Tue, 29 Nov 2022 12:02:42 -0800 Message-Id: <20221129200242.298120-3-robdclark@gmail.com> X-Mailer: git-send-email 2.38.1 In-Reply-To: <20221129200242.298120-1-robdclark@gmail.com> References: <20221129200242.298120-1-robdclark@gmail.com> MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Spam-Status: No, score=-2.1 required=5.0 tests=BAYES_00,DKIM_SIGNED, DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,FREEMAIL_FROM, RCVD_IN_DNSWL_NONE,SPF_HELO_NONE,SPF_PASS autolearn=ham autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on lindbergh.monkeyblade.net Precedence: bulk List-ID: <linux-kernel.vger.kernel.org> X-Mailing-List: linux-kernel@vger.kernel.org X-getmail-retrieved-from-mailbox: =?utf-8?q?INBOX?= X-GMAIL-THRID: =?utf-8?q?1750862450611577685?= X-GMAIL-MSGID: =?utf-8?q?1750862450611577685?= |
Series |
drm/shmem-helper: Fix a couple of error path bugs
|
|
Commit Message
Rob Clark
Nov. 29, 2022, 8:02 p.m. UTC
From: Rob Clark <robdclark@chromium.org> vm_open() is not allowed to fail. Fortunately we are guaranteed that the pages are already pinned, and only need to increment the refcnt. So just increment it directly. Fixes: 2194a63a818d ("drm: Add library for shmem backed GEM objects") Cc: stable@vger.kernel.org Signed-off-by: Rob Clark <robdclark@chromium.org> --- drivers/gpu/drm/drm_gem_shmem_helper.c | 14 +++++++++++--- 1 file changed, 11 insertions(+), 3 deletions(-)
Comments
On Tue, Nov 29, 2022 at 12:02:42PM -0800, Rob Clark wrote: > From: Rob Clark <robdclark@chromium.org> > > vm_open() is not allowed to fail. Fortunately we are guaranteed that > the pages are already pinned, and only need to increment the refcnt. So > just increment it directly. I don't know anything about drm or gem, but I am wondering _how_ this would be guaranteed. Would it be through the pin function ? Just wondering, because that function does not seem to be mandatory. > > Fixes: 2194a63a818d ("drm: Add library for shmem backed GEM objects") > Cc: stable@vger.kernel.org > Signed-off-by: Rob Clark <robdclark@chromium.org> > --- > drivers/gpu/drm/drm_gem_shmem_helper.c | 14 +++++++++++--- > 1 file changed, 11 insertions(+), 3 deletions(-) > > diff --git a/drivers/gpu/drm/drm_gem_shmem_helper.c b/drivers/gpu/drm/drm_gem_shmem_helper.c > index 110a9eac2af8..9885ba64127f 100644 > --- a/drivers/gpu/drm/drm_gem_shmem_helper.c > +++ b/drivers/gpu/drm/drm_gem_shmem_helper.c > @@ -571,12 +571,20 @@ static void drm_gem_shmem_vm_open(struct vm_area_struct *vma) > { > struct drm_gem_object *obj = vma->vm_private_data; > struct drm_gem_shmem_object *shmem = to_drm_gem_shmem_obj(obj); > - int ret; > > WARN_ON(shmem->base.import_attach); > > - ret = drm_gem_shmem_get_pages(shmem); > - WARN_ON_ONCE(ret != 0); > + mutex_lock(&shmem->pages_lock); > + > + /* > + * We should have already pinned the pages, vm_open() just grabs should or guaranteed ? This sounds a bit weaker than the commit description. > + * an additional reference for the new mm the vma is getting > + * copied into. > + */ > + WARN_ON_ONCE(!shmem->pages_use_count); > + > + shmem->pages_use_count++; > + mutex_unlock(&shmem->pages_lock); The previous code, in that situation, would not increment pages_use_count, and it would not set not set shmem->pages. Hopefully, it would not try to do anything with the pages it was unable to get. The new code assumes that shmem->pages is valid even if pages_use_count is 0, while at the same time taking into account that this can possibly happen (or the WARN_ON_ONCE would not be needed). Again, I don't know anything about gem and drm, but it seems to me that there might now be a severe problem later on if the WARN_ON_ONCE() ever triggers. Thanks, Guenter > > drm_gem_vm_open(vma); > }
On Tue, Nov 29, 2022 at 12:32 PM Guenter Roeck <linux@roeck-us.net> wrote: > > On Tue, Nov 29, 2022 at 12:02:42PM -0800, Rob Clark wrote: > > From: Rob Clark <robdclark@chromium.org> > > > > vm_open() is not allowed to fail. Fortunately we are guaranteed that > > the pages are already pinned, and only need to increment the refcnt. So > > just increment it directly. > > I don't know anything about drm or gem, but I am wondering _how_ > this would be guaranteed. Would it be through the pin function ? > Just wondering, because that function does not seem to be mandatory. We've pinned the pages already in mmap.. vm->open() is perhaps not the best name for the callback function, but it is called for copying an existing vma into a new process (and for some other cases which do not apply here because VM_DONTEXPAND). (Other drivers pin pages in the fault handler, where there is actually potential to return an error, but that change was a bit more like re-writing shmem helper ;-)) BR, -R > > > > Fixes: 2194a63a818d ("drm: Add library for shmem backed GEM objects") > > Cc: stable@vger.kernel.org > > Signed-off-by: Rob Clark <robdclark@chromium.org> > > --- > > drivers/gpu/drm/drm_gem_shmem_helper.c | 14 +++++++++++--- > > 1 file changed, 11 insertions(+), 3 deletions(-) > > > > diff --git a/drivers/gpu/drm/drm_gem_shmem_helper.c b/drivers/gpu/drm/drm_gem_shmem_helper.c > > index 110a9eac2af8..9885ba64127f 100644 > > --- a/drivers/gpu/drm/drm_gem_shmem_helper.c > > +++ b/drivers/gpu/drm/drm_gem_shmem_helper.c > > @@ -571,12 +571,20 @@ static void drm_gem_shmem_vm_open(struct vm_area_struct *vma) > > { > > struct drm_gem_object *obj = vma->vm_private_data; > > struct drm_gem_shmem_object *shmem = to_drm_gem_shmem_obj(obj); > > - int ret; > > > > WARN_ON(shmem->base.import_attach); > > > > - ret = drm_gem_shmem_get_pages(shmem); > > - WARN_ON_ONCE(ret != 0); > > + mutex_lock(&shmem->pages_lock); > > + > > + /* > > + * We should have already pinned the pages, vm_open() just grabs > > should or guaranteed ? This sounds a bit weaker than the commit > description. > > > + * an additional reference for the new mm the vma is getting > > + * copied into. > > + */ > > + WARN_ON_ONCE(!shmem->pages_use_count); > > + > > + shmem->pages_use_count++; > > + mutex_unlock(&shmem->pages_lock); > > The previous code, in that situation, would not increment pages_use_count, > and it would not set not set shmem->pages. Hopefully, it would not try to > do anything with the pages it was unable to get. The new code assumes that > shmem->pages is valid even if pages_use_count is 0, while at the same time > taking into account that this can possibly happen (or the WARN_ON_ONCE > would not be needed). > > Again, I don't know anything about gem and drm, but it seems to me that > there might now be a severe problem later on if the WARN_ON_ONCE() > ever triggers. > > Thanks, > Guenter > > > > > drm_gem_vm_open(vma); > > }
On 11/29/22 12:47, Rob Clark wrote: > On Tue, Nov 29, 2022 at 12:32 PM Guenter Roeck <linux@roeck-us.net> wrote: >> >> On Tue, Nov 29, 2022 at 12:02:42PM -0800, Rob Clark wrote: >>> From: Rob Clark <robdclark@chromium.org> >>> >>> vm_open() is not allowed to fail. Fortunately we are guaranteed that >>> the pages are already pinned, and only need to increment the refcnt. So >>> just increment it directly. >> >> I don't know anything about drm or gem, but I am wondering _how_ >> this would be guaranteed. Would it be through the pin function ? >> Just wondering, because that function does not seem to be mandatory. > > We've pinned the pages already in mmap.. vm->open() is perhaps not the > best name for the callback function, but it is called for copying an > existing vma into a new process (and for some other cases which do not > apply here because VM_DONTEXPAND). > > (Other drivers pin pages in the fault handler, where there is actually > potential to return an error, but that change was a bit more like > re-writing shmem helper ;-)) > Maybe add a bit of that (where the pinning happened) to the commit description and to the patch itself ? > BR, > -R > >>> >>> Fixes: 2194a63a818d ("drm: Add library for shmem backed GEM objects") >>> Cc: stable@vger.kernel.org >>> Signed-off-by: Rob Clark <robdclark@chromium.org> >>> --- >>> drivers/gpu/drm/drm_gem_shmem_helper.c | 14 +++++++++++--- >>> 1 file changed, 11 insertions(+), 3 deletions(-) >>> >>> diff --git a/drivers/gpu/drm/drm_gem_shmem_helper.c b/drivers/gpu/drm/drm_gem_shmem_helper.c >>> index 110a9eac2af8..9885ba64127f 100644 >>> --- a/drivers/gpu/drm/drm_gem_shmem_helper.c >>> +++ b/drivers/gpu/drm/drm_gem_shmem_helper.c >>> @@ -571,12 +571,20 @@ static void drm_gem_shmem_vm_open(struct vm_area_struct *vma) >>> { >>> struct drm_gem_object *obj = vma->vm_private_data; >>> struct drm_gem_shmem_object *shmem = to_drm_gem_shmem_obj(obj); >>> - int ret; >>> >>> WARN_ON(shmem->base.import_attach); >>> >>> - ret = drm_gem_shmem_get_pages(shmem); >>> - WARN_ON_ONCE(ret != 0); >>> + mutex_lock(&shmem->pages_lock); >>> + >>> + /* >>> + * We should have already pinned the pages, vm_open() just grabs >> >> should or guaranteed ? This sounds a bit weaker than the commit >> description. >> like ... the pages were already pinned in (mmap function). >>> + * an additional reference for the new mm the vma is getting >>> + * copied into. >>> + */ >>> + WARN_ON_ONCE(!shmem->pages_use_count); If the code can't be trusted and still needs the warning, how about something like the following ? if (WARN_ON_ONCE(!shmem->pages_use_count)) { mutex_unlock(&shmem->pages_lock); return; } Thanks, Guenter >>> + >>> + shmem->pages_use_count++; >>> + mutex_unlock(&shmem->pages_lock); >> >> The previous code, in that situation, would not increment pages_use_count, >> and it would not set not set shmem->pages. Hopefully, it would not try to >> do anything with the pages it was unable to get. The new code assumes that >> shmem->pages is valid even if pages_use_count is 0, while at the same time >> taking into account that this can possibly happen (or the WARN_ON_ONCE >> would not be needed). >> >> Again, I don't know anything about gem and drm, but it seems to me that >> there might now be a severe problem later on if the WARN_ON_ONCE() >> ever triggers. >> >> Thanks, >> Guenter >> >>> >>> drm_gem_vm_open(vma); >>> }
On Tue, Nov 29, 2022 at 12:47:42PM -0800, Rob Clark wrote: > On Tue, Nov 29, 2022 at 12:32 PM Guenter Roeck <linux@roeck-us.net> wrote: > > > > On Tue, Nov 29, 2022 at 12:02:42PM -0800, Rob Clark wrote: > > > From: Rob Clark <robdclark@chromium.org> > > > > > > vm_open() is not allowed to fail. Fortunately we are guaranteed that > > > the pages are already pinned, and only need to increment the refcnt. So > > > just increment it directly. > > > > I don't know anything about drm or gem, but I am wondering _how_ > > this would be guaranteed. Would it be through the pin function ? > > Just wondering, because that function does not seem to be mandatory. > > We've pinned the pages already in mmap.. vm->open() is perhaps not the > best name for the callback function, but it is called for copying an > existing vma into a new process (and for some other cases which do not > apply here because VM_DONTEXPAND). > > (Other drivers pin pages in the fault handler, where there is actually > potential to return an error, but that change was a bit more like > re-writing shmem helper ;-)) Yhea vm_ops->open should really be called vm_ops->dupe or ->copy or something like that ... -Daniel > > BR, > -R > > > > > > > Fixes: 2194a63a818d ("drm: Add library for shmem backed GEM objects") > > > Cc: stable@vger.kernel.org > > > Signed-off-by: Rob Clark <robdclark@chromium.org> > > > --- > > > drivers/gpu/drm/drm_gem_shmem_helper.c | 14 +++++++++++--- > > > 1 file changed, 11 insertions(+), 3 deletions(-) > > > > > > diff --git a/drivers/gpu/drm/drm_gem_shmem_helper.c b/drivers/gpu/drm/drm_gem_shmem_helper.c > > > index 110a9eac2af8..9885ba64127f 100644 > > > --- a/drivers/gpu/drm/drm_gem_shmem_helper.c > > > +++ b/drivers/gpu/drm/drm_gem_shmem_helper.c > > > @@ -571,12 +571,20 @@ static void drm_gem_shmem_vm_open(struct vm_area_struct *vma) > > > { > > > struct drm_gem_object *obj = vma->vm_private_data; > > > struct drm_gem_shmem_object *shmem = to_drm_gem_shmem_obj(obj); > > > - int ret; > > > > > > WARN_ON(shmem->base.import_attach); > > > > > > - ret = drm_gem_shmem_get_pages(shmem); > > > - WARN_ON_ONCE(ret != 0); > > > + mutex_lock(&shmem->pages_lock); > > > + > > > + /* > > > + * We should have already pinned the pages, vm_open() just grabs > > > > should or guaranteed ? This sounds a bit weaker than the commit > > description. > > > > > + * an additional reference for the new mm the vma is getting > > > + * copied into. > > > + */ > > > + WARN_ON_ONCE(!shmem->pages_use_count); > > > + > > > + shmem->pages_use_count++; > > > + mutex_unlock(&shmem->pages_lock); > > > > The previous code, in that situation, would not increment pages_use_count, > > and it would not set not set shmem->pages. Hopefully, it would not try to > > do anything with the pages it was unable to get. The new code assumes that > > shmem->pages is valid even if pages_use_count is 0, while at the same time > > taking into account that this can possibly happen (or the WARN_ON_ONCE > > would not be needed). > > > > Again, I don't know anything about gem and drm, but it seems to me that > > there might now be a severe problem later on if the WARN_ON_ONCE() > > ever triggers. > > > > Thanks, > > Guenter > > > > > > > > drm_gem_vm_open(vma); > > > }
On Tue, Nov 29, 2022 at 12:02:42PM -0800, Rob Clark wrote: > From: Rob Clark <robdclark@chromium.org> > > vm_open() is not allowed to fail. Fortunately we are guaranteed that > the pages are already pinned, and only need to increment the refcnt. So > just increment it directly. Please mention hare that the only issue is the mutex_lock_interruptible, and the only way we've found to hit this is if you send a signal to the original process in a fork() at _just_ the right time. With that: Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch> > > Fixes: 2194a63a818d ("drm: Add library for shmem backed GEM objects") > Cc: stable@vger.kernel.org > Signed-off-by: Rob Clark <robdclark@chromium.org> > --- > drivers/gpu/drm/drm_gem_shmem_helper.c | 14 +++++++++++--- > 1 file changed, 11 insertions(+), 3 deletions(-) > > diff --git a/drivers/gpu/drm/drm_gem_shmem_helper.c b/drivers/gpu/drm/drm_gem_shmem_helper.c > index 110a9eac2af8..9885ba64127f 100644 > --- a/drivers/gpu/drm/drm_gem_shmem_helper.c > +++ b/drivers/gpu/drm/drm_gem_shmem_helper.c > @@ -571,12 +571,20 @@ static void drm_gem_shmem_vm_open(struct vm_area_struct *vma) > { > struct drm_gem_object *obj = vma->vm_private_data; > struct drm_gem_shmem_object *shmem = to_drm_gem_shmem_obj(obj); > - int ret; > > WARN_ON(shmem->base.import_attach); > > - ret = drm_gem_shmem_get_pages(shmem); > - WARN_ON_ONCE(ret != 0); > + mutex_lock(&shmem->pages_lock); > + > + /* > + * We should have already pinned the pages, vm_open() just grabs > + * an additional reference for the new mm the vma is getting > + * copied into. > + */ > + WARN_ON_ONCE(!shmem->pages_use_count); > + > + shmem->pages_use_count++; > + mutex_unlock(&shmem->pages_lock); > > drm_gem_vm_open(vma); > } > -- > 2.38.1 > -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch
diff --git a/drivers/gpu/drm/drm_gem_shmem_helper.c b/drivers/gpu/drm/drm_gem_shmem_helper.c index 110a9eac2af8..9885ba64127f 100644 --- a/drivers/gpu/drm/drm_gem_shmem_helper.c +++ b/drivers/gpu/drm/drm_gem_shmem_helper.c @@ -571,12 +571,20 @@ static void drm_gem_shmem_vm_open(struct vm_area_struct *vma) { struct drm_gem_object *obj = vma->vm_private_data; struct drm_gem_shmem_object *shmem = to_drm_gem_shmem_obj(obj); - int ret; WARN_ON(shmem->base.import_attach); - ret = drm_gem_shmem_get_pages(shmem); - WARN_ON_ONCE(ret != 0); + mutex_lock(&shmem->pages_lock); + + /* + * We should have already pinned the pages, vm_open() just grabs + * an additional reference for the new mm the vma is getting + * copied into. + */ + WARN_ON_ONCE(!shmem->pages_use_count); + + shmem->pages_use_count++; + mutex_unlock(&shmem->pages_lock); drm_gem_vm_open(vma); }