[PATCH] block/compat_ioctl: fix range check in BLKGETSIZE
kernel ulong and compat_ulong_t may not be same width. Use type directly to eliminate mismatches. This would result in truncation rather than EFBIG for 32bit mode for large disks. Signed-off-by: Khazhismel Kumykov <kha...@google.com> --- block/compat_ioctl.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/block/compat_ioctl.c b/block/compat_ioctl.c index 6ca015f92766..3a2c77f07da8 100644 --- a/block/compat_ioctl.c +++ b/block/compat_ioctl.c @@ -388,7 +388,7 @@ long compat_blkdev_ioctl(struct file *file, unsigned cmd, unsigned long arg) return 0; case BLKGETSIZE: size = i_size_read(bdev->bd_inode); - if ((size >> 9) > ~0UL) + if ((size >> 9) > ~((compat_ulong_t)0UL)) return -EFBIG; return compat_put_ulong(arg, size >> 9); -- 2.17.0.484.g0c8726318c-goog smime.p7s Description: S/MIME Cryptographic Signature
Re: [RFC PATCH] blk-throttle: add burst allowance.
On Mon, Dec 18, 2017 at 1:01 PM, Vivek Goyal <vgo...@redhat.com> wrote: > On Mon, Dec 18, 2017 at 12:39:50PM -0800, Khazhismel Kumykov wrote: >> On Mon, Dec 18, 2017 at 10:29 AM, Vivek Goyal <vgo...@redhat.com> wrote: >> > On Mon, Dec 18, 2017 at 10:16:02AM -0800, Khazhismel Kumykov wrote: >> >> On Mon, Nov 20, 2017 at 8:36 PM, Khazhismel Kumykov <kha...@google.com> >> >> wrote: >> >> > On Fri, Nov 17, 2017 at 11:26 AM, Shaohua Li <s...@kernel.org> wrote: >> >> >> On Thu, Nov 16, 2017 at 08:25:58PM -0800, Khazhismel Kumykov wrote: >> >> >>> On Thu, Nov 16, 2017 at 8:50 AM, Shaohua Li <s...@kernel.org> wrote: >> >> >>> > On Tue, Nov 14, 2017 at 03:10:22PM -0800, Khazhismel Kumykov wrote: >> >> >>> >> Allows configuration additional bytes or ios before a throttle is >> >> >>> >> triggered. >> >> >>> >> >> >> >>> >> This allows implementation of a bucket style rate-limit/throttle >> >> >>> >> on a >> >> >>> >> block device. Previously, bursting to a device was limited to >> >> >>> >> allowance >> >> >>> >> granted in a single throtl_slice (similar to a bucket with limit N >> >> >>> >> and >> >> >>> >> refill rate N/slice). >> >> >>> >> >> >> >>> >> Additional parameters bytes/io_burst_conf defined for tg, which >> >> >>> >> define a >> >> >>> >> number of bytes/ios that must be depleted before throttling >> >> >>> >> happens. A >> >> >>> >> tg that does not deplete this allowance functions as though it has >> >> >>> >> no >> >> >>> >> configured limits. tgs earn additional allowance at rate defined by >> >> >>> >> bps/iops for the tg. Once a tg has *_disp > *_burst_conf, >> >> >>> >> throttling >> >> >>> >> kicks in. If a tg is idle for a while, it will again have some >> >> >>> >> burst >> >> >>> >> allowance before it gets throttled again. >> >> >>> >> >> >> >>> >> slice_end for a tg is extended until io_disp/byte_disp would fall >> >> >>> >> to 0, >> >> >>> >> when all "used" burst allowance would be earned back. trim_slice >> >> >>> >> still >> >> >>> >> does progress slice_start as before and decrements *_disp as >> >> >>> >> before, and >> >> >>> >> tgs continue to get bytes/ios in throtl_slice intervals. >> >> >>> > >> >> >>> > Can you describe why we need this? It would be great if you can >> >> >>> > describe the >> >> >>> > usage model and an example. Does this work for io.low/io.max or >> >> >>> > both? >> >> >>> > >> >> >>> > Thanks, >> >> >>> > Shaohua >> >> >>> > >> >> >>> >> >> >>> Use case that brought this up was configuring limits for a remote >> >> >>> shared device. Bursting beyond io.max is desired but only for so much >> >> >>> before the limit kicks in, afterwards with sustained usage throughput >> >> >>> is capped. (This proactively avoids remote-side limits). In that case >> >> >>> one would configure in a root container io.max + io.burst, and >> >> >>> configure low/other limits on descendants sharing the resource on the >> >> >>> same node. >> >> >>> >> >> >>> With this patch, so long as tg has not dispatched more than the burst, >> >> >>> no limit is applied at all by that tg, including limit imposed by >> >> >>> io.low in tg_iops_limit, etc. >> >> >> >> >> >> I'd appreciate if you can give more details about the 'why'. >> >> >> 'configuring >> >> >> limits for a remote shared device' doesn't justify the change. >> >> > >> >> > This is to configure a bursty workload (and associated device) with >> >> > known/a
Re: [RFC PATCH] blk-throttle: add burst allowance.
On Mon, Dec 18, 2017 at 10:29 AM, Vivek Goyal <vgo...@redhat.com> wrote: > On Mon, Dec 18, 2017 at 10:16:02AM -0800, Khazhismel Kumykov wrote: >> On Mon, Nov 20, 2017 at 8:36 PM, Khazhismel Kumykov <kha...@google.com> >> wrote: >> > On Fri, Nov 17, 2017 at 11:26 AM, Shaohua Li <s...@kernel.org> wrote: >> >> On Thu, Nov 16, 2017 at 08:25:58PM -0800, Khazhismel Kumykov wrote: >> >>> On Thu, Nov 16, 2017 at 8:50 AM, Shaohua Li <s...@kernel.org> wrote: >> >>> > On Tue, Nov 14, 2017 at 03:10:22PM -0800, Khazhismel Kumykov wrote: >> >>> >> Allows configuration additional bytes or ios before a throttle is >> >>> >> triggered. >> >>> >> >> >>> >> This allows implementation of a bucket style rate-limit/throttle on a >> >>> >> block device. Previously, bursting to a device was limited to >> >>> >> allowance >> >>> >> granted in a single throtl_slice (similar to a bucket with limit N and >> >>> >> refill rate N/slice). >> >>> >> >> >>> >> Additional parameters bytes/io_burst_conf defined for tg, which >> >>> >> define a >> >>> >> number of bytes/ios that must be depleted before throttling happens. A >> >>> >> tg that does not deplete this allowance functions as though it has no >> >>> >> configured limits. tgs earn additional allowance at rate defined by >> >>> >> bps/iops for the tg. Once a tg has *_disp > *_burst_conf, throttling >> >>> >> kicks in. If a tg is idle for a while, it will again have some burst >> >>> >> allowance before it gets throttled again. >> >>> >> >> >>> >> slice_end for a tg is extended until io_disp/byte_disp would fall to >> >>> >> 0, >> >>> >> when all "used" burst allowance would be earned back. trim_slice still >> >>> >> does progress slice_start as before and decrements *_disp as before, >> >>> >> and >> >>> >> tgs continue to get bytes/ios in throtl_slice intervals. >> >>> > >> >>> > Can you describe why we need this? It would be great if you can >> >>> > describe the >> >>> > usage model and an example. Does this work for io.low/io.max or both? >> >>> > >> >>> > Thanks, >> >>> > Shaohua >> >>> > >> >>> >> >>> Use case that brought this up was configuring limits for a remote >> >>> shared device. Bursting beyond io.max is desired but only for so much >> >>> before the limit kicks in, afterwards with sustained usage throughput >> >>> is capped. (This proactively avoids remote-side limits). In that case >> >>> one would configure in a root container io.max + io.burst, and >> >>> configure low/other limits on descendants sharing the resource on the >> >>> same node. >> >>> >> >>> With this patch, so long as tg has not dispatched more than the burst, >> >>> no limit is applied at all by that tg, including limit imposed by >> >>> io.low in tg_iops_limit, etc. >> >> >> >> I'd appreciate if you can give more details about the 'why'. 'configuring >> >> limits for a remote shared device' doesn't justify the change. >> > >> > This is to configure a bursty workload (and associated device) with >> > known/allowed expected burst size, but to not allow full utilization >> > of the device for extended periods of time for QoS. During idle or low >> > use periods the burst allowance accrues, and then tasks can burst well >> > beyond the configured throttle up to the limit, afterwards is >> > throttled. A constant throttle speed isn't sufficient for this as you >> > can only burst 1 slice worth, but a limit of sorts is desirable for >> > preventing over utilization of the shared device. This type of limit >> > is also slightly different than what i understand io.low does in local >> > cases in that tg is only high priority/unthrottled if it is bursty, >> > and is limited with constant usage >> > >> > Khazhy >> >> Hi Shaohua, >> >> Does this clarify the reason for this patch? Is this (or something >> similar) a good fit for inclusion in blk-throttle? >> > > So does this brus
Re: [RFC PATCH] blk-throttle: add burst allowance.
On Mon, Nov 20, 2017 at 8:36 PM, Khazhismel Kumykov <kha...@google.com> wrote: > On Fri, Nov 17, 2017 at 11:26 AM, Shaohua Li <s...@kernel.org> wrote: >> On Thu, Nov 16, 2017 at 08:25:58PM -0800, Khazhismel Kumykov wrote: >>> On Thu, Nov 16, 2017 at 8:50 AM, Shaohua Li <s...@kernel.org> wrote: >>> > On Tue, Nov 14, 2017 at 03:10:22PM -0800, Khazhismel Kumykov wrote: >>> >> Allows configuration additional bytes or ios before a throttle is >>> >> triggered. >>> >> >>> >> This allows implementation of a bucket style rate-limit/throttle on a >>> >> block device. Previously, bursting to a device was limited to allowance >>> >> granted in a single throtl_slice (similar to a bucket with limit N and >>> >> refill rate N/slice). >>> >> >>> >> Additional parameters bytes/io_burst_conf defined for tg, which define a >>> >> number of bytes/ios that must be depleted before throttling happens. A >>> >> tg that does not deplete this allowance functions as though it has no >>> >> configured limits. tgs earn additional allowance at rate defined by >>> >> bps/iops for the tg. Once a tg has *_disp > *_burst_conf, throttling >>> >> kicks in. If a tg is idle for a while, it will again have some burst >>> >> allowance before it gets throttled again. >>> >> >>> >> slice_end for a tg is extended until io_disp/byte_disp would fall to 0, >>> >> when all "used" burst allowance would be earned back. trim_slice still >>> >> does progress slice_start as before and decrements *_disp as before, and >>> >> tgs continue to get bytes/ios in throtl_slice intervals. >>> > >>> > Can you describe why we need this? It would be great if you can describe >>> > the >>> > usage model and an example. Does this work for io.low/io.max or both? >>> > >>> > Thanks, >>> > Shaohua >>> > >>> >>> Use case that brought this up was configuring limits for a remote >>> shared device. Bursting beyond io.max is desired but only for so much >>> before the limit kicks in, afterwards with sustained usage throughput >>> is capped. (This proactively avoids remote-side limits). In that case >>> one would configure in a root container io.max + io.burst, and >>> configure low/other limits on descendants sharing the resource on the >>> same node. >>> >>> With this patch, so long as tg has not dispatched more than the burst, >>> no limit is applied at all by that tg, including limit imposed by >>> io.low in tg_iops_limit, etc. >> >> I'd appreciate if you can give more details about the 'why'. 'configuring >> limits for a remote shared device' doesn't justify the change. > > This is to configure a bursty workload (and associated device) with > known/allowed expected burst size, but to not allow full utilization > of the device for extended periods of time for QoS. During idle or low > use periods the burst allowance accrues, and then tasks can burst well > beyond the configured throttle up to the limit, afterwards is > throttled. A constant throttle speed isn't sufficient for this as you > can only burst 1 slice worth, but a limit of sorts is desirable for > preventing over utilization of the shared device. This type of limit > is also slightly different than what i understand io.low does in local > cases in that tg is only high priority/unthrottled if it is bursty, > and is limited with constant usage > > Khazhy Hi Shaohua, Does this clarify the reason for this patch? Is this (or something similar) a good fit for inclusion in blk-throttle? Thanks, Khazhy smime.p7s Description: S/MIME Cryptographic Signature
Re: [RFC PATCH] blk-throttle: add burst allowance.
On Mon, Nov 20, 2017 at 8:36 PM, Khazhismel Kumykov <kha...@google.com> wrote: > On Fri, Nov 17, 2017 at 11:26 AM, Shaohua Li <s...@kernel.org> wrote: >> On Thu, Nov 16, 2017 at 08:25:58PM -0800, Khazhismel Kumykov wrote: >>> On Thu, Nov 16, 2017 at 8:50 AM, Shaohua Li <s...@kernel.org> wrote: >>> > On Tue, Nov 14, 2017 at 03:10:22PM -0800, Khazhismel Kumykov wrote: >>> >> Allows configuration additional bytes or ios before a throttle is >>> >> triggered. >>> >> >>> >> This allows implementation of a bucket style rate-limit/throttle on a >>> >> block device. Previously, bursting to a device was limited to allowance >>> >> granted in a single throtl_slice (similar to a bucket with limit N and >>> >> refill rate N/slice). >>> >> >>> >> Additional parameters bytes/io_burst_conf defined for tg, which define a >>> >> number of bytes/ios that must be depleted before throttling happens. A >>> >> tg that does not deplete this allowance functions as though it has no >>> >> configured limits. tgs earn additional allowance at rate defined by >>> >> bps/iops for the tg. Once a tg has *_disp > *_burst_conf, throttling >>> >> kicks in. If a tg is idle for a while, it will again have some burst >>> >> allowance before it gets throttled again. >>> >> >>> >> slice_end for a tg is extended until io_disp/byte_disp would fall to 0, >>> >> when all "used" burst allowance would be earned back. trim_slice still >>> >> does progress slice_start as before and decrements *_disp as before, and >>> >> tgs continue to get bytes/ios in throtl_slice intervals. >>> > >>> > Can you describe why we need this? It would be great if you can describe >>> > the >>> > usage model and an example. Does this work for io.low/io.max or both? >>> > >>> > Thanks, >>> > Shaohua >>> > >>> >>> Use case that brought this up was configuring limits for a remote >>> shared device. Bursting beyond io.max is desired but only for so much >>> before the limit kicks in, afterwards with sustained usage throughput >>> is capped. (This proactively avoids remote-side limits). In that case >>> one would configure in a root container io.max + io.burst, and >>> configure low/other limits on descendants sharing the resource on the >>> same node. >>> >>> With this patch, so long as tg has not dispatched more than the burst, >>> no limit is applied at all by that tg, including limit imposed by >>> io.low in tg_iops_limit, etc. >> >> I'd appreciate if you can give more details about the 'why'. 'configuring >> limits for a remote shared device' doesn't justify the change. > > This is to configure a bursty workload (and associated device) with > known/allowed expected burst size, but to not allow full utilization > of the device for extended periods of time for QoS. During idle or low > use periods the burst allowance accrues, and then tasks can burst well > beyond the configured throttle up to the limit, afterwards is > throttled. A constant throttle speed isn't sufficient for this as you > can only burst 1 slice worth, but a limit of sorts is desirable for > preventing over utilization of the shared device. This type of limit > is also slightly different than what i understand io.low does in local > cases in that tg is only high priority/unthrottled if it is bursty, > and is limited with constant usage > > Khazhy Ping this time without html (oops) smime.p7s Description: S/MIME Cryptographic Signature
Re: [RFC PATCH] blk-throttle: add burst allowance.
On Fri, Nov 17, 2017 at 11:26 AM, Shaohua Li <s...@kernel.org> wrote: > On Thu, Nov 16, 2017 at 08:25:58PM -0800, Khazhismel Kumykov wrote: >> On Thu, Nov 16, 2017 at 8:50 AM, Shaohua Li <s...@kernel.org> wrote: >> > On Tue, Nov 14, 2017 at 03:10:22PM -0800, Khazhismel Kumykov wrote: >> >> Allows configuration additional bytes or ios before a throttle is >> >> triggered. >> >> >> >> This allows implementation of a bucket style rate-limit/throttle on a >> >> block device. Previously, bursting to a device was limited to allowance >> >> granted in a single throtl_slice (similar to a bucket with limit N and >> >> refill rate N/slice). >> >> >> >> Additional parameters bytes/io_burst_conf defined for tg, which define a >> >> number of bytes/ios that must be depleted before throttling happens. A >> >> tg that does not deplete this allowance functions as though it has no >> >> configured limits. tgs earn additional allowance at rate defined by >> >> bps/iops for the tg. Once a tg has *_disp > *_burst_conf, throttling >> >> kicks in. If a tg is idle for a while, it will again have some burst >> >> allowance before it gets throttled again. >> >> >> >> slice_end for a tg is extended until io_disp/byte_disp would fall to 0, >> >> when all "used" burst allowance would be earned back. trim_slice still >> >> does progress slice_start as before and decrements *_disp as before, and >> >> tgs continue to get bytes/ios in throtl_slice intervals. >> > >> > Can you describe why we need this? It would be great if you can describe >> > the >> > usage model and an example. Does this work for io.low/io.max or both? >> > >> > Thanks, >> > Shaohua >> > >> >> Use case that brought this up was configuring limits for a remote >> shared device. Bursting beyond io.max is desired but only for so much >> before the limit kicks in, afterwards with sustained usage throughput >> is capped. (This proactively avoids remote-side limits). In that case >> one would configure in a root container io.max + io.burst, and >> configure low/other limits on descendants sharing the resource on the >> same node. >> >> With this patch, so long as tg has not dispatched more than the burst, >> no limit is applied at all by that tg, including limit imposed by >> io.low in tg_iops_limit, etc. > > I'd appreciate if you can give more details about the 'why'. 'configuring > limits for a remote shared device' doesn't justify the change. This is to configure a bursty workload (and associated device) with known/allowed expected burst size, but to not allow full utilization of the device for extended periods of time for QoS. During idle or low use periods the burst allowance accrues, and then tasks can burst well beyond the configured throttle up to the limit, afterwards is throttled. A constant throttle speed isn't sufficient for this as you can only burst 1 slice worth, but a limit of sorts is desirable for preventing over utilization of the shared device. This type of limit is also slightly different than what i understand io.low does in local cases in that tg is only high priority/unthrottled if it is bursty, and is limited with constant usage Khazhy smime.p7s Description: S/MIME Cryptographic Signature
Re: [RFC PATCH] blk-throttle: add burst allowance.
On Thu, Nov 16, 2017 at 8:50 AM, Shaohua Li <s...@kernel.org> wrote:
> On Tue, Nov 14, 2017 at 03:10:22PM -0800, Khazhismel Kumykov wrote:
>> Allows configuration additional bytes or ios before a throttle is
>> triggered.
>>
>> This allows implementation of a bucket style rate-limit/throttle on a
>> block device. Previously, bursting to a device was limited to allowance
>> granted in a single throtl_slice (similar to a bucket with limit N and
>> refill rate N/slice).
>>
>> Additional parameters bytes/io_burst_conf defined for tg, which define a
>> number of bytes/ios that must be depleted before throttling happens. A
>> tg that does not deplete this allowance functions as though it has no
>> configured limits. tgs earn additional allowance at rate defined by
>> bps/iops for the tg. Once a tg has *_disp > *_burst_conf, throttling
>> kicks in. If a tg is idle for a while, it will again have some burst
>> allowance before it gets throttled again.
>>
>> slice_end for a tg is extended until io_disp/byte_disp would fall to 0,
>> when all "used" burst allowance would be earned back. trim_slice still
>> does progress slice_start as before and decrements *_disp as before, and
>> tgs continue to get bytes/ios in throtl_slice intervals.
>
> Can you describe why we need this? It would be great if you can describe the
> usage model and an example. Does this work for io.low/io.max or both?
>
> Thanks,
> Shaohua
>
Use case that brought this up was configuring limits for a remote
shared device. Bursting beyond io.max is desired but only for so much
before the limit kicks in, afterwards with sustained usage throughput
is capped. (This proactively avoids remote-side limits). In that case
one would configure in a root container io.max + io.burst, and
configure low/other limits on descendants sharing the resource on the
same node.
With this patch, so long as tg has not dispatched more than the burst,
no limit is applied at all by that tg, including limit imposed by
io.low in tg_iops_limit, etc.
Khazhy
>> Signed-off-by: Khazhismel Kumykov <kha...@google.com>
>> ---
>> block/Kconfig| 11 +++
>> block/blk-throttle.c | 192
>> +++
>> 2 files changed, 189 insertions(+), 14 deletions(-)
>>
>> diff --git a/block/Kconfig b/block/Kconfig
>> index 28ec55752b68..fbd05b419f93 100644
>> --- a/block/Kconfig
>> +++ b/block/Kconfig
>> @@ -128,6 +128,17 @@ config BLK_DEV_THROTTLING_LOW
>>
>> Note, this is an experimental interface and could be changed someday.
>>
>> +config BLK_DEV_THROTTLING_BURST
>> +bool "Block throttling .burst allowance interface"
>> +depends on BLK_DEV_THROTTLING
>> +default n
>> +---help---
>> +Add .burst allowance for block throttling. Burst allowance allows for
>> +additional unthrottled usage, while still limiting speed for sustained
>> +usage.
>> +
>> +If in doubt, say N.
>> +
>> config BLK_CMDLINE_PARSER
>> bool "Block device command line partition parser"
>> default n
>> diff --git a/block/blk-throttle.c b/block/blk-throttle.c
>> index 96ad32623427..27c084312772 100644
>> --- a/block/blk-throttle.c
>> +++ b/block/blk-throttle.c
>> @@ -157,6 +157,11 @@ struct throtl_grp {
>> /* Number of bio's dispatched in current slice */
>> unsigned int io_disp[2];
>>
>> +#ifdef CONFIG_BLK_DEV_THROTTLING_BURST
>> + uint64_t bytes_burst_conf[2];
>> + unsigned int io_burst_conf[2];
>> +#endif
>> +
>> unsigned long last_low_overflow_time[2];
>>
>> uint64_t last_bytes_disp[2];
>> @@ -507,6 +512,12 @@ static struct blkg_policy_data *throtl_pd_alloc(gfp_t
>> gfp, int node)
>> tg->bps_conf[WRITE][LIMIT_MAX] = U64_MAX;
>> tg->iops_conf[READ][LIMIT_MAX] = UINT_MAX;
>> tg->iops_conf[WRITE][LIMIT_MAX] = UINT_MAX;
>> +#ifdef CONFIG_BLK_DEV_THROTTLING_BURST
>> + tg->bytes_burst_conf[READ] = 0;
>> + tg->bytes_burst_conf[WRITE] = 0;
>> + tg->io_burst_conf[READ] = 0;
>> + tg->io_burst_conf[WRITE] = 0;
>> +#endif
>> /* LIMIT_LOW will have default value 0 */
>>
>> tg->latency_target = DFL_LATENCY_TARGET;
>> @@ -800,6 +811,26 @@ static inline void throtl_start_new_slice(struct
>> throtl_grp *tg, bool rw)
>> tg->slice_end[rw], jiffies);
>> }
>>
>> +/*
>> + * When current slice should end.
>> + *
>> + * With CONFIG_B
Re: [RFC PATCH] blk-throttle: add burst allowance.
(Botched the to line, sorry) smime.p7s Description: S/MIME Cryptographic Signature
