Re: [PATCH] cfq-iosched: Fix wrong children_weight calculation

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Tue, Aug 26, 2014 at 08:56:36PM +0900, Toshiaki Makita wrote:
> cfq_group_service_tree_add() is applying new_weight at the beginning of
> the function via cfq_update_group_weight().
> This actually allows weight to change between adding it to and subtracting
> it from children_weight, and triggers WARN_ON_ONCE() in
> cfq_group_service_tree_del(), or even causes oops by divide error during
> vfr calculation in cfq_group_service_tree_add().
> 
> The detailed scenario is as follows:
> 1. Create blkio cgroups X and Y as a child of X.

This is a bit confusing.  Maybe use a different letter for the parent
X?

>    Set X's weight to 500 and perform some I/O to apply new_weight.
>    This X's I/O completes before starting Y's I/O.
> 2. Y starts I/O and cfq_group_service_tree_add() is called with Y.
> 3. cfq_group_service_tree_add() walks up the tree during children_weight
>    calculation and adds parent X's weight (500) to children_weight of root.
>    children_weight becomes 500.
> 4. Set X's weight to 1000.
> 5. X starts I/O and cfq_group_service_tree_add() is called with X.
> 6. cfq_group_service_tree_add() applies its new_weight (1000).
> 7. I/O of Y completes and cfq_group_service_tree_del() is called with Y.
> 8. I/O of X completes and cfq_group_service_tree_del() is called with X.
> 9. cfq_group_service_tree_del() subtracts X's weight (1000) from
>    children_weight of root. children_weight becomes -500.
>    This triggers WARN_ON_ONCE().
> 10. Set X's weight to 500.
> 11. X starts I/O and cfq_group_service_tree_add() is called with X.
> 12. cfq_group_service_tree_add() applies its new_weight (500) and adds it
>     to children_weight of root. children_weight becomes 0. Calcularion of
>     vfr triggers oops by divide error.
> 
> weight should be updated right before adding it to children_weight.
> 
> Reported-by: Ruki Sekiya <sekiya.ruki@xxxxxxxxxxxxx>
> Signed-off-by: Toshiaki Makita <makita.toshiaki@xxxxxxxxxxxxx>

Acked-by: Tejun Heo <tj@xxxxxxxxxx>

Comment in the code explaining that new_weight application has to
happen only on activation would be nice tho.

Thanks.

-- 
tejun
--
To unsubscribe from this list: send the line "unsubscribe cgroups" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html




[Index of Archives]     [Linux ARM Kernel]     [Linux ARM]     [Linux Omap]     [Fedora ARM]     [IETF Annouce]     [Security]     [Bugtraq]     [Linux OMAP]     [Linux MIPS]     [eCos]     [Asterisk Internet PBX]     [Linux API]     [Monitors]

  Powered by Linux