Messages in this thread | | | Subject | Re: [PATCH -next v2 9/9] blk-iocost: fix walk_list corruption | From | Yu Kuai <> | Date | Thu, 1 Dec 2022 21:43:25 +0800 |
| |
在 2022/12/01 18:29, Tejun Heo 写道: > On Thu, Dec 01, 2022 at 06:14:32PM +0800, Yu Kuai wrote: >> Hi, >> >> 在 2022/12/01 18:00, Tejun Heo 写道: >>> On Thu, Dec 01, 2022 at 09:19:54AM +0800, Yu Kuai wrote: >>>>>> diff --git a/block/blk-iocost.c b/block/blk-iocost.c >>>>>> index 710cf63a1643..d2b873908f88 100644 >>>>>> --- a/block/blk-iocost.c >>>>>> +++ b/block/blk-iocost.c >>>>>> @@ -2813,13 +2813,14 @@ static void ioc_rqos_exit(struct rq_qos *rqos) >>>>>> { >>>>>> struct ioc *ioc = rqos_to_ioc(rqos); >>>>>> + del_timer_sync(&ioc->timer); >>>>>> + >>>>>> blkcg_deactivate_policy(rqos->q, &blkcg_policy_iocost); >>>>>> spin_lock_irq(&ioc->lock); >>>>>> ioc->running = IOC_STOP; >>>>>> spin_unlock_irq(&ioc->lock); >>>>>> - del_timer_sync(&ioc->timer); >>>>> >>>>> I don't about this workaround. Let's fix properly? >>>> >>>> Ok, and by the way, is there any reason to delete timer after >>>> deactivate policy? This seems a litter wreid to me. >>> >>> ioc->running is what controls whether the timer gets rescheduled or not. If >>> we don't shut that down, the timer may as well get rescheduled after being >>> deleted. Here, the only extra activation point is IO issue which shouldn't >>> trigger during rq_qos_exit, so the ordering shouldn't matter but this is the >>> right order for anything which can get restarted. >> >> Thanks for the explanation. >> >> I'm trying to figure out how to make sure child blkg pins it's parent, >> btw, do you think following cleanup make sense? > > It's on you to explain why any change that you're suggesting is better and > safe. I know it's not intentional but you're repeatedly suggesting operation > reorderings in code paths which are really sensitive to ordering at least > seemingly without putting much effort into thinking through the side > effects. This costs disproportionate amount of review bandwidth, and > increases the chance of new subtle bugs. Can you please slow down a bit and > be more deliberate?
Thanks for the suggestion, I'll pay close attention to explain this "why the change is better and safe". And sorry for the review pressure. 😔
> > Thanks. >
| |