[PATCH v2] add time_now_after and other macros which compare with jiffies

[PATCH v2] add time_now_after and other macros which compare with jiffies

[Dave Young]

Changes from previous version:
1. Add comments
2. Change names easy to understand. For example, now time_now_after(a) means time now after a will return true.

---
Signed-off-by: Dave Young <hidave.darkstar@gmail.com>

---
include/linux/jiffies.h |   16 ++++++++++++++++
1 file changed, 16 insertions(+)

diff -upr linux/include/linux/jiffies.h linux.new/include/linux/jiffies.h
--- linux/include/linux/jiffies.h	2008-03-07 19:56:02.000000000 +0800
+++ linux.new/include/linux/jiffies.h	2008-03-07 20:10:25.000000000 +0800
@@ -135,6 +135,22 @@ static inline u64 get_jiffies_64(void)
 #define time_before_eq64(a,b)	time_after_eq64(b,a)
 
 /*
+ * These four macros compare jiffies and 'a' for convenience.
+ */
+
+/* time_now_after(a) return true if now (jiffies) is after a */
+#define time_now_after(a) time_after(jiffies, a)
+
+/* time_now_before(a) return true if now (jiffies) is before a */
+#define time_now_before(a) time_before(jiffies, a)
+
+/* time_now_after_eq(a) return true if now (jiffies) is after or equal to a */
+#define time_now_after_eq(a) time_after_eq(jiffies, a)
+
+/* time_now_before_eq(a) return true if now (jiffies) is before or equal to a */
+#define time_now_before_eq(a) time_before_eq(jiffies, a)
+
+/*
  * Have the 32 bit jiffies value wrap 5 minutes after boot
  * so jiffies wrap bugs show up earlier.
  */

Re: [PATCH v2] add time_now_after and other macros which compare with jiffies

[Johannes Weiner]

Hi Dave, Andrew and all

Dave Young <hidave.darkstar@gmail.com> writes:

> +/* time_now_after(a) return true if now (jiffies) is after a */
> +#define time_now_after(a) time_after(jiffies, a)
> +
> +/* time_now_before(a) return true if now (jiffies) is before a */
> +#define time_now_before(a) time_before(jiffies, a)
> +
> +/* time_now_after_eq(a) return true if now (jiffies) is after or equal to a */
> +#define time_now_after_eq(a) time_after_eq(jiffies, a)
> +
> +/* time_now_before_eq(a) return true if now (jiffies) is before or equal to a */
> +#define time_now_before_eq(a) time_before_eq(jiffies, a)

How about even more obvious names like time_is_past(), time_is_future(),
...?

Sorry, I missed v1.  Should have proposed that earlier.

	Hannes

Re: [PATCH v2] add time_now_after and other macros which compare with jiffies

[Dave Young]

On Sun, Mar 9, 2008 at 12:12 AM, Johannes Weiner <hannes@saeurebad.de> wrote:
> Hi Dave, Andrew and all
>
>
>  Dave Young <hidave.darkstar@gmail.com> writes:
>
>  > +/* time_now_after(a) return true if now (jiffies) is after a */
>  > +#define time_now_after(a) time_after(jiffies, a)
>  > +
>  > +/* time_now_before(a) return true if now (jiffies) is before a */
>  > +#define time_now_before(a) time_before(jiffies, a)
>  > +
>  > +/* time_now_after_eq(a) return true if now (jiffies) is after or equal to a */
>  > +#define time_now_after_eq(a) time_after_eq(jiffies, a)
>  > +
>  > +/* time_now_before_eq(a) return true if now (jiffies) is before or equal to a */
>  > +#define time_now_before_eq(a) time_before_eq(jiffies, a)
>
>  How about even more obvious names like time_is_past(), time_is_future(),
>  ...?

Thanks for comment.

Then how do we name the _eq version?  IMHO, the time_now_* is enough.

>
>  Sorry, I missed v1.  Should have proposed that earlier.
>
>         Hannes
>

Regards
dave

Re: [PATCH v2] add time_now_after and other macros which compare with jiffies

[Alan Cox]

> >  > +/* time_now_before_eq(a) return true if now (jiffies) is before or equal to a */
> >  > +#define time_now_before_eq(a) time_before_eq(jiffies, a)
> >
> >  How about even more obvious names like time_is_past(), time_is_future(),
> >  ...?
> 
> Thanks for comment.
> 
> Then how do we name the _eq version?  IMHO, the time_now_* is enough.

Why do you even need them. I don't see the point of *any* of these extra
macros as they simply obfuscate code and hide what is actually going on.
The initial macros were added because of the type safety and correct
comparison rules being complex. They have a purpose.

Even if you want these you can use !time_future() if you don't want the
_eq variants. Generally speaking drivers should be using timers not
polled loops, and most of our loops comparing with jiffies want removing.

Alan

Re: [PATCH v2] add time_now_after and other macros which compare with jiffies

[Dave Young]

On 3/9/08, Alan Cox <alan@lxorguk.ukuu.org.uk> wrote:
> > >  > +/* time_now_before_eq(a) return true if now (jiffies) is before or equal to a */
>  > >  > +#define time_now_before_eq(a) time_before_eq(jiffies, a)
>  > >
>  > >  How about even more obvious names like time_is_past(), time_is_future(),
>  > >  ...?
>  >
>  > Thanks for comment.
>  >
>  > Then how do we name the _eq version?  IMHO, the time_now_* is enough.
>
>
> Why do you even need them. I don't see the point of *any* of these extra
>  macros as they simply obfuscate code and hide what is actually going on.
>  The initial macros were added because of the type safety and correct
>  comparison rules being complex. They have a purpose.

