Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: calculate log2i properly #7567

Merged
merged 1 commit into from
Aug 3, 2023
Merged

Conversation

smira
Copy link
Member

@smira smira commented Aug 3, 2023

Fixes #7080

The real bug was off-by-one in log2i implementation, other changes are cleanups as x/sys/unix package now contains all the constants we need.

@smira smira added this to the v1.6 milestone Aug 3, 2023
return 0
}

return 63 - bits.LeadingZeros64(v)
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this is the real fix, previous code was s/63/64.

the code in systemd for comparison: https://github.com/systemd/systemd/blob/6639ac474eb7a5325a72a3d7370492792dd00bc0/src/fundamental/logarithm.h#L39-L40

Copy link
Member

@frezbo frezbo left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

+1 on not needing to maintain all these constants

Fixes siderolabs#7080

The real bug was off-by-one in `log2i` implementation, other changes are
cleanups as `x/sys/unix` package now contains all the constants we need.

Signed-off-by: Andrey Smirnov <[email protected]>
@smira
Copy link
Member Author

smira commented Aug 3, 2023

one more PR coming to help debugging things

@smira
Copy link
Member Author

smira commented Aug 3, 2023

/m

@talos-bot talos-bot merged commit 4eab301 into siderolabs:main Aug 3, 2023
5 checks passed
smira added a commit to smira/talos that referenced this pull request Aug 3, 2023
This is a follow-up for siderolabs#7567, which won't be backported to 1.5.

This allows to get an output like:

```
$ talosctl -n 172.20.0.5 get adjtimestatus -w
NODE         *   NAMESPACE TYPE            ID     VERSION   OFFSET        ESTERROR   MAXERROR   STATUS               SYNC
172.20.0.5   +   runtime   AdjtimeStatus   node   47        -18.14306ms   0s         191.5ms    STA_PLL | STA_NANO   true
172.20.0.5       runtime   AdjtimeStatus   node   48        -17.109555ms  0s         206.5ms    STA_NANO | STA_PLL   true
172.20.0.5       runtime   AdjtimeStatus   node   49        -16.134923ms  0s         221.5ms    STA_NANO | STA_PLL   true
172.20.0.5       runtime   AdjtimeStatus   node   50        -15.21581ms   0s         236.5ms    STA_PLL | STA_NANO   true
```

Signed-off-by: Andrey Smirnov <[email protected]>
smira added a commit to smira/talos that referenced this pull request Aug 3, 2023
This is a follow-up for siderolabs#7567, which won't be backported to 1.5.

This allows to get an output like:

```
$ talosctl -n 172.20.0.5 get adjtimestatus -w
NODE         *   NAMESPACE TYPE            ID     VERSION   OFFSET        ESTERROR   MAXERROR   STATUS               SYNC
172.20.0.5   +   runtime   AdjtimeStatus   node   47        -18.14306ms   0s         191.5ms    STA_PLL | STA_NANO   true
172.20.0.5       runtime   AdjtimeStatus   node   48        -17.109555ms  0s         206.5ms    STA_NANO | STA_PLL   true
172.20.0.5       runtime   AdjtimeStatus   node   49        -16.134923ms  0s         221.5ms    STA_NANO | STA_PLL   true
172.20.0.5       runtime   AdjtimeStatus   node   50        -15.21581ms   0s         236.5ms    STA_PLL | STA_NANO   true
```

Signed-off-by: Andrey Smirnov <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: Backported
Status: Backported
Development

Successfully merging this pull request may close these issues.

clock skew
3 participants