Repository: opencontainers/runc
File: libcontainer/intelrdt/intelrdt.go (Go)

Code before:
	// The available bandwidth control steps are: min_bw + N * bw_gran.
	// Intermediate values are rounded to the next control step available
	// on the hardware.
	//
	// If MBA Software Controller is enabled through mount option
	// "-o mba_MBps": mount -t resctrl resctrl -o mba_MBps /sys/fs/resctrl
	// We could specify memory bandwidth in "MBps" (Mega Bytes per second)
	// unit instead of "percentages". The kernel underneath would use a
	// software feedback mechanism or a "Software Controller" which reads
	// the actual bandwidth using MBM counters and adjust the memory
	// bandwidth percentages to ensure:
	// "actual memory bandwidth < user specified memory bandwidth".
	//
	// For example, on a two-socket machine, the schema line could be
	// "MB:0=5000;1=7000" which means 5000 MBps memory bandwidth limit on
	// socket 0 and 7000 MBps memory bandwidth limit on socket 1.
	if container.IntelRdt != nil {
		path := m.GetPath()
		l3CacheSchema := container.IntelRdt.L3CacheSchema
		memBwSchema := container.IntelRdt.MemBwSchema

		// TODO: verify that l3CacheSchema and/or memBwSchema match the
		// existing schemata if ClosID has been specified. This is a more
		// involved than reading the file and doing plain string comparison as
		// the value written in does not necessarily match what gets read out
		// (leading zeros, cache id ordering etc).

		// Write a single joint schema string to schemata file
		if l3CacheSchema != "" && memBwSchema != "" {
			if err := writeFile(path, "schemata", l3CacheSchema+"\n"+memBwSchema); err != nil {
				return err
			}
		}

		// Write only L3 cache schema string to schemata file
		if l3CacheSchema != "" && memBwSchema == "" {
			if err := writeFile(path, "schemata", l3CacheSchema); err != nil {
				return err
			}
		}

		// Write only memory bandwidth schema string to schemata file
		if l3CacheSchema == "" && memBwSchema != "" {
			if err := writeFile(path, "schemata", memBwSchema); err != nil {
				return err
			}
		}
	}

	return nil
}

Review comment (bug):
Sorry, this change is not 100% obvious to me.

Before we were not doing this, why do it now? Checking quickly, it seems like a different improvement, as the "info" file this ends up checking _might_ have some more explanation on why it failed.

Am I reading this correctly?

A different commit doing this change, with its explanation, seems better IMHO. If that is too much of a hassle, then mention it in the commit you do this change. If this causes an issue in the future, it will seem like something not intentional when it really was intentional, that is very valuable information for future me :)

Code after:
	// software feedback mechanism or a "Software Controller" which reads
	// the actual bandwidth using MBM counters and adjust the memory
	// bandwidth percentages to ensure:
	// "actual memory bandwidth < user specified memory bandwidth".
	//
	// For example, on a two-socket machine, the schema line could be
	// "MB:0=5000;1=7000" which means 5000 MBps memory bandwidth limit on
	// socket 0 and 7000 MBps memory bandwidth limit on socket 1.
	if r := container.IntelRdt; r != nil {
		// TODO: verify that l3CacheSchema and/or memBwSchema match the
		// existing schemata if ClosID has been specified. This is a more
		// involved than reading the file and doing plain string comparison as
		// the value written in does not necessarily match what gets read out
		// (leading zeros, cache id ordering etc).
		var schemata strings.Builder
		for _, s := range append([]string{r.L3CacheSchema, r.MemBwSchema}, r.Schemata...) {
			if s != "" {
				schemata.WriteString(s)
				schemata.WriteString("\n")
			}
		}

		if schemata.Len() > 0 {
			path := filepath.Join(m.GetPath(), "schemata")
			if err := os.WriteFile(path, []byte(schemata.String()), 0o600); err != nil {
				return newLastCmdError(fmt.Errorf("intelrdt: unable to write %q: %w", schemata.String(), err))
			}
		}
	}

	return nil
}

func newLastCmdError(err error) error {
	status, err1 := getLastCmdStatus()
	if err1 == nil {
		return fmt.Errorf("%w, last_cmd_status: %s", err, status)
	}
	return err
}