qapi/block-core.json | 73 +++++++++++++++++++++++++++++++++- include/block/accounting.h | 9 +++++ block/accounting.c | 97 ++++++++++++++++++++++++++++++++++++++++++++++ block/qapi.c | 31 +++++++++++++++ blockdev.c | 19 +++++++++ 5 files changed, 228 insertions(+), 1 deletion(-)
v2:
01: add block_latency_histogram_clear()
02: fix spelling (sorry =()
some rewordings
remove histogram if latency parameter unspecified
Vladimir Sementsov-Ogievskiy (2):
block/accounting: introduce latency histogram
qapi: add block latency histogram interface
qapi/block-core.json | 73 +++++++++++++++++++++++++++++++++-
include/block/accounting.h | 9 +++++
block/accounting.c | 97 ++++++++++++++++++++++++++++++++++++++++++++++
block/qapi.c | 31 +++++++++++++++
blockdev.c | 19 +++++++++
5 files changed, 228 insertions(+), 1 deletion(-)
--
2.11.1
Hi,
This series failed build test on ppc host. Please find the details below.
Type: series
Subject: [Qemu-devel] [PATCH v2 0/2] block latency histogram
Message-id: 20180207125037.13510-1-vsementsov@virtuozzo.com
=== TEST SCRIPT BEGIN ===
#!/bin/bash
# Testing script will be invoked under the git checkout with
# HEAD pointing to a commit that has the patches applied on top of "base"
# branch
set -e
echo "=== ENV ==="
env
echo "=== PACKAGES ==="
rpm -qa
echo "=== TEST BEGIN ==="
INSTALL=$PWD/install
BUILD=$PWD/build
mkdir -p $BUILD $INSTALL
SRC=$PWD
cd $BUILD
$SRC/configure --prefix=$INSTALL
make -j100
# XXX: we need reliable clean up
# make check -j100 V=1
make install
=== TEST SCRIPT END ===
Updating 3c8cf5a9c21ff8782164d1def7f44bd888713384
error: RPC failed; result=18, HTTP code = 200
fatal: The remote end hung up unexpectedly
error: Could not fetch 3c8cf5a9c21ff8782164d1def7f44bd888713384
Traceback (most recent call last):
File "/home/patchew/patchew/patchew-cli", line 504, in test_one
git_clone_repo(clone, r["repo"], r["head"], logf)
File "/home/patchew/patchew/patchew-cli", line 48, in git_clone_repo
stdout=logf, stderr=logf)
File "/usr/lib64/python3.4/subprocess.py", line 558, in check_call
raise CalledProcessError(retcode, cmd)
subprocess.CalledProcessError: Command '['git', 'remote', 'add', '-f', '--mirror=fetch', '3c8cf5a9c21ff8782164d1def7f44bd888713384', 'https://github.com/patchew-project/qemu']' returned non-zero exit status 1
---
Email generated automatically by Patchew [http://patchew.org/].
Please send your feedback to patchew-devel@freelists.org
looks strange and unrelated. 07.02.2018 16:20, no-reply@patchew.org wrote: > Hi, > > This series failed build test on ppc host. Please find the details below. > > Type: series > Subject: [Qemu-devel] [PATCH v2 0/2] block latency histogram > Message-id: 20180207125037.13510-1-vsementsov@virtuozzo.com > > === TEST SCRIPT BEGIN === > #!/bin/bash > # Testing script will be invoked under the git checkout with > # HEAD pointing to a commit that has the patches applied on top of "base" > # branch > set -e > echo "=== ENV ===" > env > echo "=== PACKAGES ===" > rpm -qa > echo "=== TEST BEGIN ===" > INSTALL=$PWD/install > BUILD=$PWD/build > mkdir -p $BUILD $INSTALL > SRC=$PWD > cd $BUILD > $SRC/configure --prefix=$INSTALL > make -j100 > # XXX: we need reliable clean up > # make check -j100 V=1 > make install > === TEST SCRIPT END === > > Updating 3c8cf5a9c21ff8782164d1def7f44bd888713384 > error: RPC failed; result=18, HTTP code = 200 > fatal: The remote end hung up unexpectedly > error: Could not fetch 3c8cf5a9c21ff8782164d1def7f44bd888713384 > Traceback (most recent call last): > File "/home/patchew/patchew/patchew-cli", line 504, in test_one > git_clone_repo(clone, r["repo"], r["head"], logf) > File "/home/patchew/patchew/patchew-cli", line 48, in git_clone_repo > stdout=logf, stderr=logf) > File "/usr/lib64/python3.4/subprocess.py", line 558, in check_call > raise CalledProcessError(retcode, cmd) > subprocess.CalledProcessError: Command '['git', 'remote', 'add', '-f', '--mirror=fetch', '3c8cf5a9c21ff8782164d1def7f44bd888713384', 'https://github.com/patchew-project/qemu']' returned non-zero exit status 1 > > > > --- > Email generated automatically by Patchew [http://patchew.org/]. > Please send your feedback to patchew-devel@freelists.org -- Best regards, Vladimir
On Wed, Feb 07, 2018 at 03:50:35PM +0300, Vladimir Sementsov-Ogievskiy wrote: > v2: > > 01: add block_latency_histogram_clear() > 02: fix spelling (sorry =() > some rewordings > remove histogram if latency parameter unspecified > > Vladimir Sementsov-Ogievskiy (2): > block/accounting: introduce latency histogram > qapi: add block latency histogram interface > > qapi/block-core.json | 73 +++++++++++++++++++++++++++++++++- > include/block/accounting.h | 9 +++++ > block/accounting.c | 97 ++++++++++++++++++++++++++++++++++++++++++++++ > block/qapi.c | 31 +++++++++++++++ > blockdev.c | 19 +++++++++ > 5 files changed, 228 insertions(+), 1 deletion(-) The feature looks good. I posted documentation and code readability suggestions. Stefan
On Tue, Mar 06, 2018 at 16:00:17 +0000, Stefan Hajnoczi wrote: > On Wed, Feb 07, 2018 at 03:50:35PM +0300, Vladimir Sementsov-Ogievskiy wrote: > > Vladimir Sementsov-Ogievskiy (2): > > block/accounting: introduce latency histogram > > qapi: add block latency histogram interface (snip) > > 5 files changed, 228 insertions(+), 1 deletion(-) > > The feature looks good. I posted documentation and code readability > suggestions. Hi Vladimir, Did you consider using qdist? For reference, see commit bf3afd5f419 and/or grep 'qdist'. Thanks, Emilio
Hi Emilio! Looked through qdist, if I understand correctly, it saves each added (with different value) element. It is not effective for disk io timing - we'll have too much elements. In my approach, histogram don't grow, it initially have several ranges and counts hits to each range. 06.03.2018 20:49, Emilio G. Cota wrote: > On Tue, Mar 06, 2018 at 16:00:17 +0000, Stefan Hajnoczi wrote: >> On Wed, Feb 07, 2018 at 03:50:35PM +0300, Vladimir Sementsov-Ogievskiy wrote: >>> Vladimir Sementsov-Ogievskiy (2): >>> block/accounting: introduce latency histogram >>> qapi: add block latency histogram interface > (snip) >>> 5 files changed, 228 insertions(+), 1 deletion(-) >> The feature looks good. I posted documentation and code readability >> suggestions. > Hi Vladimir, > > Did you consider using qdist? For reference, see commit bf3afd5f419 > and/or grep 'qdist'. > > Thanks, > > Emilio -- Best regards, Vladimir
On Thu, Mar 08, 2018 at 14:42:29 +0300, Vladimir Sementsov-Ogievskiy wrote:
> Hi Emilio!
>
> Looked through qdist, if I understand correctly, it saves each added (with
> different value) element. It is not effective for disk io timing - we'll
> have too much elements. In my approach, histogram don't grow, it initially
> have several ranges and counts hits to each range.
I thought about this use case, i.e. having a gazillion elements.
You should just do some pre-binning before inserting samples
into qdist, as pointed out in this comment in qdist.h:
/*
* Samples with the same 'x value' end up in the same qdist_entry,
* e.g. inc(0.1) and inc(0.1) end up as {x=0.1, count=2}.
*
* Binning happens only at print time, so that we retain the flexibility to
* choose the binning. This might not be ideal for workloads that do not care
* much about precision and insert many samples all with different x values;
* in that case, pre-binning (e.g. entering both 0.115 and 0.097 as 0.1)
* should be considered.
*/
struct qdist_entry {
double x;
unsigned long count;
};
Let me know if you need help with that.
Thanks,
Emilio
08.03.2018 21:56, Emilio G. Cota wrote:
> On Thu, Mar 08, 2018 at 14:42:29 +0300, Vladimir Sementsov-Ogievskiy wrote:
>> Hi Emilio!
>>
>> Looked through qdist, if I understand correctly, it saves each added (with
>> different value) element. It is not effective for disk io timing - we'll
>> have too much elements. In my approach, histogram don't grow, it initially
>> have several ranges and counts hits to each range.
> I thought about this use case, i.e. having a gazillion elements.
> You should just do some pre-binning before inserting samples
> into qdist, as pointed out in this comment in qdist.h:
>
> /*
> * Samples with the same 'x value' end up in the same qdist_entry,
> * e.g. inc(0.1) and inc(0.1) end up as {x=0.1, count=2}.
> *
> * Binning happens only at print time, so that we retain the flexibility to
> * choose the binning. This might not be ideal for workloads that do not care
> * much about precision and insert many samples all with different x values;
> * in that case, pre-binning (e.g. entering both 0.115 and 0.097 as 0.1)
> * should be considered.
> */
> struct qdist_entry {
> double x;
> unsigned long count;
> };
>
> Let me know if you need help with that.
>
> Thanks,
>
> Emilio
In this case, I'll have to do same bin search (and store same interval
settings) as I already do, on my part, to calculate a parameter for
qdist interface. And I'll have store almost all same data on my part.
So, it doesn't really help. And I need nothing of qdist benefits: I
don't need (and don't want) dynamic allocation of bins on adding an
element or any type of visualization.
--
Best regards,
Vladimir
On Thu, Mar 08, 2018 at 22:07:35 +0300, Vladimir Sementsov-Ogievskiy wrote: > 08.03.2018 21:56, Emilio G. Cota wrote: > > * Binning happens only at print time, so that we retain the flexibility to > > * choose the binning. This might not be ideal for workloads that do not care > > * much about precision and insert many samples all with different x values; > > * in that case, pre-binning (e.g. entering both 0.115 and 0.097 as 0.1) > > * should be considered. (snip) > In this case, I'll have to do same bin search (and store same interval > settings) as I already do, on my part, to calculate a parameter for qdist > interface. And I'll have store almost all same data on my part. So, it > doesn't really help. And I need nothing of qdist benefits: I don't need (and > don't want) dynamic allocation of bins on adding an element or any type of > visualization. I see. You require a couple of features that qdist doesn't yet support: - Arbitrarily-sized, pre-defined bins. - Support for querying the data programmatically instead of just printing it out. We could circumvent the first missing feature with pre-binning, but in that case we'd do a bsearch twice as you point out (BTW your concern about memory allocation wouldn't apply though). The second missing feature should be easy to add to qdist. That said, given that you want this in for 2.12, I'd go with your approach for now. In the future we should look into supporting your use case in qdist, since it is likely that there will be more users with a similar need. Thanks, Emilio
On 03/06/2018 10:00 AM, Stefan Hajnoczi wrote: > On Wed, Feb 07, 2018 at 03:50:35PM +0300, Vladimir Sementsov-Ogievskiy wrote: >> v2: >> >> 01: add block_latency_histogram_clear() >> 02: fix spelling (sorry =() >> some rewordings >> remove histogram if latency parameter unspecified >> >> Vladimir Sementsov-Ogievskiy (2): >> block/accounting: introduce latency histogram >> qapi: add block latency histogram interface >> >> qapi/block-core.json | 73 +++++++++++++++++++++++++++++++++- >> include/block/accounting.h | 9 +++++ >> block/accounting.c | 97 ++++++++++++++++++++++++++++++++++++++++++++++ >> block/qapi.c | 31 +++++++++++++++ >> blockdev.c | 19 +++++++++ >> 5 files changed, 228 insertions(+), 1 deletion(-) > > The feature looks good. I posted documentation and code readability > suggestions. > I also think it makes sense; if a v3 hits the list in time, I will probably include it in my last QAPI pull before softfreeze. -- Eric Blake, Principal Software Engineer Red Hat, Inc. +1-919-301-3266 Virtualization: qemu.org | libvirt.org
ping 07.02.2018 15:50, Vladimir Sementsov-Ogievskiy wrote: > v2: > > 01: add block_latency_histogram_clear() > 02: fix spelling (sorry =() > some rewordings > remove histogram if latency parameter unspecified > > Vladimir Sementsov-Ogievskiy (2): > block/accounting: introduce latency histogram > qapi: add block latency histogram interface > > qapi/block-core.json | 73 +++++++++++++++++++++++++++++++++- > include/block/accounting.h | 9 +++++ > block/accounting.c | 97 ++++++++++++++++++++++++++++++++++++++++++++++ > block/qapi.c | 31 +++++++++++++++ > blockdev.c | 19 +++++++++ > 5 files changed, 228 insertions(+), 1 deletion(-) > -- Best regards, Vladimir
ping 07.02.2018 15:50, Vladimir Sementsov-Ogievskiy wrote: > v2: > > 01: add block_latency_histogram_clear() > 02: fix spelling (sorry =() > some rewordings > remove histogram if latency parameter unspecified > > Vladimir Sementsov-Ogievskiy (2): > block/accounting: introduce latency histogram > qapi: add block latency histogram interface > > qapi/block-core.json | 73 +++++++++++++++++++++++++++++++++- > include/block/accounting.h | 9 +++++ > block/accounting.c | 97 ++++++++++++++++++++++++++++++++++++++++++++++ > block/qapi.c | 31 +++++++++++++++ > blockdev.c | 19 +++++++++ > 5 files changed, 228 insertions(+), 1 deletion(-) > -- Best regards, Vladimir
© 2016 - 2024 Red Hat, Inc.