From: Jens Axboe <axboe@kernel.dk>
To: scameron@beardog.cce.hp.com, fio@vger.kernel.org
Subject: Re: fio crash when using verify
Date: Wed, 23 Jul 2014 12:10:09 +0200 [thread overview]
Message-ID: <53CF8A01.2090001@kernel.dk> (raw)
In-Reply-To: <20140722205947.GD14599@beardog.cce.hp.com>
[-- Attachment #1: Type: text/plain, Size: 765 bytes --]
On 2014-07-22 22:59, scameron@beardog.cce.hp.com wrote:
>
> Hi Jens,
>
> I'm running into a crash with fio today, seems to be easily reproducible.
>
> [root@msablackburn saperf]# fio --version
> fio-2.1.11-2-g90811
>
> (top of current git tree.)
>
>
> [root@msablackburn saperf]# fio ./2drive_sdr_verify.fio
> 4_KiB_RR_drive_r: (g=0): rw=randwrite, bs=4K-4K/4K-4K/4K-4K, ioengine=libaio, iodepth=170
> 4_KiB_RR_drive_s: (g=0): rw=randwrite, bs=4K-4K/4K-4K/4K-4K, ioengine=libaio, iodepth=170
> fio-2.1.11-2-g90811
> Starting 2 threads
> fio: io_u.c:1315: __get_io_u: Assertion `io_u->flags & IO_U_F_FREE' failed.a 58m:59s]
> Aborted (core dumped)
Looks like there's a long standing race in the async verification. Does
this patch fix it for you?
--
Jens Axboe
[-- Attachment #2: async-verify-race.patch --]
[-- Type: text/x-patch, Size: 7082 bytes --]
diff --git a/io_u.c b/io_u.c
index 16b52d63aa09..c3c1d76d2ec1 100644
--- a/io_u.c
+++ b/io_u.c
@@ -688,10 +688,10 @@ void put_io_u(struct thread_data *td, struct io_u *io_u)
{
td_io_u_lock(td);
- if (io_u->file && !(io_u->flags & IO_U_F_FREE_DEF))
+ if (io_u->file && !(io_u->flags & IO_U_F_NO_FILE_PUT))
put_file_log(td, io_u->file);
+
io_u->file = NULL;
- io_u->flags &= ~IO_U_F_FREE_DEF;
io_u->flags |= IO_U_F_FREE;
if (io_u->flags & IO_U_F_IN_CUR_DEPTH)
@@ -1313,9 +1313,9 @@ again:
if (io_u) {
assert(io_u->flags & IO_U_F_FREE);
- io_u->flags &= ~(IO_U_F_FREE | IO_U_F_FREE_DEF);
- io_u->flags &= ~(IO_U_F_TRIMMED | IO_U_F_BARRIER);
- io_u->flags &= ~IO_U_F_VER_LIST;
+ io_u->flags &= ~(IO_U_F_FREE | IO_U_F_NO_FILE_PUT |
+ IO_U_F_TRIMMED | IO_U_F_BARRIER |
+ IO_U_F_VER_LIST);
io_u->error = 0;
io_u->acct_ddir = -1;
@@ -1607,9 +1607,11 @@ static long long usec_for_io(struct thread_data *td, enum fio_ddir ddir)
return remainder * 1000000 / bps + secs * 1000000;
}
-static void io_completed(struct thread_data *td, struct io_u *io_u,
+static void io_completed(struct thread_data *td, struct io_u **io_u_ptr,
struct io_completion_data *icd)
{
+ struct io_u *io_u = *io_u_ptr;
+ enum fio_ddir ddir = io_u->ddir;
struct fio_file *f;
dprint_io_u(io_u, "io complete");
@@ -1635,7 +1637,7 @@ static void io_completed(struct thread_data *td, struct io_u *io_u,
td_io_u_unlock(td);
- if (ddir_sync(io_u->ddir)) {
+ if (ddir_sync(ddir)) {
td->last_was_sync = 1;
f = io_u->file;
if (f) {
@@ -1646,12 +1648,12 @@ static void io_completed(struct thread_data *td, struct io_u *io_u,
}
td->last_was_sync = 0;
- td->last_ddir = io_u->ddir;
+ td->last_ddir = ddir;
- if (!io_u->error && ddir_rw(io_u->ddir)) {
+ if (!io_u->error && ddir_rw(ddir)) {
unsigned int bytes = io_u->buflen - io_u->resid;
- const enum fio_ddir idx = io_u->ddir;
- const enum fio_ddir odx = io_u->ddir ^ 1;
+ const enum fio_ddir idx = ddir;
+ const enum fio_ddir odx = ddir ^ 1;
int ret;
td->io_blocks[idx]++;
@@ -1691,7 +1693,8 @@ static void io_completed(struct thread_data *td, struct io_u *io_u,
icd->bytes_done[idx] += bytes;
if (io_u->end_io) {
- ret = io_u->end_io(td, io_u);
+ ret = io_u->end_io(td, io_u_ptr);
+ io_u = *io_u_ptr;
if (ret && !icd->error)
icd->error = ret;
}
@@ -1700,9 +1703,11 @@ static void io_completed(struct thread_data *td, struct io_u *io_u,
io_u_log_error(td, io_u);
}
if (icd->error) {
- enum error_type_bit eb = td_error_type(io_u->ddir, icd->error);
+ enum error_type_bit eb = td_error_type(ddir, icd->error);
+
if (!td_non_fatal_error(td, eb, icd->error))
return;
+
/*
* If there is a non_fatal error, then add to the error count
* and clear all the errors.
@@ -1710,7 +1715,8 @@ static void io_completed(struct thread_data *td, struct io_u *io_u,
update_error_count(td, icd->error);
td_clear_error(td);
icd->error = 0;
- io_u->error = 0;
+ if (io_u)
+ io_u->error = 0;
}
}
@@ -1738,9 +1744,9 @@ static void ios_completed(struct thread_data *td,
for (i = 0; i < icd->nr; i++) {
io_u = td->io_ops->event(td, i);
- io_completed(td, io_u, icd);
+ io_completed(td, &io_u, icd);
- if (!(io_u->flags & IO_U_F_FREE_DEF))
+ if (io_u)
put_io_u(td, io_u);
}
}
@@ -1754,9 +1760,9 @@ int io_u_sync_complete(struct thread_data *td, struct io_u *io_u,
struct io_completion_data icd;
init_icd(td, &icd, 1);
- io_completed(td, io_u, &icd);
+ io_completed(td, &io_u, &icd);
- if (!(io_u->flags & IO_U_F_FREE_DEF))
+ if (io_u)
put_io_u(td, io_u);
if (icd.error) {
diff --git a/io_u_queue.h b/io_u_queue.h
index 5b6cad0ef173..b702b1f39875 100644
--- a/io_u_queue.h
+++ b/io_u_queue.h
@@ -12,8 +12,13 @@ struct io_u_queue {
static inline struct io_u *io_u_qpop(struct io_u_queue *q)
{
- if (q->nr)
- return q->io_us[--q->nr];
+ if (q->nr) {
+ const unsigned int next = --q->nr;
+ struct io_u *io_u = q->io_us[next];
+
+ q->io_us[next] = NULL;
+ return io_u;
+ }
return NULL;
}
diff --git a/ioengine.h b/ioengine.h
index 37bf5fce125e..29c8487a0c12 100644
--- a/ioengine.h
+++ b/ioengine.h
@@ -20,7 +20,7 @@
enum {
IO_U_F_FREE = 1 << 0,
IO_U_F_FLIGHT = 1 << 1,
- IO_U_F_FREE_DEF = 1 << 2,
+ IO_U_F_NO_FILE_PUT = 1 << 2,
IO_U_F_IN_CUR_DEPTH = 1 << 3,
IO_U_F_BUSY_OK = 1 << 4,
IO_U_F_TRIMMED = 1 << 5,
@@ -90,7 +90,7 @@ struct io_u {
/*
* Callback for io completion
*/
- int (*end_io)(struct thread_data *, struct io_u *);
+ int (*end_io)(struct thread_data *, struct io_u **);
union {
#ifdef CONFIG_LIBAIO
diff --git a/verify.c b/verify.c
index 7c99e15e73be..217b68695045 100644
--- a/verify.c
+++ b/verify.c
@@ -656,19 +656,21 @@ static int verify_io_u_md5(struct verify_header *hdr, struct vcont *vc)
/*
* Push IO verification to a separate thread
*/
-int verify_io_u_async(struct thread_data *td, struct io_u *io_u)
+int verify_io_u_async(struct thread_data *td, struct io_u **io_u_ptr)
{
- if (io_u->file)
- put_file_log(td, io_u->file);
+ struct io_u *io_u = *io_u_ptr;
pthread_mutex_lock(&td->io_u_lock);
+ if (io_u->file)
+ put_file_log(td, io_u->file);
+
if (io_u->flags & IO_U_F_IN_CUR_DEPTH) {
td->cur_depth--;
io_u->flags &= ~IO_U_F_IN_CUR_DEPTH;
}
flist_add_tail(&io_u->verify_list, &td->verify_list);
- io_u->flags |= IO_U_F_FREE_DEF;
+ *io_u_ptr = NULL;
pthread_mutex_unlock(&td->io_u_lock);
pthread_cond_signal(&td->verify_cond);
@@ -747,9 +749,10 @@ err:
return EILSEQ;
}
-int verify_io_u(struct thread_data *td, struct io_u *io_u)
+int verify_io_u(struct thread_data *td, struct io_u **io_u_ptr)
{
struct verify_header *hdr;
+ struct io_u *io_u = *io_u_ptr;
unsigned int header_size, hdr_inc, hdr_num = 0;
void *p;
int ret;
@@ -1188,9 +1191,11 @@ static void *verify_async_thread(void *data)
while (!flist_empty(&list)) {
io_u = flist_first_entry(&list, struct io_u, verify_list);
- flist_del(&io_u->verify_list);
+ flist_del_init(&io_u->verify_list);
+
+ io_u->flags |= IO_U_F_NO_FILE_PUT;
+ ret = verify_io_u(td, &io_u);
- ret = verify_io_u(td, io_u);
put_io_u(td, io_u);
if (!ret)
continue;
diff --git a/verify.h b/verify.h
index dba7743a75d6..bb3fd492ccbc 100644
--- a/verify.h
+++ b/verify.h
@@ -76,8 +76,8 @@ struct vhdr_xxhash {
*/
extern void populate_verify_io_u(struct thread_data *, struct io_u *);
extern int __must_check get_next_verify(struct thread_data *td, struct io_u *);
-extern int __must_check verify_io_u(struct thread_data *, struct io_u *);
-extern int verify_io_u_async(struct thread_data *, struct io_u *);
+extern int __must_check verify_io_u(struct thread_data *, struct io_u **);
+extern int verify_io_u_async(struct thread_data *, struct io_u **);
extern void fill_verify_pattern(struct thread_data *td, void *p, unsigned int len, struct io_u *io_u, unsigned long seed, int use_seed);
extern void fill_buffer_pattern(struct thread_data *td, void *p, unsigned int len);
extern void fio_verify_init(struct thread_data *td);
next prev parent reply other threads:[~2014-07-23 10:10 UTC|newest]
Thread overview: 6+ messages / expand[flat|nested] mbox.gz Atom feed top
2014-07-22 20:59 fio crash when using verify scameron
2014-07-23 10:10 ` Jens Axboe [this message]
2014-07-23 14:22 ` scameron
2014-07-23 14:23 ` Jens Axboe
2014-07-25 13:09 ` scameron
2014-07-25 13:16 ` Jens Axboe
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=53CF8A01.2090001@kernel.dk \
--to=axboe@kernel.dk \
--cc=fio@vger.kernel.org \
--cc=scameron@beardog.cce.hp.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
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.