[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH 2/2] block: add logging facility for long standing IO request
From: |
Denis V. Lunev |
Subject: |
Re: [PATCH 2/2] block: add logging facility for long standing IO requests |
Date: |
Wed, 29 Jul 2020 14:06:43 +0300 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:68.0) Gecko/20100101 Thunderbird/68.10.0 |
On 7/29/20 1:30 PM, Stefan Hajnoczi wrote:
> On Fri, Jul 10, 2020 at 08:27:11PM +0300, Denis V. Lunev wrote:
>> +static void block_acct_report_long(BlockAcctStats *stats,
>> + BlockAcctCookie *cookie, int64_t
>> latency_ns)
>> +{
>> + unsigned latency_ms = latency_ns / 1000000;
> Please use SCALE_MS.
sure
>
>> + int64_t start_time_host_s;
>> + char buf[64];
>> + struct tm t;
>> + BlockDriverState *bs;
>> +
>> + if (cookie->type == BLOCK_ACCT_NONE) {
>> + return;
>> + }
>> + if (stats->latency_log_threshold_ms == 0) {
>> + return;
>> + }
>> + if (latency_ms < stats->latency_log_threshold_ms) {
>> + return;
>> + }
>> +
>> + start_time_host_s = cookie->start_time_ns / 1000000000ull;
> Please use NANOSECONDS_PER_SECOND.
sure
>> diff --git a/blockdev.c b/blockdev.c
>> index 31d5eaf6bf..d87260958c 100644
>> --- a/blockdev.c
>> +++ b/blockdev.c
>> @@ -625,7 +625,8 @@ static BlockBackend *blockdev_init(const char *file,
>> QDict *bs_opts,
>>
>> bs->detect_zeroes = detect_zeroes;
>>
>> - block_acct_setup(blk_get_stats(blk), account_invalid,
>> account_failed);
>> + block_acct_setup(blk_get_stats(blk), account_invalid,
>> account_failed,
>> + qemu_opt_get_number(opts, "latency-log-threshold", 0));
>>
>> if (!parse_stats_intervals(blk_get_stats(blk), interval_list,
>> errp)) {
>> blk_unref(blk);
>> @@ -3740,6 +3741,10 @@ QemuOptsList qemu_common_drive_opts = {
>> .type = QEMU_OPT_BOOL,
>> .help = "whether to account for failed I/O operations "
>> "in the statistics",
>> + },{
>> + .name = "latency-log-threshold",
>> + .type = QEMU_OPT_STRING,
> QEMU_OPT_NUMBER?
good catch! we have this parameter on by default (10 seconds) and never
passed it from the command line :)
Thus should we keep this off be default or on? :)
Den