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

MSRIOGroup: File based save and restore does not use batch context tracking #3379

Open
2 tasks
bgeltz opened this issue Mar 22, 2024 · 0 comments
Open
2 tasks
Labels
tech debt Refactoring and maintenance tasks

Comments

@bgeltz
Copy link
Contributor

bgeltz commented Mar 22, 2024

The ability to maintain multiple batch contexts for MSRIO was introduced in PR #2972. This PR also modified the MSRIOGroup's in-memory version of save_control() to leverage the new batch context tracking by introducing m_save_restore_ctx. This version of save_control() is only ever called by the Controller during Controller::run(). This call will only ever get into the MSRIOGroup if the end-user has direct access to the MSR or msr-safe file descriptors (which is only possible presently if the user has CAP_SYS_ADMIN).

During "normal" expected operation where the end-user does not have direct access to manipulate MSRs, all of that I/O happens through the ServiceIOGroup instead. When the user initiates a new session, and that session is a write_mode() session, pio.save_control_dir() is invoked from service.py. This API ultimately calls into the file-based version of MSRIOGroup::save_control(const std::string path). This file-based version utilizes the SaveControl object which ultimately uses read_signal() to determine the value to "save" initially.

This all became apparent through the investigation of #3352.

The following needs to be changed in order for SaveControl to utilize the batch context tracking:

  • SaveControl needs to have a way to issue read_batch and sample either through the existing m_save_restore_ctx or a new one. Presently in MSRIOGroup, this is done through direct access to the MSRFieldControl objects that are contained within m_control_available. The MSRFieldControl objects have a save() method which calls sample() and stores the result in a class member. This could somehow be exposed for SaveControl to query. This could be done by making SaveControl a friend of MSRIOGroup, and making m_control_available protected instead of private.
  • A better solution would be to plumb the batch context through the IOGroup interface so that this wouldn't be a one-off solution just for the MSRIOGroup.

This is a good topic for an architecture meeting as multiple solutions are possible.

@bgeltz bgeltz added the tech debt Refactoring and maintenance tasks label Mar 22, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
tech debt Refactoring and maintenance tasks
Projects
None yet
Development

No branches or pull requests

1 participant