lkml.org 
[lkml]   [2008]   [Nov]   [19]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
Patch in this message
/
Date
From
SubjectRe: [PATCH] Exiting queue and task might race to free cic
On Wed, Nov 19 2008, Nikanth Karthikesan wrote:
> Hi Jens
>
> Looking at the bug reported here
> http://thread.gmane.org/gmane.linux.kernel/722539
> it looks like an exiting queue can race with an exiting task.
>
> When a queue exits the queue lock is taken and cfq_exit_queue() would free all
> the cic's associated with the queue.
>
> But when a task exits, cfq_exit_io_context() gets cic one by one and then
> locks the associated queue to call __cfq_exit_single_io_context. It looks like
> between getting a cic from the ioc and locking the queue, the queue might have
> exited on another cpu. Isn't this possible?
>
> If possible, either verifying whether cic->key is still not null or q->flags
> does not have QUEUE_FLAG_DEAD set would fix this.
>
> Thanks
> Nikanth Karthikesan
>
> Signed-off-by: Nikanth Karthikesan <knikanth@suse.de>
>
> ---
> diff --git a/block/cfq-iosched.c b/block/cfq-iosched.c
> index 6a062ee..b9b627a 100644
> --- a/block/cfq-iosched.c
> +++ b/block/cfq-iosched.c
> @@ -1318,7 +1318,12 @@ static void cfq_exit_single_io_context(struct
> io_context *ioc,
> unsigned long flags;
>
> spin_lock_irqsave(q->queue_lock, flags);
> - __cfq_exit_single_io_context(cfqd, cic);
> + /*
> + * cic might have been already exited when an exiting task
> + * races with an exiting queue.
> + */
> + if (likely(cic->key))
> + __cfq_exit_single_io_context(cfqd, cic);
> spin_unlock_irqrestore(q->queue_lock, flags);
> }
> }

Not sure this is enough, we probably need to copy the key to ensure that
we get a fresh value. How does this look?

Did you actually trigger this, or is it just from code inspection?

diff --git a/block/cfq-iosched.c b/block/cfq-iosched.c
index 6a062ee..560cd1c 100644
--- a/block/cfq-iosched.c
+++ b/block/cfq-iosched.c
@@ -1318,7 +1318,14 @@ static void cfq_exit_single_io_context(struct io_context *ioc,
unsigned long flags;

spin_lock_irqsave(q->queue_lock, flags);
- __cfq_exit_single_io_context(cfqd, cic);
+
+ /*
+ * Ensure we get a fresh copy of the ->key to prevent
+ * race between exiting task and queue
+ */
+ smp_read_barrier_depends();
+ if (cic->key)
+ __cfq_exit_single_io_context(cfqd, cic);
spin_unlock_irqrestore(q->queue_lock, flags);
}
}
--
Jens Axboe



\
 
 \ /
  Last update: 2008-11-19 15:19    [W:0.048 / U:1.168 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site