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 issue when parsing large CPU Masks (+32C) systems #5

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

benjojo
Copy link

@benjojo benjojo commented Dec 6, 2023

Previously when using 32Thread+ systems, the libary would misparse the cores beyond the 32T threashold, causing bad behaviour.

This has now been fixed, with tests (and the parsing being moved to it's own testable sub-function)

Tested on a 12C*HT*2S, 32C*HT, and 32C*HT*2S system's output on modern linux kernels

Previously when using 32Thread+ systems, the libary would misparse
the cores beyond the 32T threashold, causing bad behaviour.

This has now been fixed, with tests (and the parsing being moved
to it's own testable sub-function)

Tested on a 12C*HT*2S, 32C*HT, and 32C*HT*2S system's output on
modern linux kernels
@lrita
Copy link
Owner

lrita commented Dec 7, 2023

Thanks for your reporting this issue. After I investigate the cpumap string format, it is outputed by this. We can find there only first phrase maybe less than 8 character. So I think the really correct parse function maybe:

func parseCPUMapMask(d string, cpucount int) Bitmask {
	cpumask := NewBitmask(cpucount)
	tokens := strings.Split(strings.TrimSpace(d), ",")
	base := 0
	for j := 0; j < len(tokens); j++ {
                token := tokens[len(tokens)-1-j]
                nn := len(token) * 4
		mask, _ := strconv.ParseUint(token, 16, 64)
		for k := 0; k < nn; k++ {
			if (mask>>uint64(k))&0x01 != 0 {
				cpumask.Set(k+base, true)
			}
		}
		base += nn
	}
	return cpumask
}

Can you improve your patch and verify it ?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants