| Message ID | 20230922122532.2021173-1-i.maximets@ovn.org |
|---|---|
| State | Accepted |
| Delegated to: | Dumitru Ceara |
| Headers | show |
| Series | [ovs-dev] memory-trim: Fix timestamp overflow warning right after reboot. | expand |
| Context | Check | Description |
|---|---|---|
| ovsrobot/apply-robot | success | apply and check: success |
| ovsrobot/github-robot-_Build_and_Test | success | github build: passed |
| ovsrobot/github-robot-_ovn-kubernetes | fail | github build: failed |
On 9/22/23 14:21, Ilya Maximets wrote: > If OVN is started less than 30 seconds after system boot, it logs: > > |memory_trim|WARN|Detected last active timestamp overflow > > The 'now < trim_timeout_ms' is not for a timestamp overflow, but > for the later subtraction. 'now < last_active_ms' is enough to > check for the overflow. > > Technically, we shouldn't need to check that now > trim_timeout_ms > before subtraction, because the result should be a signed integer, > but it's cleaner this way. > > Fixes: 12ddb1c9d908 ("lflow-cache: Automatically trim cache when it becomes inactive.") > Signed-off-by: Ilya Maximets <i.maximets@ovn.org> > --- > > Note: In case of backporting, branches below 23.03 have a bit different code, > so the fix will need to be slightly different. > Thanks for the fix, Ilya! Acked-by: Dumitru Ceara <dceara@redhat.com>
On 9/22/23 14:57, Dumitru Ceara wrote: > On 9/22/23 14:21, Ilya Maximets wrote: >> If OVN is started less than 30 seconds after system boot, it logs: >> >> |memory_trim|WARN|Detected last active timestamp overflow >> >> The 'now < trim_timeout_ms' is not for a timestamp overflow, but >> for the later subtraction. 'now < last_active_ms' is enough to >> check for the overflow. >> >> Technically, we shouldn't need to check that now > trim_timeout_ms >> before subtraction, because the result should be a signed integer, >> but it's cleaner this way. >> >> Fixes: 12ddb1c9d908 ("lflow-cache: Automatically trim cache when it becomes inactive.") >> Signed-off-by: Ilya Maximets <i.maximets@ovn.org> >> --- >> >> Note: In case of backporting, branches below 23.03 have a bit different code, >> so the fix will need to be slightly different. >> > > Thanks for the fix, Ilya! > > Acked-by: Dumitru Ceara <dceara@redhat.com> > I applied this to main and backported it to all branches down to 22.03. The branches below 23.03 did have a bit of a different code but the changes were straightforward. It would be great though if you could have another look, just in case. Thanks, Dumitru
diff --git a/lib/memory-trim.c b/lib/memory-trim.c index d8c6b4743..be8983fa7 100644 --- a/lib/memory-trim.c +++ b/lib/memory-trim.c @@ -71,13 +71,14 @@ memory_trimmer_can_run(struct memory_trimmer *mt) } long long int now = time_msec(); - if (now < mt->last_active_ms || now < mt->trim_timeout_ms) { + if (now < mt->last_active_ms) { VLOG_WARN_RL(&rl, "Detected last active timestamp overflow"); mt->recently_active = false; return true; } - if (now - mt->trim_timeout_ms >= mt->last_active_ms) { + if (now > mt->trim_timeout_ms + && now - mt->trim_timeout_ms >= mt->last_active_ms) { VLOG_INFO_RL(&rl, "Detected inactivity " "(last active %lld ms ago): trimming memory", now - mt->last_active_ms);
If OVN is started less than 30 seconds after system boot, it logs: |memory_trim|WARN|Detected last active timestamp overflow The 'now < trim_timeout_ms' is not for a timestamp overflow, but for the later subtraction. 'now < last_active_ms' is enough to check for the overflow. Technically, we shouldn't need to check that now > trim_timeout_ms before subtraction, because the result should be a signed integer, but it's cleaner this way. Fixes: 12ddb1c9d908 ("lflow-cache: Automatically trim cache when it becomes inactive.") Signed-off-by: Ilya Maximets <i.maximets@ovn.org> --- Note: In case of backporting, branches below 23.03 have a bit different code, so the fix will need to be slightly different. lib/memory-trim.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-)