On Fri, 2011-04-08 at 10:19 +1000, Dave Chinner wrote:
> On Thu, Apr 07, 2011 at 04:16:22PM -0500, Alex Elder wrote:
> > On Thu, 2011-04-07 at 11:57 +1000, Dave Chinner wrote:
> > > + if (!(mp->m_super->s_flags & MS_ACTIVE))
> > > + return;
> > > +
> > > + rcu_read_lock();
> > > + if (radix_tree_tagged(&mp->m_perag_tree, XFS_ICI_RECLAIM_TAG)) {
> > > + queue_delayed_work(xfs_syncd_wq, &mp->m_reclaim_work,
> > > + msecs_to_jiffies(xfs_syncd_centisecs / 6 * 10));
> >
> > Probably better to do the multiply before the divide here.
> > (But whatever... it's heuristic.)
>
> I always tend to divide before multiply to prevent the multiple from
> overflowing before the divide is done. In this case the granularity
> of xfs_syncd_centisecs is sufficient that the rounding error of the
> divide is meaningless. ie. 30s = 3000.
Funny, I normally do the same, but in this case
I was thinking about getting better accuracy.
The numbers involved (msecs / jiffies and the
number of msec) are not likely to be even close
to sqrt(INT32_MAX) (let alone sqrt(UINT64_MAX)).
Putting the calculation inside the parenthesis
is much better though, and I did notice that
change.
-Alex
> FWIW, I changed this from:
>
> xfs_syncd_centisecs / 6 * msecs_to_jiffies(10)
>
> because msecs_to_jiffies() has larger rounding problems. e.g. @
> CONFIG_HZ=250, msecs_to_jiffies(10) = 3 which is actually 12ms. That
> is, we want to sleep for 5s at a time, and the two different
> calculations give:
>
> New:
> msecs_to_jiffies(3000 / 6 * 10) = 5000 / 4 jiffies
> = 1250 jiffies
> = 5s
> Old:
> 3000 / 6 * msecs_to_jiffies(10) = 500 * 3 jiffies
> = 1500 jiffies
> = 6s
>
> This 20% rounding error is the reason we've recently noticed xfssyncd
> running every 36s rather than every 30s....
>
> Cheers,
>
> Dave.
_______________________________________________
xfs mailing list
xfs@xxxxxxxxxxx
http://oss.sgi.com/mailman/listinfo/xfs
