dri-devel Archive on lore.kernel.org
 help / color / mirror / Atom feed
From: Maarten Lankhorst <maarten.lankhorst@canonical.com>
To: "Christian König" <christian.koenig@amd.com>,
	"Jammy Zhou" <Jammy.Zhou@amd.com>,
	dri-devel@lists.freedesktop.org
Cc: alexander.deucher@amd.com
Subject: Re: [PATCH] reservation: wait only with non-zero timeout specified (v3)
Date: Tue, 13 Jan 2015 11:18:19 +0100	[thread overview]
Message-ID: <54B4F0EB.4030900@canonical.com> (raw)
In-Reply-To: <54B4E3E1.9020401@amd.com>

Op 13-01-15 om 10:22 schreef Christian König:
> Am 13.01.2015 um 10:04 schrieb Maarten Lankhorst:
>> Op 13-01-15 om 10:36 schreef Jammy Zhou:
>>> When the timeout value passed to reservation_object_wait_timeout_rcu
>>> is zero, no wait should be done if the fences are not signaled.
>>>
>>> Return '1' for idle and '0' for busy if the specified timeout is '0'
>>> to keep consistent with the case of non-zero timeout.
>>>
>>> v2: call fence_put if not signaled in the case of timeout==0
>>>
>>>
>> Looks more sane, but where do you need this?
>
> The rational is that we want a single function to call from the driver no matter what timeout we have and get a zero if the call timed out and a non zero value otherwise.
>
> When we called reservation_object_wait_timeout_rcu with a timeout of zero we got a return value of zero as well independent of whether or not the fences were signaled. That behavior looked inconsistent and Jammy is trying to fix this with this patch.
>
> We could of course do the check in the calling code as well, but to me it rather looked like the calling convention of reservation_object_wait_timeout_rcu should be thought over one more time.
>
> Either calling it with timeout=0 is invalid and the driver needs to call reservation_object_test_signaled_rcu directly instead or we apply this patch or something similar to get an useful behavior in the case of timeout=0.
I think it would be best to leave timeout=0 returning 0. Not handling it differently gives the same semantics as used by fence_wait_time and wait_event_timeout.
Are there really many cases in which you don't know if timeout = 0 before or not?

~Maarten

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

  reply	other threads:[~2015-01-13 10:18 UTC|newest]

Thread overview: 9+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2015-01-13  9:36 [PATCH] reservation: wait only with non-zero timeout specified (v3) Jammy Zhou
2015-01-13  9:04 ` Maarten Lankhorst
2015-01-13  9:19   ` Zhou, Jammy
2015-01-13  9:22   ` Christian König
2015-01-13 10:18     ` Maarten Lankhorst [this message]
2015-01-13 22:10       ` Daniel Vetter
2015-01-14  2:16         ` Zhou, Jammy
2015-01-21  9:12           ` Maarten Lankhorst
2015-01-21  9:35             ` Zhou, Jammy

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=54B4F0EB.4030900@canonical.com \
    --to=maarten.lankhorst@canonical.com \
    --cc=Jammy.Zhou@amd.com \
    --cc=alexander.deucher@amd.com \
    --cc=christian.koenig@amd.com \
    --cc=dri-devel@lists.freedesktop.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