Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::3:6 as permitted sender) client-ip=2620:137:e000::3:6;
Message-ID: <bbbd82a5-41bf-4ca3-476d-e5039e94631b@collabora.com>
Date:   Tue, 3 Oct 2023 03:31:32 +0300
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 v17 13/18] drm/shmem-helper: Add memory shrinker
Content-Language: en-US
To:     Boris Brezillon <boris.brezillon@collabora.com>
Cc:     David Airlie <airlied@gmail.com>,
        Gerd Hoffmann <kraxel@redhat.com>,
        Gurchetan Singh <gurchetansingh@chromium.org>,
        Chia-I Wu <olvaffe@gmail.com>, Daniel Vetter <daniel@ffwll.ch>,
        Maarten Lankhorst <maarten.lankhorst@linux.intel.com>,
        Maxime Ripard <mripard@kernel.org>,
        Thomas Zimmermann <tzimmermann@suse.de>,
        =?UTF-8?Q?Christian_K=c3=b6nig?= <christian.koenig@amd.com>,
        Qiang Yu <yuq825@gmail.com>,
        Steven Price <steven.price@arm.com>,
        Emma Anholt <emma@anholt.net>, Melissa Wen <mwen@igalia.com>,
        dri-devel@lists.freedesktop.org, linux-kernel@vger.kernel.org,
        kernel@collabora.com, virtualization@lists.linux-foundation.org
References: <20230914232721.408581-1-dmitry.osipenko@collabora.com>
 <20230914232721.408581-14-dmitry.osipenko@collabora.com>
 <20230915104633.0d5c3932@collabora.com>
 <454c464e-4534-7ec3-6d38-49b7df83c7be@collabora.com>
 <20230926093517.11a172ad@collabora.com>
From:   Dmitry Osipenko <dmitry.osipenko@collabora.com>
In-Reply-To: <20230926093517.11a172ad@collabora.com>
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 7bit
Precedence: bulk

On 9/26/23 10:35, Boris Brezillon wrote:
>>>> +	__drm_gem_shmem_release_pages(shmem);  
>>> Make sure you drop the implicit pages_use_count ref the sgt had, this
>>> way you can still tie the necessity to drop the pages to sgt != NULL in
>>> drm_gem_shmem_free().  
>> This will require further refcnt re-initialization when pages are
>> restored if it's dropped to zero. I don't see how this will improve
>> anything.
> Sorry to disagree, but I do think it matters to have a clear ownership
> model, and if I look at the code (drm_gem_shmem_get_pages_sgt_locked()),
> the sgt clearly owns a reference to the pages it points to.

It creates too much unnecessary trouble because, again, pages_use_count
can't drop to zero easily. Shrinker doesn't own the refcnt and not
allowed to touch it. The pages_use_count is then used by things like
mmap() and etc that use get_pages(), which can be invoked for evicted GEM.

I'd prefer to keep refcounting as is, don't see how to implement your
suggestion. Will be happy to help with reviewing and testing patches
made on top of this series ;)

-- 
Best regards,
Dmitry