Messages in this thread | | | Date | Thu, 19 Jan 2012 11:16:42 -0500 | From | Vivek Goyal <> | Subject | Re: [PATCH 06/12] blkcg: use q and plid instead of opaque void * for blkio_group association |
| |
On Thu, Jan 19, 2012 at 07:55:45AM -0800, Tejun Heo wrote:
[..] > > > +static void cfq_unlink_blkio_group(struct request_queue *q, > > > + struct blkio_group *blkg) > > > { > > > - unsigned long flags; > > > - struct cfq_data *cfqd = key; > > > + struct cfq_data *cfqd = q->elevator->elevator_data; > > > + unsigned long flags; > > > > > > - spin_lock_irqsave(cfqd->queue->queue_lock, flags); > > > + spin_lock_irqsave(q->queue_lock, flags); > > > cfq_destroy_cfqg(cfqd, cfqg_of_blkg(blkg)); > > > - spin_unlock_irqrestore(cfqd->queue->queue_lock, flags); > > > + spin_unlock_irqrestore(q->queue_lock, flags); > > > > I think this code will create problem where both old elevator group and > > new elevator group is on blkcg list and upon cgroup removal one can not > > rely that q->elevator->elevator_data will give us old elevator's cfqd. > > Again, if I didn't botch up earlier elevator switch code, it shouldn't.
I think I am missing something. IIUC, following is new elevator switch sequence.
1. elv_quiesce_start 2. unregister old elevator 3. ioc_clear_queue 4. allocate new elevator 5. init new elevator 6. exit old elevator
So any groups on old elevator, will be cleaned up in step 6. So till step 5 these groups are still present on blkcg list. Now assume between step 5 and step 6, if a cgroup removal takes place and blkcg tries to call into elevator to remove that group, will it not be accessing the wrong cfqd in cfq_destroy_cfqg() (cfqd of new elevator instead of old elevator).
What am I missing?
Thanks Vivek
| |