Message ID | 20221217030527.1250083-2-yukuai1@huaweicloud.com (mailing list archive) |
---|---|
State | New, archived |
Headers | show |
Series | blk-iocost: make sure parent iocg is exited before child | expand |
On Sat, Dec 17, 2022 at 11:05:24AM +0800, Yu Kuai wrote: > @@ -459,6 +459,8 @@ struct ioc_gq { > struct blkg_policy_data pd; > struct ioc *ioc; > > + bool online; Nit: maybe tab align this field like the fields above it. > +static void ioc_pd_offline(struct blkg_policy_data *pd) > +{ > + struct ioc_gq *iocg = pd_to_iocg(pd); > + struct ioc *ioc = iocg->ioc; > + unsigned long flags; > + > + if (ioc) { How could ioc be NULL here?
Hi, Christoph 在 2022/12/21 18:33, Christoph Hellwig 写道: > On Sat, Dec 17, 2022 at 11:05:24AM +0800, Yu Kuai wrote: >> @@ -459,6 +459,8 @@ struct ioc_gq { >> struct blkg_policy_data pd; >> struct ioc *ioc; >> >> + bool online; > > Nit: maybe tab align this field like the fields above it. > >> +static void ioc_pd_offline(struct blkg_policy_data *pd) >> +{ >> + struct ioc_gq *iocg = pd_to_iocg(pd); >> + struct ioc *ioc = iocg->ioc; >> + unsigned long flags; >> + >> + if (ioc) { > > How could ioc be NULL here? > As I explained in another thread.. pd_offline_fn() can be called without pd_init_fn(), which is a bug from upper layer... blkcg_activate_policy spin_lock_irq(&q->queue_lock); list_for_each_entry_reverse(blkg, &q->blkg_list pd_alloc_fn(GFP_NOWAIT | __GFP_NOWARN,...) -> failed spin_unlock_irq(&q->queue_lock); // release queue_lock here is problematic, this will cause pd_offline_fn called without pd_init_fn. pd_alloc_fn(__GFP_NOWARN,...) Thanks, Kuai > . >
diff --git a/block/blk-iocost.c b/block/blk-iocost.c index 3aeb8538f0c3..1498879c4a52 100644 --- a/block/blk-iocost.c +++ b/block/blk-iocost.c @@ -459,6 +459,8 @@ struct ioc_gq { struct blkg_policy_data pd; struct ioc *ioc; + bool online; + /* * A iocg can get its weight from two sources - an explicit * per-device-cgroup configuration or the default weight of the @@ -2952,6 +2954,7 @@ static void ioc_pd_init(struct blkg_policy_data *pd) ioc_now(ioc, &now); iocg->ioc = ioc; + iocg->online = true; atomic64_set(&iocg->vtime, now.vnow); atomic64_set(&iocg->done_vtime, now.vnow); atomic64_set(&iocg->active_period, atomic64_read(&ioc->cur_period)); @@ -2977,6 +2980,19 @@ static void ioc_pd_init(struct blkg_policy_data *pd) spin_unlock_irqrestore(&ioc->lock, flags); } +static void ioc_pd_offline(struct blkg_policy_data *pd) +{ + struct ioc_gq *iocg = pd_to_iocg(pd); + struct ioc *ioc = iocg->ioc; + unsigned long flags; + + if (ioc) { + spin_lock_irqsave(&ioc->lock, flags); + iocg->online = false; + spin_unlock_irqrestore(&ioc->lock, flags); + } +} + static void ioc_pd_free(struct blkg_policy_data *pd) { struct ioc_gq *iocg = pd_to_iocg(pd); @@ -3467,6 +3483,7 @@ static struct blkcg_policy blkcg_policy_iocost = { .cpd_free_fn = ioc_cpd_free, .pd_alloc_fn = ioc_pd_alloc, .pd_init_fn = ioc_pd_init, + .pd_offline_fn = ioc_pd_offline, .pd_free_fn = ioc_pd_free, .pd_stat_fn = ioc_pd_stat, };