linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Tejun Heo <tj@kernel.org>
To: Vivek Goyal <vgoyal@redhat.com>
Cc: axboe@kernel.dk, hughd@google.com, avi@redhat.com,
	nate@cpanel.net, cl@linux-foundation.org,
	linux-kernel@vger.kernel.org, dpshah@google.com,
	ctalbott@google.com, rni@google.com,
	Andrew Morton <akpm@linux-foundation.org>
Subject: Re: [PATCHSET] mempool, percpu, blkcg: fix percpu stat allocation and remove stats_lock
Date: Mon, 5 Mar 2012 09:20:00 -0800	[thread overview]
Message-ID: <20120305172000.GQ22536@google.com> (raw)
In-Reply-To: <20120229190354.GC5930@redhat.com>

Hello, Vivek.

Sorry about the delay.  Was off last week.

On Wed, Feb 29, 2012 at 02:03:54PM -0500, Vivek Goyal wrote:
> Ok, more debugging revealed that culprit here is "floppy" driver module.
> It instanciates a queue but never registers a disk against it. Better
> skip such queues/groups. Here is the patch.

Urgh.. I hate that driver.  I ended up folding the following patch in
the series right after "blkcg: kill the mind-bending blkg->dev".

Thanks.

Subject: blkcg: skip blkg printing if q isn't associated with disk

blk-cgroup printing code currently assumes that there is a device/disk
associated with every queue in the system, but modules like floppy,
can instantiate request queues without registering disk which can lead
to oops.

Skip the queue/blkg which don't have dev/disk associated with them.

-tj: Factored out backing_dev_info check into blkg_dev_name().

Signed-off-by: Vivek Goyal <vgoyal@redhat.com>
Signed-off-by: Tejun Heo <tj@kernel.org>
---
 block/blk-cgroup.c |   17 ++++++++++++++---
 1 file changed, 14 insertions(+), 3 deletions(-)

Index: work/block/blk-cgroup.c
===================================================================
--- work.orig/block/blk-cgroup.c
+++ work/block/blk-cgroup.c
@@ -951,13 +951,24 @@ static int blkiocg_file_write(struct cgr
 	return ret;
 }
 
