[Intel-gfx] [Linaro-mm-sig] [PATCH v3 6/9] dma-buf: Move dma-buf attachment to dynamic locking specification
Dmitry Osipenko
dmitry.osipenko at collabora.com
Wed Aug 24 15:49:28 UTC 2022
On 8/24/22 18:24, 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.
>>
>> Intel and AMD GPU drivers already were mapping the attached dma-bufs
>> under
>> the held lock during attachment, hence these drivers are updated to use
>> the locked functions.
>>
>> Signed-off-by: Dmitry Osipenko <dmitry.osipenko at collabora.com>
>> ---
>> drivers/dma-buf/dma-buf.c | 115 ++++++++++++++-------
>> drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c | 4 +-
>> drivers/gpu/drm/i915/gem/i915_gem_dmabuf.c | 8 +-
>> drivers/gpu/drm/i915/gem/i915_gem_object.c | 12 +++
>> include/linux/dma-buf.h | 20 ++--
>> 5 files changed, 110 insertions(+), 49 deletions(-)
>>
>> diff --git a/drivers/dma-buf/dma-buf.c b/drivers/dma-buf/dma-buf.c
>> index 4556a12bd741..f2a5a122da4a 100644
>> --- a/drivers/dma-buf/dma-buf.c
>> +++ b/drivers/dma-buf/dma-buf.c
>> @@ -559,7 +559,7 @@ static struct file *dma_buf_getfile(struct dma_buf
>> *dmabuf, int flags)
>> * 2. Userspace passes this file-descriptors to all drivers it wants
>> this buffer
>> * to share with: First the file descriptor is converted to a
>> &dma_buf using
>> * dma_buf_get(). Then the buffer is attached to the device using
>> - * dma_buf_attach().
>> + * dma_buf_attach_unlocked().
>
> Now I get why this is confusing me so much.
>
> The _unlocked postfix implies that there is another function which
> should be called with the locks already held, but this is not the case
> for attach/detach (because they always need to grab the lock themselves).
That's correct. The attach/detach ops of exporter can take the lock
(like i915 exporter does it), hence importer must not grab the lock
around dma_buf_attach() invocation.
> So I suggest to drop the _unlocked postfix for the attach/detach
> functions. Another step would then be to unify attach/detach with
> dynamic_attach/dynamic_detach when both have the same locking convention
> anyway.
It's not a problem to change the name, but it's unclear to me why we
should do it. The _unlocked postfix tells importer that reservation must
be unlocked and it must be unlocked in case of dma_buf_attach().
Dropping the postfix will make dma_buf_attach() inconsistent with the
rest of the _unlocked functions(?). Are you sure we need to rename it?
> Sorry that this is going so much back and forth, it's really complicated
> to keep all the stuff in my head at the moment :)
Not a problem at all, I expected that it will take some time for this
patchset to settle down.
--
Best regards,
Dmitry
More information about the Intel-gfx
mailing list