[PATCH 1/6] configure: Drop default CONFIG_LITTLE_ENDIAN for Cygwin

[PATCH 1/6] configure: Drop default CONFIG_LITTLE_ENDIAN for Cygwin

[kusumi.tomohiro]

From: Tomohiro Kusumi <tkusumi@tuxera.com>

It's a bit strange that Cygwin is assumed to be le by default when
no other platforms have such assumption even if they only target
a single or certain arch (e.g. non-Android Linux often runs on x86,
DragonFlyBSD only runs on x86, and so on..).

Endianness will be detected by pointer arithmetic anyway, and it's
better to rely on it to handle any sort of newly supported archs
in the future.

Signed-off-by: Tomohiro Kusumi <tkusumi@tuxera.com>
---
 configure | 1 -
 1 file changed, 1 deletion(-)

diff --git a/configure b/configure
index 4f17cf8..cba81af 100755
--- a/configure
+++ b/configure
@@ -306,7 +306,6 @@ CYGWIN*)
       fi
     fi
   fi
-  output_sym "CONFIG_LITTLE_ENDIAN"
   if test ! -z "$build_32bit_win" && test "$build_32bit_win" = "yes"; then
     output_sym "CONFIG_32BIT"
   else
-- 
2.9.3

[PATCH 2/6] configure: Use x86 instead of i386 for $cpu for IA32

[kusumi.tomohiro]

From: Tomohiro Kusumi <tkusumi@tuxera.com>

fio internally uses "x86" for any sort of IA32, so use x86 for $cpu
as well which gets printed on ./configure.

 # grep i386 . -rI
 ./arch/arch.h:#if defined(__i386__)
 ./configure:elif check_define __i386__ ; then
 ./configure:  cpu="i386"
 ./configure:  i386|i486|i586|i686|i86pc|BePC)
 ./configure:    cpu="i386"
 ./crc/sha1.c:#if defined(__i386__) || defined(__x86_64__)
 ./crc/xxhash.c:#if defined(__ARM_FEATURE_UNALIGNED) || defined(__i386) || defined(_M_IX86) || defined(__x86_64__) || defined(_M_X64)

Signed-off-by: Tomohiro Kusumi <tkusumi@tuxera.com>
---
 configure | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/configure b/configure
index cba81af..3f5386f 100755
--- a/configure
+++ b/configure
@@ -377,7 +377,7 @@ case "$cpu" in
     cpu="$cpu"
   ;;
   i386|i486|i586|i686|i86pc|BePC)
-    cpu="i386"
+    cpu="x86"
   ;;
   x86_64|amd64)
     cpu="x86_64"
-- 
2.9.3

RE: [PATCH 2/6] configure: Use x86 instead of i386 for $cpu for IA32

[Elliott, Robert (Persistent Memory)]

> -----Original Message-----
> From: fio-owner@vger.kernel.org [mailto:fio-owner@vger.kernel.org] On
> Behalf Of kusumi.tomohiro@gmail.com
> Sent: Monday, February 13, 2017 10:33 AM
> To: axboe@kernel.dk; fio@vger.kernel.org
> Cc: Tomohiro Kusumi <tkusumi@tuxera.com>
> Subject: [PATCH 2/6] configure: Use x86 instead of i386 for $cpu for IA32
> 
> From: Tomohiro Kusumi <tkusumi@tuxera.com>
> 
> fio internally uses "x86" for any sort of IA32, so use x86 for $cpu
> as well which gets printed on ./configure.

That's the gcc cpu_type, and derives from  the __i386__  macro value
that configure is checking for. All the other architectures stay with
the cpu_type name at this level.

>  # grep i386 . -rI
>  ./arch/arch.h:#if defined(__i386__)
>  ./configure:elif check_define __i386__ ; then
>  ./configure:  cpu="i386"
>  ./configure:  i386|i486|i586|i686|i86pc|BePC)
>  ./configure:    cpu="i386"
>  ./crc/sha1.c:#if defined(__i386__) || defined(__x86_64__)
>  ./crc/xxhash.c:#if defined(__ARM_FEATURE_UNALIGNED) ||
> defined(__i386) || defined(_M_IX86) || defined(__x86_64__) ||
> defined(_M_X64)

That search points out crc/xxhash.c is missing the trailing __ on the
__i386__ macro, which seems like a bug.

The search should have also found this in arch/arch-x86.h:
    #define FIO_ARCH        (arch_i386)

