Re: [PATCH 07/15] drm/i915: Check the timeout passed to i915_wait_request

From: Tvrtko Ursulin <tvrtko.ursulin@linux.intel.com>
To: Chris Wilson <chris@chris-wilson.co.uk>,
	intel-gfx@lists.freedesktop.org, "Gong,
	Zhipeng" <zhipeng.gong@intel.com>,
	"Rogozhkin, Dmitry V" <dmitry.v.rogozhkin@intel.com>,
	Lionel Landwerlin <lionel.g.landwerlin@linux.intel.com>,
	Tvrtko Ursulin <tvrtko.ursulin@intel.com>,
	Daniel Vetter <daniel.vetter@ffwll.ch>
Subject: Re: [PATCH 07/15] drm/i915: Check the timeout passed to i915_wait_request
Date: Mon, 30 Nov 2015 10:27:27 +0000	[thread overview]
Message-ID: <565C248F.3060208@linux.intel.com> (raw)
In-Reply-To: <20151130101937.GC7587@nuc-i3427.alporthouse.com>

On 30/11/15 10:19, Chris Wilson wrote:
> On Mon, Nov 30, 2015 at 10:14:04AM +0000, Tvrtko Ursulin wrote:
>>
>> On 29/11/15 08:48, Chris Wilson wrote:
>>> We have relied upon the sole caller (wait_ioctl) validating the timeout
>>> argument. However, when waiting for multiple requests I forgot to ensure
>>> that the timeout was still positive on the later requests. This is more
>>> simply done inside __i915_wait_request.
>>
>> As discussed on IRC please mention that the extra jiffie happens
>> because nsecs_to_jiffies_timeout adds it. Otherwise it is not
>> immediately clear why would it wait an extra one since
>> __i915_wait_request has explicit code to ensure timeout does not go
>> negative already.
>
> Sorry, I was under the impression that everyone knew the history of our
> *to_jiffies_timeout function variants.

I don't know if I am the only one who didn't, but when I ask or suggest 
for more or specific comments, or commit message additions during 
review, it is always genuinely for things which I think would have 
helped me with the review.  Which means I also think they would help 
anyone else getting up to speed with the code base.

Regards,

Tvrtko
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx