* [PATCH] Fix segfault with verify_async
@ 2011-10-25 20:23 Steven Lang
2011-10-25 21:00 ` Steven Lang
2011-10-26 7:51 ` Jens Axboe
0 siblings, 2 replies; 3+ messages in thread
From: Steven Lang @ 2011-10-25 20:23 UTC (permalink / raw)
To: fio; +Cc: Jens Axboe
At some point the filename was added to the report on verify failures,
however this broke verify_async, as the file pointer on the io_u is
set to NULL before the verify thread sees the io_u. �The result is a
segfault when there is a verify mismatch.
This patch changes the semantics of a deferred free (IO_U_F_FREE_DEF)
to have already called put_file, but not set the file pointer to NULL.
�This is safe to do as the file list is only freed after all the
verify threads have been terminated.
diff --git a/io_u.c b/io_u.c
index d1f66a9..fc3ee49 100644
--- a/io_u.c
+++ b/io_u.c
@@ -597,13 +597,12 @@ void put_io_u(struct thread_data *td, struct io_u *io_u)
{
td_io_u_lock(td);
- io_u->flags |= IO_U_F_FREE;
- io_u->flags &= ~IO_U_F_FREE_DEF;
-
- if (io_u->file)
+ if (io_u->file && !(io_u->flags & IO_U_F_FREE_DEF))
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)
td->cur_depth--;
flist_del_init(&io_u->list);
diff --git a/verify.c b/verify.c
index c450e88..68ee60f 100644
--- a/verify.c
+++ b/verify.c
@@ -599,10 +599,8 @@ int verify_io_u_async(struct thread_data *td,
struct io_u *io_u)
if (io_u->file)
put_file_log(td, io_u->file);
- io_u->file = NULL;
-
pthread_mutex_lock(&td->io_u_lock);
-
+
if (io_u->flags & IO_U_F_IN_CUR_DEPTH) {
td->cur_depth--;
io_u->flags &= ~IO_U_F_IN_CUR_DEPTH;
^ permalink raw reply related [flat|nested] 3+ messages in thread* Re: [PATCH] Fix segfault with verify_async
2011-10-25 20:23 [PATCH] Fix segfault with verify_async Steven Lang
@ 2011-10-25 21:00 ` Steven Lang
2011-10-26 7:51 ` Jens Axboe
1 sibling, 0 replies; 3+ messages in thread
From: Steven Lang @ 2011-10-25 21:00 UTC (permalink / raw)
To: fio; +Cc: Jens Axboe
FYI - this is the minimal config to cause the error this patch fixes,
and is what I used to test the fix. (Not using thread still causes a
segfault, but it is reported more subtly since just a sub-process hits
it.)
[segv]
ioengine=null
size=4k
verify=meta
rw=read
thread
verify_async=1
On Tue, Oct 25, 2011 at 1:23 PM, Steven Lang <tirea@google.com> wrote:
> At some point the filename was added to the report on verify failures,
> however this broke verify_async, as the file pointer on the io_u is
> set to NULL before the verify thread sees the io_u. �The result is a
> segfault when there is a verify mismatch.
>
> This patch changes the semantics of a deferred free (IO_U_F_FREE_DEF)
> to have already called put_file, but not set the file pointer to NULL.
> �This is safe to do as the file list is only freed after all the
> verify threads have been terminated.
>
> diff --git a/io_u.c b/io_u.c
> index d1f66a9..fc3ee49 100644
> --- a/io_u.c
> +++ b/io_u.c
> @@ -597,13 +597,12 @@ void put_io_u(struct thread_data *td, struct io_u *io_u)
> �{
> � � � �td_io_u_lock(td);
>
> - � � � io_u->flags |= IO_U_F_FREE;
> - � � � io_u->flags &= ~IO_U_F_FREE_DEF;
> -
> - � � � if (io_u->file)
> + � � � if (io_u->file && !(io_u->flags & IO_U_F_FREE_DEF))
> � � � � � � � �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)
> � � � � � � � �td->cur_depth--;
> � � � �flist_del_init(&io_u->list);
> diff --git a/verify.c b/verify.c
> index c450e88..68ee60f 100644
> --- a/verify.c
> +++ b/verify.c
> @@ -599,10 +599,8 @@ int verify_io_u_async(struct thread_data *td,
> struct io_u *io_u)
> � � � �if (io_u->file)
> � � � � � � � �put_file_log(td, io_u->file);
>
> - � � � io_u->file = NULL;
> -
> � � � �pthread_mutex_lock(&td->io_u_lock);
> -
> +
> � � � �if (io_u->flags & IO_U_F_IN_CUR_DEPTH) {
> � � � � � � � �td->cur_depth--;
> � � � � � � � �io_u->flags &= ~IO_U_F_IN_CUR_DEPTH;
>
^ permalink raw reply [flat|nested] 3+ messages in thread* Re: [PATCH] Fix segfault with verify_async
2011-10-25 20:23 [PATCH] Fix segfault with verify_async Steven Lang
2011-10-25 21:00 ` Steven Lang
@ 2011-10-26 7:51 ` Jens Axboe
1 sibling, 0 replies; 3+ messages in thread
From: Jens Axboe @ 2011-10-26 7:51 UTC (permalink / raw)
To: Steven Lang; +Cc: fio
On 2011-10-25 22:23, Steven Lang wrote:
> At some point the filename was added to the report on verify failures,
> however this broke verify_async, as the file pointer on the io_u is
> set to NULL before the verify thread sees the io_u. The result is a
> segfault when there is a verify mismatch.
>
> This patch changes the semantics of a deferred free (IO_U_F_FREE_DEF)
> to have already called put_file, but not set the file pointer to NULL.
> This is safe to do as the file list is only freed after all the
> verify threads have been terminated.
Thanks Steven, good catch. Applied.
--
Jens Axboe
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2011-10-26 7:51 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2011-10-25 20:23 [PATCH] Fix segfault with verify_async Steven Lang
2011-10-25 21:00 ` Steven Lang
2011-10-26 7:51 ` Jens Axboe
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox