Re: iperf 2.0.2 bug?
On Wed, 26 Sep 2007 18:28:55 -0500
Ken Wong <kenw --at-- bang.arl.wustl.edu> wrote:
> Hi,
>
> I am running iperf-2.0.2 on Linux 2.6.21 on x86 hosts. We just upgraded
> to Linux 2.6.21 and the behavior of the UDP client seems to have
> changed ... even though we applied the linux-2.6.21 patch.
>
> The symptoms are that the UDP client outputs a message at the end of
> a transmission that says there are 2 out-of-order packets. After looking
> at tcpdump output, I realized that the client is outputting an incorrect
> sequence number in the FIN packets that are sent at the end of a UDP
> transmission. Furthermore, the cause seems to be that the FIN packet is
> sent out before the Reporter thread has had time to process all of its work;
> i.e., the sequence number seen by the transfer agent (packet->packetID)
> is different than the count seen by the Reporter (agent->report.cntDatagrams)
> because the Reporter still has a backlog of packet reports that it has
> not processed.
>
> I fixed the problem by calling thread_rest() in Client::Run() right before
> calling CloseReport() to give the Reporter thread to catch up but that
> may or may not be the best solution.
>
> Regards,
> Ken Wong
>
The whole thread_rest() methodolgy is crap. Please use the proper fix
from Ingo.
Subject: iperf: fix locking
From: Ingo Molnar <mingo --at-- elte.hu>
fix iperf locking - it was burning CPU time while polling
unnecessarily, instead of using the proper wait primitives.
Signed-off-by: Ingo Molnar <mingo --at-- elte.hu>
---
compat/Thread.c | 3 ---
src/Reporter.c | 13 +++++++++----
src/main.cpp | 2 ++
3 files changed, 11 insertions(+), 7 deletions(-)
--- a/src/Reporter.c 2005-05-02 13:09:27.000000000 -0700
+++ b/src/Reporter.c 2007-09-26 09:29:18.000000000 -0700
@@ -111,6 +111,7 @@ report_statistics multiple_reports[kRepo
char buffer[64]; // Buffer for printing
ReportHeader *ReportRoot = NULL;
extern Condition ReportCond;
+extern Condition ReportDoneCond;
int reporter_process_report ( ReportHeader *report );
void process_report ( ReportHeader *report );
int reporter_handle_packet( ReportHeader *report );
@@ -338,7 +339,7 @@ void ReportPacket( ReportHeader* agent,
// item
while ( index == 0 ) {
Condition_Signal( &ReportCond );
- thread_rest();
+ Condition_Wait( &ReportDoneCond );
index = agent->reporterindex;
}
agent->agentindex = 0;
@@ -346,7 +347,7 @@ void ReportPacket( ReportHeader* agent,
// Need to make sure that reporter is not about to be "lapped"
while ( index - 1 == agent->agentindex ) {
Condition_Signal( &ReportCond );
- thread_rest();
+ Condition_Wait( &ReportDoneCond );
index = agent->reporterindex;
}
@@ -553,6 +554,7 @@ void reporter_spawn( thread_Settings *th
}
Condition_Unlock ( ReportCond );
+again:
if ( ReportRoot != NULL ) {
ReportHeader *temp = ReportRoot;
//Condition_Unlock ( ReportCond );
@@ -575,9 +577,12 @@ void reporter_spawn( thread_Settings *th
// finished with report so free it
free( temp );
Condition_Unlock ( ReportCond );
+ Condition_Signal( &ReportDoneCond );
+ if (ReportRoot)
+ goto again;
}
- // yield control of CPU is another thread is waiting
- thread_rest();
+ Condition_Signal( &ReportDoneCond );
+ usleep(10000);
} else {
//Condition_Unlock ( ReportCond );
}
--- a/src/main.cpp 2005-05-02 13:09:27.000000000 -0700
+++ b/src/main.cpp 2007-09-26 09:29:18.000000000 -0700
@@ -96,6 +96,7 @@ extern "C" {
// records being accessed in a report and also to
// serialize modification of the report list
Condition ReportCond;
+ Condition ReportDoneCond;
}
// global variables only accessed within this file
@@ -141,6 +142,7 @@ int main( int argc, char **argv ) {
// Initialize global mutexes and conditions
Condition_Initialize ( &ReportCond );
+ Condition_Initialize ( &ReportDoneCond );
Mutex_Initialize( &groupCond );
Mutex_Initialize( &clients_mutex );