ZFS: clock_t overflow in l2arc_feed_thread

[Martin Matuska]

This is a problem for several places in the ZFS code, most of them in
arc.c. The L2 issuse is quite clear:
in arc.c: l2arc_feed_thread() we have the following statement:

(void) cv_timedwait(&l2arc_feed_thr_cv, &l2arc_feed_thr_lock,
next - LBOLT);

next gets calculated from l2arc_write_interval() from the previous
while() loop.
Now a value of 0 will occur if l2arc_writ_interval() returns the same
LBOLT, effectively making this a cv_wait(),
negative values for the timeout are theoretically possible, too.

In addition there are other areas in the code where comparative
decisions based on a stored LBOLT and current LBOLT are made.
A wrong decision made on clock_t overflow time might be harmful and
cause unexpected behaviour.

Affected are (what I have seen so far, in
sys/cddl/contrib/opensolaris/uts/common/fs/zfs/):
arc.c: arc_access(), arc_reclaim_thread(), l2arc_write_interval(),
l2arc_feed_thread()
dmu_zfetch.c: dmu_zfetch_dofetch(), dmu_zfetch_stream_reclaim()
zil.c: zil_replay()

In userland we have some clock_t in:
lib/libzpool/common/kernel.c


Dňa 31.01.2011 06:25, Bruce Evans wrote / napísal(a):
> On Mon, 31 Jan 2011, Martin Matuska wrote:
>
>> I have re-checked OpenSolaris, and discovered that long is a int32_t.
>>
>> I agree, we should go for int64_t in our case.
>
> Why be different from both OpenSolaris and FreeBSD? clock_t is 32 bits
> on all, and only bogusly signed on some.
>
> Also, there must be another bug for overflow to occur after only 24 days.
> clock_t is only specified for holding statclock ticks, which have a
> frequency of about 128 Hz, so 32-bit unsigned ints hold 388 days of
> ticks.
> 32-bit signed ints hold 194 days of ticks. It is a units/type error to
> use clock_t for any other type of ticks. I used this an excuse to not
> expand clock_t on i386 about 15 years ago when I worked on this most.
> Clock ticks should be virtual and have a frequency much higher than 128,
> especially 15 years later. 1 MHz would have been OK 15 years ago, and
> some broken standards (XSI?) even required this. 1 GHz would have been
> good to cover the next 15 years, but it actually only lasted about 5
> years since CPU frequencies exceeded it in about 2001. Now that CPU
> frequencies have stopped growing rapidly, there seem to be no clock
> periods much smaller than 0.1 nsec on the horizon, so 1 THz might last
> more than 5 years. But even 1 GHz will overflow a 32-bit signed int
> in about 2 seconds. IIRC, POSIX only requires clock_t to work for
> 24 hours, so overflow after 24 days isn't necessarily a bug, but overflow
> after 2 or 2000 seconds (the latter for 1 MHz) would be. POSIX mostly
> uses timespecs when more resolution is required, but this has only been
> standard for 23 years so support and use of them for are patchy.
>
> [Context lost to top posting]
>
> Bruce
> _______________________________________________
> freebsd-fs at freebsd.org mailing list
> http://lists.freebsd.org/mailman/listinfo/freebsd-fs
> To unsubscribe, send any mail to "freebsd-fs-unsubscribe at freebsd.org"