All of lore.kernel.org
 help / color / mirror / Atom feed
From: Nikanth Karthikesan <knikanth@suse.de>
To: Jens Axboe <jens.axboe@oracle.com>
Cc: linux-kernel@vger.kernel.org, Fabio Checconi <fchecconi@gmail.com>
Subject: [PATCH] Exiting queue and task might race to free cic
Date: Wed, 19 Nov 2008 15:27:17 +0530	[thread overview]
Message-ID: <200811191527.18539.knikanth@suse.de> (raw)

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);
 	}
 }

Or this would also work

diff --git a/block/cfq-iosched.c b/block/cfq-iosched.c
index 6a062ee..7a068bd 100644
--- a/block/cfq-iosched.c
+++ b/block/cfq-iosched.c
@@ -1318,7 +1318,11 @@ 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);
+		/*
+		 * Make sure the queue is not dead.
+		 */
+		if (likely(!test_bit(QUEUE_FLAG_DEAD, &q->queue_flags)))
+			__cfq_exit_single_io_context(cfqd, cic);
 		spin_unlock_irqrestore(q->queue_lock, flags);
 	}
 }



             reply	other threads:[~2008-11-19  9:55 UTC|newest]

Thread overview: 5+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2008-11-19  9:57 Nikanth Karthikesan [this message]
2008-11-19 14:15 ` [PATCH] Exiting queue and task might race to free cic Jens Axboe
2008-11-19 15:02   ` Fabio Checconi
2008-11-20  4:57   ` Nikanth Karthikesan
2008-11-22  7:17     ` Nikanth Karthikesan

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=200811191527.18539.knikanth@suse.de \
    --to=knikanth@suse.de \
    --cc=fchecconi@gmail.com \
    --cc=jens.axboe@oracle.com \
    --cc=linux-kernel@vger.kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.