Fine, I will do the corresponding changes and post v5. Thanks, Joseph On 18/3/14 04:19, Tejun Heo wrote: > Hello, Joseph. > > On Mon, Mar 05, 2018 at 11:03:16PM +0800, Joseph Qi wrote: >> +static void blkg_pd_offline(struct blkcg_gq *blkg) >> +{ >> + int i; >> + >> + lockdep_assert_held(blkg->q->queue_lock); >> + lockdep_assert_held(&blkg->blkcg->lock); >> + >> + for (i = 0; i < BLKCG_MAX_POLS; i++) { >> + struct blkcg_policy *pol = blkcg_policy[i]; >> + >> + if (blkg->pd[i] && !blkg->pd_offline[i] && pol->pd_offline_fn) { >> + pol->pd_offline_fn(blkg->pd[i]); >> + blkg->pd_offline[i] = true; > > Can we move this flag into blkg_policy_data? > >> + while (!hlist_empty(&blkcg->blkg_list)) { >> + struct blkcg_gq *blkg = hlist_entry(blkcg->blkg_list.first, >> + struct blkcg_gq, >> + blkcg_node); >> + struct request_queue *q = blkg->q; >> + >> + if (spin_trylock(q->queue_lock)) { >> + blkg_destroy(blkg); >> + spin_unlock(q->queue_lock); >> + } else { >> + spin_unlock_irq(&blkcg->lock); >> + cpu_relax(); >> + spin_lock_irq(&blkcg->lock); >> + } > > Can we factor out the above loop? It's something subtle and painful > and I think it'd be better to have it separated out and documented. > > Other than that, looks great. > > Thanks. > -- 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