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

xen-console: Optimize console_display_thrd #66

Merged
merged 1 commit into from
Apr 17, 2024

Conversation

Deedone
Copy link
Contributor

@Deedone Deedone commented Apr 12, 2024

Optimize console_display_thrd by removing the inner loop that copies data from the console buffer to the output buffer. Instead, calculate the size of the data to be copied and print it in one go.

if (read) {
shell_fprintf(shell, SHELL_NORMAL, "%s", buf);
}
buf_pos = (console->int_cons++) &
Copy link
Collaborator

Choose a reason for hiding this comment

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

Increment should be removed

size = (console->int_prod - console->int_cons) &
(XEN_CONSOLE_BUFFER_SZ - 1);
shell_fprintf(shell, SHELL_NORMAL, "%.*s", size + 1,
&console->int_buf[buf_pos]);
Copy link
Collaborator

Choose a reason for hiding this comment

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

fprintf may overrun the end of int_buf if buf_pos is closer than size+1 bytes to end of array. This will cause access to foreign memory, so this should be normalized and divided into 2 parts in this corner case.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Yep. This is a circular buffer so wrap around should be handled gracefully.

@Deedone Deedone force-pushed the optimize-console branch 2 times, most recently from cbd5195 to cb01d94 Compare April 15, 2024 13:41
@Deedone
Copy link
Contributor Author

Deedone commented Apr 15, 2024

Fixed logic to prevent buffer overruns.

Copy link
Collaborator

@firscity firscity left a comment

Choose a reason for hiding this comment

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

With last small comment:
Reviewed-by: Dmytro Firsov <[email protected]>

(XEN_CONSOLE_BUFFER_SZ - 1);
prod_buf_pos = (console->int_prod) &
(XEN_CONSOLE_BUFFER_SZ - 1);
if (buf_pos < prod_buf_pos) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

Please, comment that this condition means no buffer wrap around and in other case we print only part before buffer end

Copy link
Collaborator

@lorc lorc left a comment

Choose a reason for hiding this comment

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

With @firscity comment addressed:

Reviewed-by: Volodymyr Babchuk <[email protected]>

Optimize console_display_thrd by removing the inner loop that copies
data from the console buffer to the output buffer. Instead, calculate
the size of the data to be copied and print it in one go.

Signed-off-by: Mykyta Poturai <[email protected]>
Reviewed-by: Dmytro Firsov <[email protected]>
Reviewed-by: Volodymyr Babchuk <[email protected]>
@firscity firscity merged commit 69edbe6 into xen-troops:main Apr 17, 2024
8 checks passed
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.

3 participants