Re: [PATCH 7/7] blk-throtl: don't throttle the same IO multiple times

2017-11-13 Thread Tejun Heo
Hello, On Mon, Nov 13, 2017 at 11:54:13AM -0800, Shaohua Li wrote: > I'm not sure how you are going to make this correct. The mechanism is very > fragile. So for example, 'q->make_request_fn(q, bio)' could just queue the bio > somewhere and handle in other context (both dm and md do this). The

Re: [PATCH 7/7] blk-throtl: don't throttle the same IO multiple times

2017-11-13 Thread Tejun Heo
Hello, On Mon, Nov 13, 2017 at 11:54:13AM -0800, Shaohua Li wrote: > I'm not sure how you are going to make this correct. The mechanism is very > fragile. So for example, 'q->make_request_fn(q, bio)' could just queue the bio > somewhere and handle in other context (both dm and md do this). The

Re: [PATCH 7/7] blk-throtl: don't throttle the same IO multiple times

2017-11-13 Thread Shaohua Li
On Mon, Nov 13, 2017 at 07:57:45AM -0800, Tejun Heo wrote: > Hello, > > On Mon, Nov 13, 2017 at 03:13:48AM -0800, Tejun Heo wrote: > > You're right. If we wanna take this approach, we need to keep the > > throttled flag while cloning. The clearing part is still correct tho. > > Without that, I

Re: [PATCH 7/7] blk-throtl: don't throttle the same IO multiple times

2017-11-13 Thread Shaohua Li
On Mon, Nov 13, 2017 at 07:57:45AM -0800, Tejun Heo wrote: > Hello, > > On Mon, Nov 13, 2017 at 03:13:48AM -0800, Tejun Heo wrote: > > You're right. If we wanna take this approach, we need to keep the > > throttled flag while cloning. The clearing part is still correct tho. > > Without that, I

Re: [PATCH 7/7] blk-throtl: don't throttle the same IO multiple times

2017-11-13 Thread Shaohua Li
On Mon, Nov 13, 2017 at 07:57:45AM -0800, Tejun Heo wrote: > Hello, > > On Mon, Nov 13, 2017 at 03:13:48AM -0800, Tejun Heo wrote: > > You're right. If we wanna take this approach, we need to keep the > > throttled flag while cloning. The clearing part is still correct tho. > > Without that, I

Re: [PATCH 7/7] blk-throtl: don't throttle the same IO multiple times

2017-11-13 Thread Shaohua Li
On Mon, Nov 13, 2017 at 07:57:45AM -0800, Tejun Heo wrote: > Hello, > > On Mon, Nov 13, 2017 at 03:13:48AM -0800, Tejun Heo wrote: > > You're right. If we wanna take this approach, we need to keep the > > throttled flag while cloning. The clearing part is still correct tho. > > Without that, I

Re: [PATCH 7/7] blk-throtl: don't throttle the same IO multiple times

2017-11-13 Thread Tejun Heo
Hello, On Mon, Nov 13, 2017 at 03:13:48AM -0800, Tejun Heo wrote: > You're right. If we wanna take this approach, we need to keep the > throttled flag while cloning. The clearing part is still correct tho. > Without that, I get 1/4 bw limit enforced. Hmm... I'm not quite sure > where that 1/4

Re: [PATCH 7/7] blk-throtl: don't throttle the same IO multiple times

2017-11-13 Thread Tejun Heo
Hello, On Mon, Nov 13, 2017 at 03:13:48AM -0800, Tejun Heo wrote: > You're right. If we wanna take this approach, we need to keep the > throttled flag while cloning. The clearing part is still correct tho. > Without that, I get 1/4 bw limit enforced. Hmm... I'm not quite sure > where that 1/4

Re: [PATCH 7/7] blk-throtl: don't throttle the same IO multiple times

2017-11-13 Thread Tejun Heo
Hello, Shaohua. On Sun, Nov 12, 2017 at 08:07:16PM -0800, Shaohua Li wrote: > Thanks for looking into this while I was absence. I don't understand how this > works. Assume a bio will be splitted into 2 small bios. In > generic_make_request, we charge the whole bio. 'q->make_request_fn' will >

Re: [PATCH 7/7] blk-throtl: don't throttle the same IO multiple times

2017-11-13 Thread Tejun Heo
Hello, Shaohua. On Sun, Nov 12, 2017 at 08:07:16PM -0800, Shaohua Li wrote: > Thanks for looking into this while I was absence. I don't understand how this > works. Assume a bio will be splitted into 2 small bios. In > generic_make_request, we charge the whole bio. 'q->make_request_fn' will >

Re: [PATCH 7/7] blk-throtl: don't throttle the same IO multiple times

2017-11-12 Thread Shaohua Li
On Sun, Nov 12, 2017 at 02:26:13PM -0800, Tejun Heo wrote: > BIO_THROTTLED is used to mark already throttled bios so that a bio > doesn't get throttled multiple times. The flag gets set when the bio > starts getting dispatched from blk-throtl and cleared when it leaves > blk-throtl. > >

Re: [PATCH 7/7] blk-throtl: don't throttle the same IO multiple times

2017-11-12 Thread Shaohua Li
On Sun, Nov 12, 2017 at 02:26:13PM -0800, Tejun Heo wrote: > BIO_THROTTLED is used to mark already throttled bios so that a bio > doesn't get throttled multiple times. The flag gets set when the bio > starts getting dispatched from blk-throtl and cleared when it leaves > blk-throtl. > >

[PATCH 7/7] blk-throtl: don't throttle the same IO multiple times

2017-11-12 Thread Tejun Heo
BIO_THROTTLED is used to mark already throttled bios so that a bio doesn't get throttled multiple times. The flag gets set when the bio starts getting dispatched from blk-throtl and cleared when it leaves blk-throtl. Unfortunately, this doesn't work when the request_queue decides to split or

[PATCH 7/7] blk-throtl: don't throttle the same IO multiple times

2017-11-12 Thread Tejun Heo
BIO_THROTTLED is used to mark already throttled bios so that a bio doesn't get throttled multiple times. The flag gets set when the bio starts getting dispatched from blk-throtl and cleared when it leaves blk-throtl. Unfortunately, this doesn't work when the request_queue decides to split or