Re: [PATCH v3 6/9] dma-buf: Move dma-buf attachment to dynamic locking specification

From: Dmitry Osipenko <dmitry.osipenko@collabora.com>
To: "Christian König" <christian.koenig@amd.com>,
	"David Airlie" <airlied@linux.ie>,
	"Gerd Hoffmann" <kraxel@redhat.com>,
	"Gurchetan Singh" <gurchetansingh@chromium.org>,
	"Chia-I Wu" <olvaffe@gmail.com>,
	"Daniel Vetter" <daniel@ffwll.ch>,
	"Daniel Almeida" <daniel.almeida@collabora.com>,
	"Gert Wollny" <gert.wollny@collabora.com>,
	"Gustavo Padovan" <gustavo.padovan@collabora.com>,
	"Daniel Stone" <daniel@fooishbar.org>,
	"Tomeu Vizoso" <tomeu.vizoso@collabora.com>,
	"Maarten Lankhorst" <maarten.lankhorst@linux.intel.com>,
	"Maxime Ripard" <mripard@kernel.org>,
	"Thomas Zimmermann" <tzimmermann@suse.de>,
	"Rob Clark" <robdclark@gmail.com>,
	"Sumit Semwal" <sumit.semwal@linaro.org>,
	"Pan, Xinhui" <Xinhui.Pan@amd.com>,
	"Thierry Reding" <thierry.reding@gmail.com>,
	"Tomasz Figa" <tfiga@chromium.org>,
	"Marek Szyprowski" <m.szyprowski@samsung.com>,
	"Mauro Carvalho Chehab" <mchehab@kernel.org>,
	"Alex Deucher" <alexander.deucher@amd.com>,
	"Jani Nikula" <jani.nikula@linux.intel.com>,
	"Joonas Lahtinen" <joonas.lahtinen@linux.intel.com>,
	"Rodrigo Vivi" <rodrigo.vivi@intel.com>,
	"Tvrtko Ursulin" <tvrtko.ursulin@linux.intel.com>,
	"Thomas Hellström" <thomas_os@shipmail.org>,
	"Qiang Yu" <yuq825@gmail.com>
Cc: lima@lists.freedesktop.org, linux-rdma@vger.kernel.org,
	linux-arm-msm@vger.kernel.org, intel-gfx@lists.freedesktop.org,
	linux-kernel@vger.kernel.org, amd-gfx@lists.freedesktop.org,
	virtualization@lists.linux-foundation.org,
	linaro-mm-sig@lists.linaro.org, dri-devel@lists.freedesktop.org,
	Dmitry Osipenko <digetx@gmail.com>,
	kernel@collabora.com, linux-media@vger.kernel.org
Subject: Re: [PATCH v3 6/9] dma-buf: Move dma-buf attachment to dynamic locking specification
Date: Wed, 24 Aug 2022 18:22:00 +0300	[thread overview]
Message-ID: <76beb362-09c3-bbff-c50d-794a200cb641@collabora.com> (raw)
In-Reply-To: <b24017e9-e8e7-148e-951b-0f2a1dffdca7@amd.com>

On 8/24/22 18:14, Christian König wrote:
> Am 24.08.22 um 17:03 schrieb Dmitry Osipenko:
>> On 8/24/22 17:08, Christian König wrote:
>>> Am 24.08.22 um 12:22 schrieb Dmitry Osipenko:
>>>> Move dma-buf attachment API functions to the dynamic locking
>>>> specification.
>>>> The strict locking convention prevents deadlock situations for dma-buf
>>>> importers and exporters.
>>>>
>>>> Previously, the "unlocked" versions of the attachment API functions
>>>> weren't taking the reservation lock and this patch makes them to take
>>>> the lock.
>>> Didn't we concluded that we need to keep the attach and detach callbacks
>>> without the lock and only move the map/unmap callbacks over?
>>>
>>> Otherwise it won't be possible for drivers to lock multiple buffers if
>>> they have to shuffle things around for a specific attachment.
>> We did conclude that. The attach/detach dma-buf ops are unlocked, but
>> the map_dma_buf/unmap_dma_buf must be invoked under lock and
>> dma_buf_dynamic_attach_unlocked() maps dma-buf if either importer or
>> exporter can't handle the dynamic mapping [1].
> 
> Ah! You are confusing me over and over again with that :)
> 
> Ok in this case that here is fine, I just need to re-read the patch.

It's indeed not trivial to review this patch, not sure if we can make it
simpler. Maybe it's possible to factor out the changes related to
dynamic mapping, or maybe it's not worthwhile..

Anyways, thank you for helping with reviewing it :)

-- 
Best regards,
Dmitry