Skip to content

nvme-top: add paging support and improve topology update handling #3529

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.

Sign up for GitHub

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

Open
shroffni wants to merge 3 commits into
base: master
Choose a base branch
from
+186 −33
Open

nvme-top: add paging support and improve topology update handling #3529

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
8f08d72
nvme-top: restore selected subsystem to the first entry
shroffni Jul 1, 2026
52b5d5f
nvme-top: support paging in the top dashboard
shroffni Jul 1, 2026
43608d4
nvme-top: synthesize NVMe kobject uevent on ENOBUFS
shroffni Jul 1, 2026
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
120 changes: 113 additions & 7 deletions nvme-print-stdout-top.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1069,7 +1069,7 @@ static void stdout_top_print_subsys_topology_footer(
struct dashboard_ctx *db_ctx, FILE *stream)
{
fprintf(stream, "\n--------------------------------------\n");
fprintf(stream, "[ESC to go back to the previous screen, q to quit]\n");
fprintf(stream, "[up/down keys to navigate, ESC to go back to the previous screen, q to quit]\n");

dashboard_set_footer_rows(db_ctx, 3);

Expand Down Expand Up @@ -1162,6 +1162,57 @@ static int stdout_top_find_subsys_by_name(libnvme_subsystem_t *subsys_arr,
return -1;
}

static int handle_event_page_down(struct dashboard_ctx *db_ctx)
{
int data_start, data_rows, frame_rows;
int scroll_down, scroll = 0;

data_start = dashboard_get_data_start(db_ctx);
data_rows = dashboard_get_data_rows(db_ctx);
frame_rows = dashboard_get_frame_data_rows(db_ctx);

/*
* Scroll down max up to one page frame from current data_start index.
* While scrolling down, ensure we don't move past the max available
* data rows. If we are already on the last page frame or there's no
* data if move past the current page frame then ignore the key press.
*/
scroll_down = data_start + frame_rows;

if (scroll_down < data_rows) {
dashboard_set_data_start(db_ctx, scroll_down);
scroll = 1;
}

return scroll;
}

static int handle_event_page_up(struct dashboard_ctx *db_ctx)
{
int data_start, frame_rows, off;
int scroll_up, scroll = 0;

data_start = dashboard_get_data_start(db_ctx);
frame_rows = dashboard_get_frame_data_rows(db_ctx);

/*
* Scroll back up max up to one page frame. Determine the num of data
* rows we can scroll back up based on current data start index and max
* num of rows which could be drawn in one page frame. If we're already
* on the first page frame and hence we can't scroll back then ignore
* the key press.
*/
off = min(data_start, frame_rows);
scroll_up = data_start - off;

if (scroll_up != data_start) {
dashboard_set_data_start(db_ctx, scroll_up);
scroll = 1;
}

return scroll;
}

/*
* Draws subsys topology screen of susbystem @s
* Returns: 0 if ESC key is pressed or needs to draw subsystem selection screen
Expand Down Expand Up @@ -1257,7 +1308,22 @@ static int stdout_top_draw_subsys_topology_screen(struct dashboard_ctx *db_ctx,
* topology screen.
*/
scroll = 0;
} /* else unknown event, ignore */
} else if (event == EVENT_TYPE_KEY_PAGE_DOWN) {

scroll = handle_event_page_down(db_ctx);
if (scroll)
goto draw;

goto wait_for_event;

} else if (event == EVENT_TYPE_KEY_PAGE_UP) {

scroll = handle_event_page_up(db_ctx);
if (scroll)
goto draw;

goto wait_for_event;
} /* else ignore */
}

