All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v2 0/7]
@ 2017-02-14 15:19 kusumi.tomohiro
  2017-02-14 15:19 ` [PATCH v2 1/7] configure: Drop default CONFIG_LITTLE_ENDIAN for Cygwin kusumi.tomohiro
                   ` (7 more replies)
  0 siblings, 8 replies; 15+ messages in thread
From: kusumi.tomohiro @ 2017-02-14 15:19 UTC (permalink / raw)
  To: axboe, fio; +Cc: Tomohiro Kusumi

From: Tomohiro Kusumi <tkusumi@tuxera.com>

configure: Use x86 instead of i386 for $cpu for IA32
  Added "This change should have been a part of e12f4ede in 2016.".

Acquire file ->lock while the lock itself is being copied
  Rewrote the locking using (un)lock_file().

Tomohiro Kusumi (7):
  configure: Drop default CONFIG_LITTLE_ENDIAN for Cygwin
  configure: Use x86 instead of i386 for $cpu for IA32
  Drop conditional declaration of disk_list
  Always set ->real_file_size to -1 when failed to get file size
  Add missing "rand"/"trimwrite" strings to corresponding ddir slots
  Acquire file ->lock while the lock itself is being copied
  Use 0 instead of DDIR_READ to iterate from 0 to DDIR_RWDIR_CNT

 configure        |  3 +--
 diskutil.c       |  2 --
 engines/falloc.c |  2 +-
 eta.c            |  6 +++---
 file.h           |  2 +-
 filesetup.c      | 18 +++++++++++-------
 io_ddir.h        |  4 ++--
 io_u.c           |  6 +++---
 libfio.c         |  5 -----
 stat.c           |  4 ++--
 steadystate.c    |  2 +-
 11 files changed, 25 insertions(+), 29 deletions(-)

-- 
2.9.3



^ permalink raw reply	[flat|nested] 15+ messages in thread

* [PATCH v2 1/7] configure: Drop default CONFIG_LITTLE_ENDIAN for Cygwin
  2017-02-14 15:19 [PATCH v2 0/7] kusumi.tomohiro
@ 2017-02-14 15:19 ` kusumi.tomohiro
  2017-02-14 15:19 ` [PATCH v2 2/7] configure: Use x86 instead of i386 for $cpu for IA32 kusumi.tomohiro
                   ` (6 subsequent siblings)
  7 siblings, 0 replies; 15+ messages in thread
From: kusumi.tomohiro @ 2017-02-14 15:19 UTC (permalink / raw)
  To: axboe, fio; +Cc: Tomohiro Kusumi

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



^ permalink raw reply related	[flat|nested] 15+ messages in thread

* [PATCH v2 2/7] configure: Use x86 instead of i386 for $cpu for IA32
  2017-02-14 15:19 [PATCH v2 0/7] kusumi.tomohiro
  2017-02-14 15:19 ` [PATCH v2 1/7] configure: Drop default CONFIG_LITTLE_ENDIAN for Cygwin kusumi.tomohiro
@ 2017-02-14 15:19 ` kusumi.tomohiro
  2017-02-14 15:19 ` [PATCH v2 3/7] Drop conditional declaration of disk_list kusumi.tomohiro
                   ` (5 subsequent siblings)
  7 siblings, 0 replies; 15+ messages in thread
From: kusumi.tomohiro @ 2017-02-14 15:19 UTC (permalink / raw)
  To: axboe, fio; +Cc: Tomohiro Kusumi

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.

This change should have been a part of e12f4ede in 2016.

 # 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



^ permalink raw reply related	[flat|nested] 15+ messages in thread

* [PATCH v2 3/7] Drop conditional declaration of disk_list
  2017-02-14 15:19 [PATCH v2 0/7] kusumi.tomohiro
  2017-02-14 15:19 ` [PATCH v2 1/7] configure: Drop default CONFIG_LITTLE_ENDIAN for Cygwin kusumi.tomohiro
  2017-02-14 15:19 ` [PATCH v2 2/7] configure: Use x86 instead of i386 for $cpu for IA32 kusumi.tomohiro
@ 2017-02-14 15:19 ` kusumi.tomohiro
  2017-02-14 15:19 ` [PATCH v2 4/7] Always set ->real_file_size to -1 when failed to get file size kusumi.tomohiro
                   ` (4 subsequent siblings)
  7 siblings, 0 replies; 15+ messages in thread
