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
Merged
Changes from all commits
Commits
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
34 changes: 19 additions & 15 deletions xen-console-srv/src/xen_console.c
Original file line number Diff line number Diff line change
Expand Up @@ -367,9 +367,8 @@ static void console_display_thrd(void *p1, void *p2, void *p3)
ARG_UNUSED(p3);
struct xen_domain_console *console = p1;
const struct shell *shell = p2;
/* Buffer input a little */
char buf[32];
int read;
size_t buf_pos, prod_buf_pos;
int size;

shell_info(shell, "Attached to a domain console");

Expand All @@ -387,19 +386,24 @@ static void console_display_thrd(void *p1, void *p2, void *p3)
}

while (console->int_cons < console->int_prod) {
read = 0;
memset(buf, 0, sizeof(buf));

/* TODO: There is a room for optimization.... */
while (console->int_cons < console->int_prod &&
read < sizeof(buf) - 1) {
size_t buf_pos = (console->int_cons++) &
(XEN_CONSOLE_BUFFER_SZ - 1);
buf[read++] = console->int_buf[buf_pos];
}
if (read) {
shell_fprintf(shell, SHELL_NORMAL, "%s", buf);
buf_pos = (console->int_cons) &
(XEN_CONSOLE_BUFFER_SZ - 1);
prod_buf_pos = (console->int_prod) &
(XEN_CONSOLE_BUFFER_SZ - 1);
/* int_buf is a circular buffer so we need to check for
* the wrap around condition to print it safely.
* In the case of the wrap around condition, we first
* print from the buf_pos to the end of the buffer,
* and then continue printing from the beginning of the buffer.
*/
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

size = prod_buf_pos - buf_pos;
} else {
size = XEN_CONSOLE_BUFFER_SZ - buf_pos;
}
shell_fprintf(shell, SHELL_NORMAL, "%.*s", size,
&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.

console->int_cons += size;
}
k_mutex_unlock(&console->lock);
}
Expand Down