Re: [PATCH 0/4] Allow MMIO regions to be exported through dma-buf

From: "Christian König" <christian.koenig@amd.com>
To: Jason Gunthorpe <jgg@nvidia.com>
Cc: Alex Williamson <alex.williamson@redhat.com>,
	Cornelia Huck <cohuck@redhat.com>,
	dri-devel@lists.freedesktop.org, kvm@vger.kernel.org,
	linaro-mm-sig@lists.linaro.org, linux-media@vger.kernel.org,
	Sumit Semwal <sumit.semwal@linaro.org>,
	Daniel Vetter <daniel.vetter@ffwll.ch>,
	Leon Romanovsky <leon@kernel.org>,
	linux-rdma@vger.kernel.org, Maor Gottlieb <maorg@nvidia.com>,
	Oded Gabbay <ogabbay@kernel.org>
Subject: Re: [PATCH 0/4] Allow MMIO regions to be exported through dma-buf
Date: Fri, 19 Aug 2022 15:47:20 +0200	[thread overview]
Message-ID: <77d47856-4b01-cfb6-bdd6-ccb3f68014d9@amd.com> (raw)
In-Reply-To: <Yv+SfymnK9RF0qTA@nvidia.com>

Am 19.08.22 um 15:39 schrieb Jason Gunthorpe:
> On Fri, Aug 19, 2022 at 03:33:04PM +0200, Christian König wrote:
>
>>>>> So we could delete the try_buf and just rely on move being safe on
>>>>> partially destroyed dma_buf's as part of the API design.
>>>> I think that might be the more defensive approach. A comment on the
>>>> dma_buf_move_notify() function should probably be a good idea.
>>> IMHO, it is an anti-pattern. The caller should hold a strong reference
>>> on an object before invoking any API surface. Upgrading a weak
>>> reference to a strong reference requires the standard "try get" API.
>>>
>>> But if you feel strongly I don't mind dropping the try_get around move.
>> Well I see it as well that both approaches are not ideal, but my gut feeling
>> tells me that just documenting that dma_buf_move_notify() can still be
>> called as long as the release callback wasn't called yet is probably the
>> better approach.
> The comment would say something like:
>
>   "dma_resv_lock(), dma_buf_move_notify(), dma_resv_unlock() may be
>    called with a 0 refcount so long as ops->release() hasn't returned"
>
> Which is a really abnormal API design, IMHO.

Mhm, Daniel or other do you have any opinion on that as well?

Thanks,
Christian.

>
> Jason