mbox series

[-next,v3,0/3] blk-cgroup: make sure pd_free_fn() is called in order

Message ID 20230119110350.2287325-1-yukuai1@huaweicloud.com
Headers
Series blk-cgroup: make sure pd_free_fn() is called in order |

Message

Yu Kuai Jan. 19, 2023, 11:03 a.m. UTC 
  From: Yu Kuai <yukuai3@huawei.com>

Changes in v3:
 - add ack tag from Tejun for patch 1,2
 - as suggested by Tejun, update commit message and comments in patch 3

The problem was found in iocost orignally([1]) that ioc can be freed in
ioc_pd_free(). And later we found that there are more problem in
iocost([2]).

After some discussion, as suggested by Tejun([3]), we decide to fix the
problem that parent pd can be freed before child pd in cgroup layer
first. And the problem in [1] will be fixed later if this patchset is
applied.

[1] https://lore.kernel.org/all/20221130132156.2836184-8-linan122@huawei.com/
[2] https://lore.kernel.org/all/aa924294-2f54-1b53-fc6e-e4f8fa019b14@huaweicloud.com/
[3] https://lore.kernel.org/all/20221227125502.541931-1-yukuai1@huaweicloud.com/

Yu Kuai (3):
  blk-cgroup: dropping parent refcount after pd_free_fn() is done
  blk-cgroup: support to track if policy is online
  blk-cgroup: synchronize pd_free_fn() from blkg_free_workfn() and
    blkcg_deactivate_policy()

 block/blk-cgroup.c     | 63 ++++++++++++++++++++++++++++++++----------
 block/blk-cgroup.h     |  1 +
 include/linux/blkdev.h |  1 +
 3 files changed, 50 insertions(+), 15 deletions(-)

Comments

Jens Axboe Jan. 19, 2023, 6:54 p.m. UTC | #1 
On 1/19/23 4:03 AM, Yu Kuai wrote:
> From: Yu Kuai <yukuai3@huawei.com>
> 
> Changes in v3:
>  - add ack tag from Tejun for patch 1,2
>  - as suggested by Tejun, update commit message and comments in patch 3
> 
> The problem was found in iocost orignally([1]) that ioc can be freed in
> ioc_pd_free(). And later we found that there are more problem in
> iocost([2]).
> 
> After some discussion, as suggested by Tejun([3]), we decide to fix the
> problem that parent pd can be freed before child pd in cgroup layer
> first. And the problem in [1] will be fixed later if this patchset is
> applied.

Doesn't apply against for-6.3/block (or linux-next or my for-next, for
that matter). Can you resend a tested one against for-6.3/block?
Yu Kuai Jan. 29, 2023, 6:06 a.m. UTC | #2 
Hi, Jens

在 2023/01/20 2:54, Jens Axboe 写道:
> On 1/19/23 4:03 AM, Yu Kuai wrote:
>> From: Yu Kuai <yukuai3@huawei.com>
>>
>> Changes in v3:
>>   - add ack tag from Tejun for patch 1,2
>>   - as suggested by Tejun, update commit message and comments in patch 3
>>
>> The problem was found in iocost orignally([1]) that ioc can be freed in
>> ioc_pd_free(). And later we found that there are more problem in
>> iocost([2]).
>>
>> After some discussion, as suggested by Tejun([3]), we decide to fix the
>> problem that parent pd can be freed before child pd in cgroup layer
>> first. And the problem in [1] will be fixed later if this patchset is
>> applied.
> 
> Doesn't apply against for-6.3/block (or linux-next or my for-next, for
> that matter). Can you resend a tested one against for-6.3/block?
> 

This is weird, I just test latest linux-next, and I can apply this
patchset on the top of following commit:

For latest for-6.3/block, this patch 2 can't be applied because
following commit is not here:

e3ff8887e7db blk-cgroup: fix missing pd_online_fn() while activating policy

But this patch is already merged into 6.2-rc5.

Thanks,
Kuai
Jens Axboe Jan. 29, 2023, 9:48 p.m. UTC | #3 
On 1/28/23 11:06 PM, Yu Kuai wrote:
> Hi, Jens
> 
> 在 2023/01/20 2:54, Jens Axboe 写道:
>> On 1/19/23 4:03 AM, Yu Kuai wrote:
>>> From: Yu Kuai <yukuai3@huawei.com>
>>>
>>> Changes in v3:
>>>   - add ack tag from Tejun for patch 1,2
>>>   - as suggested by Tejun, update commit message and comments in patch 3
>>>
>>> The problem was found in iocost orignally([1]) that ioc can be freed in
>>> ioc_pd_free(). And later we found that there are more problem in
>>> iocost([2]).
>>>
>>> After some discussion, as suggested by Tejun([3]), we decide to fix the
>>> problem that parent pd can be freed before child pd in cgroup layer
>>> first. And the problem in [1] will be fixed later if this patchset is
>>> applied.
>>
>> Doesn't apply against for-6.3/block (or linux-next or my for-next, for
>> that matter). Can you resend a tested one against for-6.3/block?
>>
> 
> This is weird, I just test latest linux-next, and I can apply this
> patchset on the top of following commit:
> 
> For latest for-6.3/block, this patch 2 can't be applied because
> following commit is not here:
> 
> e3ff8887e7db blk-cgroup: fix missing pd_online_fn() while activating policy
> 
> But this patch is already merged into 6.2-rc5.

Since I have one more conflict, I think we'll just rebase for-6.3/block
when -rc6 is out, and then it should apply cleanly.
Jens Axboe Jan. 29, 2023, 10:20 p.m. UTC | #4 
On Thu, 19 Jan 2023 19:03:47 +0800, Yu Kuai wrote:
> Changes in v3:
>  - add ack tag from Tejun for patch 1,2
>  - as suggested by Tejun, update commit message and comments in patch 3
> 
> The problem was found in iocost orignally([1]) that ioc can be freed in
> ioc_pd_free(). And later we found that there are more problem in
> iocost([2]).
> 
> [...]

Applied, thanks!

[1/3] blk-cgroup: dropping parent refcount after pd_free_fn() is done
      commit: c7241babf0855d8a6180cd1743ff0ec34de40b4e
[2/3] blk-cgroup: support to track if policy is online
      commit: dfd6200a095440b663099d8d42f1efb0175a1ce3
[3/3] blk-cgroup: synchronize pd_free_fn() from blkg_free_workfn() and blkcg_deactivate_policy()
      commit: f1c006f1c6850c14040f8337753a63119bba39b9

Best regards,