From mboxrd@z Thu Jan 1 00:00:00 1970 From: Steven Price Date: Mon, 24 Jul 2023 12:17:40 +0100 Subject: [Cluster-devel] [PATCH v2 24/47] drm/panfrost: dynamically allocate the drm-panfrost shrinker In-Reply-To: <20230724094354.90817-25-zhengqi.arch@bytedance.com> References: <20230724094354.90817-1-zhengqi.arch@bytedance.com> <20230724094354.90817-25-zhengqi.arch@bytedance.com> Message-ID: List-Id: To: cluster-devel.redhat.com MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit On 24/07/2023 10:43, Qi Zheng wrote: > In preparation for implementing lockless slab shrink, use new APIs to > dynamically allocate the drm-panfrost shrinker, so that it can be freed > asynchronously using kfree_rcu(). Then it doesn't need to wait for RCU > read-side critical section when releasing the struct panfrost_device. > > Signed-off-by: Qi Zheng One nit below, but otherwise: Reviewed-by: Steven Price > --- > drivers/gpu/drm/panfrost/panfrost_device.h | 2 +- > drivers/gpu/drm/panfrost/panfrost_drv.c | 6 +++- > drivers/gpu/drm/panfrost/panfrost_gem.h | 2 +- > .../gpu/drm/panfrost/panfrost_gem_shrinker.c | 32 ++++++++++++------- > 4 files changed, 27 insertions(+), 15 deletions(-) > > diff --git a/drivers/gpu/drm/panfrost/panfrost_device.h b/drivers/gpu/drm/panfrost/panfrost_device.h > index b0126b9fbadc..e667e5689353 100644 > --- a/drivers/gpu/drm/panfrost/panfrost_device.h > +++ b/drivers/gpu/drm/panfrost/panfrost_device.h > @@ -118,7 +118,7 @@ struct panfrost_device { > > struct mutex shrinker_lock; > struct list_head shrinker_list; > - struct shrinker shrinker; > + struct shrinker *shrinker; > > struct panfrost_devfreq pfdevfreq; > }; > diff --git a/drivers/gpu/drm/panfrost/panfrost_drv.c b/drivers/gpu/drm/panfrost/panfrost_drv.c > index bbada731bbbd..f705bbdea360 100644 > --- a/drivers/gpu/drm/panfrost/panfrost_drv.c > +++ b/drivers/gpu/drm/panfrost/panfrost_drv.c > @@ -598,10 +598,14 @@ static int panfrost_probe(struct platform_device *pdev) > if (err < 0) > goto err_out1; > > - panfrost_gem_shrinker_init(ddev); > + err = panfrost_gem_shrinker_init(ddev); > + if (err) > + goto err_out2; > > return 0; > > +err_out2: > + drm_dev_unregister(ddev); > err_out1: > pm_runtime_disable(pfdev->dev); > panfrost_device_fini(pfdev); > diff --git a/drivers/gpu/drm/panfrost/panfrost_gem.h b/drivers/gpu/drm/panfrost/panfrost_gem.h > index ad2877eeeccd..863d2ec8d4f0 100644 > --- a/drivers/gpu/drm/panfrost/panfrost_gem.h > +++ b/drivers/gpu/drm/panfrost/panfrost_gem.h > @@ -81,7 +81,7 @@ panfrost_gem_mapping_get(struct panfrost_gem_object *bo, > void panfrost_gem_mapping_put(struct panfrost_gem_mapping *mapping); > void panfrost_gem_teardown_mappings_locked(struct panfrost_gem_object *bo); > > -void panfrost_gem_shrinker_init(struct drm_device *dev); > +int panfrost_gem_shrinker_init(struct drm_device *dev); > void panfrost_gem_shrinker_cleanup(struct drm_device *dev); > > #endif /* __PANFROST_GEM_H__ */ > diff --git a/drivers/gpu/drm/panfrost/panfrost_gem_shrinker.c b/drivers/gpu/drm/panfrost/panfrost_gem_shrinker.c > index bf0170782f25..9a90dfb5301f 100644 > --- a/drivers/gpu/drm/panfrost/panfrost_gem_shrinker.c > +++ b/drivers/gpu/drm/panfrost/panfrost_gem_shrinker.c > @@ -18,8 +18,7 @@ > static unsigned long > panfrost_gem_shrinker_count(struct shrinker *shrinker, struct shrink_control *sc) > { > - struct panfrost_device *pfdev = > - container_of(shrinker, struct panfrost_device, shrinker); > + struct panfrost_device *pfdev = shrinker->private_data; > struct drm_gem_shmem_object *shmem; > unsigned long count = 0; > > @@ -65,8 +64,7 @@ static bool panfrost_gem_purge(struct drm_gem_object *obj) > static unsigned long > panfrost_gem_shrinker_scan(struct shrinker *shrinker, struct shrink_control *sc) > { > - struct panfrost_device *pfdev = > - container_of(shrinker, struct panfrost_device, shrinker); > + struct panfrost_device *pfdev = shrinker->private_data; > struct drm_gem_shmem_object *shmem, *tmp; > unsigned long freed = 0; > > @@ -97,13 +95,24 @@ panfrost_gem_shrinker_scan(struct shrinker *shrinker, struct shrink_control *sc) > * > * This function registers and sets up the panfrost shrinker. > */ > -void panfrost_gem_shrinker_init(struct drm_device *dev) > +int panfrost_gem_shrinker_init(struct drm_device *dev) > { > struct panfrost_device *pfdev = dev->dev_private; > - pfdev->shrinker.count_objects = panfrost_gem_shrinker_count; > - pfdev->shrinker.scan_objects = panfrost_gem_shrinker_scan; > - pfdev->shrinker.seeks = DEFAULT_SEEKS; > - WARN_ON(register_shrinker(&pfdev->shrinker, "drm-panfrost")); > + > + pfdev->shrinker = shrinker_alloc(0, "drm-panfrost"); > + if (!pfdev->shrinker) { > + WARN_ON(1); I don't think this WARN_ON is particularly useful - if there's a failed memory allocation we should see output from the kernel anyway. And we're changing the semantics from "continue just without a shrinker" (which argueably justifies the warning) to "probe fails, device doesn't work" which will be obvious to the user so I don't feel we need the additional warn. > + return -ENOMEM; > + } > + > + pfdev->shrinker->count_objects = panfrost_gem_shrinker_count; > + pfdev->shrinker->scan_objects = panfrost_gem_shrinker_scan; > + pfdev->shrinker->seeks = DEFAULT_SEEKS; > + pfdev->shrinker->private_data = pfdev; > + > + shrinker_register(pfdev->shrinker); > + > + return 0; > } > > /** > @@ -116,7 +125,6 @@ void panfrost_gem_shrinker_cleanup(struct drm_device *dev) > { > struct panfrost_device *pfdev = dev->dev_private; > > - if (pfdev->shrinker.nr_deferred) { > - unregister_shrinker(&pfdev->shrinker); > - } > + if (pfdev->shrinker) > + shrinker_unregister(pfdev->shrinker); > } 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 Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [170.10.129.124]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.lore.kernel.org (Postfix) with ESMTPS id 25223C0015E for ; Tue, 25 Jul 2023 06:43:10 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1690267389; h=from:from:sender:sender:reply-to:subject:subject:date:date: message-id:message-id:to:to:cc:cc:mime-version:mime-version: content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references:list-id:list-help: list-unsubscribe:list-subscribe:list-post; bh=Gl9FblQ87nBS8027Fx5g6S00qwqMSy0cyqWVKKUmW00=; b=elZeBgNaRnmQ3uYLaRlhbA0I1sIUkppG+tl6qpuWhO5AwFsSw3+kf0PTFswl2dFTDvqKJ3 w0+P0j69s0i0IlHlMgHWMwgmU3S2LeMnxWXV0SPWfQ7VEtk1KdOoOkvnWIgtMPimgspZdM 95Y6MBj+ttr5DuBIeFW8bNdQWNuc6C4= Received: from mimecast-mx02.redhat.com (mimecast-mx02.redhat.com [66.187.233.88]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id us-mta-323-_32gr4KBN0SI0OQ46ZIOAQ-1; Tue, 25 Jul 2023 02:43:07 -0400 X-MC-Unique: _32gr4KBN0SI0OQ46ZIOAQ-1 Received: from smtp.corp.redhat.com (int-mx02.intmail.prod.int.rdu2.redhat.com [10.11.54.2]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mimecast-mx02.redhat.com (Postfix) with ESMTPS id 2357D830DB6; Tue, 25 Jul 2023 06:42:49 +0000 (UTC) Received: from mm-prod-listman-01.mail-001.prod.us-east-1.aws.redhat.com (unknown [10.30.29.100]) by smtp.corp.redhat.com (Postfix) with ESMTP id 11F6F40D2839; Tue, 25 Jul 2023 06:42:49 +0000 (UTC) Received: from mm-prod-listman-01.mail-001.prod.us-east-1.aws.redhat.com (localhost [IPv6:::1]) by mm-prod-listman-01.mail-001.prod.us-east-1.aws.redhat.com (Postfix) with ESMTP id F350719543A2; Tue, 25 Jul 2023 06:42:44 +0000 (UTC) Received: from smtp.corp.redhat.com (int-mx05.intmail.prod.int.rdu2.redhat.com [10.11.54.5]) by mm-prod-listman-01.mail-001.prod.us-east-1.aws.redhat.com (Postfix) with ESMTP id AE3861946588 for ; Mon, 24 Jul 2023 11:17:49 +0000 (UTC) Received: by smtp.corp.redhat.com (Postfix) id 9ACA0F7832; Mon, 24 Jul 2023 11:17:49 +0000 (UTC) Received: from mimecast-mx02.redhat.com (mimecast04.extmail.prod.ext.rdu2.redhat.com [10.11.55.20]) by smtp.corp.redhat.com (Postfix) with ESMTPS id 926F810E5E for ; Mon, 24 Jul 2023 11:17:49 +0000 (UTC) Received: from us-smtp-1.mimecast.com (us-smtp-1.mimecast.com [207.211.31.81]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mimecast-mx02.redhat.com (Postfix) with ESMTPS id 5DC05101A54E for ; Mon, 24 Jul 2023 11:17:49 +0000 (UTC) Received: from foss.arm.com (foss.arm.com [217.140.110.172]) by relay.mimecast.com with ESMTP id us-mta-625-LO3eNK_XMxC74DCWS_UNmg-1; Mon, 24 Jul 2023 07:17:47 -0400 X-MC-Unique: LO3eNK_XMxC74DCWS_UNmg-1 Received: from usa-sjc-imap-foss1.foss.arm.com (unknown [10.121.207.14]) by usa-sjc-mx-foss1.foss.arm.com (Postfix) with ESMTP id B206CDE0; Mon, 24 Jul 2023 04:18:29 -0700 (PDT) Received: from [10.57.34.62] (unknown [10.57.34.62]) by usa-sjc-imap-foss1.foss.arm.com (Postfix) with ESMTPSA id C518B3F5A1; Mon, 24 Jul 2023 04:17:39 -0700 (PDT) Message-ID: Date: Mon, 24 Jul 2023 12:17:40 +0100 MIME-Version: 1.0 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:102.0) Gecko/20100101 Thunderbird/102.13.0 To: Qi Zheng , akpm@linux-foundation.org, david@fromorbit.com, tkhai@ya.ru, vbabka@suse.cz, roman.gushchin@linux.dev, djwong@kernel.org, brauner@kernel.org, paulmck@kernel.org, tytso@mit.edu, cel@kernel.org, senozhatsky@chromium.org, yujie.liu@intel.com, gregkh@linuxfoundation.org, muchun.song@linux.dev References: <20230724094354.90817-1-zhengqi.arch@bytedance.com> <20230724094354.90817-25-zhengqi.arch@bytedance.com> From: Steven Price In-Reply-To: <20230724094354.90817-25-zhengqi.arch@bytedance.com> X-Mimecast-Impersonation-Protect: Policy=CLT - Impersonation Protection Definition; Similar Internal Domain=false; Similar Monitored External Domain=false; Custom External Domain=false; Mimecast External Domain=false; Newly Observed Domain=false; Internal User Name=false; Custom Display Name List=false; Reply-to Address Mismatch=false; Targeted Threat Dictionary=false; Mimecast Threat Dictionary=false; Custom Threat Dictionary=false X-Scanned-By: MIMEDefang 3.1 on 10.11.54.5 X-Mailman-Approved-At: Tue, 25 Jul 2023 06:42:42 +0000 Subject: Re: [dm-devel] [PATCH v2 24/47] drm/panfrost: dynamically allocate the drm-panfrost shrinker X-BeenThere: dm-devel@redhat.com X-Mailman-Version: 2.1.29 Precedence: list List-Id: device-mapper development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: kvm@vger.kernel.org, dri-devel@lists.freedesktop.org, virtualization@lists.linux-foundation.org, linux-mm@kvack.org, dm-devel@redhat.com, linux-mtd@lists.infradead.org, x86@kernel.org, cluster-devel@redhat.com, xen-devel@lists.xenproject.org, linux-ext4@vger.kernel.org, linux-arm-msm@vger.kernel.org, rcu@vger.kernel.org, linux-bcache@vger.kernel.org, linux-raid@vger.kernel.org, linux-nfs@vger.kernel.org, netdev@vger.kernel.org, linux-kernel@vger.kernel.org, linux-f2fs-devel@lists.sourceforge.net, linux-xfs@vger.kernel.org, linux-fsdevel@vger.kernel.org, linux-erofs@lists.ozlabs.org, linux-btrfs@vger.kernel.org Errors-To: dm-devel-bounces@redhat.com Sender: "dm-devel" X-Scanned-By: MIMEDefang 3.1 on 10.11.54.2 X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: arm.com Content-Language: en-GB Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit On 24/07/2023 10:43, Qi Zheng wrote: > In preparation for implementing lockless slab shrink, use new APIs to > dynamically allocate the drm-panfrost shrinker, so that it can be freed > asynchronously using kfree_rcu(). Then it doesn't need to wait for RCU > read-side critical section when releasing the struct panfrost_device. > > Signed-off-by: Qi Zheng One nit below, but otherwise: Reviewed-by: Steven Price > --- > drivers/gpu/drm/panfrost/panfrost_device.h | 2 +- > drivers/gpu/drm/panfrost/panfrost_drv.c | 6 +++- > drivers/gpu/drm/panfrost/panfrost_gem.h | 2 +- > .../gpu/drm/panfrost/panfrost_gem_shrinker.c | 32 ++++++++++++------- > 4 files changed, 27 insertions(+), 15 deletions(-) > > diff --git a/drivers/gpu/drm/panfrost/panfrost_device.h b/drivers/gpu/drm/panfrost/panfrost_device.h > index b0126b9fbadc..e667e5689353 100644 > --- a/drivers/gpu/drm/panfrost/panfrost_device.h > +++ b/drivers/gpu/drm/panfrost/panfrost_device.h > @@ -118,7 +118,7 @@ struct panfrost_device { > > struct mutex shrinker_lock; > struct list_head shrinker_list; > - struct shrinker shrinker; > + struct shrinker *shrinker; > > struct panfrost_devfreq pfdevfreq; > }; > diff --git a/drivers/gpu/drm/panfrost/panfrost_drv.c b/drivers/gpu/drm/panfrost/panfrost_drv.c > index bbada731bbbd..f705bbdea360 100644 > --- a/drivers/gpu/drm/panfrost/panfrost_drv.c > +++ b/drivers/gpu/drm/panfrost/panfrost_drv.c > @@ -598,10 +598,14 @@ static int panfrost_probe(struct platform_device *pdev) > if (err < 0) > goto err_out1; > > - panfrost_gem_shrinker_init(ddev); > + err = panfrost_gem_shrinker_init(ddev); > + if (err) > + goto err_out2; > > return 0; > > +err_out2: > + drm_dev_unregister(ddev); > err_out1: > pm_runtime_disable(pfdev->dev); > panfrost_device_fini(pfdev); > diff --git a/drivers/gpu/drm/panfrost/panfrost_gem.h b/drivers/gpu/drm/panfrost/panfrost_gem.h > index ad2877eeeccd..863d2ec8d4f0 100644 > --- a/drivers/gpu/drm/panfrost/panfrost_gem.h > +++ b/drivers/gpu/drm/panfrost/panfrost_gem.h > @@ -81,7 +81,7 @@ panfrost_gem_mapping_get(struct panfrost_gem_object *bo, > void panfrost_gem_mapping_put(struct panfrost_gem_mapping *mapping); > void panfrost_gem_teardown_mappings_locked(struct panfrost_gem_object *bo); > > -void panfrost_gem_shrinker_init(struct drm_device *dev); > +int panfrost_gem_shrinker_init(struct drm_device *dev); > void panfrost_gem_shrinker_cleanup(struct drm_device *dev); > > #endif /* __PANFROST_GEM_H__ */ > diff --git a/drivers/gpu/drm/panfrost/panfrost_gem_shrinker.c b/drivers/gpu/drm/panfrost/panfrost_gem_shrinker.c > index bf0170782f25..9a90dfb5301f 100644 > --- a/drivers/gpu/drm/panfrost/panfrost_gem_shrinker.c > +++ b/drivers/gpu/drm/panfrost/panfrost_gem_shrinker.c > @@ -18,8 +18,7 @@ > static unsigned long > panfrost_gem_shrinker_count(struct shrinker *shrinker, struct shrink_control *sc) > { > - struct panfrost_device *pfdev = > - container_of(shrinker, struct panfrost_device, shrinker); > + struct panfrost_device *pfdev = shrinker->private_data; > struct drm_gem_shmem_object *shmem; > unsigned long count = 0; > > @@ -65,8 +64,7 @@ static bool panfrost_gem_purge(struct drm_gem_object *obj) > static unsigned long > panfrost_gem_shrinker_scan(struct shrinker *shrinker, struct shrink_control *sc) > { > - struct panfrost_device *pfdev = > - container_of(shrinker, struct panfrost_device, shrinker); > + struct panfrost_device *pfdev = shrinker->private_data; > struct drm_gem_shmem_object *shmem, *tmp; > unsigned long freed = 0; > > @@ -97,13 +95,24 @@ panfrost_gem_shrinker_scan(struct shrinker *shrinker, struct shrink_control *sc) > * > * This function registers and sets up the panfrost shrinker. > */ > -void panfrost_gem_shrinker_init(struct drm_device *dev) > +int panfrost_gem_shrinker_init(struct drm_device *dev) > { > struct panfrost_device *pfdev = dev->dev_private; > - pfdev->shrinker.count_objects = panfrost_gem_shrinker_count; > - pfdev->shrinker.scan_objects = panfrost_gem_shrinker_scan; > - pfdev->shrinker.seeks = DEFAULT_SEEKS; > - WARN_ON(register_shrinker(&pfdev->shrinker, "drm-panfrost")); > + > + pfdev->shrinker = shrinker_alloc(0, "drm-panfrost"); > + if (!pfdev->shrinker) { > + WARN_ON(1); I don't think this WARN_ON is particularly useful - if there's a failed memory allocation we should see output from the kernel anyway. And we're changing the semantics from "continue just without a shrinker" (which argueably justifies the warning) to "probe fails, device doesn't work" which will be obvious to the user so I don't feel we need the additional warn. > + return -ENOMEM; > + } > + > + pfdev->shrinker->count_objects = panfrost_gem_shrinker_count; > + pfdev->shrinker->scan_objects = panfrost_gem_shrinker_scan; > + pfdev->shrinker->seeks = DEFAULT_SEEKS; > + pfdev->shrinker->private_data = pfdev; > + > + shrinker_register(pfdev->shrinker); > + > + return 0; > } > > /** > @@ -116,7 +125,6 @@ void panfrost_gem_shrinker_cleanup(struct drm_device *dev) > { > struct panfrost_device *pfdev = dev->dev_private; > > - if (pfdev->shrinker.nr_deferred) { > - unregister_shrinker(&pfdev->shrinker); > - } > + if (pfdev->shrinker) > + shrinker_unregister(pfdev->shrinker); > } -- dm-devel mailing list dm-devel@redhat.com https://listman.redhat.com/mailman/listinfo/dm-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 Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by smtp.lore.kernel.org (Postfix) with ESMTP id 4525BC001DE for ; Mon, 24 Jul 2023 11:18:10 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S229921AbjGXLSJ (ORCPT ); Mon, 24 Jul 2023 07:18:09 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:43994 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S229813AbjGXLSI (ORCPT ); Mon, 24 Jul 2023 07:18:08 -0400 Received: from foss.arm.com (foss.arm.com [217.140.110.172]) by lindbergh.monkeyblade.net (Postfix) with ESMTP id 24B84E65; Mon, 24 Jul 2023 04:17:47 -0700 (PDT) Received: from usa-sjc-imap-foss1.foss.arm.com (unknown [10.121.207.14]) by usa-sjc-mx-foss1.foss.arm.com (Postfix) with ESMTP id B206CDE0; Mon, 24 Jul 2023 04:18:29 -0700 (PDT) Received: from [10.57.34.62] (unknown [10.57.34.62]) by usa-sjc-imap-foss1.foss.arm.com (Postfix) with ESMTPSA id C518B3F5A1; Mon, 24 Jul 2023 04:17:39 -0700 (PDT) Message-ID: Date: Mon, 24 Jul 2023 12:17:40 +0100 MIME-Version: 1.0 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:102.0) Gecko/20100101 Thunderbird/102.13.0 Subject: Re: [PATCH v2 24/47] drm/panfrost: dynamically allocate the drm-panfrost shrinker To: Qi Zheng , akpm@linux-foundation.org, david@fromorbit.com, tkhai@ya.ru, vbabka@suse.cz, roman.gushchin@linux.dev, djwong@kernel.org, brauner@kernel.org, paulmck@kernel.org, tytso@mit.edu, cel@kernel.org, senozhatsky@chromium.org, yujie.liu@intel.com, gregkh@linuxfoundation.org, muchun.song@linux.dev Cc: linux-kernel@vger.kernel.org, linux-mm@kvack.org, x86@kernel.org, kvm@vger.kernel.org, xen-devel@lists.xenproject.org, linux-erofs@lists.ozlabs.org, linux-f2fs-devel@lists.sourceforge.net, cluster-devel@redhat.com, linux-nfs@vger.kernel.org, linux-mtd@lists.infradead.org, rcu@vger.kernel.org, netdev@vger.kernel.org, dri-devel@lists.freedesktop.org, linux-arm-msm@vger.kernel.org, dm-devel@redhat.com, linux-raid@vger.kernel.org, linux-bcache@vger.kernel.org, virtualization@lists.linux-foundation.org, linux-fsdevel@vger.kernel.org, linux-ext4@vger.kernel.org, linux-xfs@vger.kernel.org, linux-btrfs@vger.kernel.org References: <20230724094354.90817-1-zhengqi.arch@bytedance.com> <20230724094354.90817-25-zhengqi.arch@bytedance.com> Content-Language: en-GB From: Steven Price In-Reply-To: <20230724094354.90817-25-zhengqi.arch@bytedance.com> Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 7bit Precedence: bulk List-ID: X-Mailing-List: linux-bcache@vger.kernel.org On 24/07/2023 10:43, Qi Zheng wrote: > In preparation for implementing lockless slab shrink, use new APIs to > dynamically allocate the drm-panfrost shrinker, so that it can be freed > asynchronously using kfree_rcu(). Then it doesn't need to wait for RCU > read-side critical section when releasing the struct panfrost_device. > > Signed-off-by: Qi Zheng One nit below, but otherwise: Reviewed-by: Steven Price > --- > drivers/gpu/drm/panfrost/panfrost_device.h | 2 +- > drivers/gpu/drm/panfrost/panfrost_drv.c | 6 +++- > drivers/gpu/drm/panfrost/panfrost_gem.h | 2 +- > .../gpu/drm/panfrost/panfrost_gem_shrinker.c | 32 ++++++++++++------- > 4 files changed, 27 insertions(+), 15 deletions(-) > > diff --git a/drivers/gpu/drm/panfrost/panfrost_device.h b/drivers/gpu/drm/panfrost/panfrost_device.h > index b0126b9fbadc..e667e5689353 100644 > --- a/drivers/gpu/drm/panfrost/panfrost_device.h > +++ b/drivers/gpu/drm/panfrost/panfrost_device.h > @@ -118,7 +118,7 @@ struct panfrost_device { > > struct mutex shrinker_lock; > struct list_head shrinker_list; > - struct shrinker shrinker; > + struct shrinker *shrinker; > > struct panfrost_devfreq pfdevfreq; > }; > diff --git a/drivers/gpu/drm/panfrost/panfrost_drv.c b/drivers/gpu/drm/panfrost/panfrost_drv.c > index bbada731bbbd..f705bbdea360 100644 > --- a/drivers/gpu/drm/panfrost/panfrost_drv.c > +++ b/drivers/gpu/drm/panfrost/panfrost_drv.c > @@ -598,10 +598,14 @@ static int panfrost_probe(struct platform_device *pdev) > if (err < 0) > goto err_out1; > > - panfrost_gem_shrinker_init(ddev); > + err = panfrost_gem_shrinker_init(ddev); > + if (err) > + goto err_out2; > > return 0; > > +err_out2: > + drm_dev_unregister(ddev); > err_out1: > pm_runtime_disable(pfdev->dev); > panfrost_device_fini(pfdev); > diff --git a/drivers/gpu/drm/panfrost/panfrost_gem.h b/drivers/gpu/drm/panfrost/panfrost_gem.h > index ad2877eeeccd..863d2ec8d4f0 100644 > --- a/drivers/gpu/drm/panfrost/panfrost_gem.h > +++ b/drivers/gpu/drm/panfrost/panfrost_gem.h > @@ -81,7 +81,7 @@ panfrost_gem_mapping_get(struct panfrost_gem_object *bo, > void panfrost_gem_mapping_put(struct panfrost_gem_mapping *mapping); > void panfrost_gem_teardown_mappings_locked(struct panfrost_gem_object *bo); > > -void panfrost_gem_shrinker_init(struct drm_device *dev); > +int panfrost_gem_shrinker_init(struct drm_device *dev); > void panfrost_gem_shrinker_cleanup(struct drm_device *dev); > > #endif /* __PANFROST_GEM_H__ */ > diff --git a/drivers/gpu/drm/panfrost/panfrost_gem_shrinker.c b/drivers/gpu/drm/panfrost/panfrost_gem_shrinker.c > index bf0170782f25..9a90dfb5301f 100644 > --- a/drivers/gpu/drm/panfrost/panfrost_gem_shrinker.c > +++ b/drivers/gpu/drm/panfrost/panfrost_gem_shrinker.c > @@ -18,8 +18,7 @@ > static unsigned long > panfrost_gem_shrinker_count(struct shrinker *shrinker, struct shrink_control *sc) > { > - struct panfrost_device *pfdev = > - container_of(shrinker, struct panfrost_device, shrinker); > + struct panfrost_device *pfdev = shrinker->private_data; > struct drm_gem_shmem_object *shmem; > unsigned long count = 0; > > @@ -65,8 +64,7 @@ static bool panfrost_gem_purge(struct drm_gem_object *obj) > static unsigned long > panfrost_gem_shrinker_scan(struct shrinker *shrinker, struct shrink_control *sc) > { > - struct panfrost_device *pfdev = > - container_of(shrinker, struct panfrost_device, shrinker); > + struct panfrost_device *pfdev = shrinker->private_data; > struct drm_gem_shmem_object *shmem, *tmp; > unsigned long freed = 0; > > @@ -97,13 +95,24 @@ panfrost_gem_shrinker_scan(struct shrinker *shrinker, struct shrink_control *sc) > * > * This function registers and sets up the panfrost shrinker. > */ > -void panfrost_gem_shrinker_init(struct drm_device *dev) > +int panfrost_gem_shrinker_init(struct drm_device *dev) > { > struct panfrost_device *pfdev = dev->dev_private; > - pfdev->shrinker.count_objects = panfrost_gem_shrinker_count; > - pfdev->shrinker.scan_objects = panfrost_gem_shrinker_scan; > - pfdev->shrinker.seeks = DEFAULT_SEEKS; > - WARN_ON(register_shrinker(&pfdev->shrinker, "drm-panfrost")); > + > + pfdev->shrinker = shrinker_alloc(0, "drm-panfrost"); > + if (!pfdev->shrinker) { > + WARN_ON(1); I don't think this WARN_ON is particularly useful - if there's a failed memory allocation we should see output from the kernel anyway. And we're changing the semantics from "continue just without a shrinker" (which argueably justifies the warning) to "probe fails, device doesn't work" which will be obvious to the user so I don't feel we need the additional warn. > + return -ENOMEM; > + } > + > + pfdev->shrinker->count_objects = panfrost_gem_shrinker_count; > + pfdev->shrinker->scan_objects = panfrost_gem_shrinker_scan; > + pfdev->shrinker->seeks = DEFAULT_SEEKS; > + pfdev->shrinker->private_data = pfdev; > + > + shrinker_register(pfdev->shrinker); > + > + return 0; > } > > /** > @@ -116,7 +125,6 @@ void panfrost_gem_shrinker_cleanup(struct drm_device *dev) > { > struct panfrost_device *pfdev = dev->dev_private; > > - if (pfdev->shrinker.nr_deferred) { > - unregister_shrinker(&pfdev->shrinker); > - } > + if (pfdev->shrinker) > + shrinker_unregister(pfdev->shrinker); > } 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 Received: from lists.ozlabs.org (lists.ozlabs.org [112.213.38.117]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.lore.kernel.org (Postfix) with ESMTPS id 1938EC001B0 for ; Mon, 24 Jul 2023 11:25:55 +0000 (UTC) Received: from boromir.ozlabs.org (localhost [IPv6:::1]) by lists.ozlabs.org (Postfix) with ESMTP id 4R8dBp4QNdz3012 for ; Mon, 24 Jul 2023 21:25:54 +1000 (AEST) Authentication-Results: lists.ozlabs.org; spf=pass (sender SPF authorized) smtp.mailfrom=arm.com (client-ip=217.140.110.172; helo=foss.arm.com; envelope-from=steven.price@arm.com; receiver=lists.ozlabs.org) X-Greylist: delayed 444 seconds by postgrey-1.37 at boromir; Mon, 24 Jul 2023 21:25:47 AEST Received: from foss.arm.com (foss.arm.com [217.140.110.172]) by lists.ozlabs.org (Postfix) with ESMTP id 4R8dBg0dMqz2yVR for ; Mon, 24 Jul 2023 21:25:45 +1000 (AEST) Received: from usa-sjc-imap-foss1.foss.arm.com (unknown [10.121.207.14]) by usa-sjc-mx-foss1.foss.arm.com (Postfix) with ESMTP id B206CDE0; Mon, 24 Jul 2023 04:18:29 -0700 (PDT) Received: from [10.57.34.62] (unknown [10.57.34.62]) by usa-sjc-imap-foss1.foss.arm.com (Postfix) with ESMTPSA id C518B3F5A1; Mon, 24 Jul 2023 04:17:39 -0700 (PDT) Message-ID: Date: Mon, 24 Jul 2023 12:17:40 +0100 MIME-Version: 1.0 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:102.0) Gecko/20100101 Thunderbird/102.13.0 Subject: Re: [PATCH v2 24/47] drm/panfrost: dynamically allocate the drm-panfrost shrinker To: Qi Zheng , akpm@linux-foundation.org, david@fromorbit.com, tkhai@ya.ru, vbabka@suse.cz, roman.gushchin@linux.dev, djwong@kernel.org, brauner@kernel.org, paulmck@kernel.org, tytso@mit.edu, cel@kernel.org, senozhatsky@chromium.org, yujie.liu@intel.com, gregkh@linuxfoundation.org, muchun.song@linux.dev References: <20230724094354.90817-1-zhengqi.arch@bytedance.com> <20230724094354.90817-25-zhengqi.arch@bytedance.com> Content-Language: en-GB From: Steven Price In-Reply-To: <20230724094354.90817-25-zhengqi.arch@bytedance.com> Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 7bit X-BeenThere: linux-erofs@lists.ozlabs.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Development of Linux EROFS file system List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: kvm@vger.kernel.org, dri-devel@lists.freedesktop.org, virtualization@lists.linux-foundation.org, linux-mm@kvack.org, dm-devel@redhat.com, linux-mtd@lists.infradead.org, x86@kernel.org, cluster-devel@redhat.com, xen-devel@lists.xenproject.org, linux-ext4@vger.kernel.org, linux-arm-msm@vger.kernel.org, rcu@vger.kernel.org, linux-bcache@vger.kernel.org, linux-raid@vger.kernel.org, linux-nfs@vger.kernel.org, netdev@vger.kernel.org, linux-kernel@vger.kernel.org, linux-f2fs-devel@lists.sourceforge.net, linux-xfs@vger.kernel.org, linux-fsdevel@vger.kernel.org, linux-erofs@lists.ozlabs.org, linux-btrfs@vger.kernel.org Errors-To: linux-erofs-bounces+linux-erofs=archiver.kernel.org@lists.ozlabs.org Sender: "Linux-erofs" On 24/07/2023 10:43, Qi Zheng wrote: > In preparation for implementing lockless slab shrink, use new APIs to > dynamically allocate the drm-panfrost shrinker, so that it can be freed > asynchronously using kfree_rcu(). Then it doesn't need to wait for RCU > read-side critical section when releasing the struct panfrost_device. > > Signed-off-by: Qi Zheng One nit below, but otherwise: Reviewed-by: Steven Price > --- > drivers/gpu/drm/panfrost/panfrost_device.h | 2 +- > drivers/gpu/drm/panfrost/panfrost_drv.c | 6 +++- > drivers/gpu/drm/panfrost/panfrost_gem.h | 2 +- > .../gpu/drm/panfrost/panfrost_gem_shrinker.c | 32 ++++++++++++------- > 4 files changed, 27 insertions(+), 15 deletions(-) > > diff --git a/drivers/gpu/drm/panfrost/panfrost_device.h b/drivers/gpu/drm/panfrost/panfrost_device.h > index b0126b9fbadc..e667e5689353 100644 > --- a/drivers/gpu/drm/panfrost/panfrost_device.h > +++ b/drivers/gpu/drm/panfrost/panfrost_device.h > @@ -118,7 +118,7 @@ struct panfrost_device { > > struct mutex shrinker_lock; > struct list_head shrinker_list; > - struct shrinker shrinker; > + struct shrinker *shrinker; > > struct panfrost_devfreq pfdevfreq; > }; > diff --git a/drivers/gpu/drm/panfrost/panfrost_drv.c b/drivers/gpu/drm/panfrost/panfrost_drv.c > index bbada731bbbd..f705bbdea360 100644 > --- a/drivers/gpu/drm/panfrost/panfrost_drv.c > +++ b/drivers/gpu/drm/panfrost/panfrost_drv.c > @@ -598,10 +598,14 @@ static int panfrost_probe(struct platform_device *pdev) > if (err < 0) > goto err_out1; > > - panfrost_gem_shrinker_init(ddev); > + err = panfrost_gem_shrinker_init(ddev); > + if (err) > + goto err_out2; > > return 0; > > +err_out2: > + drm_dev_unregister(ddev); > err_out1: > pm_runtime_disable(pfdev->dev); > panfrost_device_fini(pfdev); > diff --git a/drivers/gpu/drm/panfrost/panfrost_gem.h b/drivers/gpu/drm/panfrost/panfrost_gem.h > index ad2877eeeccd..863d2ec8d4f0 100644 > --- a/drivers/gpu/drm/panfrost/panfrost_gem.h > +++ b/drivers/gpu/drm/panfrost/panfrost_gem.h > @@ -81,7 +81,7 @@ panfrost_gem_mapping_get(struct panfrost_gem_object *bo, > void panfrost_gem_mapping_put(struct panfrost_gem_mapping *mapping); > void panfrost_gem_teardown_mappings_locked(struct panfrost_gem_object *bo); > > -void panfrost_gem_shrinker_init(struct drm_device *dev); > +int panfrost_gem_shrinker_init(struct drm_device *dev); > void panfrost_gem_shrinker_cleanup(struct drm_device *dev); > > #endif /* __PANFROST_GEM_H__ */ > diff --git a/drivers/gpu/drm/panfrost/panfrost_gem_shrinker.c b/drivers/gpu/drm/panfrost/panfrost_gem_shrinker.c > index bf0170782f25..9a90dfb5301f 100644 > --- a/drivers/gpu/drm/panfrost/panfrost_gem_shrinker.c > +++ b/drivers/gpu/drm/panfrost/panfrost_gem_shrinker.c > @@ -18,8 +18,7 @@ > static unsigned long > panfrost_gem_shrinker_count(struct shrinker *shrinker, struct shrink_control *sc) > { > - struct panfrost_device *pfdev = > - container_of(shrinker, struct panfrost_device, shrinker); > + struct panfrost_device *pfdev = shrinker->private_data; > struct drm_gem_shmem_object *shmem; > unsigned long count = 0; > > @@ -65,8 +64,7 @@ static bool panfrost_gem_purge(struct drm_gem_object *obj) > static unsigned long > panfrost_gem_shrinker_scan(struct shrinker *shrinker, struct shrink_control *sc) > { > - struct panfrost_device *pfdev = > - container_of(shrinker, struct panfrost_device, shrinker); > + struct panfrost_device *pfdev = shrinker->private_data; > struct drm_gem_shmem_object *shmem, *tmp; > unsigned long freed = 0; > > @@ -97,13 +95,24 @@ panfrost_gem_shrinker_scan(struct shrinker *shrinker, struct shrink_control *sc) > * > * This function registers and sets up the panfrost shrinker. > */ > -void panfrost_gem_shrinker_init(struct drm_device *dev) > +int panfrost_gem_shrinker_init(struct drm_device *dev) > { > struct panfrost_device *pfdev = dev->dev_private; > - pfdev->shrinker.count_objects = panfrost_gem_shrinker_count; > - pfdev->shrinker.scan_objects = panfrost_gem_shrinker_scan; > - pfdev->shrinker.seeks = DEFAULT_SEEKS; > - WARN_ON(register_shrinker(&pfdev->shrinker, "drm-panfrost")); > + > + pfdev->shrinker = shrinker_alloc(0, "drm-panfrost"); > + if (!pfdev->shrinker) { > + WARN_ON(1); I don't think this WARN_ON is particularly useful - if there's a failed memory allocation we should see output from the kernel anyway. And we're changing the semantics from "continue just without a shrinker" (which argueably justifies the warning) to "probe fails, device doesn't work" which will be obvious to the user so I don't feel we need the additional warn. > + return -ENOMEM; > + } > + > + pfdev->shrinker->count_objects = panfrost_gem_shrinker_count; > + pfdev->shrinker->scan_objects = panfrost_gem_shrinker_scan; > + pfdev->shrinker->seeks = DEFAULT_SEEKS; > + pfdev->shrinker->private_data = pfdev; > + > + shrinker_register(pfdev->shrinker); > + > + return 0; > } > > /** > @@ -116,7 +125,6 @@ void panfrost_gem_shrinker_cleanup(struct drm_device *dev) > { > struct panfrost_device *pfdev = dev->dev_private; > > - if (pfdev->shrinker.nr_deferred) { > - unregister_shrinker(&pfdev->shrinker); > - } > + if (pfdev->shrinker) > + shrinker_unregister(pfdev->shrinker); > } 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 Received: from lists.sourceforge.net (lists.sourceforge.net [216.105.38.7]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.lore.kernel.org (Postfix) with ESMTPS id 174FEC001DF for ; Mon, 24 Jul 2023 11:35:24 +0000 (UTC) Received: from [127.0.0.1] (helo=sfs-ml-4.v29.lw.sourceforge.com) by sfs-ml-4.v29.lw.sourceforge.com with esmtp (Exim 4.95) (envelope-from ) id 1qNtqU-0008NO-95; Mon, 24 Jul 2023 11:35:22 +0000 Received: from [172.30.20.202] (helo=mx.sourceforge.net) by sfs-ml-4.v29.lw.sourceforge.com with esmtps (TLS1.2) tls TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384 (Exim 4.95) (envelope-from ) id 1qNtqS-0008NC-6s for linux-f2fs-devel@lists.sourceforge.net; Mon, 24 Jul 2023 11:35:20 +0000 DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=sourceforge.net; s=x; h=Content-Transfer-Encoding:Content-Type:In-Reply-To: From:References:Cc:To:Subject:MIME-Version:Date:Message-ID:Sender:Reply-To: Content-ID:Content-Description:Resent-Date:Resent-From:Resent-Sender: Resent-To:Resent-Cc:Resent-Message-ID:List-Id:List-Help:List-Unsubscribe: List-Subscribe:List-Post:List-Owner:List-Archive; bh=fRq9dwwGFiUYnhelgiMFCqaFSvyn1Iy7L8f3oV4pu3I=; b=gDCOalCd9crO5J2GEzHNOvdVsL fyZpYYDnWbm3kcj4PmFDuyRIGBeRG8dknA34C3mms4ztXgUnhXGiIG5eP8WBWJq1dG11uV2uxdylH K+fMZpmVwfQKx7Zd5tocURWT8WdpxKyZxQeln+DyZk/rsTj2oyTFolIbC8ECvtZau0a0=; DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=sf.net; s=x ; h=Content-Transfer-Encoding:Content-Type:In-Reply-To:From:References:Cc:To: Subject:MIME-Version:Date:Message-ID:Sender:Reply-To:Content-ID: Content-Description:Resent-Date:Resent-From:Resent-Sender:Resent-To:Resent-Cc :Resent-Message-ID:List-Id:List-Help:List-Unsubscribe:List-Subscribe: List-Post:List-Owner:List-Archive; bh=fRq9dwwGFiUYnhelgiMFCqaFSvyn1Iy7L8f3oV4pu3I=; b=R6rCgwe3Paefhkuz5PcAi/qW0g XEjdYwhP1YTwIaltUtKAaj7vlg1A0Trb270pFDoWdAO/c5OPecJKhJ1969SiPJ6/Kcv+CzxpXwfc5 wQbzdc3NedmytnQNyS6SAbeoykxDdPkUocHzspnxUNFHsAyu5DFnyYXs3BFnbknLpiqI=; Received: from foss.arm.com ([217.140.110.172]) by sfi-mx-1.v28.lw.sourceforge.com with esmtp (Exim 4.95) id 1qNtqR-000TKp-2P for linux-f2fs-devel@lists.sourceforge.net; Mon, 24 Jul 2023 11:35:20 +0000 Received: from usa-sjc-imap-foss1.foss.arm.com (unknown [10.121.207.14]) by usa-sjc-mx-foss1.foss.arm.com (Postfix) with ESMTP id B206CDE0; Mon, 24 Jul 2023 04:18:29 -0700 (PDT) Received: from [10.57.34.62] (unknown [10.57.34.62]) by usa-sjc-imap-foss1.foss.arm.com (Postfix) with ESMTPSA id C518B3F5A1; Mon, 24 Jul 2023 04:17:39 -0700 (PDT) Message-ID: Date: Mon, 24 Jul 2023 12:17:40 +0100 MIME-Version: 1.0 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:102.0) Gecko/20100101 Thunderbird/102.13.0 To: Qi Zheng , akpm@linux-foundation.org, david@fromorbit.com, tkhai@ya.ru, vbabka@suse.cz, roman.gushchin@linux.dev, djwong@kernel.org, brauner@kernel.org, paulmck@kernel.org, tytso@mit.edu, cel@kernel.org, senozhatsky@chromium.org, yujie.liu@intel.com, gregkh@linuxfoundation.org, muchun.song@linux.dev References: <20230724094354.90817-1-zhengqi.arch@bytedance.com> <20230724094354.90817-25-zhengqi.arch@bytedance.com> Content-Language: en-GB From: Steven Price In-Reply-To: <20230724094354.90817-25-zhengqi.arch@bytedance.com> X-Headers-End: 1qNtqR-000TKp-2P Subject: Re: [f2fs-dev] [PATCH v2 24/47] drm/panfrost: dynamically allocate the drm-panfrost shrinker X-BeenThere: linux-f2fs-devel@lists.sourceforge.net X-Mailman-Version: 2.1.21 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: kvm@vger.kernel.org, dri-devel@lists.freedesktop.org, virtualization@lists.linux-foundation.org, linux-mm@kvack.org, dm-devel@redhat.com, linux-mtd@lists.infradead.org, x86@kernel.org, cluster-devel@redhat.com, xen-devel@lists.xenproject.org, linux-ext4@vger.kernel.org, linux-arm-msm@vger.kernel.org, rcu@vger.kernel.org, linux-bcache@vger.kernel.org, linux-raid@vger.kernel.org, linux-nfs@vger.kernel.org, netdev@vger.kernel.org, linux-kernel@vger.kernel.org, linux-f2fs-devel@lists.sourceforge.net, linux-xfs@vger.kernel.org, linux-fsdevel@vger.kernel.org, linux-erofs@lists.ozlabs.org, linux-btrfs@vger.kernel.org Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Errors-To: linux-f2fs-devel-bounces@lists.sourceforge.net On 24/07/2023 10:43, Qi Zheng wrote: > In preparation for implementing lockless slab shrink, use new APIs to > dynamically allocate the drm-panfrost shrinker, so that it can be freed > asynchronously using kfree_rcu(). Then it doesn't need to wait for RCU > read-side critical section when releasing the struct panfrost_device. > > Signed-off-by: Qi Zheng One nit below, but otherwise: Reviewed-by: Steven Price > --- > drivers/gpu/drm/panfrost/panfrost_device.h | 2 +- > drivers/gpu/drm/panfrost/panfrost_drv.c | 6 +++- > drivers/gpu/drm/panfrost/panfrost_gem.h | 2 +- > .../gpu/drm/panfrost/panfrost_gem_shrinker.c | 32 ++++++++++++------- > 4 files changed, 27 insertions(+), 15 deletions(-) > > diff --git a/drivers/gpu/drm/panfrost/panfrost_device.h b/drivers/gpu/drm/panfrost/panfrost_device.h > index b0126b9fbadc..e667e5689353 100644 > --- a/drivers/gpu/drm/panfrost/panfrost_device.h > +++ b/drivers/gpu/drm/panfrost/panfrost_device.h > @@ -118,7 +118,7 @@ struct panfrost_device { > > struct mutex shrinker_lock; > struct list_head shrinker_list; > - struct shrinker shrinker; > + struct shrinker *shrinker; > > struct panfrost_devfreq pfdevfreq; > }; > diff --git a/drivers/gpu/drm/panfrost/panfrost_drv.c b/drivers/gpu/drm/panfrost/panfrost_drv.c > index bbada731bbbd..f705bbdea360 100644 > --- a/drivers/gpu/drm/panfrost/panfrost_drv.c > +++ b/drivers/gpu/drm/panfrost/panfrost_drv.c > @@ -598,10 +598,14 @@ static int panfrost_probe(struct platform_device *pdev) > if (err < 0) > goto err_out1; > > - panfrost_gem_shrinker_init(ddev); > + err = panfrost_gem_shrinker_init(ddev); > + if (err) > + goto err_out2; > > return 0; > > +err_out2: > + drm_dev_unregister(ddev); > err_out1: > pm_runtime_disable(pfdev->dev); > panfrost_device_fini(pfdev); > diff --git a/drivers/gpu/drm/panfrost/panfrost_gem.h b/drivers/gpu/drm/panfrost/panfrost_gem.h > index ad2877eeeccd..863d2ec8d4f0 100644 > --- a/drivers/gpu/drm/panfrost/panfrost_gem.h > +++ b/drivers/gpu/drm/panfrost/panfrost_gem.h > @@ -81,7 +81,7 @@ panfrost_gem_mapping_get(struct panfrost_gem_object *bo, > void panfrost_gem_mapping_put(struct panfrost_gem_mapping *mapping); > void panfrost_gem_teardown_mappings_locked(struct panfrost_gem_object *bo); > > -void panfrost_gem_shrinker_init(struct drm_device *dev); > +int panfrost_gem_shrinker_init(struct drm_device *dev); > void panfrost_gem_shrinker_cleanup(struct drm_device *dev); > > #endif /* __PANFROST_GEM_H__ */ > diff --git a/drivers/gpu/drm/panfrost/panfrost_gem_shrinker.c b/drivers/gpu/drm/panfrost/panfrost_gem_shrinker.c > index bf0170782f25..9a90dfb5301f 100644 > --- a/drivers/gpu/drm/panfrost/panfrost_gem_shrinker.c > +++ b/drivers/gpu/drm/panfrost/panfrost_gem_shrinker.c > @@ -18,8 +18,7 @@ > static unsigned long > panfrost_gem_shrinker_count(struct shrinker *shrinker, struct shrink_control *sc) > { > - struct panfrost_device *pfdev = > - container_of(shrinker, struct panfrost_device, shrinker); > + struct panfrost_device *pfdev = shrinker->private_data; > struct drm_gem_shmem_object *shmem; > unsigned long count = 0; > > @@ -65,8 +64,7 @@ static bool panfrost_gem_purge(struct drm_gem_object *obj) > static unsigned long > panfrost_gem_shrinker_scan(struct shrinker *shrinker, struct shrink_control *sc) > { > - struct panfrost_device *pfdev = > - container_of(shrinker, struct panfrost_device, shrinker); > + struct panfrost_device *pfdev = shrinker->private_data; > struct drm_gem_shmem_object *shmem, *tmp; > unsigned long freed = 0; > > @@ -97,13 +95,24 @@ panfrost_gem_shrinker_scan(struct shrinker *shrinker, struct shrink_control *sc) > * > * This function registers and sets up the panfrost shrinker. > */ > -void panfrost_gem_shrinker_init(struct drm_device *dev) > +int panfrost_gem_shrinker_init(struct drm_device *dev) > { > struct panfrost_device *pfdev = dev->dev_private; > - pfdev->shrinker.count_objects = panfrost_gem_shrinker_count; > - pfdev->shrinker.scan_objects = panfrost_gem_shrinker_scan; > - pfdev->shrinker.seeks = DEFAULT_SEEKS; > - WARN_ON(register_shrinker(&pfdev->shrinker, "drm-panfrost")); > + > + pfdev->shrinker = shrinker_alloc(0, "drm-panfrost"); > + if (!pfdev->shrinker) { > + WARN_ON(1); I don't think this WARN_ON is particularly useful - if there's a failed memory allocation we should see output from the kernel anyway. And we're changing the semantics from "continue just without a shrinker" (which argueably justifies the warning) to "probe fails, device doesn't work" which will be obvious to the user so I don't feel we need the additional warn. > + return -ENOMEM; > + } > + > + pfdev->shrinker->count_objects = panfrost_gem_shrinker_count; > + pfdev->shrinker->scan_objects = panfrost_gem_shrinker_scan; > + pfdev->shrinker->seeks = DEFAULT_SEEKS; > + pfdev->shrinker->private_data = pfdev; > + > + shrinker_register(pfdev->shrinker); > + > + return 0; > } > > /** > @@ -116,7 +125,6 @@ void panfrost_gem_shrinker_cleanup(struct drm_device *dev) > { > struct panfrost_device *pfdev = dev->dev_private; > > - if (pfdev->shrinker.nr_deferred) { > - unregister_shrinker(&pfdev->shrinker); > - } > + if (pfdev->shrinker) > + shrinker_unregister(pfdev->shrinker); > } _______________________________________________ Linux-f2fs-devel mailing list Linux-f2fs-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/linux-f2fs-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 Received: from bombadil.infradead.org (bombadil.infradead.org [198.137.202.133]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.lore.kernel.org (Postfix) with ESMTPS id 06FA1C0015E for ; Mon, 24 Jul 2023 11:18:19 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=lists.infradead.org; s=bombadil.20210309; h=Sender: Content-Transfer-Encoding:Content-Type:List-Subscribe:List-Help:List-Post: List-Archive:List-Unsubscribe:List-Id:In-Reply-To:From:References:Cc:To: Subject:MIME-Version:Date:Message-ID:Reply-To:Content-ID:Content-Description: Resent-Date:Resent-From:Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID: List-Owner; bh=/til7E688mHOer2LLA9Rz7CF+AB1q2msLBr87xaDiRs=; b=TQMMq8Q6sg7uly QGsow7En5ZOTiKjrGF1N2G8/c3qT+ndiG13AE7ibfELGcmLzRHdVC4mAmXxfwnUV9ZIXphhTQwDSH J5fxN9sgMRnbumzagI7Flj53AugvXHuJ610kGE19h39vgfwUY8QvFGkLtL+cbZRX7cVyupL+tgKLR V0IWjP8pIAlgPh0Gibk+Gv5x2z+fWZxAJgumilgD02tV8qCGp05EIodWvo3Sque8LBHrL4ZTCRdM3 G4rt9bVN12LYrnEO7wyoY/zBkiYQRZYUZlOa2oqFusQqGTF2+S9L8jzoawOpCSd66Id5g99R1mQrM Liz6f/uxO0oassqXS7DQ==; Received: from localhost ([::1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.96 #2 (Red Hat Linux)) id 1qNtZs-0041Nj-1w; Mon, 24 Jul 2023 11:18:12 +0000 Received: from foss.arm.com ([217.140.110.172]) by bombadil.infradead.org with esmtp (Exim 4.96 #2 (Red Hat Linux)) id 1qNtZp-0041Gy-0D for linux-mtd@lists.infradead.org; Mon, 24 Jul 2023 11:18:11 +0000 Received: from usa-sjc-imap-foss1.foss.arm.com (unknown [10.121.207.14]) by usa-sjc-mx-foss1.foss.arm.com (Postfix) with ESMTP id B206CDE0; Mon, 24 Jul 2023 04:18:29 -0700 (PDT) Received: from [10.57.34.62] (unknown [10.57.34.62]) by usa-sjc-imap-foss1.foss.arm.com (Postfix) with ESMTPSA id C518B3F5A1; Mon, 24 Jul 2023 04:17:39 -0700 (PDT) Message-ID: Date: Mon, 24 Jul 2023 12:17:40 +0100 MIME-Version: 1.0 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:102.0) Gecko/20100101 Thunderbird/102.13.0 Subject: Re: [PATCH v2 24/47] drm/panfrost: dynamically allocate the drm-panfrost shrinker To: Qi Zheng , akpm@linux-foundation.org, david@fromorbit.com, tkhai@ya.ru, vbabka@suse.cz, roman.gushchin@linux.dev, djwong@kernel.org, brauner@kernel.org, paulmck@kernel.org, tytso@mit.edu, cel@kernel.org, senozhatsky@chromium.org, yujie.liu@intel.com, gregkh@linuxfoundation.org, muchun.song@linux.dev Cc: linux-kernel@vger.kernel.org, linux-mm@kvack.org, x86@kernel.org, kvm@vger.kernel.org, xen-devel@lists.xenproject.org, linux-erofs@lists.ozlabs.org, linux-f2fs-devel@lists.sourceforge.net, cluster-devel@redhat.com, linux-nfs@vger.kernel.org, linux-mtd@lists.infradead.org, rcu@vger.kernel.org, netdev@vger.kernel.org, dri-devel@lists.freedesktop.org, linux-arm-msm@vger.kernel.org, dm-devel@redhat.com, linux-raid@vger.kernel.org, linux-bcache@vger.kernel.org, virtualization@lists.linux-foundation.org, linux-fsdevel@vger.kernel.org, linux-ext4@vger.kernel.org, linux-xfs@vger.kernel.org, linux-btrfs@vger.kernel.org References: <20230724094354.90817-1-zhengqi.arch@bytedance.com> <20230724094354.90817-25-zhengqi.arch@bytedance.com> Content-Language: en-GB From: Steven Price In-Reply-To: <20230724094354.90817-25-zhengqi.arch@bytedance.com> X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20230724_041809_217419_0B52C4B9 X-CRM114-Status: GOOD ( 25.98 ) X-BeenThere: linux-mtd@lists.infradead.org X-Mailman-Version: 2.1.34 Precedence: list List-Id: Linux MTD discussion mailing list List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Sender: "linux-mtd" Errors-To: linux-mtd-bounces+linux-mtd=archiver.kernel.org@lists.infradead.org On 24/07/2023 10:43, Qi Zheng wrote: > In preparation for implementing lockless slab shrink, use new APIs to > dynamically allocate the drm-panfrost shrinker, so that it can be freed > asynchronously using kfree_rcu(). Then it doesn't need to wait for RCU > read-side critical section when releasing the struct panfrost_device. > > Signed-off-by: Qi Zheng One nit below, but otherwise: Reviewed-by: Steven Price > --- > drivers/gpu/drm/panfrost/panfrost_device.h | 2 +- > drivers/gpu/drm/panfrost/panfrost_drv.c | 6 +++- > drivers/gpu/drm/panfrost/panfrost_gem.h | 2 +- > .../gpu/drm/panfrost/panfrost_gem_shrinker.c | 32 ++++++++++++------- > 4 files changed, 27 insertions(+), 15 deletions(-) > > diff --git a/drivers/gpu/drm/panfrost/panfrost_device.h b/drivers/gpu/drm/panfrost/panfrost_device.h > index b0126b9fbadc..e667e5689353 100644 > --- a/drivers/gpu/drm/panfrost/panfrost_device.h > +++ b/drivers/gpu/drm/panfrost/panfrost_device.h > @@ -118,7 +118,7 @@ struct panfrost_device { > > struct mutex shrinker_lock; > struct list_head shrinker_list; > - struct shrinker shrinker; > + struct shrinker *shrinker; > > struct panfrost_devfreq pfdevfreq; > }; > diff --git a/drivers/gpu/drm/panfrost/panfrost_drv.c b/drivers/gpu/drm/panfrost/panfrost_drv.c > index bbada731bbbd..f705bbdea360 100644 > --- a/drivers/gpu/drm/panfrost/panfrost_drv.c > +++ b/drivers/gpu/drm/panfrost/panfrost_drv.c > @@ -598,10 +598,14 @@ static int panfrost_probe(struct platform_device *pdev) > if (err < 0) > goto err_out1; > > - panfrost_gem_shrinker_init(ddev); > + err = panfrost_gem_shrinker_init(ddev); > + if (err) > + goto err_out2; > > return 0; > > +err_out2: > + drm_dev_unregister(ddev); > err_out1: > pm_runtime_disable(pfdev->dev); > panfrost_device_fini(pfdev); > diff --git a/drivers/gpu/drm/panfrost/panfrost_gem.h b/drivers/gpu/drm/panfrost/panfrost_gem.h > index ad2877eeeccd..863d2ec8d4f0 100644 > --- a/drivers/gpu/drm/panfrost/panfrost_gem.h > +++ b/drivers/gpu/drm/panfrost/panfrost_gem.h > @@ -81,7 +81,7 @@ panfrost_gem_mapping_get(struct panfrost_gem_object *bo, > void panfrost_gem_mapping_put(struct panfrost_gem_mapping *mapping); > void panfrost_gem_teardown_mappings_locked(struct panfrost_gem_object *bo); > > -void panfrost_gem_shrinker_init(struct drm_device *dev); > +int panfrost_gem_shrinker_init(struct drm_device *dev); > void panfrost_gem_shrinker_cleanup(struct drm_device *dev); > > #endif /* __PANFROST_GEM_H__ */ > diff --git a/drivers/gpu/drm/panfrost/panfrost_gem_shrinker.c b/drivers/gpu/drm/panfrost/panfrost_gem_shrinker.c > index bf0170782f25..9a90dfb5301f 100644 > --- a/drivers/gpu/drm/panfrost/panfrost_gem_shrinker.c > +++ b/drivers/gpu/drm/panfrost/panfrost_gem_shrinker.c > @@ -18,8 +18,7 @@ > static unsigned long > panfrost_gem_shrinker_count(struct shrinker *shrinker, struct shrink_control *sc) > { > - struct panfrost_device *pfdev = > - container_of(shrinker, struct panfrost_device, shrinker); > + struct panfrost_device *pfdev = shrinker->private_data; > struct drm_gem_shmem_object *shmem; > unsigned long count = 0; > > @@ -65,8 +64,7 @@ static bool panfrost_gem_purge(struct drm_gem_object *obj) > static unsigned long > panfrost_gem_shrinker_scan(struct shrinker *shrinker, struct shrink_control *sc) > { > - struct panfrost_device *pfdev = > - container_of(shrinker, struct panfrost_device, shrinker); > + struct panfrost_device *pfdev = shrinker->private_data; > struct drm_gem_shmem_object *shmem, *tmp; > unsigned long freed = 0; > > @@ -97,13 +95,24 @@ panfrost_gem_shrinker_scan(struct shrinker *shrinker, struct shrink_control *sc) > * > * This function registers and sets up the panfrost shrinker. > */ > -void panfrost_gem_shrinker_init(struct drm_device *dev) > +int panfrost_gem_shrinker_init(struct drm_device *dev) > { > struct panfrost_device *pfdev = dev->dev_private; > - pfdev->shrinker.count_objects = panfrost_gem_shrinker_count; > - pfdev->shrinker.scan_objects = panfrost_gem_shrinker_scan; > - pfdev->shrinker.seeks = DEFAULT_SEEKS; > - WARN_ON(register_shrinker(&pfdev->shrinker, "drm-panfrost")); > + > + pfdev->shrinker = shrinker_alloc(0, "drm-panfrost"); > + if (!pfdev->shrinker) { > + WARN_ON(1); I don't think this WARN_ON is particularly useful - if there's a failed memory allocation we should see output from the kernel anyway. And we're changing the semantics from "continue just without a shrinker" (which argueably justifies the warning) to "probe fails, device doesn't work" which will be obvious to the user so I don't feel we need the additional warn. > + return -ENOMEM; > + } > + > + pfdev->shrinker->count_objects = panfrost_gem_shrinker_count; > + pfdev->shrinker->scan_objects = panfrost_gem_shrinker_scan; > + pfdev->shrinker->seeks = DEFAULT_SEEKS; > + pfdev->shrinker->private_data = pfdev; > + > + shrinker_register(pfdev->shrinker); > + > + return 0; > } > > /** > @@ -116,7 +125,6 @@ void panfrost_gem_shrinker_cleanup(struct drm_device *dev) > { > struct panfrost_device *pfdev = dev->dev_private; > > - if (pfdev->shrinker.nr_deferred) { > - unregister_shrinker(&pfdev->shrinker); > - } > + if (pfdev->shrinker) > + shrinker_unregister(pfdev->shrinker); > } ______________________________________________________ Linux MTD discussion mailing list http://lists.infradead.org/mailman/listinfo/linux-mtd/