From: kusumi.tomohiro @ 2017-02-14 15:19 UTC (permalink / raw)
  To: axboe, fio; +Cc: Tomohiro Kusumi

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



^ permalink raw reply related	[flat|nested] 15+ messages in thread

* [PATCH v2 4/7] Always set ->real_file_size to -1 when failed to get file size
  2017-02-14 15:19 [PATCH v2 0/7] kusumi.tomohiro
                   ` (2 preceding siblings ...)
  2017-02-14 15:19 ` [PATCH v2 3/7] Drop conditional declaration of disk_list kusumi.tomohiro
@ 2017-02-14 15:19 ` kusumi.tomohiro
  2017-02-14 15:19 ` [PATCH v2 5/7] Add missing "rand"/"trimwrite" strings to corresponding ddir slots kusumi.tomohiro
                   ` (3 subsequent siblings)
  7 siblings, 0 replies; 15+ messages in thread
From: kusumi.tomohiro @ 2017-02-14 15:19 UTC (permalink / raw)
  To: axboe, fio; +Cc: Tomohiro Kusumi

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



^ permalink raw reply related	[flat|nested] 15+ messages in thread

* [PATCH v2 5/7] Add missing "rand"/"trimwrite" strings to corresponding ddir slots
  2017-02-14 15:19 [PATCH v2 0/7] kusumi.tomohiro
                   ` (3 preceding siblings ...)
  2017-02-14 15:19 ` [PATCH v2 4/7] Always set ->real_file_size to -1 when failed to get file size kusumi.tomohiro
@ 2017-02-14 15:19 ` kusumi.tomohiro
  2017-02-14 15:19 ` [PATCH v2 6/7] Acquire file ->lock while the lock itself is being copied kusumi.tomohiro
                   ` (2 subsequent siblings)
  7 siblings, 0 replies; 15+ messages in thread
From: kusumi.tomohiro @ 2017-02-14 15:19 UTC (permalink / raw)
  To: axboe, fio; +Cc: Tomohiro Kusumi

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



^ permalink raw reply related	[flat|nested] 15+ messages in thread

* [PATCH v2 6/7] Acquire file ->lock while the lock itself is being copied
  2017-02-14 15:19 [PATCH v2 0/7] kusumi.tomohiro
                   ` (4 preceding siblings ...)
  2017-02-14 15:19 ` [PATCH v2 5/7] Add missing "rand"/"trimwrite" strings to corresponding ddir slots kusumi.tomohiro
@ 2017-02-14 15:19 ` kusumi.tomohiro
  2017-02-14 15:23   ` Jens Axboe
  2017-02-14 15:19 ` [PATCH v2 7/7] Use 0 instead of DDIR_READ to iterate from 0 to DDIR_RWDIR_CNT kusumi.tomohiro
  2017-02-14 15:25 ` [PATCH v2 0/7] Jens Axboe
  7 siblings, 1 reply; 15+ messages in thread
From: kusumi.tomohiro @ 2017-02-14 15:19 UTC (permalink / raw)
  To: axboe, fio; +Cc: Tomohiro Kusumi

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>
---
 engines/falloc.c |  2 +-
 file.h           |  2 +-
 filesetup.c      | 12 +++++++-----
 3 files changed, 9 insertions(+), 7 deletions(-)

diff --git a/engines/falloc.c b/engines/falloc.c
index 2b00d52..9dff9bf 100644
--- a/engines/falloc.c
+++ b/engines/falloc.c
@@ -39,7 +39,7 @@ static int open_file(struct thread_data *td, struct fio_file *f)
 	}
 
 open_again:
