| Message ID | 20180725121701.12867.49392.stgit@pasha-VirtualBox (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Fixing record/replay and adding reverse debugging | expand |
Pavel Dovgalyuk <Pavel.Dovgaluk@ispras.ru> writes: > Replay is capable of recording normal BH events, but sometimes > there are single use callbacks scheduled with aio_bh_schedule_oneshot > function. This patch enables recording and replaying such callbacks. > Block layer uses these events for calling the completion function. > Replaying these calls makes the execution deterministic. > > Signed-off-by: Pavel Dovgalyuk <Pavel.Dovgaluk@ispras.ru> I'm not sure what about this commit causes the compile breakage I'm seeing: LINK aarch64-linux-user/qemu-aarch64 ../libqemuutil.a(cpu-get-icount.o):(.bss+0x0): multiple definition of `use_icount' exec.o:(.bss+0x58): first defined here collect2: error: ld returned 1 exit status Makefile:199: recipe for target 'qemu-aarch64' failed make[1]: *** [qemu-aarch64] Error 1 Makefile:481: recipe for target 'subdir-aarch64-linux-user' failed make: *** [subdir-aarch64-linux-user] Error 2 It only occurs on a make clean && make -j on that commit though. It's hidden if you do incremental builds. > --- > block/block-backend.c | 3 ++- > include/sysemu/replay.h | 3 +++ > replay/replay-events.c | 16 ++++++++++++++++ > replay/replay-internal.h | 1 + > replay/replay.c | 2 +- > stubs/replay.c | 6 ++++++ > 6 files changed, 29 insertions(+), 2 deletions(-) > > diff --git a/block/block-backend.c b/block/block-backend.c > index f2f75a9..232d114 100644 > --- a/block/block-backend.c > +++ b/block/block-backend.c > @@ -17,6 +17,7 @@ > #include "block/throttle-groups.h" > #include "sysemu/blockdev.h" > #include "sysemu/sysemu.h" > +#include "sysemu/replay.h" > #include "qapi/error.h" > #include "qapi/qapi-events-block.h" > #include "qemu/id.h" > @@ -1370,7 +1371,7 @@ static BlockAIOCB *blk_aio_prwv(BlockBackend *blk, int64_t offset, int bytes, > > acb->has_returned = true; > if (acb->rwco.ret != NOT_DONE) { > - aio_bh_schedule_oneshot(blk_get_aio_context(blk), > + replay_bh_schedule_oneshot_event(blk_get_aio_context(blk), > blk_aio_complete_bh, acb); > } > > diff --git a/include/sysemu/replay.h b/include/sysemu/replay.h > index 8118b00..945bc74 100644 > --- a/include/sysemu/replay.h > +++ b/include/sysemu/replay.h > @@ -152,6 +152,9 @@ bool replay_events_enabled(void); > void replay_flush_events(void); > /*! Adds bottom half event to the queue */ > void replay_bh_schedule_event(QEMUBH *bh); > +/*! Adds oneshot bottom half event to the queue */ > +void replay_bh_schedule_oneshot_event(AioContext *ctx, > + QEMUBHFunc *cb, void *opaque); > /*! Adds input event to the queue */ > void replay_input_event(QemuConsole *src, InputEvent *evt); > /*! Adds input sync event to the queue */ > diff --git a/replay/replay-events.c b/replay/replay-events.c > index 0964a82..0ac8a5c 100644 > --- a/replay/replay-events.c > +++ b/replay/replay-events.c > @@ -37,6 +37,9 @@ static void replay_run_event(Event *event) > case REPLAY_ASYNC_EVENT_BH: > aio_bh_call(event->opaque); > break; > + case REPLAY_ASYNC_EVENT_BH_ONESHOT: > + ((QEMUBHFunc*)event->opaque)(event->opaque2); > + break; > case REPLAY_ASYNC_EVENT_INPUT: > qemu_input_event_send_impl(NULL, (InputEvent *)event->opaque); > qapi_free_InputEvent((InputEvent *)event->opaque); > @@ -132,6 +135,17 @@ void replay_bh_schedule_event(QEMUBH *bh) > } > } > > +void replay_bh_schedule_oneshot_event(AioContext *ctx, > + QEMUBHFunc *cb,void *opaque) > +{ > + if (events_enabled) { > + uint64_t id = replay_get_current_step(); > + replay_add_event(REPLAY_ASYNC_EVENT_BH_ONESHOT, cb, opaque, id); > + } else { > + aio_bh_schedule_oneshot(ctx, cb, opaque); > + } > +} > + > void replay_add_input_event(struct InputEvent *event) > { > replay_add_event(REPLAY_ASYNC_EVENT_INPUT, event, NULL, 0); > @@ -162,6 +176,7 @@ static void replay_save_event(Event *event, int checkpoint) > /* save event-specific data */ > switch (event->event_kind) { > case REPLAY_ASYNC_EVENT_BH: > + case REPLAY_ASYNC_EVENT_BH_ONESHOT: > replay_put_qword(event->id); > break; > case REPLAY_ASYNC_EVENT_INPUT: > @@ -216,6 +231,7 @@ static Event *replay_read_event(int checkpoint) > /* Events that has not to be in the queue */ > switch (replay_state.read_event_kind) { > case REPLAY_ASYNC_EVENT_BH: > + case REPLAY_ASYNC_EVENT_BH_ONESHOT: > if (replay_state.read_event_id == -1) { > replay_state.read_event_id = replay_get_qword(); > } > diff --git a/replay/replay-internal.h b/replay/replay-internal.h > index 08ef2ec..0c0ed16 100644 > --- a/replay/replay-internal.h > +++ b/replay/replay-internal.h > @@ -51,6 +51,7 @@ enum ReplayEvents { > > enum ReplayAsyncEventKind { > REPLAY_ASYNC_EVENT_BH, > + REPLAY_ASYNC_EVENT_BH_ONESHOT, > REPLAY_ASYNC_EVENT_INPUT, > REPLAY_ASYNC_EVENT_INPUT_SYNC, > REPLAY_ASYNC_EVENT_CHAR_READ, > diff --git a/replay/replay.c b/replay/replay.c > index 6e82764..061b1e2 100644 > --- a/replay/replay.c > +++ b/replay/replay.c > @@ -22,7 +22,7 @@ > > /* Current version of the replay mechanism. > Increase it when file format changes. */ > -#define REPLAY_VERSION 0xe02007 > +#define REPLAY_VERSION 0xe02008 > /* Size of replay log header */ > #define HEADER_SIZE (sizeof(uint32_t) + sizeof(uint64_t)) > > diff --git a/stubs/replay.c b/stubs/replay.c > index 781974e..cbdac80 100644 > --- a/stubs/replay.c > +++ b/stubs/replay.c > @@ -90,3 +90,9 @@ bool replay_reverse_continue(void) > { > return false; > } > + > +void replay_bh_schedule_oneshot_event(AioContext *ctx, > + QEMUBHFunc *cb,void *opaque) > +{ > + aio_bh_schedule_oneshot(ctx, cb, opaque); > +} -- Alex Bennée
Pavel Dovgalyuk <Pavel.Dovgaluk@ispras.ru> writes: > Replay is capable of recording normal BH events, but sometimes > there are single use callbacks scheduled with aio_bh_schedule_oneshot > function. This patch enables recording and replaying such callbacks. > Block layer uses these events for calling the completion function. > Replaying these calls makes the execution deterministic. > > Signed-off-by: Pavel Dovgalyuk <Pavel.Dovgaluk@ispras.ru> > --- > block/block-backend.c | 3 ++- > include/sysemu/replay.h | 3 +++ > replay/replay-events.c | 16 ++++++++++++++++ > replay/replay-internal.h | 1 + > replay/replay.c | 2 +- > stubs/replay.c | 6 ++++++ > 6 files changed, 29 insertions(+), 2 deletions(-) > > diff --git a/block/block-backend.c b/block/block-backend.c > index f2f75a9..232d114 100644 > --- a/block/block-backend.c > +++ b/block/block-backend.c > @@ -17,6 +17,7 @@ > #include "block/throttle-groups.h" > #include "sysemu/blockdev.h" > #include "sysemu/sysemu.h" > +#include "sysemu/replay.h" > #include "qapi/error.h" > #include "qapi/qapi-events-block.h" > #include "qemu/id.h" > @@ -1370,7 +1371,7 @@ static BlockAIOCB *blk_aio_prwv(BlockBackend *blk, int64_t offset, int bytes, > > acb->has_returned = true; > if (acb->rwco.ret != NOT_DONE) { > - aio_bh_schedule_oneshot(blk_get_aio_context(blk), > + replay_bh_schedule_oneshot_event(blk_get_aio_context(blk), > blk_aio_complete_bh, acb); > } > > diff --git a/include/sysemu/replay.h b/include/sysemu/replay.h > index 8118b00..945bc74 100644 > --- a/include/sysemu/replay.h > +++ b/include/sysemu/replay.h > @@ -152,6 +152,9 @@ bool replay_events_enabled(void); > void replay_flush_events(void); > /*! Adds bottom half event to the queue */ > void replay_bh_schedule_event(QEMUBH *bh); > +/*! Adds oneshot bottom half event to the queue */ > +void replay_bh_schedule_oneshot_event(AioContext *ctx, > + QEMUBHFunc *cb, void *opaque); > /*! Adds input event to the queue */ > void replay_input_event(QemuConsole *src, InputEvent *evt); > /*! Adds input sync event to the queue */ > diff --git a/replay/replay-events.c b/replay/replay-events.c > index 0964a82..0ac8a5c 100644 > --- a/replay/replay-events.c > +++ b/replay/replay-events.c > @@ -37,6 +37,9 @@ static void replay_run_event(Event *event) > case REPLAY_ASYNC_EVENT_BH: > aio_bh_call(event->opaque); > break; > + case REPLAY_ASYNC_EVENT_BH_ONESHOT: > + ((QEMUBHFunc*)event->opaque)(event->opaque2); > + break; > case REPLAY_ASYNC_EVENT_INPUT: > qemu_input_event_send_impl(NULL, (InputEvent *)event->opaque); > qapi_free_InputEvent((InputEvent *)event->opaque); > @@ -132,6 +135,17 @@ void replay_bh_schedule_event(QEMUBH *bh) > } > } > > +void replay_bh_schedule_oneshot_event(AioContext *ctx, > + QEMUBHFunc *cb,void *opaque) > +{ > + if (events_enabled) { > + uint64_t id = replay_get_current_step(); > + replay_add_event(REPLAY_ASYNC_EVENT_BH_ONESHOT, cb, opaque, id); > + } else { > + aio_bh_schedule_oneshot(ctx, cb, opaque); > + } > +} > + > void replay_add_input_event(struct InputEvent *event) > { > replay_add_event(REPLAY_ASYNC_EVENT_INPUT, event, NULL, 0); > @@ -162,6 +176,7 @@ static void replay_save_event(Event *event, int checkpoint) > /* save event-specific data */ > switch (event->event_kind) { > case REPLAY_ASYNC_EVENT_BH: > + case REPLAY_ASYNC_EVENT_BH_ONESHOT: > replay_put_qword(event->id); > break; > case REPLAY_ASYNC_EVENT_INPUT: > @@ -216,6 +231,7 @@ static Event *replay_read_event(int checkpoint) > /* Events that has not to be in the queue */ > switch (replay_state.read_event_kind) { > case REPLAY_ASYNC_EVENT_BH: > + case REPLAY_ASYNC_EVENT_BH_ONESHOT: > if (replay_state.read_event_id == -1) { > replay_state.read_event_id = replay_get_qword(); > } > diff --git a/replay/replay-internal.h b/replay/replay-internal.h > index 08ef2ec..0c0ed16 100644 > --- a/replay/replay-internal.h > +++ b/replay/replay-internal.h > @@ -51,6 +51,7 @@ enum ReplayEvents { > > enum ReplayAsyncEventKind { > REPLAY_ASYNC_EVENT_BH, > + REPLAY_ASYNC_EVENT_BH_ONESHOT, > REPLAY_ASYNC_EVENT_INPUT, > REPLAY_ASYNC_EVENT_INPUT_SYNC, > REPLAY_ASYNC_EVENT_CHAR_READ, > diff --git a/replay/replay.c b/replay/replay.c > index 6e82764..061b1e2 100644 > --- a/replay/replay.c > +++ b/replay/replay.c > @@ -22,7 +22,7 @@ > > /* Current version of the replay mechanism. > Increase it when file format changes. */ > -#define REPLAY_VERSION 0xe02007 > +#define REPLAY_VERSION 0xe02008 > /* Size of replay log header */ > #define HEADER_SIZE (sizeof(uint32_t) + sizeof(uint64_t)) > > diff --git a/stubs/replay.c b/stubs/replay.c > index 781974e..cbdac80 100644 > --- a/stubs/replay.c > +++ b/stubs/replay.c > @@ -90,3 +90,9 @@ bool replay_reverse_continue(void) > { > return false; > } > + > +void replay_bh_schedule_oneshot_event(AioContext *ctx, > + QEMUBHFunc *cb,void *opaque) > +{ > + aio_bh_schedule_oneshot(ctx, cb, opaque); > +} It seems wrong to have something in stubs that actively does stuff. Isn't this meant to be a bunch of NOPs? -- Alex Bennée
Alex Bennée <alex.bennee@linaro.org> writes: > Pavel Dovgalyuk <Pavel.Dovgaluk@ispras.ru> writes: > >> Replay is capable of recording normal BH events, but sometimes >> there are single use callbacks scheduled with aio_bh_schedule_oneshot >> function. This patch enables recording and replaying such callbacks. >> Block layer uses these events for calling the completion function. >> Replaying these calls makes the execution deterministic. >> >> Signed-off-by: Pavel Dovgalyuk <Pavel.Dovgaluk@ispras.ru> > > I'm not sure what about this commit causes the compile breakage I'm > seeing: > > LINK aarch64-linux-user/qemu-aarch64 > ../libqemuutil.a(cpu-get-icount.o):(.bss+0x0): multiple definition of `use_icount' > exec.o:(.bss+0x58): first defined here > collect2: error: ld returned 1 exit status > Makefile:199: recipe for target 'qemu-aarch64' failed > make[1]: *** [qemu-aarch64] Error 1 > Makefile:481: recipe for target 'subdir-aarch64-linux-user' failed > make: *** [subdir-aarch64-linux-user] Error 2 > > It only occurs on a make clean && make -j on that commit though. It's > hidden if you do incremental builds. And it seems to be the same failure across all the cross builds: https://app.shippable.com/github/stsquad/qemu/runs/538/summary/console -- Alex Bennée
> From: Alex Bennée [mailto:alex.bennee@linaro.org] > > Replay is capable of recording normal BH events, but sometimes > > there are single use callbacks scheduled with aio_bh_schedule_oneshot > > function. This patch enables recording and replaying such callbacks. > > Block layer uses these events for calling the completion function. > > Replaying these calls makes the execution deterministic. > > > > Signed-off-by: Pavel Dovgalyuk <Pavel.Dovgaluk@ispras.ru> > > I'm not sure what about this commit causes the compile breakage I'm > seeing: > > LINK aarch64-linux-user/qemu-aarch64 > ../libqemuutil.a(cpu-get-icount.o):(.bss+0x0): multiple definition of `use_icount' > exec.o:(.bss+0x58): first defined here > collect2: error: ld returned 1 exit status > Makefile:199: recipe for target 'qemu-aarch64' failed > make[1]: *** [qemu-aarch64] Error 1 > Makefile:481: recipe for target 'subdir-aarch64-linux-user' failed > make: *** [subdir-aarch64-linux-user] Error 2 > > It only occurs on a make clean && make -j on that commit though. It's > hidden if you do incremental builds. make distclean works for me in such cases. Pavel Dovgalyuk
Pavel Dovgalyuk <dovgaluk@ispras.ru> writes: >> From: Alex Bennée [mailto:alex.bennee@linaro.org] >> > Replay is capable of recording normal BH events, but sometimes >> > there are single use callbacks scheduled with aio_bh_schedule_oneshot >> > function. This patch enables recording and replaying such callbacks. >> > Block layer uses these events for calling the completion function. >> > Replaying these calls makes the execution deterministic. >> > >> > Signed-off-by: Pavel Dovgalyuk <Pavel.Dovgaluk@ispras.ru> >> >> I'm not sure what about this commit causes the compile breakage I'm >> seeing: >> >> LINK aarch64-linux-user/qemu-aarch64 >> ../libqemuutil.a(cpu-get-icount.o):(.bss+0x0): multiple definition of `use_icount' >> exec.o:(.bss+0x58): first defined here >> collect2: error: ld returned 1 exit status >> Makefile:199: recipe for target 'qemu-aarch64' failed >> make[1]: *** [qemu-aarch64] Error 1 >> Makefile:481: recipe for target 'subdir-aarch64-linux-user' failed >> make: *** [subdir-aarch64-linux-user] Error 2 >> >> It only occurs on a make clean && make -j on that commit though. It's >> hidden if you do incremental builds. > > make distclean works for me in such cases. Hmmm make distclean ./configure make And I get: LINK aarch64-linux-user/qemu-aarch64 ../libqemuutil.a(cpu-get-icount.o):(.bss+0x0): multiple definition of `use_icount' exec.o:(.bss+0x58): first defined here collect2: error: ld returned 1 exit status Makefile:199: recipe for target 'qemu-aarch64' failed make[1]: *** [qemu-aarch64] Error 1 Makefile:481: recipe for target 'subdir-aarch64-linux-user' failed make: *** [subdir-aarch64-linux-user] Error 2 The CI builds all look pretty broken too: https://travis-ci.org/stsquad/qemu/builds/408537385 https://app.shippable.com/github/stsquad/qemu/runs/538/summary/console > > Pavel Dovgalyuk -- Alex Bennée
> From: Alex Bennée [mailto:alex.bennee@linaro.org] > Pavel Dovgalyuk <Pavel.Dovgaluk@ispras.ru> writes: > > > Replay is capable of recording normal BH events, but sometimes > > there are single use callbacks scheduled with aio_bh_schedule_oneshot > > function. This patch enables recording and replaying such callbacks. > > Block layer uses these events for calling the completion function. > > Replaying these calls makes the execution deterministic. > > > > Signed-off-by: Pavel Dovgalyuk <Pavel.Dovgaluk@ispras.ru> > > --- > > block/block-backend.c | 3 ++- > > include/sysemu/replay.h | 3 +++ > > replay/replay-events.c | 16 ++++++++++++++++ > > replay/replay-internal.h | 1 + > > replay/replay.c | 2 +- > > stubs/replay.c | 6 ++++++ > > 6 files changed, 29 insertions(+), 2 deletions(-) > > > > diff --git a/block/block-backend.c b/block/block-backend.c > > index f2f75a9..232d114 100644 > > --- a/block/block-backend.c > > +++ b/block/block-backend.c > > @@ -17,6 +17,7 @@ > > #include "block/throttle-groups.h" > > #include "sysemu/blockdev.h" > > #include "sysemu/sysemu.h" > > +#include "sysemu/replay.h" > > #include "qapi/error.h" > > #include "qapi/qapi-events-block.h" > > #include "qemu/id.h" > > @@ -1370,7 +1371,7 @@ static BlockAIOCB *blk_aio_prwv(BlockBackend *blk, int64_t offset, int > bytes, > > > > acb->has_returned = true; > > if (acb->rwco.ret != NOT_DONE) { > > - aio_bh_schedule_oneshot(blk_get_aio_context(blk), > > + replay_bh_schedule_oneshot_event(blk_get_aio_context(blk), > > blk_aio_complete_bh, acb); > > } > > > > diff --git a/include/sysemu/replay.h b/include/sysemu/replay.h > > index 8118b00..945bc74 100644 > > --- a/include/sysemu/replay.h > > +++ b/include/sysemu/replay.h > > @@ -152,6 +152,9 @@ bool replay_events_enabled(void); > > void replay_flush_events(void); > > /*! Adds bottom half event to the queue */ > > void replay_bh_schedule_event(QEMUBH *bh); > > +/*! Adds oneshot bottom half event to the queue */ > > +void replay_bh_schedule_oneshot_event(AioContext *ctx, > > + QEMUBHFunc *cb, void *opaque); > > /*! Adds input event to the queue */ > > void replay_input_event(QemuConsole *src, InputEvent *evt); > > /*! Adds input sync event to the queue */ > > diff --git a/replay/replay-events.c b/replay/replay-events.c > > index 0964a82..0ac8a5c 100644 > > --- a/replay/replay-events.c > > +++ b/replay/replay-events.c > > @@ -37,6 +37,9 @@ static void replay_run_event(Event *event) > > case REPLAY_ASYNC_EVENT_BH: > > aio_bh_call(event->opaque); > > break; > > + case REPLAY_ASYNC_EVENT_BH_ONESHOT: > > + ((QEMUBHFunc*)event->opaque)(event->opaque2); > > + break; > > case REPLAY_ASYNC_EVENT_INPUT: > > qemu_input_event_send_impl(NULL, (InputEvent *)event->opaque); > > qapi_free_InputEvent((InputEvent *)event->opaque); > > @@ -132,6 +135,17 @@ void replay_bh_schedule_event(QEMUBH *bh) > > } > > } > > > > +void replay_bh_schedule_oneshot_event(AioContext *ctx, > > + QEMUBHFunc *cb,void *opaque) > > +{ > > + if (events_enabled) { > > + uint64_t id = replay_get_current_step(); > > + replay_add_event(REPLAY_ASYNC_EVENT_BH_ONESHOT, cb, opaque, id); > > + } else { > > + aio_bh_schedule_oneshot(ctx, cb, opaque); > > + } > > +} > > + > > void replay_add_input_event(struct InputEvent *event) > > { > > replay_add_event(REPLAY_ASYNC_EVENT_INPUT, event, NULL, 0); > > @@ -162,6 +176,7 @@ static void replay_save_event(Event *event, int checkpoint) > > /* save event-specific data */ > > switch (event->event_kind) { > > case REPLAY_ASYNC_EVENT_BH: > > + case REPLAY_ASYNC_EVENT_BH_ONESHOT: > > replay_put_qword(event->id); > > break; > > case REPLAY_ASYNC_EVENT_INPUT: > > @@ -216,6 +231,7 @@ static Event *replay_read_event(int checkpoint) > > /* Events that has not to be in the queue */ > > switch (replay_state.read_event_kind) { > > case REPLAY_ASYNC_EVENT_BH: > > + case REPLAY_ASYNC_EVENT_BH_ONESHOT: > > if (replay_state.read_event_id == -1) { > > replay_state.read_event_id = replay_get_qword(); > > } > > diff --git a/replay/replay-internal.h b/replay/replay-internal.h > > index 08ef2ec..0c0ed16 100644 > > --- a/replay/replay-internal.h > > +++ b/replay/replay-internal.h > > @@ -51,6 +51,7 @@ enum ReplayEvents { > > > > enum ReplayAsyncEventKind { > > REPLAY_ASYNC_EVENT_BH, > > + REPLAY_ASYNC_EVENT_BH_ONESHOT, > > REPLAY_ASYNC_EVENT_INPUT, > > REPLAY_ASYNC_EVENT_INPUT_SYNC, > > REPLAY_ASYNC_EVENT_CHAR_READ, > > diff --git a/replay/replay.c b/replay/replay.c > > index 6e82764..061b1e2 100644 > > --- a/replay/replay.c > > +++ b/replay/replay.c > > @@ -22,7 +22,7 @@ > > > > /* Current version of the replay mechanism. > > Increase it when file format changes. */ > > -#define REPLAY_VERSION 0xe02007 > > +#define REPLAY_VERSION 0xe02008 > > /* Size of replay log header */ > > #define HEADER_SIZE (sizeof(uint32_t) + sizeof(uint64_t)) > > > > diff --git a/stubs/replay.c b/stubs/replay.c > > index 781974e..cbdac80 100644 > > --- a/stubs/replay.c > > +++ b/stubs/replay.c > > @@ -90,3 +90,9 @@ bool replay_reverse_continue(void) > > { > > return false; > > } > > + > > +void replay_bh_schedule_oneshot_event(AioContext *ctx, > > + QEMUBHFunc *cb,void *opaque) > > +{ > > + aio_bh_schedule_oneshot(ctx, cb, opaque); > > +} > > It seems wrong to have something in stubs that actively does stuff. > Isn't this meant to be a bunch of NOPs? I thinks stubs are meant to be functions that are used by qemu-img and other tools. As I replaced aio_bh_schedule_oneshot with replay_bh_schedule_oneshot_event, the tools should not notice this. Therefore the stub performs this call. Pavel Dovgalyuk
On 26/07/2018 19:17, Alex Bennée wrote: >> + >> +void replay_bh_schedule_oneshot_event(AioContext *ctx, >> + QEMUBHFunc *cb,void *opaque) >> +{ >> + aio_bh_schedule_oneshot(ctx, cb, opaque); >> +} > It seems wrong to have something in stubs that actively does stuff. > Isn't this meant to be a bunch of NOPs? No, not necessarily, for example int64_t cpu_get_clock(void) { return get_clock_realtime(); } or even void error_vprintf(const char *fmt, va_list ap) { if (g_test_initialized() && !g_test_subprocess() && getenv("QTEST_SILENT_ERRORS")) { char *msg = g_strdup_vprintf(fmt, ap); g_test_message("%s", msg); g_free(msg); } else { vfprintf(stderr, fmt, ap); } } Paolo
> -----Original Message----- > From: Alex Bennée [mailto:alex.bennee@linaro.org] > Sent: Friday, July 27, 2018 7:45 PM > To: Pavel Dovgalyuk > Cc: 'Pavel Dovgalyuk'; qemu-devel@nongnu.org; kwolf@redhat.com; peter.maydell@linaro.org; > war2jordan@live.com; crosthwaite.peter@gmail.com; boost.lists@gmail.com; quintela@redhat.com; > ciro.santilli@gmail.com; jasowang@redhat.com; mst@redhat.com; zuban32s@gmail.com; > armbru@redhat.com; maria.klimushenkova@ispras.ru; kraxel@redhat.com; > thomas.dullien@googlemail.com; pbonzini@redhat.com; mreitz@redhat.com; dgilbert@redhat.com; > rth@twiddle.net > Subject: Re: [PATCH v5 23/24] replay: add BH oneshot event for block layer > > > Pavel Dovgalyuk <dovgaluk@ispras.ru> writes: > > >> From: Alex Bennée [mailto:alex.bennee@linaro.org] > >> > Replay is capable of recording normal BH events, but sometimes > >> > there are single use callbacks scheduled with aio_bh_schedule_oneshot > >> > function. This patch enables recording and replaying such callbacks. > >> > Block layer uses these events for calling the completion function. > >> > Replaying these calls makes the execution deterministic. > >> > > >> > Signed-off-by: Pavel Dovgalyuk <Pavel.Dovgaluk@ispras.ru> > >> > >> I'm not sure what about this commit causes the compile breakage I'm > >> seeing: > >> > >> LINK aarch64-linux-user/qemu-aarch64 > >> ../libqemuutil.a(cpu-get-icount.o):(.bss+0x0): multiple definition of `use_icount' > >> exec.o:(.bss+0x58): first defined here > >> collect2: error: ld returned 1 exit status > >> Makefile:199: recipe for target 'qemu-aarch64' failed > >> make[1]: *** [qemu-aarch64] Error 1 > >> Makefile:481: recipe for target 'subdir-aarch64-linux-user' failed > >> make: *** [subdir-aarch64-linux-user] Error 2 > >> > >> It only occurs on a make clean && make -j on that commit though. It's > >> hidden if you do incremental builds. > > > > make distclean works for me in such cases. > > Hmmm > > make distclean > ./configure > make > > And I get: > > LINK aarch64-linux-user/qemu-aarch64 > ../libqemuutil.a(cpu-get-icount.o):(.bss+0x0): multiple definition of `use_icount' > exec.o:(.bss+0x58): first defined here > collect2: error: ld returned 1 exit status > Makefile:199: recipe for target 'qemu-aarch64' failed > make[1]: *** [qemu-aarch64] Error 1 > Makefile:481: recipe for target 'subdir-aarch64-linux-user' failed > make: *** [subdir-aarch64-linux-user] Error 2 That's strange. As I understand, linux-user emulators does not include block layer. They also don't include record/replay. Why then block- and record-related stub affects the build? Pavel Dovgalyuk
diff --git a/block/block-backend.c b/block/block-backend.c index f2f75a9..232d114 100644 --- a/block/block-backend.c +++ b/block/block-backend.c @@ -17,6 +17,7 @@ #include "block/throttle-groups.h" #include "sysemu/blockdev.h" #include "sysemu/sysemu.h" +#include "sysemu/replay.h" #include "qapi/error.h" #include "qapi/qapi-events-block.h" #include "qemu/id.h" @@ -1370,7 +1371,7 @@ static BlockAIOCB *blk_aio_prwv(BlockBackend *blk, int64_t offset, int bytes, acb->has_returned = true; if (acb->rwco.ret != NOT_DONE) { - aio_bh_schedule_oneshot(blk_get_aio_context(blk), + replay_bh_schedule_oneshot_event(blk_get_aio_context(blk), blk_aio_complete_bh, acb); } diff --git a/include/sysemu/replay.h b/include/sysemu/replay.h index 8118b00..945bc74 100644 --- a/include/sysemu/replay.h +++ b/include/sysemu/replay.h @@ -152,6 +152,9 @@ bool replay_events_enabled(void); void replay_flush_events(void); /*! Adds bottom half event to the queue */ void replay_bh_schedule_event(QEMUBH *bh); +/*! Adds oneshot bottom half event to the queue */ +void replay_bh_schedule_oneshot_event(AioContext *ctx, + QEMUBHFunc *cb, void *opaque); /*! Adds input event to the queue */ void replay_input_event(QemuConsole *src, InputEvent *evt); /*! Adds input sync event to the queue */ diff --git a/replay/replay-events.c b/replay/replay-events.c index 0964a82..0ac8a5c 100644 --- a/replay/replay-events.c +++ b/replay/replay-events.c @@ -37,6 +37,9 @@ static void replay_run_event(Event *event) case REPLAY_ASYNC_EVENT_BH: aio_bh_call(event->opaque); break; + case REPLAY_ASYNC_EVENT_BH_ONESHOT: + ((QEMUBHFunc*)event->opaque)(event->opaque2); + break; case REPLAY_ASYNC_EVENT_INPUT: qemu_input_event_send_impl(NULL, (InputEvent *)event->opaque); qapi_free_InputEvent((InputEvent *)event->opaque); @@ -132,6 +135,17 @@ void replay_bh_schedule_event(QEMUBH *bh) } } +void replay_bh_schedule_oneshot_event(AioContext *ctx, + QEMUBHFunc *cb,void *opaque) +{ + if (events_enabled) { + uint64_t id = replay_get_current_step(); + replay_add_event(REPLAY_ASYNC_EVENT_BH_ONESHOT, cb, opaque, id); + } else { + aio_bh_schedule_oneshot(ctx, cb, opaque); + } +} + void replay_add_input_event(struct InputEvent *event) { replay_add_event(REPLAY_ASYNC_EVENT_INPUT, event, NULL, 0); @@ -162,6 +176,7 @@ static void replay_save_event(Event *event, int checkpoint) /* save event-specific data */ switch (event->event_kind) { case REPLAY_ASYNC_EVENT_BH: + case REPLAY_ASYNC_EVENT_BH_ONESHOT: replay_put_qword(event->id); break; case REPLAY_ASYNC_EVENT_INPUT: @@ -216,6 +231,7 @@ static Event *replay_read_event(int checkpoint) /* Events that has not to be in the queue */ switch (replay_state.read_event_kind) { case REPLAY_ASYNC_EVENT_BH: + case REPLAY_ASYNC_EVENT_BH_ONESHOT: if (replay_state.read_event_id == -1) { replay_state.read_event_id = replay_get_qword(); } diff --git a/replay/replay-internal.h b/replay/replay-internal.h index 08ef2ec..0c0ed16 100644 --- a/replay/replay-internal.h +++ b/replay/replay-internal.h @@ -51,6 +51,7 @@ enum ReplayEvents { enum ReplayAsyncEventKind { REPLAY_ASYNC_EVENT_BH, + REPLAY_ASYNC_EVENT_BH_ONESHOT, REPLAY_ASYNC_EVENT_INPUT, REPLAY_ASYNC_EVENT_INPUT_SYNC, REPLAY_ASYNC_EVENT_CHAR_READ, diff --git a/replay/replay.c b/replay/replay.c index 6e82764..061b1e2 100644 --- a/replay/replay.c +++ b/replay/replay.c @@ -22,7 +22,7 @@ /* Current version of the replay mechanism. Increase it when file format changes. */ -#define REPLAY_VERSION 0xe02007 +#define REPLAY_VERSION 0xe02008 /* Size of replay log header */ #define HEADER_SIZE (sizeof(uint32_t) + sizeof(uint64_t)) diff --git a/stubs/replay.c b/stubs/replay.c index 781974e..cbdac80 100644 --- a/stubs/replay.c +++ b/stubs/replay.c @@ -90,3 +90,9 @@ bool replay_reverse_continue(void) { return false; } + +void replay_bh_schedule_oneshot_event(AioContext *ctx, + QEMUBHFunc *cb,void *opaque) +{ + aio_bh_schedule_oneshot(ctx, cb, opaque); +}
Replay is capable of recording normal BH events, but sometimes there are single use callbacks scheduled with aio_bh_schedule_oneshot function. This patch enables recording and replaying such callbacks. Block layer uses these events for calling the completion function. Replaying these calls makes the execution deterministic. Signed-off-by: Pavel Dovgalyuk <Pavel.Dovgaluk@ispras.ru> --- block/block-backend.c | 3 ++- include/sysemu/replay.h | 3 +++ replay/replay-events.c | 16 ++++++++++++++++ replay/replay-internal.h | 1 + replay/replay.c | 2 +- stubs/replay.c | 6 ++++++ 6 files changed, 29 insertions(+), 2 deletions(-)