and an enum arch/arch.h:
    enum {
      arch_i386,

I think they would also need to be changed, if the change is made.
FIO_ARCH is used by gfio in the Architecture box.

---
Robert Elliott, HPE Persistent Memory

Re: [PATCH 2/6] configure: Use x86 instead of i386 for $cpu for IA32

[Tomohiro Kusumi]

Hi

2017-02-13 21:24 GMT+02:00 Elliott, Robert (Persistent Memory)
<elliott@hpe.com>:
>
>
>> -----Original Message-----
>> From: fio-owner@vger.kernel.org [mailto:fio-owner@vger.kernel.org] On
>> Behalf Of kusumi.tomohiro@gmail.com
>> Sent: Monday, February 13, 2017 10:33 AM
>> To: axboe@kernel.dk; fio@vger.kernel.org
>> Cc: Tomohiro Kusumi <tkusumi@tuxera.com>
>> Subject: [PATCH 2/6] configure: Use x86 instead of i386 for $cpu for IA32
>>
>> From: Tomohiro Kusumi <tkusumi@tuxera.com>
>>
>> fio internally uses "x86" for any sort of IA32, so use x86 for $cpu
>> as well which gets printed on ./configure.
>
> That's the gcc cpu_type, and derives from  the __i386__  macro value
> that configure is checking for. All the other architectures stay with
> the cpu_type name at this level.
>
>>  # grep i386 . -rI
>>  ./arch/arch.h:#if defined(__i386__)
>>  ./configure:elif check_define __i386__ ; then
>>  ./configure:  cpu="i386"
>>  ./configure:  i386|i486|i586|i686|i86pc|BePC)
>>  ./configure:    cpu="i386"
>>  ./crc/sha1.c:#if defined(__i386__) || defined(__x86_64__)
>>  ./crc/xxhash.c:#if defined(__ARM_FEATURE_UNALIGNED) ||
>> defined(__i386) || defined(_M_IX86) || defined(__x86_64__) ||
>> defined(_M_X64)
>
> That search points out crc/xxhash.c is missing the trailing __ on the
> __i386__ macro, which seems like a bug.
>
> The search should have also found this in arch/arch-x86.h:
>     #define FIO_ARCH        (arch_i386)
>
> and an enum arch/arch.h:
>     enum {
>       arch_i386,


This has been changed to x86 (actually by myself in e12f4ede46 in 2016) also.
What this patch does actually should have been changed at that time.


>
> I think they would also need to be changed, if the change is made.
> FIO_ARCH is used by gfio in the Architecture box.
>
> ---
> Robert Elliott, HPE Persistent Memory
>
>
>
>

[PATCH 3/6] Drop conditional declaration of disk_list

[kusumi.tomohiro]

From: Tomohiro Kusumi <tkusumi@tuxera.com>

disk_list is used unconditionally whether it's Linux or not,
so leave the one in libfio.c, and remove the one in diskutil.c.

Signed-off-by: Tomohiro Kusumi <tkusumi@tuxera.com>
---
 diskutil.c | 2 --
 libfio.c   | 5 -----
 2 files changed, 7 deletions(-)

diff --git a/diskutil.c b/diskutil.c
index c3bcec9..dca3748 100644
--- a/diskutil.c
+++ b/diskutil.c
@@ -18,8 +18,6 @@ static struct disk_util *last_du;
 
 static struct fio_mutex *disk_util_mutex;
 
-FLIST_HEAD(disk_list);
-
 static struct disk_util *__init_per_file_disk_util(struct thread_data *td,
 		int majdev, int mindev, char *path);
 
diff --git a/libfio.c b/libfio.c
index 7e0d32c..4b53c92 100644
--- a/libfio.c
+++ b/libfio.c
@@ -36,12 +36,7 @@
 #include "helper_thread.h"
 #include "filehash.h"
 
-/*
- * Just expose an empty list, if the OS does not support disk util stats
- */
-#ifndef FIO_HAVE_DISK_UTIL
 FLIST_HEAD(disk_list);
-#endif
 
 unsigned long arch_flags = 0;
 
-- 
2.9.3

[PATCH 4/6] Always set ->real_file_size to -1 when failed to get file size

[kusumi.tomohiro]

From: Tomohiro Kusumi <tkusumi@tuxera.com>

In some cases it's set to -1, but on others it's not,
while the existing code (e.g. get_fs_free_counts(), setup_files(), etc)
seem to expect ->real_file_size to be -1 in case of any error.

Signed-off-by: Tomohiro Kusumi <tkusumi@tuxera.com>
---
 filesetup.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/filesetup.c b/filesetup.c
index eb28826..3fa8b32 100644
--- a/filesetup.c
+++ b/filesetup.c
@@ -375,10 +375,12 @@ static int get_file_size(struct thread_data *td, struct fio_file *f)
 	else if (f->filetype == FIO_TYPE_CHAR)
 		ret = char_size(td, f);
 	else
-		f->real_file_size = -1;
+		f->real_file_size = -1ULL;
 
-	if (ret)
+	if (ret) {
+		f->real_file_size = -1ULL;
 		return ret;
+	}
 
 	if (f->file_offset > f->real_file_size) {
 		log_err("%s: offset extends end (%llu > %llu)\n", td->o.name,
-- 
2.9.3

[PATCH 5/6] Acquire file ->lock while the lock itself is being copied

[kusumi.tomohiro]

From: Tomohiro Kusumi <tkusumi@tuxera.com>

to the destination file pointer.
The ones in dup_files() doesn't seem to require locking from
the way it's been called.

Signed-off-by: Tomohiro Kusumi <tkusumi@tuxera.com>
---
 filesetup.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/filesetup.c b/filesetup.c
index 3fa8b32..7a67fe6 100644
--- a/filesetup.c
+++ b/filesetup.c
@@ -501,9 +501,11 @@ int file_lookup_open(struct fio_file *f, int flags)
 	if (__f) {
 		dprint(FD_FILE, "found file in hash %s\n", f->file_name);
 		/*
-		 * racy, need the __f->lock locked
+		 * get __f->lock while it gets copied to f.
 		 */
+		fio_mutex_down(__f->lock);
 		f->lock = __f->lock;
+		fio_mutex_up(__f->lock);
 		from_hash = 1;
 	} else {
 		dprint(FD_FILE, "file not found in hash %s\n", f->file_name);
-- 
2.9.3

Re: [PATCH 5/6] Acquire file ->lock while the lock itself is being copied

[Jens Axboe]

On Mon, Feb 13 2017, kusumi.tomohiro@gmail.com wrote:
> From: Tomohiro Kusumi <tkusumi@tuxera.com>
> 
> to the destination file pointer.
> The ones in dup_files() doesn't seem to require locking from
> the way it's been called.
> 
> Signed-off-by: Tomohiro Kusumi <tkusumi@tuxera.com>
> ---
>  filesetup.c | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
> 
> diff --git a/filesetup.c b/filesetup.c
> index 3fa8b32..7a67fe6 100644
> --- a/filesetup.c
> +++ b/filesetup.c
> @@ -501,9 +501,11 @@ int file_lookup_open(struct fio_file *f, int flags)
>  	if (__f) {
>  		dprint(FD_FILE, "found file in hash %s\n", f->file_name);
>  		/*
> -		 * racy, need the __f->lock locked
> +		 * get __f->lock while it gets copied to f.
>  		 */
> +		fio_mutex_down(__f->lock);
>  		f->lock = __f->lock;
> +		fio_mutex_up(__f->lock);
>  		from_hash = 1;

What if the file lock mode is FILE_LOCK_READWRITE?

-- 
Jens Axboe

Re: [PATCH 5/6] Acquire file ->lock while the lock itself is being copied

[Tomohiro Kusumi]

we simply can't tell the lock mode from the prototype here
   struct fio_file *f, int flags
which needs a bit of work to get the right lock ?

ok, please review the rest of the patches.


2017-02-13 21:49 GMT+02:00 Jens Axboe <axboe@kernel.dk>:
> On Mon, Feb 13 2017, kusumi.tomohiro@gmail.com wrote:
>> From: Tomohiro Kusumi <tkusumi@tuxera.com>
>>
>> to the destination file pointer.
>> The ones in dup_files() doesn't seem to require locking from
>> the way it's been called.
>>
>> Signed-off-by: Tomohiro Kusumi <tkusumi@tuxera.com>
>> ---
>>  filesetup.c | 4 +++-
>>  1 file changed, 3 insertions(+), 1 deletion(-)
>>
>> diff --git a/filesetup.c b/filesetup.c
>> index 3fa8b32..7a67fe6 100644
>> --- a/filesetup.c
>> +++ b/filesetup.c
>> @@ -501,9 +501,11 @@ int file_lookup_open(struct fio_file *f, int flags)
>>       if (__f) {
>>               dprint(FD_FILE, "found file in hash %s\n", f->file_name);
>>               /*
>> -              * racy, need the __f->lock locked
>> +              * get __f->lock while it gets copied to f.
>>                */
>> +             fio_mutex_down(__f->lock);
>>               f->lock = __f->lock;
>> +             fio_mutex_up(__f->lock);
>>               from_hash = 1;
>
> What if the file lock mode is FILE_LOCK_READWRITE?
>
> --
> Jens Axboe
>

[PATCH 6/6] Add missing "rand"/"trimwrite" strings to corresponding ddir slots

[kusumi.tomohiro]

From: Tomohiro Kusumi <tkusumi@tuxera.com>

though "rand" alone is unused.

Signed-off-by: Tomohiro Kusumi <tkusumi@tuxera.com>
---
 io_ddir.h | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/io_ddir.h b/io_ddir.h
index 2141119..613d5fb 100644
--- a/io_ddir.h
+++ b/io_ddir.h
@@ -61,9 +61,9 @@ static inline int ddir_rw(enum fio_ddir ddir)
 
 static inline const char *ddir_str(enum td_ddir ddir)
 {
-	static const char *__str[] = { NULL, "read", "write", "rw", NULL,
+	static const char *__str[] = { NULL, "read", "write", "rw", "rand",
 				"randread", "randwrite", "randrw",
-				"trim", NULL, NULL, NULL, "randtrim" };
+				"trim", NULL, "trimwrite", NULL, "randtrim" };
 
 	return __str[ddir];
 }
-- 
2.9.3