Re: [PATCH V4 09/15] blk-throttle: make bandwidth change smooth

[Shaohua Li]

On Wed, Nov 23, 2016 at 04:23:35PM -0500, Tejun Heo wrote:
> Hello,
> 
> On Mon, Nov 14, 2016 at 02:22:16PM -0800, Shaohua Li wrote:
> > cg1/cg2 bps: 10/80 -> 15/105 -> 20/100 -> 25/95 -> 30/90 -> 35/85 -> 40/80
> > -> 45/75 -> 10/80
> 
> I wonder whether it'd make sense to make the clamping down gradual too
> (way faster than the ramping up but still gradual).  The control model
> isn't immediate anyway and doing the other direction gradually too
> might not hurt the overall control accuracy all that much.

Sure, that's in my todo list. Part of the reason is I didn't figure out an easy
to do it. I'll keep trying later.

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 10/15] blk-throttle: add a simple idle detection

[Tejun Heo]

Hello, Shaohua.

On Mon, Nov 14, 2016 at 02:22:17PM -0800, Shaohua Li wrote:
> Unfortunately it's very hard to determine if a cgroup is real idle. This
> patch uses the 'think time check' idea from CFQ for the purpose. Please
> note, the idea doesn't work for all workloads. For example, a workload
> with io depth 8 has disk utilization 100%, hence think time is 0, eg,
> not idle. But the workload can run higher bandwidth with io depth 16.
> Compared to io depth 16, the io depth 8 workload is idle. We use the
> idea to roughly determine if a cgroup is idle.

Hmm... I'm not sure thinktime is the best measure here.  Think time is
used by cfq mainly to tell the likely future behavior of a workload so
that cfq can take speculative actions on the prediction.  However,
given that the implemented high limit behavior tries to provide a
certain level of latency target, using the predictive thinktime to
regulate behavior might lead to too unpredictable behaviors.

Moreover, I don't see why we need to bother with predictions anyway.
cfq needed it but I don't think that's the case for blk-throtl.  It
can just provide idle threshold where a cgroup which hasn't issued an
IO over that threshold is considered idle.  That'd be a lot easier to
understand and configure from userland while providing a good enough
mechanism to prevent idle cgroups from clamping down utilization for
too long.

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

Re: [PATCH V4 07/15] blk-throttle: make throtl_slice tunable

[Tejun Heo]

Hello,

On Tue, Nov 22, 2016 at 03:18:24PM -0800, Shaohua Li wrote:
> Hmm, it's not a real 'time slice'. The name is a bit confusion. Maybe rename 
> it
> to 'throtl_interval' or 'throtl_sampling_time'? not sure. bandwidth and iops
> are always in terms of a time interval we measure them. We can't say the
> iops/bw for a single io. So this is really a tuable knob.

Yeah, maybe using a more indicative name is better.  However, even if
we say that this is the sampling period, it's not clear how adjusting
the knob would affect the behavior as that's not something clearly
defined in blk-throtl's operation model.  For contrast, compare it
with the latency target, the implemented behavior might not succeed to
follow the intended configuration perfectly but what the intention of
the configuration is clear regardless.

That said, if this needs to be a tunable knob, it's fine to have it,
but let's at least try to document what the effects of changing the
variable is.

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 0/8] lightnvm: simplify media manager V2

[Javier González]

> On 23 Nov 2016, at 13.06, Matias Bjørling  wrote:
> 
> On 11/21/2016 01:10 PM, Javier González wrote:
>> V2:
>>  - Fix 2 bad memory free on error rrpc init error handling. Reported by
>>kbuild.
>> 
>> This patchset simplifies the generic media manager interface and moves
>> lun and block functionality to lightnvm targets. This makes that
>> rrpc-specific functionality is not exposed on the media manager. This is
>> done in preparation for the pblk target.
>> 
>> Javier González (8):
>>  lightnvm: move block provisioning to targets
>>  lightnvm: remove get_lun operation on gennvm
>>  lightnvm: remove debug lun statistics from gennvm
>>  lightnvm: eliminate nvm_block abstraction on mm
>>  lightnvm: eliminate nvm_lun abstraction in mm
>>  lightnvm: introduce helpers for generic ops in rrpc
>>  lightnvm: introduce max_phys_sects helper function
>>  lightnvm: use target nvm on target-specific ops.
>> 
>> drivers/lightnvm/core.c  | 154 ++-
>> drivers/lightnvm/gennvm.c| 612 
>> ---
>> drivers/lightnvm/gennvm.h|  20 +-
>> drivers/lightnvm/rrpc.c  | 443 +--
>> drivers/lightnvm/rrpc.h  |  62 -
>> drivers/lightnvm/sysblk.c|  65 +++--
>> drivers/nvme/host/lightnvm.c |  14 +-
>> include/linux/lightnvm.h | 217 +++
>> 8 files changed, 885 insertions(+), 702 deletions(-)
> 
> Thanks Javier. Picked up for 4.10. Please note I fixed up an
> initialization bug with null_blk device in the "lightnvm: introduce
> helpers for generic ops in rrpc" patch.

Great. Thanks!

Javier--
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