Hey, On Tue, Mar 08, 2022 at 03:20:17PM -0600, Glenn Washburn wrote: > The first patch looks like it was a copy/paste error. If the net module is > unloaded, grub_net_poll_cards_idle should be NULL so that a function in the > net module which now doesn't exist. > > The second and third patches are for performance and were helpful when > debugging GRUB. When the net module is loaded, even if there are no net > cards found, grub_net_poll_cards_idle will call grub_net_tcp_retransmit() > unconditionally. But there's no need to go through tcp retransmit logic if > there aren't any cards that we could be retransmitting on. > > As for the third patch, if the machine has network cards found, but there are > no tcp open sockets (because the user doesn't use the network to boot), then > grub_net_tcp_retransmit() should be a noop. Thus is doesn't need to call > grub_get_time_ms(), which does a call into firmware on powerpc-ieee1275. So > only call grub_get_time_ms() if there are tcp sockets. > > Calls to grub_net_poll_cards_idle can happen a lot when GRUB is waiting for a > keypress (grub_getkey_noblock calls grub_net_poll_cards_idle). I found this > annoying when debugging an issue in GRUB with GDB and I found that nearly > every time I interrupted GRUB with the GDB I was landing in OpenBIOS's > milliseconds call and then I had to step out back into GRUB which could be > a hundred or more instructions. This reduces the probability that interrupting > GRUB lands in the firmware when GRUB is blocked waiting on a keypress.
In general I think this cover letter should be split into three parts and they should go into relevant patches. Additionally, if you check pointers for NULL please use "== NULL" comparison explicitly instead of shortened version like "if (pointer)". Otherwise patches LGTM. Daniel _______________________________________________ Grub-devel mailing list Grub-devel@gnu.org https://lists.gnu.org/mailman/listinfo/grub-devel
