Re: [PATCH] fix hang when BLKTRACESETUP fails and "-o -" is used

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 change here looks good to me.

-Jeff

Eric Sandeen <[email protected]> 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 <[email protected]>
> ---
>
> 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())