On Fri, 1 Mar 2002, Andreas Dilger wrote:
> On Mar 01, 2002 03:55 +0100, Tim Schmielau wrote:
[...]
> > As no other comments turned up, this will go to Marcelo RSN.
> > (wondered why noone vetoed this as overkill...)
>
> Minor nit - the indenting of #ifdefs is not really used in the kernel.
>
> > +u64 get_jiffies64(void)
> > +{
> > + unsigned long jiffies_tmp, jiffies_hi_tmp;
> > +
> > + spin_lock(&jiffies64_lock);
> > + jiffies_tmp = jiffies; /* avoid races */
> > + jiffies_hi_tmp = jiffies_hi;
> > + if (unlikely(jiffies_tmp < jiffies_last)) /* We have a wrap */
> > + jiffies_hi++;
> > + jiffies_last = jiffies_tmp;
> > + spin_unlock(&jiffies64_lock);
> > +
> > + return (jiffies_tmp | ((u64)jiffies_hi_tmp) << BITS_PER_LONG);
> > +}
>
> If jiffies_hi is incremented, then jiffies_hi_tmp will be wrong on return.
Thanks!
I thought I had corrected this but somehow must have posted a previous
version. I will probably soon be known as a sloppy coder :-(
> note:----------------------------------------------------------------------^
>
> Since check_jiffieswrap() and get_jiffies64() are substantially the same,
> you may want to define a function _inc_jiffies64() which does:
>
> +#ifdef NEEDS_JIFFIES64
> +/* jiffies_hi and jiffies_last are protected by jiffies64_lock */
> +static unsigned long jiffies_hi, jiffies_last;
> +static spinlock_t jiffies64_lock = SPIN_LOCK_UNLOCKED;
> +#endif
>
> static inline void _inc_jiffies64(unsigned long jiffies_tmp)
> {
> jiffies_tmp = jiffies; /* avoid races */
> if (jiffies_tmp < jiffies_last) /* We have a wrap */
> jiffies_hi++;
> jiffies_last = jiffies_tmp;
> }
Shouldn't this be
static inline void _inc_jiffies64(unsigned long *jiffies_tmp)
{
*jiffies_tmp = jiffies; /* avoid races */
if (*jiffies_tmp < jiffies_last) /* We have a wrap */
jiffies_hi++;
jiffies_last = *jiffies_tmp;
}
?
So that we'd then need
static u64 get_jiffies64(void)
{
unsigned long jiffies_tmp, jiffies_hi_tmp;
spin_lock(&jiffies64_lock);
_inc_jiffies64(&jiffies_tmp);
jiffies_hi_tmp = jiffies_hi;
spin_unlock(&jiffies64_lock);
return (jiffies_tmp | ((u64)jiffies_hi_tmp) << BITS_PER_LONG);
}
...
And I'm still thinking of a better name that _inc_jiffies64(), since
we most of the time don't increment anything.
Tim
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
This archive was generated by hypermail 2b29 : Thu Mar 07 2002 - 21:00:18 EST