* [Qemu-devel] bug introduced by "block: Move throttling fields from BDS to BB"
@ 2016-10-14 14:11 Paolo Bonzini
2016-10-14 19:29 ` Paolo Bonzini
2016-10-17 8:49 ` Alberto Garcia
0 siblings, 2 replies; 4+ messages in thread
From: Paolo Bonzini @ 2016-10-14 14:11 UTC (permalink / raw)
To: qemu-devel, qemu block, Kevin Wolf, Alberto Garcia
Here is next_throttle_token:
- ThrottleGroup *tg = container_of(blk_bs(blk)->throttle_state,
- ThrottleGroup, ts);
+ BlockBackendPublic *blkp = blk_get_public(blk);
+ ThrottleGroup *tg = container_of(blkp->throttle_state, ThrottleGroup, ts);
BlockBackend *token, *start;
start = token = tg->tokens[is_write];
/* get next bs round in round robin style */
token = throttle_group_next_blk(token);
- while (token != start && !blk_bs(token)->pending_reqs[is_write]) {
+ while (token != start && !blkp->pending_reqs[is_write]) {
token = throttle_group_next_blk(token);
}
blkp isn't updated every time token is updated.
The same bug occurs in schedule_next_request. This patch was only
in 2.7.x.
Thanks,
Paolo
^ permalink raw reply [flat|nested] 4+ messages in thread* Re: [Qemu-devel] bug introduced by "block: Move throttling fields from BDS to BB"
2016-10-14 14:11 [Qemu-devel] bug introduced by "block: Move throttling fields from BDS to BB" Paolo Bonzini
@ 2016-10-14 19:29 ` Paolo Bonzini
2016-10-17 8:49 ` Alberto Garcia
1 sibling, 0 replies; 4+ messages in thread
From: Paolo Bonzini @ 2016-10-14 19:29 UTC (permalink / raw)
To: qemu-devel, qemu block, Kevin Wolf, Alberto Garcia
On 14/10/2016 16:11, Paolo Bonzini wrote:
> - ThrottleGroup *tg = container_of(blk_bs(blk)->throttle_state,
> - ThrottleGroup, ts);
> + BlockBackendPublic *blkp = blk_get_public(blk);
> + ThrottleGroup *tg = container_of(blkp->throttle_state, ThrottleGroup, ts);
> BlockBackend *token, *start;
>
> start = token = tg->tokens[is_write];
>
> /* get next bs round in round robin style */
> token = throttle_group_next_blk(token);
> - while (token != start && !blk_bs(token)->pending_reqs[is_write]) {
> + while (token != start && !blkp->pending_reqs[is_write]) {
> token = throttle_group_next_blk(token);
> }
>
>
> blkp isn't updated every time token is updated.
BTW, the simplest fix is probably to introduce a function
static inline bool blk_has_pending_reqs(BlockBackend *blk,
bool is_write)
Paolo
^ permalink raw reply [flat|nested] 4+ messages in thread* Re: [Qemu-devel] bug introduced by "block: Move throttling fields from BDS to BB"
2016-10-14 14:11 [Qemu-devel] bug introduced by "block: Move throttling fields from BDS to BB" Paolo Bonzini
2016-10-14 19:29 ` Paolo Bonzini
@ 2016-10-17 8:49 ` Alberto Garcia
2016-10-17 11:18 ` Kevin Wolf
1 sibling, 1 reply; 4+ messages in thread
From: Alberto Garcia @ 2016-10-17 8:49 UTC (permalink / raw)
To: Paolo Bonzini, qemu-devel, qemu block, Kevin Wolf
On Fri 14 Oct 2016 04:11:46 PM CEST, Paolo Bonzini wrote:
> Here is next_throttle_token:
>
> - ThrottleGroup *tg = container_of(blk_bs(blk)->throttle_state,
> - ThrottleGroup, ts);
> + BlockBackendPublic *blkp = blk_get_public(blk);
> + ThrottleGroup *tg = container_of(blkp->throttle_state, ThrottleGroup, ts);
> BlockBackend *token, *start;
>
> start = token = tg->tokens[is_write];
>
> /* get next bs round in round robin style */
> token = throttle_group_next_blk(token);
> - while (token != start && !blk_bs(token)->pending_reqs[is_write]) {
> + while (token != start && !blkp->pending_reqs[is_write]) {
> token = throttle_group_next_blk(token);
> }
>
>
> blkp isn't updated every time token is updated.
You're right, I'll write a patch. I'd also try to check why this was not
detected by any iotest.
Thanks!
Berto
^ permalink raw reply [flat|nested] 4+ messages in thread* Re: [Qemu-devel] bug introduced by "block: Move throttling fields from BDS to BB"
2016-10-17 8:49 ` Alberto Garcia
@ 2016-10-17 11:18 ` Kevin Wolf
0 siblings, 0 replies; 4+ messages in thread
From: Kevin Wolf @ 2016-10-17 11:18 UTC (permalink / raw)
To: Alberto Garcia; +Cc: Paolo Bonzini, qemu-devel, qemu block
Am 17.10.2016 um 10:49 hat Alberto Garcia geschrieben:
> On Fri 14 Oct 2016 04:11:46 PM CEST, Paolo Bonzini wrote:
> > Here is next_throttle_token:
> >
> > - ThrottleGroup *tg = container_of(blk_bs(blk)->throttle_state,
> > - ThrottleGroup, ts);
> > + BlockBackendPublic *blkp = blk_get_public(blk);
> > + ThrottleGroup *tg = container_of(blkp->throttle_state, ThrottleGroup, ts);
> > BlockBackend *token, *start;
> >
> > start = token = tg->tokens[is_write];
> >
> > /* get next bs round in round robin style */
> > token = throttle_group_next_blk(token);
> > - while (token != start && !blk_bs(token)->pending_reqs[is_write]) {
> > + while (token != start && !blkp->pending_reqs[is_write]) {
> > token = throttle_group_next_blk(token);
> > }
> >
> >
> > blkp isn't updated every time token is updated.
>
> You're right, I'll write a patch. I'd also try to check why this was not
> detected by any iotest.
>
> Thanks!
Thanks a lot, Berto! (Both for fixing my bug and thinking of test cases)
Kevin
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2016-10-17 11:18 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2016-10-14 14:11 [Qemu-devel] bug introduced by "block: Move throttling fields from BDS to BB" Paolo Bonzini
2016-10-14 19:29 ` Paolo Bonzini
2016-10-17 8:49 ` Alberto Garcia
2016-10-17 11:18 ` Kevin Wolf
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).