return ret;
Expand Down Expand Up @@ -1404,7 +1470,7 @@ static int stdout_top_draw_subsys_screen(struct dashboard_ctx *db_ctx,
table_print_stream(stream, t);

fprintf(stream, "\n--------------------------------------\n");
fprintf(stream, "[up/down arrow keys to navigate, Enter to view, q to quit]\n");
fprintf(stream, "[up/down keys to navigate, Enter to view, q to quit]\n");

/*
* Footer rows are calculated manually.
Expand Down Expand Up @@ -1475,20 +1541,43 @@ void stdout_top(int refresh_interval)
s = subsys_arr[subsys_idx];
quit = stdout_top_draw_subsys_topology_screen(db_ctx,
stream, s);
scroll = 0;
if (quit)
break;
fallthrough;

scroll = 0;
free(subsys_arr);
subsys_arr = NULL;
libnvme_free_global_ctx(ctx);

ctx = stdout_top_rescan_topology();

@igaw igaw Jul 1, 2026

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

I haven't noticed during the initial review. I assume you have to free the global context so that all resources are freed from libnvme_scan_topology? The idea about the global context that it is allocated only once, so we need to look into a libnvme_rescan_topology I think.

@igaw igaw Jul 1, 2026

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

So for this series this is fine, but we should really make the library able to deal with this use case.

@igaw igaw Jul 2, 2026

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

There is libnvme_refresh_topology, which should do the trick,

@shroffni shroffni Jul 2, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Yes this looks good. Should I fix it in this series? Or shall I post it in a separate patch?

@igaw igaw Jul 2, 2026

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

you can post a separate patch for this or update the series. whatever you prefer. Could you look through the copilot findings below though? No need for long explanation if you think it's wrong.

@shroffni shroffni Jul 3, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Okay so I have addressed comments from copilot and per the review comment I will update the code. I am glad that copilot-pro is really able to find subtle issues which human eyes may miss.

Regarding libnvme_refresh_topology(), I will handle it in a separate patch.

if (!ctx) {
quit = 1;
break;
}

/*
* The topology may have changed while the subsystem
* dashboard was active. Restart from the first
* subsystem instead of trying to restore the previous
* screen position.
*/
subsys_idx = 0;
subsys_arr = stdout_top_build_subsys_arr(ctx,
&num_subsys);
if (!subsys_arr)
Comment on lines +1564 to +1567
quit = 1;

break;
case EVENT_TYPE_NVME_UEVENT: {
__cleanup_free char *subsys_name = NULL;
libnvme_subsystem_t s;
libnvme_subsystem_t s = subsys_arr[subsys_idx];

s = subsys_arr[subsys_idx];
subsys_name = strdup(libnvme_subsystem_get_name(s));
if (!subsys_name) {
quit = 1;
break;
}

free(subsys_arr);
subsys_arr = NULL;
libnvme_free_global_ctx(ctx);
Expand All @@ -1511,6 +1600,7 @@ void stdout_top(int refresh_interval)
if (subsys_idx < 0)
subsys_idx = 0;

scroll = 0;
break;
}
case EVENT_TYPE_KEY_DOWN:
Expand Down Expand Up @@ -1582,6 +1672,22 @@ void stdout_top(int refresh_interval)
*/
scroll = 0;
break;
case EVENT_TYPE_KEY_PAGE_DOWN:
scroll = handle_event_page_down(db_ctx);
if (scroll) {
subsys_idx = dashboard_get_data_start(db_ctx);
goto draw;
}

goto wait_for_event;
case EVENT_TYPE_KEY_PAGE_UP:
scroll = handle_event_page_up(db_ctx);
if (scroll) {
subsys_idx = dashboard_get_data_start(db_ctx);
goto draw;
}

goto wait_for_event;
default: /* unknown event, ignore */
continue;
}
Expand Down
71 changes: 57 additions & 14 deletions util/dashboard.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -463,6 +463,16 @@ static int wait_for_event(struct dashboard_ctx *db_ctx,
if (errno == EINTR)
continue;

/*
* We may have missed some netlink
* uevents from kernel. This is not a
* fatal error and we may synthesize it
* as an NVME kobject change event and
* force a topology rescan.
*/
if (errno == ENOBUFS)
return EVENT_TYPE_NVME_UEVENT;

nvme_show_perror("read from uevent fd");
return n;
}
Expand Down Expand Up @@ -509,6 +519,48 @@ static int wait_for_event(struct dashboard_ctx *db_ctx,
}
}

