On Mon, Feb 24, 2025 at 08:03:32PM +0800, Yu Kuai wrote: > Hi, > > 在 2025/02/24 16:56, Ming Lei 写道: > > On Mon, Feb 24, 2025 at 03:03:18PM +0800, Yu Kuai wrote: > > > Hi, Ming! > > > > > > 在 2025/02/24 11:28, Ming Lei 写道: > > > > throtl_trim_slice() returns immediately if throtl_slice_used() > > > > is true. > > > > > > > > And throtl_slice_used() checks jiffies in [start, end] via time_in_range(), > > > > so if `start <= jiffies <= end', it still returns false. > > > > > > Yes, I misread the code, by thinking throtl_slice_used() will return > > > true if the slice is still used. :( > > > > > > > > > > > BTW, throtl_trim_slice() looks like problematic: > > > > > > > > > > - if (bytes_trim <= 0 && io_trim <= 0) > > > > > + if (bytes_trim <= 0 || io_trim <= 0 || > > > > > + tg->bytes_disp[rw] < bytes_trim || tg->io_disp[rw] < io_trim) > > > > > return; > > > > That is exactly what my patch is doing, just taking deviation and > > > > timeout into account, also U64_MAX limit has to be excluded. > > > Yes, perhaps you can add some comments in the last two conditions of > > > your patch. > > > > Yes, we need to add comment on the check, how about the following words? > > > > ``` > > > > If actually rate doesn't match with expected rate, do not trim slice > > otherwise the present rate control info is lost, we don't have chance > > to compensate it in the following period of this slice any more. > > So, I just give your patch a test, and result is 1.3s while 1s is > expected. While debuging, a new idea come up in mind. :) > > How about keep at least one slice out of consideration from > throtl_trim_slice()? With following patch, the result is between > 1.01-1.03s in my VM. That is easy to get the same result with the approach I suggested, another big benefit: it is adaptive, and blk-throttle may get simplified. > > diff --git a/block/blk-throttle.c b/block/blk-throttle.c > index 8d149aff9fd0..5207c85098a5 100644 > --- a/block/blk-throttle.c > +++ b/block/blk-throttle.c > @@ -604,9 +604,12 @@ static inline void throtl_trim_slice(struct throtl_grp > *tg, bool rw) > > time_elapsed = rounddown(jiffies - tg->slice_start[rw], > tg->td->throtl_slice); > - if (!time_elapsed) > + /* don't trim slice until at least 2 slice is used */ > + if (time_elapsed < tg->td->throtl_slice * 2) > return; If you just want to fix throtl/001, the above patch might work(sometimes, it might not, and timer may expire by 2 jiffies), but it is easy to fail other tests, such as, reduce the bps limit a bit, and increase BS a bit to make the IO cross exactly two slices. Also the big question is that how you can make sure that rate is always good when the window is >= 2 slice? Thanks, Ming
