On Fri, Aug 26, 2005 at 03:45:10PM +0200, Jens Axboe wrote:
> On Fri, Aug 26 2005, Christoph Hellwig wrote:
> > We're trying to get rid of as much as possible tasklist walks, or at
> > least moving them to core code. This patch falls into the second
> > category.
> >
> > Instead of walking the tasklist in cfq-iosched move that into
> > elv_unregister. The added benefit is that with this change the as
> > ioscheduler might be might unloadable more easily aswell.
> >
> > The new code uses read_lock instead of read_lock_irq because the
> > tasklist_lock only needs irq disabling for writers.
>
> Looks innocent enough, fine with me. 'as' will need additional work to
> be unloadable, but it wont break anything since it's running with an
> elevated module count right now anyways.
any chance to send this on to akpm?
Signed-off-by: Christoph Hellwig <[email protected]>
Index: linux-2.6/drivers/block/as-iosched.c
===================================================================
--- linux-2.6.orig/drivers/block/as-iosched.c 2005-08-11 16:45:55.000000000 +0200
+++ linux-2.6/drivers/block/as-iosched.c 2005-08-14 12:13:08.000000000 +0200
@@ -2119,8 +2119,8 @@
static void __exit as_exit(void)
{
- kmem_cache_destroy(arq_pool);
elv_unregister(&iosched_as);
+ kmem_cache_destroy(arq_pool);
}
module_init(as_init);
Index: linux-2.6/drivers/block/cfq-iosched.c
===================================================================
--- linux-2.6.orig/drivers/block/cfq-iosched.c 2005-08-11 16:45:55.000000000 +0200
+++ linux-2.6/drivers/block/cfq-iosched.c 2005-08-14 12:09:17.000000000 +0200
@@ -2609,28 +2609,8 @@
static void __exit cfq_exit(void)
{
- struct task_struct *g, *p;
- unsigned long flags;
-
- read_lock_irqsave(&tasklist_lock, flags);
-
- /*
- * iterate each process in the system, removing our io_context
- */
- do_each_thread(g, p) {
- struct io_context *ioc = p->io_context;
-
- if (ioc && ioc->cic) {
- ioc->cic->exit(ioc->cic);
- cfq_free_io_context(ioc->cic);
- ioc->cic = NULL;
- }
- } while_each_thread(g, p);
-
- read_unlock_irqrestore(&tasklist_lock, flags);
-
- cfq_slab_kill();
elv_unregister(&iosched_cfq);
+ cfq_slab_kill();
}
module_init(cfq_init);
Index: linux-2.6/drivers/block/elevator.c
===================================================================
--- linux-2.6.orig/drivers/block/elevator.c 2005-08-11 16:45:55.000000000 +0200
+++ linux-2.6/drivers/block/elevator.c 2005-08-14 12:12:35.000000000 +0200
@@ -572,6 +572,27 @@
void elv_unregister(struct elevator_type *e)
{
+ struct task_struct *g, *p;
+
+ /*
+ * Iterate every thread in the process to remove the io contexts.
+ */
+ read_lock(&tasklist_lock);
+ do_each_thread(g, p) {
+ struct io_context *ioc = p->io_context;
+ if (ioc && ioc->cic) {
+ ioc->cic->exit(ioc->cic);
+ ioc->cic->dtor(ioc->cic);
+ ioc->cic = NULL;
+ }
+ if (ioc && ioc->aic) {
+ ioc->aic->exit(ioc->aic);
+ ioc->aic->dtor(ioc->aic);
+ ioc->aic = NULL;
+ }
+ } while_each_thread(g, p);
+ read_unlock(&tasklist_lock);
+
spin_lock_irq(&elv_list_lock);
list_del_init(&e->list);
spin_unlock_irq(&elv_list_lock);
On Mon, Sep 19 2005, Christoph Hellwig wrote:
> On Fri, Aug 26, 2005 at 03:45:10PM +0200, Jens Axboe wrote:
> > On Fri, Aug 26 2005, Christoph Hellwig wrote:
> > > We're trying to get rid of as much as possible tasklist walks, or at
> > > least moving them to core code. This patch falls into the second
> > > category.
> > >
> > > Instead of walking the tasklist in cfq-iosched move that into
> > > elv_unregister. The added benefit is that with this change the as
> > > ioscheduler might be might unloadable more easily aswell.
> > >
> > > The new code uses read_lock instead of read_lock_irq because the
> > > tasklist_lock only needs irq disabling for writers.
> >
> > Looks innocent enough, fine with me. 'as' will need additional work to
> > be unloadable, but it wont break anything since it's running with an
> > elevated module count right now anyways.
>
> any chance to send this on to akpm?
Sure, already ack'ed it and I thought akpm was cc'ed on the mail.
Anyways, andrew can you add this? Thanks!
Signed-off-by: Jens Axboe <[email protected]>
> Signed-off-by: Christoph Hellwig <[email protected]>
>
> Index: linux-2.6/drivers/block/as-iosched.c
> ===================================================================
> --- linux-2.6.orig/drivers/block/as-iosched.c 2005-08-11 16:45:55.000000000 +0200
> +++ linux-2.6/drivers/block/as-iosched.c 2005-08-14 12:13:08.000000000 +0200
> @@ -2119,8 +2119,8 @@
>
> static void __exit as_exit(void)
> {
> - kmem_cache_destroy(arq_pool);
> elv_unregister(&iosched_as);
> + kmem_cache_destroy(arq_pool);
> }
>
> module_init(as_init);
> Index: linux-2.6/drivers/block/cfq-iosched.c
> ===================================================================
> --- linux-2.6.orig/drivers/block/cfq-iosched.c 2005-08-11 16:45:55.000000000 +0200
> +++ linux-2.6/drivers/block/cfq-iosched.c 2005-08-14 12:09:17.000000000 +0200
> @@ -2609,28 +2609,8 @@
>
> static void __exit cfq_exit(void)
> {
> - struct task_struct *g, *p;
> - unsigned long flags;
> -
> - read_lock_irqsave(&tasklist_lock, flags);
> -
> - /*
> - * iterate each process in the system, removing our io_context
> - */
> - do_each_thread(g, p) {
> - struct io_context *ioc = p->io_context;
> -
> - if (ioc && ioc->cic) {
> - ioc->cic->exit(ioc->cic);
> - cfq_free_io_context(ioc->cic);
> - ioc->cic = NULL;
> - }
> - } while_each_thread(g, p);
> -
> - read_unlock_irqrestore(&tasklist_lock, flags);
> -
> - cfq_slab_kill();
> elv_unregister(&iosched_cfq);
> + cfq_slab_kill();
> }
>
> module_init(cfq_init);
> Index: linux-2.6/drivers/block/elevator.c
> ===================================================================
> --- linux-2.6.orig/drivers/block/elevator.c 2005-08-11 16:45:55.000000000 +0200
> +++ linux-2.6/drivers/block/elevator.c 2005-08-14 12:12:35.000000000 +0200
> @@ -572,6 +572,27 @@
>
> void elv_unregister(struct elevator_type *e)
> {
> + struct task_struct *g, *p;
> +
> + /*
> + * Iterate every thread in the process to remove the io contexts.
> + */
> + read_lock(&tasklist_lock);
> + do_each_thread(g, p) {
> + struct io_context *ioc = p->io_context;
> + if (ioc && ioc->cic) {
> + ioc->cic->exit(ioc->cic);
> + ioc->cic->dtor(ioc->cic);
> + ioc->cic = NULL;
> + }
> + if (ioc && ioc->aic) {
> + ioc->aic->exit(ioc->aic);
> + ioc->aic->dtor(ioc->aic);
> + ioc->aic = NULL;
> + }
> + } while_each_thread(g, p);
> + read_unlock(&tasklist_lock);
> +
> spin_lock_irq(&elv_list_lock);
> list_del_init(&e->list);
> spin_unlock_irq(&elv_list_lock);
>
--
Jens Axboe