| Message ID | 52aae27038728bf0fd1b2b3b6536fcc28c9f2e6c.1479161136.git.shli@fb.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Mon, Nov 14, 2016 at 02:22:18PM -0800, Shaohua Li wrote: > Add interface to configure the threshold > > Signed-off-by: Shaohua Li <shli@fb.com> > --- > 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.
On Wed, Nov 23, 2016 at 04:32:43PM -0500, Tejun Heo wrote: > On Mon, Nov 14, 2016 at 02:22:18PM -0800, Shaohua Li wrote: > > Add interface to configure the threshold > > > > Signed-off-by: Shaohua Li <shli@fb.com> > > --- > > 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. 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. Thanks, Shaohua -- To unsubscribe from this list: send the line "unsubscribe linux-block" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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.
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 majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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, +}; #endif static struct attribute *default_attrs[] = { @@ -563,6 +569,7 @@ static struct attribute *default_attrs[] = { &queue_dax_entry.attr, #ifdef CONFIG_BLK_DEV_THROTTLING &throtl_slice_entry.attr, + &throtl_idle_threshold_entry.attr, #endif NULL, }; diff --git a/block/blk-throttle.c b/block/blk-throttle.c index cb5fd85..e403e88 100644 --- a/block/blk-throttle.c +++ b/block/blk-throttle.c @@ -2139,6 +2139,31 @@ ssize_t blk_throtl_slice_store(struct request_queue *q, return count; } +ssize_t blk_throtl_idle_threshold_show(struct request_queue *q, char *page) +{ + u64 threshold = q->td->idle_ttime_threshold; + + if (!q->td) + return -EINVAL; + do_div(threshold, 1000); + return sprintf(page, "%lluus\n", threshold); +} + +ssize_t blk_throtl_idle_threshold_store(struct request_queue *q, + const char *page, size_t count) +{ + unsigned long v; + + if (!q->td) + return -EINVAL; + if (kstrtoul(page, 10, &v)) + return -EINVAL; + if (v == 0) + return -EINVAL; + q->td->idle_ttime_threshold = v * 1000; + return count; +} + static int __init throtl_init(void) { kthrotld_workqueue = alloc_workqueue("kthrotld", WQ_MEM_RECLAIM, 0); diff --git a/block/blk.h b/block/blk.h index b433f35..2ebde12 100644 --- a/block/blk.h +++ b/block/blk.h @@ -292,6 +292,10 @@ extern void blk_throtl_exit(struct request_queue *q); extern ssize_t blk_throtl_slice_show(struct request_queue *q, char *page); extern ssize_t blk_throtl_slice_store(struct request_queue *q, const char *page, size_t count); +extern ssize_t blk_throtl_idle_threshold_show(struct request_queue *q, + char *page); +extern ssize_t blk_throtl_idle_threshold_store(struct request_queue *q, + const char *page, size_t count); extern void blk_throtl_bio_endio(struct bio *bio); #else /* CONFIG_BLK_DEV_THROTTLING */ static inline void blk_throtl_drain(struct request_queue *q) { }
Add interface to configure the threshold Signed-off-by: Shaohua Li <shli@fb.com> --- block/blk-sysfs.c | 7 +++++++ block/blk-throttle.c | 25 +++++++++++++++++++++++++ block/blk.h | 4 ++++ 3 files changed, 36 insertions(+)