diff mbox

[02/17] trace: Show blockjob actions via bytes, not sectors

Message ID 20170411222945.11741-3-eblake@redhat.com (mailing list archive)
State New, archived
Headers show

Commit Message

Eric Blake April 11, 2017, 10:29 p.m. UTC
Upcoming patches are going to switch to byte-based interfaces
instead of sector-based.  Even worse, trace_backup_do_cow_enter()
had a weird mix of cluster and sector indices.  Make the tracing
uniformly use bytes.

Signed-off-by: Eric Blake <eblake@redhat.com>
---
 block/backup.c     | 16 ++++++++++------
 block/commit.c     |  3 ++-
 block/mirror.c     | 26 +++++++++++++++++---------
 block/stream.c     |  3 ++-
 block/trace-events | 14 +++++++-------
 5 files changed, 38 insertions(+), 24 deletions(-)

Comments

John Snow April 17, 2017, 7:18 p.m. UTC | #1
On 04/11/2017 06:29 PM, Eric Blake wrote:
> Upcoming patches are going to switch to byte-based interfaces
> instead of sector-based.  Even worse, trace_backup_do_cow_enter()
> had a weird mix of cluster and sector indices.  Make the tracing
> uniformly use bytes.
> 
> Signed-off-by: Eric Blake <eblake@redhat.com>
> ---
>  block/backup.c     | 16 ++++++++++------
>  block/commit.c     |  3 ++-
>  block/mirror.c     | 26 +++++++++++++++++---------
>  block/stream.c     |  3 ++-
>  block/trace-events | 14 +++++++-------
>  5 files changed, 38 insertions(+), 24 deletions(-)
> 
> diff --git a/block/backup.c b/block/backup.c
> index bcfa321..b28df8c 100644
> --- a/block/backup.c
> +++ b/block/backup.c
> @@ -102,6 +102,7 @@ static int coroutine_fn backup_do_cow(BackupBlockJob *job,
>      void *bounce_buffer = NULL;
>      int ret = 0;
>      int64_t sectors_per_cluster = cluster_size_sectors(job);
> +    int64_t bytes_per_cluster = sectors_per_cluster * BDRV_SECTOR_SIZE;
>      int64_t start, end;
>      int n;
> 
> @@ -110,18 +111,20 @@ static int coroutine_fn backup_do_cow(BackupBlockJob *job,
>      start = sector_num / sectors_per_cluster;
>      end = DIV_ROUND_UP(sector_num + nb_sectors, sectors_per_cluster);
> 
> -    trace_backup_do_cow_enter(job, start, sector_num, nb_sectors);
> +    trace_backup_do_cow_enter(job, start * bytes_per_cluster,
> +                              sector_num * BDRV_SECTOR_SIZE,
> +                              nb_sectors * BDRV_SECTOR_SIZE);
> 
>      wait_for_overlapping_requests(job, start, end);
>      cow_request_begin(&cow_request, job, start, end);
> 
>      for (; start < end; start++) {
>          if (test_bit(start, job->done_bitmap)) {
> -            trace_backup_do_cow_skip(job, start);
> +            trace_backup_do_cow_skip(job, start * bytes_per_cluster);
>              continue; /* already copied */
>          }
> 
> -        trace_backup_do_cow_process(job, start);
> +        trace_backup_do_cow_process(job, start * bytes_per_cluster);
> 
>          n = MIN(sectors_per_cluster,
>                  job->common.len / BDRV_SECTOR_SIZE -
> @@ -138,7 +141,7 @@ static int coroutine_fn backup_do_cow(BackupBlockJob *job,
>                              bounce_qiov.size, &bounce_qiov,
>                              is_write_notifier ? BDRV_REQ_NO_SERIALISING : 0);
>          if (ret < 0) {
> -            trace_backup_do_cow_read_fail(job, start, ret);
> +            trace_backup_do_cow_read_fail(job, start * bytes_per_cluster, ret);
>              if (error_is_read) {
>                  *error_is_read = true;
>              }
> @@ -154,7 +157,7 @@ static int coroutine_fn backup_do_cow(BackupBlockJob *job,
>                                   job->compress ? BDRV_REQ_WRITE_COMPRESSED : 0);
>          }
>          if (ret < 0) {
> -            trace_backup_do_cow_write_fail(job, start, ret);
> +            trace_backup_do_cow_write_fail(job, start * bytes_per_cluster, ret);
>              if (error_is_read) {
>                  *error_is_read = false;
>              }
> @@ -177,7 +180,8 @@ out:
> 
>      cow_request_end(&cow_request);
> 
> -    trace_backup_do_cow_return(job, sector_num, nb_sectors, ret);
> +    trace_backup_do_cow_return(job, sector_num * BDRV_SECTOR_SIZE,
> +                               nb_sectors * BDRV_SECTOR_SIZE, ret);
> 
>      qemu_co_rwlock_unlock(&job->flush_rwlock);
> 
> diff --git a/block/commit.c b/block/commit.c
> index 8b684e0..11dab98 100644
> --- a/block/commit.c
> +++ b/block/commit.c
> @@ -176,7 +176,8 @@ static void coroutine_fn commit_run(void *opaque)
>                                        COMMIT_BUFFER_SIZE / BDRV_SECTOR_SIZE,
>                                        &n);
>          copy = (ret == 1);
> -        trace_commit_one_iteration(s, sector_num, n, ret);
> +        trace_commit_one_iteration(s, sector_num * BDRV_SECTOR_SIZE,
> +                                   n * BDRV_SECTOR_SIZE, ret);
>          if (copy) {
>              ret = commit_populate(s->top, s->base, sector_num, n, buf);
>              bytes_written += n * BDRV_SECTOR_SIZE;
> diff --git a/block/mirror.c b/block/mirror.c
> index a7d0960..d83d482 100644
> --- a/block/mirror.c
> +++ b/block/mirror.c
> @@ -103,7 +103,8 @@ static void mirror_iteration_done(MirrorOp *op, int ret)
>      int64_t chunk_num;
>      int i, nb_chunks, sectors_per_chunk;
> 
> -    trace_mirror_iteration_done(s, op->sector_num, op->nb_sectors, ret);
> +    trace_mirror_iteration_done(s, op->sector_num * BDRV_SECTOR_SIZE,
> +                                op->nb_sectors * BDRV_SECTOR_SIZE, ret);
> 
>      s->in_flight--;
>      s->sectors_in_flight -= op->nb_sectors;
> @@ -268,7 +269,8 @@ static int mirror_do_read(MirrorBlockJob *s, int64_t sector_num,
>      nb_chunks = DIV_ROUND_UP(nb_sectors, sectors_per_chunk);
> 
>      while (s->buf_free_count < nb_chunks) {
> -        trace_mirror_yield_in_flight(s, sector_num, s->in_flight);
> +        trace_mirror_yield_in_flight(s, sector_num * BDRV_SECTOR_SIZE,
> +                                     s->in_flight);
>          mirror_wait_for_io(s);
>      }
> 
> @@ -294,7 +296,8 @@ static int mirror_do_read(MirrorBlockJob *s, int64_t sector_num,
>      /* Copy the dirty cluster.  */
>      s->in_flight++;
>      s->sectors_in_flight += nb_sectors;
> -    trace_mirror_one_iteration(s, sector_num, nb_sectors);
> +    trace_mirror_one_iteration(s, sector_num * BDRV_SECTOR_SIZE,
> +                               nb_sectors * BDRV_SECTOR_SIZE);
> 
>      blk_aio_preadv(source, sector_num * BDRV_SECTOR_SIZE, &op->qiov, 0,
>                     mirror_read_complete, op);
> @@ -346,13 +349,15 @@ static uint64_t coroutine_fn mirror_iteration(MirrorBlockJob *s)
>      if (sector_num < 0) {
>          bdrv_set_dirty_iter(s->dbi, 0);
>          sector_num = bdrv_dirty_iter_next(s->dbi);
> -        trace_mirror_restart_iter(s, bdrv_get_dirty_count(s->dirty_bitmap));
> +        trace_mirror_restart_iter(s, bdrv_get_dirty_count(s->dirty_bitmap) *
> +                                  BDRV_SECTOR_SIZE);

mirror_restart_iter(void *s, int64_t cnt) "s %p dirty count %"PRId64

I guess the print doesn't really bother to say what the unit is, just a
"dirty count."

Does this conflict with the bitmap series, though? (Won't need to scale
by BDRV_SECTOR_SIZE after that, I think.)

>          assert(sector_num >= 0);
>      }
> 
>      first_chunk = sector_num / sectors_per_chunk;
>      while (test_bit(first_chunk, s->in_flight_bitmap)) {
> -        trace_mirror_yield_in_flight(s, sector_num, s->in_flight);
> +        trace_mirror_yield_in_flight(s, sector_num * BDRV_SECTOR_SIZE,
> +                                     s->in_flight);
>          mirror_wait_for_io(s);
>      }
> 
> @@ -428,7 +433,8 @@ static uint64_t coroutine_fn mirror_iteration(MirrorBlockJob *s)
>          }
> 
>          while (s->in_flight >= MAX_IN_FLIGHT) {
> -            trace_mirror_yield_in_flight(s, sector_num, s->in_flight);
> +            trace_mirror_yield_in_flight(s, sector_num * BDRV_SECTOR_SIZE,
> +                                         s->in_flight);
>              mirror_wait_for_io(s);
>          }
> 
> @@ -811,7 +817,8 @@ static void coroutine_fn mirror_run(void *opaque)
>              s->common.iostatus == BLOCK_DEVICE_IO_STATUS_OK) {
>              if (s->in_flight >= MAX_IN_FLIGHT || s->buf_free_count == 0 ||
>                  (cnt == 0 && s->in_flight > 0)) {
> -                trace_mirror_yield(s, cnt, s->buf_free_count, s->in_flight);
> +                trace_mirror_yield(s, cnt * BDRV_SECTOR_SIZE,
> +                                   s->buf_free_count, s->in_flight);

I guess this is another case where the unit wasn't really specified, so
we can just change it.

>                  mirror_wait_for_io(s);
>                  continue;
>              } else if (cnt != 0) {
> @@ -852,7 +859,7 @@ static void coroutine_fn mirror_run(void *opaque)
>               * whether to switch to target check one last time if I/O has
>               * come in the meanwhile, and if not flush the data to disk.
>               */
> -            trace_mirror_before_drain(s, cnt);
> +            trace_mirror_before_drain(s, cnt * BDRV_SECTOR_SIZE);
> 
>              bdrv_drained_begin(bs);
>              cnt = bdrv_get_dirty_count(s->dirty_bitmap);
> @@ -871,7 +878,8 @@ static void coroutine_fn mirror_run(void *opaque)
>          }
> 
>          ret = 0;
> -        trace_mirror_before_sleep(s, cnt, s->synced, delay_ns);
> +        trace_mirror_before_sleep(s, cnt * BDRV_SECTOR_SIZE,
> +                                  s->synced, delay_ns);
>          if (!s->synced) {
>              block_job_sleep_ns(&s->common, QEMU_CLOCK_REALTIME, delay_ns);
>              if (block_job_is_cancelled(&s->common)) {
> diff --git a/block/stream.c b/block/stream.c
> index 11c5cf1..3134ec5 100644
> --- a/block/stream.c
> +++ b/block/stream.c
> @@ -168,7 +168,8 @@ static void coroutine_fn stream_run(void *opaque)
> 
>              copy = (ret == 1);
>          }
> -        trace_stream_one_iteration(s, sector_num, n, ret);
> +        trace_stream_one_iteration(s, sector_num * BDRV_SECTOR_SIZE,
> +                                   n * BDRV_SECTOR_SIZE, ret);
>          if (copy) {
>              ret = stream_populate(blk, sector_num, n, buf);
>          }
> diff --git a/block/trace-events b/block/trace-events
> index 0bc5c0a..04f6463 100644
> --- a/block/trace-events
> +++ b/block/trace-events
> @@ -18,11 +18,11 @@ bdrv_co_pwrite_zeroes(void *bs, int64_t offset, int count, int flags) "bs %p off
>  bdrv_co_do_copy_on_readv(void *bs, int64_t offset, unsigned int bytes, int64_t cluster_offset, unsigned int cluster_bytes) "bs %p offset %"PRId64" bytes %u cluster_offset %"PRId64" cluster_bytes %u"
> 
>  # block/stream.c
> -stream_one_iteration(void *s, int64_t sector_num, int nb_sectors, int is_allocated) "s %p sector_num %"PRId64" nb_sectors %d is_allocated %d"
> +stream_one_iteration(void *s, int64_t offset, uint64_t bytes, int is_allocated) "s %p offset %" PRId64 " bytes %" PRIu64 " is_allocated %d"
>  stream_start(void *bs, void *base, void *s) "bs %p base %p s %p"
> 
>  # block/commit.c
> -commit_one_iteration(void *s, int64_t sector_num, int nb_sectors, int is_allocated) "s %p sector_num %"PRId64" nb_sectors %d is_allocated %d"
> +commit_one_iteration(void *s, int64_t offset, uint64_t bytes, int is_allocated) "s %p offset %" PRId64 " bytes %" PRIu64 " is_allocated %d"
>  commit_start(void *bs, void *base, void *top, void *s) "bs %p base %p top %p s %p"
> 
>  # block/mirror.c
> @@ -31,14 +31,14 @@ mirror_restart_iter(void *s, int64_t cnt) "s %p dirty count %"PRId64
>  mirror_before_flush(void *s) "s %p"
>  mirror_before_drain(void *s, int64_t cnt) "s %p dirty count %"PRId64
>  mirror_before_sleep(void *s, int64_t cnt, int synced, uint64_t delay_ns) "s %p dirty count %"PRId64" synced %d delay %"PRIu64"ns"
> -mirror_one_iteration(void *s, int64_t sector_num, int nb_sectors) "s %p sector_num %"PRId64" nb_sectors %d"
> -mirror_iteration_done(void *s, int64_t sector_num, int nb_sectors, int ret) "s %p sector_num %"PRId64" nb_sectors %d ret %d"
> +mirror_one_iteration(void *s, int64_t offset, uint64_t bytes) "s %p offset %" PRId64 " bytes %" PRIu64
> +mirror_iteration_done(void *s, int64_t offset, uint64_t bytes, int ret) "s %p offset %" PRId64 " bytes %" PRIu64 " ret %d"
>  mirror_yield(void *s, int64_t cnt, int buf_free_count, int in_flight) "s %p dirty count %"PRId64" free buffers %d in_flight %d"
> -mirror_yield_in_flight(void *s, int64_t sector_num, int in_flight) "s %p sector_num %"PRId64" in_flight %d"
> +mirror_yield_in_flight(void *s, int64_t offset, int in_flight) "s %p offset %" PRId64 " in_flight %d"
> 
>  # block/backup.c
> -backup_do_cow_enter(void *job, int64_t start, int64_t sector_num, int nb_sectors) "job %p start %"PRId64" sector_num %"PRId64" nb_sectors %d"
> -backup_do_cow_return(void *job, int64_t sector_num, int nb_sectors, int ret) "job %p sector_num %"PRId64" nb_sectors %d ret %d"
> +backup_do_cow_enter(void *job, int64_t start, int64_t offset, uint64_t bytes) "job %p start %" PRId64 " offset %" PRId64 " bytes %" PRIu64
> +backup_do_cow_return(void *job, int64_t offset, uint64_t bytes, int ret) "job %p offset %" PRId64 " bytes %" PRIu64 " ret %d"

>  backup_do_cow_skip(void *job, int64_t start) "job %p start %"PRId64
>  backup_do_cow_process(void *job, int64_t start) "job %p start %"PRId64
>  backup_do_cow_read_fail(void *job, int64_t start, int ret) "job %p start %"PRId64" ret %d"

I guess these three were "cluster" based, but you didn't need to change
anything to assert them as byte-based.

> 

Under the assumption that it's okay to change tracing output without
being able to differentiate between new and old output:

Reviewed-by: John Snow <jsnow@redhat.com>
Eric Blake April 17, 2017, 7:55 p.m. UTC | #2
On 04/17/2017 02:18 PM, John Snow wrote:

>> @@ -346,13 +349,15 @@ static uint64_t coroutine_fn mirror_iteration(MirrorBlockJob *s)
>>      if (sector_num < 0) {
>>          bdrv_set_dirty_iter(s->dbi, 0);
>>          sector_num = bdrv_dirty_iter_next(s->dbi);
>> -        trace_mirror_restart_iter(s, bdrv_get_dirty_count(s->dirty_bitmap));
>> +        trace_mirror_restart_iter(s, bdrv_get_dirty_count(s->dirty_bitmap) *
>> +                                  BDRV_SECTOR_SIZE);
> 
> mirror_restart_iter(void *s, int64_t cnt) "s %p dirty count %"PRId64
> 
> I guess the print doesn't really bother to say what the unit is, just a
> "dirty count."
> 
> Does this conflict with the bitmap series, though? (Won't need to scale
> by BDRV_SECTOR_SIZE after that, I think.)
> 

That series depends on this one going in first (as currently written),
and indeed, in that series, the code DOES simplify to drop the '*
BDRV_SECTOR_SIZE' in the patch that changes the return scale of
bdrv_get_dirty_count().


>>  backup_do_cow_skip(void *job, int64_t start) "job %p start %"PRId64
>>  backup_do_cow_process(void *job, int64_t start) "job %p start %"PRId64
>>  backup_do_cow_read_fail(void *job, int64_t start, int ret) "job %p start %"PRId64" ret %d"
> 
> I guess these three were "cluster" based, but you didn't need to change
> anything to assert them as byte-based.
> 
>>
> 
> Under the assumption that it's okay to change tracing output without
> being able to differentiate between new and old output:

I'll leave that to Stefan's discretion as trace maintainer, but my
thoughts are that tracing is for debugging purposes, and as long as you
know what binary you are debugging, you can figure out what the trace
message meant by referring back to the source that generated that binary.

> 
> Reviewed-by: John Snow <jsnow@redhat.com>
>
Stefan Hajnoczi April 19, 2017, 9:12 a.m. UTC | #3
On Mon, Apr 17, 2017 at 02:55:53PM -0500, Eric Blake wrote:
> On 04/17/2017 02:18 PM, John Snow wrote:
> >> @@ -346,13 +349,15 @@ static uint64_t coroutine_fn mirror_iteration(MirrorBlockJob *s)
> >>  backup_do_cow_skip(void *job, int64_t start) "job %p start %"PRId64
> >>  backup_do_cow_process(void *job, int64_t start) "job %p start %"PRId64
> >>  backup_do_cow_read_fail(void *job, int64_t start, int ret) "job %p start %"PRId64" ret %d"
> > 
> > I guess these three were "cluster" based, but you didn't need to change
> > anything to assert them as byte-based.
> > 
> >>
> > 
> > Under the assumption that it's okay to change tracing output without
> > being able to differentiate between new and old output:
> 
> I'll leave that to Stefan's discretion as trace maintainer, but my
> thoughts are that tracing is for debugging purposes, and as long as you
> know what binary you are debugging, you can figure out what the trace
> message meant by referring back to the source that generated that binary.

That's okay.  Trace events are not a stable interface.  Most trace
events are low-level and require access to the corresponding source tree
anyway.

The only exception is that tools in scripts/ may need to be updated when
trace events change (e.g. scripts to graph or analyze specific trace
events).

Stefan
diff mbox

Patch

diff --git a/block/backup.c b/block/backup.c
index bcfa321..b28df8c 100644
--- a/block/backup.c
+++ b/block/backup.c
@@ -102,6 +102,7 @@  static int coroutine_fn backup_do_cow(BackupBlockJob *job,
     void *bounce_buffer = NULL;
     int ret = 0;
     int64_t sectors_per_cluster = cluster_size_sectors(job);
+    int64_t bytes_per_cluster = sectors_per_cluster * BDRV_SECTOR_SIZE;
     int64_t start, end;
     int n;

@@ -110,18 +111,20 @@  static int coroutine_fn backup_do_cow(BackupBlockJob *job,
     start = sector_num / sectors_per_cluster;
     end = DIV_ROUND_UP(sector_num + nb_sectors, sectors_per_cluster);

-    trace_backup_do_cow_enter(job, start, sector_num, nb_sectors);
+    trace_backup_do_cow_enter(job, start * bytes_per_cluster,
+                              sector_num * BDRV_SECTOR_SIZE,
+                              nb_sectors * BDRV_SECTOR_SIZE);

     wait_for_overlapping_requests(job, start, end);
     cow_request_begin(&cow_request, job, start, end);

     for (; start < end; start++) {
         if (test_bit(start, job->done_bitmap)) {
-            trace_backup_do_cow_skip(job, start);
+            trace_backup_do_cow_skip(job, start * bytes_per_cluster);
             continue; /* already copied */
         }

-        trace_backup_do_cow_process(job, start);
+        trace_backup_do_cow_process(job, start * bytes_per_cluster);

         n = MIN(sectors_per_cluster,
                 job->common.len / BDRV_SECTOR_SIZE -
@@ -138,7 +141,7 @@  static int coroutine_fn backup_do_cow(BackupBlockJob *job,
                             bounce_qiov.size, &bounce_qiov,
                             is_write_notifier ? BDRV_REQ_NO_SERIALISING : 0);
         if (ret < 0) {
-            trace_backup_do_cow_read_fail(job, start, ret);
+            trace_backup_do_cow_read_fail(job, start * bytes_per_cluster, ret);
             if (error_is_read) {
                 *error_is_read = true;
             }
@@ -154,7 +157,7 @@  static int coroutine_fn backup_do_cow(BackupBlockJob *job,
                                  job->compress ? BDRV_REQ_WRITE_COMPRESSED : 0);
         }
         if (ret < 0) {
-            trace_backup_do_cow_write_fail(job, start, ret);
+            trace_backup_do_cow_write_fail(job, start * bytes_per_cluster, ret);
             if (error_is_read) {
                 *error_is_read = false;
             }
@@ -177,7 +180,8 @@  out:

     cow_request_end(&cow_request);

-    trace_backup_do_cow_return(job, sector_num, nb_sectors, ret);
+    trace_backup_do_cow_return(job, sector_num * BDRV_SECTOR_SIZE,
+                               nb_sectors * BDRV_SECTOR_SIZE, ret);

     qemu_co_rwlock_unlock(&job->flush_rwlock);

diff --git a/block/commit.c b/block/commit.c
index 8b684e0..11dab98 100644
--- a/block/commit.c
+++ b/block/commit.c
@@ -176,7 +176,8 @@  static void coroutine_fn commit_run(void *opaque)
                                       COMMIT_BUFFER_SIZE / BDRV_SECTOR_SIZE,
                                       &n);
         copy = (ret == 1);
-        trace_commit_one_iteration(s, sector_num, n, ret);
+        trace_commit_one_iteration(s, sector_num * BDRV_SECTOR_SIZE,
+                                   n * BDRV_SECTOR_SIZE, ret);
         if (copy) {
             ret = commit_populate(s->top, s->base, sector_num, n, buf);
             bytes_written += n * BDRV_SECTOR_SIZE;
diff --git a/block/mirror.c b/block/mirror.c
index a7d0960..d83d482 100644
--- a/block/mirror.c
+++ b/block/mirror.c
@@ -103,7 +103,8 @@  static void mirror_iteration_done(MirrorOp *op, int ret)
     int64_t chunk_num;
     int i, nb_chunks, sectors_per_chunk;

-    trace_mirror_iteration_done(s, op->sector_num, op->nb_sectors, ret);
+    trace_mirror_iteration_done(s, op->sector_num * BDRV_SECTOR_SIZE,
+                                op->nb_sectors * BDRV_SECTOR_SIZE, ret);

     s->in_flight--;
     s->sectors_in_flight -= op->nb_sectors;
@@ -268,7 +269,8 @@  static int mirror_do_read(MirrorBlockJob *s, int64_t sector_num,
     nb_chunks = DIV_ROUND_UP(nb_sectors, sectors_per_chunk);

     while (s->buf_free_count < nb_chunks) {
-        trace_mirror_yield_in_flight(s, sector_num, s->in_flight);
+        trace_mirror_yield_in_flight(s, sector_num * BDRV_SECTOR_SIZE,
+                                     s->in_flight);
         mirror_wait_for_io(s);
     }

@@ -294,7 +296,8 @@  static int mirror_do_read(MirrorBlockJob *s, int64_t sector_num,
     /* Copy the dirty cluster.  */
     s->in_flight++;
     s->sectors_in_flight += nb_sectors;
-    trace_mirror_one_iteration(s, sector_num, nb_sectors);
+    trace_mirror_one_iteration(s, sector_num * BDRV_SECTOR_SIZE,
+                               nb_sectors * BDRV_SECTOR_SIZE);

     blk_aio_preadv(source, sector_num * BDRV_SECTOR_SIZE, &op->qiov, 0,
                    mirror_read_complete, op);
@@ -346,13 +349,15 @@  static uint64_t coroutine_fn mirror_iteration(MirrorBlockJob *s)
     if (sector_num < 0) {
         bdrv_set_dirty_iter(s->dbi, 0);
         sector_num = bdrv_dirty_iter_next(s->dbi);
-        trace_mirror_restart_iter(s, bdrv_get_dirty_count(s->dirty_bitmap));
+        trace_mirror_restart_iter(s, bdrv_get_dirty_count(s->dirty_bitmap) *
+                                  BDRV_SECTOR_SIZE);
         assert(sector_num >= 0);
     }

     first_chunk = sector_num / sectors_per_chunk;
     while (test_bit(first_chunk, s->in_flight_bitmap)) {
-        trace_mirror_yield_in_flight(s, sector_num, s->in_flight);
+        trace_mirror_yield_in_flight(s, sector_num * BDRV_SECTOR_SIZE,
+                                     s->in_flight);
         mirror_wait_for_io(s);
     }

@@ -428,7 +433,8 @@  static uint64_t coroutine_fn mirror_iteration(MirrorBlockJob *s)
         }

         while (s->in_flight >= MAX_IN_FLIGHT) {
-            trace_mirror_yield_in_flight(s, sector_num, s->in_flight);
+            trace_mirror_yield_in_flight(s, sector_num * BDRV_SECTOR_SIZE,
+                                         s->in_flight);
             mirror_wait_for_io(s);
         }

@@ -811,7 +817,8 @@  static void coroutine_fn mirror_run(void *opaque)
             s->common.iostatus == BLOCK_DEVICE_IO_STATUS_OK) {
             if (s->in_flight >= MAX_IN_FLIGHT || s->buf_free_count == 0 ||
                 (cnt == 0 && s->in_flight > 0)) {
-                trace_mirror_yield(s, cnt, s->buf_free_count, s->in_flight);
+                trace_mirror_yield(s, cnt * BDRV_SECTOR_SIZE,
+                                   s->buf_free_count, s->in_flight);
                 mirror_wait_for_io(s);
                 continue;
             } else if (cnt != 0) {
@@ -852,7 +859,7 @@  static void coroutine_fn mirror_run(void *opaque)
              * whether to switch to target check one last time if I/O has
              * come in the meanwhile, and if not flush the data to disk.
              */
-            trace_mirror_before_drain(s, cnt);
+            trace_mirror_before_drain(s, cnt * BDRV_SECTOR_SIZE);

             bdrv_drained_begin(bs);
             cnt = bdrv_get_dirty_count(s->dirty_bitmap);
@@ -871,7 +878,8 @@  static void coroutine_fn mirror_run(void *opaque)
         }

         ret = 0;
-        trace_mirror_before_sleep(s, cnt, s->synced, delay_ns);
+        trace_mirror_before_sleep(s, cnt * BDRV_SECTOR_SIZE,
+                                  s->synced, delay_ns);
         if (!s->synced) {
             block_job_sleep_ns(&s->common, QEMU_CLOCK_REALTIME, delay_ns);
             if (block_job_is_cancelled(&s->common)) {
diff --git a/block/stream.c b/block/stream.c
index 11c5cf1..3134ec5 100644
--- a/block/stream.c
+++ b/block/stream.c
@@ -168,7 +168,8 @@  static void coroutine_fn stream_run(void *opaque)

             copy = (ret == 1);
         }
-        trace_stream_one_iteration(s, sector_num, n, ret);
+        trace_stream_one_iteration(s, sector_num * BDRV_SECTOR_SIZE,
+                                   n * BDRV_SECTOR_SIZE, ret);
         if (copy) {
             ret = stream_populate(blk, sector_num, n, buf);
         }
diff --git a/block/trace-events b/block/trace-events
index 0bc5c0a..04f6463 100644
--- a/block/trace-events
+++ b/block/trace-events
@@ -18,11 +18,11 @@  bdrv_co_pwrite_zeroes(void *bs, int64_t offset, int count, int flags) "bs %p off
 bdrv_co_do_copy_on_readv(void *bs, int64_t offset, unsigned int bytes, int64_t cluster_offset, unsigned int cluster_bytes) "bs %p offset %"PRId64" bytes %u cluster_offset %"PRId64" cluster_bytes %u"

 # block/stream.c
-stream_one_iteration(void *s, int64_t sector_num, int nb_sectors, int is_allocated) "s %p sector_num %"PRId64" nb_sectors %d is_allocated %d"
+stream_one_iteration(void *s, int64_t offset, uint64_t bytes, int is_allocated) "s %p offset %" PRId64 " bytes %" PRIu64 " is_allocated %d"
 stream_start(void *bs, void *base, void *s) "bs %p base %p s %p"

 # block/commit.c
-commit_one_iteration(void *s, int64_t sector_num, int nb_sectors, int is_allocated) "s %p sector_num %"PRId64" nb_sectors %d is_allocated %d"
+commit_one_iteration(void *s, int64_t offset, uint64_t bytes, int is_allocated) "s %p offset %" PRId64 " bytes %" PRIu64 " is_allocated %d"
 commit_start(void *bs, void *base, void *top, void *s) "bs %p base %p top %p s %p"

 # block/mirror.c
@@ -31,14 +31,14 @@  mirror_restart_iter(void *s, int64_t cnt) "s %p dirty count %"PRId64
 mirror_before_flush(void *s) "s %p"
 mirror_before_drain(void *s, int64_t cnt) "s %p dirty count %"PRId64
 mirror_before_sleep(void *s, int64_t cnt, int synced, uint64_t delay_ns) "s %p dirty count %"PRId64" synced %d delay %"PRIu64"ns"
-mirror_one_iteration(void *s, int64_t sector_num, int nb_sectors) "s %p sector_num %"PRId64" nb_sectors %d"
-mirror_iteration_done(void *s, int64_t sector_num, int nb_sectors, int ret) "s %p sector_num %"PRId64" nb_sectors %d ret %d"
+mirror_one_iteration(void *s, int64_t offset, uint64_t bytes) "s %p offset %" PRId64 " bytes %" PRIu64
+mirror_iteration_done(void *s, int64_t offset, uint64_t bytes, int ret) "s %p offset %" PRId64 " bytes %" PRIu64 " ret %d"
 mirror_yield(void *s, int64_t cnt, int buf_free_count, int in_flight) "s %p dirty count %"PRId64" free buffers %d in_flight %d"
-mirror_yield_in_flight(void *s, int64_t sector_num, int in_flight) "s %p sector_num %"PRId64" in_flight %d"
+mirror_yield_in_flight(void *s, int64_t offset, int in_flight) "s %p offset %" PRId64 " in_flight %d"

 # block/backup.c
-backup_do_cow_enter(void *job, int64_t start, int64_t sector_num, int nb_sectors) "job %p start %"PRId64" sector_num %"PRId64" nb_sectors %d"
-backup_do_cow_return(void *job, int64_t sector_num, int nb_sectors, int ret) "job %p sector_num %"PRId64" nb_sectors %d ret %d"
+backup_do_cow_enter(void *job, int64_t start, int64_t offset, uint64_t bytes) "job %p start %" PRId64 " offset %" PRId64 " bytes %" PRIu64
+backup_do_cow_return(void *job, int64_t offset, uint64_t bytes, int ret) "job %p offset %" PRId64 " bytes %" PRIu64 " ret %d"
 backup_do_cow_skip(void *job, int64_t start) "job %p start %"PRId64
 backup_do_cow_process(void *job, int64_t start) "job %p start %"PRId64
 backup_do_cow_read_fail(void *job, int64_t start, int ret) "job %p start %"PRId64" ret %d"