[Bug][CPU Backend]: Improve L2 cache size detection and usage on aarch64

[Radu2k]

Purpose

• Add /sys/devices/system/cpu/cpu0/cache/index2/size parsing for L2 cache on aarch64
• Fallback to 1MB with warning if the sysfs file cannot be opened
• Use 70% of detected L2 cache for attention scheduling to leave enough room for sys processes

Test Plan

l2_cache_size gets the right value.

Test Result

l2_cache_size is set to 204810240.7 bytes on c8g instance type which is the correct value give that it has 2MB of L2 cache per core and we want to use 70%.

Solves: #30487

cc: @fadara01 @aditew01

[chatgpt-codex-connector]

Codex usage limits have been reached for code reviews. Please check with the admins of this repo to increase the limits by adding credits.

[gemini-code-assist]

Code Review

This pull request enhances L2 cache size detection on aarch64 by reading from sysfs, which is a valuable improvement. The fallback mechanism is also a good addition for robustness. However, the implementation for parsing the cache size from the sysfs file contains critical flaws that could lead to application crashes or incorrect cache size calculations. I have provided a detailed comment with a more robust implementation to address these issues.

[gemini-code-assist]

The current parsing logic for the L2 cache size is fragile and can lead to incorrect behavior or crashes for several reasons:

1. Undefined Behavior: If size_str is empty (e.g., empty sysfs file), size_str.back() results in undefined behavior, which can crash the application.
2. Unhandled Exceptions: std::stol can throw std::invalid_argument or std::out_of_range if the string cannot be parsed or is out of range. These exceptions are not handled, which will terminate the program.
3. Incorrect Parsing: The logic assumes a suffix is always present. If the file contains just a number (e.g., "2048"), it will be mis-parsed as "204" with a suffix of '8', leading to a wildly incorrect cache size. According to kernel documentation, a value without a suffix should be interpreted as KiB.

A more robust implementation is suggested below to address these critical issues.

      // Parse size based on suffix
      long l2_cache_size;
      try {
        size_t pos = 0;
        l2_cache_size = std::stol(size_str, &pos);
        while (pos < size_str.length() && std::isspace(size_str[pos])) {
          pos++;
        }
        if (pos < size_str.length()) {
          char suffix = std::toupper(size_str[pos]);
          if (suffix == 'K') {
            l2_cache_size *= 1024;
          } else if (suffix == 'M') {
            l2_cache_size *= 1024 * 1024;
          }
        } else {
          // No suffix, kernel documentation says the value is in KiB.
          l2_cache_size *= 1024;
        }
      } catch (const std::exception& e) {
        std::printf(
            "Failed to parse L2 cache size from string '%s', setting l2_cache_size=1MB. "
            "This might have an impact on performance.", size_str.c_str());
        return static_cast<long>(1024 * 1024);
      }

[aditew01]

Nice, I like the logic!
Although, for reusability, can we use something like pytorch/cpuinfo to get the data? Unless it's not possible?

[fadara01]

+1 to using pytorch/cpuinfo, great suggestion @aditew01

[Radu2k]

I agree, good idea.

[fadara01]

Let's use TORCH_WARN

[fadara01]

Could we explain what these mean?

[Radu2k]

Keeping in mind that the usual format is <value><unit(K/M)> we get the unit in suffix, pop it form the back, and then convert the string(value) to a long type.

[fadara01]

Great catch and fix!
I'll test performance on my side as well

Thank you

[mergify]

This pull request has merge conflicts that must be resolved before it can be
merged. Please rebase the PR, @Radu2k.

https://docs.github.com/en/pull-requests/collaborating-with-pull-requests/working-with-forks/syncing-a-fork

[Radu2k]

Closing this as it will be treated in cpuinfo repo.

[Rohanjames1997]

@Radu2k thanks for this.

Are you planning to raise a similar PR to https://github.com/pytorch/cpuinfo? If not I can give it a shot later this week.

[Radu2k]

Hi @Rohanjames1997, I have this on my todo list currently but quite busy atm. Would be great to see a fix for this as soon as possible so please feel free to go ahead if you have availability. You can cc me on the PR and I am glad to help review it and test it across platforms. Thanks as well.

[Rohanjames1997]

Thanks @Radu2k

I've raised one here pytorch/cpuinfo#372

[Radu2k]

Great! Will review.