Messages in this thread |  | | | Date | Thu, 25 Mar 2010 17:17:34 -0700 | | Subject | Re: [PATCH 1/4] blkio_group key change: void * -> request_queue * | | From | Chad Talbott <> |
| |
On Thu, Mar 25, 2010 at 4:25 PM, Vivek Goyal <vgoyal@redhat.com> wrote: > human-readable disk names probably are more convenient. I got few general > concerns.
Thanks for the quick and detailed review.
> - Can we change the sysfs interface now. In 2.6.33 kernel we released the > code to export blkio.time and blkio.sectors using device numbers. We > shall have to change those interfaces also to reflect device stats in > terms of device names.
I wasn't aware of those interface, but I'm all for consistency. I'll look into it.
> - I had kept void* as the key in blkg object so that it does not make any > assumption about the key. This allowed that any xyz code in kernel can > register with blkio code and implement some kind of IO control policy > and it does not have to instanciate a request queue. > > But I am not sure if there will be any subsystems which will really do > that. I am assuming that all the device mapper and md code do > instanciate the request queue? (In case we implement max bw policy > there).
I understand your preference to keep the key generic, but I wonder if there is *any* device that won't have a request_queue? The penalty of keeping it very generic might be the complexity of another auxiliary structure to lookup gendisks or the next thing. I wish the device mapper folks would speak up and be involved in these discussions.
> - How do you make sure that request queue does not go away and while > somebody is accessing blkg->queue under rcu read lock? This can happen > while we call unlink code.
Here I'll admit to being very new to RCU, and I'll defer to your worries. More homework needed on my part.
> blkio_unlink_group_fn(). > > Because we store the pointer to cfqd as key, we make sure cfqd does not > get deleted before one rcu grace period. (call_rcu). But this gurantees > only that cfqd object is around and not cfqd->queue. So this is a > problem with even today's code. > > ... > - How do you make sure that gendisk does not go away while q->disk is > being accessed under rcu lock. (Already asked in other thread too.)
On locking between gendisk deletion and stats printing: I suppose that you've already considered and discarded using the queue_lock to protect a gendisk pointer in request_queue?
> These are some quick thoughs. More later...
Thanks again! Chad -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
|  |