yubin00145865 / iperf

Automatically exported from code.google.com/p/iperf
Other
0 stars 0 forks source link

better error message for CPU affinity failure, add FreeBSD support #128

Closed GoogleCodeExporter closed 8 years ago

GoogleCodeExporter commented 8 years ago

On OSX I get this:

iperf3: error - unable to set CPU affinity: 

Maybe a better error message would "Setting CPU affinity not supported in OSX" ?

Original issue reported on code.google.com by bltier...@es.net on 20 Dec 2013 at 10:48

GoogleCodeExporter commented 8 years ago

Original comment by bltier...@es.net on 20 Dec 2013 at 11:20

GoogleCodeExporter commented 8 years ago
I guess this should do it . Don't have Mac :(

$ hg diff
diff -r 1536985dc854 src/iperf_error.c
--- a/src/iperf_error.c Fri Dec 20 15:05:21 2013 -0800
+++ b/src/iperf_error.c Sun Dec 22 20:21:38 2013 +0530
@@ -238,7 +238,11 @@
             snprintf(errstr, len, "protocol does not exist");
             break;
         case IEAFFINITY:
+#if defined(__APPLE__) && defined(__MACH__)  
+            snprintf(errstr, len, "Setting CPU affinity not supported in OSX");
+#else 
             snprintf(errstr, len, "unable to set CPU affinity");
+#endif
             perr = 1;
             break;
    case IEDAEMON:

Original comment by susant%redhat.com@gtempaccount.com on 22 Dec 2013 at 2:52

GoogleCodeExporter commented 8 years ago
One though occurred to me as I was looking at this...given that (at the moment) 
this feature is only supported on Linux, why do we even allow users to specify 
the -A option on platforms that don't support it?

(Just for completeness, I note that FreeBSD allows setting CPU affinity, but 
the API is completely different, see cpuset_getaffinity(2).)

Original comment by bmah@es.net on 3 Jan 2014 at 9:39

GoogleCodeExporter commented 8 years ago
I agree, just eliminate the ability to use -A in OSX, and add an issue to add 
freeBSD support in the future is probably the way to go.

Original comment by bltier...@gmail.com on 4 Jan 2014 at 3:24

GoogleCodeExporter commented 8 years ago
I implemented in freebsd with ifdef linux and freebsd. Tested with freebsd 9.2 
with a 4 cpu having 4 core each . Guess it's working .

./iperf3 -s -A 1
-----------------------------------------------------------
Server listening on 5201

./iperf3 -s -A 20
iperf3: error - unable to set CPU affinity: Resource deadlock avoided

Original comment by susant%redhat.com@gtempaccount.com on 5 Jan 2014 at 3:17

Attachments:

GoogleCodeExporter commented 8 years ago
I looked at the patch in Comment 5.  Looks mostly good (at least it's mostly 
along the lines of what I had in mind).

It's doing something that's a little different on FreeBSD than on Linux.  On 
Linux, -A takes either a single integer or two integers separated by commas.  
Let's ignore the second case for now.  The single integer appears to be a CPU 
number because it's used directly as the first argument to CPU_SET.

On FreeBSD, it takes the same parameters, but the single-integer parameter is 
used as a bitmask...almost like it's being used to recreate the CPU affinity 
mask.  It seems to me that to make FreeBSD to behave the same as Linux we 
should replace the loop in the FreeBSD case with a single call to CPU_SET:

  CPU_SET(affinity, &cpumask);

According to FreeBSD's cpuset(2) manpage, -1 can be used as the id_t argument 
to cpuset_* (with cpuset_which set to CPUSET_PID) to indicate "this process".  
So there's a (very small) optimization that can be made here to avoid the call 
to getpid() and just use -1 instead of the id variable.

We probably need some code in iperf_clearaffinity() to handle FreeBSD as well.  
I think this code would be extremely similar to the Linux case in that it'd 
pass a cpumask of all 1s to cpuset_setaffinity(2).

Thanks for making all of the -A support conditional.  There's at least one 
comment that says "Linux only" or some such thing that should be expanded to 
include FreeBSD.

Original comment by bmah@es.net on 6 Jan 2014 at 6:07

GoogleCodeExporter commented 8 years ago
1. I modified from getpid() to -1. And also got rid of some variables .

2. The issues came in iperf_clearaffinity
    I tried to do in the same way as linux. Binding a process to each CPU available let us unbind. It really did not worked out. Then Introduced a variable for freebsd 'cpumask' . The trick is to before binding store the current cpumask and while clearaffinity restore the same . This worked.

Original comment by susant%redhat.com@gtempaccount.com on 7 Jan 2014 at 7:40

Attachments:

GoogleCodeExporter commented 8 years ago
Committed a modified version of the patch from Comment 7 in 
85f085ddd2cb...thanks!

Closing as fixed.

Original comment by bmah@es.net on 13 Jan 2014 at 7:00