+static const char *blkg_dev_name(struct blkio_group *blkg)
+{
+	/* some drivers (floppy) instantiate a queue w/o disk registered */
+	if (blkg->q->backing_dev_info.dev)
+		return dev_name(blkg->q->backing_dev_info.dev);
+	return NULL;
+}
+
 static void blkio_print_group_conf(struct cftype *cft, struct blkio_group *blkg,
 				   struct seq_file *m)
 {
-	const char *dname = dev_name(blkg->q->backing_dev_info.dev);
+	const char *dname = blkg_dev_name(blkg);
 	int fileid = BLKIOFILE_ATTR(cft->private);
 	int rw = WRITE;
 
+	if (!dname)
+		return;
+
 	switch (blkg->plid) {
 		case BLKIO_POLICY_PROP:
 			if (blkg->conf.weight)
@@ -1049,9 +1060,9 @@ static int blkio_read_blkg_stats(struct 
 
 	rcu_read_lock();
 	hlist_for_each_entry_rcu(blkg, n, &blkcg->blkg_list, blkcg_node) {
-		const char *dname = dev_name(blkg->q->backing_dev_info.dev);
+		const char *dname = blkg_dev_name(blkg);
 
-		if (BLKIOFILE_POLICY(cft->private) != blkg->plid)
+		if (!dname || BLKIOFILE_POLICY(cft->private) != blkg->plid)
 			continue;
 		if (pcpu)
 			cgroup_total += blkio_get_stat_cpu(blkg, cb, dname,

  reply	other threads:[~2012-03-05 17:20 UTC|newest]

Thread overview: 56+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2012-02-23 22:30 [PATCHSET] mempool, percpu, blkcg: fix percpu stat allocation and remove stats_lock Tejun Heo
2012-02-23 22:30 ` [PATCH 1/8] mempool: factor out mempool_fill() Tejun Heo
2012-02-23 22:30 ` [PATCH 2/8] mempool: separate out __mempool_create() Tejun Heo
2012-02-23 22:30 ` [PATCH 3/8] mempool, percpu: implement percpu mempool Tejun Heo
2012-02-23 22:30 ` [PATCH 4/8] block: fix deadlock through percpu allocation in blk-cgroup Tejun Heo
2012-02-23 22:30 ` [PATCH 5/8] blkcg: don't use percpu for merged stats Tejun Heo
2012-02-23 22:30 ` [PATCH 6/8] blkcg: simplify stat reset Tejun Heo
2012-02-23 22:30 ` [PATCH 7/8] blkcg: restructure blkio_get_stat() Tejun Heo
2012-02-23 22:30 ` [PATCH 8/8] blkcg: remove blkio_group->stats_lock Tejun Heo
2012-02-23 22:43 ` [PATCHSET] mempool, percpu, blkcg: fix percpu stat allocation and remove stats_lock Andrew Morton
2012-02-23 23:01   ` Tejun Heo
2012-02-23 23:12     ` Tejun Heo
2012-02-23 23:22       ` Andrew Morton
2012-02-23 23:24         ` Tejun Heo
2012-02-24 14:20       ` Vivek Goyal
2012-02-25 21:44         ` Tejun Heo
2012-02-27  3:11           ` Vivek Goyal
2012-02-27  9:11             ` Tejun Heo
2012-02-27 19:43               ` Vivek Goyal
2012-02-29 17:36                 ` Vivek Goyal
2012-03-05 22:13                   ` Tejun Heo
2012-03-06 21:09                     ` Vivek Goyal
2012-03-06 21:20                       ` Andrew Morton
2012-03-06 21:34                         ` Vivek Goyal
2012-03-06 21:55                           ` Andrew Morton
2012-03-07 14:55                             ` Vivek Goyal
2012-03-07 17:05                               ` Tejun Heo
2012-03-07 19:13                                 ` Vivek Goyal
2012-03-07 19:22                                   ` Tejun Heo
2012-03-07 19:42                                     ` Vivek Goyal
2012-03-07 22:56                                       ` Tejun Heo
2012-03-07 23:08                                         ` Andrew Morton
2012-03-07 23:15                                           ` Tejun Heo
2012-03-07 23:05                               ` Andrew Morton
2012-03-08 17:57                                 ` Vivek Goyal
2012-03-08 18:08                                   ` Tejun Heo
2012-03-08 18:11                                     ` Tejun Heo
2012-03-08 18:22                                       ` Vivek Goyal
2012-03-08 18:27                                         ` Tejun Heo
2012-03-15 16:48                                           ` Vivek Goyal
2012-03-15 16:59                                             ` Tejun Heo
2012-03-20 11:50                                               ` Jens Axboe
2012-03-08 20:16                                     ` Vivek Goyal
2012-03-08 20:33                                       ` Vivek Goyal
2012-03-08 20:35                                         ` Tejun Heo
2012-03-08 19:06                                   ` Andrew Morton
2012-02-25  3:44       ` Vivek Goyal
2012-02-25 21:46         ` Tejun Heo
2012-02-25 22:21           ` Tejun Heo
2012-02-27 14:25             ` Vivek Goyal
2012-02-27 14:40               ` Vivek Goyal
2012-03-05 17:45                 ` Tejun Heo
2012-02-27 18:22       ` Vivek Goyal
2012-02-29 19:03         ` Vivek Goyal
2012-03-05 17:20           ` Tejun Heo [this message]
2012-03-05 18:03             ` Vivek Goyal

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=20120305172000.GQ22536@google.com \
    --to=tj@kernel.org \
    --cc=akpm@linux-foundation.org \
    --cc=avi@redhat.com \
    --cc=axboe@kernel.dk \
    --cc=cl@linux-foundation.org \
    --cc=ctalbott@google.com \
    --cc=dpshah@google.com \
    --cc=hughd@google.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=nate@cpanel.net \
    --cc=rni@google.com \
    --cc=vgoyal@redhat.com \
    /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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).