We've lost the +1 required for correct timeouts in
commit 5ed0bdf21a85d78e04f89f15ccf227562177cbd9
Author: Thomas Gleixner <[email protected]>
Date: Wed Jul 16 21:05:06 2014 +0000
drm: i915: Use nsec based interfaces
Use ktime_get_raw_ns() and get rid of the back and forth timespec
conversions.
Signed-off-by: Thomas Gleixner <[email protected]>
Acked-by: Daniel Vetter <[email protected]>
Signed-off-by: John Stultz <[email protected]>
So fix this up by reinstating our handrolled _timeout function. While
at it bother with handling MAX_JIFFIES.
v2: Convert to usecs (we don't care about the accuracy anyway) first
to avoid overflow issues Dave Gordon spotted.
Cc: Dave Gordon <[email protected]>
Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=82749
Cc: Thomas Gleixner <[email protected]>
Cc: John Stultz <[email protected]>
Signed-off-by: Daniel Vetter <[email protected]>
---
drivers/gpu/drm/i915/i915_drv.h | 11 +++++++++++
drivers/gpu/drm/i915/i915_gem.c | 3 ++-
2 files changed, 13 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 049482f5d9ac..ef1f00e0a7b3 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -3097,6 +3097,17 @@ static inline unsigned long msecs_to_jiffies_timeout(const unsigned int m)
return min_t(unsigned long, MAX_JIFFY_OFFSET, j + 1);
}
+static inline unsigned long nsecs_to_jiffies_timeout(const u64 m)
+{
+ u64 usecs = div_u64(m + 999, 1000);
+ unsigned long j = usecs_to_jiffies(usecs);
+
+ if (usecs > (u64)jiffies_to_usecs(MAX_JIFFY_OFFSET))
+ return MAX_JIFFY_OFFSET;
+
+ return min_t(unsigned long, MAX_JIFFY_OFFSET, j + 1);
+}
+
static inline unsigned long
timespec_to_jiffies_timeout(const struct timespec *value)
{
diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
index 9d362d320d82..04a9f26407ae 100644
--- a/drivers/gpu/drm/i915/i915_gem.c
+++ b/drivers/gpu/drm/i915/i915_gem.c
@@ -1226,7 +1226,8 @@ int __i915_wait_request(struct drm_i915_gem_request *req,
if (i915_gem_request_completed(req, true))
return 0;
- timeout_expire = timeout ? jiffies + nsecs_to_jiffies((u64)*timeout) : 0;
+ timeout_expire = timeout ?
+ jiffies + nsecs_to_jiffies_timeout((u64)*timeout) : 0;
if (INTEL_INFO(dev)->gen >= 6 && ring->id == RCS && can_wait_boost(file_priv)) {
gen6_rps_boost(dev_priv);
--
1.9.3
We've lost the +1 required for correct timeouts in
commit 5ed0bdf21a85d78e04f89f15ccf227562177cbd9
Author: Thomas Gleixner <[email protected]>
Date: Wed Jul 16 21:05:06 2014 +0000
drm: i915: Use nsec based interfaces
Use ktime_get_raw_ns() and get rid of the back and forth timespec
conversions.
Signed-off-by: Thomas Gleixner <[email protected]>
Acked-by: Daniel Vetter <[email protected]>
Signed-off-by: John Stultz <[email protected]>
So fix this up by reinstating our handrolled _timeout function. While
at it bother with handling MAX_JIFFIES.
v2: Convert to usecs (we don't care about the accuracy anyway) first
to avoid overflow issues Dave Gordon spotted.
v3: Drop the explicit MAX_JIFFY_OFFSET check, usecs_to_jiffies should
take care of that already. It might be a bit too enthusiastic about it
though.
Cc: Dave Gordon <[email protected]>
Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=82749
Cc: Thomas Gleixner <[email protected]>
Cc: John Stultz <[email protected]>
Signed-off-by: Daniel Vetter <[email protected]>
---
drivers/gpu/drm/i915/i915_drv.h | 8 ++++++++
drivers/gpu/drm/i915/i915_gem.c | 3 ++-
2 files changed, 10 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 049482f5d9ac..4ea14a8c31f7 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -3097,6 +3097,14 @@ static inline unsigned long msecs_to_jiffies_timeout(const unsigned int m)
return min_t(unsigned long, MAX_JIFFY_OFFSET, j + 1);
}
+static inline unsigned long nsecs_to_jiffies_timeout(const u64 m)
+{
+ u64 usecs = div_u64(m + 999, 1000);
+ unsigned long j = usecs_to_jiffies(usecs);
+
+ return min_t(unsigned long, MAX_JIFFY_OFFSET, j + 1);
+}
+
static inline unsigned long
timespec_to_jiffies_timeout(const struct timespec *value)
{
diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
index 9d362d320d82..04a9f26407ae 100644
--- a/drivers/gpu/drm/i915/i915_gem.c
+++ b/drivers/gpu/drm/i915/i915_gem.c
@@ -1226,7 +1226,8 @@ int __i915_wait_request(struct drm_i915_gem_request *req,
if (i915_gem_request_completed(req, true))
return 0;
- timeout_expire = timeout ? jiffies + nsecs_to_jiffies((u64)*timeout) : 0;
+ timeout_expire = timeout ?
+ jiffies + nsecs_to_jiffies_timeout((u64)*timeout) : 0;
if (INTEL_INFO(dev)->gen >= 6 && ring->id == RCS && can_wait_boost(file_priv)) {
gen6_rps_boost(dev_priv);
--
1.9.3
On Tue, Dec 02, 2014 at 04:36:22PM +0100, Daniel Vetter wrote:
> We've lost the +1 required for correct timeouts in
>
> commit 5ed0bdf21a85d78e04f89f15ccf227562177cbd9
> Author: Thomas Gleixner <[email protected]>
> Date: Wed Jul 16 21:05:06 2014 +0000
>
> drm: i915: Use nsec based interfaces
>
> Use ktime_get_raw_ns() and get rid of the back and forth timespec
> conversions.
>
> Signed-off-by: Thomas Gleixner <[email protected]>
> Acked-by: Daniel Vetter <[email protected]>
> Signed-off-by: John Stultz <[email protected]>
>
> So fix this up by reinstating our handrolled _timeout function. While
> at it bother with handling MAX_JIFFIES.
>
> v2: Convert to usecs (we don't care about the accuracy anyway) first
> to avoid overflow issues Dave Gordon spotted.
>
> v3: Drop the explicit MAX_JIFFY_OFFSET check, usecs_to_jiffies should
> take care of that already. It might be a bit too enthusiastic about it
> though.
>
> Cc: Dave Gordon <[email protected]>
> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=82749
> Cc: Thomas Gleixner <[email protected]>
> Cc: John Stultz <[email protected]>
> Signed-off-by: Daniel Vetter <[email protected]>
> ---
> drivers/gpu/drm/i915/i915_drv.h | 8 ++++++++
> drivers/gpu/drm/i915/i915_gem.c | 3 ++-
> 2 files changed, 10 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> index 049482f5d9ac..4ea14a8c31f7 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -3097,6 +3097,14 @@ static inline unsigned long msecs_to_jiffies_timeout(const unsigned int m)
> return min_t(unsigned long, MAX_JIFFY_OFFSET, j + 1);
> }
>
> +static inline unsigned long nsecs_to_jiffies_timeout(const u64 m)
> +{
> + u64 usecs = div_u64(m + 999, 1000);
> + unsigned long j = usecs_to_jiffies(usecs);
> +
> + return min_t(unsigned long, MAX_JIFFY_OFFSET, j + 1);
Or more concisely and review friendly:
static inline unsigned long nsecs_to_jiffies_timeout(const u64 n)
{
return min_t(u64, MAX_JIFFY_OFFSET, nsecs_to_jiffies64(n) + 1);
}
-Chris
--
Chris Wilson, Intel Open Source Technology Centre
On Tue, Dec 2, 2014 at 8:35 AM, Chris Wilson <[email protected]> wrote:
> On Tue, Dec 02, 2014 at 04:36:22PM +0100, Daniel Vetter wrote:
>> +static inline unsigned long nsecs_to_jiffies_timeout(const u64 m)
>> +{
>> + u64 usecs = div_u64(m + 999, 1000);
>> + unsigned long j = usecs_to_jiffies(usecs);
>> +
>> + return min_t(unsigned long, MAX_JIFFY_OFFSET, j + 1);
>
> Or more concisely and review friendly:
>
> static inline unsigned long nsecs_to_jiffies_timeout(const u64 n)
> {
> return min_t(u64, MAX_JIFFY_OFFSET, nsecs_to_jiffies64(n) + 1);
> }
Yea. This looks much nicer. Seems generic enough it might be better
added next to nsec_to_jiffies64() in kernel/time/time.c or jiffies.h
rather then in a driver header.
And clearly the header comment in nsec_to_jiffies() warning its only
for the scheduler and not for use for drivers (for exactly the reason
of this patch) are not obvious/memorable enough for me and Thomas
makes me wonder if we should change its name to be more clear that its
a sched only function.
thanks
-john
On Tue, Dec 02, 2014 at 08:54:13AM -0800, John Stultz wrote:
> On Tue, Dec 2, 2014 at 8:35 AM, Chris Wilson <[email protected]> wrote:
> > On Tue, Dec 02, 2014 at 04:36:22PM +0100, Daniel Vetter wrote:
> >> +static inline unsigned long nsecs_to_jiffies_timeout(const u64 m)
> >> +{
> >> + u64 usecs = div_u64(m + 999, 1000);
> >> + unsigned long j = usecs_to_jiffies(usecs);
> >> +
> >> + return min_t(unsigned long, MAX_JIFFY_OFFSET, j + 1);
> >
> > Or more concisely and review friendly:
> >
> > static inline unsigned long nsecs_to_jiffies_timeout(const u64 n)
> > {
> > return min_t(u64, MAX_JIFFY_OFFSET, nsecs_to_jiffies64(n) + 1);
> > }
>
> Yea. This looks much nicer. Seems generic enough it might be better
> added next to nsec_to_jiffies64() in kernel/time/time.c or jiffies.h
> rather then in a driver header.
>
> And clearly the header comment in nsec_to_jiffies() warning its only
> for the scheduler and not for use for drivers (for exactly the reason
> of this patch) are not obvious/memorable enough for me and Thomas
> makes me wonder if we should change its name to be more clear that its
> a sched only function.
This bug here isn't about nsect_to_jiffies vs the 64 bit variant, but
about the +1 that we need to not have a short sleep. In i915 we have a
bunch of jiffies_timeout functions which do just the +1 compared to the
versions in time.c because we screwed this up too often.
Iirc I did float an rfc to move these to time.c once but it resulted in
some bikeshed fest (no, I'm not going to audit every single user of
existing _to_jiffies functions). If there's interest I could try again,
the i915 versions are in drivers/gpu/drm/i915/i915_drv.h.
Cheers, Daniel
--
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
On Wed, 2014-12-03 at 10:22 +0100, Daniel Vetter wrote:
> On Tue, Dec 02, 2014 at 08:54:13AM -0800, John Stultz wrote:
> > On Tue, Dec 2, 2014 at 8:35 AM, Chris Wilson <[email protected]> wrote:
> > > On Tue, Dec 02, 2014 at 04:36:22PM +0100, Daniel Vetter wrote:
> > >> +static inline unsigned long nsecs_to_jiffies_timeout(const u64 m)
> > >> +{
> > >> + u64 usecs = div_u64(m + 999, 1000);
> > >> + unsigned long j = usecs_to_jiffies(usecs);
> > >> +
> > >> + return min_t(unsigned long, MAX_JIFFY_OFFSET, j + 1);
> > >
> > > Or more concisely and review friendly:
> > >
> > > static inline unsigned long nsecs_to_jiffies_timeout(const u64 n)
> > > {
> > > return min_t(u64, MAX_JIFFY_OFFSET, nsecs_to_jiffies64(n) + 1);
> > > }
> >
> > Yea. This looks much nicer. Seems generic enough it might be better
> > added next to nsec_to_jiffies64() in kernel/time/time.c or jiffies.h
> > rather then in a driver header.
> >
> > And clearly the header comment in nsec_to_jiffies() warning its only
> > for the scheduler and not for use for drivers (for exactly the reason
> > of this patch) are not obvious/memorable enough for me and Thomas
> > makes me wonder if we should change its name to be more clear that its
> > a sched only function.
>
> This bug here isn't about nsect_to_jiffies vs the 64 bit variant, but
> about the +1 that we need to not have a short sleep. In i915 we have a
> bunch of jiffies_timeout functions which do just the +1 compared to the
> versions in time.c because we screwed this up too often.
>
> Iirc I did float an rfc to move these to time.c once but it resulted in
> some bikeshed fest (no, I'm not going to audit every single user of
> existing _to_jiffies functions). If there's interest I could try again,
> the i915 versions are in drivers/gpu/drm/i915/i915_drv.h.
There was at least this attempt:
https://lkml.org/lkml/2013/5/10/187
--Imre
On Tue, Dec 02, 2014 at 08:54:13AM -0800, John Stultz wrote:
> On Tue, Dec 2, 2014 at 8:35 AM, Chris Wilson <[email protected]> wrote:
> > On Tue, Dec 02, 2014 at 04:36:22PM +0100, Daniel Vetter wrote:
> >> +static inline unsigned long nsecs_to_jiffies_timeout(const u64 m)
> >> +{
> >> + u64 usecs = div_u64(m + 999, 1000);
> >> + unsigned long j = usecs_to_jiffies(usecs);
> >> +
> >> + return min_t(unsigned long, MAX_JIFFY_OFFSET, j + 1);
> >
> > Or more concisely and review friendly:
> >
> > static inline unsigned long nsecs_to_jiffies_timeout(const u64 n)
> > {
> > return min_t(u64, MAX_JIFFY_OFFSET, nsecs_to_jiffies64(n) + 1);
> > }
>
> Yea. This looks much nicer. Seems generic enough it might be better
> added next to nsec_to_jiffies64() in kernel/time/time.c or jiffies.h
> rather then in a driver header.
Ok, that needs an EXPORT_SYMBOL for nsecs_to_jiffies64. Can I count your
"Yea" above as an ack for adding that and pulling it in through
drm-intel.git?
-Daniel
--
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
On Wed, Dec 3, 2014 at 6:30 AM, Daniel Vetter <[email protected]> wrote:
> On Tue, Dec 02, 2014 at 08:54:13AM -0800, John Stultz wrote:
>> On Tue, Dec 2, 2014 at 8:35 AM, Chris Wilson <[email protected]> wrote:
>> > On Tue, Dec 02, 2014 at 04:36:22PM +0100, Daniel Vetter wrote:
>> >> +static inline unsigned long nsecs_to_jiffies_timeout(const u64 m)
>> >> +{
>> >> + u64 usecs = div_u64(m + 999, 1000);
>> >> + unsigned long j = usecs_to_jiffies(usecs);
>> >> +
>> >> + return min_t(unsigned long, MAX_JIFFY_OFFSET, j + 1);
>> >
>> > Or more concisely and review friendly:
>> >
>> > static inline unsigned long nsecs_to_jiffies_timeout(const u64 n)
>> > {
>> > return min_t(u64, MAX_JIFFY_OFFSET, nsecs_to_jiffies64(n) + 1);
>> > }
>>
>> Yea. This looks much nicer. Seems generic enough it might be better
>> added next to nsec_to_jiffies64() in kernel/time/time.c or jiffies.h
>> rather then in a driver header.
>
> Ok, that needs an EXPORT_SYMBOL for nsecs_to_jiffies64. Can I count your
> "Yea" above as an ack for adding that and pulling it in through
> drm-intel.git?
Do you need an EXPORT_SYMBOL if you add the _timeout version next to
nsecs_to_jiffies64 in time.c?
Otherwise no objections to the approach, but I'd like to properly do
an Acked-by: after I see the patch. :)
thanks
-john
On Wed, Dec 03, 2014 at 11:07:08AM -0800, John Stultz wrote:
> On Wed, Dec 3, 2014 at 6:30 AM, Daniel Vetter <[email protected]> wrote:
> > On Tue, Dec 02, 2014 at 08:54:13AM -0800, John Stultz wrote:
> >> On Tue, Dec 2, 2014 at 8:35 AM, Chris Wilson <[email protected]> wrote:
> >> > On Tue, Dec 02, 2014 at 04:36:22PM +0100, Daniel Vetter wrote:
> >> >> +static inline unsigned long nsecs_to_jiffies_timeout(const u64 m)
> >> >> +{
> >> >> + u64 usecs = div_u64(m + 999, 1000);
> >> >> + unsigned long j = usecs_to_jiffies(usecs);
> >> >> +
> >> >> + return min_t(unsigned long, MAX_JIFFY_OFFSET, j + 1);
> >> >
> >> > Or more concisely and review friendly:
> >> >
> >> > static inline unsigned long nsecs_to_jiffies_timeout(const u64 n)
> >> > {
> >> > return min_t(u64, MAX_JIFFY_OFFSET, nsecs_to_jiffies64(n) + 1);
> >> > }
> >>
> >> Yea. This looks much nicer. Seems generic enough it might be better
> >> added next to nsec_to_jiffies64() in kernel/time/time.c or jiffies.h
> >> rather then in a driver header.
> >
> > Ok, that needs an EXPORT_SYMBOL for nsecs_to_jiffies64. Can I count your
> > "Yea" above as an ack for adding that and pulling it in through
> > drm-intel.git?
>
> Do you need an EXPORT_SYMBOL if you add the _timeout version next to
> nsecs_to_jiffies64 in time.c?
I wouldn't but the patch from Imre to add all the _timeout was killed with
a few bikesheds so really not volunteering. And just moving this single
one doesn't make a lot of sense imo. Also the next patch I'll do is just
add the +1 that we lost to the code and call it a day, really ;-)
> Otherwise no objections to the approach, but I'd like to properly do
> an Acked-by: after I see the patch. :)
I'll send it out.
-Daniel
--
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
On Thu, Dec 4, 2014 at 2:42 AM, Daniel Vetter <[email protected]> wrote:
> On Wed, Dec 03, 2014 at 11:07:08AM -0800, John Stultz wrote:
>> On Wed, Dec 3, 2014 at 6:30 AM, Daniel Vetter <[email protected]> wrote:
>> > On Tue, Dec 02, 2014 at 08:54:13AM -0800, John Stultz wrote:
>> >> On Tue, Dec 2, 2014 at 8:35 AM, Chris Wilson <[email protected]> wrote:
>> >> > On Tue, Dec 02, 2014 at 04:36:22PM +0100, Daniel Vetter wrote:
>> >> >> +static inline unsigned long nsecs_to_jiffies_timeout(const u64 m)
>> >> >> +{
>> >> >> + u64 usecs = div_u64(m + 999, 1000);
>> >> >> + unsigned long j = usecs_to_jiffies(usecs);
>> >> >> +
>> >> >> + return min_t(unsigned long, MAX_JIFFY_OFFSET, j + 1);
>> >> >
>> >> > Or more concisely and review friendly:
>> >> >
>> >> > static inline unsigned long nsecs_to_jiffies_timeout(const u64 n)
>> >> > {
>> >> > return min_t(u64, MAX_JIFFY_OFFSET, nsecs_to_jiffies64(n) + 1);
>> >> > }
>> >>
>> >> Yea. This looks much nicer. Seems generic enough it might be better
>> >> added next to nsec_to_jiffies64() in kernel/time/time.c or jiffies.h
>> >> rather then in a driver header.
>> >
>> > Ok, that needs an EXPORT_SYMBOL for nsecs_to_jiffies64. Can I count your
>> > "Yea" above as an ack for adding that and pulling it in through
>> > drm-intel.git?
>>
>> Do you need an EXPORT_SYMBOL if you add the _timeout version next to
>> nsecs_to_jiffies64 in time.c?
>
> I wouldn't but the patch from Imre to add all the _timeout was killed with
> a few bikesheds so really not volunteering. And just moving this single
> one doesn't make a lot of sense imo. Also the next patch I'll do is just
> add the +1 that we lost to the code and call it a day, really ;-)
>
Sigh. So you're going to make me write a separate patch that moves it over?
I know you'll probably say this is bikeshedding, but the reason why
avoiding the EXPORT_SYMBOL on nsec_to_jiffies would be good is because
nsec_to_jiffies explicitly states in the comments that its not for any
use but the scheduler.
But still, I do see our change broke you here, so I'm not going to object.
thanks
-john
On Thu, Dec 4, 2014 at 6:42 PM, John Stultz <[email protected]> wrote:
> On Thu, Dec 4, 2014 at 2:42 AM, Daniel Vetter <[email protected]> wrote:
>> On Wed, Dec 03, 2014 at 11:07:08AM -0800, John Stultz wrote:
>>> On Wed, Dec 3, 2014 at 6:30 AM, Daniel Vetter <[email protected]> wrote:
>>> > On Tue, Dec 02, 2014 at 08:54:13AM -0800, John Stultz wrote:
>>> >> On Tue, Dec 2, 2014 at 8:35 AM, Chris Wilson <[email protected]> wrote:
>>> >> > On Tue, Dec 02, 2014 at 04:36:22PM +0100, Daniel Vetter wrote:
>>> >> >> +static inline unsigned long nsecs_to_jiffies_timeout(const u64 m)
>>> >> >> +{
>>> >> >> + u64 usecs = div_u64(m + 999, 1000);
>>> >> >> + unsigned long j = usecs_to_jiffies(usecs);
>>> >> >> +
>>> >> >> + return min_t(unsigned long, MAX_JIFFY_OFFSET, j + 1);
>>> >> >
>>> >> > Or more concisely and review friendly:
>>> >> >
>>> >> > static inline unsigned long nsecs_to_jiffies_timeout(const u64 n)
>>> >> > {
>>> >> > return min_t(u64, MAX_JIFFY_OFFSET, nsecs_to_jiffies64(n) + 1);
>>> >> > }
>>> >>
>>> >> Yea. This looks much nicer. Seems generic enough it might be better
>>> >> added next to nsec_to_jiffies64() in kernel/time/time.c or jiffies.h
>>> >> rather then in a driver header.
>>> >
>>> > Ok, that needs an EXPORT_SYMBOL for nsecs_to_jiffies64. Can I count your
>>> > "Yea" above as an ack for adding that and pulling it in through
>>> > drm-intel.git?
>>>
>>> Do you need an EXPORT_SYMBOL if you add the _timeout version next to
>>> nsecs_to_jiffies64 in time.c?
>>
>> I wouldn't but the patch from Imre to add all the _timeout was killed with
>> a few bikesheds so really not volunteering. And just moving this single
>> one doesn't make a lot of sense imo. Also the next patch I'll do is just
>> add the +1 that we lost to the code and call it a day, really ;-)
>>
>
> Sigh. So you're going to make me write a separate patch that moves it over?
We've written it already, Imre posted the link to the old discussion:
https://lkml.org/lkml/2013/5/10/187
But if the first attempt doesn't sufficiently stick I tend to chase
the patches any more. But if you want to resurrect this I could ping
Imre and ask him to pick it up again or you could rebase his patches.
> I know you'll probably say this is bikeshedding, but the reason why
> avoiding the EXPORT_SYMBOL on nsec_to_jiffies would be good is because
> nsec_to_jiffies explicitly states in the comments that its not for any
> use but the scheduler.
Well I only export the 64 variant, the other one (with the too small
return type which would overflow) is already exported with
commit d560fed6abe0f9975b509e4fb824e08ac19adc93
Author: Thomas Gleixner <[email protected]>
Date: Wed Jul 16 21:04:31 2014 +0000
time: Export nsecs_to_jiffies()
Required for moving drivers to the nanosecond based interfaces.
Signed-off-by: Thomas Gleixner <[email protected]>
Signed-off-by: John Stultz <[email protected]>
So I've figured this is ok.
> But still, I do see our change broke you here, so I'm not going to object.
Ok, thanks I'll pull this in through drm-intel for 3.19 (3.18 is kinda
done already I guess) with cc: stable.
-Daniel
--
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
On Thu, Dec 4, 2014 at 9:50 AM, Daniel Vetter <[email protected]> wrote:
> On Thu, Dec 4, 2014 at 6:42 PM, John Stultz <[email protected]> wrote:
>> Sigh. So you're going to make me write a separate patch that moves it over?
>
> We've written it already, Imre posted the link to the old discussion:
>
> https://lkml.org/lkml/2013/5/10/187
>
> But if the first attempt doesn't sufficiently stick I tend to chase
> the patches any more. But if you want to resurrect this I could ping
> Imre and ask him to pick it up again or you could rebase his patches.
Well, last I saw the initial patch was buggy, no? I don't think I saw
it being resubmitted.
>> But still, I do see our change broke you here, so I'm not going to object.
>
> Ok, thanks I'll pull this in through drm-intel for 3.19 (3.18 is kinda
> done already I guess) with cc: stable.
You probably should submit it for 3.18 and let Linus decide if its too
late. I've already gotten yelled at by Ingo for pushing patches in the
merge window that cc stable. Even if its out of a desire to let the
patches get wider testing, its something of a hot-button item for
folks. :)
thanks
-john
On Thu, Dec 4, 2014 at 7:16 PM, John Stultz <[email protected]> wrote:
> On Thu, Dec 4, 2014 at 9:50 AM, Daniel Vetter <[email protected]> wrote:
>> On Thu, Dec 4, 2014 at 6:42 PM, John Stultz <[email protected]> wrote:
>>> Sigh. So you're going to make me write a separate patch that moves it over?
>>
>> We've written it already, Imre posted the link to the old discussion:
>>
>> https://lkml.org/lkml/2013/5/10/187
>>
>> But if the first attempt doesn't sufficiently stick I tend to chase
>> the patches any more. But if you want to resurrect this I could ping
>> Imre and ask him to pick it up again or you could rebase his patches.
>
> Well, last I saw the initial patch was buggy, no? I don't think I saw
> it being resubmitted.
I didn't see your reply in that thread nor in the v2 follow up at
http://marc.info/?l=linux-kernel&m=136854294730957&w=2 Maybe I missed
it, but response seems to have been lukewarm overall.
>>> But still, I do see our change broke you here, so I'm not going to object.
>>
>> Ok, thanks I'll pull this in through drm-intel for 3.19 (3.18 is kinda
>> done already I guess) with cc: stable.
>
> You probably should submit it for 3.18 and let Linus decide if its too
> late. I've already gotten yelled at by Ingo for pushing patches in the
> merge window that cc stable. Even if its out of a desire to let the
> patches get wider testing, its something of a hot-button item for
> folks. :)
Oh I know, but if you count your regression rate in bugs-per-day you
end up with different standards ;-)
-Daniel
--
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
On Thu, Dec 4, 2014 at 10:51 AM, Daniel Vetter <[email protected]> wrote:
> On Thu, Dec 4, 2014 at 7:16 PM, John Stultz <[email protected]> wrote:
>> On Thu, Dec 4, 2014 at 9:50 AM, Daniel Vetter <[email protected]> wrote:
>>> On Thu, Dec 4, 2014 at 6:42 PM, John Stultz <[email protected]> wrote:
>>>> Sigh. So you're going to make me write a separate patch that moves it over?
>>>
>>> We've written it already, Imre posted the link to the old discussion:
>>>
>>> https://lkml.org/lkml/2013/5/10/187
>>>
>>> But if the first attempt doesn't sufficiently stick I tend to chase
>>> the patches any more. But if you want to resurrect this I could ping
>>> Imre and ask him to pick it up again or you could rebase his patches.
>>
>> Well, last I saw the initial patch was buggy, no? I don't think I saw
>> it being resubmitted.
>
> I didn't see your reply in that thread nor in the v2 follow up at
> http://marc.info/?l=linux-kernel&m=136854294730957&w=2 Maybe I missed
> it, but response seems to have been lukewarm overall.
Ok, I wasn't cc'ed on the v2, thanks for the pointer. There's some
general lukewarmness to all things jiffies, since getting rid of them
has been a long term goal forever. But overall that patch set seemed
ok (though I'm not a fan of macro generation of functions). But minor
details..
thanks
-john
On Thu, Dec 04, 2014 at 12:35:44PM -0800, John Stultz wrote:
> On Thu, Dec 4, 2014 at 10:51 AM, Daniel Vetter <[email protected]> wrote:
> > On Thu, Dec 4, 2014 at 7:16 PM, John Stultz <[email protected]> wrote:
> >> On Thu, Dec 4, 2014 at 9:50 AM, Daniel Vetter <[email protected]> wrote:
> >>> On Thu, Dec 4, 2014 at 6:42 PM, John Stultz <[email protected]> wrote:
> >>>> Sigh. So you're going to make me write a separate patch that moves it over?
> >>>
> >>> We've written it already, Imre posted the link to the old discussion:
> >>>
> >>> https://lkml.org/lkml/2013/5/10/187
> >>>
> >>> But if the first attempt doesn't sufficiently stick I tend to chase
> >>> the patches any more. But if you want to resurrect this I could ping
> >>> Imre and ask him to pick it up again or you could rebase his patches.
> >>
> >> Well, last I saw the initial patch was buggy, no? I don't think I saw
> >> it being resubmitted.
> >
> > I didn't see your reply in that thread nor in the v2 follow up at
> > http://marc.info/?l=linux-kernel&m=136854294730957&w=2 Maybe I missed
> > it, but response seems to have been lukewarm overall.
>
> Ok, I wasn't cc'ed on the v2, thanks for the pointer. There's some
> general lukewarmness to all things jiffies, since getting rid of them
> has been a long term goal forever. But overall that patch set seemed
> ok (though I'm not a fan of macro generation of functions). But minor
> details..
btw have you seen the other fallout from the ktime->nsec conversion in
i915?
http://www.spinics.net/lists/intel-gfx/msg56445.html
Is this just the inaccuracy of nsec_to_jiffies (and why it explicitly
states that this is for the scheduler only) or is there some bigger fish
in there?
Insight very much appreciated.
Thanks, Daniel
--
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch