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 kanga.kvack.org (kanga.kvack.org [205.233.56.17]) by smtp.lore.kernel.org (Postfix) with ESMTP id 72968C001B3 for ; Fri, 23 Jun 2023 13:33:39 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id AB8BF8D0003; Fri, 23 Jun 2023 09:33:38 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id A41B48D0001; Fri, 23 Jun 2023 09:33:38 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 8BB0C8D0003; Fri, 23 Jun 2023 09:33:38 -0400 (EDT) X-Delivered-To: linux-mm@kvack.org Received: from relay.hostedemail.com (smtprelay0010.hostedemail.com [216.40.44.10]) by kanga.kvack.org (Postfix) with ESMTP id 768098D0001 for ; Fri, 23 Jun 2023 09:33:38 -0400 (EDT) Received: from smtpin23.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay02.hostedemail.com (Postfix) with ESMTP id 2C8AE12047C for ; Fri, 23 Jun 2023 13:33:38 +0000 (UTC) X-FDA: 80934104916.23.04DA380 Received: from mail-pl1-f173.google.com (mail-pl1-f173.google.com [209.85.214.173]) by imf02.hostedemail.com (Postfix) with ESMTP id 85BF380021 for ; Fri, 23 Jun 2023 13:33:35 +0000 (UTC) Authentication-Results: imf02.hostedemail.com; dkim=pass header.d=bytedance.com header.s=google header.b=fa1yyl1X; spf=pass (imf02.hostedemail.com: domain of zhengqi.arch@bytedance.com designates 209.85.214.173 as permitted sender) smtp.mailfrom=zhengqi.arch@bytedance.com; dmarc=pass (policy=quarantine) header.from=bytedance.com ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1687527215; h=from:from: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:dkim-signature; bh=ACoox6lHywQa9wO0MGPPVAOCZzDjfudqnGd2G0lw0os=; b=jjUxU2weNj/3hxHXwGQKL5dVTYQBE5p6WmvE6Me1vjrPBZFvfHuRK2b0RSEGyxlSiFJ5SI vtGSCohGhUoozjQP4EwibCLMrI1y8XGxGvD78gCjg9okK9c/rGD8N7t37oL/ibkY/OWq0d HqUeNNT3/Rqc1dz764iKrhWIO2k3j+I= ARC-Authentication-Results: i=1; imf02.hostedemail.com; dkim=pass header.d=bytedance.com header.s=google header.b=fa1yyl1X; spf=pass (imf02.hostedemail.com: domain of zhengqi.arch@bytedance.com designates 209.85.214.173 as permitted sender) smtp.mailfrom=zhengqi.arch@bytedance.com; dmarc=pass (policy=quarantine) header.from=bytedance.com ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1687527215; a=rsa-sha256; cv=none; b=V5/NkS8nvfuCIlbrABzQb+DGTY83r+18ZG/byKi637Nx8p0wVPzKu7DUzGsZaUWO6e96A1 XFsA9c5jkD56ZBN9XH5NFF9E3bqTIYfvTwIHOn52jTKTm0Pe44MVhLMpLTqc4ZO1IP663q Gn0HsEx5O/Wn03cJmfVybRLYWr+uyvE= Received: by mail-pl1-f173.google.com with SMTP id d9443c01a7336-1b52418c25bso936925ad.0 for ; Fri, 23 Jun 2023 06:33:34 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=bytedance.com; s=google; t=1687527214; x=1690119214; h=content-transfer-encoding:in-reply-to:from:references:cc:to :content-language:subject:user-agent:mime-version:date:message-id :from:to:cc:subject:date:message-id:reply-to; bh=ACoox6lHywQa9wO0MGPPVAOCZzDjfudqnGd2G0lw0os=; b=fa1yyl1XuChlZVeuHnAOkkfkq2hZyLQlU7Qx7FIzebWZlha3QE3pftRSrj+DJxI7Rv UhXvlkHo3Vcdolq+ME68HhnTz0k5XIGoqWorIAsqCzinXOgMm/YBV2p6rPjxxhB9xA+p CKbtixmq104K/XaeP7c/FRQEzfBhOolOJ13XEqLz2Vl/3W9UCdfjI5o3p38fxwPZQ7jM f2Nv1spQskpxwNqmrAc0KV9Etlyt/W0NlU64HVCq3FTgTv7qi6xblmtVtEFrypVRLxTd swjUrDnQOk53j5e/5rffnzu2n8zAvNVx/4KpVQ/7vpT1VvkShyKZT45f4U4Mq3kIEUCc sA+A== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20221208; t=1687527214; x=1690119214; h=content-transfer-encoding:in-reply-to:from:references:cc:to :content-language:subject:user-agent:mime-version:date:message-id :x-gm-message-state:from:to:cc:subject:date:message-id:reply-to; bh=ACoox6lHywQa9wO0MGPPVAOCZzDjfudqnGd2G0lw0os=; b=E7tCDOPkL8AR16gy+QydBUp2+1NbGCDzHzt4gzzg/Ibi20dAuN7DbsjGzSvQQr8w76 3OZU6MP/w6Xh1SEO8S1Vhn2TVDzvdS4hrCob7TXtxMc1NZHeP5NDp7I0GaJvlsP17qRk JCoG7KhSjap+ojl6vnz0bTxsdN8gJI84I7pHUbQeloNkQpQw69fyXx8ZHFaU47ANWE7H hv2Ec733eA8kbCTT2LwDN5s2werr19K24z5x2kHZB9X5wvsBUO5I3BkmloYsnsw7JWtI c86JJPD5n2KkhWTBzRrY2eD+L6W1Mu7WOm1oG7NAxQNAdEIRD//aKjjBxDZ6Vb+NlwHT TyaA== X-Gm-Message-State: AC+VfDz89qIkYDjURzfYo6ufa8xLLZcF7d/fp9Nw4dRaZ+iIoR3KvdYc D7e6QhgUswWONkcAZF6eAb3ing== X-Google-Smtp-Source: ACHHUZ7fC8cfi9SbpxWbw1P19vY2WFeiY5bxjWE/lWxj83YOdeMLdFfAi7QBoo42W5+SZOFqK06Ryw== X-Received: by 2002:a17:902:c945:b0:1ae:3ff8:7fa7 with SMTP id i5-20020a170902c94500b001ae3ff87fa7mr26069073pla.4.1687527213878; Fri, 23 Jun 2023 06:33:33 -0700 (PDT) Received: from [10.4.168.167] ([139.177.225.254]) by smtp.gmail.com with ESMTPSA id kg14-20020a170903060e00b001b6a27dff99sm4341406plb.159.2023.06.23.06.33.26 (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Fri, 23 Jun 2023 06:33:33 -0700 (PDT) Message-ID: <43a07dbe-5049-8596-da58-51e0a0d6243c@bytedance.com> Date: Fri, 23 Jun 2023 21:33:24 +0800 MIME-Version: 1.0 User-Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10.15; rv:102.0) Gecko/20100101 Thunderbird/102.12.0 Subject: Re: [PATCH 05/29] drm/panfrost: dynamically allocate the drm-panfrost shrinker Content-Language: en-US To: Steven Price Cc: linux-kernel@vger.kernel.org, linux-mm@kvack.org, intel-gfx@lists.freedesktop.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-nfs@vger.kernel.org, linux-xfs@vger.kernel.org, linux-btrfs@vger.kernel.org, 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 References: <20230622085335.77010-1-zhengqi.arch@bytedance.com> <20230622085335.77010-6-zhengqi.arch@bytedance.com> From: Qi Zheng In-Reply-To: <20230622085335.77010-6-zhengqi.arch@bytedance.com> Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 7bit X-Rspamd-Queue-Id: 85BF380021 X-Rspam-User: X-Stat-Signature: z4nzrwzd8z8y49ne5guq4jb9kd9qrja3 X-Rspamd-Server: rspam01 X-HE-Tag: 1687527215-564847 X-HE-Meta: U2FsdGVkX199R9gGWv6rXIJn8N5/g7K3CsSoDUiwyr/1sdgcVKS8jldHCor53+huzD2mIdKFF/leyQGWGmxshvKrlNhspgTbciuwfEIsAvI+PtePYxKGjYZkJ+EIvGQFiiAlvwK2YsYc5Te4vBuryUKFd7dxnJEjf0eu9fWhr9hIJ4ZeTcx8lChWvHGJ8AFOqiLExDmRY1z0kszrU48DLMQQN2Fd/a9Ft4+CY7ipNCGuiwHKwndQbf82F1yq7lJbqnjX44qFSTbZ2yvjylKt5uzBLoaSilhKuNfCuX3FsMoRdC4mFDlGr6ekadhq7yI4xwrKXIz2POBJUB6Y4/68gstTKs67dpDyMqHLS++2VvFNkk7QpRA/ivLMvh1jyejUGrkyFxRctRo/jvygpQhyrSBGqdKhufSBf0bGdNq57UVz/vWSFiYlTbjG9abjelIBKdaSsS7YubrQctxIF03V5eZGKBGoTn6qejbsNV6rUogReCRuV3TcqPdGrPR0rvA6sUAv7ggFS2gBdVQgpk8zmO1uHwvH1o9UV00O8RjXCumOvoupuTz2p1stZl7h+aVbnfUhHyPA82nLpDDy4AjpN4UZFycoRzKe6ufXz8D1kZ0E9LhJnRLyFfB5lZw0g2iF2YJbGQSrD1tay7mUNHiBqIH6ymIg6OyTS7HFAhtTui4PEkLOpM2sC9EgJpnXIG3LcabrHaLt9HrEDNtP9SgFIbECG+S9ZujUaOfgWrKKu9LhGyLU/DZjmSuW2Y6/opi4CPUhviCG/x6ylMPtqM3HPHixFEK978lENRU9f+/XklDq1JRIhc63nqCTtggVNhpGK7grMTW5ZRAglmvz3d9oimgbn07eHwoORu0HGOT/gxCP5RzgtHr5Xu3eO3hVS2k7v0p+3xfyNchqjJbf94AetGnxgjSCeAJ4imB5BEbS7mM0OpQCdwAsF57tjMnfbjswfVKE777PM7bi8SYvPhp vJ7PF/RE O1tIGmK+Z19aqqW6uWwRMMw7Gev1Lzh0GhdwGnU93TXlQXG3Nv/H44lSw3kPS2/m6233BzfnltPRkrscStwZP6jWo0lPOmtgaMg/PD40NQcy2hlEM0USNtxc5kSEFEFI7J3GrGmY/T5rzteJQrD5MDwlVusVZLWgD3jwWfUn//lJF6wF3soEUhYw+ajIhuvmV3QpIxInQqfA+jwvPD3jmzARBFH3b8vs2Isij0k1QLbrZq/GHdV5GsTw/gSKoxHKSx/bk4ZxVChjs30Cm+o8Ns6yU24SY/nsJ+3YQXsHTrjxd0ktyP4lAVjgklnXBfQaNug52R1D2bDeyAfWDlY1WCAP7kXs21nSPG28Mqu9lduOCLQrcPmbNVwNRGbklWasgSQlv8gT4nqQJLrmmJAVrNQ6sffPC9n1vUEARiXds61woH6g= X-Bogosity: Ham, tests=bogofilter, spamicity=0.000000, version=1.2.4 Sender: owner-linux-mm@kvack.org Precedence: bulk X-Loop: owner-majordomo@kvack.org List-ID: Hi Steven, The email you replied to was the failed version (due to the error below), so I copied your reply and replied to you on this successful version. (4.7.1 Error: too many recipients from 49.7.199.173) On 2023/6/23 18:01, Steven Price wrote: > On 22/06/2023 09:39, Qi Zheng wrote: >> From: Qi Zheng >> >> In preparation for implementing lockless slab shrink, >> we need 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 >> --- >> drivers/gpu/drm/panfrost/panfrost_device.h | 2 +- >> .../gpu/drm/panfrost/panfrost_gem_shrinker.c | 24 ++++++++++--------- >> 2 files changed, 14 insertions(+), 12 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_gem_shrinker.c b/drivers/gpu/drm/panfrost/panfrost_gem_shrinker.c >> index bf0170782f25..2a5513eb9e1f 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; >> >> @@ -100,10 +98,15 @@ panfrost_gem_shrinker_scan(struct shrinker *shrinker, struct shrink_control *sc) >> void 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_and_init(panfrost_gem_shrinker_count, >> + panfrost_gem_shrinker_scan, 0, >> + DEFAULT_SEEKS, 0, pfdev); >> + if (pfdev->shrinker && >> + register_shrinker(pfdev->shrinker, "drm-panfrost")) { >> + shrinker_free(pfdev->shrinker); >> + WARN_ON(1); >> + } > > So we didn't have good error handling here before, but this is > significantly worse. Previously if register_shrinker() failed then the > driver could safely continue without a shrinker - it would waste memory > but still function. > > However we now have two failure conditions: > * shrinker_alloc_init() returns NULL. No warning and NULL deferences > will happen later. > > * register_shrinker() fails, shrinker_free() will free pdev->shrinker > we get a warning, but followed by a use-after-free later. > > I think we need to modify panfrost_gem_shrinker_init() to be able to > return an error, so a change something like the below (untested) before > your change. Indeed. I will fix it in the v2. Thanks, Qi > > Steve > > ----8<--- > 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..90265b37636f 100644 > --- a/drivers/gpu/drm/panfrost/panfrost_gem_shrinker.c > +++ b/drivers/gpu/drm/panfrost/panfrost_gem_shrinker.c > @@ -97,13 +97,17 @@ 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; > + int ret; > + > 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")); > + ret = register_shrinker(&pfdev->shrinker, "drm-panfrost"); > + > + return ret; > } > > /** >