[ltt-dev] [UST PATCH] Fix the once in a while freeze of the tests

[ltt-dev] [UST PATCH] Fix the once in a while freeze of the tests

[Yannick Brosseau]

Sometimes, the thread in the read function would lock the pipe so the
setlinebuf would freeze on it. Set the linebuf before we create the
thread to fix this deadlock

Signed-off-by: Yannick Brosseau <yannick.brosseau at gmail.com>
---
 tests/tap.c |    5 ++++-
 1 files changed, 4 insertions(+), 1 deletions(-)

diff --git a/tests/tap.c b/tests/tap.c
index a54fd17..4bb695e 100644
--- a/tests/tap.c
+++ b/tests/tap.c
@@ -58,6 +58,9 @@ static void tap_comment_stdout(void)
 		goto close_pipe;
 	}
 
+	/* Set it before we create the reading thread */
+	setlinebuf(pipe_r_file);
+
 	stdout_fileno = fileno(stdout);
 	if (stdout_fileno < 0) {
 		perror("# Couldn't get fileno for stdout!?");
@@ -112,7 +115,7 @@ static void tap_comment_stdout(void)
 
 	setlinebuf(stdout);
 	setlinebuf(stderr);
-	setlinebuf(pipe_r_file);
+
 
 	return;
 
-- 
1.7.2.3

[ltt-dev] [UST PATCH] Fix the once in a while freeze of the tests

[Mathieu Desnoyers]

* Yannick Brosseau (yannick.brosseau at gmail.com) wrote:
> Sometimes, the thread in the read function would lock the pipe so the
> setlinebuf would freeze on it. Set the linebuf before we create the
> thread to fix this deadlock

Can you update the description to show which locks are involved in this
deadlock scenario ? E.g.

- CPU A               - CPU B

function              function
lock A (taken)        lock B (taken)
lock B (waiting)
                      lock A (waiting)  <-- deadlock

Or show it with the lock chain dependency analysis. But it's important
to have this information along with this kind of fix.

Thanks,

Mathieu
> 
> Signed-off-by: Yannick Brosseau <yannick.brosseau at gmail.com>
> ---
>  tests/tap.c |    5 ++++-
>  1 files changed, 4 insertions(+), 1 deletions(-)
> 
> diff --git a/tests/tap.c b/tests/tap.c
> index a54fd17..4bb695e 100644
> --- a/tests/tap.c
> +++ b/tests/tap.c
> @@ -58,6 +58,9 @@ static void tap_comment_stdout(void)
>  		goto close_pipe;
>  	}
>  
> +	/* Set it before we create the reading thread */
> +	setlinebuf(pipe_r_file);
> +
>  	stdout_fileno = fileno(stdout);
>  	if (stdout_fileno < 0) {
>  		perror("# Couldn't get fileno for stdout!?");
> @@ -112,7 +115,7 @@ static void tap_comment_stdout(void)
>  
>  	setlinebuf(stdout);
>  	setlinebuf(stderr);
> -	setlinebuf(pipe_r_file);
> +
>  
>  	return;
>  
> -- 
> 1.7.2.3
> 
> 
> _______________________________________________
> ltt-dev mailing list
> ltt-dev at lists.casi.polymtl.ca
> http://lists.casi.polymtl.ca/cgi-bin/mailman/listinfo/ltt-dev
> 

-- 
Mathieu Desnoyers
Operating System Efficiency R&D Consultant
EfficiOS Inc.
http://www.efficios.com

[ltt-dev] [UST PATCH] Fix the once in a while freeze of the tests

[Mathieu Desnoyers]

* Mathieu Desnoyers (compudj at krystal.dyndns.org) wrote:
> * Yannick Brosseau (yannick.brosseau at gmail.com) wrote:
> > Sometimes, the thread in the read function would lock the pipe so the
> > setlinebuf would freeze on it. Set the linebuf before we create the
> > thread to fix this deadlock
> 
> Can you update the description to show which locks are involved in this
> deadlock scenario ? E.g.
> 
> - CPU A               - CPU B
> 
> function              function
> lock A (taken)        lock B (taken)
> lock B (waiting)
>                       lock A (waiting)  <-- deadlock
> 
> Or show it with the lock chain dependency analysis. But it's important
> to have this information along with this kind of fix.

Hrm, ok I looked at tap.c, and it's not a deadlock at all: it's rather
that the _tap_comment_stdout thread can start using pipe_r_file when it
is still uninitialized.

I'll update the comment and pull the patch.

Thanks,

Mathieu

> 
> Thanks,
> 
> Mathieu
> > 
> > Signed-off-by: Yannick Brosseau <yannick.brosseau at gmail.com>
> > ---
> >  tests/tap.c |    5 ++++-
> >  1 files changed, 4 insertions(+), 1 deletions(-)
> > 
> > diff --git a/tests/tap.c b/tests/tap.c
> > index a54fd17..4bb695e 100644
> > --- a/tests/tap.c
> > +++ b/tests/tap.c
> > @@ -58,6 +58,9 @@ static void tap_comment_stdout(void)
> >  		goto close_pipe;
> >  	}
> >  
> > +	/* Set it before we create the reading thread */
> > +	setlinebuf(pipe_r_file);
> > +
> >  	stdout_fileno = fileno(stdout);
> >  	if (stdout_fileno < 0) {
> >  		perror("# Couldn't get fileno for stdout!?");
> > @@ -112,7 +115,7 @@ static void tap_comment_stdout(void)
> >  
> >  	setlinebuf(stdout);
> >  	setlinebuf(stderr);
> > -	setlinebuf(pipe_r_file);
> > +
> >  
> >  	return;
> >  
> > -- 
> > 1.7.2.3
> > 
> > 
> > _______________________________________________
> > ltt-dev mailing list
> > ltt-dev at lists.casi.polymtl.ca
> > http://lists.casi.polymtl.ca/cgi-bin/mailman/listinfo/ltt-dev
> > 
> 
> -- 
> Mathieu Desnoyers
> Operating System Efficiency R&D Consultant
> EfficiOS Inc.
> http://www.efficios.com
> 
> _______________________________________________
> ltt-dev mailing list
> ltt-dev at lists.casi.polymtl.ca
> http://lists.casi.polymtl.ca/cgi-bin/mailman/listinfo/ltt-dev
> 

-- 
Mathieu Desnoyers
Operating System Efficiency R&D Consultant
EfficiOS Inc.
http://www.efficios.com

[ltt-dev] [UST PATCH] Fix the once in a while freeze of the tests

[Yannick Brosseau]

On 2011-02-20 10:17, Mathieu Desnoyers wrote:
> * Mathieu Desnoyers (compudj at krystal.dyndns.org) wrote:
>   
>> * Yannick Brosseau (yannick.brosseau at gmail.com) wrote:
>>     
>>> Sometimes, the thread in the read function would lock the pipe so the
>>> setlinebuf would freeze on it. Set the linebuf before we create the
>>> thread to fix this deadlock
>>>       
>> Can you update the description to show which locks are involved in this
>> deadlock scenario ? E.g.
>>
>> - CPU A               - CPU B
>>
>> function              function
>> lock A (taken)        lock B (taken)
>> lock B (waiting)
>>                       lock A (waiting)  <-- deadlock
>>
>> Or show it with the lock chain dependency analysis. But it's important
>> to have this information along with this kind of fix.
>>     
> Hrm, ok I looked at tap.c, and it's not a deadlock at all: it's rather
> that the _tap_comment_stdout thread can start using pipe_r_file when it
> is still uninitialized.
>
>   
I found a lock in the calls to fgets and setlinebuf, deep in the glibc.
If what you says is true, the fix I've proposed is not right. Its the
pthread_create that should be moved.

[ltt-dev] [UST PATCH] Fix the once in a while freeze of the tests

[Mathieu Desnoyers]

* Yannick Brosseau (yannick.brosseau at gmail.com) wrote:
> On 2011-02-20 10:17, Mathieu Desnoyers wrote:
> > * Mathieu Desnoyers (compudj at krystal.dyndns.org) wrote:
> >   
> >> * Yannick Brosseau (yannick.brosseau at gmail.com) wrote:
> >>     
> >>> Sometimes, the thread in the read function would lock the pipe so the
> >>> setlinebuf would freeze on it. Set the linebuf before we create the
> >>> thread to fix this deadlock
> >>>       
> >> Can you update the description to show which locks are involved in this
> >> deadlock scenario ? E.g.
> >>
> >> - CPU A               - CPU B
> >>
> >> function              function
> >> lock A (taken)        lock B (taken)
> >> lock B (waiting)
> >>                       lock A (waiting)  <-- deadlock
> >>
> >> Or show it with the lock chain dependency analysis. But it's important
> >> to have this information along with this kind of fix.
> >>     
> > Hrm, ok I looked at tap.c, and it's not a deadlock at all: it's rather
> > that the _tap_comment_stdout thread can start using pipe_r_file when it
> > is still uninitialized.
> >
> >   
> I found a lock in the calls to fgets and setlinebuf, deep in the glibc.
> If what you says is true, the fix I've proposed is not right. Its the
> pthread_create that should be moved.
> 

I don't understand. It should only be the relative order of init vs
pthread_create (the user of this structure) that matters, no ?

Mathieu


-- 
Mathieu Desnoyers
Operating System Efficiency R&D Consultant
EfficiOS Inc.
http://www.efficios.com