cross-job overlap patches
v2 cover cross-job overlap patches
From: Vincent Fu <[email protected]>
Jens, this series of patches allocates io_u's and io_u_all members from shared memory when serialize_overlap=1 and io_submit_mode=offload so that processes can now be used. Also included are the locking comments you requested and some other cleanups. Please let me know what comments you have.
Vincent Fu (4):
fio: add function to check for serialize_overlap with offload
submission
fio: enable cross-thread overlap checking with processes
fio: document locking for overlap checking in offload mode
docs: serialize_overlap=1 with io_submit_mode=offload no longer
requires threads
HOWTO | 2 +- backend.c | 21 +++++++++++++-------- fio.1 | 2 +- fio.h | 5 +++++ io_u_queue.c | 19 +++++++++++++++---- io_u_queue.h | 4 ++-- ioengines.c | 9 ++++++++- lib/memalign.c | 16 ++++++++++++---- lib/memalign.h | 5 +++-- rate-submit.c | 8 ++++++++ t/dedupe.c | 12 ++++++------ 11 files changed, 74 insertions(+), 29 deletions(-)
Signature · 1 line
2.17.1
PATCH v2 1/4 fio: add function to check for serialize_overlap with offload submission
From: Vincent Fu <[email protected]>
In multiple places fio needs to check whether it is carrying out overlap checking in offload submission mode. Make this check a function to improve code readability.
--- backend.c | 4 ++-- fio.h | 5 +++++ ioengines.c | 2 +- 3 files changed, 8 insertions(+), 3 deletions(-)
backend.c +2 -2
@@ -1874,10 +1874,10 @@ static void *thread_main(void *data)"perhaps try --debug=io option for details?\n" , td->o.name, td->io_ops->name); -if (td->o.serialize_overlap && td->o.io_submit_mode ==IO_MODE_OFFLOAD ) +if (td_offload_overlap (td))pthread_mutex_lock (&overlap_check);td_set_runstate (td,TD_FINISHING ); -if (td->o.serialize_overlap && td->o.io_submit_mode ==IO_MODE_OFFLOAD ) +if (td_offload_overlap (td))pthread_mutex_unlock (&overlap_check);update_rusage_stat (td);
fio.h +5 -0
@@ -772,6 +772,11 @@ static inline bool td_async_processing(struct thread_data *td)return (td->flags &TD_F_NEED_LOCK ) !=0 ; } +static inline bool td_offload_overlap (struct thread_data *td) +{ +return (td->o.serialize_overlap && td->o.io_submit_mode ==IO_MODE_OFFLOAD ); +} +/* * We currently only need to do locking if we have verifier threads * accessing our internal structures too
ioengines.c +1 -1
@@ -288,7 +288,7 @@ 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 (td->o.serialize_overlap && td->o.io_submit_mode ==IO_MODE_OFFLOAD ) +if (td_offload_overlap (td))pthread_mutex_unlock (&overlap_check);assert (fio_file_open (io_u->file));
Signature · 1 line
2.17.1
Re: [PATCH v2 1/4] fio: add function to check for serialize_overlap with offload submission
Quoted 33 lines · On 2018-10-17 10:52 AM, [email protected] wrote:
From: Vincent Fu <[email protected]>
In multiple places fio needs to check whether it is carrying out overlap checking in offload submission mode. Make this check a function to improve code readability.
--- backend.c | 4 ++-- fio.h | 5 +++++ ioengines.c | 2 +- 3 files changed, 8 insertions(+), 3 deletions(-)backend.c +2
-2@@ -1874,10 +1874,10 @@ static void *thread_main(void *data)"perhaps try --debug=io option for details?\n" , td->o.name, td->io_ops->name); -if (td->o.serialize_overlap && td->o.io_submit_mode ==IO_MODE_OFFLOAD ) +if (td_offload_overlap (td))pthread_mutex_lock (&overlap_check);td_set_runstate (td,TD_FINISHING ); -if (td->o.serialize_overlap && td->o.io_submit_mode ==IO_MODE_OFFLOAD ) +if (td_offload_overlap (td))pthread_mutex_unlock (&overlap_check);update_rusage_stat (td);fio.h +0
-0
fio.h static inline bool td_async_processing(struct thread_data *td) · line 777
Quoted 6 lines
fio.h +3
-0@@ -772,6 +772,11 @@ static inline bool td_async_processing(struct thread_data *td)return (td->flags &TD_F_NEED_LOCK ) !=0 ; } +static inline bool td_offload_overlap (struct thread_data *td) +{ +return (td->o.serialize_overlap && td->o.io_submit_mode ==IO_MODE_OFFLOAD );
The parentheses can be removed:
return td->o.serialize_overlap && td->o.io_submit_mode == IO_MODE_OFFLOAD;
Quoted 11 lines
fio.h +2
-0+} +/* * We currently only need to do locking if we have verifier threads * accessing our internal structures too ioengines.c +0
-0
ioengines.c enum fio_q_status td_io_queue(struct thread_data *td, struct io_u *io_u) · line 294
Quoted 8 lines
ioengines.c +1
-1@@ -288,7 +288,7 @@ 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 (td->o.serialize_overlap && td->o.io_submit_mode ==IO_MODE_OFFLOAD ) +if (td_offload_overlap (td))pthread_mutex_unlock (&overlap_check);assert (fio_file_open (io_u->file));
PATCH v2 2/4 fio: enable cross-thread overlap checking with processes
In reply to [email protected]From: Vincent Fu <[email protected]>
Overlap checking with io_submit_mode=offload requires relevant jobs to access each other's io_u's and io_u_all members. This patch modifies the fio_memalign and io_u_queue helpers to include an indicator signifying whether operations should use the shared memory pool. When fio is carrying out cross-job overlap checking in offload submission mode, these variables will be allocated from shared memory so that processes can be used and threads will no longer be required.
--- backend.c | 12 ++++++------ io_u_queue.c | 19 +++++++++++++++---- io_u_queue.h | 4 ++-- lib/memalign.c | 16 ++++++++++++---- lib/memalign.h | 5 +++-- t/dedupe.c | 12 ++++++------ 6 files changed, 44 insertions(+), 24 deletions(-)
backend.c +6 -6
@@ -1189,14 +1189,14 @@ static void cleanup_io_u(struct thread_data *td)if (td->io_ops->io_u_free) td->io_ops->io_u_free (td, io_u); -fio_memfree (io_u,sizeof (*io_u)); +fio_memfree (io_u,sizeof (*io_u),td_offload_overlap (td)); }free_io_mem (td);io_u_rexit (&td->io_u_requeues); -io_u_qexit (&td->io_u_freelist); -io_u_qexit (&td->io_u_all); +io_u_qexit (&td->io_u_freelist, false); +io_u_qexit (&td->io_u_all,td_offload_overlap (td));free_file_completion_logging (td); }
@@ -1211,8 +1211,8 @@ static int init_io_u(struct thread_data *td) err =0 ; err += !io_u_rinit (&td->io_u_requeues, td->o.iodepth); - err += !io_u_qinit (&td->io_u_freelist, td->o.iodepth); - err += !io_u_qinit (&td->io_u_all, td->o.iodepth); + err += !io_u_qinit (&td->io_u_freelist, td->o.iodepth, false); + err += !io_u_qinit (&td->io_u_all, td->o.iodepth,td_offload_overlap (td));if (err) {log_err ("fio: failed setting up IO queues\n" );
@@ -1227,7 +1227,7 @@ static int init_io_u(struct thread_data *td)if (td->terminate)return 1 ; - ptr =fio_memalign (cl_align,sizeof (*io_u)); + ptr =fio_memalign (cl_align,sizeof (*io_u),td_offload_overlap (td));if (!ptr) {log_err ("fio: unable to allocate aligned memory\n" );break ;
io_u_queue.c +15 -4
@@ -1,9 +1,17 @@#include <stdlib.h> +#include <string.h> #include "io_u_queue.h" +#include "smalloc.h" -bool io_u_qinit (struct io_u_queue *q,unsigned int nr) +bool io_u_qinit (struct io_u_queue *q,unsigned int nr,bool shared) { - q->io_us =calloc (nr,sizeof (struct io_u *)); +if (shared) { + q->io_us =smalloc (nr *sizeof (struct io_u *)); +memset (q->io_us,0 , nr *sizeof (struct io_u *)); + } +else + q->io_us =calloc (nr,sizeof (struct io_u *)); +if (!q->io_us)return false;
@@ -12,9 +20,12 @@ bool io_u_qinit(struct io_u_queue *q, unsigned int nr)return true; } -void io_u_qexit (struct io_u_queue *q) +void io_u_qexit (struct io_u_queue *q,bool shared) { -free (q->io_us); +if (shared) +sfree (q->io_us); +else +free (q->io_us); }bool io_u_rinit (struct io_u_ring *ring,unsigned int nr)
io_u_queue.h +2 -2
@@ -45,8 +45,8 @@ static inline int io_u_qempty(const struct io_u_queue *q)#define io_u_qiter (q, io_u, i) \ for (i = 0; i < (q)->nr && (io_u = (q)->io_us[i]); i++) -bool io_u_qinit (struct io_u_queue *q,unsigned int nr); -void io_u_qexit (struct io_u_queue *q); +bool io_u_qinit (struct io_u_queue *q,unsigned int nr,bool shared); +void io_u_qexit (struct io_u_queue *q,bool shared);struct io_u_ring {unsigned int head;
lib/memalign.c +12 -4
@@ -2,6 +2,7 @@#include <stdlib.h> #include "memalign.h" +#include "smalloc.h" #define PTR_ALIGN (ptr, mask) \ (char *)((uintptr_t)((ptr) + (mask)) & ~(mask))
@@ -10,14 +11,18 @@ struct align_footer {unsigned int offset; }; -void *fio_memalign (size_t alignment,size_t size) +void *fio_memalign (size_t alignment,size_t size,bool shared) {struct align_footer *f;void *ptr, *ret =NULL ;assert (!(alignment & (alignment -1 ))); - ptr =malloc (size + alignment +sizeof (*f) -1 ); +if (shared) + ptr =smalloc (size + alignment +sizeof (*f) -1 ); +else + ptr =malloc (size + alignment +sizeof (*f) -1 ); +if (ptr) { ret =PTR_ALIGN (ptr, alignment -1 ); f = ret + size;
@@ -27,9 +32,12 @@ void *fio_memalign(size_t alignment, size_t size)return ret; } -void fio_memfree (void *ptr,size_t size) +void fio_memfree (void *ptr,size_t size,bool shared) {struct align_footer *f = ptr + size; -free (ptr - f->offset); +if (shared) +sfree (ptr - f->offset); +else +free (ptr - f->offset); }
lib/memalign.h +3 -2
@@ -2,8 +2,9 @@#define FIO_MEMALIGN_H #include <inttypes.h> +#include <stdbool.h> -extern void *fio_memalign (size_t alignment,size_t size); -extern void fio_memfree (void *ptr,size_t size); +extern void *fio_memalign (size_t alignment,size_t size,bool shared); +extern void fio_memfree (void *ptr,size_t size,bool shared);#endif
t/dedupe.c +6 -6
@@ -158,8 +158,8 @@ static int col_check(struct chunk *c, struct item *i)char *cbuf, *ibuf;int ret =1 ; - cbuf =fio_memalign (blocksize, blocksize); - ibuf =fio_memalign (blocksize, blocksize); + cbuf =fio_memalign (blocksize, blocksize, false); + ibuf =fio_memalign (blocksize, blocksize, false); e = flist_entry(c->extent_list[0].next, struct extent, list);if (read_block (file.fd, cbuf, e->offset))
@@ -170,8 +170,8 @@ static int col_check(struct chunk *c, struct item *i) ret =memcmp (ibuf, cbuf, blocksize); out: -fio_memfree (cbuf, blocksize); -fio_memfree (ibuf, blocksize); +fio_memfree (cbuf, blocksize, false); +fio_memfree (ibuf, blocksize, false);return ret; }
@@ -309,7 +309,7 @@ static void *thread_fn(void *data)struct worker_thread *thread = data;void *buf; - buf =fio_memalign (blocksize, chunk_size); + buf =fio_memalign (blocksize, chunk_size, false); do { if (get_work(&thread->cur_offset, &thread->size)) {
@@ -323,7 +323,7 @@ static void *thread_fn(void *data) }while (1 ); thread->done =1 ; -fio_memfree (buf, chunk_size); +fio_memfree (buf, chunk_size, false);return NULL ; }
Signature · 1 line
2.17.1
Re: [PATCH v2 2/4] fio: enable cross-thread overlap checking with processes
io_u_queue.c line 14
Quoted 11 lines · On 10/17/18 8:52 AM, [email protected] wrote:
io_u_queue.c +8
-2-bool io_u_qinit (struct io_u_queue *q,unsigned int nr) +bool io_u_qinit (struct io_u_queue *q,unsigned int nr,bool shared) { - q->io_us =calloc (nr,sizeof (struct io_u *)); +if (shared) { + q->io_us =smalloc (nr *sizeof (struct io_u *)); +memset (q->io_us,0 , nr *sizeof (struct io_u *)); + } +else + q->io_us =calloc (nr,sizeof (struct io_u *)); +
smalloc() returns zeroed memory.
Apart from that, I think this patch looks fine.
Signature · 1 line
Jens Axboe
PATCH v2 3/4 fio: document locking for overlap checking in offload mode
In reply to [email protected]From: Vincent Fu <[email protected]>
Overlap checking in offload submission mode uses locks in an unusual manner. Add some comments in the code to clarify how locks are used.
--- backend.c | 5 +++++ ioengines.c | 7 +++++++ rate-submit.c | 8 ++++++++ 3 files changed, 20 insertions(+)
backend.c +5 -0
@@ -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_offload_overlap (td))pthread_mutex_lock (&overlap_check);td_set_runstate (td,TD_FINISHING );
ioengines.c +7 -0
@@ -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_offload_overlap (td))pthread_mutex_unlock (&overlap_check);
rate-submit.c +8 -0
@@ -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) {
Signature · 1 line
2.17.1
PATCH v2 4/4 docs: serialize_overlap=1 with io_submit_mode=offload no longer requires threads
In reply to [email protected]From: Vincent Fu <[email protected]>
--- HOWTO | 2 +- fio.1 | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-)
HOWTO +1 -1
@@ -2343,7 +2343,7 @@ I/O depth This option only applies to I/Os issued for a single job except when it is enabled along with :option:`io_submit_mode`=offload. In offload mode, fio will check for overlap among all I/Os submitted by offload jobs with :option:`serialize_overlap` - enabled. Threads must be used for all such jobs. + enabled. Default: false.
fio.1 +1 -1
@@ -2075,7 +2075,7 @@ this option can reduce both performance and the \fBiodepth\fR achieved. This option only applies to I/Os issued for a single job except when it is enabled along with \fBio_submit_mode\fR=offload. In offload mode, fio will check for overlap among all I/Os submitted by offload jobs with \fBserialize_overlap\fR -enabled. Threads must be used for all such jobs. +enabled. .P Default: false. .RE
Signature · 1 line
2.17.1