mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
From: "Christian König" <christian.koenig@amd.com>
To: phasta@kernel.org, Sumit Semwal <sumit.semwal@linaro.org>
Cc: linux-media@vger.kernel.org, dri-devel@lists.freedesktop.org,
	linux-kernel@vger.kernel.org
Subject: Re: [PATCH] dma-buf/dma-fence: Mark two callbacks as deprecated
Date: Fri, 25 Sep 2026 10:21:34 +0200	[thread overview]
Message-ID: <c7d9c659-d903-456f-87f2-d75aafb8de3b@amd.com> (raw)
In-Reply-To: <a4655e4ab7a2477b75351a0e83b98e0cb0396fdb.camel@mailbox.org>

On 9/24/26 10:14, Philipp Stanner wrote:
> On Wed, 2026-09-23 at 17:35 +0200, Christian König wrote:
>> On 9/23/26 17:27, Philipp Stanner wrote:
>>> On Wed, 2026-09-23 at 17:13 +0200, Christian König wrote:
>>>> On 9/23/26 17:03, Philipp Stanner wrote:
>>>>>
>>>
>>> […]
>>>
>>>>
>>>>> 	Consumers of a fence can instead notify themselves by
>>>>> +	 * registering a callback on the fence.
>>>>
>>>> Mhm, the wait callback is transparent to consumers it's just that
>>>> implementations used it for quite a number of different hacks.
>>>
>>> Right…
>>>
>>> but doesn't the question then become why dma_fence_wait_timeout() even
>>> exists? IOW, shall we deprecate it, too?
>>
>> Yes, without the wait callback it is only a wrapper to block the
>> current thread for a dma_fence to signal using a callback.
> 
> I agree that it's probably quite a common use-case. I'm not sure
> whether it's possible to write a convenient wrapper, though, since you
> need to carry a waitqueue around.
> 
> Maybe we can put a task for it onto the DRM TODO list?

Maybe, but I'm not even sure if that is even possible/doable/make sense now.

A dma_fence is indeed very similar to a waitqueue, but with different locking semantics (at least at the moment) and different callbacks etc...

All of that is changeable, e.g. no uAPI dependencies, but also rather tricky to do because a lot of different components are involved.

On the other hand that code now works, it is just quite awkward to re-implement more or less the same functionality as a workqueue.
>>
>> It's still quite useful to have a common function for that I think.
>>
>>> It seems to be a reimplementation of waitqueues. The driver could get
>>> this functionality by using a waitqueue whose event gets triggered by a
>>> fence callback.
>>>
>>> dma_fence_default_wait() interacts directly with the task state with
>>> __XX_task() functions which looks very.. deep to me :)
>>
>> That is *exactly* what I pointed out as well >10 years ago before that stuff was merged upstream :)
>>
>> A wait_event based implementation would be tons of cleaner if you ask me.
> 
> So you objected and it was merged anyways? With any rationale?

Well not quite, I didn't explicitly NAKed it.

I just pointed out the different problems I saw, but at that time nobody (including me) expected that I was Nostradamus foretelling the future and putting the finger on exactly what we have forgotten to take into account.

A good bunch of the issues have been fixed over the years. Especially the dma_fence today is way more resilient to coding errors it was in the beginning, we basically had random memory corruptions all over the place because of avoidable driver bugs.

Some problems like the locking design are still WIP, but we are slowly moving towards that.

But some problems like parts of the dma_fence uAPI are unfixable without time travel.

> I think I understand now why sometimes people apply a Nacked-by, so
> that it's documented that people objected against merging.

Well I rather learned that I should take all concerns into account and explicitly say NAK when I see something fundamentally problematic which can't be fixed later on.

Regards,
Christian.

> 
> […]
> 
>>>
>>>
>>> Well, what I'm trying to say in this docu is that the driver can kick
>>> off custom operations that shall be performed once everyone is "done"
>>> with the fence after signaling it. Any driver data that might still be
>>> around cannot be accessed by fence consumers after signaling anymore.
>>> So the driver could trigger cleanup work after a graceperiod, as long
>>> as it does not involve kfree()-ing the fence itself.
>>
>> That sounds sane to me, but I'm not sure how to phrase it cleaner either.
>>
>> For now I'm ok with it, maybe somebody else has a better idea to how write this.
> 
> I try to come up with something slightly better.
> 
> 
> P.


  reply	other threads:[~2026-09-25  8:21 UTC|newest]

Thread overview: 7+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-09-23 15:03 Philipp Stanner
2026-09-23 15:13 ` Christian König
2026-09-23 15:27   ` Philipp Stanner
2026-09-23 15:35     ` Christian König
2026-09-24  8:14       ` Philipp Stanner
2026-09-25  8:21         ` Christian König [this message]
2026-09-25  8:42           ` Philipp Stanner

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=c7d9c659-d903-456f-87f2-d75aafb8de3b@amd.com \
    --to=christian.koenig@amd.com \
    --cc=dri-devel@lists.freedesktop.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-media@vger.kernel.org \
    --cc=phasta@kernel.org \
    --cc=sumit.semwal@linaro.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox

all inboxes | Powered by JetHome®