Re: [ovs-dev] [PATCH ovn] memory-trim: Fix timestamp overflow warning right after reboot.

[Dumitru Ceara]

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 
>> ---
>>
>> 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 
> 

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

___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] [PATCH ovn] memory-trim: Fix timestamp overflow warning right after reboot.

[Dumitru Ceara]

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 
> ---
> 
> 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 

___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

[ovs-dev] [PATCH ovn] memory-trim: Fix timestamp overflow warning right after reboot.

[Ilya Maximets]

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 
---

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(-)

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(, "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(, "Detected inactivity "
 "(last active %lld ms ago): trimming memory",
 now - mt->last_active_ms);
-- 
2.41.0

___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev