Hello, Vivek. On Fri, Apr 27, 2012 at 03:46:54PM -0400, Vivek Goyal wrote: > On Thu, Apr 26, 2012 at 02:59:21PM -0700, Tejun Heo wrote: > > [..] > > @@ -926,6 +936,7 @@ static struct request *__get_request(struct request_list *rl, int rw_flags, > > goto fail_alloc; > > > > blk_rq_init(q, rq); > > + blk_rq_set_rl(rq, rl); > > Given the fact that we have established the rq and blkg releation at > the time of allocation, should we modify CFQ to just use that relation > instread of trying to lookup group again based on bio. Maybe, given the lookup cache it shouldn't really matter tho. > We avoid one lookup also we avoid duplicate creation of blkg in following > corner case of bio==NULL > > - blkg_get_rl() > - request allocation fails. sleep, drop queue lock > - process is moved to a different cgroup. origincal cgroup is > deleted. pre_destroy will cleanup all blkg on blkcg. > - process wakes up, request allocated, set_request sets up new blkg > based on new cgroup. Now a request is queued in one blkg/cgroup and > it has come out of the quota of other blkg/cgroup. I don't think it really matters as long as the request gets freed to the right queue on completion. > Well, I have a question. Ideally nobody should be linking any more blkg > to a blkcg once blkg_pre_destroy() has been called? But can it happen > that bio_associate_current() takes are reference to blkcg and bio is > throttled. cgroup associated with bio is deleted resulting in > pre_destroy(). Now bio is submitted to CFQ and it will try to create > a new blkg for blkcg-queue pair and once IO is complete, bio will drop > blkcg reference, which in turn will free up blkcg and associated blkg > is still around and will not be cleaned up. > > IOW, looks like we need a mechanism to mark a blkcg dead (set in > pre_destroy() call) and any submissions to blkcg after that should result > in bio being divered to root group? Don't we already have that with css_tryget()? Thanks. -- tejun _______________________________________________ Containers mailing list Containers@xxxxxxxxxxxxxxxxxxxxxxxxxx https://lists.linuxfoundation.org/mailman/listinfo/containers
