On 9/29/20 10:56 AM, Jacob Keller wrote:
On 9/29/2020 10:18 AM, Jakub Kicinski wrote:
On Mon, 28 Sep 2020 16:49:45 -0700 Jacob Keller wrote:
For some devices, updating the flash can take significant time during
operations where no status can meaningfully be reported. This can be
somewhat confusing to a user who sees devlink appear to hang on the
terminal waiting for the device to update.
Provide a ticking counter of the time elapsed since the previous status
message in order to make it clear that the program is not simply stuck.
Do not display this message unless a few seconds have passed since the
last status update. Additionally, if the previous status notification
included a timeout, display this as part of the message. If we do not
receive an error or a new status without that time out, replace it with
the text "timeout reached".
Signed-off-by: Jacob Keller <[email protected]>
---
Sending this as an RFC because I doubt this is the best implementation. For
one, I get a weird display issue where the cursor doesn't always end up on
the end of line in my shell.. The % display works properly, so I'm not sure
what's wrong here.
Second, even though select should be timing out every 1/10th of a second for
screen updates, I don't seem to get that behavior in my test. It takes about
8 to 10 seconds for the first elapsed time message to be displayed, and it
updates really slowly. Is select just not that precise? I even tried using a
timeout of zero, but this means we refresh way too often and it looks bad. I
am not sure what is wrong here...
Strange. Did you strace it? Perhaps it's some form of output buffering?
Haven't yet, just noticed the weird output behavior and timing
inconsistency.
Add a fflush(stdout) at the end of pr_out_tty()? Or maybe simply
disable stdout buffering with setvbuf().
diff --git a/devlink/devlink.c b/devlink/devlink.c
index 0374175eda3d..7fb4b5ef1ebe 100644
--- a/devlink/devlink.c
+++ b/devlink/devlink.c
@@ -33,6 +33,7 @@
#include <sys/select.h>
#include <sys/socket.h>
#include <sys/types.h>
+#include <sys/time.h>
#include <rt_names.h>
#include "version.h"
@@ -3066,6 +3067,9 @@ static int cmd_dev_info(struct dl *dl)
struct cmd_dev_flash_status_ctx {
struct dl *dl;
+ struct timeval last_status_msg;
Having some 'time' concept in this name will make it less likely for the
reader to confuse it with something that holds a text message - 't_', or
'_secs' or something.
sln