Eric Sandeen <sandeen@xxxxxxxxxxx> writes: > On 6/6/24 6:32 AM, Jeff Moyer wrote: >> Hi, Eric, >> >> This looks like a repost? I mentioned that we should also set done = 1 >> for a failed setup_tracer_devpaths() call, and I was hoping you'd >> include that in a new version (or at least comment on whether you >> thought it was a good idea). Sorry if that wasn't clear! > > The first one was a question, this one was a patch :) Ah, ok. :) > I'm sorry, I have not had time to look into or test for failed > setup_tracer_devpaths() calls, and hoped to just fix the bug > I had encountered. That's fair. Reviewed-by: Jeff Moyer <jmoyer@xxxxxxxxxx> Thanks! > >> The change here looks good to me. > > Thanks, > -Eric > >> -Jeff >> >> Eric Sandeen <sandeen@xxxxxxxxxx> writes: >> >>> # blktrace -o - /dev/sda /dev/sdb /dev/sdc >>> >>> has to be SIGKILLed if BLKTRACESETUP fails for any or all of the devices >>> listed. (I simulated this by just catching one of the devices in >>> setup_buts(), skipping the ioctl, and doing ret++). >>> >>> This seems to be because with "-o -" on the command line, use_tracer_devpaths() >>> sees piped_output set, so we call process_trace_bufs which ends up waiting on >>> (!done) and "done" is never set. i.e. >>> >>> atexit(exit_tracing) >>> wait_tracers >>> if (use_tracer_devpaths()) // true because "-o -" >>> process_trace_bufs >>> while (wait_empty_entries()) >>> wait_empty_entries >>> while (!done ... ) >>> <loop forever> >>> >>> I think this can be avoided by just setting "done = 1" before returning >>> when setup_buts() fails in run_tracers(). >>> >>> Signed-off-by: Eric Sandeen <sandeen@xxxxxxxxxx> >>> --- >>> >>> diff --git a/blktrace.c b/blktrace.c >>> index 3444fbb..038b2cb 100644 >>> --- a/blktrace.c >>> +++ b/blktrace.c >>> @@ -2684,8 +2684,10 @@ static int run_tracers(void) >>> if (net_mode == Net_client) >>> printf("blktrace: connecting to %s\n", hostname); >>> >>> - if (setup_buts()) >>> + if (setup_buts()) { >>> + done = 1; >>> return 1; >>> + } >>> >>> if (use_tracer_devpaths()) { >>> if (setup_tracer_devpaths()) >> >>