The timestamps for the cache are all in boottime seconds, so they don't overflow 32-bit values, but the use of time_t is deprecated because it generally does overflow when used with wall-clock time. There are multiple possible ways of avoiding it: - leave time_t, which is safe here, but forces others to look into this code to determine that it is over and over. - use a more generic type, like 'int' or 'long', which is known to be sufficient here but loses the documentation of referring to timestamps - use ktime_t everywhere, and convert into seconds in the few places where we want realtime-seconds. The conversion is sometimes expensive, but not more so than the conversion we do today. - use time64_t to clarify that this code is safe. Nothing would change for 64-bit architectures, but it is slightly less efficient on 32-bit architectures. Without a clear winner of the three approaches above, this picks the last one, favouring readability over a small performance loss on 32-bit architectures.
Showing the changes to include/linux (1 of 4 files). Show all
Unified · Split
| @@ -45,8 +45,8 @@ · 44 unmodified lines | |||
| 45 | */ | 45 | */ |
| 46 | struct cache_head { | 46 | struct cache_head { |
| 47 | struct hlist_node cache_list; | 47 | struct hlist_node cache_list; |
| 48 | time_t expiry_time; /* After time time, don't use the data */ | 48 | time64_t expiry_time; /* After time time, don't use the data */ |
| 49 | time_t last_refresh; /* If CACHE_PENDING, this is when upcall was | 49 | time64_t last_refresh; /* If CACHE_PENDING, this is when upcall was |
| 50 | * sent, else this is when update was | 50 | * sent, else this is when update was |
| 51 | * received, though it is alway set to | 51 | * received, though it is alway set to |
| 52 | * be *after* ->flush_time. | 52 | * be *after* ->flush_time. |
| @@ -95,22 +95,22 @@ struct cache_detail { · 42 unmodified lines | |||
| 95 | /* fields below this comment are for internal use | 95 | /* fields below this comment are for internal use |
| 96 | * and should not be touched by cache owners | 96 | * and should not be touched by cache owners |
| 97 | */ | 97 | */ |
| 98 | time_t flush_time; /* flush all cache items with | 98 | time64_t flush_time; /* flush all cache items with |
| 99 | * last_refresh at or earlier | 99 | * last_refresh at or earlier |
| 100 | * than this. last_refresh | 100 | * than this. last_refresh |
| 101 | * is never set at or earlier | 101 | * is never set at or earlier |
| 102 | * than this. | 102 | * than this. |
| 103 | */ | 103 | */ |
| 104 | struct list_head others; | 104 | struct list_head others; |
| 105 | time_t nextcheck; | 105 | time64_t nextcheck; |
| 106 | int entries; | 106 | int entries; |
| 107 | 107 | ||
| 108 | /* fields for communication over channel */ | 108 | /* fields for communication over channel */ |
| 109 | struct list_head queue; | 109 | struct list_head queue; |
| 110 | 110 | ||
| 111 | atomic_t writers; /* how many time is /channel open */ | 111 | atomic_t writers; /* how many time is /channel open */ |
| 112 | time_t last_close; /* if no writers, when did last close */ | 112 | time64_t last_close; /* if no writers, when did last close */ |
| 113 | time_t last_warn; /* when we last warned about no writers */ | 113 | time64_t last_warn; /* when we last warned about no writers */ |
| 114 | 114 | ||
| 115 | union { | 115 | union { |
| 116 | struct proc_dir_entry *procfs; | 116 | struct proc_dir_entry *procfs; |
| @@ -147,18 +147,22 @@ struct cache_deferred_req { · 30 unmodified lines | |||
| 147 | * timestamps kept in the cache are expressed in seconds | 147 | * timestamps kept in the cache are expressed in seconds |
| 148 | * since boot. This is the best for measuring differences in | 148 | * since boot. This is the best for measuring differences in |
| 149 | * real time. | 149 | * real time. |
| 150 | * This reimplemnts ktime_get_boottime_seconds() in a slightly | ||
| 151 | * faster but less accurate way. When we end up converting | ||
| 152 | * back to wallclock (CLOCK_REALTIME), that error often | ||
| 153 | * cancels out during the reverse operation. | ||
| 150 | */ | 154 | */ |
| 151 | static inline time_t seconds_since_boot(void) | 155 | static inline time64_t seconds_since_boot(void) |
| 152 | { | 156 | { |
| 153 | struct timespec boot; | 157 | struct timespec64 boot; |
| 154 | getboottime(&boot); | 158 | getboottime64(&boot); |
| 155 | return get_seconds() - boot.tv_sec; | 159 | return ktime_get_real_seconds() - boot.tv_sec; |
| 156 | } | 160 | } |
| 157 | 161 | ||
| 158 | static inline time_t convert_to_wallclock(time_t sinceboot) | 162 | static inline time64_t convert_to_wallclock(time64_t sinceboot) |
| 159 | { | 163 | { |
| 160 | struct timespec boot; | 164 | struct timespec64 boot; |
| 161 | getboottime(&boot); | 165 | getboottime64(&boot); |
| 162 | return boot.tv_sec + sinceboot; | 166 | return boot.tv_sec + sinceboot; |
| 163 | } | 167 | } |
| 164 | 168 | ||
| @@ -273,7 +277,7 @@ static inline int get_uint(char **bpp, unsigned int *anint) · 108 unmodified lines | |||
| 273 | return 0; | 277 | return 0; |
| 274 | } | 278 | } |
| 275 | 279 | ||
| 276 | static inline int get_time(char **bpp, time_t *time) | 280 | static inline int get_time(char **bpp, time64_t *time) |
| 277 | { | 281 | { |
| 278 | char buf[50]; | 282 | char buf[50]; |
| 279 | long long ll; | 283 | long long ll; |
| @@ -287,20 +291,20 @@ static inline int get_time(char **bpp, time_t *time) · 7 unmodified lines | |||
| 287 | if (kstrtoll(buf, 0, &ll)) | 291 | if (kstrtoll(buf, 0, &ll)) |
| 288 | return -EINVAL; | 292 | return -EINVAL; |
| 289 | 293 | ||
| 290 | *time = (time_t)ll; | 294 | *time = ll; |
| 291 | return 0; | 295 | return 0; |
| 292 | } | 296 | } |
| 293 | 297 | ||
| 294 | static inline time_t get_expiry(char **bpp) | 298 | static inline time64_t get_expiry(char **bpp) |
| 295 | { | 299 | { |
| 296 | time_t rv; | 300 | time64_t rv; |
| 297 | struct timespec boot; | 301 | struct timespec64 boot; |
| 298 | 302 | ||
| 299 | if (get_time(bpp, &rv)) | 303 | if (get_time(bpp, &rv)) |
| 300 | return 0; | 304 | return 0; |
| 301 | if (rv < 0) | 305 | if (rv < 0) |
| 302 | return 0; | 306 | return 0; |
| 303 | getboottime(&boot); | 307 | getboottime64(&boot); |
| 304 | return rv - boot.tv_sec; | 308 | return rv - boot.tv_sec; |
| 305 | } | 309 | } |
| 306 | 310 | ||