Re: [PATCH V4 11/15] blk-throttle: add interface to configure think time threshold

[Shaohua Li]

On Mon, Nov 28, 2016 at 05:08:18PM -0500, Tejun Heo wrote:
> Hello, Shaohua.
> 
> On Wed, Nov 23, 2016 at 05:06:30PM -0800, Shaohua Li wrote:
> > > Shouldn't this be a per-cgroup setting along with latency target?
> > > These two are the parameters which define how the cgroup should be
> > > treated time-wise.
> > 
> > It should be easy to make it per-cgroup. Just not sure if it should be
> > per-cgroup. The logic is if the disk is faster, wait time should be shorter 
> > to
> > not harm performance. So it sounds like a per-disk characteristic.
> 
> Yes, this is something dependent on the device, but also on the
> workload.  For both this parameter and the latency target, it seems
> that they should be specified along with the actual device limits so
> that they follow the same convention and can be specified per cgroup *
> block device.  What do you think?

That's ok, I'm totally fine to make it per cgroup and per disk.

Thanks,
Shaohua
--
To unsubscribe from this list: send the line "unsubscribe linux-block" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH V4 11/15] blk-throttle: add interface to configure think time threshold

[Tejun Heo]

Hello, Shaohua.

On Wed, Nov 23, 2016 at 05:06:30PM -0800, Shaohua Li wrote:
> > Shouldn't this be a per-cgroup setting along with latency target?
> > These two are the parameters which define how the cgroup should be
> > treated time-wise.
> 
> It should be easy to make it per-cgroup. Just not sure if it should be
> per-cgroup. The logic is if the disk is faster, wait time should be shorter to
> not harm performance. So it sounds like a per-disk characteristic.

Yes, this is something dependent on the device, but also on the
workload.  For both this parameter and the latency target, it seems
that they should be specified along with the actual device limits so
that they follow the same convention and can be specified per cgroup *
block device.  What do you think?

Thanks.

-- 
tejun
--
To unsubscribe from this list: send the line "unsubscribe linux-block" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH V4 11/15] blk-throttle: add interface to configure think time threshold

[Tejun Heo]

On Mon, Nov 14, 2016 at 02:22:18PM -0800, Shaohua Li wrote:
> Add interface to configure the threshold
> 
> Signed-off-by: Shaohua Li 
> ---
>  block/blk-sysfs.c|  7 +++
>  block/blk-throttle.c | 25 +
>  block/blk.h  |  4 
>  3 files changed, 36 insertions(+)
> 
> diff --git a/block/blk-sysfs.c b/block/blk-sysfs.c
> index 3e284e4..f15aeed 100644
> --- a/block/blk-sysfs.c
> +++ b/block/blk-sysfs.c
> @@ -532,6 +532,12 @@ static struct queue_sysfs_entry throtl_slice_entry = {
>   .show = blk_throtl_slice_show,
>   .store = blk_throtl_slice_store,
>  };
> +
> +static struct queue_sysfs_entry throtl_idle_threshold_entry = {
> + .attr = {.name = "throttling_idle_threshold", .mode = S_IRUGO | S_IWUSR 
> },
> + .show = blk_throtl_idle_threshold_show,
> + .store = blk_throtl_idle_threshold_store,
> +};

Shouldn't this be a per-cgroup setting along with latency target?
These two are the parameters which define how the cgroup should be
treated time-wise.

Thanks.

-- 
tejun
--
To unsubscribe from this list: send the line "unsubscribe linux-block" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html