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.

Sign up for GitHub

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

Faster Intel RDT init if the feature is unavailable #3306

Merged
merged 3 commits into from
Jan 19, 2022
Merged

Faster Intel RDT init if the feature is unavailable #3306

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
02e961b
libct/intelrdt: wrap Root in sync.Once
kolyshkin Dec 2, 2021
6c6b14e
libct/intelrdt: remove findMountpointDir test
kolyshkin Dec 2, 2021
edeb3b3
libct/intelrdt: faster init if rdt is unsupported
kolyshkin Dec 3, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
41 changes: 18 additions & 23 deletions libcontainer/intelrdt/intelrdt.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -276,35 +276,30 @@ func findIntelRdtMountpointDir(f io.Reader) (string, error) {

// For Root() use only.
var (
intelRdtRoot string
rootMu sync.Mutex
intelRdtRoot string
intelRdtRootErr error
rootOnce sync.Once
)

// Root returns the Intel RDT "resource control" filesystem mount point.
func Root() (string, error) {
rootMu.Lock()
defer rootMu.Unlock()

if intelRdtRoot != "" {
return intelRdtRoot, nil
}

f, err := os.Open("/proc/self/mountinfo")
if err != nil {
return "", err
}
root, err := findIntelRdtMountpointDir(f)
f.Close()
if err != nil {
return "", err
}
rootOnce.Do(func() {
f, err := os.Open("/proc/self/mountinfo")
if err != nil {
intelRdtRootErr = err
return
}
root, err := findIntelRdtMountpointDir(f)
Copy link
Member

Choose a reason for hiding this comment

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

I guess technically we wouldn't need the intermediate root and err variables (no need to change)

f.Close()
if err != nil {
intelRdtRootErr = err
return
}
Copy link
Contributor

Choose a reason for hiding this comment

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

Do we have to take into account that resctrl might get mounted after the first check?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I don't think so. If we do, we have to re-read /proc/self/mountinfo over and over (assuming it's some long-lived daemon that imports libcontainer/intelrdt, such as kubelet; for runc itself it's not a issue, since it's a short-lived binary).

I suppose that resctrl is mounted during system init, and it should be mounted by the time we start runc or e.g. kubernetes. If not, it's a configuration issue.

Copy link
Contributor

Choose a reason for hiding this comment

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

K, fair enough

Copy link
Member

Choose a reason for hiding this comment

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

I suppose that resctrl is mounted during system init, and it should be mounted by the time we start runc or e.g. kubernetes. If not, it's a configuration issue.

Are there specific systemd targets to take this into account and that should be recommended for that? I know containerd has local-fs.target, does that cover this as well?

After=network.target local-fs.target


if _, err := os.Stat(root); err != nil {
return "", err
}
intelRdtRoot = root
})

intelRdtRoot = root
return intelRdtRoot, nil
return intelRdtRoot, intelRdtRootErr
}

type cpuInfoFlags struct {
Expand Down
5 changes: 5 additions & 0 deletions libcontainer/intelrdt/util_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,12 @@ func NewIntelRdtTestUtil(t *testing.T) *intelRdtTestUtil {
config := &configs.Config{
IntelRdt: &configs.IntelRdt{},
}

// Assign fake intelRtdRoot value, returned by Root().
intelRdtRoot = t.TempDir()
// Make sure Root() won't even try to parse mountinfo.
rootOnce.Do(func() {})

testIntelRdtPath := filepath.Join(intelRdtRoot, "resctrl")

// Ensure the full mock Intel RDT "resource control" filesystem path exists
Expand Down