Here is what the recently updated benchmarks/iozone fails like for armv6 builds based on the updates that were recently made (only listing the first few warnings but including the one error): cc -c -pipe -mcpu=cortex-a7 -g -fno-strict-aliasing -DFreeBSD -Dunix -Dbsd4_4 -DHAVE_ANSIC_C -DASYNC_IO -DHAVE_PREAD -DNAME='"freebsd"' -DSHARED_MEM -pipe - mcpu=cortex-a7 -g -fno-strict-aliasing iozone.c -o iozone_freebsd.o iozone.c:363:14: error: typedef redefinition with different types ('long' vs '__off64_t' (aka 'long long')) typedef long off64_t; ^ /usr/include/sys/types.h:178:19: note: previous definition is here typedef __off64_t off64_t; /* file offset (alias) */ ^ iozone.c:762:9: warning: 'CACHE_LINE_SIZE' macro redefined [-Wmacro-redefined] #define CACHE_LINE_SIZE 32 ^ /usr/include/machine/param.h:109:9: note: previous definition is here #define CACHE_LINE_SIZE (1 << CACHE_LINE_SHIFT) ^ iozone.c:1921:24: warning: format specifies type 'long *' but the argument has type 'off64_t *' (aka 'long long *') [-Wformat] sscanf(optarg,"%ld",&kilobytes64); ~~~ ^~~~~~~~~~~~ %lld iozone.c:1945:68: warning: format specifies type 'long' but the argument has type 'off64_t' (aka 'long long') [-Wformat] sprintf(splash[splash_line++],"\tFile size set to %ld kB\n",kilobytes64); ~~~ ^~~~~~~~~~~ %lld iozone.c:2319:82: warning: format specifies type 'long' but the argument has type 'off64_t' (aka 'long long') [-Wformat] sprintf(splash[splash_line++],"\tUsing minimum file size of %ld kilobytes.\n",minimum_file_size); ~~~ ^~~~~~~~~~~~~~~~~ %lld powerpc (32-bit) likely would get the same sort of messages, including the error. The same for other ILP32 FreeBSD architectures: long is only 32-bits but off_t is 64 bits. -r418913 of /usr/ports was used for the above under 11.0-BETA2 (so clang 3.8.0).
Investigating this now.
A commit references this bug: Author: jpaetzel Date: Sat Jul 23 17:06:53 UTC 2016 New revision: 418971 URL: https://svnweb.freebsd.org/changeset/ports/418971 Log: Unbreak on 32bit FreeBSD PR: 211289 Sponsored by: iXsystems Pointyhat: jpaetzel Cluebat: jhb Changes: head/benchmarks/iozone/files/patch-iozone.c
(In reply to commit-hook from comment #2) This allows the compile to complete but iozone is still broken for ILP32 architectures: the warnings produces are real problems because of format vs. size mismatches and the like. I've submitted 211317 for the problems with using long formats such as %ld for treating long long storage or values in an ILP32 FreeBSD archtiecture. powerpc (32-bit) has more issues than armv6/i836 because of being big endian. But I'll not have access to the powerpc's again for weeks yet.
Ok, so I have a couple questions: 1) Was this broken before I started mucking with it? 2) For a tool like iozone do we really care if it doesn't work on platforms that will most likely be network/router? 3) Will using j for the format specifier work? (it seems to work for amd64/i386 which are the only platforms I have access to) Roger on the no upstreaming the fixes, *my* interest is keeping this working on amd64 / FreeNAS / ZFS. I'm assuming if I try hard enough I can get the other platforms working in qemu.
(In reply to Josh Paetzel from comment #4) >Ok, so I have a couple questions: >1) Was this broken before I started mucking with it? 211152 predated your working on benchmarks/iozone. armv6 and the like compiled but amd64 did not. Your initial change swapped that status. w.schwarzenfeld@utanet.at was working on 211152 trying to make benchmarks/iozone better for FreeBSD generally, including for making things cleaner for FreeBSD upstream if his changes were accepted (once completed). He also was attempting to base things on the modern iozone releases (3.434 -> 3.444 at the time, now possibly 3.446). You have been going back through some of the issues he had run into and had been figuring out and updating is patches for. As far as I know w.schwarzenfeld@utanet.at may have been trying for just a one time update. He may not have been trying for eventually being the maintainer. (I've never asked about that.) >2) For a tool like iozone do we really care if it doesn't work on platforms that will most likely be network/router? I do not use the rpi2 as a network/router. Nor do I use the powerpc's for such whenI have access. To some extent the machines are used to check for a degree of code portability. But this tends to involve using other software (ports) in supporting development activities or checking the context's characteristics. [I've no clue about proportions of users of various types.] >3) Will using j for the format specifier work? (it seems to work for amd64/i386 which are the only platforms I have access to) j format and intmax_t are too modern to go upstream. But for a FreeBSD specific fork would be fine. Fixing the unsafe sscanf usage with strtoll and strtol usage that did more validation and the like would have a similar status. >*my* interest is keeping this working on amd64 / FreeNAS / ZFS Sounds more like you want to have a special purpose fork of iozone. It does not sounds like you plan/want to track upstream updates but to diverge to be specific to specific FreeBSD-related architectures. As the maintainer, how will you handle requests to apply patches that target allowing other FreeBSD architectures or cleaner/smaller updates when/if upstream adopts changes? I have no clue how common it may be for ports to target only specific FreeBSD architectures when upstream targets a wider range of contexts generally. [Note at this point upstream iozone 3.446 likely gets warnings for %lld vs. long for LP64 FreeBSD architectures but always with the layouts under FreeBSD always actually matching, no size mismatches. This is unlike the %ld changes that were recently made. w.schwarzenfeld@utanet.at was targeting avoiding most of the %lld vs. long warnings for LP64 as well.] I will note that there is work upstream to handle new architectures, such as covering a NVIDIA GPU based port of iozone in the official iozone. As the upstream author put it: "So, more changes are on the way...".
Ok, thanks for catching me up. I'm fine if someone else wants to maintain this port, and I'm also fine with helping this work on multiple architectures. I see there is yet another upstream version as of yesterday, so let me give that a shot and see what happens. I'll also try to get some other archers running in qemu.
I never has the intention to maintain this port. I also don't wanted to make the patch in 211152. Cause as Kubilay ask if I provide a patch I tried it. My intention was to react, cause I know from the past it seems nobody will look in iozone PR's. In the past I made two patches for iozone. The first one was easy. The second one worked, but has a lot of warnings. Someone other helped me and made the libasync.c and pitserver.c patch. With help from Mark, I think we made a good solution, except two things. I overlooked two hunks in the pitserver-patch (you provided in your patch) and the sscanf was left. Now, I think it will be better to split the port. Make one for FreeBSD, Dragonfly and all systems you can find in the definitions of iozone.c. And one other for other platforms. I think it will be clearer for some typedefs and defintions in the program.
Or we make it simple and say ONLY_FOR_ARCHS=... or NOT_FOR_ARCHS=....
(In reply to Josh Paetzel from comment #6) Updating to 3.446 sounds good. I do not know if you will want to cut down on the number of warnings or not. If you did want to . . . 3.446 could still use updates for %lld usage such as: - sprintf(splash[splash_line++],"\tFile size set to %lld kB\n",kilobytes64); + sprintf(splash[splash_line++],"\tFile size set to %lld kB\n",(long long)kilobytes64); (make the change via a cast instead of to the %lld format). This would get rid of unnecessary sprintf warnings in a way that deals with both long long (ILP64 FreeBSD) vs. long (LP64 FreeBSD) for signed 63-bit precision (64-bits total) in a way that should be good outside FreeBSD and so be acceptable upstream. It is LP64 (for example amd64) that would produce the warnings without such changes: long would be in use for 64-bit. ILP32 will not produce the warnings either way (long long is in use for 64-bit). If the printf's that produce warnings are taken care of via such changes that would leave a couple of sscanf's that produce warnings for %lld use. That number of warnings may be more acceptable. But an option would be code like (ignore formatting details here): {long long tmp; sscanf(optarg,"%lld",&tmp); kilobytes64 = tmp; } This would allow layout variations for LP64 FreeBSD when variables like kilobytes64 are long instead of long long: it leaves any conversion for the final storage update explicitly to the compiler. It would apply to other (non-FreeBSD) off64_t contexts as well. Another way to deal with the 2 %lld sscanf's is code changes such as: - sscanf(optarg,"%lld",&kilobytes64); + sscanf(optarg,"%lld",(long long*)&kilobytes64); This presumes the storage layout are the same for long and long long for LP64 FreeBSD (and other off64_t contexts): no potential storage layout conversion by the compiler.
So trying to update to the newest version blows your patches up, which I can rejigger, however, have you seen the iozone license? I'm not entirely sure it allows us to patch the source code and still call it iozone. Did you get permission for those patches? This is turning in to a bit of a hassle, but that's fine. I'm going to reach out to the iozone maintainers and see if I can talk to them. Can anyone send me an intro email to them?
3.449 is showing following errors: Build iozone for FreeBSD /usr/local/libexec/ccache/world/cc -c -O2 -pipe -fstack-protector -fno-strict-aliasing -DFreeBSD -Dunix -Dbsd4_4 -DHAVE_ANSIC_C -DASYNC_IO -DHAVE_PREAD -DNAME='"freebsd"' -DSHARED_MEM -O2 -pipe -fstack-protector -fno-strict-aliasing iozone.c -o iozone_freebsd.o iozone.c:997:30: error: function cannot return function type '__sighandler_t' (aka 'void (int)') __sighandler_t signal_handler(void(int)); /* clean up if user interrupts us */ ^ iozone.c:1865:28: error: use of undeclared identifier 'sighandler_t'; did you mean '__sighandler_t'? signal((int) SIGINT, (sighandler_t) signal_handler); /* handle user interrupt */ ^ /usr/include/sys/signal.h:142:14: note: '__sighandler_t' declared here typedef void __sighandler_t(int); ^ iozone.c:1866:29: error: use of undeclared identifier 'sighandler_t'; did you mean '__sighandler_t'? signal((int) SIGTERM, (sighandler_t) signal_handler); /* handle kill from shell */ ^ /usr/include/sys/signal.h:142:14: note: '__sighandler_t' declared here typedef void __sighandler_t(int); ^ iozone.c:3595:1: error: unknown type name 'sighandler_t'; did you mean '__sighandler_t'? sighandler_t signal_handler(int) ^~~~~~~~~~~~ __sighandler_t /usr/include/sys/signal.h:142:14: note: '__sighandler_t' declared here typedef void __sighandler_t(int); ^ iozone.c:3595:28: error: function cannot return function type '__sighandler_t' (aka 'void (int)') sighandler_t signal_handler(int) ^ iozone.c:3595:32: error: parameter name omitted sighandler_t signal_handler(int) I sendt a mail to iozone.org.
Will be fixed in version 3.452 coming in the next 24 hours.
(In reply to w.schwarzenfeld from comment #12) Don C. sent the updated source and it will not work. He has more to do if it is to work. I wrote to him: __sighandler_t is from the implementation namespace and is not required to exist with that spelling. Nor is its definition standardized. In Mac OS X 10.11.6 with the command line tools downloaded and installed (that creates and fills in /usr/include ) I find: #if !defined(_POSIX_C_SOURCE) || defined(_DARWIN_C_SOURCE) typedef void (*sig_t)(int); /* type of signal function */ #endif in sys/signal.h . Note that sig_t has the pointer to the function in the type. I do not find sighandler_t or __sighandler_t anywhere in Mac OS X's /usr/include . In fact: markmi$ find /usr/include -type f -exec grep sigh {} \; -print | more /* Define to 1 if you have the `sighold' function. */ /usr/include/net-snmp/net-snmp-config.h /* [sigh] some implementations return the "illegal" op for unsupported ops */ \ /usr/include/nfs/nfsm_subs.h int sighold(int); /usr/include/signal.h So no variation of "sighandler" at all. I do not have OpenBSD, DragonFly, or NetBSD available. As for FreeBSD it does have a __sighandler_t but it does not have the pointer to a function. Instead sig_t that also involves the pointer: # find /usr/include/ -type f -exec grep sigh {} \; -print | more __sighandler_t *sf_handler; __sighandler_t *sf_handler; /usr/include/x86/sigframe.h /* TODO: Handle Linux stat32/stat64 ugliness. <sigh> */ /usr/include/archive.h #define SIG_DFL ((__sighandler_t *)0) #define SIG_IGN ((__sighandler_t *)1) #define SIG_ERR ((__sighandler_t *)-1) /* #define SIG_CATCH ((__sighandler_t *)2) See signalvar.h */ #define SIG_HOLD ((__sighandler_t *)3) * programs can avoid the warnings by casting to (__sighandler_t *) or typedef void __sighandler_t(int); typedef __sighandler_t *sig_t; /* type of pointer to a signal function */ __sighandler_t *sv_handler; /* signal handler */ __sighandler_t *signal(int, __sighandler_t *); /usr/include/sys/signal.h int signalcontext(ucontext_t *, int, __sighandler_t *); /usr/include/sys/ucontext.h #define SIG_CATCH ((__sighandler_t *)2) /* #define SIG_HOLD ((__sighandler_t *)3) See signal.h */ /usr/include/sys/signalvar.h int sighold(int); /usr/include/signal.h The easiest thing may be to define: typedef void (*my_sig_t)(int); /* type of signal function */ and use my_sig_t. This would avoid all the varied ways of having a typedef that was never standardized. It also avoids use of C/system implementation-space names that are subject to change without notification.
Today 3,457 is out. _sighandler_t is fixed. The warnings seems all fixed. (Now is no additional patch needed, patch-pitserver.c is obsolete). I am not sure if ALL things Mark critisized are fixed, but it ssems so.
3.457 compiles with no patches and no warnings on HEAD amd64 and i386. I'm going to commit that.
A commit references this bug: Author: jpaetzel Date: Thu Jul 28 14:40:45 UTC 2016 New revision: 419222 URL: https://svnweb.freebsd.org/changeset/ports/419222 Log: Update to latest version Incorporates port patches Fixes type warnings on 32 bit and 64 bit with clang Should fix iozone on some of the more "esoteric" arches PR: 211289 Sponsored by: iXsystems Changes: head/benchmarks/iozone/Makefile head/benchmarks/iozone/distinfo head/benchmarks/iozone/files/
Mark can you/we close this too? Or is anything still open?
(In reply to commit-hook from comment #16) I updated to -r419226 for /usr/ports on the armv6 (rpi2 cortext-a7 actually) in a 11.0-BETA2 context. The rebuild got no warnings. An "iozone -a" ran to completion. Lots of figures make sense only based on in-memory caching, including all the read-related KBytes/sec figures. Other than "record rewrite" for <= 64K reclen: the write figures for large files fit with the expected rpi2 USB 2.0 interface speed (given the fast SSD in use). The maximum file size may well not have been large enough to use up the RAM so this is not unexpected. Side note: For FreeBSD the "Compiled for 32 bit mode" iozone output notice is always generated does not indicate either: A) file system position-addressing/size handling nor B) LP64 vs. ILP32 processor/FreeBSD architecture. For (A) FreeBSD is always 64 bit (signed). For (B) it matches the host it was built on: the iozone source auto adjusts based on __LP64__ and/or _LP64 vs. neither being defined. The same is true if the port had used freebsd64 instead of freebsd as the target, although it would then always say "Compiled for 64 bit mode".
(In reply to w.schwarzenfeld from comment #17) I've closed it after experimenting on the rpi2, per comment 18 that was being composed while you sent comment 17.
(In reply to Mark Millard from comment #18) I probably should have noted that I use noatime on the mount of the SSD in /etc/fstab .