[PATCH 0/2] cross-job overlap patches

[PATCH 0/2] cross-job overlap patches

[vincentfu]

Jens, please consider these two patches related to cross-job overlap checking:

- Better document how locking is used in cross-job overlap checking
- Force threads when checking overlap across jobs in offload mode

[PATCH 1/2] serialize_overlap: document locking for cross-job overlap checking

[vincentfu]

From: Vincent Fu <vincent.fu@wdc.com>

Add some comments to clarify how locking is used for cross-job overlap
checking
---
 backend.c     | 5 +++++
 ioengines.c   | 7 +++++++
 rate-submit.c | 8 ++++++++
 3 files changed, 20 insertions(+)

diff --git a/backend.c b/backend.c
index cc3c4e78..2a05abb7 100644
--- a/backend.c
+++ b/backend.c
@@ -1874,6 +1874,11 @@ static void *thread_main(void *data)
 			 "perhaps try --debug=io option for details?\n",
 			 td->o.name, td->io_ops->name);
 
+	/*
+	 * Acquire this lock if we were doing overlap checking in
+	 * offload mode so that we don't clean up this job while
+	 * another thread is checking its io_u's for overlap
+	 */
 	if (td->o.serialize_overlap && td->o.io_submit_mode == IO_MODE_OFFLOAD)
 		pthread_mutex_lock(&overlap_check);
 	td_set_runstate(td, TD_FINISHING);
diff --git a/ioengines.c b/ioengines.c
index 47f606a7..fca1f0ed 100644
--- a/ioengines.c
+++ b/ioengines.c
@@ -288,6 +288,13 @@ enum fio_q_status td_io_queue(struct thread_data *td, struct io_u *io_u)
 
 	assert((io_u->flags & IO_U_F_FLIGHT) == 0);
 	io_u_set(td, io_u, IO_U_F_FLIGHT);
+
+	/*
+	 * If overlap checking was enabled in offload mode we
+	 * can release this lock that was acquired when we
+	 * started the overlap check because the IO_U_F_FLIGHT
+	 * flag is now set
+	 */
 	if (td->o.serialize_overlap && td->o.io_submit_mode == IO_MODE_OFFLOAD)
 		pthread_mutex_unlock(&overlap_check);
 
diff --git a/rate-submit.c b/rate-submit.c
index 68ad755d..e5c62043 100644
--- a/rate-submit.c
+++ b/rate-submit.c
@@ -21,6 +21,14 @@ static void check_overlap(struct io_u *io_u)
 		 * time to prevent two threads from thinking the coast
 		 * is clear and then submitting IOs that overlap with
 		 * each other
+		 *
+		 * If an overlap is found, release the lock and
+		 * re-acquire it before checking again to give other
+		 * threads a chance to make progress
+		 *
+		 * If an overlap is not found, release the lock when the
+		 * io_u's IO_U_F_FLIGHT flag is set so that this io_u
+		 * can be checked by other threads as they assess overlap
 		 */
 		pthread_mutex_lock(&overlap_check);
 		for_each_td(td, i) {
-- 
2.17.1

[PATCH 2/2] init: force threads when checking overlap in offload mode

[vincentfu]

From: Vincent Fu <vincent.fu@wdc.com>

serialize_overlap combined with io_submit_mode=offload requires
threads. Print a warning and force the use of threads if the user
did not specify threads.
---
 init.c | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/init.c b/init.c
index a2b70c4a..224eb8ab 100644
--- a/init.c
+++ b/init.c
@@ -751,6 +751,14 @@ static int fixup_options(struct thread_data *td)
 	    o->io_submit_mode != IO_MODE_OFFLOAD)
 		o->serialize_overlap = 0;
 
+	if (o->serialize_overlap && o->io_submit_mode == IO_MODE_OFFLOAD && !o->use_thread) {
+		o->use_thread = 1;
+		log_info("fio: threads must be used when overlap checking is"
+			 " enabled in offload mode. Use the 'thread' option"
+			 " to get rid of this warning.\n");
+		ret |= warnings_fatal;
+	}
+
 	if (o->nr_files > td->files_index)
 		o->nr_files = td->files_index;
 
-- 
2.17.1

Re: [PATCH 2/2] init: force threads when checking overlap in offload mode

[Jens Axboe]

On 10/16/18 9:26 AM, vincentfu@gmail.com wrote:
> From: Vincent Fu <vincent.fu@wdc.com>
> 
> serialize_overlap combined with io_submit_mode=offload requires
> threads. Print a warning and force the use of threads if the user
> did not specify threads.

Why does it require threads? Is it because of the use of a pthread
mutex? If so, that should just be a fio mutex instead, and you would
not have this restrictions (on most platforms, at least).

-- 
Jens Axboe

Re: [PATCH 2/2] init: force threads when checking overlap in offload mode

[Vincent Fu]

On 10/16/2018 11:29 AM, Jens Axboe wrote:
> On 10/16/18 9:26 AM, vincentfu@gmail.com wrote:
>> From: Vincent Fu <vincent.fu@wdc.com>
>>
>> serialize_overlap combined with io_submit_mode=offload requires
>> threads. Print a warning and force the use of threads if the user
>> did not specify threads.
> Why does it require threads? Is it because of the use of a pthread
> mutex? If so, that should just be a fio mutex instead, and you would
> not have this restrictions (on most platforms, at least).
>
It requires threads because io_u's are not allocated from the shared
memory area. Jobs need to access each other's io_u's to check for overlap.

Re: [PATCH 2/2] init: force threads when checking overlap in offload mode

[Jens Axboe]

On 10/16/18 9:33 AM, Vincent Fu wrote:
> On 10/16/2018 11:29 AM, Jens Axboe wrote:
>> On 10/16/18 9:26 AM, vincentfu@gmail.com wrote:
>>> From: Vincent Fu <vincent.fu@wdc.com>
>>>
>>> serialize_overlap combined with io_submit_mode=offload requires
>>> threads. Print a warning and force the use of threads if the user
>>> did not specify threads.
>> Why does it require threads? Is it because of the use of a pthread
>> mutex? If so, that should just be a fio mutex instead, and you would
>> not have this restrictions (on most platforms, at least).
>>
> It requires threads because io_u's are not allocated from the shared
> memory area. Jobs need to access each other's io_u's to check for overlap.

Gotcha. Might be better to change that rather than impose limitations
on needing threads for this specific configuration.

Probably make it dependent on offload and serialize_overlap for now,
it'd be less risky. The risk here being that higher queue depth
jobs running out of memory in the smalloc pool.

-- 
Jens Axboe