-	from_hash = file_lookup_open(f, O_CREAT|O_RDWR);
+	from_hash = file_lookup_open(td, f, O_CREAT|O_RDWR);
 
 	if (f->fd == -1) {
 		char buf[FIO_VERROR_SIZE];
diff --git a/file.h b/file.h
index ac00ff8..b9bf75f 100644
--- a/file.h
+++ b/file.h
@@ -192,7 +192,7 @@ extern int __must_check file_invalidate_cache(struct thread_data *, struct fio_f
 extern int __must_check generic_open_file(struct thread_data *, struct fio_file *);
 extern int __must_check generic_close_file(struct thread_data *, struct fio_file *);
 extern int __must_check generic_get_file_size(struct thread_data *, struct fio_file *);
-extern int __must_check file_lookup_open(struct fio_file *f, int flags);
+extern int __must_check file_lookup_open(struct thread_data *, struct fio_file *, int flags);
 extern int __must_check pre_read_files(struct thread_data *);
 extern unsigned long long get_rand_file_size(struct thread_data *td);
 extern int add_file(struct thread_data *, const char *, int, int);
diff --git a/filesetup.c b/filesetup.c
index 3fa8b32..73fc7e6 100644
--- a/filesetup.c
+++ b/filesetup.c
@@ -492,7 +492,7 @@ int generic_close_file(struct thread_data fio_unused *td, struct fio_file *f)
 	return ret;
 }
 
-int file_lookup_open(struct fio_file *f, int flags)
+int file_lookup_open(struct thread_data *td, struct fio_file *f, int flags)
 {
 	struct fio_file *__f;
 	int from_hash;
@@ -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
+		 * acquire file lock while it gets copied to f.
 		 */
+		lock_file(td, __f, td_read(td) ? DDIR_READ : DDIR_WRITE);
 		f->lock = __f->lock;
+		unlock_file(td, __f);
 		from_hash = 1;
 	} else {
 		dprint(FD_FILE, "file not found in hash %s\n", f->file_name);
@@ -588,7 +590,7 @@ open_again:
 		if (is_std)
 			f->fd = dup(STDOUT_FILENO);
 		else
-			from_hash = file_lookup_open(f, flags);
+			from_hash = file_lookup_open(td, f, flags);
 	} else if (td_read(td)) {
 		if (f->filetype == FIO_TYPE_CHAR && !read_only)
 			flags |= O_RDWR;
@@ -598,10 +600,10 @@ open_again:
 		if (is_std)
 			f->fd = dup(STDIN_FILENO);
 		else
-			from_hash = file_lookup_open(f, flags);
+			from_hash = file_lookup_open(td, f, flags);
 	} else { //td trim
 		flags |= O_RDWR;
-		from_hash = file_lookup_open(f, flags);
+		from_hash = file_lookup_open(td, f, flags);
 	}
 
 	if (f->fd == -1) {
-- 
2.9.3



^ permalink raw reply related	[flat|nested] 15+ messages in thread

* [PATCH v2 7/7] Use 0 instead of DDIR_READ to iterate from 0 to DDIR_RWDIR_CNT
  2017-02-14 15:19 [PATCH v2 0/7] kusumi.tomohiro
                   ` (5 preceding siblings ...)
  2017-02-14 15:19 ` [PATCH v2 6/7] Acquire file ->lock while the lock itself is being copied kusumi.tomohiro
@ 2017-02-14 15:19 ` kusumi.tomohiro
  2017-02-14 15:25 ` [PATCH v2 0/7] Jens Axboe
  7 siblings, 0 replies; 15+ messages in thread
From: kusumi.tomohiro @ 2017-02-14 15:19 UTC (permalink / raw)
  To: axboe, fio; +Cc: Tomohiro Kusumi

From: Tomohiro Kusumi <tkusumi@tuxera.com>

I think this is better since we want to count up DDIR_RWDIR_CNT,
but not specifically (count - something) even if something is 0,
and that's basically what DDIR_RWDIR_CNT is there for to equal
SYNC=3 after READ/WRITE/TRIM in io_ddir.h.

Signed-off-by: Tomohiro Kusumi <tkusumi@tuxera.com>
---
 eta.c         | 6 +++---
 io_u.c        | 6 +++---
 stat.c        | 4 ++--
 steadystate.c | 2 +-
 4 files changed, 9 insertions(+), 9 deletions(-)

diff --git a/eta.c b/eta.c
index 1d66163..adf7f94 100644
--- a/eta.c
+++ b/eta.c
@@ -440,7 +440,7 @@ bool calc_thread_status(struct jobs_eta *je, int force)
 		if (td->runstate > TD_SETTING_UP) {
 			int ddir;
 
-			for (ddir = DDIR_READ; ddir < DDIR_RWDIR_CNT; ddir++) {
+			for (ddir = 0; ddir < DDIR_RWDIR_CNT; ddir++) {
 				if (unified_rw_rep) {
 					io_bytes[0] += td->io_bytes[ddir];
 					io_iops[0] += td->io_blocks[ddir];
@@ -574,7 +574,7 @@ void display_thread_status(struct jobs_eta *je)
 			sprintf(perc_str, "%3.1f%%", perc);
 		}
 
-		for (ddir = DDIR_READ; ddir < DDIR_RWDIR_CNT; ddir++) {
+		for (ddir = 0; ddir < DDIR_RWDIR_CNT; ddir++) {
 			rate_str[ddir] = num2str(je->rate[ddir], 4,
 						1024, je->is_pow2, je->unit_base);
 			iops_str[ddir] = num2str(je->iops[ddir], 4, 1, 0, N2S_NONE);
@@ -601,7 +601,7 @@ void display_thread_status(struct jobs_eta *je)
 			p += sprintf(p, "%*s", linelen_last - l, "");
 		linelen_last = l;
 
-		for (ddir = DDIR_READ; ddir < DDIR_RWDIR_CNT; ddir++) {
+		for (ddir = 0; ddir < DDIR_RWDIR_CNT; ddir++) {
 			free(rate_str[ddir]);
 			free(iops_str[ddir]);
 		}
diff --git a/io_u.c b/io_u.c
index 1daaf7b..f1a3916 100644
--- a/io_u.c
+++ b/io_u.c
@@ -1902,7 +1902,7 @@ static void init_icd(struct thread_data *td, struct io_completion_data *icd,
 	icd->nr = nr;
 
 	icd->error = 0;
-	for (ddir = DDIR_READ; ddir < DDIR_RWDIR_CNT; ddir++)
+	for (ddir = 0; ddir < DDIR_RWDIR_CNT; ddir++)
 		icd->bytes_done[ddir] = 0;
 }
 
@@ -1941,7 +1941,7 @@ int io_u_sync_complete(struct thread_data *td, struct io_u *io_u)
 		return -1;
 	}
 
-	for (ddir = DDIR_READ; ddir < DDIR_RWDIR_CNT; ddir++)
+	for (ddir = 0; ddir < DDIR_RWDIR_CNT; ddir++)
 		td->bytes_done[ddir] += icd.bytes_done[ddir];
 
 	return 0;
@@ -1980,7 +1980,7 @@ int io_u_queued_complete(struct thread_data *td, int min_evts)
 		return -1;
 	}
 
-	for (ddir = DDIR_READ; ddir < DDIR_RWDIR_CNT; ddir++)
+	for (ddir = 0; ddir < DDIR_RWDIR_CNT; ddir++)
 		td->bytes_done[ddir] += icd.bytes_done[ddir];
 
 	return ret;
diff --git a/stat.c b/stat.c
index f1d468c..0bb21d0 100644
--- a/stat.c
+++ b/stat.c
@@ -2442,7 +2442,7 @@ static int add_bw_samples(struct thread_data *td, struct timeval *t)
 	/*
 	 * Compute both read and write rates for the interval.
 	 */
-	for (ddir = DDIR_READ; ddir < DDIR_RWDIR_CNT; ddir++) {
+	for (ddir = 0; ddir < DDIR_RWDIR_CNT; ddir++) {
 		uint64_t delta;
 
 		delta = td->this_io_bytes[ddir] - td->stat_io_bytes[ddir];
@@ -2517,7 +2517,7 @@ static int add_iops_samples(struct thread_data *td, struct timeval *t)
 	/*
 	 * Compute both read and write rates for the interval.
 	 */
-	for (ddir = DDIR_READ; ddir < DDIR_RWDIR_CNT; ddir++) {
+	for (ddir = 0; ddir < DDIR_RWDIR_CNT; ddir++) {
 		uint64_t delta;
 
 		delta = td->this_io_blocks[ddir] - td->stat_io_blocks[ddir];
diff --git a/steadystate.c b/steadystate.c
index 43c715c..98f027c 100644
--- a/steadystate.c
+++ b/steadystate.c
@@ -231,7 +231,7 @@ void steadystate_check(void)
 		}
 
 		td_io_u_lock(td);
-		for (ddir = DDIR_READ; ddir < DDIR_RWDIR_CNT; ddir++) {
+		for (ddir = 0; ddir < DDIR_RWDIR_CNT; ddir++) {
 			td_iops += td->io_blocks[ddir];
 			td_bytes += td->io_bytes[ddir];
 		}
-- 
2.9.3



^ permalink raw reply related	[flat|nested] 15+ messages in thread

* Re: [PATCH v2 6/7] Acquire file ->lock while the lock itself is being copied
  2017-02-14 15:19 ` [PATCH v2 6/7] Acquire file ->lock while the lock itself is being copied kusumi.tomohiro
@ 2017-02-14 15:23   ` Jens Axboe
  2017-02-14 15:27     ` Tomohiro Kusumi
  0 siblings, 1 reply; 15+ messages in thread
From: Jens Axboe @ 2017-02-14 15:23 UTC (permalink / raw)
  To: kusumi.tomohiro; +Cc: fio, Tomohiro Kusumi

On Tue, Feb 14 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.

I've been thinking about this a little bit, since I could not see what
the race was. Do you spot one, or are you just acting on the comment?
I think the original race was that we copied over the lock, lock owner,
batch, etc. Hence the race was that if someone else grabbed the lock
while we were doing that copy, the fields weren't in a consistent state.
But now we're just copying the pointer to the lock, so I don't think
there's a race. It's just a stale comment.

-- 
Jens Axboe



^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH v2 0/7]
  2017-02-14 15:19 [PATCH v2 0/7] kusumi.tomohiro
                   ` (6 preceding siblings ...)
  2017-02-14 15:19 ` [PATCH v2 7/7] Use 0 instead of DDIR_READ to iterate from 0 to DDIR_RWDIR_CNT kusumi.tomohiro
@ 2017-02-14 15:25 ` Jens Axboe
  7 siblings, 0 replies; 15+ messages in thread
From: Jens Axboe @ 2017-02-14 15:25 UTC (permalink / raw)
  To: kusumi.tomohiro; +Cc: fio, Tomohiro Kusumi

On Tue, Feb 14 2017, kusumi.tomohiro@gmail.com wrote:
> From: Tomohiro Kusumi <tkusumi@tuxera.com>
> 
> configure: Use x86 instead of i386 for $cpu for IA32
>   Added "This change should have been a part of e12f4ede in 2016.".
> 
> Acquire file ->lock while the lock itself is being copied
>   Rewrote the locking using (un)lock_file().
> 
> Tomohiro Kusumi (7):
>   configure: Drop default CONFIG_LITTLE_ENDIAN for Cygwin
>   configure: Use x86 instead of i386 for $cpu for IA32
>   Drop conditional declaration of disk_list
>   Always set ->real_file_size to -1 when failed to get file size
>   Add missing "rand"/"trimwrite" strings to corresponding ddir slots
>   Acquire file ->lock while the lock itself is being copied
>   Use 0 instead of DDIR_READ to iterate from 0 to DDIR_RWDIR_CNT

Applied 1-5, and 7. I don't think we need 6, but I emailed on that
separately. Thanks!

-- 
Jens Axboe



^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH v2 6/7] Acquire file ->lock while the lock itself is being copied
  2017-02-14 15:23   ` Jens Axboe
@ 2017-02-14 15:27     ` Tomohiro Kusumi
  2017-02-14 15:33       ` Jens Axboe
  0 siblings, 1 reply; 15+ messages in thread
From: Tomohiro Kusumi @ 2017-02-14 15:27 UTC (permalink / raw)
  To: Jens Axboe; +Cc: fio, Tomohiro Kusumi

I couldn't see a race too, but assumed there is since you (the author) say so.
For the other one in dup_files(), I didn't think there's any race as commented.

But yes, if there's no race, please drop this.

2017-02-14 17:23 GMT+02:00 Jens Axboe <axboe@kernel.dk>:
> On Tue, Feb 14 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.
>
> I've been thinking about this a little bit, since I could not see what
> the race was. Do you spot one, or are you just acting on the comment?
> I think the original race was that we copied over the lock, lock owner,
> batch, etc. Hence the race was that if someone else grabbed the lock
> while we were doing that copy, the fields weren't in a consistent state.
> But now we're just copying the pointer to the lock, so I don't think
> there's a race. It's just a stale comment.
>
> --
> Jens Axboe
>


^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH v2 6/7] Acquire file ->lock while the lock itself is being copied
  2017-02-14 15:27     ` Tomohiro Kusumi
@ 2017-02-14 15:33       ` Jens Axboe
  0 siblings, 0 replies; 15+ messages in thread
From: Jens Axboe @ 2017-02-14 15:33 UTC (permalink / raw)
  To: Tomohiro Kusumi; +Cc: fio, Tomohiro Kusumi

On 02/14/2017 08:27 AM, Tomohiro Kusumi wrote:
> I couldn't see a race too, but assumed there is since you (the author) say so.
> For the other one in dup_files(), I didn't think there's any race as commented.
> 
> But yes, if there's no race, please drop this.

OK good, then I'll just drop it. BTW, for reference, this was the
original code:

if (__f) {
        /*
         * racy, need the __f->lock locked
         */
        f->lock = __f->lock;
        f->lock_owner = __f->lock_owner;
        f->lock_batch = __f->lock_batch;
        f->lock_ddir = __f->lock_ddir;
	[...]

where that comment was from.

-- 
Jens Axboe



^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH v2 0/7]
  2021-07-27 21:01 Josef Bacik
@ 2021-09-17 15:06 ` David Sterba
  0 siblings, 0 replies; 15+ messages in thread
From: David Sterba @ 2021-09-17 15:06 UTC (permalink / raw)
  To: Josef Bacik; +Cc: linux-btrfs, kernel-team

On Tue, Jul 27, 2021 at 05:01:12PM -0400, Josef Bacik wrote:
> v1->v2:
> - Rework the first patch as it was wrong because we need it for seed devices.
> - Fix another lockdep splat I uncovered while testing against seed devices to
>   make sure I hadn't broken anything.
> 
> --- Original email ---
> 
> Hello,
> 
> The commit 87579e9b7d8d ("loop: use worker per cgroup instead of kworker")
> enabled the use of workqueues for loopback devices, which brought with it
> lockdep annotations for the workqueues for loopback devices.  This uncovered a
> cascade of lockdep warnings because of how we mess with the block_device while
> under the sb writers lock while doing the device removal.
> 
> The first patch seems innocuous but we have a lockdep_assert_held(&uuid_mutex)
> in one of the helpers, which is why I have it first.  The code should never be
> called which is why it is removed, but I'm removing it specifically to remove
> confusion about the role of the uuid_mutex here.
> 
> The next 4 patches are to resolve the lockdep messages as they occur.  There are
> several issues and I address them one at a time until we're no longer getting
> lockdep warnings.
> 
> The final patch doesn't necessarily have to go in right away, it's just a
> cleanup as I noticed we have a lot of duplicated code between the v1 and v2
> device removal handling.  Thanks,
> 
> Josef
> 
> Josef Bacik (7):
>   btrfs: do not call close_fs_devices in btrfs_rm_device
>   btrfs: do not take the uuid_mutex in btrfs_rm_device
>   btrfs: do not read super look for a device path
>   btrfs: update the bdev time directly when closing
>   btrfs: delay blkdev_put until after the device remove
>   btrfs: unify common code for the v1 and v2 versions of device remove
>   btrfs: do not take the device_list_mutex in clone_fs_devices

I've merged what looked ok and did not have comments. Remaining patches
are 1, 3 and 7. Please have a look and resend, thanks.

^ permalink raw reply	[flat|nested] 15+ messages in thread

* [PATCH v2 0/7]
@ 2021-07-27 21:01 Josef Bacik
  2021-09-17 15:06 ` David Sterba
  0 siblings, 1 reply; 15+ messages in thread
From: Josef Bacik @ 2021-07-27 21:01 UTC (permalink / raw)
  To: linux-btrfs, kernel-team

v1->v2:
- Rework the first patch as it was wrong because we need it for seed devices.
- Fix another lockdep splat I uncovered while testing against seed devices to
  make sure I hadn't broken anything.

--- Original email ---

Hello,

The commit 87579e9b7d8d ("loop: use worker per cgroup instead of kworker")
enabled the use of workqueues for loopback devices, which brought with it
lockdep annotations for the workqueues for loopback devices.  This uncovered a
cascade of lockdep warnings because of how we mess with the block_device while
under the sb writers lock while doing the device removal.

The first patch seems innocuous but we have a lockdep_assert_held(&uuid_mutex)
in one of the helpers, which is why I have it first.  The code should never be
called which is why it is removed, but I'm removing it specifically to remove
confusion about the role of the uuid_mutex here.

The next 4 patches are to resolve the lockdep messages as they occur.  There are
several issues and I address them one at a time until we're no longer getting
lockdep warnings.

The final patch doesn't necessarily have to go in right away, it's just a
cleanup as I noticed we have a lot of duplicated code between the v1 and v2
device removal handling.  Thanks,

Josef

Josef Bacik (7):
  btrfs: do not call close_fs_devices in btrfs_rm_device
  btrfs: do not take the uuid_mutex in btrfs_rm_device
  btrfs: do not read super look for a device path
  btrfs: update the bdev time directly when closing
  btrfs: delay blkdev_put until after the device remove
  btrfs: unify common code for the v1 and v2 versions of device remove
  btrfs: do not take the device_list_mutex in clone_fs_devices

 fs/btrfs/ioctl.c   |  92 +++++++++++++++--------------------
 fs/btrfs/volumes.c | 118 ++++++++++++++++++++++-----------------------
 fs/btrfs/volumes.h |   3 +-
 3 files changed, 101 insertions(+), 112 deletions(-)

-- 
2.26.3


^ permalink raw reply	[flat|nested] 15+ messages in thread

* [PATCH v2 0/7]
@ 2018-11-24 17:53 Sergio Paracuellos
  0 siblings, 0 replies; 15+ messages in thread
From: Sergio Paracuellos @ 2018-11-24 17:53 UTC (permalink / raw)
  To: gregkh; +Cc: neil, driverdev-devel

Previous cleanup series was added to the staging tree without any
testing. After get testing feedback some issues appear and this patch
series should make the driver works properly again.

Previous series are here:
* http://driverdev.linuxdriverproject.org/pipermail/driverdev-devel/2018-November/128200.html

Feedback after testing from Neil Brown is here:
* http://driverdev.linuxdriverproject.org/pipermail/driverdev-devel/2018-November/129031.html

There is one issue with chip revision and reset lines where those
are inverted. I achieve this including some wrappers for checking
the version in driver code and use proper reset_control_* functions.
I checked the 'arch/mips/ralink/reset.c' and think a good way to add
a quirk there but I ended up handling those inside the driver.

Changes in v2:
    - PATCH 7: In commit message: 's/mt7621-pcie/mt7621-pci/g'

Hope this helps.

Best regards,
    Sergio Paracuellos

Sergio Paracuellos (7):
  staging: mt7621-pci: avoid mapping sysctls registers
  staging: mt7621-dts: remove sysctl registers from pcie bindings
  staging: mt7621-pci: dt-bindings: update bindings doc removing sysctls
    registers
  staging: mt7621-pci: fix reset lines for each pcie port
  staging: mt7621-pci: avoid using clk_* operations
  staging: mt7621-dts: remove clocks for pcie bindings
  staging: mt7621-pci: dt-bindings: update bindings doc removing clocks

 drivers/staging/mt7621-dts/mt7621.dtsi        |  5 +-
 .../mt7621-pci/mediatek,mt7621-pci.txt        |  9 +--
 drivers/staging/mt7621-pci/pci-mt7621.c       | 67 +++++++++----------
 3 files changed, 34 insertions(+), 47 deletions(-)

-- 
2.19.1

_______________________________________________
devel mailing list
devel@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

^ permalink raw reply	[flat|nested] 15+ messages in thread

end of thread, other threads:[~2021-09-17 15:07 UTC | newest]

Thread overview: 15+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-02-14 15:19 [PATCH v2 0/7] kusumi.tomohiro
2017-02-14 15:19 ` [PATCH v2 1/7] configure: Drop default CONFIG_LITTLE_ENDIAN for Cygwin kusumi.tomohiro
2017-02-14 15:19 ` [PATCH v2 2/7] configure: Use x86 instead of i386 for $cpu for IA32 kusumi.tomohiro
2017-02-14 15:19 ` [PATCH v2 3/7] Drop conditional declaration of disk_list kusumi.tomohiro
2017-02-14 15:19 ` [PATCH v2 4/7] Always set ->real_file_size to -1 when failed to get file size kusumi.tomohiro
2017-02-14 15:19 ` [PATCH v2 5/7] Add missing "rand"/"trimwrite" strings to corresponding ddir slots kusumi.tomohiro
2017-02-14 15:19 ` [PATCH v2 6/7] Acquire file ->lock while the lock itself is being copied kusumi.tomohiro
2017-02-14 15:23   ` Jens Axboe
2017-02-14 15:27     ` Tomohiro Kusumi
2017-02-14 15:33       ` Jens Axboe
2017-02-14 15:19 ` [PATCH v2 7/7] Use 0 instead of DDIR_READ to iterate from 0 to DDIR_RWDIR_CNT kusumi.tomohiro
2017-02-14 15:25 ` [PATCH v2 0/7] Jens Axboe
2018-11-24 17:53 Sergio Paracuellos
2021-07-27 21:01 Josef Bacik
2021-09-17 15:06 ` David Sterba

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.