Yes, This has a purpose as well. You will find most of the usage of these
macros are just compare some value with jiffies after doing some grep.

For these cases this adding will easy use and understand.

>
>  Even if you want these you can use !time_future() if you don't want the
>  _eq variants. Generally speaking drivers should be using timers not
>  polled loops, and most of our loops comparing with jiffies want removing.
>

IMO time_after is a confusing name actually, If I don't read the comment I
will think it as time_future

To some extent I agree with you that the timer will be better.

>
>  Alan
>

Thanks

Re: [PATCH v2] add time_now_after and other macros which compare with jiffies

[Alan Cox]

> Yes, This has a purpose as well. You will find most of the usage of these
> macros are just compare some value with jiffies after doing some grep.

That is no reason to add more macros that hide how they do it, or make
jiffies itself invisible to the grep search or variable usage finding
tools (and for tickless that very thing is important)

> For these cases this adding will easy use and understand.

It hides information.

Alan

Re: [PATCH v2] add time_now_after and other macros which compare with jiffies

[Andrew Morton]

On Sun, 9 Mar 2008 11:08:08 +0000 Alan Cox <alan@lxorguk.ukuu.org.uk> wrote:

> > Yes, This has a purpose as well. You will find most of the usage of these
> > macros are just compare some value with jiffies after doing some grep.
> 
> That is no reason to add more macros that hide how they do it, or make
> jiffies itself invisible to the grep search or variable usage finding
> tools (and for tickless that very thing is important)

We could call them time_is_after_jiffies(), time_is_before_jiffies(), etc.

Those names are actually better than time_after_now(), etc.

Re: [PATCH v2] add time_now_after and other macros which compare with jiffies

[Alan Cox]

> We could call them time_is_after_jiffies(), time_is_before_jiffies(), etc.
> 
> Those names are actually better than time_after_now(), etc.

Agreed - and if you think its worth doing I bow to your expertise on
buggy patch submissions..

Re: [PATCH v2] add time_now_after and other macros which compare with jiffies

[Dave Young]

On Mon, Mar 10, 2008 at 3:01 AM, Andrew Morton
<akpm@linux-foundation.org> wrote:
> On Sun, 9 Mar 2008 11:08:08 +0000 Alan Cox <alan@lxorguk.ukuu.org.uk> wrote:
>
>  > > Yes, This has a purpose as well. You will find most of the usage of these
>  > > macros are just compare some value with jiffies after doing some grep.
>  >
>  > That is no reason to add more macros that hide how they do it, or make
>  > jiffies itself invisible to the grep search or variable usage finding
>  > tools (and for tickless that very thing is important)
>
>  We could call them time_is_after_jiffies(), time_is_before_jiffies(), etc.
>
>  Those names are actually better than time_after_now(), etc.
>
>
Andrew, Hannes, Alan,
thanks for your comment, I have send the new time_is_* patch as v3.

Hannes, It's more like your is_past/is_future but looks better when
along with _eq

Re: [PATCH v2] add time_now_after and other macros which compare with jiffies

[Johannes Weiner]

Hi Dave,

"Dave Young" <hidave.darkstar@gmail.com> writes:

> Hannes, It's more like your is_past/is_future but looks better when
> along with _eq

I agree.  And it's better to have `jiffies' in the names, anyway.

	Hannes

Re: [PATCH v2] add time_now_after and other macros which compare with jiffies

[Andrew Morton]

On Sun, 9 Mar 2008 09:58:02 +0000 Alan Cox <alan@lxorguk.ukuu.org.uk> wrote:

> > >  > +/* time_now_before_eq(a) return true if now (jiffies) is before or equal to a */
> > >  > +#define time_now_before_eq(a) time_before_eq(jiffies, a)
> > >
> > >  How about even more obvious names like time_is_past(), time_is_future(),
> > >  ...?
> > 
> > Thanks for comment.
> > 
> > Then how do we name the _eq version?  IMHO, the time_now_* is enough.
> 
> Why do you even need them. I don't see the point of *any* of these extra
> macros as they simply obfuscate code and hide what is actually going on.

Two reasons:

a) the existing macros are (I believe) a right royal pita.  It's very
   hard to remember which order the args are supposed to be in.

   So each time I see a time_foo() when reviewing a patch I have to go
   off and re-read the implementation then have a big think to check that
   they got it right (a sure sign of a poor interface, no)?

   And I'm not the only one - people get this wrong fairly regularly.

b) around 90% of the usages of time_after() are to compare against jiffies!

The macros which Dave is developing aren't just less-error-prone,
easier-to-review transformations - they offer higher-level functionality.
Because time_after() is just a basic comparison operation, whereas
time_now_before() is an *application* of that operation.

Trust me on this - they will lead to easier-to-review code and less bugs.

> The initial macros were added because of the type safety and correct
> comparison rules being complex. They have a purpose.

They are hard to use and hard to review.

> Even if you want these you can use !time_future() if you don't want the
> _eq variants. Generally speaking drivers should be using timers not
> polled loops, and most of our loops comparing with jiffies want removing.

Re: [PATCH v2] add time_now_after and other macros which compare with jiffies

[Alan Cox]

> a) the existing macros are (I believe) a right royal pita.  It's very
>    hard to remember which order the args are supposed to be in.

Its operator(first, second) - the C++ infix operators aren't available in
C so there isn't much choice.

Really we should be trying to stamp out all these uses of jiffies tests
so that we can make tickless work ever better.

Alan