linux-kernel - Re: [PATCH V2] block/throttle: Add IO throttled information in blkio.throttle.

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20120901010540.GA19535@dhcp-172-17-108-109.mtv.corp.google.com>
Date:	Fri, 31 Aug 2012 18:05:40 -0700
From:	Tejun Heo <tj@...nel.org>
To:	Tao Ma <tm@....ma>
Cc:	linux-kernel@...r.kernel.org, Vivek Goyal <vgoyal@...hat.com>,
	Jens Axboe <axboe@...nel.dk>
Subject: Re: [PATCH V2] block/throttle: Add IO throttled information in
 blkio.throttle.

On Fri, Aug 31, 2012 at 01:15:09PM +0800, Tao Ma wrote:
> From: Tao Ma <boyu.mt@...bao.com>
> 
> Currently, if the IO is throttled by io-throttle, the SA has no idea of

What's SA?

> the situation and can't report it to the real application user about
> that he/she has to do something. So this patch adds a new interface

Why does the application user "has to" do something?  There's nothing
the upper layer "must" do.  I'm not necessarily objecting to adding
the stat but the description seems a bit misleading.

> named blkio.throttle.io_queued which indicates how many IOs are
> currently throttled.

Also, the suggested stat is rather lacking for such purposes.  There's
no way other than keeping polling to find out the condition, which is
rather sad.  What's the actual use case here?

> Also another function blkg_rwstat_dec is added since the number of throttled
> IOs can be either added or decreased.

Maybe just make blkg_rwstat_add() to take int64_t instead of uint64_t?

> +static void throtl_update_queued_stats(struct throtl_grp *tg, int rw, int add)
> +{
> +	struct tg_stats_cpu *stats_cpu;
> +	unsigned long flags;
> +
> +	/* If per cpu stats are not allocated yet, don't do any accounting. */
> +	if (tg->stats_cpu == NULL)
> +		return;
> +
> +	/*
> +	 * Disabling interrupts to provide mutual exclusion between two
> +	 * writes on same cpu. It probably is not needed for 64bit. Not
> +	 * optimizing that case yet.
> +	 */
> +	local_irq_save(flags);
> +
> +	stats_cpu = this_cpu_ptr(tg->stats_cpu);
> +	if (add)
> +		blkg_rwstat_add(&stats_cpu->io_queued, rw, 1);
> +	else
> +		blkg_rwstat_dec(&stats_cpu->io_queued, rw, 1);
> +
> +	local_irq_restore(flags);

Adding throttle.io_queued could be a bit more consistent?

Thanks.

-- 
tejun
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/