static enum event_type wait_for_esc_seq(struct dashboard_ctx *db_ctx)
{
unsigned char c;
enum event_type event;

event = wait_for_event(db_ctx, &c, 1);
switch (event) {
case EVENT_TYPE_ERROR: /* fall through */
case EVENT_TYPE_KEY_QUIT: /* fall through */
case EVENT_TYPE_TIMEOUT:
return event;
default:
Comment on lines +528 to +533

@shroffni shroffni Jul 3, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I agree we may receive EVENT_TYPE_NVME_UEVENT or EVENT_TYPE_SIGWINCH while waiting for escape key sequence. I'd rather update wait_for_esc_seq() to not include fds which monitor uevents and sigwinch.

As the name wait_for_esc_seq() suggests we're waiting for escape sequence, it makes more sense to only include key stroke monitor fd. I'll update the patch and resend.

if (c == 65)
return EVENT_TYPE_KEY_UP;
else if (c == 66)
return EVENT_TYPE_KEY_DOWN;
else if (c == 53 || c == 54) {
char prev = c;

event = wait_for_event(db_ctx, &c, 1);
switch (event) {
case EVENT_TYPE_ERROR: /* fall through */
case EVENT_TYPE_KEY_QUIT: /* fall through */
case EVENT_TYPE_TIMEOUT:
return event;
default:
Comment on lines +542 to +547

@shroffni shroffni Jul 3, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Same comment as above. I will exclude monitor fds which doesn't wait for or monitor key press.

if (c == 126) {
if (prev == 53)
return EVENT_TYPE_KEY_PAGE_UP;
else
return EVENT_TYPE_KEY_PAGE_DOWN;
}
/* else ignore */
break;
}
}
/* else ignore */
break;
}
return EVENT_TYPE_IGNORE;
}

enum event_type dashboard_wait_for_event(struct dashboard_ctx *db_ctx)
{
int event;
Expand Down Expand Up @@ -538,21 +590,12 @@ enum event_type dashboard_wait_for_event(struct dashboard_ctx *db_ctx)
return EVENT_TYPE_KEY_ESC;
default:
if (c == 91) { /* '[' key */
event = wait_for_event(db_ctx, &c, 1);
switch (event) {
case EVENT_TYPE_ERROR: /* fall through */
case EVENT_TYPE_KEY_QUIT:
return event;
case EVENT_TYPE_TIMEOUT:
break;
default:
if (c == 65)
return EVENT_TYPE_KEY_UP;
else if (c == 66)
return EVENT_TYPE_KEY_DOWN;
/* else ignore */
event = wait_for_esc_seq(db_ctx);
if (event == EVENT_TYPE_TIMEOUT ||
event == EVENT_TYPE_IGNORE)
break;
}

return event;
} /* else ignore */
break;
}
Expand Down
28 changes: 16 additions & 12 deletions util/dashboard.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,18 +6,22 @@
struct dashboard_ctx;

enum event_type {
EVENT_TYPE_ERROR = -1, /* error waiting for event */
EVENT_TYPE_TIMEOUT, /* timed out waiting for event */

EVENT_TYPE_KEY_PRESS, /* key pressed event */
EVENT_TYPE_KEY_ESC, /* ESC key is pressed*/
EVENT_TYPE_KEY_UP, /* UP arrow key is pressed */
EVENT_TYPE_KEY_DOWN, /* DOWN arrow key is pressed */
EVENT_TYPE_KEY_RETURN, /* Return/Enter key is pressed */
EVENT_TYPE_KEY_QUIT, /* q is pressed */

EVENT_TYPE_NVME_UEVENT, /* kobject uevent received; rescan topology */
EVENT_TYPE_SIGWINCH, /* SIGWINCH received */
EVENT_TYPE_ERROR = -1, /* error waiting for event */
EVENT_TYPE_TIMEOUT, /* timed out waiting for event */

EVENT_TYPE_KEY_PRESS, /* key pressed event */
EVENT_TYPE_KEY_ESC, /* ESC key is pressed*/
EVENT_TYPE_KEY_UP, /* UP arrow key is pressed */
EVENT_TYPE_KEY_DOWN, /* DOWN arrow key is pressed */
EVENT_TYPE_KEY_PAGE_UP, /* Page UP key is pressed*/
EVENT_TYPE_KEY_PAGE_DOWN, /* Page DOWN key is pressed */
EVENT_TYPE_KEY_RETURN, /* Return/Enter key is pressed */
EVENT_TYPE_KEY_QUIT, /* q is pressed */

EVENT_TYPE_NVME_UEVENT, /* kobject uevent received; rescan topology */
EVENT_TYPE_SIGWINCH, /* SIGWINCH received */

EVENT_TYPE_IGNORE, /* ignore event */
};

int dashboard_get_interval(struct dashboard_ctx *db_ctx);
Expand Down
Loading