Re: [Qemu-devel] [PATCH] block: do not use get_clock()
On Fri, Nov 28, 2014 at 02:41:54PM +0100, Markus Armbruster wrote: Stefan Hajnoczi stefa...@gmail.com writes: On Wed, Nov 26, 2014 at 03:01:02PM +0100, Paolo Bonzini wrote: Use the external qemu-timer API instead. Cc: kw...@redhat.com Cc: stefa...@redhat.com Signed-off-by: Paolo Bonzini pbonz...@redhat.com --- block/accounting.c | 6 -- block/raw-posix.c | 8 2 files changed, 8 insertions(+), 6 deletions(-) Thanks, applied to my block-next tree: https://github.com/stefanha/qemu/commits/block-next Please wait for Paolo's v2 with rationale in the commit message. Got it. Stefan pgpUbpqRyP0Vy.pgp Description: PGP signature
Re: [Qemu-devel] [PATCH] block: do not use get_clock()
Stefan Hajnoczi stefa...@redhat.com writes: On Fri, Nov 28, 2014 at 02:41:54PM +0100, Markus Armbruster wrote: Stefan Hajnoczi stefa...@gmail.com writes: On Wed, Nov 26, 2014 at 03:01:02PM +0100, Paolo Bonzini wrote: Use the external qemu-timer API instead. Cc: kw...@redhat.com Cc: stefa...@redhat.com Signed-off-by: Paolo Bonzini pbonz...@redhat.com --- block/accounting.c | 6 -- block/raw-posix.c | 8 2 files changed, 8 insertions(+), 6 deletions(-) Thanks, applied to my block-next tree: https://github.com/stefanha/qemu/commits/block-next Please wait for Paolo's v2 with rationale in the commit message. Got it. https://github.com/stefanha/qemu/commit/1800094faabbadb017acb15bc0b1bd6dde283f45 looks good, thanks!
Re: [Qemu-devel] [PATCH] block: do not use get_clock()
On Wed, Nov 26, 2014 at 03:01:02PM +0100, Paolo Bonzini wrote: Use the external qemu-timer API instead. Cc: kw...@redhat.com Cc: stefa...@redhat.com Signed-off-by: Paolo Bonzini pbonz...@redhat.com --- block/accounting.c | 6 -- block/raw-posix.c | 8 2 files changed, 8 insertions(+), 6 deletions(-) Thanks, applied to my block-next tree: https://github.com/stefanha/qemu/commits/block-next Stefan pgprTjtwyR1U3.pgp Description: PGP signature
Re: [Qemu-devel] [PATCH] block: do not use get_clock()
Stefan Hajnoczi stefa...@gmail.com writes: On Wed, Nov 26, 2014 at 03:01:02PM +0100, Paolo Bonzini wrote: Use the external qemu-timer API instead. Cc: kw...@redhat.com Cc: stefa...@redhat.com Signed-off-by: Paolo Bonzini pbonz...@redhat.com --- block/accounting.c | 6 -- block/raw-posix.c | 8 2 files changed, 8 insertions(+), 6 deletions(-) Thanks, applied to my block-next tree: https://github.com/stefanha/qemu/commits/block-next Please wait for Paolo's v2 with rationale in the commit message.
Re: [Qemu-devel] [PATCH] block: do not use get_clock()
Paolo Bonzini pbonz...@redhat.com writes: Use the external qemu-timer API instead. Ignorant question: why?
Re: [Qemu-devel] [PATCH] block: do not use get_clock()
On 27/11/2014 10:19, Markus Armbruster wrote: Paolo Bonzini pbonz...@redhat.com writes: Use the external qemu-timer API instead. Ignorant question: why? Because no one else calls it directly, it is an internal function. I want to keep it confined to qemu-timer.c (and possibly cpus.c in the icount implementation, but maybe not even that is necessary). Paolo
Re: [Qemu-devel] [PATCH] block: do not use get_clock()
On Thu, Nov 27, 2014 at 10:19:45AM +0100, Markus Armbruster wrote: Paolo Bonzini pbonz...@redhat.com writes: Use the external qemu-timer API instead. Ignorant question: why? Patch seems fine but I concur with Markus. Let's add the rationale to the commit description. pgpj67g1P8W4L.pgp Description: PGP signature
Re: [Qemu-devel] [PATCH] block: do not use get_clock()
On 27/11/2014 17:44, Stefan Hajnoczi wrote:
On Thu, Nov 27, 2014 at 10:19:45AM +0100, Markus Armbruster wrote:
Paolo Bonzini pbonz...@redhat.com writes:
Use the external qemu-timer API instead.
Ignorant question: why?
Patch seems fine but I concur with Markus. Let's add the rationale to
the commit description.
Like this?
8
From: Paolo Bonzini pbonz...@redhat.com
Subject: [PATCH] block: do not use get_clock()
Use the external qemu-timer API instead.
No one else should be calling cpu_get_clock(), get_clock() and
get_clock_realtime() directly; they are internal functions and they
should be confined to qemu-timer.c and cpus.c (where the icount
implementation resides). All accesses should go through
qemu_clock_get_ns.
Cc: kw...@redhat.com
Cc: stefa...@redhat.com
Signed-off-by: Paolo Bonzini pbonz...@redhat.com
---
block/accounting.c | 6 --
block/raw-posix.c | 8
2 files changed, 8 insertions(+), 6 deletions(-)
diff --git a/block/accounting.c b/block/accounting.c
index edbb1cc..18102f0 100644
--- a/block/accounting.c
+++ b/block/accounting.c
@@ -24,6 +24,7 @@
#include block/accounting.h
#include block/block_int.h
+#include qemu/timer.h
void block_acct_start(BlockAcctStats *stats, BlockAcctCookie *cookie,
int64_t bytes, enum BlockAcctType type)
@@ -31,7 +32,7 @@ void block_acct_start(BlockAcctStats *stats, BlockAcctCookie
*cookie,
assert(type BLOCK_MAX_IOTYPE);
cookie-bytes = bytes;
-cookie-start_time_ns = get_clock();
+cookie-start_time_ns = qemu_clock_get_ns(QEMU_CLOCK_REALTIME);
cookie-type = type;
}
@@ -41,7 +42,8 @@ void block_acct_done(BlockAcctStats *stats, BlockAcctCookie
*cookie)
stats-nr_bytes[cookie-type] += cookie-bytes;
stats-nr_ops[cookie-type]++;
-stats-total_time_ns[cookie-type] += get_clock() - cookie-start_time_ns;
+stats-total_time_ns[cookie-type] +=
+qemu_clock_get_ns(QEMU_CLOCK_REALTIME) - cookie-start_time_ns;
}
diff --git a/block/raw-posix.c b/block/raw-posix.c
index b1af77e..02e107f 100644
--- a/block/raw-posix.c
+++ b/block/raw-posix.c
@@ -1922,7 +1922,7 @@ static int fd_open(BlockDriverState *bs)
return 0;
last_media_present = (s-fd = 0);
if (s-fd = 0
-(get_clock() - s-fd_open_time) = FD_OPEN_TIMEOUT) {
+(qemu_clock_get_ns(QEMU_CLOCK_REALTIME) - s-fd_open_time) =
FD_OPEN_TIMEOUT) {
qemu_close(s-fd);
s-fd = -1;
#ifdef DEBUG_FLOPPY
@@ -1931,7 +1931,7 @@ static int fd_open(BlockDriverState *bs)
}
if (s-fd 0) {
if (s-fd_got_error
-(get_clock() - s-fd_error_time) FD_OPEN_TIMEOUT) {
+(qemu_clock_get_ns(QEMU_CLOCK_REALTIME) - s-fd_error_time)
FD_OPEN_TIMEOUT) {
#ifdef DEBUG_FLOPPY
printf(No floppy (open delayed)\n);
#endif
@@ -1939,7 +1939,7 @@ static int fd_open(BlockDriverState *bs)
}
s-fd = qemu_open(bs-filename, s-open_flags ~O_NONBLOCK);
if (s-fd 0) {
-s-fd_error_time = get_clock();
+s-fd_error_time = qemu_clock_get_ns(QEMU_CLOCK_REALTIME);
s-fd_got_error = 1;
if (last_media_present)
s-fd_media_changed = 1;
@@ -1954,7 +1954,7 @@ static int fd_open(BlockDriverState *bs)
}
if (!last_media_present)
s-fd_media_changed = 1;
-s-fd_open_time = get_clock();
+s-fd_open_time = qemu_clock_get_ns(QEMU_CLOCK_REALTIME);
s-fd_got_error = 0;
return 0;
}
-- 1.8.3.1
Re: [Qemu-devel] [PATCH] block: do not use get_clock()
Paolo Bonzini pbonz...@redhat.com writes: On 27/11/2014 10:19, Markus Armbruster wrote: Paolo Bonzini pbonz...@redhat.com writes: Use the external qemu-timer API instead. Ignorant question: why? Because no one else calls it directly, it is an internal function. I want to keep it confined to qemu-timer.c (and possibly cpus.c in the icount implementation, but maybe not even that is necessary). That's a perfectly sensible reason. Please add it to the commit message, so we have a more permanent record. Of course, the best permanent record would be a static keyword :)
Re: [Qemu-devel] [PATCH] block: do not use get_clock()
On 27/11/2014 18:45, Markus Armbruster wrote: Because no one else calls it directly, it is an internal function. I want to keep it confined to qemu-timer.c (and possibly cpus.c in the icount implementation, but maybe not even that is necessary). That's a perfectly sensible reason. Please add it to the commit message, so we have a more permanent record. Of course, the best permanent record would be a static keyword :) That's on the todo list. :) I'll send v2 as a toplevel patch if I don't hear from Kevin or Stefan in a few days. This is 2.3 work so there's time. Paolo
